Re: [PATCH v2 2/4] ArmPkg/ArmMmuLib ARM: handle unmapped sections when updating permissions

[Leif Lindholm <leif.lindholm@linaro.org>]

On Fri, Nov 30, 2018 at 12:28:27PM +0100, Ard Biesheuvel wrote:
> The ARM ArmMmuLib code currently does not take into account that
> setting permissions on a region should take into account that a
> region may not be mapped yet to begin with.
> 
> So when updating a section descriptor whose old value is zero,
> pass in the address explicitly.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> index ec51e072ab43..889b22867dc7 100644
> --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> @@ -695,8 +695,12 @@ UpdateSectionEntries (
>      } else {
>        // still a section entry
>  
> -      // mask off appropriate fields
> -      Descriptor = CurrentDescriptor & ~EntryMask;
> +      if (CurrentDescriptor != 0) {
> +        // mask off appropriate fields
> +        Descriptor = CurrentDescriptor & ~EntryMask;
> +      } else {
> +        Descriptor = ((UINTN)FirstLevelIdx + i) << TT_DESCRIPTOR_SECTION_BASE_SHIFT;
> +      }
>  
>        // mask in new attributes and/or permissions
>        Descriptor |= EntryValue;
> -- 
> 2.19.1
>