Re: [PATCH 5/5] OvmfPkg/AcpiPlatformDxe: replay QEMU_LOADER_WRITE_POINTER commands at S3

[Laszlo Ersek <lersek@redhat.com>]

On 02/17/17 23:41, Laszlo Ersek wrote:

[snip]

> I briefly considered a QemuFwCfgLib API that lets the caller inquire
> about DMA enablement, but it just doesn't feel right, and (considering
> the above) it's not even useful. Later I would like to abstract away
> this S3 boot script munging, from both this driver and SmmControl2Dxe,
> but the interface is a hard nut:
> 
> - SEC lib instance (exists only for x86): must return EFI_UNSUPPORTED
> - PEI/DXE instance (exists only for x86): must be split into
>   - PEI only: returns EFI_UNSUPPORTED
>   - DXE only:
>     - returns EFI_UNSUPPORTED if S3 is disabled
>     - works if the protocol is available
>     - what does it do if the protocol is not available *yet*?
>       - can't use a DEPEX (whole client module won't load with S3
>         disabled!)
>       - register protocol notify in constructor? then what?
>         - collect operations and write them to boot script at EndOfDxe?
>           - that always turns out to be a bad idea, due to unspecified
>             callback invocation order

One thing I can imagine here is: simply push the dependency handling to
the client module. If the protocol is not found, just return
EFI_NOT_STARTED, and let the client module deal with that. The client
can set its own DEPEX, or register a protocol notify as necessary. So I
guess this could be solved after all, hopefully.

Laszlo

> - DXE only instance (exists for arm/aarch64 only): very different from
>   the x86 impl already (uses chunked MMIO at the bottom, not REP IO),
>   so unification with the above is hard
> 
> While writing this patch, I was racking my brain for hours about this. I
> think I must have restarted this patch three or four times just because
> of that.
> 
> Ultimately, it's the second driver where we use a meaningful boot script
> fragment, and usually the third occurrence of a pattern is where the
> generics start to emerge, and (hopefully) become a candidate for
> factoring out. I dislike the repetitive and hard-to-read nature of the
> opcode additions (hence the profuse comments above), but from the two
> instances we can look at (SmmControl2Dxe and this patch), I couldn't
> really abstract away a natural interface.
> 
> I think it is a relatively big task anyway, deserving its own BZ (not to
> be confused with the also separate BZ that I proposed for residual /
> cleanup patches on top of this series, should you recommend such.) If
> you felt inclined to research / prototype that, that would be huge...
> 
> Thanks!
> Laszlo
> 
>>
>> -Jordan