Re: [PULL 04/21] acpi: ged: add control regs

[Peter Maydell]

On Thu, 17 Sept 2020 at 14:53, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> Add control regs (sleep, reset) for hw-reduced acpi.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Message-id: 20200915120909.20838-5-kraxel@redhat.com
> ---
>  include/hw/acpi/generic_event_device.h | 12 +++++++
>  hw/acpi/generic_event_device.c         | 44 ++++++++++++++++++++++++++
>  2 files changed, 56 insertions(+)

Hi; I've just run across this code because I found a bug in
a different device and was doing a grep to see if anybody
else had made the same mistake...

> +static void ged_regs_write(void *opaque, hwaddr addr, uint64_t data,
> +                           unsigned int size)
> +{
> +    bool slp_en;
> +    int slp_typ;
> +
> +    switch (addr) {
> +    case ACPI_GED_REG_SLEEP_CTL:
> +        slp_typ = (data >> 2) & 0x07;
> +        slp_en  = (data >> 5) & 0x01;
> +        if (slp_en && slp_typ == 5) {
> +            qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> +        }
> +        return;
> +    case ACPI_GED_REG_SLEEP_STS:
> +        return;
> +    case ACPI_GED_REG_RESET:
> +        if (data == ACPI_GED_RESET_VALUE) {
> +            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);

Here we call qemu_system_reset_request(), but we pass it a cause
value of GUEST_SHUTDOWN. Is this trying to do a reset (in which
case it should probably be SHUTDOWN_CAUSE_GUEST_RESET) or a shutdown
(in which case it needs to call qemu_system_shutdown_request()) ?

> +        }
> +        return;
> +    }
> +}

thanks
-- PMM