From: Michael Kubacki <michael.kubacki@microsoft.com>

This series contains two changes:

1. To fix a compiler warning with the current state of code.
2. To update the code to pass the integer width needed for a
   comparison to set EFI_MEMORY_XP in the GCD attribute returned
   for a given page attribute.

Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>

Michael Kubacki (2):
  ArmPkg/Drivers/CpuDxe: Explicitly cast table entry
  ArmPkg/Drivers/CpuDxe: Use lower and upper attributes

 ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

-- 
2.42.0.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111766): https://edk2.groups.io/g/devel/message/111766
Mute This Topic: https://groups.io/mt/102841999/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Tue, 28 Nov 2023 at 01:15, <mikuback@linux.microsoft.com> wrote:
>
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> This series contains two changes:
>
> 1. To fix a compiler warning with the current state of code.
> 2. To update the code to pass the integer width needed for a
>    comparison to set EFI_MEMORY_XP in the GCD attribute returned
>    for a given page attribute.
>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
>
> Michael Kubacki (2):
>   ArmPkg/Drivers/CpuDxe: Explicitly cast table entry
>   ArmPkg/Drivers/CpuDxe: Use lower and upper attributes
>

Thanks for the fixes. This code is in a rather poor state, unfortunately.

I don't quite get the motivation for fixing this using two different
patches: the implicit UINT32 cast obviously loses some attributes (the
ones in the 12 upper bits, notably UXN and PXN), so making it explicit
removes the warning but preserves the bug. (If GCC had better
diagnostics, we'd spotted this problem years ago)

Maybe it is sufficient to simply squash the two patches together?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111803): https://edk2.groups.io/g/devel/message/111803
Mute This Topic: https://groups.io/mt/102841999/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 11/28/2023 4:51 AM, Ard Biesheuvel wrote:
> On Tue, 28 Nov 2023 at 01:15, <mikuback@linux.microsoft.com> wrote:
>>
>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>
>> This series contains two changes:
>>
>> 1. To fix a compiler warning with the current state of code.
>> 2. To update the code to pass the integer width needed for a
>>     comparison to set EFI_MEMORY_XP in the GCD attribute returned
>>     for a given page attribute.
>>
>> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>>
>> Michael Kubacki (2):
>>    ArmPkg/Drivers/CpuDxe: Explicitly cast table entry
>>    ArmPkg/Drivers/CpuDxe: Use lower and upper attributes
>>
> 
> Thanks for the fixes. This code is in a rather poor state, unfortunately.
> 
> I don't quite get the motivation for fixing this using two different
> patches: the implicit UINT32 cast obviously loses some attributes (the
> ones in the 12 upper bits, notably UXN and PXN), so making it explicit
> removes the warning but preserves the bug. (If GCC had better
> diagnostics, we'd spotted this problem years ago)
> 
> Maybe it is sufficient to simply squash the two patches together?
> 
I wanted to leave that open to discussion and maintainer discretion. I 
think a squash is fine and sent a v2 with it done.

> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111816): https://edk2.groups.io/g/devel/message/111816
Mute This Topic: https://groups.io/mt/102841999/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Tue, 28 Nov 2023 at 18:15, Michael Kubacki
<mikuback@linux.microsoft.com> wrote:
>
> On 11/28/2023 4:51 AM, Ard Biesheuvel wrote:
> > On Tue, 28 Nov 2023 at 01:15, <mikuback@linux.microsoft.com> wrote:
> >>
> >> From: Michael Kubacki <michael.kubacki@microsoft.com>
> >>
> >> This series contains two changes:
> >>
> >> 1. To fix a compiler warning with the current state of code.
> >> 2. To update the code to pass the integer width needed for a
> >>     comparison to set EFI_MEMORY_XP in the GCD attribute returned
> >>     for a given page attribute.
> >>
> >> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> >> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> >> Cc: Sami Mujawar <sami.mujawar@arm.com>
> >>
> >> Michael Kubacki (2):
> >>    ArmPkg/Drivers/CpuDxe: Explicitly cast table entry
> >>    ArmPkg/Drivers/CpuDxe: Use lower and upper attributes
> >>
> >
> > Thanks for the fixes. This code is in a rather poor state, unfortunately.
> >
> > I don't quite get the motivation for fixing this using two different
> > patches: the implicit UINT32 cast obviously loses some attributes (the
> > ones in the 12 upper bits, notably UXN and PXN), so making it explicit
> > removes the warning but preserves the bug. (If GCC had better
> > diagnostics, we'd spotted this problem years ago)
> >
> > Maybe it is sufficient to simply squash the two patches together?
> >
> I wanted to leave that open to discussion and maintainer discretion. I
> think a squash is fine and sent a v2 with it done.
>

Thanks. I'll go and merge that.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111818): https://edk2.groups.io/g/devel/message/111818
Mute This Topic: https://groups.io/mt/102841999/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-