Re: [edk2-devel] [PATCH v3 0/3] ArmPkg/ArmMmuLib AARCH64: correctness fix

["Laszlo Ersek" <lersek@redhat.com>]

On 03/25/20 16:29, Ard Biesheuvel wrote:
> The new ArmMmuLib code is easier to reason about, so that is what I did:
> currently, when we create mappings that cover existing table entries, we
> may end up overwriting those with block entries without taking the mapping
> attributes of the original table entries into account. So let's fix this.
> 
> I honestly don't know whether the original code was better at dealing with
> this: I do remember some changes from Heyi that may have been related, but
> the old code is not easy to follow. In any case, I didn't manage to hit this
> case in practice, given that we typically start out with large mappings, and
> break them down later (to set permissions), rather than the other way around.
> 
> Patch #1 adds some helpers to hide the insane way the type bits change
> meaning when you change to level 3.
> 
> Patch #2 ensures that we only replace (and free) table entries with block
> entries if it is guaranteed that doing so will not lose any attribute
> information.
> 
> Changes since v2:
> - add patch to limit recursion to levels < 3 in FreePageTablesRecursive()
> 
> Changes since v1:
> - zero newly allocated pages before splitting a block entry into a table
>   entry, to avoid garbage in that page being misidentified as entry type
>   attributes - this should fix the crash observed by Laszlo
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Ashish Singhal <ashishsingha@nvidia.com>
> 
> Ard Biesheuvel (3):
>   ArmPkg/ArmMmuLib AARCH64: limit recursion when freeing page tables
>   ArmPkg/ArmMmuLib AARCH64: use helpers to determine table entry types
>   ArmPkg/ArmMmuLib AARCH64: preserve attributes when replacing a table
>     entry
> 
>  .../Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 83 +++++++++++++++----
>  1 file changed, 68 insertions(+), 15 deletions(-)
> 

Tested-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo