Re: [RFC 0/3] Enable runtime serial debug for ArmVirtQemu

[Laszlo Ersek <lersek@redhat.com>]

On 03/16/19 10:41, Heyi Guo wrote:
> On 2019/3/13 1:05, Laszlo Ersek wrote:

>> Given that you'd have to implement a "special" SerialPortLib instance
>> just for StatusCodeHandlerRuntimeDxe anyway, can we perhaps:
>>
>> (1) Undo -- dependent on a build flag? -- commit ebfca258f5d7; i.e.,
>> stick universally with BaseDebugLibSerialPort, for DebugLib,
>>
>> (2) add a new implementation of "FdtPL011SerialPortLib.inf" with both
>> (a) runtime behavior and (b) handling of a special serial port?
>>
>> In other words, push down the "runtime?" distinction from DebugLib (see
>> commit 7f029e1b31ee) to SerialPortLib. The new SerialPortLib instance
>> would be used only with DXE_RUNTIME_DRIVERs.
>>
>> The lib instance would prepare everything in advance for accessing the
>> "special" serial port at runtime (which could require allocating runtime
>> memory in advance). Once ExitBootServices() is called, a callback should
>> switch over the behavior of the lib instance, without allocating further
>> memory. (The switched-over behavior would not have to be functional
>> until the virtual address map is set.)
> My first idea was also simply implement a runtime serial port instance,
> but the problem is for the initialization.
> 
> Since serial port lib is only a library, it seems we can only initialize
> everything in SerialPortInitialize() or library constructor, but either
> of which could no work in current EDK2 code framework. Please see my
> explanation in earlier mail as below:
> 
> "The long story is: at first I proposed to still use
> BaseSerialPortDebugLib, so I added RuntimePrepare() function into the
> constructor directly, but the builder complained about looped
> constructors, for RuntimePrepare() invokes gBS and some RunTime
> libraries. Then I tried disabling the constructor and moved
> RuntimePrepare() into SerialPortInitialize() which could complete the
> build, but still couldn't guarantee the calling order of these
> constructors, for BaseSerialPortDebugLib has a constructor to call
> SerialPortInitialize()."

(Sorry about the late response (I've been away for a few days), and also
for missing that construction loop that you apparently mentioned earlier.)

You mention that "RuntimePrepare() invokes gBS and some RunTime
libraries". How important is it to rely on those libraries?

For example, "gBS" is simply a convenience; you can get at the same
thing from the parameter list of the constructor function too (assuming
you use a MODULE_TYPE of (minimally) DXE_DRIVER, in the lib instance's
INF file).

If we don't have to flatten a ridiculous amount of other library code
into the RuntimePrepare() function, I think such flattening would be a
viable approach. We've run into this constructor loop several times
before, and -- if I remember correctly anyway! -- one approach has been
to declare SerialPortLib the "lowest level abstraction", and make the
affected lib instance self-contained.

> Can we use event notify or protocol notify to do the runtime
> intialization? Like EndOfDxe event group. Any other advice?

I guess this could work too. I'm not really a fan of callbacks as long
as we can manage otherwise (for example, we can't propagate errors out
of callbacks), but if the lib flattening thing gets too complex, we
could do this as well.

Regarding EndOfDxe -- I think EndOfDxe is considered a trust barrier
(namely between platform fw modules and 3rd party modules), so I
wouldn't tie our initialization to that, for clarity's sake. ReadyToBoot
seems a tiny bit less "forced" -- it is sufficiently late, you can still
allocate resources, and we can claim it is "on the path towards OS
runtime". Just be aware that ReadyToBoot can be signaled multiple times,
e.g. if some boot options fail (so uninstall the handler / close the
event when serving the first callback I guess).

Thanks
Laszlo