public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ard Biesheuvel" <ardb@kernel.org>
To: Laszlo Ersek <lersek@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	devel@edk2.groups.io,  Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Gerd Hoffmann <kraxel@redhat.com>,  Julien Grall <julien@xen.org>,
	Leif Lindholm <quic_llindhol@quicinc.com>,
	 Sami Mujawar <sami.mujawar@arm.com>
Subject: Re: [edk2-devel] [PATCH 0/9] ArmVirtPkg: support two PL011 UARTs
Date: Thu, 26 Oct 2023 17:21:21 +0200	[thread overview]
Message-ID: <CAMj1kXFzbmiJQcRDHsyiboQ3vqU4gcC9W+3M-djJ7LiEYb+DVg@mail.gmail.com> (raw)
In-Reply-To: <9e698cf7-7e58-b32b-58e9-e18f54736f75@redhat.com>

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>


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



  reply	other threads:[~2023-10-26 15:21 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-08 15:39 [edk2-devel] [PATCH 0/9] ArmVirtPkg: support two PL011 UARTs Laszlo Ersek
2023-10-08 15:39 ` [edk2-devel] [PATCH 1/9] ArmVirtPkg: introduce FdtSerialPortAddressLib Laszlo Ersek
2023-10-08 15:39 ` [edk2-devel] [PATCH 2/9] ArmVirtPkg/Fdt16550SerialPortHookLib: rebase to FdtSerialPortAddressLib Laszlo Ersek
2023-10-08 15:39 ` [edk2-devel] [PATCH 3/9] ArmVirtPkg: adjust whitespace in block scope declarations Laszlo Ersek
2023-10-08 15:39 ` [edk2-devel] [PATCH 4/9] ArmVirtPkg: adhere to the serial port selected by /chosen "stdout-path" Laszlo Ersek
2023-10-08 15:39 ` [edk2-devel] [PATCH 5/9] ArmVirtPkg: store separate console and debug PL011 addresses in GUID HOB Laszlo Ersek
2023-10-08 15:39 ` [edk2-devel] [PATCH 6/9] ArmVirtPkg: introduce DebugLibFdtPL011Uart Flash instance Laszlo Ersek
2023-10-08 15:39 ` [edk2-devel] [PATCH 7/9] ArmVirtPkg: introduce DebugLibFdtPL011Uart RAM instance Laszlo Ersek
2023-10-08 15:39 ` [edk2-devel] [PATCH 8/9] ArmVirtPkg: introduce DebugLibFdtPL011Uart DXE Runtime instance Laszlo Ersek
2023-10-08 15:39 ` [edk2-devel] [PATCH 9/9] ArmVirtPkg: steer DebugLib output away from SerialPortLib+console traffic Laszlo Ersek
2023-10-10  7:43 ` [edk2-devel] [PATCH 0/9] ArmVirtPkg: support two PL011 UARTs Ard Biesheuvel
2023-10-10 15:33   ` Laszlo Ersek
2023-10-26 14:21     ` Peter Maydell
2023-10-26 14:46       ` Julien Grall
2023-10-26 14:55         ` Ard Biesheuvel
2023-10-26 15:36           ` Laszlo Ersek
2023-10-26 15:30         ` Laszlo Ersek
2023-10-26 15:19       ` Laszlo Ersek
2023-10-26 15:21         ` Ard Biesheuvel [this message]
2023-10-26 19:00           ` Laszlo Ersek
2023-10-27 10:57         ` Gerd Hoffmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAMj1kXFzbmiJQcRDHsyiboQ3vqU4gcC9W+3M-djJ7LiEYb+DVg@mail.gmail.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox