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

[Heyi Guo <guoheyi@huawei.com>]

On 2019/3/20 17:55, Laszlo Ersek wrote:
> 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.)
Nothing at all, and thanks for your time in reading and replying the mails even when out of office :)
>
> 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.
In my RFC, we need to use gDS which is from DxeServicesTableLib->EfiGetSystemConfigurationTable()->UefiLib, so that we need to flatten EfiGetSystemConfigurationTable(). And gDS->SetMemorySpaceAttributes() actually relies on gEfiCpuArchProtocolGuid or it will return EFI_NOT_AVAILABLE_YET.

Thanks,
Heyi

>
>> 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
>
> .
>