diff mbox series

[v3,03/10] hw/arm/virt: Add virtual ACPI device

Message ID 20190321104745.28068-4-shameerali.kolothum.thodi@huawei.com (mailing list archive)
State New, archived
Headers show
Series ARM virt: ACPI memory hotplug support | expand

Commit Message

Shameerali Kolothum Thodi March 21, 2019, 10:47 a.m. UTC
From: Samuel Ortiz <sameo@linux.intel.com>

This adds the skeleton to support an acpi device interface
for HW-reduced acpi platforms via ACPI GED - Generic Event
Device (ACPI v6.1 5.6.9).

This will be used by Arm/Virt to add hotplug support.

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 hw/acpi/Kconfig                        |  4 ++
 hw/acpi/Makefile.objs                  |  1 +
 hw/acpi/generic_event_device.c         | 72 ++++++++++++++++++++++++++++++++++
 include/hw/acpi/generic_event_device.h | 29 ++++++++++++++
 4 files changed, 106 insertions(+)
 create mode 100644 hw/acpi/generic_event_device.c
 create mode 100644 include/hw/acpi/generic_event_device.h

Comments

Eric Auger March 28, 2019, 2:14 p.m. UTC | #1
Hi Shameer,

On 3/21/19 11:47 AM, Shameer Kolothum wrote:
> From: Samuel Ortiz <sameo@linux.intel.com>
> 
> This adds the skeleton to support an acpi device interface
> for HW-reduced acpi platforms via ACPI GED - Generic Event
> Device (ACPI v6.1 5.6.9).
> 
> This will be used by Arm/Virt to add hotplug support.
> 
> Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
>  hw/acpi/Kconfig                        |  4 ++
>  hw/acpi/Makefile.objs                  |  1 +
>  hw/acpi/generic_event_device.c         | 72 ++++++++++++++++++++++++++++++++++
>  include/hw/acpi/generic_event_device.h | 29 ++++++++++++++
>  4 files changed, 106 insertions(+)
>  create mode 100644 hw/acpi/generic_event_device.c
>  create mode 100644 include/hw/acpi/generic_event_device.h
> 
> diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig
> index eca3bee..01a8b41 100644
> --- a/hw/acpi/Kconfig
> +++ b/hw/acpi/Kconfig
> @@ -27,3 +27,7 @@ config ACPI_VMGENID
>      bool
>      default y
>      depends on PC
> +
> +config ACPI_HW_REDUCED
> +    bool
> +    depends on ACPI
> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> index 2d46e37..b753232 100644
> --- a/hw/acpi/Makefile.objs
> +++ b/hw/acpi/Makefile.objs
> @@ -6,6 +6,7 @@ common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
>  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
>  common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
>  common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
> +common-obj-$(CONFIG_ACPI_HW_REDUCED) += generic_event_device.o
>  common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
>  
>  common-obj-y += acpi_interface.o
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> new file mode 100644
> index 0000000..b21a551
> --- /dev/null
> +++ b/hw/acpi/generic_event_device.c
> @@ -0,0 +1,72 @@
> +/*
> + *
> + * Copyright (c) 2018 Intel Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/sysbus.h"
> +#include "hw/acpi/acpi.h"
> +#include "hw/acpi/generic_event_device.h"
the files are named generic_event_device.c/h while the device is named
"virt-acpi". I would suggest to use the same naming as in nemu ie. ged
or acpi_ged.

If think you should clarify what is the exact scope of this device. The
patch title make think this is bound to be used only in machvirt (+ the
virt prefix used in numerous functions?). Is it also bound to be used by
other architectures?
> +
> +static void virt_device_plug_cb(HotplugHandler *hotplug_dev,
> +                                DeviceState *dev, Error **errp)
> +{
> +}
> +
> +static void virt_send_ged(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
> +{
> +}
> +
> +static void virt_device_realize(DeviceState *dev, Error **errp)
> +{
> +}
> +
> +static Property virt_acpi_properties[] = {
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void virt_acpi_class_init(ObjectClass *class, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(class);
> +    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(class);
> +    AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(class);
> +
> +    dc->desc = "ACPI";
> +    dc->props = virt_acpi_properties;
> +    dc->realize = virt_device_realize;
> +
> +    hc->plug = virt_device_plug_cb;
> +
> +    adevc->send_event = virt_send_ged;
> +}
> +
> +static const TypeInfo virt_acpi_info = {
> +    .name          = TYPE_VIRT_ACPI,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(VirtAcpiState),
> +    .class_init    = virt_acpi_class_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_HOTPLUG_HANDLER },
> +        { TYPE_ACPI_DEVICE_IF },
> +        { }
> +    }
> +};
> +
> +static void virt_acpi_register_types(void)
> +{
> +    type_register_static(&virt_acpi_info);
> +}
> +
> +type_init(virt_acpi_register_types)
> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
> new file mode 100644
> index 0000000..f314515
> --- /dev/null
> +++ b/include/hw/acpi/generic_event_device.h
> @@ -0,0 +1,29 @@
> +/*
> + *
> + * Copyright (c) 2018 Intel Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
Add a comment in the header introducing what is the role of this device?
link to GED spec? Explain the subset of the interfaces being implemented
by the device.
> +
> +#ifndef HW_ACPI_GED_H
> +#define HW_ACPI_GED_H
> +
> +#define TYPE_VIRT_ACPI "virt-acpi"
> +#define VIRT_ACPI(obj) \
> +    OBJECT_CHECK(VirtAcpiState, (obj), TYPE_VIRT_ACPI)
> +
> +typedef struct VirtAcpiState {
> +    SysBusDevice parent_obj;
> +} VirtAcpiState;
> +
> +#endif
>
Shameerali Kolothum Thodi March 29, 2019, 11:22 a.m. UTC | #2
> -----Original Message-----
> From: Auger Eric [mailto:eric.auger@redhat.com]
> Sent: 28 March 2019 14:15
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> qemu-devel@nongnu.org; qemu-arm@nongnu.org; imammedo@redhat.com;
> peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> sameo@linux.intel.com; sebastien.boeuf@intel.com
> Cc: Linuxarm <linuxarm@huawei.com>; xuwei (O) <xuwei5@huawei.com>
> Subject: Re: [PATCH v3 03/10] hw/arm/virt: Add virtual ACPI device
> 
> Hi Shameer,
> 
> On 3/21/19 11:47 AM, Shameer Kolothum wrote:
> > From: Samuel Ortiz <sameo@linux.intel.com>
> >
> > This adds the skeleton to support an acpi device interface for
> > HW-reduced acpi platforms via ACPI GED - Generic Event Device (ACPI
> > v6.1 5.6.9).
> >
> > This will be used by Arm/Virt to add hotplug support.
> >
> > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
> > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> > ---
> >  hw/acpi/Kconfig                        |  4 ++
> >  hw/acpi/Makefile.objs                  |  1 +
> >  hw/acpi/generic_event_device.c         | 72
> ++++++++++++++++++++++++++++++++++
> >  include/hw/acpi/generic_event_device.h | 29 ++++++++++++++
> >  4 files changed, 106 insertions(+)
> >  create mode 100644 hw/acpi/generic_event_device.c  create mode
> 100644
> > include/hw/acpi/generic_event_device.h
> >
> > diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig index eca3bee..01a8b41
> > 100644
> > --- a/hw/acpi/Kconfig
> > +++ b/hw/acpi/Kconfig
> > @@ -27,3 +27,7 @@ config ACPI_VMGENID
> >      bool
> >      default y
> >      depends on PC
> > +
> > +config ACPI_HW_REDUCED
> > +    bool
> > +    depends on ACPI
> > diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs index
> > 2d46e37..b753232 100644
> > --- a/hw/acpi/Makefile.objs
> > +++ b/hw/acpi/Makefile.objs
> > @@ -6,6 +6,7 @@ common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) +=
> > memory_hotplug.o
> >  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
> >  common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
> >  common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
> > +common-obj-$(CONFIG_ACPI_HW_REDUCED) += generic_event_device.o
> >  common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
> >
> >  common-obj-y += acpi_interface.o
> > diff --git a/hw/acpi/generic_event_device.c
> > b/hw/acpi/generic_event_device.c new file mode 100644 index
> > 0000000..b21a551
> > --- /dev/null
> > +++ b/hw/acpi/generic_event_device.c
> > @@ -0,0 +1,72 @@
> > +/*
> > + *
> > + * Copyright (c) 2018 Intel Corporation
> > + *
> > + * This program is free software; you can redistribute it and/or
> > +modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but
> > +WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY
> > +or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> > +License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > +along with
> > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/sysbus.h"
> > +#include "hw/acpi/acpi.h"
> > +#include "hw/acpi/generic_event_device.h"
> the files are named generic_event_device.c/h while the device is named
> "virt-acpi". I would suggest to use the same naming as in nemu ie. ged or
> acpi_ged.

Agree. The naming is a bit confusing. In nemu they have a separate virt-acpi
dev which makes use of GED. Here, we are rolling those two into one. I am
still not very sure whether we should leave it as virt-acpi, because the actual
device on which this is implemented can be changed eg, GED vs GPIO. 

> If think you should clarify what is the exact scope of this device. The patch title
> make think this is bound to be used only in machvirt (+ the virt prefix used in
> numerous functions?). Is it also bound to be used by other architectures?
> > +
> > +static void virt_device_plug_cb(HotplugHandler *hotplug_dev,
> > +                                DeviceState *dev, Error **errp) { }
> > +
> > +static void virt_send_ged(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
> > +{ }
> > +
> > +static void virt_device_realize(DeviceState *dev, Error **errp) { }
> > +
> > +static Property virt_acpi_properties[] = {
> > +    DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > +static void virt_acpi_class_init(ObjectClass *class, void *data) {
> > +    DeviceClass *dc = DEVICE_CLASS(class);
> > +    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(class);
> > +    AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(class);
> > +
> > +    dc->desc = "ACPI";
> > +    dc->props = virt_acpi_properties;
> > +    dc->realize = virt_device_realize;
> > +
> > +    hc->plug = virt_device_plug_cb;
> > +
> > +    adevc->send_event = virt_send_ged; }
> > +
> > +static const TypeInfo virt_acpi_info = {
> > +    .name          = TYPE_VIRT_ACPI,
> > +    .parent        = TYPE_SYS_BUS_DEVICE,
> > +    .instance_size = sizeof(VirtAcpiState),
> > +    .class_init    = virt_acpi_class_init,
> > +    .interfaces = (InterfaceInfo[]) {
> > +        { TYPE_HOTPLUG_HANDLER },
> > +        { TYPE_ACPI_DEVICE_IF },
> > +        { }
> > +    }
> > +};
> > +
> > +static void virt_acpi_register_types(void) {
> > +    type_register_static(&virt_acpi_info);
> > +}
> > +
> > +type_init(virt_acpi_register_types)
> > diff --git a/include/hw/acpi/generic_event_device.h
> > b/include/hw/acpi/generic_event_device.h
> > new file mode 100644
> > index 0000000..f314515
> > --- /dev/null
> > +++ b/include/hw/acpi/generic_event_device.h
> > @@ -0,0 +1,29 @@
> > +/*
> > + *
> > + * Copyright (c) 2018 Intel Corporation
> > + *
> > + * This program is free software; you can redistribute it and/or
> > +modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but
> > +WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY
> > +or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> > +License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > +along with
> > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > + */
> Add a comment in the header introducing what is the role of this device?
> link to GED spec? Explain the subset of the interfaces being implemented by
> the device.

Ok. I have added comments to that effect in patch #10, but I think I will make it
clear here as well.

Cheers,
Shameer

> > +
> > +#ifndef HW_ACPI_GED_H
> > +#define HW_ACPI_GED_H
> > +
> > +#define TYPE_VIRT_ACPI "virt-acpi"
> > +#define VIRT_ACPI(obj) \
> > +    OBJECT_CHECK(VirtAcpiState, (obj), TYPE_VIRT_ACPI)
> > +
> > +typedef struct VirtAcpiState {
> > +    SysBusDevice parent_obj;
> > +} VirtAcpiState;
> > +
> > +#endif
> >
Igor Mammedov April 1, 2019, 1:08 p.m. UTC | #3
On Fri, 29 Mar 2019 11:22:02 +0000
Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote:

> > -----Original Message-----
> > From: Auger Eric [mailto:eric.auger@redhat.com]
> > Sent: 28 March 2019 14:15
> > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> > qemu-devel@nongnu.org; qemu-arm@nongnu.org; imammedo@redhat.com;
> > peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> > sameo@linux.intel.com; sebastien.boeuf@intel.com
> > Cc: Linuxarm <linuxarm@huawei.com>; xuwei (O) <xuwei5@huawei.com>
> > Subject: Re: [PATCH v3 03/10] hw/arm/virt: Add virtual ACPI device
> > 
> > Hi Shameer,
> > 
> > On 3/21/19 11:47 AM, Shameer Kolothum wrote:  
> > > From: Samuel Ortiz <sameo@linux.intel.com>
> > >
> > > This adds the skeleton to support an acpi device interface for
> > > HW-reduced acpi platforms via ACPI GED - Generic Event Device (ACPI
> > > v6.1 5.6.9).
> > >
> > > This will be used by Arm/Virt to add hotplug support.
> > >
> > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
> > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> > > ---
> > >  hw/acpi/Kconfig                        |  4 ++
> > >  hw/acpi/Makefile.objs                  |  1 +
> > >  hw/acpi/generic_event_device.c         | 72  
> > ++++++++++++++++++++++++++++++++++  
> > >  include/hw/acpi/generic_event_device.h | 29 ++++++++++++++
> > >  4 files changed, 106 insertions(+)
> > >  create mode 100644 hw/acpi/generic_event_device.c  create mode  
> > 100644  
> > > include/hw/acpi/generic_event_device.h
> > >
> > > diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig index eca3bee..01a8b41
> > > 100644
> > > --- a/hw/acpi/Kconfig
> > > +++ b/hw/acpi/Kconfig
> > > @@ -27,3 +27,7 @@ config ACPI_VMGENID
> > >      bool
> > >      default y
> > >      depends on PC
> > > +
> > > +config ACPI_HW_REDUCED
> > > +    bool
> > > +    depends on ACPI
> > > diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs index
> > > 2d46e37..b753232 100644
> > > --- a/hw/acpi/Makefile.objs
> > > +++ b/hw/acpi/Makefile.objs
> > > @@ -6,6 +6,7 @@ common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) +=
> > > memory_hotplug.o
> > >  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
> > >  common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
> > >  common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
> > > +common-obj-$(CONFIG_ACPI_HW_REDUCED) += generic_event_device.o
> > >  common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
> > >
> > >  common-obj-y += acpi_interface.o
> > > diff --git a/hw/acpi/generic_event_device.c
> > > b/hw/acpi/generic_event_device.c new file mode 100644 index
> > > 0000000..b21a551
> > > --- /dev/null
> > > +++ b/hw/acpi/generic_event_device.c
> > > @@ -0,0 +1,72 @@
> > > +/*
> > > + *
> > > + * Copyright (c) 2018 Intel Corporation
> > > + *
> > > + * This program is free software; you can redistribute it and/or
> > > +modify it
> > > + * under the terms and conditions of the GNU General Public License,
> > > + * version 2 or later, as published by the Free Software Foundation.
> > > + *
> > > + * This program is distributed in the hope it will be useful, but
> > > +WITHOUT
> > > + * ANY WARRANTY; without even the implied warranty of  
> > MERCHANTABILITY  
> > > +or
> > > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> > > +License for
> > > + * more details.
> > > + *
> > > + * You should have received a copy of the GNU General Public License
> > > +along with
> > > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > > + */
> > > +
> > > +#include "qemu/osdep.h"
> > > +#include "hw/sysbus.h"
> > > +#include "hw/acpi/acpi.h"
> > > +#include "hw/acpi/generic_event_device.h"  
> > the files are named generic_event_device.c/h while the device is named
> > "virt-acpi". I would suggest to use the same naming as in nemu ie. ged or
> > acpi_ged.  
> 
> Agree. The naming is a bit confusing. In nemu they have a separate virt-acpi
> dev which makes use of GED. Here, we are rolling those two into one. I am
> still not very sure whether we should leave it as virt-acpi, because the actual
> device on which this is implemented can be changed eg, GED vs GPIO. 

I probably lacking context here, could you clarify and maybe compare
differences between x86 and ARM implementations and why it should be different devices?


> > If think you should clarify what is the exact scope of this device. The patch title
> > make think this is bound to be used only in machvirt (+ the virt prefix used in
> > numerous functions?). Is it also bound to be used by other architectures?  
> > > +
> > > +static void virt_device_plug_cb(HotplugHandler *hotplug_dev,
> > > +                                DeviceState *dev, Error **errp) { }
> > > +
> > > +static void virt_send_ged(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
> > > +{ }
> > > +
> > > +static void virt_device_realize(DeviceState *dev, Error **errp) { }
> > > +
> > > +static Property virt_acpi_properties[] = {
> > > +    DEFINE_PROP_END_OF_LIST(),
> > > +};
> > > +
> > > +static void virt_acpi_class_init(ObjectClass *class, void *data) {
> > > +    DeviceClass *dc = DEVICE_CLASS(class);
> > > +    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(class);
> > > +    AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(class);
> > > +
> > > +    dc->desc = "ACPI";
> > > +    dc->props = virt_acpi_properties;
> > > +    dc->realize = virt_device_realize;
> > > +
> > > +    hc->plug = virt_device_plug_cb;
> > > +
> > > +    adevc->send_event = virt_send_ged; }
> > > +
> > > +static const TypeInfo virt_acpi_info = {
> > > +    .name          = TYPE_VIRT_ACPI,
> > > +    .parent        = TYPE_SYS_BUS_DEVICE,
> > > +    .instance_size = sizeof(VirtAcpiState),
> > > +    .class_init    = virt_acpi_class_init,
> > > +    .interfaces = (InterfaceInfo[]) {
> > > +        { TYPE_HOTPLUG_HANDLER },
> > > +        { TYPE_ACPI_DEVICE_IF },
> > > +        { }
> > > +    }
> > > +};
> > > +
> > > +static void virt_acpi_register_types(void) {
> > > +    type_register_static(&virt_acpi_info);
> > > +}
> > > +
> > > +type_init(virt_acpi_register_types)
> > > diff --git a/include/hw/acpi/generic_event_device.h
> > > b/include/hw/acpi/generic_event_device.h
> > > new file mode 100644
> > > index 0000000..f314515
> > > --- /dev/null
> > > +++ b/include/hw/acpi/generic_event_device.h
> > > @@ -0,0 +1,29 @@
> > > +/*
> > > + *
> > > + * Copyright (c) 2018 Intel Corporation
> > > + *
> > > + * This program is free software; you can redistribute it and/or
> > > +modify it
> > > + * under the terms and conditions of the GNU General Public License,
> > > + * version 2 or later, as published by the Free Software Foundation.
> > > + *
> > > + * This program is distributed in the hope it will be useful, but
> > > +WITHOUT
> > > + * ANY WARRANTY; without even the implied warranty of  
> > MERCHANTABILITY  
> > > +or
> > > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> > > +License for
> > > + * more details.
> > > + *
> > > + * You should have received a copy of the GNU General Public License
> > > +along with
> > > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > > + */  
> > Add a comment in the header introducing what is the role of this device?
> > link to GED spec? Explain the subset of the interfaces being implemented by
> > the device.  
> 
> Ok. I have added comments to that effect in patch #10, but I think I will make it
> clear here as well.
> 
> Cheers,
> Shameer
> 
> > > +
> > > +#ifndef HW_ACPI_GED_H
> > > +#define HW_ACPI_GED_H
> > > +
> > > +#define TYPE_VIRT_ACPI "virt-acpi"
> > > +#define VIRT_ACPI(obj) \
> > > +    OBJECT_CHECK(VirtAcpiState, (obj), TYPE_VIRT_ACPI)
> > > +
> > > +typedef struct VirtAcpiState {
> > > +    SysBusDevice parent_obj;
> > > +} VirtAcpiState;
> > > +
> > > +#endif
> > >
Shameerali Kolothum Thodi April 1, 2019, 2:21 p.m. UTC | #4
Hi Igor,

> -----Original Message-----
> From: Igor Mammedov [mailto:imammedo@redhat.com]
> Sent: 01 April 2019 14:09
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: Auger Eric <eric.auger@redhat.com>; qemu-devel@nongnu.org;
> qemu-arm@nongnu.org; peter.maydell@linaro.org;
> shannon.zhaosl@gmail.com; sameo@linux.intel.com;
> sebastien.boeuf@intel.com; Linuxarm <linuxarm@huawei.com>; xuwei (O)
> <xuwei5@huawei.com>
> Subject: Re: [Qemu-devel] [PATCH v3 03/10] hw/arm/virt: Add virtual ACPI
> device
> 
> On Fri, 29 Mar 2019 11:22:02 +0000
> Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote:
> 
> > > -----Original Message-----
> > > From: Auger Eric [mailto:eric.auger@redhat.com]
> > > Sent: 28 March 2019 14:15
> > > To: Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>;
> > > qemu-devel@nongnu.org; qemu-arm@nongnu.org;
> imammedo@redhat.com;
> > > peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> > > sameo@linux.intel.com; sebastien.boeuf@intel.com
> > > Cc: Linuxarm <linuxarm@huawei.com>; xuwei (O) <xuwei5@huawei.com>
> > > Subject: Re: [PATCH v3 03/10] hw/arm/virt: Add virtual ACPI device
> > >
> > > Hi Shameer,
> > >
> > > On 3/21/19 11:47 AM, Shameer Kolothum wrote:
> > > > From: Samuel Ortiz <sameo@linux.intel.com>
> > > >
> > > > This adds the skeleton to support an acpi device interface for
> > > > HW-reduced acpi platforms via ACPI GED - Generic Event Device (ACPI
> > > > v6.1 5.6.9).
> > > >
> > > > This will be used by Arm/Virt to add hotplug support.
> > > >
> > > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
> > > > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com>
> > > > ---
> > > >  hw/acpi/Kconfig                        |  4 ++
> > > >  hw/acpi/Makefile.objs                  |  1 +
> > > >  hw/acpi/generic_event_device.c         | 72
> > > ++++++++++++++++++++++++++++++++++
> > > >  include/hw/acpi/generic_event_device.h | 29 ++++++++++++++
> > > >  4 files changed, 106 insertions(+)
> > > >  create mode 100644 hw/acpi/generic_event_device.c  create mode
> > > 100644
> > > > include/hw/acpi/generic_event_device.h
> > > >
> > > > diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig index eca3bee..01a8b41
> > > > 100644
> > > > --- a/hw/acpi/Kconfig
> > > > +++ b/hw/acpi/Kconfig
> > > > @@ -27,3 +27,7 @@ config ACPI_VMGENID
> > > >      bool
> > > >      default y
> > > >      depends on PC
> > > > +
> > > > +config ACPI_HW_REDUCED
> > > > +    bool
> > > > +    depends on ACPI
> > > > diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs index
> > > > 2d46e37..b753232 100644
> > > > --- a/hw/acpi/Makefile.objs
> > > > +++ b/hw/acpi/Makefile.objs
> > > > @@ -6,6 +6,7 @@ common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG)
> +=
> > > > memory_hotplug.o
> > > >  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
> > > >  common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
> > > >  common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
> > > > +common-obj-$(CONFIG_ACPI_HW_REDUCED) +=
> generic_event_device.o
> > > >  common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
> > > >
> > > >  common-obj-y += acpi_interface.o
> > > > diff --git a/hw/acpi/generic_event_device.c
> > > > b/hw/acpi/generic_event_device.c new file mode 100644 index
> > > > 0000000..b21a551
> > > > --- /dev/null
> > > > +++ b/hw/acpi/generic_event_device.c
> > > > @@ -0,0 +1,72 @@
> > > > +/*
> > > > + *
> > > > + * Copyright (c) 2018 Intel Corporation
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or
> > > > +modify it
> > > > + * under the terms and conditions of the GNU General Public License,
> > > > + * version 2 or later, as published by the Free Software Foundation.
> > > > + *
> > > > + * This program is distributed in the hope it will be useful, but
> > > > +WITHOUT
> > > > + * ANY WARRANTY; without even the implied warranty of
> > > MERCHANTABILITY
> > > > +or
> > > > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> > > > +License for
> > > > + * more details.
> > > > + *
> > > > + * You should have received a copy of the GNU General Public License
> > > > +along with
> > > > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > > > + */
> > > > +
> > > > +#include "qemu/osdep.h"
> > > > +#include "hw/sysbus.h"
> > > > +#include "hw/acpi/acpi.h"
> > > > +#include "hw/acpi/generic_event_device.h"
> > > the files are named generic_event_device.c/h while the device is named
> > > "virt-acpi". I would suggest to use the same naming as in nemu ie. ged or
> > > acpi_ged.
> >
> > Agree. The naming is a bit confusing. In nemu they have a separate virt-acpi
> > dev which makes use of GED. Here, we are rolling those two into one. I am
> > still not very sure whether we should leave it as virt-acpi, because the actual
> > device on which this is implemented can be changed eg, GED vs GPIO.
> 
> I probably lacking context here, could you clarify and maybe compare
> differences between x86 and ARM implementations and why it should be
> different devices?
> 

Right. I was not comparing against x86, but just pointing out how Nemu has
done this. They seems to have a virt-acpi dev specific to virt platforms
(hw/i386/virt/acpi.c) and then moved all GED related code in a separate file
(hw/acpi/ged.c) [1].

I was just thinking whether that approach makes any sense going forward where
there are cases where platforms support GED or GPIO for hotplug support and
virt-acpi dev can be configured to use either of those. May be not.

Thanks,
Shameer

[1]https://github.com/intel/nemu/commit/bcff7ee8588f7049cd919ee8b349f219a873ec41#diff-82ce92e28467c5894c90311f0e6a75fb

> 
> > > If think you should clarify what is the exact scope of this device. The patch
> title
> > > make think this is bound to be used only in machvirt (+ the virt prefix used in
> > > numerous functions?). Is it also bound to be used by other architectures?
> > > > +
> > > > +static void virt_device_plug_cb(HotplugHandler *hotplug_dev,
> > > > +                                DeviceState *dev, Error **errp)
> { }
> > > > +
> > > > +static void virt_send_ged(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
> > > > +{ }
> > > > +
> > > > +static void virt_device_realize(DeviceState *dev, Error **errp) { }
> > > > +
> > > > +static Property virt_acpi_properties[] = {
> > > > +    DEFINE_PROP_END_OF_LIST(),
> > > > +};
> > > > +
> > > > +static void virt_acpi_class_init(ObjectClass *class, void *data) {
> > > > +    DeviceClass *dc = DEVICE_CLASS(class);
> > > > +    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(class);
> > > > +    AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(class);
> > > > +
> > > > +    dc->desc = "ACPI";
> > > > +    dc->props = virt_acpi_properties;
> > > > +    dc->realize = virt_device_realize;
> > > > +
> > > > +    hc->plug = virt_device_plug_cb;
> > > > +
> > > > +    adevc->send_event = virt_send_ged; }
> > > > +
> > > > +static const TypeInfo virt_acpi_info = {
> > > > +    .name          = TYPE_VIRT_ACPI,
> > > > +    .parent        = TYPE_SYS_BUS_DEVICE,
> > > > +    .instance_size = sizeof(VirtAcpiState),
> > > > +    .class_init    = virt_acpi_class_init,
> > > > +    .interfaces = (InterfaceInfo[]) {
> > > > +        { TYPE_HOTPLUG_HANDLER },
> > > > +        { TYPE_ACPI_DEVICE_IF },
> > > > +        { }
> > > > +    }
> > > > +};
> > > > +
> > > > +static void virt_acpi_register_types(void) {
> > > > +    type_register_static(&virt_acpi_info);
> > > > +}
> > > > +
> > > > +type_init(virt_acpi_register_types)
> > > > diff --git a/include/hw/acpi/generic_event_device.h
> > > > b/include/hw/acpi/generic_event_device.h
> > > > new file mode 100644
> > > > index 0000000..f314515
> > > > --- /dev/null
> > > > +++ b/include/hw/acpi/generic_event_device.h
> > > > @@ -0,0 +1,29 @@
> > > > +/*
> > > > + *
> > > > + * Copyright (c) 2018 Intel Corporation
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or
> > > > +modify it
> > > > + * under the terms and conditions of the GNU General Public License,
> > > > + * version 2 or later, as published by the Free Software Foundation.
> > > > + *
> > > > + * This program is distributed in the hope it will be useful, but
> > > > +WITHOUT
> > > > + * ANY WARRANTY; without even the implied warranty of
> > > MERCHANTABILITY
> > > > +or
> > > > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> > > > +License for
> > > > + * more details.
> > > > + *
> > > > + * You should have received a copy of the GNU General Public License
> > > > +along with
> > > > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > > > + */
> > > Add a comment in the header introducing what is the role of this device?
> > > link to GED spec? Explain the subset of the interfaces being implemented by
> > > the device.
> >
> > Ok. I have added comments to that effect in patch #10, but I think I will make
> it
> > clear here as well.
> >
> > Cheers,
> > Shameer
> >
> > > > +
> > > > +#ifndef HW_ACPI_GED_H
> > > > +#define HW_ACPI_GED_H
> > > > +
> > > > +#define TYPE_VIRT_ACPI "virt-acpi"
> > > > +#define VIRT_ACPI(obj) \
> > > > +    OBJECT_CHECK(VirtAcpiState, (obj), TYPE_VIRT_ACPI)
> > > > +
> > > > +typedef struct VirtAcpiState {
> > > > +    SysBusDevice parent_obj;
> > > > +} VirtAcpiState;
> > > > +
> > > > +#endif
> > > >
Igor Mammedov April 2, 2019, 6:31 a.m. UTC | #5
On Mon, 1 Apr 2019 14:21:40 +0000
Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote:

> Hi Igor,
> 
> > -----Original Message-----
> > From: Igor Mammedov [mailto:imammedo@redhat.com]
> > Sent: 01 April 2019 14:09
> > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> > Cc: Auger Eric <eric.auger@redhat.com>; qemu-devel@nongnu.org;
> > qemu-arm@nongnu.org; peter.maydell@linaro.org;
> > shannon.zhaosl@gmail.com; sameo@linux.intel.com;
> > sebastien.boeuf@intel.com; Linuxarm <linuxarm@huawei.com>; xuwei (O)
> > <xuwei5@huawei.com>
> > Subject: Re: [Qemu-devel] [PATCH v3 03/10] hw/arm/virt: Add virtual ACPI
> > device
> > 
> > On Fri, 29 Mar 2019 11:22:02 +0000
> > Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote:
> >   
> > > > -----Original Message-----
> > > > From: Auger Eric [mailto:eric.auger@redhat.com]
> > > > Sent: 28 March 2019 14:15
> > > > To: Shameerali Kolothum Thodi  
> > <shameerali.kolothum.thodi@huawei.com>;  
> > > > qemu-devel@nongnu.org; qemu-arm@nongnu.org;  
> > imammedo@redhat.com;  
> > > > peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> > > > sameo@linux.intel.com; sebastien.boeuf@intel.com
> > > > Cc: Linuxarm <linuxarm@huawei.com>; xuwei (O) <xuwei5@huawei.com>
> > > > Subject: Re: [PATCH v3 03/10] hw/arm/virt: Add virtual ACPI device
> > > >
> > > > Hi Shameer,
> > > >
> > > > On 3/21/19 11:47 AM, Shameer Kolothum wrote:  
> > > > > From: Samuel Ortiz <sameo@linux.intel.com>
> > > > >
> > > > > This adds the skeleton to support an acpi device interface for
> > > > > HW-reduced acpi platforms via ACPI GED - Generic Event Device (ACPI
> > > > > v6.1 5.6.9).
> > > > >
> > > > > This will be used by Arm/Virt to add hotplug support.
> > > > >
> > > > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
> > > > > Signed-off-by: Shameer Kolothum  
> > <shameerali.kolothum.thodi@huawei.com>  
> > > > > ---
> > > > >  hw/acpi/Kconfig                        |  4 ++
> > > > >  hw/acpi/Makefile.objs                  |  1 +
> > > > >  hw/acpi/generic_event_device.c         | 72  
> > > > ++++++++++++++++++++++++++++++++++  
> > > > >  include/hw/acpi/generic_event_device.h | 29 ++++++++++++++
> > > > >  4 files changed, 106 insertions(+)
> > > > >  create mode 100644 hw/acpi/generic_event_device.c  create mode  
> > > > 100644  
> > > > > include/hw/acpi/generic_event_device.h
> > > > >
> > > > > diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig index eca3bee..01a8b41
> > > > > 100644
> > > > > --- a/hw/acpi/Kconfig
> > > > > +++ b/hw/acpi/Kconfig
> > > > > @@ -27,3 +27,7 @@ config ACPI_VMGENID
> > > > >      bool
> > > > >      default y
> > > > >      depends on PC
> > > > > +
> > > > > +config ACPI_HW_REDUCED
> > > > > +    bool
> > > > > +    depends on ACPI
> > > > > diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs index
> > > > > 2d46e37..b753232 100644
> > > > > --- a/hw/acpi/Makefile.objs
> > > > > +++ b/hw/acpi/Makefile.objs
> > > > > @@ -6,6 +6,7 @@ common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG)  
> > +=  
> > > > > memory_hotplug.o
> > > > >  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
> > > > >  common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
> > > > >  common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
> > > > > +common-obj-$(CONFIG_ACPI_HW_REDUCED) +=  
> > generic_event_device.o  
> > > > >  common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
> > > > >
> > > > >  common-obj-y += acpi_interface.o
> > > > > diff --git a/hw/acpi/generic_event_device.c
> > > > > b/hw/acpi/generic_event_device.c new file mode 100644 index
> > > > > 0000000..b21a551
> > > > > --- /dev/null
> > > > > +++ b/hw/acpi/generic_event_device.c
> > > > > @@ -0,0 +1,72 @@
> > > > > +/*
> > > > > + *
> > > > > + * Copyright (c) 2018 Intel Corporation
> > > > > + *
> > > > > + * This program is free software; you can redistribute it and/or
> > > > > +modify it
> > > > > + * under the terms and conditions of the GNU General Public License,
> > > > > + * version 2 or later, as published by the Free Software Foundation.
> > > > > + *
> > > > > + * This program is distributed in the hope it will be useful, but
> > > > > +WITHOUT
> > > > > + * ANY WARRANTY; without even the implied warranty of  
> > > > MERCHANTABILITY  
> > > > > +or
> > > > > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> > > > > +License for
> > > > > + * more details.
> > > > > + *
> > > > > + * You should have received a copy of the GNU General Public License
> > > > > +along with
> > > > > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > > > > + */
> > > > > +
> > > > > +#include "qemu/osdep.h"
> > > > > +#include "hw/sysbus.h"
> > > > > +#include "hw/acpi/acpi.h"
> > > > > +#include "hw/acpi/generic_event_device.h"  
> > > > the files are named generic_event_device.c/h while the device is named
> > > > "virt-acpi". I would suggest to use the same naming as in nemu ie. ged or
> > > > acpi_ged.  
> > >
> > > Agree. The naming is a bit confusing. In nemu they have a separate virt-acpi
> > > dev which makes use of GED. Here, we are rolling those two into one. I am
> > > still not very sure whether we should leave it as virt-acpi, because the actual
> > > device on which this is implemented can be changed eg, GED vs GPIO.  
> > 
> > I probably lacking context here, could you clarify and maybe compare
> > differences between x86 and ARM implementations and why it should be
> > different devices?
> >   
> 
> Right. I was not comparing against x86, but just pointing out how Nemu has
> done this. They seems to have a virt-acpi dev specific to virt platforms
> (hw/i386/virt/acpi.c) and then moved all GED related code in a separate file
> (hw/acpi/ged.c) [1].
> 
> I was just thinking whether that approach makes any sense going forward where
> there are cases where platforms support GED or GPIO for hotplug support and
> virt-acpi dev can be configured to use either of those. May be not.

from what I see that nemu uses GED only as ACPI aml code, while TYPE_VIRT_ACPI
actually implements hardware part of GED (i.e. initializes and owns MMIO/IRQs).
So it is GED device in practice.

If it's possible by ACPI spec to use GPIO with GED device, then I'd add it
later when there is actual usecase for it. Otherwise GPIO is just another
device with its own AML part to go with.

So I'd second Eric's suggestion to rename virt-acpi to acpi-ged

> Thanks,
> Shameer
> 
> [1]https://github.com/intel/nemu/commit/bcff7ee8588f7049cd919ee8b349f219a873ec41#diff-82ce92e28467c5894c90311f0e6a75fb
> 
> >   
> > > > If think you should clarify what is the exact scope of this device. The patch  
> > title  
> > > > make think this is bound to be used only in machvirt (+ the virt prefix used in
> > > > numerous functions?). Is it also bound to be used by other architectures?  
> > > > > +
> > > > > +static void virt_device_plug_cb(HotplugHandler *hotplug_dev,
> > > > > +                                DeviceState *dev, Error **errp)  
> > { }  
> > > > > +
> > > > > +static void virt_send_ged(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
> > > > > +{ }
> > > > > +
> > > > > +static void virt_device_realize(DeviceState *dev, Error **errp) { }
> > > > > +
> > > > > +static Property virt_acpi_properties[] = {
> > > > > +    DEFINE_PROP_END_OF_LIST(),
> > > > > +};
> > > > > +
> > > > > +static void virt_acpi_class_init(ObjectClass *class, void *data) {
> > > > > +    DeviceClass *dc = DEVICE_CLASS(class);
> > > > > +    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(class);
> > > > > +    AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(class);
> > > > > +
> > > > > +    dc->desc = "ACPI";
> > > > > +    dc->props = virt_acpi_properties;
> > > > > +    dc->realize = virt_device_realize;
> > > > > +
> > > > > +    hc->plug = virt_device_plug_cb;
> > > > > +
> > > > > +    adevc->send_event = virt_send_ged; }
> > > > > +
> > > > > +static const TypeInfo virt_acpi_info = {
> > > > > +    .name          = TYPE_VIRT_ACPI,
> > > > > +    .parent        = TYPE_SYS_BUS_DEVICE,
> > > > > +    .instance_size = sizeof(VirtAcpiState),
> > > > > +    .class_init    = virt_acpi_class_init,
> > > > > +    .interfaces = (InterfaceInfo[]) {
> > > > > +        { TYPE_HOTPLUG_HANDLER },
> > > > > +        { TYPE_ACPI_DEVICE_IF },
> > > > > +        { }
> > > > > +    }
> > > > > +};
> > > > > +
> > > > > +static void virt_acpi_register_types(void) {
> > > > > +    type_register_static(&virt_acpi_info);
> > > > > +}
> > > > > +
> > > > > +type_init(virt_acpi_register_types)
> > > > > diff --git a/include/hw/acpi/generic_event_device.h
> > > > > b/include/hw/acpi/generic_event_device.h
> > > > > new file mode 100644
> > > > > index 0000000..f314515
> > > > > --- /dev/null
> > > > > +++ b/include/hw/acpi/generic_event_device.h
> > > > > @@ -0,0 +1,29 @@
> > > > > +/*
> > > > > + *
> > > > > + * Copyright (c) 2018 Intel Corporation
> > > > > + *
> > > > > + * This program is free software; you can redistribute it and/or
> > > > > +modify it
> > > > > + * under the terms and conditions of the GNU General Public License,
> > > > > + * version 2 or later, as published by the Free Software Foundation.
> > > > > + *
> > > > > + * This program is distributed in the hope it will be useful, but
> > > > > +WITHOUT
> > > > > + * ANY WARRANTY; without even the implied warranty of  
> > > > MERCHANTABILITY  
> > > > > +or
> > > > > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> > > > > +License for
> > > > > + * more details.
> > > > > + *
> > > > > + * You should have received a copy of the GNU General Public License
> > > > > +along with
> > > > > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > > > > + */  
> > > > Add a comment in the header introducing what is the role of this device?
> > > > link to GED spec? Explain the subset of the interfaces being implemented by
> > > > the device.  
> > >
> > > Ok. I have added comments to that effect in patch #10, but I think I will make  
> > it  
> > > clear here as well.
> > >
> > > Cheers,
> > > Shameer
> > >  
> > > > > +
> > > > > +#ifndef HW_ACPI_GED_H
> > > > > +#define HW_ACPI_GED_H
> > > > > +
> > > > > +#define TYPE_VIRT_ACPI "virt-acpi"
> > > > > +#define VIRT_ACPI(obj) \
> > > > > +    OBJECT_CHECK(VirtAcpiState, (obj), TYPE_VIRT_ACPI)
> > > > > +
> > > > > +typedef struct VirtAcpiState {
> > > > > +    SysBusDevice parent_obj;
> > > > > +} VirtAcpiState;
> > > > > +
> > > > > +#endif
> > > > >  
> 
>
diff mbox series

Patch

diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig
index eca3bee..01a8b41 100644
--- a/hw/acpi/Kconfig
+++ b/hw/acpi/Kconfig
@@ -27,3 +27,7 @@  config ACPI_VMGENID
     bool
     default y
     depends on PC
+
+config ACPI_HW_REDUCED
+    bool
+    depends on ACPI
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index 2d46e37..b753232 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -6,6 +6,7 @@  common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
 common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
 common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
 common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
+common-obj-$(CONFIG_ACPI_HW_REDUCED) += generic_event_device.o
 common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
 
 common-obj-y += acpi_interface.o
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
new file mode 100644
index 0000000..b21a551
--- /dev/null
+++ b/hw/acpi/generic_event_device.c
@@ -0,0 +1,72 @@ 
+/*
+ *
+ * Copyright (c) 2018 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/generic_event_device.h"
+
+static void virt_device_plug_cb(HotplugHandler *hotplug_dev,
+                                DeviceState *dev, Error **errp)
+{
+}
+
+static void virt_send_ged(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
+{
+}
+
+static void virt_device_realize(DeviceState *dev, Error **errp)
+{
+}
+
+static Property virt_acpi_properties[] = {
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virt_acpi_class_init(ObjectClass *class, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(class);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(class);
+    AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(class);
+
+    dc->desc = "ACPI";
+    dc->props = virt_acpi_properties;
+    dc->realize = virt_device_realize;
+
+    hc->plug = virt_device_plug_cb;
+
+    adevc->send_event = virt_send_ged;
+}
+
+static const TypeInfo virt_acpi_info = {
+    .name          = TYPE_VIRT_ACPI,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(VirtAcpiState),
+    .class_init    = virt_acpi_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { TYPE_ACPI_DEVICE_IF },
+        { }
+    }
+};
+
+static void virt_acpi_register_types(void)
+{
+    type_register_static(&virt_acpi_info);
+}
+
+type_init(virt_acpi_register_types)
diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
new file mode 100644
index 0000000..f314515
--- /dev/null
+++ b/include/hw/acpi/generic_event_device.h
@@ -0,0 +1,29 @@ 
+/*
+ *
+ * Copyright (c) 2018 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef HW_ACPI_GED_H
+#define HW_ACPI_GED_H
+
+#define TYPE_VIRT_ACPI "virt-acpi"
+#define VIRT_ACPI(obj) \
+    OBJECT_CHECK(VirtAcpiState, (obj), TYPE_VIRT_ACPI)
+
+typedef struct VirtAcpiState {
+    SysBusDevice parent_obj;
+} VirtAcpiState;
+
+#endif