On Mon, 26 Jun 2023 08:29:19 -0400
"Michael S. Tsirkin" <m...@redhat.com> wrote:
> From: BALATON Zoltan <bala...@eik.bme.hu>
>
> On pegasos2 which has ACPI as part of VT8231 south bridge the board
> firmware writes PM control register by accessing the second byte so
> addr will be 1. This wasn't handled correctly and the write went to
> addr 0 instead. Remove the acpi_pm1_cnt_write() function which is used
> only once and does not take addr into account and handle non-zero
> address in acpi_pm_cnt_{read|write}. This fixes ACPI shutdown with
> pegasos2 firmware.
>
> The issue below is possibly related to the same memory core bug.
>
> Link: https://gitlab.com/qemu-project/qemu/-/issues/360
> Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu>
> Message-Id: <20230607200125.a9988746...@zero.eik.bme.hu>
> Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
> Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
Somewhere you lost mine
Reviewed-by: Igor Mammedov <imamm...@redhat.com>
> ---
> hw/acpi/core.c | 56 +++++++++++++++++++++++++-------------------------
> 1 file changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index 6da275c599..00b1e79a30 100644
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -551,8 +551,35 @@ void acpi_pm_tmr_reset(ACPIREGS *ar)
> }
>
> /* ACPI PM1aCNT */
> -static void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
> +void acpi_pm1_cnt_update(ACPIREGS *ar,
> + bool sci_enable, bool sci_disable)
> {
> + /* ACPI specs 3.0, 4.7.2.5 */
> + if (ar->pm1.cnt.acpi_only) {
> + return;
> + }
> +
> + if (sci_enable) {
> + ar->pm1.cnt.cnt |= ACPI_BITMASK_SCI_ENABLE;
> + } else if (sci_disable) {
> + ar->pm1.cnt.cnt &= ~ACPI_BITMASK_SCI_ENABLE;
> + }
> +}
> +
> +static uint64_t acpi_pm_cnt_read(void *opaque, hwaddr addr, unsigned width)
> +{
> + ACPIREGS *ar = opaque;
> + return ar->pm1.cnt.cnt >> addr * 8;
> +}
> +
> +static void acpi_pm_cnt_write(void *opaque, hwaddr addr, uint64_t val,
> + unsigned width)
> +{
> + ACPIREGS *ar = opaque;
> +
> + if (addr == 1) {
> + val = val << 8 | (ar->pm1.cnt.cnt & 0xff);
> + }
> ar->pm1.cnt.cnt = val & ~(ACPI_BITMASK_SLEEP_ENABLE);
>
> if (val & ACPI_BITMASK_SLEEP_ENABLE) {
> @@ -575,33 +602,6 @@ static void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t
> val)
> }
> }
>
> -void acpi_pm1_cnt_update(ACPIREGS *ar,
> - bool sci_enable, bool sci_disable)
> -{
> - /* ACPI specs 3.0, 4.7.2.5 */
> - if (ar->pm1.cnt.acpi_only) {
> - return;
> - }
> -
> - if (sci_enable) {
> - ar->pm1.cnt.cnt |= ACPI_BITMASK_SCI_ENABLE;
> - } else if (sci_disable) {
> - ar->pm1.cnt.cnt &= ~ACPI_BITMASK_SCI_ENABLE;
> - }
> -}
> -
> -static uint64_t acpi_pm_cnt_read(void *opaque, hwaddr addr, unsigned width)
> -{
> - ACPIREGS *ar = opaque;
> - return ar->pm1.cnt.cnt;
> -}
> -
> -static void acpi_pm_cnt_write(void *opaque, hwaddr addr, uint64_t val,
> - unsigned width)
> -{
> - acpi_pm1_cnt_write(opaque, val);
> -}
> -
> static const MemoryRegionOps acpi_pm_cnt_ops = {
> .read = acpi_pm_cnt_read,
> .write = acpi_pm_cnt_write,
