Re: [PATCH v2 0/2] StandaloneMM: Update permissions for Standalone MM drivers memory area

[Sughosh Ganu <sughosh.ganu@arm.com>]

hi Achin,

On Sat Dec 01, 2018 at 05:08:50AM +0530, Achin Gupta wrote:
> Hi Sughosh,
> 
> +Jiewen
> 
> I took the patches for a spin and it looks like the FVP port is broken. Some
> reasons are:
> 
> 1. The build breaks due to a reference to ArmMmuLib in StandaloneMmPkg.dsc
> 2. There is a broken dependency on PL011UartClockLib in StandaloneMmPkg.dsc
> 3. GCC flags to enforce strict alignment and no fp are required in
>    StandaloneMmPkg.dsc to avoid a runtime fault
> 4. There is a data structre in StandaloneMmCoreEntryPoint.c that needs to be
>    memzeroed due to the alignment checks
> 
> Even after these fixes, I am unable to boot the MM SP. The SP boots with the
> previous revision of your patches and the above fixes. Something has broken
> between the two. I am suspecting the MMU library for S-EL0.

I had tested the patches which i had sent out for ArmPkg changes with
the error handling and error reporting feature on sgi575 before
posting the patches. In addition to the changes that you mention, i
was required to make a couple of more changes in the StandadloneMm
description file.

1) StandaloneMmMmuLib|ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf

This was changed from ArmMmuLib which was used earlier.

2) PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf

I had changed this to reflect the change made in the patch
StandaloneMM: Update permissions for Standalone MM drivers memory area

Can you please confirm that you tested with these two additional
changes made. Meanwhile, I will incorporate your review comments which
you make below. Thanks.

-sughosh

> 
> Lets sort this out first. Apart from this, could you move this library into
> an AArch64 directory as is the case for other Arm specific libraries
> e.g. StandaloneMmCoreEntryPoint/AArch64
> 
> cheers,
> Achin
> 
> On Tue, Nov 27, 2018 at 11:52:53AM +0530, Sughosh Ganu wrote:
> > Changes since v1:
> > A new patch has been added to reflect the library class added for
> > changing the MMU attributes in StandaloneMM image, based on review
> > comments from Ard Biesheuvel.
> > 
> > 
> > These patches needs to be applied on top of the following patch series
> >  - "ArmPkg related changes for StandaloneMM package".
> > 
> > 
> > Sughosh Ganu (2):
> >   StandaloneMM: Include the newly added library class for MMU functions
> >   StandaloneMM: Update permissions for Standalone MM drivers memory area
> > 
> >  .../StandaloneMmCoreEntryPoint.inf                 |   2 +-
> >  .../StandaloneMmPeCoffExtraActionLib.inf           |  18 +-
> >  .../StandaloneMmPeCoffExtraActionLib.c             | 222 +++++++++++++++++++++
> >  3 files changed, 234 insertions(+), 8 deletions(-)
> >  copy ArmPkg/Library/RvdPeCoffExtraActionLib/RvdPeCoffExtraActionLib.inf => StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf (72%)
> >  create mode 100644 StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.c
> > 
> > -- 
> > 2.7.4
> > 

-- 
-sughosh