Re: [edk2-devel] [PATCH 0/9] ArmVirtPkg: support two PL011 UARTs

["Laszlo Ersek" <lersek@redhat.com>]

On 10/26/23 17:21, Ard Biesheuvel wrote:
> On Thu, 26 Oct 2023 at 17:19, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> On 10/26/23 16:21, Peter Maydell wrote:
>>> On Tue, 10 Oct 2023 at 16:33, Laszlo Ersek <lersek@redhat.com> wrote:
>>>> On 10/10/23 09:43, Ard Biesheuvel wrote:
>>>>> Thanks for looking into this - a cleanup was overdue here.
>>>>>
>>>>> I will take a look in more detail later, but one thing that occurred
>>>>> to me when reading this overview is that having a separate DEBUG
>>>>> serial port would permit us to
>>>>>
>>>>> a) remove it from the DT
>>>>
>>>> ... as in, hide it from Linux, I assume?
>>>>
>>>>> b) add a runtime mapping for it
>>>>> c) keep using it after ExitBootServices
>>>>>
>>>>> This could be useful for debugging issues with the variable store etc.
>>>>>
>>>>> Not saying this is something to address in this series, but I'd like
>>>>> to hear your take on this.
>>>>>
>>>>
>>>> Sounds like a useful feature.
>>>>
>>>> I see four challenges:
>>>>
>>>>
>>>> (1) We'd have to coordinate it with Peter. If we hide any one of the
>>>> serial ports from Linux, that may not be what QEMU intends for Linux to
>>>> happen. Linux currently ties getties to all serial ports -- via the
>>>> serial* aliases, IIUC. Thus, some "positive identification" in the DT
>>>> could be necessary (i.e., that edk2 was welcome to hide that port from
>>>> Linux).
>>>
>>> The potential awkwardness here is that what the guest thinks about
>>> the serial ports depends on the ACPI table fragments which QEMU
>>> provides. EDK2 would need to edit the table fragment to remove any
>>> mention of the second UART if it wanted to hide it from the kernel.
>>> I don't know how hard that would be in EDK2.
>>>
>>> (As far as I'm aware usually a boot via EDK2 doesn't pass the
>>> dtb on to Linux, though I guess there's no reason it can't.)
>>>
>>> From QEMU's point of view, we provide two UARTs to the guest, and we
>>> don't really care whether that means one is used by EDK2 and one by
>>> Linux, or both are used as getty terminals by Linux, or whether the
>>> Linux guest uses one serial as a terminal and leaves the other to its
>>> userspace programs  -- it's all just guest software to us :-)
>>>
>>> [snip other technical stuff]
>>
>> Thanks, good point -- I wasn't aware of the ACPI impact.
>>
>> We don't edit / patch QEMU's ACPI tables, ever. (Beyond obeying the ACPI
>> linker/loader script.) That's a principle we've upheld many times.
>> Whenever ACPI content needs to change, that implies a QEMU patch.
>>
>> So, for this purpose, only the following could have a chance of working:
>>
>> - Expose a new config option on the QEMU command line to the user,
>> regarding the intended use of the serial port(s). This could be of any
>> tolerable form (machine property, front-end (device) property, whatever
>> -- anything that QEMU reviewers can accept).
>>
>> - In QEMU, generate both the DT and the ACPI tables accordingly. The
>> ACPI tables would have to immediately *not* contain the UART-to-hide (so
>> as to keep it secret from the guest OS). The DT at the same time would
>> still have to expose the "runtime DEBUG UART", because edk2 would have
>> to know where that UART was (and that it was meant specifically for OS
>> runtime debug output).
>>
>> - Edk2 would have to patch the DT (we tend to do that already), because
>> (in some configs) we do forward the DT to the guest OS. This need for
>> patching could be lifted if QEMU adopted such a form of expression for
>> the "runtime DEBUG UART" that would be ignored by Linux out of the box.
>>
>>>
>>>> All in all, I think the implementation would be quite a steep divergence
>>>> from, or on top of, this patch set. :)
>>>
>>> I agree with this and with Ard's "not something to address in this
>>> series" comment above; it doesn't sound like this is something that
>>> needs to hold up the patchset we have currently.
>>
>> Right; I'd like to flush this one. The runtime debug UART seems to need
>> more joint pondering.
>>
>>>
>>> Does anybody have time to review Laszlo's code? It would be nice
>>> to be able to get this into the next EDK2 release.
>>
> 
> I'm happy for this to go in if it covers our needs.
> 
> Acked-by: Ard Biesheuvel <ardb@kernel.org>

Thank you, Ard! Merged as commit range 74c687cc2f2f..f945b72331d7 via
<https://github.com/tianocore/edk2/pull/4967>.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110141): https://edk2.groups.io/g/devel/message/110141
Mute This Topic: https://groups.io/mt/101834880/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-