Re: [Qemu-devel] [PATCH v2 13/33] acpi: extend ACPI interface to provide send_event hook

[Igor Mammedov]

On Thu, 2 Jun 2016 14:09:12 +0300
Marcel Apfelbaum <address@hidden> wrote:

> On 05/31/2016 12:57 PM, Igor Mammedov wrote:
> > send_event() hook will allow to send ACPI event in
> > a target specific way (GPE or GPIO based impl.)
> > it will also simplify proxy wrappers in piix4pm/ich9
> > that access ACPI regs and SCI which are part of
> > piix4pm/lcp_ich9 devices and call acpi_foo() API directly.
> >
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> > Following patch will use hook to simplify hotplug callbacks
> > in piix4pm/ich9.
> >
> > v2:
> >   -  make ACPI_SEND_EVENT macro a function
> >         Marcel Apfelbaum <address@hidden>
> > ---
> >   hw/acpi/acpi_interface.c             |  9 +++++++++
> >   hw/acpi/core.c                       |  2 +-
> >   hw/acpi/piix4.c                      |  8 ++++++++
> >   hw/isa/lpc_ich9.c                    |  8 ++++++++
> >   include/hw/acpi/acpi.h               | 10 ++--------
> >   include/hw/acpi/acpi_dev_interface.h | 11 +++++++++++
> >   6 files changed, 39 insertions(+), 9 deletions(-)
> >
> > diff --git a/hw/acpi/acpi_interface.c b/hw/acpi/acpi_interface.c
> > index d821313..6583917 100644
> > --- a/hw/acpi/acpi_interface.c
> > +++ b/hw/acpi/acpi_interface.c
> > @@ -2,6 +2,15 @@
> >   #include "hw/acpi/acpi_dev_interface.h"
> >   #include "qemu/module.h"
> >
> > +void acpi_send_event(DeviceState *dev, AcpiEventStatusBits event)
> > +{
> > +    AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(dev);  
> 
> 
> Is the above conversion safe? Sorry for not mention it earlier.
It should assert if dev doesn't implement interface,
and that's fine as acpi_send_event() is supposed to be only used
by device that handles ACPI hardware side and implements that interface.

> Thanks,
> Marcel
> 
> 
> > +    if (adevc->send_event) {
> > +        AcpiDeviceIf *adev = ACPI_DEVICE_IF(dev);
> > +        adevc->send_event(adev, event);
> > +    }
> > +}
> > +
> >   static void register_types(void)
> >   {
> >       static const TypeInfo acpi_dev_if_info = {
> > diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> > index 1ffd155..d24b9a9 100644
> > --- a/hw/acpi/core.c
> > +++ b/hw/acpi/core.c
> > @@ -698,7 +698,7 @@ uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t 
> > addr)
> >   }
> >
> >   void acpi_send_gpe_event(ACPIREGS *ar, qemu_irq irq,
> > -                         AcpiGPEStatusBits status)
> > +                         AcpiEventStatusBits status)
> >   {
> >       ar->gpe.sts[0] |= status;
> >       acpi_update_sci(ar, irq);
> > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > index 522c9a8..5b4fcb5 100644
> > --- a/hw/acpi/piix4.c
> > +++ b/hw/acpi/piix4.c
> > @@ -586,6 +586,13 @@ static void piix4_ospm_status(AcpiDeviceIf *adev, 
> > ACPIOSTInfoList ***list)
> >       acpi_memory_ospm_status(&s->acpi_memory_hotplug, list);
> >   }
> >
> > +static void piix4_send_gpe(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
> > +{
> > +    PIIX4PMState *s = PIIX4_PM(adev);
> > +
> > +    acpi_send_gpe_event(&s->ar, s->irq, ev);
> > +}
> > +
> >   static Property piix4_pm_properties[] = {
> >       DEFINE_PROP_UINT32("smb_io_base", PIIX4PMState, smb_io_base, 0),
> >       DEFINE_PROP_UINT8(ACPI_PM_PROP_S3_DISABLED, PIIX4PMState, disable_s3, 
> > 0),
> > @@ -624,6 +631,7 @@ static void piix4_pm_class_init(ObjectClass *klass, 
> > void *data)
> >       hc->unplug_request = piix4_device_unplug_request_cb;
> >       hc->unplug = piix4_device_unplug_cb;
> >       adevc->ospm_status = piix4_ospm_status;
> > +    adevc->send_event = piix4_send_gpe;
> >   }
> >
> >   static const TypeInfo piix4_pm_info = {
> > diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
> > index 4f8ca45..72d0781 100644
> > --- a/hw/isa/lpc_ich9.c
> > +++ b/hw/isa/lpc_ich9.c
> > @@ -703,6 +703,13 @@ static Property ich9_lpc_properties[] = {
> >       DEFINE_PROP_END_OF_LIST(),
> >   };
> >
> > +static void ich9_send_gpe(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
> > +{
> > +    ICH9LPCState *s = ICH9_LPC_DEVICE(adev);
> > +
> > +    acpi_send_gpe_event(&s->pm.acpi_regs, s->pm.irq, ev);
> > +}
> > +
> >   static void ich9_lpc_class_init(ObjectClass *klass, void *data)
> >   {
> >       DeviceClass *dc = DEVICE_CLASS(klass);
> > @@ -730,6 +737,7 @@ static void ich9_lpc_class_init(ObjectClass *klass, 
> > void *data)
> >       hc->unplug_request = ich9_device_unplug_request_cb;
> >       hc->unplug = ich9_device_unplug_cb;
> >       adevc->ospm_status = ich9_pm_ospm_status;
> > +    adevc->send_event = ich9_send_gpe;
> >   }
> >
> >   static const TypeInfo ich9_lpc_info = {
> > diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
> > index dc6ee00..c717f15 100644
> > --- a/include/hw/acpi/acpi.h
> > +++ b/include/hw/acpi/acpi.h
> > @@ -23,6 +23,7 @@
> >   #include "qemu/option.h"
> >   #include "exec/memory.h"
> >   #include "hw/irq.h"
> > +#include "hw/acpi/acpi_dev_interface.h"
> >
> >   /*
> >    * current device naming scheme supports up to 256 memory devices
> > @@ -89,13 +90,6 @@
> >   /* PM2_CNT */
> >   #define ACPI_BITMASK_ARB_DISABLE                0x0001
> >
> > -/* These values are part of guest ABI, and can not be changed */
> > -typedef enum {
> > -    ACPI_PCI_HOTPLUG_STATUS = 2,
> > -    ACPI_CPU_HOTPLUG_STATUS = 4,
> > -    ACPI_MEMORY_HOTPLUG_STATUS = 8,
> > -} AcpiGPEStatusBits;
> > -
> >   /* structs */
> >   typedef struct ACPIPMTimer ACPIPMTimer;
> >   typedef struct ACPIPM1EVT ACPIPM1EVT;
> > @@ -172,7 +166,7 @@ void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t 
> > addr, uint32_t val);
> >   uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr);
> >
> >   void acpi_send_gpe_event(ACPIREGS *ar, qemu_irq irq,
> > -                         AcpiGPEStatusBits status);
> > +                         AcpiEventStatusBits status);
> >
> >   void acpi_update_sci(ACPIREGS *acpi_regs, qemu_irq irq);
> >
> > diff --git a/include/hw/acpi/acpi_dev_interface.h 
> > b/include/hw/acpi/acpi_dev_interface.h
> > index f245f8d..a0c4a33 100644
> > --- a/include/hw/acpi/acpi_dev_interface.h
> > +++ b/include/hw/acpi/acpi_dev_interface.h
> > @@ -4,6 +4,13 @@
> >   #include "qom/object.h"
> >   #include "qapi-types.h"
> >
> > +/* These values are part of guest ABI, and can not be changed */
> > +typedef enum {
> > +    ACPI_PCI_HOTPLUG_STATUS = 2,
> > +    ACPI_CPU_HOTPLUG_STATUS = 4,
> > +    ACPI_MEMORY_HOTPLUG_STATUS = 8,
> > +} AcpiEventStatusBits;
> > +
> >   #define TYPE_ACPI_DEVICE_IF "acpi-device-interface"
> >
> >   #define ACPI_DEVICE_IF_CLASS(klass) \
> > @@ -22,11 +29,14 @@ typedef struct AcpiDeviceIf {
> >       Object Parent;
> >   } AcpiDeviceIf;
> >
> > +void acpi_send_event(DeviceState *dev, AcpiEventStatusBits event);
> > +
> >   /**
> >    * AcpiDeviceIfClass:
> >    *
> >    * ospm_status: returns status of ACPI device objects, reported
> >    *              via _OST method if device supports it.
> > + * send_event: inject a specified event into guest
> >    *
> >    * Interface is designed for providing unified interface
> >    * to generic ACPI functionality that could be used without
> > @@ -39,5 +49,6 @@ typedef struct AcpiDeviceIfClass {
> >
> >       /* <public> */
> >       void (*ospm_status)(AcpiDeviceIf *adev, ACPIOSTInfoList ***list);
> > +    void (*send_event)(AcpiDeviceIf *adev, AcpiEventStatusBits ev);
> >   } AcpiDeviceIfClass;
> >   #endif
> >  
>