PATCH: #1938: [ARM/PL011] Wrong UART register spacing reported in DBG2/SPCR

[Udo Steinberg]

From: Udo Steinberg <udo@hypervisor.org>
Date: Sat, 21 Oct 2023 14:10:30 +0200
Subject: [PATCH] hw/arm/virt: Report correct register sizes in ACPI DBG2/SPCR
 tables.

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 (see individual registers), so access size must be 2.

The PL011 documentation does not mention whether 8-bit accesses or 32-bit
accesses to the registers are also allowed. Because a standard PL011 (not
the SBSA version) is connected via a 16-bit bus using PWDATA[15:0] and
PRDATA[15:0] (see Figure 2-1), using 16-bit access is the safest choice.

For SBSA-compatible UARTs the DBG2/SPCR table should report a different
subtype (0xd or 0xe) instead of 0x3.

Fixes #1938.

Signed-off-by: Udo Steinberg <udo@hypervisor.org>
---
 hw/arm/virt-acpi-build.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 9ce136cd88..91ed7fc94a 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -482,7 +482,7 @@ build_spcr(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
     build_append_int_noprefix(table_data, 3, 1); /* ARM PL011 UART */
     build_append_int_noprefix(table_data, 0, 3); /* Reserved */
     /* Base Address */
-    build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 8, 0, 1,
+    build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 32, 0, 2,
                      vms->memmap[VIRT_UART].base);
     /* Interrupt Type */
     build_append_int_noprefix(table_data,
@@ -673,7 +673,7 @@ build_dbg2(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
     build_append_int_noprefix(table_data, 34, 2);
 
     /* BaseAddressRegister[] */
-    build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 8, 0, 1,
+    build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 32, 0, 2,
                      vms->memmap[VIRT_UART].base);
 
     /* AddressSize[] */
-- 
2.42.0

pgp34WwfirRIS.pgp
Description: OpenPGP digital signature