Re: [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

[Laszlo Ersek <lersek@redhat.com>]

On 01/23/19 10:26, Ard Biesheuvel wrote:
> On Wed, 23 Jan 2019 at 10:14, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 01/22/19 16:37, Ard Biesheuvel wrote:

>>> Is SetUefiImageMemoryAttributes() being
>>> called to remap the memory R-X ?
>>
>> No, it is not; the grub binary in question doesn't have the required
>> section alignment (... I hope at least that that's what your question
>> refers to):
>>
>>> ProtectUefiImageCommon - 0x3E6C54C0
>>>   - 0x000000013BEEF000 - 0x0000000000030600
>>> !!!!!!!!  ProtectUefiImageCommon - Section Alignment(0x200) is
>> incorrect  !!!!!!!!
>>
> 
> This is puzzling, given that the exact same binary works on Mustang.

And even on the original (unspecified) hardware, the same binary works
frequently. My understanding is that there are five VMs executing reboot
loops in parallel, on the same host, and 4 out of 5 may hit the issue in
a reasonable time period (300 reboots or so).

> So when loaded, GRUB should cover the following regions:
> 
> 0x13beef0000 - 0x13bf000000 (0x11000)
> 0x13bf000000 - 0x13bf01f600 (0x1f600)
> 
> where neither covers a 2 MB block fully, which means that the TLB
> entry that we are hitting is stale.
> 
> Since ProtectUefiImageCommon() does not do anything in this case, the
> stale translation must be the result of
> PcdDxeNxMemoryProtectionPolicy, which either sets the wrong
> permissions for EfiLoaderCode (relying on ProtectUefiImageCommon), or
> we don't flush the TLBs correctly after updating the permissions when
> converting the memory from EfiConventionalMemory to EfiLoaderCode
> 
> Are you using the default value for PcdDxeNxMemoryProtectionPolicy?

Yes, we have

ArmVirtPkg/ArmVirt.dsc.inc:
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1

from commit 1acd7c54a724 ("ArmVirtPkg AARCH64: enable NX memory
protection for all platforms", 2017-03-01).

The binary is from the RPM
"edk2-aarch64-20180508gitee3198e672e2-5.el8+1789+f0947240.noarch", which
is basically upstream ee3198e672e2 plus a small number of backports and
downstream customizations.

Thanks!
Laszlo