Re: [PATCH 1/5] OvmfPkg/VirtioNetDxe: map VRING using VirtioRingMap()

[Laszlo Ersek <lersek@redhat.com>]

On 09/06/17 00:18, Ard Biesheuvel wrote:
> On 5 September 2017 at 22:59, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 09/05/17 23:11, Ard Biesheuvel wrote:

>>> (which means it could be the second time it was called).
>>
>> Side remark: the CoreExitBootServices() implementation does not notice
>> memory map changes incurred by the ExitBootServices() handlers, in my
>> interpretation.
>>
>> CoreExitBootServices() has a MapKey (= "memmap generation") check early
>> on, in CoreTerminateMemoryMap(). This check catches memmap changes
>> between the last GetMemoryMap(), and the call to ExitBootServices().
>>
>> After this check succeeds, the notify functions are kicked, and on this
>> path, CoreExitBootServices() simply cannot return any other value than
>> EFI_SUCCESS. So whatever mess the individual callbacks make, it doesn't
>> notice or report.
>>
> 
> CoreExitBootServices() disables the timer first, and so it will return
> with event dispatch disabled if it fails, ensuring that it is no
> longer possible for an event handler to be invoked between
> GetMemoryMap and ExitBootServices.

True, but that's not what I meant -- I meant that, if an EBS handler
violates its contract and changes the memory map (= "it makes mess"), on
the call stack of

  CoreNotifySignalList (&gEfiEventExitBootServicesGuid);

then CoreExitBootServices() won't notice it, it won't return an error,
and the caller of EBS() won't go back to calling GetMemoryMap() again.

> Indeed. I think it is justified to treat the IOMMU protocol specially.

The MemoryProtectionExitBootServicesCallback() function call looks like
prior art for such customizations.

This would mean more delay for SEV, but it would be a very desirable
cleanup, for the long term!

Thanks!
Laszlo