On Wed, Jul 01, 2020 at 12:05:49PM +0100, Anthony PERARD wrote:
> The ACPI spec state that "Accesses to PM1 control registers are
> accessed through byte and word accesses." (In section 4.7.3.2.1 PM1
> Control Registers of my old spec copy rev 4.0a).
>
> With commit 5d971f9e6725 ("memory: Revert "memory: accept mismatching
> sizes in memory_region_access_valid""), it wasn't possible anymore to
> access the pm1_cnt register by reading a single byte, and that is use
> by at least a Xen firmware called "hvmloader".
>
> Also, take care of the PM1 Status Registers which also have "Accesses
> to the PM1 status registers are done through byte or word accesses"
> (In section 4.7.3.1.1 PM1 Status Registers).
>
> Signed-off-by: Anthony PERARD <anthony.per...@citrix.com>
Can't we set impl.min_access_size to convert byte accesses
to word accesses?
> ---
> hw/acpi/core.c | 46 +++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 37 insertions(+), 9 deletions(-)
>
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index 45cbed49abdd..31974e2f91bf 100644
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -394,9 +394,17 @@ uint16_t acpi_pm1_evt_get_sts(ACPIREGS *ar)
> return ar->pm1.evt.sts;
> }
>
> -static void acpi_pm1_evt_write_sts(ACPIREGS *ar, uint16_t val)
> +static void acpi_pm1_evt_write_sts(ACPIREGS *ar, hwaddr addr, uint16_t val,
> + unsigned width)
> {
> uint16_t pm1_sts = acpi_pm1_evt_get_sts(ar);
> + if (width == 1) {
> + if (addr == 0) {
> + val |= pm1_sts & 0xff00;
> + } else if (addr == 1) {
> + val = (val << BITS_PER_BYTE) | (pm1_sts & 0xff);
> + }
> + }
> if (pm1_sts & val & ACPI_BITMASK_TIMER_STATUS) {
> /* if TMRSTS is reset, then compute the new overflow time */
> acpi_pm_tmr_calc_overflow_time(ar);
> @@ -404,8 +412,16 @@ static void acpi_pm1_evt_write_sts(ACPIREGS *ar,
> uint16_t val)
> ar->pm1.evt.sts &= ~val;
> }
>
> -static void acpi_pm1_evt_write_en(ACPIREGS *ar, uint16_t val)
> +static void acpi_pm1_evt_write_en(ACPIREGS *ar, hwaddr addr, uint16_t val,
> + unsigned width)
> {
> + if (width == 1) {
> + if (addr == 0) {
> + val |= ar->pm1.evt.en & 0xff00;
> + } else if (addr == 1) {
> + val = (val << BITS_PER_BYTE) | (ar->pm1.evt.en & 0xff);
> + }
> + }
> ar->pm1.evt.en = val;
> qemu_system_wakeup_enable(QEMU_WAKEUP_REASON_RTC,
> val & ACPI_BITMASK_RT_CLOCK_ENABLE);
> @@ -434,9 +450,11 @@ static uint64_t acpi_pm_evt_read(void *opaque, hwaddr
> addr, unsigned width)
> ACPIREGS *ar = opaque;
> switch (addr) {
> case 0:
> - return acpi_pm1_evt_get_sts(ar);
> + case 1:
> + return acpi_pm1_evt_get_sts(ar) >> (addr * BITS_PER_BYTE);
> case 2:
> - return ar->pm1.evt.en;
> + case 3:
> + return ar->pm1.evt.en >> ((addr - 2) * BITS_PER_BYTE);
> default:
> return 0;
> }
> @@ -448,11 +466,13 @@ static void acpi_pm_evt_write(void *opaque, hwaddr
> addr, uint64_t val,
> ACPIREGS *ar = opaque;
> switch (addr) {
> case 0:
> - acpi_pm1_evt_write_sts(ar, val);
> + case 1:
> + acpi_pm1_evt_write_sts(ar, addr, val, width);
> ar->pm1.evt.update_sci(ar);
> break;
> case 2:
> - acpi_pm1_evt_write_en(ar, val);
> + case 3:
> + acpi_pm1_evt_write_en(ar, addr - 2, val, width);
> ar->pm1.evt.update_sci(ar);
> break;
> }
> @@ -461,7 +481,7 @@ static void acpi_pm_evt_write(void *opaque, hwaddr addr,
> uint64_t val,
> static const MemoryRegionOps acpi_pm_evt_ops = {
> .read = acpi_pm_evt_read,
> .write = acpi_pm_evt_write,
> - .valid.min_access_size = 2,
> + .valid.min_access_size = 1,
> .valid.max_access_size = 2,
> .endianness = DEVICE_LITTLE_ENDIAN,
> };
> @@ -590,19 +610,27 @@ void acpi_pm1_cnt_update(ACPIREGS *ar,
> static uint64_t acpi_pm_cnt_read(void *opaque, hwaddr addr, unsigned width)
> {
> ACPIREGS *ar = opaque;
> - return ar->pm1.cnt.cnt;
> + return ar->pm1.cnt.cnt >> (addr * BITS_PER_BYTE);
> }
>
> static void acpi_pm_cnt_write(void *opaque, hwaddr addr, uint64_t val,
> unsigned width)
> {
> + ACPIREGS *ar = opaque;
> + if (width == 1) {
> + if (addr == 0) {
> + val |= ar->pm1.cnt.cnt & 0xff00;
> + } else if (addr == 1) {
> + val = (val << BITS_PER_BYTE) | (ar->pm1.cnt.cnt & 0xff);
> + }
> + }
> acpi_pm1_cnt_write(opaque, val);
> }
>
> static const MemoryRegionOps acpi_pm_cnt_ops = {
> .read = acpi_pm_cnt_read,
> .write = acpi_pm_cnt_write,
> - .valid.min_access_size = 2,
> + .valid.min_access_size = 1,
> .valid.max_access_size = 2,
> .endianness = DEVICE_LITTLE_ENDIAN,
> };
> --
> Anthony PERARD
