Re: SetMemorySpaceAttributes with EFI_MEMORY_XP

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

On 20 March 2017 at 11:38, Laszlo Ersek <lersek@redhat.com> wrote:
> On 03/20/17 12:20, Ard Biesheuvel wrote:
>> On 20 March 2017 at 11:16, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote:
>>> Ard, why is SetMSetMemorySpaceAttributes being called in first place?
>>> (ignoring the recent NX patch)
>>> Looking at the initial GCD, it looks like unused memory usually
>>> doesn't have any attributes set anyway.
>>>
>>
>> Originally, we added the new memory with
>> EFI_MEMORY_WB|EFI_MEMORY_WT|EFI_MEMORY_WC|EFI_MEMORY_UC capabilities,
>> and selected the EFI_MEMORY_WB attribute with the call to
>> gDS->SetMemorySpaceAttributes. Later, we removed all capabilities
>> expect EFI_MEMORY_WB, since the other ones cannot be supported under
>> virtualization with KVM.
>>
>> Whether that makes the SetMemorySpaceAttributes redundant is not
>> entirely clear to me, but it does appear the adding the memory does
>> the right thing wrt non-exec permissions if the policy is enabled. So
>> perhaps we can simply drop this call?
>
> Won't that turn off caching for the memory just added?
>

I think it may not map the memory at all in this case, so we need to
do something. It looks like calling mCpu->SetMemoryAttributes() should
be sufficient here, and so I wonder whether we violate anything by
replacing gDS->SetMemorySpaceAttributes with mCpu->SetMemoryAttributes
here.