Re: [PATCH] Allow acpi-tmr size=2

[Michael S. Tsirkin]

On Mon, Jul 13, 2020 at 10:43:19AM +0300, Michael Tokarev wrote:
> 13.07.2020 10:20, Michael Tokarev пишет:
> > 12.07.2020 15:00, Simon John wrote:
> >> macos guests no longer boot after commit 
> >> 5d971f9e672507210e77d020d89e0e89165c8fc9
> >>
> >> acpi-tmr needs 2 byte memory accesses, so breaks as that commit only 
> >> allows 4 bytes.
> >>
> >> Fixes: 5d971f9e672507210e7 (memory: Revert "memory: accept mismatching 
> >> sizes in memory_region_access_valid")
> >> Buglink: https://bugs.launchpad.net/qemu/+bug/1886318
> > 
> > Actually this fixes 77d58b1e47c8d1c661f98f12b47ab519d3561488
> > Author: Gerd Hoffmann <kraxel@redhat.com>
> > Date:   Thu Nov 22 12:12:30 2012 +0100
> > Subject: apci: switch timer to memory api
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > 
> > because this is the commit which put min_access_size = 4 in there
> > (5d971f9e672507210e7 is just a messenger, actual error were here
> > earlier but it went unnoticed).
> > 
> > While min_access_size=4 was most likely an error, I wonder why
> > we use 1 now, while the subject says it needs 2? What real min
> > size is here for ACPI PM timer?
> 
> Actually it is more twisted than that. We can't just change the size,
> we must update the corresponding code too.
> 
> 
> static uint64_t acpi_pm_tmr_read(void *opaque, hwaddr addr, unsigned width)
> {
>     return acpi_pm_tmr_get(opaque);
> }
> 
> note the actual read function does not even know neither the requested
> address nor the requested width, it assumes the min/max constraints
> are enforced and the read goes to all 4 bytes. If this pm timer can
> be read byte-by-byte, we should return the right byte of the value,
> not always the whole value.
> 
> /mjt


I think that specifying .impl.min_access_size is a way to do that easily
without major code changes.

-- 
MST