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

[Peter Maydell]

On Mon, 6 Nov 2023 at 14:05, Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Fri, 3 Nov 2023 15:26:22 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:
>
> > 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.
>
> it might be worth mentioning that QEMU impl. uses 32 bit registers and
> can correctly handle 32 bit access only, while 16 (or any other) bit access
> to 32 bit registers won't actually work.
>
> ex:
> pl011_write()
>    ...
>    switch (offset >> 2)
>
> essentially only 1st byte will be accessed correctly,
> the rest will be misplaced as read/write handlers do not account
> for split access possibility.

No, 16 bit accesses should work OK -- we set our .impl.min_access_size
and .impl.max_access_size to 4, so the memory subsystem should
implement 8 and 16 bit accesses for us by converting them to
32 bit accesses.

thanks
-- PMM