Re: [PATCH 1/4] ArmPkg/CpuDxe: Correct EFI_MEMORY_RO usage

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

On Fri, Feb 10, 2017 at 06:25:00PM +0000, Ard Biesheuvel wrote:
> On 10 February 2017 at 18:17, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Thu, Feb 09, 2017 at 05:38:08PM +0000, Ard Biesheuvel wrote:
> >> From: Jiewen Yao <jiewen.yao@intel.com>
> >>
> >> Current Arm CpuDxe driver uses EFI_MEMORY_WP for write protection,
> >> according to UEFI spec, we should use EFI_MEMORY_RO for write protection.
> >> The EFI_MEMORY_WP is the cache attribute instead of memory attribute.
> >>
> >> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> > No objections to this patch, but I would have expected it to be 4/4,
> > if it caused issues requiring the other 3 to be created?
> >
> 
> Not quite: it is the feature itself that requires these fixes, and
> this patch actually makes sense as 1/4, since it removes uses of
> EFI_MEMORY_WP that are no longer appropriate. Implementing 2-4 with
> EFI_MEMORY_WP instead of EFI_MEMORY_RO and then changing it at the end
> would make no sense at all.

OK, so basically, the issue was already in the existing code?

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