[v2,04/11] hw/arm/virt: Add virtual ACPI device

Commit Message

Shameerali Kolothum Thodi March 8, 2019, 11:42 a.m. UTC

From: Samuel Ortiz <[e-mail]>

This is to provide an acpi device interface for Arm/virt.
This will be used by Arm/Virt to add hotplug support via
ACPI GED device.

Signed-off-by: Samuel Ortiz <[e-mail]>
Signed-off-by: Shameer Kolothum <[e-mail]>
---
 hw/arm/Makefile.objs  |   2 +-
 hw/arm/virt-acpi.c    | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/virt.h |   1 +
 3 files changed, 113 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/virt-acpi.c

Comments

Eric Auger March 11, 2019, 1:24 p.m. UTC | #1

Hi Shameer,

On 3/8/19 12:42 PM, Shameer Kolothum wrote:
> From: Samuel Ortiz <[e-mail]>
> 
> This is to provide an acpi device interface for Arm/virt.
> This will be used by Arm/Virt to add hotplug support via
> ACPI GED device.

I think this would deserves to mention this is a skeleton or,
wouldn't it make sense to merge the virt-acpi part of "hw/arm/virt: Add
ACPI support for device memory cold-plug" inside this patch; and keep
its instantiation in machvirt in a separate patch?

> 
> Signed-off-by: Samuel Ortiz <[e-mail]>
> Signed-off-by: Shameer Kolothum <[e-mail]>
> ---
>  hw/arm/Makefile.objs  |   2 +-
>  hw/arm/virt-acpi.c    | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/arm/virt.h |   1 +
>  3 files changed, 113 insertions(+), 1 deletion(-)
>  create mode 100644 hw/arm/virt-acpi.c
> 
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index fa57c7c..e0db3cd 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -1,6 +1,6 @@
>  obj-y += boot.o sysbus-fdt.o
>  obj-$(CONFIG_ARM_VIRT) += virt.o
> -obj-$(CONFIG_ACPI) += virt-acpi-build.o
> +obj-$(CONFIG_ACPI) += virt-acpi-build.o virt-acpi.o
>  obj-$(CONFIG_DIGIC) += digic_boards.o
>  obj-$(CONFIG_EXYNOS4) += exynos4_boards.o
>  obj-$(CONFIG_HIGHBANK) += highbank.o
> diff --git a/hw/arm/virt-acpi.c b/hw/arm/virt-acpi.c
> new file mode 100644
> index 0000000..df8a02b
> --- /dev/null
> +++ b/hw/arm/virt-acpi.c
> @@ -0,0 +1,111 @@
> +/*
> + *
> + * 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 "qemu/range.h"
> +#include "qapi/error.h"
> +#include "exec/address-spaces.h"
> +
> +#include "hw/hw.h"
> +#include "hw/hotplug.h"
> +#include "hw/sysbus.h"
> +#include "hw/arm/virt.h"
> +
> +#include "hw/acpi/acpi.h"
> +
> +typedef struct VirtAcpiState {
> +    SysBusDevice parent_obj;
> +} VirtAcpiState;
> +
> +#define TYPE_VIRT_ACPI "virt-acpi"
> +#define VIRT_ACPI(obj) \
> +    OBJECT_CHECK(VirtAcpiState, (obj), TYPE_VIRT_ACPI)
> +
> +static const VMStateDescription vmstate_acpi = {
> +    .name = "virt_acpi",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +};
> +
> +static void virt_device_plug_cb(HotplugHandler *hotplug_dev,
> +                                DeviceState *dev, Error **errp)
> +{
> +}
> +
> +static void virt_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> +                                          DeviceState *dev, Error **errp)
> +{
> +}
> +
> +static void virt_device_unplug_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)
> +{
> +}
> +
> +DeviceState *virt_acpi_init(void)
> +{
> +    return sysbus_create_simple(TYPE_VIRT_ACPI, -1, NULL);
> +}
> +
> +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->vmsd = &vmstate_acpi;
do we really need the vmsd field as of now?
> +    dc->props = virt_acpi_properties;> +    dc->realize = virt_device_realize;
> +
> +    hc->plug = virt_device_plug_cb;
> +    hc->unplug_request = virt_device_unplug_request_cb;
> +    hc->unplug = virt_device_unplug_cb;
unplug_request and unplug still aren't implemented at the end of the
series. Maybe we can ignore them at the moment?
> +
> +    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/arm/virt.h b/include/hw/arm/virt.h
> index 507517c..6076167 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -145,6 +145,7 @@ typedef struct {
>      OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE)
>  
>  void virt_acpi_setup(VirtMachineState *vms);
> +DeviceState *virt_acpi_init(void);
>  
>  /* Return the number of used redistributor regions  */
>  static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)
> 

Thanks

Eric

Shameerali Kolothum Thodi March 11, 2019, 5:36 p.m. UTC | #2

Hi Eric,

> -----Original Message-----
> From: Auger Eric [mailto:[e-mail]]
> Sent: 11 March 2019 13:24
> To: Shameerali Kolothum Thodi <[e-mail]>;
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]
> Cc: Linuxarm <[e-mail]>; xuwei (O) <[e-mail]>
> Subject: Re: [PATCH v2 04/11] hw/arm/virt: Add virtual ACPI device
> 
> Hi Shameer,
> 
> On 3/8/19 12:42 PM, Shameer Kolothum wrote:
> > From: Samuel Ortiz <[e-mail]>
> >
> > This is to provide an acpi device interface for Arm/virt.
> > This will be used by Arm/Virt to add hotplug support via
> > ACPI GED device.
> 
> I think this would deserves to mention this is a skeleton or,
> wouldn't it make sense to merge the virt-acpi part of "hw/arm/virt: Add
> ACPI support for device memory cold-plug" inside this patch; and keep
> its instantiation in machvirt in a separate patch?

Ok. I will look into it.

> >
> > Signed-off-by: Samuel Ortiz <[e-mail]>
> > Signed-off-by: Shameer Kolothum <[e-mail]>
> > ---
> >  hw/arm/Makefile.objs  |   2 +-
> >  hw/arm/virt-acpi.c    | 111
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/hw/arm/virt.h |   1 +
> >  3 files changed, 113 insertions(+), 1 deletion(-)
> >  create mode 100644 hw/arm/virt-acpi.c
> >
> > diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> > index fa57c7c..e0db3cd 100644
> > --- a/hw/arm/Makefile.objs
> > +++ b/hw/arm/Makefile.objs
> > @@ -1,6 +1,6 @@
> >  obj-y += boot.o sysbus-fdt.o
> >  obj-$(CONFIG_ARM_VIRT) += virt.o
> > -obj-$(CONFIG_ACPI) += virt-acpi-build.o
> > +obj-$(CONFIG_ACPI) += virt-acpi-build.o virt-acpi.o
> >  obj-$(CONFIG_DIGIC) += digic_boards.o
> >  obj-$(CONFIG_EXYNOS4) += exynos4_boards.o
> >  obj-$(CONFIG_HIGHBANK) += highbank.o
> > diff --git a/hw/arm/virt-acpi.c b/hw/arm/virt-acpi.c
> > new file mode 100644
> > index 0000000..df8a02b
> > --- /dev/null
> > +++ b/hw/arm/virt-acpi.c
> > @@ -0,0 +1,111 @@
> > +/*
> > + *
> > + * 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 "qemu/range.h"
> > +#include "qapi/error.h"
> > +#include "exec/address-spaces.h"
> > +
> > +#include "hw/hw.h"
> > +#include "hw/hotplug.h"
> > +#include "hw/sysbus.h"
> > +#include "hw/arm/virt.h"
> > +
> > +#include "hw/acpi/acpi.h"
> > +
> > +typedef struct VirtAcpiState {
> > +    SysBusDevice parent_obj;
> > +} VirtAcpiState;
> > +
> > +#define TYPE_VIRT_ACPI "virt-acpi"
> > +#define VIRT_ACPI(obj) \
> > +    OBJECT_CHECK(VirtAcpiState, (obj), TYPE_VIRT_ACPI)
> > +
> > +static const VMStateDescription vmstate_acpi = {
> > +    .name = "virt_acpi",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +};
> > +
> > +static void virt_device_plug_cb(HotplugHandler *hotplug_dev,
> > +                                DeviceState *dev, Error **errp)
> > +{
> > +}
> > +
> > +static void virt_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> > +                                          DeviceState *dev, Error
> **errp)
> > +{
> > +}
> > +
> > +static void virt_device_unplug_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)
> > +{
> > +}
> > +
> > +DeviceState *virt_acpi_init(void)
> > +{
> > +    return sysbus_create_simple(TYPE_VIRT_ACPI, -1, NULL);
> > +}
> > +
> > +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->vmsd = &vmstate_acpi;
> do we really need the vmsd field as of now?

I am not sure. Is this only relevant for migration cases only?

> > +    dc->props = virt_acpi_properties;> +    dc->realize =
> virt_device_realize;
> > +
> > +    hc->plug = virt_device_plug_cb;
> > +    hc->unplug_request = virt_device_unplug_request_cb;
> > +    hc->unplug = virt_device_unplug_cb;
> unplug_request and unplug still aren't implemented at the end of the
> series. Maybe we can ignore them at the moment?

Agreed. I will remove those.

Thanks,
Shameer

> > +
> > +    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/arm/virt.h b/include/hw/arm/virt.h
> > index 507517c..6076167 100644
> > --- a/include/hw/arm/virt.h
> > +++ b/include/hw/arm/virt.h
> > @@ -145,6 +145,7 @@ typedef struct {
> >      OBJECT_CLASS_CHECK(VirtMachineClass, klass,
> TYPE_VIRT_MACHINE)
> >
> >  void virt_acpi_setup(VirtMachineState *vms);
> > +DeviceState *virt_acpi_init(void);
> >
> >  /* Return the number of used redistributor regions  */
> >  static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)
> >
> 
> Thanks
> 
> Eric

Igor Mammedov March 12, 2019, 3:48 p.m. UTC | #3

On Fri, 8 Mar 2019 11:42:11 +0000
Shameer Kolothum <[e-mail]> wrote:

> From: Samuel Ortiz <[e-mail]>
> 
> This is to provide an acpi device interface for Arm/virt.
> This will be used by Arm/Virt to add hotplug support via
> ACPI GED device.
> 
> Signed-off-by: Samuel Ortiz <[e-mail]>
> Signed-off-by: Shameer Kolothum <[e-mail]>
> ---
>  hw/arm/Makefile.objs  |   2 +-
>  hw/arm/virt-acpi.c    | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++

does it have to be ARM specific?
could it be shared with future x86/virt? (if it could then it would be better to put into hw/acpi/)
also considering it implements vitual 'GED' device
why not s/virt-acpi/generic-event-device/ ?


>  include/hw/arm/virt.h |   1 +
>  3 files changed, 113 insertions(+), 1 deletion(-)
>  create mode 100644 hw/arm/virt-acpi.c
> 
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index fa57c7c..e0db3cd 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -1,6 +1,6 @@
>  obj-y += boot.o sysbus-fdt.o
>  obj-$(CONFIG_ARM_VIRT) += virt.o
> -obj-$(CONFIG_ACPI) += virt-acpi-build.o
> +obj-$(CONFIG_ACPI) += virt-acpi-build.o virt-acpi.o
>  obj-$(CONFIG_DIGIC) += digic_boards.o
>  obj-$(CONFIG_EXYNOS4) += exynos4_boards.o
>  obj-$(CONFIG_HIGHBANK) += highbank.o
> diff --git a/hw/arm/virt-acpi.c b/hw/arm/virt-acpi.c
> new file mode 100644
> index 0000000..df8a02b
> --- /dev/null
> +++ b/hw/arm/virt-acpi.c
> @@ -0,0 +1,111 @@
> +/*
> + *
> + * 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 "qemu/range.h"
> +#include "qapi/error.h"
> +#include "exec/address-spaces.h"
> +
> +#include "hw/hw.h"
> +#include "hw/hotplug.h"
> +#include "hw/sysbus.h"
> +#include "hw/arm/virt.h"
> +
> +#include "hw/acpi/acpi.h"
> +
> +typedef struct VirtAcpiState {
> +    SysBusDevice parent_obj;
> +} VirtAcpiState;
> +
> +#define TYPE_VIRT_ACPI "virt-acpi"
> +#define VIRT_ACPI(obj) \
> +    OBJECT_CHECK(VirtAcpiState, (obj), TYPE_VIRT_ACPI)
> +
> +static const VMStateDescription vmstate_acpi = {
> +    .name = "virt_acpi",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +};
> +
> +static void virt_device_plug_cb(HotplugHandler *hotplug_dev,
> +                                DeviceState *dev, Error **errp)
> +{
> +}
> +
> +static void virt_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> +                                          DeviceState *dev, Error **errp)
> +{
> +}
> +
> +static void virt_device_unplug_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)
> +{
> +}
> +
> +DeviceState *virt_acpi_init(void)
> +{
> +    return sysbus_create_simple(TYPE_VIRT_ACPI, -1, NULL);
> +}
> +
> +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->vmsd = &vmstate_acpi;
> +    dc->props = virt_acpi_properties;
> +    dc->realize = virt_device_realize;
> +
> +    hc->plug = virt_device_plug_cb;
> +    hc->unplug_request = virt_device_unplug_request_cb;
> +    hc->unplug = virt_device_unplug_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/arm/virt.h b/include/hw/arm/virt.h
> index 507517c..6076167 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -145,6 +145,7 @@ typedef struct {
>      OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE)
>  
>  void virt_acpi_setup(VirtMachineState *vms);
> +DeviceState *virt_acpi_init(void);
>  
>  /* Return the number of used redistributor regions  */
>  static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)

Shameerali Kolothum Thodi March 14, 2019, 4:55 p.m. UTC | #4

Hi Igor,

> -----Original Message-----
> From: Igor Mammedov [mailto:[e-mail]]
> Sent: 12 March 2019 15:48
> To: Shameerali Kolothum Thodi <[e-mail]>
> Cc: [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; Linuxarm <[e-mail]>; xuwei (O)
> <[e-mail]>
> Subject: Re: [PATCH v2 04/11] hw/arm/virt: Add virtual ACPI device
> 
> On Fri, 8 Mar 2019 11:42:11 +0000
> Shameer Kolothum <[e-mail]> wrote:
> 
> > From: Samuel Ortiz <[e-mail]>
> >
> > This is to provide an acpi device interface for Arm/virt.
> > This will be used by Arm/Virt to add hotplug support via
> > ACPI GED device.
> >
> > Signed-off-by: Samuel Ortiz <[e-mail]>
> > Signed-off-by: Shameer Kolothum <[e-mail]>
> > ---
> >  hw/arm/Makefile.objs  |   2 +-
> >  hw/arm/virt-acpi.c    | 111
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> 
> does it have to be ARM specific?
> could it be shared with future x86/virt? (if it could then it would be better to put
> into hw/acpi/)
> also considering it implements vitual 'GED' device
> why not s/virt-acpi/generic-event-device/ ?

I think that's a good idea to make it generic and I had a quick go at it.

But the problem I am facing is as soon as sysbus_create_simple() is called from
virt_acpi_init(), the realize() function gets called before it sets the VirtAcpiState
members.

This creates a problem as acpi_memory_hotplug_init() and acpi_ged_init() are
invoked from the realize() and it needs to have the base address, irq etc set based
on the platform.

I tried using the qdev_prop_set_*() as well, but it also fails saying attempt to set
property after it was realized.

Not sure what is the best way to solve this other than introducing a global struct to
hold the configuration.

Please let me know.

Thanks,
Shameer
 
> 
> >  include/hw/arm/virt.h |   1 +
> >  3 files changed, 113 insertions(+), 1 deletion(-)
> >  create mode 100644 hw/arm/virt-acpi.c
> >
> > diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> > index fa57c7c..e0db3cd 100644
> > --- a/hw/arm/Makefile.objs
> > +++ b/hw/arm/Makefile.objs
> > @@ -1,6 +1,6 @@
> >  obj-y += boot.o sysbus-fdt.o
> >  obj-$(CONFIG_ARM_VIRT) += virt.o
> > -obj-$(CONFIG_ACPI) += virt-acpi-build.o
> > +obj-$(CONFIG_ACPI) += virt-acpi-build.o virt-acpi.o
> >  obj-$(CONFIG_DIGIC) += digic_boards.o
> >  obj-$(CONFIG_EXYNOS4) += exynos4_boards.o
> >  obj-$(CONFIG_HIGHBANK) += highbank.o
> > diff --git a/hw/arm/virt-acpi.c b/hw/arm/virt-acpi.c
> > new file mode 100644
> > index 0000000..df8a02b
> > --- /dev/null
> > +++ b/hw/arm/virt-acpi.c
> > @@ -0,0 +1,111 @@
> > +/*
> > + *
> > + * 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 "qemu/range.h"
> > +#include "qapi/error.h"
> > +#include "exec/address-spaces.h"
> > +
> > +#include "hw/hw.h"
> > +#include "hw/hotplug.h"
> > +#include "hw/sysbus.h"
> > +#include "hw/arm/virt.h"
> > +
> > +#include "hw/acpi/acpi.h"
> > +
> > +typedef struct VirtAcpiState {
> > +    SysBusDevice parent_obj;
> > +} VirtAcpiState;
> > +
> > +#define TYPE_VIRT_ACPI "virt-acpi"
> > +#define VIRT_ACPI(obj) \
> > +    OBJECT_CHECK(VirtAcpiState, (obj), TYPE_VIRT_ACPI)
> > +
> > +static const VMStateDescription vmstate_acpi = {
> > +    .name = "virt_acpi",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +};
> > +
> > +static void virt_device_plug_cb(HotplugHandler *hotplug_dev,
> > +                                DeviceState *dev, Error **errp)
> > +{
> > +}
> > +
> > +static void virt_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> > +                                          DeviceState *dev, Error
> **errp)
> > +{
> > +}
> > +
> > +static void virt_device_unplug_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)
> > +{
> > +}
> > +
> > +DeviceState *virt_acpi_init(void)
> > +{
> > +    return sysbus_create_simple(TYPE_VIRT_ACPI, -1, NULL);
> > +}
> > +
> > +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->vmsd = &vmstate_acpi;
> > +    dc->props = virt_acpi_properties;
> > +    dc->realize = virt_device_realize;
> > +
> > +    hc->plug = virt_device_plug_cb;
> > +    hc->unplug_request = virt_device_unplug_request_cb;
> > +    hc->unplug = virt_device_unplug_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/arm/virt.h b/include/hw/arm/virt.h
> > index 507517c..6076167 100644
> > --- a/include/hw/arm/virt.h
> > +++ b/include/hw/arm/virt.h
> > @@ -145,6 +145,7 @@ typedef struct {
> >      OBJECT_CLASS_CHECK(VirtMachineClass, klass,
> TYPE_VIRT_MACHINE)
> >
> >  void virt_acpi_setup(VirtMachineState *vms);
> > +DeviceState *virt_acpi_init(void);
> >
> >  /* Return the number of used redistributor regions  */
> >  static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)

Igor Mammedov March 15, 2019, 8:41 a.m. UTC | #5

On Thu, 14 Mar 2019 16:55:36 +0000
Shameerali Kolothum Thodi <[e-mail]> wrote:

> Hi Igor,
> 
> > -----Original Message-----
> > From: Igor Mammedov [mailto:[e-mail]]
> > Sent: 12 March 2019 15:48
> > To: Shameerali Kolothum Thodi <[e-mail]>
> > Cc: [e-mail]; [e-mail];
> > [e-mail]; [e-mail];
> > [e-mail]; [e-mail];
> > [e-mail]; Linuxarm <[e-mail]>; xuwei (O)
> > <[e-mail]>
> > Subject: Re: [PATCH v2 04/11] hw/arm/virt: Add virtual ACPI device
> > 
> > On Fri, 8 Mar 2019 11:42:11 +0000
> > Shameer Kolothum <[e-mail]> wrote:
> >   
> > > From: Samuel Ortiz <[e-mail]>
> > >
> > > This is to provide an acpi device interface for Arm/virt.
> > > This will be used by Arm/Virt to add hotplug support via
> > > ACPI GED device.
> > >
> > > Signed-off-by: Samuel Ortiz <[e-mail]>
> > > Signed-off-by: Shameer Kolothum <[e-mail]>
> > > ---
> > >  hw/arm/Makefile.objs  |   2 +-
> > >  hw/arm/virt-acpi.c    | 111  
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 
> > does it have to be ARM specific?
> > could it be shared with future x86/virt? (if it could then it would be better to put
> > into hw/acpi/)
> > also considering it implements vitual 'GED' device
> > why not s/virt-acpi/generic-event-device/ ?  
> 
> I think that's a good idea to make it generic and I had a quick go at it.
> 
> But the problem I am facing is as soon as sysbus_create_simple() is called from
> virt_acpi_init(), the realize() function gets called before it sets the VirtAcpiState
> members.

Lets look at it from the other side, what are the reasons to use inherit device
from TYPE_SYS_BUS_DEVICE and use sysbus_create_simple()?

 
> This creates a problem as acpi_memory_hotplug_init() and acpi_ged_init() are
> invoked from the realize() and it needs to have the base address, irq etc set based
> on the platform.
> 
> I tried using the qdev_prop_set_*() as well, but it also fails saying attempt to set
> property after it was realized.
> 
> Not sure what is the best way to solve this other than introducing a global struct to
> hold the configuration.
Globals are frowned upon, we shouldn't do add more.

> 
> Please let me know.
> 
> Thanks,
> Shameer
>  
> >   
> > >  include/hw/arm/virt.h |   1 +
> > >  3 files changed, 113 insertions(+), 1 deletion(-)
> > >  create mode 100644 hw/arm/virt-acpi.c
> > >
> > > diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> > > index fa57c7c..e0db3cd 100644
> > > --- a/hw/arm/Makefile.objs
> > > +++ b/hw/arm/Makefile.objs
> > > @@ -1,6 +1,6 @@
> > >  obj-y += boot.o sysbus-fdt.o
> > >  obj-$(CONFIG_ARM_VIRT) += virt.o
> > > -obj-$(CONFIG_ACPI) += virt-acpi-build.o
> > > +obj-$(CONFIG_ACPI) += virt-acpi-build.o virt-acpi.o
> > >  obj-$(CONFIG_DIGIC) += digic_boards.o
> > >  obj-$(CONFIG_EXYNOS4) += exynos4_boards.o
> > >  obj-$(CONFIG_HIGHBANK) += highbank.o
> > > diff --git a/hw/arm/virt-acpi.c b/hw/arm/virt-acpi.c
> > > new file mode 100644
> > > index 0000000..df8a02b
> > > --- /dev/null
> > > +++ b/hw/arm/virt-acpi.c
> > > @@ -0,0 +1,111 @@
> > > +/*
> > > + *
> > > + * 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 "qemu/range.h"
> > > +#include "qapi/error.h"
> > > +#include "exec/address-spaces.h"
> > > +
> > > +#include "hw/hw.h"
> > > +#include "hw/hotplug.h"
> > > +#include "hw/sysbus.h"
> > > +#include "hw/arm/virt.h"
> > > +
> > > +#include "hw/acpi/acpi.h"
> > > +
> > > +typedef struct VirtAcpiState {
> > > +    SysBusDevice parent_obj;
> > > +} VirtAcpiState;
> > > +
> > > +#define TYPE_VIRT_ACPI "virt-acpi"
> > > +#define VIRT_ACPI(obj) \
> > > +    OBJECT_CHECK(VirtAcpiState, (obj), TYPE_VIRT_ACPI)
> > > +
> > > +static const VMStateDescription vmstate_acpi = {
> > > +    .name = "virt_acpi",
> > > +    .version_id = 1,
> > > +    .minimum_version_id = 1,
> > > +};
> > > +
> > > +static void virt_device_plug_cb(HotplugHandler *hotplug_dev,
> > > +                                DeviceState *dev, Error **errp)
> > > +{
> > > +}
> > > +
> > > +static void virt_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> > > +                                          DeviceState *dev, Error  
> > **errp)  
> > > +{
> > > +}
> > > +
> > > +static void virt_device_unplug_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)
> > > +{
> > > +}
> > > +
> > > +DeviceState *virt_acpi_init(void)
> > > +{
> > > +    return sysbus_create_simple(TYPE_VIRT_ACPI, -1, NULL);
> > > +}
> > > +
> > > +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->vmsd = &vmstate_acpi;
> > > +    dc->props = virt_acpi_properties;
> > > +    dc->realize = virt_device_realize;
> > > +
> > > +    hc->plug = virt_device_plug_cb;
> > > +    hc->unplug_request = virt_device_unplug_request_cb;
> > > +    hc->unplug = virt_device_unplug_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/arm/virt.h b/include/hw/arm/virt.h
> > > index 507517c..6076167 100644
> > > --- a/include/hw/arm/virt.h
> > > +++ b/include/hw/arm/virt.h
> > > @@ -145,6 +145,7 @@ typedef struct {
> > >      OBJECT_CLASS_CHECK(VirtMachineClass, klass,  
> > TYPE_VIRT_MACHINE)  
> > >
> > >  void virt_acpi_setup(VirtMachineState *vms);
> > > +DeviceState *virt_acpi_init(void);
> > >
> > >  /* Return the number of used redistributor regions  */
> > >  static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)  
>

Shameerali Kolothum Thodi March 15, 2019, 12:08 p.m. UTC | #6

> -----Original Message-----
> From: Igor Mammedov [mailto:[e-mail]]
> Sent: 15 March 2019 08:42
> To: Shameerali Kolothum Thodi <[e-mail]>
> Cc: [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; Linuxarm <[e-mail]>; xuwei (O)
> <[e-mail]>
> Subject: Re: [PATCH v2 04/11] hw/arm/virt: Add virtual ACPI device
> 
> On Thu, 14 Mar 2019 16:55:36 +0000
> Shameerali Kolothum Thodi <[e-mail]> wrote:
> 
> > Hi Igor,
> >
> > > -----Original Message-----
> > > From: Igor Mammedov [mailto:[e-mail]]
> > > Sent: 12 March 2019 15:48
> > > To: Shameerali Kolothum Thodi <[e-mail]>
> > > Cc: [e-mail]; [e-mail];
> > > [e-mail]; [e-mail];
> > > [e-mail]; [e-mail];
> > > [e-mail]; Linuxarm <[e-mail]>; xuwei (O)
> > > <[e-mail]>
> > > Subject: Re: [PATCH v2 04/11] hw/arm/virt: Add virtual ACPI device
> > >
> > > On Fri, 8 Mar 2019 11:42:11 +0000
> > > Shameer Kolothum <[e-mail]> wrote:
> > >
> > > > From: Samuel Ortiz <[e-mail]>
> > > >
> > > > This is to provide an acpi device interface for Arm/virt.
> > > > This will be used by Arm/Virt to add hotplug support via
> > > > ACPI GED device.
> > > >
> > > > Signed-off-by: Samuel Ortiz <[e-mail]>
> > > > Signed-off-by: Shameer Kolothum
> <[e-mail]>
> > > > ---
> > > >  hw/arm/Makefile.objs  |   2 +-
> > > >  hw/arm/virt-acpi.c    | 111
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++
> > >
> > > does it have to be ARM specific?
> > > could it be shared with future x86/virt? (if it could then it would be better to
> put
> > > into hw/acpi/)
> > > also considering it implements vitual 'GED' device
> > > why not s/virt-acpi/generic-event-device/ ?
> >
> > I think that's a good idea to make it generic and I had a quick go at it.
> >
> > But the problem I am facing is as soon as sysbus_create_simple() is called
> from
> > virt_acpi_init(), the realize() function gets called before it sets the
> VirtAcpiState
> > members.
> 
> Lets look at it from the other side, what are the reasons to use inherit device
> from TYPE_SYS_BUS_DEVICE and use sysbus_create_simple()?

Right. I think I can solve this by using qdev_create() and qdev_init_nofail().

Thanks,
Shameer
 
> 
> > This creates a problem as acpi_memory_hotplug_init() and acpi_ged_init()
> are
> > invoked from the realize() and it needs to have the base address, irq etc set
> based
> > on the platform.
> >
> > I tried using the qdev_prop_set_*() as well, but it also fails saying attempt to
> set
> > property after it was realized.
> >
> > Not sure what is the best way to solve this other than introducing a global
> struct to
> > hold the configuration.
> Globals are frowned upon, we shouldn't do add more.
> 
> >
> > Please let me know.
> >
> > Thanks,
> > Shameer
> >
> > >
> > > >  include/hw/arm/virt.h |   1 +
> > > >  3 files changed, 113 insertions(+), 1 deletion(-)
> > > >  create mode 100644 hw/arm/virt-acpi.c
> > > >
> > > > diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> > > > index fa57c7c..e0db3cd 100644
> > > > --- a/hw/arm/Makefile.objs
> > > > +++ b/hw/arm/Makefile.objs
> > > > @@ -1,6 +1,6 @@
> > > >  obj-y += boot.o sysbus-fdt.o
> > > >  obj-$(CONFIG_ARM_VIRT) += virt.o
> > > > -obj-$(CONFIG_ACPI) += virt-acpi-build.o
> > > > +obj-$(CONFIG_ACPI) += virt-acpi-build.o virt-acpi.o
> > > >  obj-$(CONFIG_DIGIC) += digic_boards.o
> > > >  obj-$(CONFIG_EXYNOS4) += exynos4_boards.o
> > > >  obj-$(CONFIG_HIGHBANK) += highbank.o
> > > > diff --git a/hw/arm/virt-acpi.c b/hw/arm/virt-acpi.c
> > > > new file mode 100644
> > > > index 0000000..df8a02b
> > > > --- /dev/null
> > > > +++ b/hw/arm/virt-acpi.c
> > > > @@ -0,0 +1,111 @@
> > > > +/*
> > > > + *
> > > > + * 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 "qemu/range.h"
> > > > +#include "qapi/error.h"
> > > > +#include "exec/address-spaces.h"
> > > > +
> > > > +#include "hw/hw.h"
> > > > +#include "hw/hotplug.h"
> > > > +#include "hw/sysbus.h"
> > > > +#include "hw/arm/virt.h"
> > > > +
> > > > +#include "hw/acpi/acpi.h"
> > > > +
> > > > +typedef struct VirtAcpiState {
> > > > +    SysBusDevice parent_obj;
> > > > +} VirtAcpiState;
> > > > +
> > > > +#define TYPE_VIRT_ACPI "virt-acpi"
> > > > +#define VIRT_ACPI(obj) \
> > > > +    OBJECT_CHECK(VirtAcpiState, (obj), TYPE_VIRT_ACPI)
> > > > +
> > > > +static const VMStateDescription vmstate_acpi = {
> > > > +    .name = "virt_acpi",
> > > > +    .version_id = 1,
> > > > +    .minimum_version_id = 1,
> > > > +};
> > > > +
> > > > +static void virt_device_plug_cb(HotplugHandler *hotplug_dev,
> > > > +                                DeviceState *dev, Error **errp)
> > > > +{
> > > > +}
> > > > +
> > > > +static void virt_device_unplug_request_cb(HotplugHandler
> *hotplug_dev,
> > > > +                                          DeviceState *dev,
> Error
> > > **errp)
> > > > +{
> > > > +}
> > > > +
> > > > +static void virt_device_unplug_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)
> > > > +{
> > > > +}
> > > > +
> > > > +DeviceState *virt_acpi_init(void)
> > > > +{
> > > > +    return sysbus_create_simple(TYPE_VIRT_ACPI, -1, NULL);
> > > > +}
> > > > +
> > > > +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->vmsd = &vmstate_acpi;
> > > > +    dc->props = virt_acpi_properties;
> > > > +    dc->realize = virt_device_realize;
> > > > +
> > > > +    hc->plug = virt_device_plug_cb;
> > > > +    hc->unplug_request = virt_device_unplug_request_cb;
> > > > +    hc->unplug = virt_device_unplug_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/arm/virt.h b/include/hw/arm/virt.h
> > > > index 507517c..6076167 100644
> > > > --- a/include/hw/arm/virt.h
> > > > +++ b/include/hw/arm/virt.h
> > > > @@ -145,6 +145,7 @@ typedef struct {
> > > >      OBJECT_CLASS_CHECK(VirtMachineClass, klass,
> > > TYPE_VIRT_MACHINE)
> > > >
> > > >  void virt_acpi_setup(VirtMachineState *vms);
> > > > +DeviceState *virt_acpi_init(void);
> > > >
> > > >  /* Return the number of used redistributor regions  */
> > > >  static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)
> >

Patch

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index fa57c7c..e0db3cd 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -1,6 +1,6 @@ 
 obj-y += boot.o sysbus-fdt.o
 obj-$(CONFIG_ARM_VIRT) += virt.o
-obj-$(CONFIG_ACPI) += virt-acpi-build.o
+obj-$(CONFIG_ACPI) += virt-acpi-build.o virt-acpi.o
 obj-$(CONFIG_DIGIC) += digic_boards.o
 obj-$(CONFIG_EXYNOS4) += exynos4_boards.o
 obj-$(CONFIG_HIGHBANK) += highbank.o
diff --git a/hw/arm/virt-acpi.c b/hw/arm/virt-acpi.c
new file mode 100644
index 0000000..df8a02b
--- /dev/null
+++ b/hw/arm/virt-acpi.c
@@ -0,0 +1,111 @@ 
+/*
+ *
+ * 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 "qemu/range.h"
+#include "qapi/error.h"
+#include "exec/address-spaces.h"
+
+#include "hw/hw.h"
+#include "hw/hotplug.h"
+#include "hw/sysbus.h"
+#include "hw/arm/virt.h"
+
+#include "hw/acpi/acpi.h"
+
+typedef struct VirtAcpiState {
+    SysBusDevice parent_obj;
+} VirtAcpiState;
+
+#define TYPE_VIRT_ACPI "virt-acpi"
+#define VIRT_ACPI(obj) \
+    OBJECT_CHECK(VirtAcpiState, (obj), TYPE_VIRT_ACPI)
+
+static const VMStateDescription vmstate_acpi = {
+    .name = "virt_acpi",
+    .version_id = 1,
+    .minimum_version_id = 1,
+};
+
+static void virt_device_plug_cb(HotplugHandler *hotplug_dev,
+                                DeviceState *dev, Error **errp)
+{
+}
+
+static void virt_device_unplug_request_cb(HotplugHandler *hotplug_dev,
+                                          DeviceState *dev, Error **errp)
+{
+}
+
+static void virt_device_unplug_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)
+{
+}
+
+DeviceState *virt_acpi_init(void)
+{
+    return sysbus_create_simple(TYPE_VIRT_ACPI, -1, NULL);
+}
+
+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->vmsd = &vmstate_acpi;
+    dc->props = virt_acpi_properties;
+    dc->realize = virt_device_realize;
+
+    hc->plug = virt_device_plug_cb;
+    hc->unplug_request = virt_device_unplug_request_cb;
+    hc->unplug = virt_device_unplug_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/arm/virt.h b/include/hw/arm/virt.h
index 507517c..6076167 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -145,6 +145,7 @@  typedef struct {
     OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE)
 
 void virt_acpi_setup(VirtMachineState *vms);
+DeviceState *virt_acpi_init(void);
 
 /* Return the number of used redistributor regions  */
 static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)