Re: [Qemu-devel] [PATCH 3/3] hw/arm/virt-acpi-build: Add second UART to ACPI tables

[Andrew Jones]

On Tue, Dec 12, 2017 at 02:28:51PM +0100, Laszlo Ersek wrote:
> On 12/12/17 13:47, Peter Maydell wrote:
> > On 12 December 2017 at 12:41, Laszlo Ersek <address@hidden> wrote:
> >> I agree. There's one user-visible complication: while with one UART,
> >> it's not possible to mix things up, with two UARTs, users will
> >> inevitably want to assign inverse roles to them, relative to what QEMU /
> >> the firmware assigns originally.
> >>
> >> E.g. if one PL011 is redirected to a regular file (debug messages) and
> >> the other one to stdio (console), there will be complaints that "I
> >> wanted it the other way around". The redirection happens on the backend
> >> (chardev) level, but the firmware won't see the difference on the
> >> frontend (PL011) level.
> >>
> >> Is it possible to add a new property to the UART nodes in the DTB, like
> >> "purpose"?
> > 
> > This is what the "chosen" stdout-path node in the DTB is for,
> > but you said you didn't want to look at that :-) (it means
> > "device to be used for boot console output".)
> 
> :(
> 
> Ard is right that we really shouldn't do that kind of parsing magic in
> very early UEFI stuff.
> 
> 
> >> Or can we make a virt-arm policy that "first -serial is always ...,
> >> second -serial is always ..."?
> > 
> > Well, the first -serial will always be the 0x09000000 one
> > that we have today, and the second -serial will be the other
> > one (unless you have -machine secure=yes, in which case
> > second -serial is the secure-only UART and third -serial is
> > the second NS UART).
> > 
> > Is this any different to the x86 case, where there are two
> > UARTs at different IO port/IRQ locations?
> 
> OVMF (x86) uses two distinct devices for the two purposes discussed.
> 
> * It uses the "debug console device" for debug message output, at
> hard-coded IO port 0x402; so if you'd like to capture those messages,
> then you have to add:
> 
>   -chardev file,id=debugfile,path=debug.log \
>   -device isa-debugcon,iobase=0x402,chardev=debugfile \
> 
> (or the more traditional
> 
>   -debugcon file:debug.log \
>   -global isa-debugcon.iobase=0x402 \
> )
> 
> * For console I/O, it uses the first serial port. (I think I have once

If "first" here means the first '-serial' argument on the command line,
then mach-virt has the same requirement. The console must be the first
'-serial' argument. The secure UART, provided when the 'secure' property
is set, gets the next one. So

 -chardev file,path=secure-uart.log,id=s -serial stdio -serial chardev:s

works as expected, but

 -chardev file,path=secure-uart.log,id=s -serial chardev:s -serial stdio

does not.

This series keeps that requirement and adds that the second NS UART's
'-serial' argument must come after the secure UART, when the machine
has the 'secure' property set.

Command line ordering requirements aren't awesome, so it might be nice
to add a serial option 'console', allowing

 -chardev file,path=secure-uart.log,id=s -serial chardev:s -serial stdio,console

to work too.

> investigated what it takes to use other serial ports for console I/O --
> I'm no longer sure if "it happens to be impossible in OVMF", or just
> "nobody ever does that".)
> 
> 
> The important distinction (on the UEFI level anyway) is that the debug
> messages need to go to a super dumb device, while console I/O can use a
> much higher-level serial IO abstraction ("protocol").
> 
> 
> > I think the only thing users really expect is that if you're
> > using just the one serial port for anything then it's the
> > first one.
> 
> You are right -- we've never had two (non-secure) UARTs on virt-arm, so
> we can invent whatever assignment, as long as:
> 
> - the single UART case doesn't break (keeps receiving both debug output
> and console IO),
> 
> - whatever we invent for the two UARTs case now, it remains consistent
> over time. I think this implies we can rely on the FDT node ordering in
> the firmware (if we want to use the 2nd UART at all, that is), and then
> QEMU shouldn't change the serial_hds[] <-> FDT node mapping in the
> future, in machvirt_init() and the callees thereof.
> 
> Thanks!
> Laszlo
> 

Personally I think AAVMF should learn how to parse stdout-path, but as
an interim solution it could just hard code the console address
(0x9000000), like it does the base of memory (which I think it should
eventually learn to get from the DT, pointed to by register x0, too :-)

Always generating the second UART's FDT node with status = "disabled",
as Ard suggests, even when the user has provided a '-serial' command
line option for it, i.e. serial_hds[i] != NULL, i > 0, doesn't seem like
what we'd want to do. Wouldn't Linux assume (when boot directly without
AAVMF) that the UART should not be used in that case?

Thanks,
drew