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

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

On Wed, 25 Mar 2020 at 20:49, Laszlo Ersek <lersek@redhat.com> wrote:
>
> 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>
>

Pushed to master

Thanks all