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: Tue, 12 Dec 2017 11:44:55 +0000	[thread overview]
Message-ID: <CAKv+Gu-Ugm1g8twoKwgXAPtDfETvXntk2h4WzMVuUTDF7GhMvA@mail.gmail.com> (raw)
In-Reply-To: <6b4b6351-eeb4-a4d3-8ddf-5401516671ae@redhat.com>

On 12 December 2017 at 11:33, Laszlo Ersek <lersek@redhat.com> wrote:
> On 12/12/17 12:11, 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?
>>> (Perhaps I should have asked this under the second patch, which updates
>>> the DTB generator.)
>>
>> I haven't tested, since I don't have a working setup for that to hand.
>> (Do you have instructions somewhere for getting it working?)
>
> The Wiki page I most frequently refer to (for a libvirt-less description
> anyway) is Ard's:
>
>   https://wiki.linaro.org/LEG/UEFIforQEMU
>
> There's also:
>
>   https://www.kraxel.org/repos/
>   https://fedoraproject.org/wiki/Architectures/AArch64/Install_with_QEMU
>
>> The behaviour we would want would be for the firmware to keep using
>> the PL011 at 0x09000000.
>
> With these QEMU patches, I reckon that's going to happen, yes.
>

IIRC the DT nodes appear in the opposite order as they are added, so
adding the second UART after adding the original one is probably going
to cause trouble.

We do check the 'status' though, so if backward compatibility with
older firmwares is a concern, we could set status=disabled, and let
newer firmware builds enable it on the fly.

>> (In an ideal world the firmware would
>> prefer the UART marked in the 'stdout-path' in the DTB /chosen node,
>> as the kernel does, I guess.)
>
> Hmmm, I recall that we used to have some code related to the /chosen
> node... We have a helper function for locating that
> (GetOrInsertChosenNode), but we no longer use it, it seems? The last
> (only?) use was apparently removed in:
>
> https://github.com/tianocore/edk2/commit/29589acf1010
>
> I'll let Ard comment too.
>

That would require us to implement some non-trivial parsing and alias
resolution, given that stdout-path is simply a string that may refer
to a UART node via an alias.