Re: [PATCH v3 2/3] hw/arm/virt: Report correct register sizes in ACPI DBG2/SPCR tables.

[Peter Maydell <peter.maydell@linaro.org>]

On Fri, 3 Nov 2023 at 15:21, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> From: Udo Steinberg <udo@hypervisor.org>
>
> Documentation for using the GAS in ACPI tables to report debug UART addresses at
> https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/acpi-debug-port-table
> states the following:
>
> - The Register Bit Width field contains the register stride and must be a
>   power of 2 that is at least as large as the access size.  On 32-bit
>   platforms this value cannot exceed 32.  On 64-bit platforms this value
>   cannot exceed 64.
> - The Access Size field is used to determine whether byte, WORD, DWORD, or
>   QWORD accesses are to be used.  QWORD accesses are only valid on 64-bit
>   architectures.
>
> Documentation for the ARM PL011 at
> https://developer.arm.com/documentation/ddi0183/latest/
> states that the registers are:
>
> - spaced 4 bytes apart (see Table 3-2), so register stride must be 32.
> - 16 bits in size in some cases (see individual registers), so access
>   size must be at least 2.
>
> Linux doesn't seem to care about this error in the table, but it does
> affect at least the NOVA microhypervisor.
>
> In theory we therefore have a choice between reporting the access
> size as 2 (16 bit accesses) or 3 (32-bit accesses).  In practice,
> Linux does not correctly handle the case where the table reports the
> access size as 2: as of kernel commit 750b95887e5678, the code in
> acpi_parse_spcr() tries to tell the serial driver to use 16 bit
> accesses by passing "mmio16" in the option string, but the PL011
> driver code in pl011_console_match() only recognizes "mmio" or
> "mmio32". The result is that unless the user has enabled 'earlycon'
>
Oops, a line seems to have got deleted here -- should continue

"there is no console output from the guest kernel."

> We therefore choose to report the access size as 32 bits; this works
> for NOVA and also for Linux.  It is also what the UEFI firmware on a
> Raspberry Pi 4 reports, so we're in line with existing real-world
> practice.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1938
> Signed-off-by: Udo Steinberg <udo@hypervisor.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> [PMM: minor commit message tweaks; use 32 bit accesses]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> --

thanks
-- PMM