[PATCH] Pkg-Module: ArmPkg Fix Mmu Initialization Problem.

[PATCH] Pkg-Module: ArmPkg Fix Mmu Initialization Problem.

[Kurt Kennett]

During Mmu initialization in the CpuDxe, for a page table any bits set in the 'NextSectionAttributes' are garbage and were set from bits that are actually part of the pagetable address.  We clear it out to zero so that the SyncCacheConfigPage will use the page attributes instead of trying to convert the (bogus) section attributes into page attributes

Contributed-under: TianoCore Contribution Agreement 1.0

Signed-off-by: Kurt Kennett <kurt.kennett@microsoft.com>
---
ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
index 63da8ba..64ef1bb 100644
--- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
@@ -297,6 +297,11 @@ SyncCacheConfig (
       }
       NextRegionLength += TT_DESCRIPTOR_SECTION_SIZE;
     } else if (TT_DESCRIPTOR_SECTION_TYPE_IS_PAGE_TABLE(FirstLevelTable[i])) {
+      // In this case any bits set in the 'NextSectionAttributes' are garbage and were set from
+      // bits that are actually part of the pagetable address.  We clear it out to zero so that
+      // the SyncCacheConfigPage will use the page attributes instead of trying to convert the
+      // section attributes into page attributes
+      NextSectionAttributes = 0;
       Status = SyncCacheConfigPage (
           i,FirstLevelTable[i],
           NumberOfDescriptors, MemorySpaceMap,
--
2.9.0.windows.1

Re: [PATCH] Pkg-Module: ArmPkg Fix Mmu Initialization Problem.

[Ard Biesheuvel]

On 26 September 2016 at 08:36, Kurt Kennett <Kurt.Kennett@microsoft.com> wrote:
> During Mmu initialization in the CpuDxe, for a page table any bits set in
> the 'NextSectionAttributes' are garbage and were set from bits that are
> actually part of the pagetable address.  We clear it out to zero so that the
> SyncCacheConfigPage will use the page attributes instead of trying to
> convert the (bogus) section attributes into page attributes
>
>
>
> Contributed-under: TianoCore Contribution Agreement 1.0
>
>
>
> Signed-off-by: Kurt Kennett <kurt.kennett@microsoft.com>
>

Hello Kurt,

This is looking a lot better, but you're not quite there yet.

- please replace the 'Pkg-Module' in the subject with the actual
package and module that you are modifying.
- line wrap the commit log
- send using plain text

It may look like we're nitpicking here, but having to manually fix up
your patches is prone to errors. Patches on the list should be
presented in a way that allows us to run 'git am' on them (*) and not
tweak the contents for them to apply

(*) Tianocore does actually require some fairly mechanical line ending
changes to patches taken from the list, unfortunately

-- 
Ard.

Re: [PATCH] Pkg-Module: ArmPkg Fix Mmu Initialization Problem.

[Kurt Kennett]

Thanks Ard.

Trying to learn how to do this for the future and I appreciate the feedback and hand-holding.

I will resubmit now with the changes you requested.

K2

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 
Sent: Monday, September 26, 2016 10:04 AM
To: Kurt Kennett <Kurt.Kennett@microsoft.com>
Cc: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>
Subject: Re: [PATCH] Pkg-Module: ArmPkg Fix Mmu Initialization Problem.

On 26 September 2016 at 08:36, Kurt Kennett <Kurt.Kennett@microsoft.com> wrote:
> During Mmu initialization in the CpuDxe, for a page table any bits set 
> in the 'NextSectionAttributes' are garbage and were set from bits that 
> are actually part of the pagetable address.  We clear it out to zero 
> so that the SyncCacheConfigPage will use the page attributes instead 
> of trying to convert the (bogus) section attributes into page 
> attributes
>
>
>
> Contributed-under: TianoCore Contribution Agreement 1.0
>
>
>
> Signed-off-by: Kurt Kennett <kurt.kennett@microsoft.com>
>

Hello Kurt,

This is looking a lot better, but you're not quite there yet.

- please replace the 'Pkg-Module' in the subject with the actual package and module that you are modifying.
- line wrap the commit log
- send using plain text

It may look like we're nitpicking here, but having to manually fix up your patches is prone to errors. Patches on the list should be presented in a way that allows us to run 'git am' on them (*) and not tweak the contents for them to apply

(*) Tianocore does actually require some fairly mechanical line ending changes to patches taken from the list, unfortunately

--
Ard.

Re: [PATCH] Pkg-Module: ArmPkg Fix Mmu Initialization Problem.

[Kurt Kennett]

Weird.  I line-wrapped the commit log and somehow it became un-line-wrapped. So... sorry, I didn't forget. :)

K2

-----Original Message-----
From: Kurt Kennett 
Sent: Monday, September 26, 2016 10:08 AM
To: 'Ard Biesheuvel' <ard.biesheuvel@linaro.org>
Cc: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>
Subject: RE: [PATCH] Pkg-Module: ArmPkg Fix Mmu Initialization Problem.

Thanks Ard.

Trying to learn how to do this for the future and I appreciate the feedback and hand-holding.

I will resubmit now with the changes you requested.

K2

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Monday, September 26, 2016 10:04 AM
To: Kurt Kennett <Kurt.Kennett@microsoft.com>
Cc: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>
Subject: Re: [PATCH] Pkg-Module: ArmPkg Fix Mmu Initialization Problem.

On 26 September 2016 at 08:36, Kurt Kennett <Kurt.Kennett@microsoft.com> wrote:
> During Mmu initialization in the CpuDxe, for a page table any bits set 
> in the 'NextSectionAttributes' are garbage and were set from bits that 
> are actually part of the pagetable address.  We clear it out to zero 
> so that the SyncCacheConfigPage will use the page attributes instead 
> of trying to convert the (bogus) section attributes into page 
> attributes
>
>
>
> Contributed-under: TianoCore Contribution Agreement 1.0
>
>
>
> Signed-off-by: Kurt Kennett <kurt.kennett@microsoft.com>
>

Hello Kurt,

This is looking a lot better, but you're not quite there yet.

- please replace the 'Pkg-Module' in the subject with the actual package and module that you are modifying.
- line wrap the commit log
- send using plain text

It may look like we're nitpicking here, but having to manually fix up your patches is prone to errors. Patches on the list should be presented in a way that allows us to run 'git am' on them (*) and not tweak the contents for them to apply

(*) Tianocore does actually require some fairly mechanical line ending changes to patches taken from the list, unfortunately

--
Ard.

Re: [PATCH] Pkg-Module: ArmPkg Fix Mmu Initialization Problem.

[Ard Biesheuvel]

On 26 September 2016 at 10:10, Kurt Kennett <Kurt.Kennett@microsoft.com> wrote:
> Weird.  I line-wrapped the commit log and somehow it became un-line-wrapped. So... sorry, I didn't forget. :)
>

No, the commit log looks fine. The patch itself contains weird
non-ASCII space characters though:

+<A0><A0><A0><A0><A0> // In this case any bits set in the
'NextSectionAttributes' are garbage and were set from
+<A0><A0><A0><A0><A0> // bits that are actually part of the pagetable
address.<A0> We clear it out to zero so that
+<A0><A0><A0><A0><A0> // the SyncCacheConfigPage will use the page
attributes instead of trying to convert the
+<A0><A0><A0><A0><A0> // section attributes into page attributes
+<A0><A0><A0><A0><A0> NextSectionAttributes = 0;

Re: [PATCH] Pkg-Module: ArmPkg Fix Mmu Initialization Problem.

[Kurt Kennett]

In the second one I sent or the first one?

The second one I sent is in Plain Text, while the first one may not have been.

Let me try once more here.  Sorry to everyone who on the list who doesn't care! :)

K2

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 
Sent: Monday, September 26, 2016 10:18 AM
To: Kurt Kennett <Kurt.Kennett@microsoft.com>
Cc: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>
Subject: Re: [PATCH] Pkg-Module: ArmPkg Fix Mmu Initialization Problem.

On 26 September 2016 at 10:10, Kurt Kennett <Kurt.Kennett@microsoft.com> wrote:
> Weird.  I line-wrapped the commit log and somehow it became 
> un-line-wrapped. So... sorry, I didn't forget. :)
>

No, the commit log looks fine. The patch itself contains weird non-ASCII space characters though:

+<A0><A0><A0><A0><A0> // In this case any bits set in the
'NextSectionAttributes' are garbage and were set from
+<A0><A0><A0><A0><A0> // bits that are actually part of the pagetable
address.<A0> We clear it out to zero so that
+<A0><A0><A0><A0><A0> // the SyncCacheConfigPage will use the page
attributes instead of trying to convert the
+<A0><A0><A0><A0><A0> // section attributes into page attributes 
+<A0><A0><A0><A0><A0> NextSectionAttributes = 0;

[PATCH] Pkg-Module: ArmPkg Fix Mmu Initialization Problem.

[Kurt Kennett]

Please see attached proposed patch.

K2