1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH 0/2] ArmPkg/ArmMmuLib AARCH64: correctness fix
  4. View patch

[edk2-devel] [PATCH 2/2] ArmPkg/ArmMmuLib AARCH64: preserve attributes when replacing a table entry

Ard Biesheuvel posted 2 patches 5 years, 11 months ago
There is a newer version of this series
[edk2-devel] [PATCH 2/2] ArmPkg/ArmMmuLib AARCH64: preserve attributes when replacing a table entry
Posted by Ard Biesheuvel 5 years, 11 months ago 
Currently, depending on the size of the region being (re)mapped, the
page table manipulation code may replace a table entry with a block entry,
even if the existing table entry uses different mapping attributes to
describe different parts of the region it covers. This is undesirable, and
instead, we should avoid doing so unless we are disregarding the original
attributes anyway. And if we make such a replacement, we should free all
the page tables that have become orphaned in the process.

So let's implement this, by taking the table entry path through the code
for block sized regions if a table entry already exists, and the clear
mask is set (which means we are preserving attributes from the existing
mapping). And when we do replace a table entry with a block entry, free
all the pages that are no longer referenced.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 21 ++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index 6f6ef5b05fbc..7b2c36a7a538 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -223,8 +223,12 @@ UpdateRegionMappingRecursive (
     // than a block, and recurse to create the block or page entries at
     // the next level. No block mappings are allowed at all at level 0,
     // so in that case, we have to recurse unconditionally.
+    // If we are changing a table entry and the AttributeClearMask is non-zero,
+    // we cannot replace it with a block entry without potentially losing
+    // attribute information, so keep the table entry in that case.
     //
-    if (Level == 0 || ((RegionStart | BlockEnd) & BlockMask) != 0) {
+    if (Level == 0 || ((RegionStart | BlockEnd) & BlockMask) != 0 ||
+        (IsTableEntry (*Entry, Level) && AttributeClearMask != 0)) {
       ASSERT (Level < 3);
 
       if (!IsTableEntry (*Entry, Level)) {
@@ -300,7 +304,20 @@ UpdateRegionMappingRecursive (
       EntryValue |= (Level == 3) ? TT_TYPE_BLOCK_ENTRY_LEVEL3
                                  : TT_TYPE_BLOCK_ENTRY;
 
-      ReplaceTableEntry (Entry, EntryValue, RegionStart, FALSE);
+      if (IsTableEntry (*Entry, Level)) {
+        //
+        // We are replacing a table entry with a block entry. This is only
+        // possible if we are keeping none of the original attributes.
+        // We can free the table entry's page table, and all the ones below
+        // it, since we are dropping the only possible reference to it.
+        //
+        ASSERT (AttributeClearMask == 0);
+        TranslationTable = (VOID *)(UINTN)(*Entry & TT_ADDRESS_MASK_BLOCK_ENTRY);
+        ReplaceTableEntry (Entry, EntryValue, RegionStart, TRUE);
+        FreePageTablesRecursive (TranslationTable);
+      } else {
+        ReplaceTableEntry (Entry, EntryValue, RegionStart, FALSE);
+      }
     }
   }
   return EFI_SUCCESS;
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55645): https://edk2.groups.io/g/devel/message/55645
Mute This Topic: https://groups.io/mt/71795104/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 2/2] ArmPkg/ArmMmuLib AARCH64: preserve attributes when replacing a table entry
Posted by Laszlo Ersek 5 years, 11 months ago 
Hi Ard,

On 03/07/20 14:34, Ard Biesheuvel wrote:
> Currently, depending on the size of the region being (re)mapped, the
> page table manipulation code may replace a table entry with a block entry,
> even if the existing table entry uses different mapping attributes to
> describe different parts of the region it covers. This is undesirable, and
> instead, we should avoid doing so unless we are disregarding the original
> attributes anyway. And if we make such a replacement, we should free all
> the page tables that have become orphaned in the process.
>
> So let's implement this, by taking the table entry path through the code
> for block sized regions if a table entry already exists, and the clear
> mask is set (which means we are preserving attributes from the existing
> mapping). And when we do replace a table entry with a block entry, free
> all the pages that are no longer referenced.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 21 ++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index 6f6ef5b05fbc..7b2c36a7a538 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -223,8 +223,12 @@ UpdateRegionMappingRecursive (
>      // than a block, and recurse to create the block or page entries at
>      // the next level. No block mappings are allowed at all at level 0,
>      // so in that case, we have to recurse unconditionally.
> +    // If we are changing a table entry and the AttributeClearMask is non-zero,
> +    // we cannot replace it with a block entry without potentially losing
> +    // attribute information, so keep the table entry in that case.
>      //
> -    if (Level == 0 || ((RegionStart | BlockEnd) & BlockMask) != 0) {
> +    if (Level == 0 || ((RegionStart | BlockEnd) & BlockMask) != 0 ||
> +        (IsTableEntry (*Entry, Level) && AttributeClearMask != 0)) {
>        ASSERT (Level < 3);
>
>        if (!IsTableEntry (*Entry, Level)) {
> @@ -300,7 +304,20 @@ UpdateRegionMappingRecursive (
>        EntryValue |= (Level == 3) ? TT_TYPE_BLOCK_ENTRY_LEVEL3
>                                   : TT_TYPE_BLOCK_ENTRY;
>
> -      ReplaceTableEntry (Entry, EntryValue, RegionStart, FALSE);
> +      if (IsTableEntry (*Entry, Level)) {
> +        //
> +        // We are replacing a table entry with a block entry. This is only
> +        // possible if we are keeping none of the original attributes.
> +        // We can free the table entry's page table, and all the ones below
> +        // it, since we are dropping the only possible reference to it.
> +        //
> +        ASSERT (AttributeClearMask == 0);
> +        TranslationTable = (VOID *)(UINTN)(*Entry & TT_ADDRESS_MASK_BLOCK_ENTRY);
> +        ReplaceTableEntry (Entry, EntryValue, RegionStart, TRUE);
> +        FreePageTablesRecursive (TranslationTable);
> +      } else {
> +        ReplaceTableEntry (Entry, EntryValue, RegionStart, FALSE);
> +      }
>      }
>    }
>    return EFI_SUCCESS;
>

This patch crashes an existent (RHEL-ALT-7) guest for me, when it tries
to launch "grubaa64.efi".

(1) I closed your PR#430 as explained here:
<https://edk2.groups.io/g/devel/message/55695>, and collected *six* of
your reviewed patches, for a new PR:

- [edk2-devel] [PATCH v4 0/2] ArmPkg/ArmMmuLib: rewrite and improve
cache handling with MMU off

- [edk2-devel] [PATCH 0/2] ArmPkg/ArmMmuLib AARCH64: final cleanups

- the present set


(2) Before submitting the PR, I figured I'd boot one of my permanent
QEMU (not KVM) guests, with all six patches applied (locally, for the
time). The boot crashed; here's the end of the log:

> [Bds]Booting Red Hat Enterprise Linux
> FSOpen: Open '\EFI\redhat\shimaa64.efi' Success
> [Bds] Expand HD(1,GPT,1F51423F-5044-4C9C-BBF7-666D366B249E,0x800,0x64000)/\EFI\redhat\shimaa64.efi -> PciRoot(0x0)/Pci(0x1,0x6)/Pci(0x0,0x0)/Scsi(0x0,0x0)/HD(1,GPT,1F51423F-5044-4C9C-BBF7-666D366B249E,0x800,0x64000)/\EFI\redhat\shimaa64.efi
> BdsDxe: loading Boot0005 "Red Hat Enterprise Linux" from HD(1,GPT,1F51423F-5044-4C9C-BBF7-666D366B249E,0x800,0x64000)/\EFI\redhat\shimaa64.efi
> [Security] 3rd party image[0] can be loaded after EndOfDxe: PciRoot(0x0)/Pci(0x1,0x6)/Pci(0x0,0x0)/Scsi(0x0,0x0)/HD(1,GPT,1F51423F-5044-4C9C-BBF7-666D366B249E,0x800,0x64000)/\EFI\redhat\shimaa64.efi.
> InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 13A2196C0
> UpdateRegionMappingRecursive(0): 137EE0000 - 137FA0000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(1): 137EE0000 - 137FA0000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(2): 137EE0000 - 137FA0000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(3): 137E00000 - 138000000 set 6000000000070C clr 0
> UpdateRegionMappingRecursive(3): 137EE0000 - 137FA0000 set 0 clr FF9F000000000F3F
> Loading driver at 0x00137EE0000 EntryPoint=0x00137EE1000
> Loading driver at 0x00137EE0000 EntryPoint=0x00137EE1000
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 13A124498
> ProtectUefiImageCommon - 0x3A2196C0
>   - 0x0000000137EE0000 - 0x00000000000BF500
>   Section - '.text   '
>   VirtualSize          - 0x0008C000
>   VirtualAddress       - 0x00001000
>   SizeOfRawData        - 0x0008C000
>   PointerToRawData     - 0x00001000
>   PointerToRelocations - 0x00000000
>   PointerToLinenumbers - 0x00000000
>   NumberOfRelocations  - 0x00000000
>   NumberOfLinenumbers  - 0x00000000
>   Characteristics      - 0x60000020
> ImageCode: 0x0000000137EE1000 - 0x000000000008C000
>   Section - '.data   '
> ImageCode SegmentCount - 0x1
> SetUefiImageMemoryAttributes - 0x0000000137EE0000 - 0x0000000000001000 (0x0000000000004008)
> UpdateRegionMappingRecursive(0): 137EE0000 - 137EE1000 set 6000000000070C clr 0
> UpdateRegionMappingRecursive(1): 137EE0000 - 137EE1000 set 6000000000070C clr 0
> UpdateRegionMappingRecursive(2): 137EE0000 - 137EE1000 set 6000000000070C clr 0
> UpdateRegionMappingRecursive(3): 137EE0000 - 137EE1000 set 6000000000070C clr 0
> SetUefiImageMemoryAttributes - 0x0000000137EE1000 - 0x000000000008C000 (0x0000000000020008)
> UpdateRegionMappingRecursive(0): 137EE1000 - 137F6D000 set 7CC clr 0
> UpdateRegionMappingRecursive(1): 137EE1000 - 137F6D000 set 7CC clr 0
> UpdateRegionMappingRecursive(2): 137EE1000 - 137F6D000 set 7CC clr 0
> UpdateRegionMappingRecursive(3): 137EE1000 - 137F6D000 set 7CC clr 0
> SetUefiImageMemoryAttributes - 0x0000000137F6D000 - 0x0000000000033000 (0x0000000000004008)
> UpdateRegionMappingRecursive(0): 137F6D000 - 137FA0000 set 6000000000070C clr 0
> UpdateRegionMappingRecursive(1): 137F6D000 - 137FA0000 set 6000000000070C clr 0
> UpdateRegionMappingRecursive(2): 137F6D000 - 137FA0000 set 6000000000070C clr 0
> UpdateRegionMappingRecursive(3): 137F6D000 - 137FA0000 set 6000000000070C clr 0
> BdsDxe: starting Boot0005 "Red Hat Enterprise Linux" from HD(1,GPT,1F51423F-5044-4C9C-BBF7-666D366B249E,0x800,0x64000)/\EFI\redhat\shimaa64.efi
> UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> InstallProtocolInterface: 605DAB50-E046-4300-ABB6-3DD810DD8B23 137F806D8
> UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> FSOpen: Open '\EFI\redhat\grubaa64.efi' Success
> UpdateRegionMappingRecursive(0): 137DD2000 - 137EE0000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(1): 137DD2000 - 137EE0000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(2): 137DD2000 - 137EE0000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(3): 137C00000 - 137E00000 set 6000000000070C clr 0
> UpdateRegionMappingRecursive(3): 137DD2000 - 137E00000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(3): 137E00000 - 137EE0000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(0): 137CC5000 - 137DD2000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(1): 137CC5000 - 137DD2000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(2): 137CC5000 - 137DD2000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(3): 137CC5000 - 137DD2000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(0): 138020000 - 138026000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(1): 138020000 - 138026000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(2): 138020000 - 138026000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(3): 138020000 - 138026000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(0): F8CB0000 - 137CC5000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(1): F8CB0000 - 137CC5000 set 0 clr FF9F000000000F3F
> UpdateRegionMappingRecursive(2): C0000000 - 100000000 set 6000000000070C clr 0
>
>
> Synchronous Exception at 0x000000013BA184F0
>
>
> Synchronous Exception at 0x000000013BA184F0
> PC 0x00013BA184F0 (0x00013BA14000+0x000044F0) [ 0] ArmCpuDxe.dll
> PC 0x00013BA18A30 (0x00013BA14000+0x00004A30) [ 0] ArmCpuDxe.dll
> PC 0x00013BA18868 (0x00013BA14000+0x00004868) [ 0] ArmCpuDxe.dll
> PC 0x00013BA188D0 (0x00013BA14000+0x000048D0) [ 0] ArmCpuDxe.dll
> PC 0x00013BA18B00 (0x00013BA14000+0x00004B00) [ 0] ArmCpuDxe.dll
> PC 0x00013BA18C60 (0x00013BA14000+0x00004C60) [ 0] ArmCpuDxe.dll
> PC 0x00013BA161E0 (0x00013BA14000+0x000021E0) [ 0] ArmCpuDxe.dll
> PC 0x00013F11AEE8 (0x00013F10D000+0x0000DEE8) [ 1] DxeCore.dll
> PC 0x00013F1284B8 (0x00013F10D000+0x0001B4B8) [ 1] DxeCore.dll
> PC 0x000137CC8380
> PC 0x000137CC8970
> PC 0x000137CC8094
> PC 0x000137CC5478
> PC 0x000137CCCFC8
> PC 0x000137EE6C64
> PC 0x00013F11420C (0x00013F10D000+0x0000720C) [ 1] DxeCore.dll
> PC 0x00013B8D5A20 (0x00013B8C5000+0x00010A20) [ 2] BdsDxe.dll
> PC 0x00013B8C6FE4 (0x00013B8C5000+0x00001FE4) [ 2] BdsDxe.dll
> PC 0x00013B8C8680 (0x00013B8C5000+0x00003680) [ 2] BdsDxe.dll
> PC 0x00013F10F88C (0x00013F10D000+0x0000288C) [ 3] DxeCore.dll
> PC 0x00013F10E8C0 (0x00013F10D000+0x000018C0) [ 3] DxeCore.dll
> PC 0x00013F10E024 (0x00013F10D000+0x00001024) [ 3] DxeCore.dll
>
> [ 0] Build/ArmVirtQemu-AARCH64/NOOPT_GCC5/AARCH64/ArmPkg/Drivers/CpuDxe/CpuDxe/DEBUG/ArmCpuDxe.dll
> [ 1] Build/ArmVirtQemu-AARCH64/NOOPT_GCC5/AARCH64/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll
> [ 2] Build/ArmVirtQemu-AARCH64/NOOPT_GCC5/AARCH64/MdeModulePkg/Universal/BdsDxe/BdsDxe/DEBUG/BdsDxe.dll
> [ 3] Build/ArmVirtQemu-AARCH64/NOOPT_GCC5/AARCH64/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll
>
>   X0 0x0000AFAFAFAFA000   X1 0x0000AFAFAFAFA000   X2 0x00000000000C0000   X3 0x0000000000000004
>   X4 0x0000000000000200   X5 0x0000000000000000   X6 0x006000000000070C   X7 0x0000000000000000
>   X8 0x0000000000C5183D   X9 0x0000000000C5183C  X10 0x000000013A216000  X11 0x000000013B643FFF
>  X12 0x0000000000000000  X13 0x0000000000000008  X14 0x0000000000000000  X15 0x0000000000000000
>  X16 0x00000000220A3B01  X17 0x000000013F10C660  X18 0x00000000CB9AE5A3  X19 0x0000000137CC5000
>  X20 0x0000000000000002  X21 0x0000000138020000  X22 0x000000013F158008  X23 0x000000000003F015
>  X24 0x00000000F8CB0000  X25 0x0000000000000030  X26 0x00000000000FFFFF  X27 0x0000000000100000
>  X28 0x0000000000000000   FP 0x000000013F10C530   LR 0x000000013BA18A30
>
>   V0 0x0000000000000000 0000000000000000   V1 0x0000000000000000 000000009E446499
>   V2 0x0000000000000000 00000000E35EF5EF   V3 0x0000000000000000 000000005A378E55
>   V4 0x0000000000000000 00000000A394033C   V5 0x0000000000000000 0000000137F566D8
>   V6 0x0000000000000000 00000000430E721A   V7 0x0000000000000000 00000000E531AE60
>   V8 0x0000000000000000 0000000000000000   V9 0x0000000000000000 0000000000000000
>  V10 0x0000000000000000 0000000000000000  V11 0x0000000000000000 0000000000000000
>  V12 0x0000000000000000 0000000000000000  V13 0x0000000000000000 0000000000000000
>  V14 0x0000000000000000 0000000000000000  V15 0x0000000000000000 0000000000000000
>  V16 0x0000000000000000 0000000000000000  V17 0x0000000000000000 0000000000000000
>  V18 0x0000000000000000 0000000000000000  V19 0x0000000000000000 0000000000000000
>  V20 0x0000000000000000 0000000000000000  V21 0x0000000000000000 0000000000000000
>  V22 0x0000000000000000 0000000000000000  V23 0x0000000000000000 0000000000000000
>  V24 0x0000000000000000 0000000000000000  V25 0x0000000000000000 0000000000000000
>  V26 0x0000000000000000 0000000000000000  V27 0x0000000000000000 0000000000000000
>  V28 0x0000000000000000 0000000000000000  V29 0x0000000000000000 0000000000000000
>  V30 0x0000000000000000 0000000000000000  V31 0x0000000000000000 0000000000000000
>
>   SP 0x000000013F10C530  ELR 0x000000013BA184F0  SPSR 0x80000205  FPSR 0x00000000
>  ESR 0x96000004          FAR 0x0000AFAFAFAFA000
>
>  ESR : EC 0x25  IL 0x1  ISS 0x00000004
>
> Data abort: Translation fault, zeroth level
>
> Stack dump:
>   000013F10C430: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013F10C450: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013F10C470: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013F10C490: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013F10C4B0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013F10C4D0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013F10C4F0: 0000000000000000 0000000000000000 000000013BA1E140 0000000060000304
>   000013F10C510: 0000000000000000 0000000096000004 0000AFAFAFAFA000 00000000FFFFFFD0
> > 000013F10C530: 000000013F10C560 000000013BA18A30 01000000C0000000 0000AFAFAFAFA000
>   000013F10C550: 00600000C000070D 0000000000000000 000000013F10C5E0 000000013BA18868
>   000013F10C570: 0000000000000002 000000013A216000 0000000000000000 006000000000070C
>   000013F10C590: 0000000100000000 00000000C0000000 0000000000000001 00000004FFFFFFD0
>   000013F10C5B0: 00600000C000070D 000000013A216000 00000000C0200000 00000000001FFFFF
>   000013F10C5D0: 000000000000002B 0000AFAFAFAFA000 000000013F10C660 000000013BA188D0
>   000013F10C5F0: 0000000000000001 000000013FFFE000 FF9F000000000F3F 0000000000000000
>   000013F10C610: 0000000137CC5000 00000000F8CB0000 000000013F10C630 00000000FFFFFFD0
> ASSERT [ArmCpuDxe] ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c(273): ((BOOLEAN)(0==1))


(3) I bisected the 6-patch range, to find the culprit. Here's the log:

> git bisect start
> # bad: [31c189a32c370d4abf7a596f7f60a0578e8c7672] ArmPkg/ArmMmuLib AARCH64: preserve attributes when replacing a table entry
> git bisect bad 31c189a32c370d4abf7a596f7f60a0578e8c7672
> # good: [a3e25cc8a1dd3d1ea24ed02f90c44221e015e965] OvmfPkg/X86QemuLoadImageLib: fix "unused variable" error in X64 DXE builds
> git bisect good a3e25cc8a1dd3d1ea24ed02f90c44221e015e965
> # good: [d4b4f49e4d43276cf410778f172eecab7fc472dc] ArmPkg/ArmMmuLib AARCH64: drop pointless page table memory type check
> git bisect good d4b4f49e4d43276cf410778f172eecab7fc472dc
> # good: [8952f2498dcdc2dfbba8216007bb0a4745d944b2] ArmPkg/ArmMmuLib AARCH64: use helpers to determine table entry types
> git bisect good 8952f2498dcdc2dfbba8216007bb0a4745d944b2
> # first bad commit: [31c189a32c370d4abf7a596f7f60a0578e8c7672] ArmPkg/ArmMmuLib AARCH64: preserve attributes when replacing a table entry

Therefore, I decided to push the first four patches (first two sets),
via <https://github.com/tianocore/edk2/pull/431>, and to report a
problem with this one.

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55701): https://edk2.groups.io/g/devel/message/55701
Mute This Topic: https://groups.io/mt/71795104/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 2/2] ArmPkg/ArmMmuLib AARCH64: preserve attributes when replacing a table entry
Posted by Ard Biesheuvel 5 years, 10 months ago 
On Tue, 10 Mar 2020 at 01:29, Laszlo Ersek <lersek@redhat.com> wrote:
>
> Hi Ard,
>
> On 03/07/20 14:34, Ard Biesheuvel wrote:
> > Currently, depending on the size of the region being (re)mapped, the
> > page table manipulation code may replace a table entry with a block entry,
> > even if the existing table entry uses different mapping attributes to
> > describe different parts of the region it covers. This is undesirable, and
> > instead, we should avoid doing so unless we are disregarding the original
> > attributes anyway. And if we make such a replacement, we should free all
> > the page tables that have become orphaned in the process.
> >
> > So let's implement this, by taking the table entry path through the code
> > for block sized regions if a table entry already exists, and the clear
> > mask is set (which means we are preserving attributes from the existing
> > mapping). And when we do replace a table entry with a block entry, free
> > all the pages that are no longer referenced.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 21 ++++++++++++++++++--
> >  1 file changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > index 6f6ef5b05fbc..7b2c36a7a538 100644
> > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > @@ -223,8 +223,12 @@ UpdateRegionMappingRecursive (
> >      // than a block, and recurse to create the block or page entries at
> >      // the next level. No block mappings are allowed at all at level 0,
> >      // so in that case, we have to recurse unconditionally.
> > +    // If we are changing a table entry and the AttributeClearMask is non-zero,
> > +    // we cannot replace it with a block entry without potentially losing
> > +    // attribute information, so keep the table entry in that case.
> >      //
> > -    if (Level == 0 || ((RegionStart | BlockEnd) & BlockMask) != 0) {
> > +    if (Level == 0 || ((RegionStart | BlockEnd) & BlockMask) != 0 ||
> > +        (IsTableEntry (*Entry, Level) && AttributeClearMask != 0)) {
> >        ASSERT (Level < 3);
> >
> >        if (!IsTableEntry (*Entry, Level)) {
> > @@ -300,7 +304,20 @@ UpdateRegionMappingRecursive (
> >        EntryValue |= (Level == 3) ? TT_TYPE_BLOCK_ENTRY_LEVEL3
> >                                   : TT_TYPE_BLOCK_ENTRY;
> >
> > -      ReplaceTableEntry (Entry, EntryValue, RegionStart, FALSE);
> > +      if (IsTableEntry (*Entry, Level)) {
> > +        //
> > +        // We are replacing a table entry with a block entry. This is only
> > +        // possible if we are keeping none of the original attributes.
> > +        // We can free the table entry's page table, and all the ones below
> > +        // it, since we are dropping the only possible reference to it.
> > +        //
> > +        ASSERT (AttributeClearMask == 0);
> > +        TranslationTable = (VOID *)(UINTN)(*Entry & TT_ADDRESS_MASK_BLOCK_ENTRY);
> > +        ReplaceTableEntry (Entry, EntryValue, RegionStart, TRUE);
> > +        FreePageTablesRecursive (TranslationTable);
> > +      } else {
> > +        ReplaceTableEntry (Entry, EntryValue, RegionStart, FALSE);
> > +      }
> >      }
> >    }
> >    return EFI_SUCCESS;
> >
>
> This patch crashes an existent (RHEL-ALT-7) guest for me, when it tries
> to launch "grubaa64.efi".
>
> (1) I closed your PR#430 as explained here:
> <https://edk2.groups.io/g/devel/message/55695>, and collected *six* of
> your reviewed patches, for a new PR:
>
> - [edk2-devel] [PATCH v4 0/2] ArmPkg/ArmMmuLib: rewrite and improve
> cache handling with MMU off
>
> - [edk2-devel] [PATCH 0/2] ArmPkg/ArmMmuLib AARCH64: final cleanups
>
> - the present set
>
>
> (2) Before submitting the PR, I figured I'd boot one of my permanent
> QEMU (not KVM) guests, with all six patches applied (locally, for the
> time). The boot crashed; here's the end of the log:
>
> > [Bds]Booting Red Hat Enterprise Linux
> > FSOpen: Open '\EFI\redhat\shimaa64.efi' Success
> > [Bds] Expand HD(1,GPT,1F51423F-5044-4C9C-BBF7-666D366B249E,0x800,0x64000)/\EFI\redhat\shimaa64.efi -> PciRoot(0x0)/Pci(0x1,0x6)/Pci(0x0,0x0)/Scsi(0x0,0x0)/HD(1,GPT,1F51423F-5044-4C9C-BBF7-666D366B249E,0x800,0x64000)/\EFI\redhat\shimaa64.efi
> > BdsDxe: loading Boot0005 "Red Hat Enterprise Linux" from HD(1,GPT,1F51423F-5044-4C9C-BBF7-666D366B249E,0x800,0x64000)/\EFI\redhat\shimaa64.efi
> > [Security] 3rd party image[0] can be loaded after EndOfDxe: PciRoot(0x0)/Pci(0x1,0x6)/Pci(0x0,0x0)/Scsi(0x0,0x0)/HD(1,GPT,1F51423F-5044-4C9C-BBF7-666D366B249E,0x800,0x64000)/\EFI\redhat\shimaa64.efi.
> > InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 13A2196C0
> > UpdateRegionMappingRecursive(0): 137EE0000 - 137FA0000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(1): 137EE0000 - 137FA0000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(2): 137EE0000 - 137FA0000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(3): 137E00000 - 138000000 set 6000000000070C clr 0
> > UpdateRegionMappingRecursive(3): 137EE0000 - 137FA0000 set 0 clr FF9F000000000F3F
> > Loading driver at 0x00137EE0000 EntryPoint=0x00137EE1000
> > Loading driver at 0x00137EE0000 EntryPoint=0x00137EE1000
> > InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 13A124498
> > ProtectUefiImageCommon - 0x3A2196C0
> >   - 0x0000000137EE0000 - 0x00000000000BF500
> >   Section - '.text   '
> >   VirtualSize          - 0x0008C000
> >   VirtualAddress       - 0x00001000
> >   SizeOfRawData        - 0x0008C000
> >   PointerToRawData     - 0x00001000
> >   PointerToRelocations - 0x00000000
> >   PointerToLinenumbers - 0x00000000
> >   NumberOfRelocations  - 0x00000000
> >   NumberOfLinenumbers  - 0x00000000
> >   Characteristics      - 0x60000020
> > ImageCode: 0x0000000137EE1000 - 0x000000000008C000
> >   Section - '.data   '
> > ImageCode SegmentCount - 0x1
> > SetUefiImageMemoryAttributes - 0x0000000137EE0000 - 0x0000000000001000 (0x0000000000004008)
> > UpdateRegionMappingRecursive(0): 137EE0000 - 137EE1000 set 6000000000070C clr 0
> > UpdateRegionMappingRecursive(1): 137EE0000 - 137EE1000 set 6000000000070C clr 0
> > UpdateRegionMappingRecursive(2): 137EE0000 - 137EE1000 set 6000000000070C clr 0
> > UpdateRegionMappingRecursive(3): 137EE0000 - 137EE1000 set 6000000000070C clr 0
> > SetUefiImageMemoryAttributes - 0x0000000137EE1000 - 0x000000000008C000 (0x0000000000020008)
> > UpdateRegionMappingRecursive(0): 137EE1000 - 137F6D000 set 7CC clr 0
> > UpdateRegionMappingRecursive(1): 137EE1000 - 137F6D000 set 7CC clr 0
> > UpdateRegionMappingRecursive(2): 137EE1000 - 137F6D000 set 7CC clr 0
> > UpdateRegionMappingRecursive(3): 137EE1000 - 137F6D000 set 7CC clr 0
> > SetUefiImageMemoryAttributes - 0x0000000137F6D000 - 0x0000000000033000 (0x0000000000004008)
> > UpdateRegionMappingRecursive(0): 137F6D000 - 137FA0000 set 6000000000070C clr 0
> > UpdateRegionMappingRecursive(1): 137F6D000 - 137FA0000 set 6000000000070C clr 0
> > UpdateRegionMappingRecursive(2): 137F6D000 - 137FA0000 set 6000000000070C clr 0
> > UpdateRegionMappingRecursive(3): 137F6D000 - 137FA0000 set 6000000000070C clr 0
> > BdsDxe: starting Boot0005 "Red Hat Enterprise Linux" from HD(1,GPT,1F51423F-5044-4C9C-BBF7-666D366B249E,0x800,0x64000)/\EFI\redhat\shimaa64.efi
> > UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> > InstallProtocolInterface: 605DAB50-E046-4300-ABB6-3DD810DD8B23 137F806D8
> > UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 60000000000000 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(0): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(1): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(2): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(3): 138112000 - 138113000 set 0 clr FF9F000000000F3F
> > FSOpen: Open '\EFI\redhat\grubaa64.efi' Success
> > UpdateRegionMappingRecursive(0): 137DD2000 - 137EE0000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(1): 137DD2000 - 137EE0000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(2): 137DD2000 - 137EE0000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(3): 137C00000 - 137E00000 set 6000000000070C clr 0
> > UpdateRegionMappingRecursive(3): 137DD2000 - 137E00000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(3): 137E00000 - 137EE0000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(0): 137CC5000 - 137DD2000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(1): 137CC5000 - 137DD2000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(2): 137CC5000 - 137DD2000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(3): 137CC5000 - 137DD2000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(0): 138020000 - 138026000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(1): 138020000 - 138026000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(2): 138020000 - 138026000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(3): 138020000 - 138026000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(0): F8CB0000 - 137CC5000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(1): F8CB0000 - 137CC5000 set 0 clr FF9F000000000F3F
> > UpdateRegionMappingRecursive(2): C0000000 - 100000000 set 6000000000070C clr 0
> >
> >
> > Synchronous Exception at 0x000000013BA184F0
> >
> >
> > Synchronous Exception at 0x000000013BA184F0
> > PC 0x00013BA184F0 (0x00013BA14000+0x000044F0) [ 0] ArmCpuDxe.dll
> > PC 0x00013BA18A30 (0x00013BA14000+0x00004A30) [ 0] ArmCpuDxe.dll
> > PC 0x00013BA18868 (0x00013BA14000+0x00004868) [ 0] ArmCpuDxe.dll
> > PC 0x00013BA188D0 (0x00013BA14000+0x000048D0) [ 0] ArmCpuDxe.dll
> > PC 0x00013BA18B00 (0x00013BA14000+0x00004B00) [ 0] ArmCpuDxe.dll
> > PC 0x00013BA18C60 (0x00013BA14000+0x00004C60) [ 0] ArmCpuDxe.dll
> > PC 0x00013BA161E0 (0x00013BA14000+0x000021E0) [ 0] ArmCpuDxe.dll
> > PC 0x00013F11AEE8 (0x00013F10D000+0x0000DEE8) [ 1] DxeCore.dll
> > PC 0x00013F1284B8 (0x00013F10D000+0x0001B4B8) [ 1] DxeCore.dll
> > PC 0x000137CC8380
> > PC 0x000137CC8970
> > PC 0x000137CC8094
> > PC 0x000137CC5478
> > PC 0x000137CCCFC8
> > PC 0x000137EE6C64
> > PC 0x00013F11420C (0x00013F10D000+0x0000720C) [ 1] DxeCore.dll
> > PC 0x00013B8D5A20 (0x00013B8C5000+0x00010A20) [ 2] BdsDxe.dll
> > PC 0x00013B8C6FE4 (0x00013B8C5000+0x00001FE4) [ 2] BdsDxe.dll
> > PC 0x00013B8C8680 (0x00013B8C5000+0x00003680) [ 2] BdsDxe.dll
> > PC 0x00013F10F88C (0x00013F10D000+0x0000288C) [ 3] DxeCore.dll
> > PC 0x00013F10E8C0 (0x00013F10D000+0x000018C0) [ 3] DxeCore.dll
> > PC 0x00013F10E024 (0x00013F10D000+0x00001024) [ 3] DxeCore.dll
> >
> > [ 0] Build/ArmVirtQemu-AARCH64/NOOPT_GCC5/AARCH64/ArmPkg/Drivers/CpuDxe/CpuDxe/DEBUG/ArmCpuDxe.dll
> > [ 1] Build/ArmVirtQemu-AARCH64/NOOPT_GCC5/AARCH64/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll
> > [ 2] Build/ArmVirtQemu-AARCH64/NOOPT_GCC5/AARCH64/MdeModulePkg/Universal/BdsDxe/BdsDxe/DEBUG/BdsDxe.dll
> > [ 3] Build/ArmVirtQemu-AARCH64/NOOPT_GCC5/AARCH64/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll
> >
> >   X0 0x0000AFAFAFAFA000   X1 0x0000AFAFAFAFA000   X2 0x00000000000C0000   X3 0x0000000000000004
> >   X4 0x0000000000000200   X5 0x0000000000000000   X6 0x006000000000070C   X7 0x0000000000000000
> >   X8 0x0000000000C5183D   X9 0x0000000000C5183C  X10 0x000000013A216000  X11 0x000000013B643FFF
> >  X12 0x0000000000000000  X13 0x0000000000000008  X14 0x0000000000000000  X15 0x0000000000000000
> >  X16 0x00000000220A3B01  X17 0x000000013F10C660  X18 0x00000000CB9AE5A3  X19 0x0000000137CC5000
> >  X20 0x0000000000000002  X21 0x0000000138020000  X22 0x000000013F158008  X23 0x000000000003F015
> >  X24 0x00000000F8CB0000  X25 0x0000000000000030  X26 0x00000000000FFFFF  X27 0x0000000000100000
> >  X28 0x0000000000000000   FP 0x000000013F10C530   LR 0x000000013BA18A30
> >
> >   V0 0x0000000000000000 0000000000000000   V1 0x0000000000000000 000000009E446499
> >   V2 0x0000000000000000 00000000E35EF5EF   V3 0x0000000000000000 000000005A378E55
> >   V4 0x0000000000000000 00000000A394033C   V5 0x0000000000000000 0000000137F566D8
> >   V6 0x0000000000000000 00000000430E721A   V7 0x0000000000000000 00000000E531AE60
> >   V8 0x0000000000000000 0000000000000000   V9 0x0000000000000000 0000000000000000
> >  V10 0x0000000000000000 0000000000000000  V11 0x0000000000000000 0000000000000000
> >  V12 0x0000000000000000 0000000000000000  V13 0x0000000000000000 0000000000000000
> >  V14 0x0000000000000000 0000000000000000  V15 0x0000000000000000 0000000000000000
> >  V16 0x0000000000000000 0000000000000000  V17 0x0000000000000000 0000000000000000
> >  V18 0x0000000000000000 0000000000000000  V19 0x0000000000000000 0000000000000000
> >  V20 0x0000000000000000 0000000000000000  V21 0x0000000000000000 0000000000000000
> >  V22 0x0000000000000000 0000000000000000  V23 0x0000000000000000 0000000000000000
> >  V24 0x0000000000000000 0000000000000000  V25 0x0000000000000000 0000000000000000
> >  V26 0x0000000000000000 0000000000000000  V27 0x0000000000000000 0000000000000000
> >  V28 0x0000000000000000 0000000000000000  V29 0x0000000000000000 0000000000000000
> >  V30 0x0000000000000000 0000000000000000  V31 0x0000000000000000 0000000000000000
> >
> >   SP 0x000000013F10C530  ELR 0x000000013BA184F0  SPSR 0x80000205  FPSR 0x00000000
> >  ESR 0x96000004          FAR 0x0000AFAFAFAFA000
> >
> >  ESR : EC 0x25  IL 0x1  ISS 0x00000004
> >
> > Data abort: Translation fault, zeroth level
> >
> > Stack dump:
> >   000013F10C430: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> >   000013F10C450: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> >   000013F10C470: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> >   000013F10C490: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> >   000013F10C4B0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> >   000013F10C4D0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> >   000013F10C4F0: 0000000000000000 0000000000000000 000000013BA1E140 0000000060000304
> >   000013F10C510: 0000000000000000 0000000096000004 0000AFAFAFAFA000 00000000FFFFFFD0
> > > 000013F10C530: 000000013F10C560 000000013BA18A30 01000000C0000000 0000AFAFAFAFA000
> >   000013F10C550: 00600000C000070D 0000000000000000 000000013F10C5E0 000000013BA18868
> >   000013F10C570: 0000000000000002 000000013A216000 0000000000000000 006000000000070C
> >   000013F10C590: 0000000100000000 00000000C0000000 0000000000000001 00000004FFFFFFD0
> >   000013F10C5B0: 00600000C000070D 000000013A216000 00000000C0200000 00000000001FFFFF
> >   000013F10C5D0: 000000000000002B 0000AFAFAFAFA000 000000013F10C660 000000013BA188D0
> >   000013F10C5F0: 0000000000000001 000000013FFFE000 FF9F000000000F3F 0000000000000000
> >   000013F10C610: 0000000137CC5000 00000000F8CB0000 000000013F10C630 00000000FFFFFFD0
> > ASSERT [ArmCpuDxe] ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c(273): ((BOOLEAN)(0==1))
>
>
> (3) I bisected the 6-patch range, to find the culprit. Here's the log:
>
> > git bisect start
> > # bad: [31c189a32c370d4abf7a596f7f60a0578e8c7672] ArmPkg/ArmMmuLib AARCH64: preserve attributes when replacing a table entry
> > git bisect bad 31c189a32c370d4abf7a596f7f60a0578e8c7672
> > # good: [a3e25cc8a1dd3d1ea24ed02f90c44221e015e965] OvmfPkg/X86QemuLoadImageLib: fix "unused variable" error in X64 DXE builds
> > git bisect good a3e25cc8a1dd3d1ea24ed02f90c44221e015e965
> > # good: [d4b4f49e4d43276cf410778f172eecab7fc472dc] ArmPkg/ArmMmuLib AARCH64: drop pointless page table memory type check
> > git bisect good d4b4f49e4d43276cf410778f172eecab7fc472dc
> > # good: [8952f2498dcdc2dfbba8216007bb0a4745d944b2] ArmPkg/ArmMmuLib AARCH64: use helpers to determine table entry types
> > git bisect good 8952f2498dcdc2dfbba8216007bb0a4745d944b2
> > # first bad commit: [31c189a32c370d4abf7a596f7f60a0578e8c7672] ArmPkg/ArmMmuLib AARCH64: preserve attributes when replacing a table entry
>
> Therefore, I decided to push the first four patches (first two sets),
> via <https://github.com/tianocore/edk2/pull/431>, and to report a
> problem with this one.
>

Thanks, Laszlo, for taking care of this.

I had another report of some issues with the new MMU code, and I'll
take a look today.

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#56269): https://edk2.groups.io/g/devel/message/56269
Mute This Topic: https://groups.io/mt/71795104/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.