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

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Laszlo Ersek <lersek@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Shannon Zhao <zhaoshenglong@huawei.com>,
	qemu-arm <qemu-arm@nongnu.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	"Jason A . Donenfeld" <Jason@zx2c4.com>,
	Drew Jones <drjones@redhat.com>,
	Andrea Bolognani <abologna@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 3/3] hw/arm/virt-acpi-build: Add second UART to ACPI tables
Date: Wed, 13 Dec 2017 16:46:46 +0000	[thread overview]
Message-ID: <CAKv+Gu-uPf9QLobM1Vqdek_rSPfZcAXV24Ybg83hZvHbH5LqHw@mail.gmail.com> (raw)
In-Reply-To: <0355e1ee-2f48-81ce-55e6-bed7bdeac771@redhat.com>

On 13 December 2017 at 16:01, Laszlo Ersek <lersek@redhat.com> wrote:
> On 12/13/17 14:56, Peter Maydell wrote:
>> On 12 December 2017 at 11:06, Laszlo Ersek <lersek@redhat.com> wrote:
>>> BTW, has anyone tested this with the ArmVirtQemu firmware? As far as I
>>> can see from the firmware code, the firmware will use the PL011 whose
>>> description comes first in the DTB (and ignore the other PL011), in an
>>> fdt_next_node() traversal. Is that OK for the intended use case?
>>
>> I have now tested this,
>
> Thank you!
>
>> and annoyingly UEFI and the kernel seem
>> to disagree about enumeration order. That is, if QEMU creates
>> them in the code in the order 0x09050000 (uart 2), 0x09000000 (uart 1),
>> then they appear in the dtb with uart 1 first, and the kernel enumerates
>> them as ttyAMA0 being uart 1 and ttyAMA1 being uart 2, but UEFI
>> outputs to uart 2...
>
> Ouch. This reminds me (remotely) of QEMU commit 587078f0ed63
> ("hw/arm/virt: explain device-to-transport mapping in
> create_virtio_devices()", 2015-02-05).
>
> I'd still like to avoid the "sophisticated" /chosen lookup (the lookup
> itself is not too complex, but evaluating whatever we find there against
> each of the scanned UART nodes appears difficult, if I'm to understand
> Ard's earlier point correctly).

This may be a lot simpler than I originally thought. stdout-path
contains a string of the from

<nodename-or-alias>[:<baudrate[<parity>[<data-bits>[<flow-control>]]]]

where nodename-or-alias can be resolved into a path via fdt_get_alias
(), or used directly as a path otherwise.

> I hope that we can match the kernel's
> logic with simple modifications to our scanning loops, e.g. we could
> simply pick the last UART rather than the first, or else do a maximum
> (or minimum) search for the UART base, and stick with the maximum (or
> minimum) found.
>
> However, for that, we first have to understand what the kernel does. Can
> someone explain that? (I tried taking a look, but it's turtles all the
> way down.)
>

The kernel uses the contents of the SPCR table or /chosen/stdout-path
as the console unless overridden on the kernel command line. This is
independent of enumeration order.