Re: [edk2] [PATCH v2 0/2] StandaloneMM: Update permissions for Standalone MM drivers memory area
hi Achin, On Sun Dec 02, 2018 at 12:15:47AM +0530, Achin Gupta wrote: > Hi Sughosh, > > On Sat, Dec 01, 2018 at 09:56:52AM +0530, Sughosh Ganu wrote: > > 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. > > yes! this was the change i was referring to in 1. > > > > > 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 > > Doh! of course it does not work without this change. Thanks! Could you roll it > in your patchstack? > > > > > 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. > > I was able to initialise the SP on the FVP and the MM communication driver > initialised correctly. I did not test the MM SP further. I have pushed my > changes on top of your patches here [1]. Could you please check they work for > you as well and include the relevant changes in the next rev of your > patchstack? Sure. I will post these fixes as well. I will post it as a separate series, since the contents are pretty different from the other two patches. Thanks. -sughosh > > cheers, > Achin > > [1] https://github.com/achingupta/edk2/commits/ag/stmm_perm_v2 > > > > > -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 -- -sughosh ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 0/2] StandaloneMM: Update permissions for Standalone MM drivers memory area
Hi Sughosh, On Sat, Dec 01, 2018 at 09:56:52AM +0530, Sughosh Ganu wrote: > 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. yes! this was the change i was referring to in 1. > > 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 Doh! of course it does not work without this change. Thanks! Could you roll it in your patchstack? > > 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. I was able to initialise the SP on the FVP and the MM communication driver initialised correctly. I did not test the MM SP further. I have pushed my changes on top of your patches here [1]. Could you please check they work for you as well and include the relevant changes in the next rev of your patchstack? cheers, Achin [1] https://github.com/achingupta/edk2/commits/ag/stmm_perm_v2 > > -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 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 0/2] StandaloneMM: Update permissions for Standalone MM drivers memory area
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 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 0/2] StandaloneMM: Update permissions for Standalone MM drivers memory area
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. 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 > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 0/2] StandaloneMM: Update permissions for Standalone MM drivers memory area
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 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
