| Message ID | 20190409102935.28292-4-shameerali.kolothum.thodi@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | ARM virt: ACPI memory hotplug support | expand |
Hi Shameer, On 4/9/19 12:29 PM, Shameer Kolothum wrote: > From: Samuel Ortiz <sameo@linux.intel.com> > > The ACPI Generic Event Device (GED) is a hardware-reduced specific > device[ACPI v6.1 Section 5.6.9] that handles all platform events, > including the hotplug ones.This patch generates the AML code that > defines GEDs. > > Platforms need to specify their own GedEvent array to describe what > kind of events they want to support through GED. Also this uses a > a single interrupt for the GED device, relying on IO memory region > to communicate the type of device affected by the interrupt. This > way, we can support up to 32 events with a unique interrupt. > > This supports only memory hotplug for now. > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> > Signed-off-by: Sebastien Boeuf <sebastien.boeuf@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 | 311 +++++++++++++++++++++++++++++++++ > include/hw/acpi/generic_event_device.h | 121 +++++++++++++ > 4 files changed, 437 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..856ca04 > --- /dev/null > +++ b/hw/acpi/generic_event_device.c > @@ -0,0 +1,311 @@ > +/* > + * > + * 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. I am not sure we need below statements: see hw/misc/armsse-mhu.c for a recent added file. > + * > + * 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 "qapi/error.h" > +#include "exec/address-spaces.h" > +#include "hw/sysbus.h" > +#include "hw/acpi/acpi.h" > +#include "hw/acpi/generic_event_device.h" > +#include "hw/mem/pc-dimm.h" > + > +static Aml *ged_event_aml(const GedEvent *event) > +{ > + > + if (!event) { > + return NULL; > + } > + > + switch (event->event) { > + case GED_MEMORY_HOTPLUG: > + /* We run a complete memory SCAN when getting a memory hotplug event */ > + return aml_call0(MEMORY_DEVICES_CONTAINER "." MEMORY_SLOT_SCAN_METHOD); > + default: > + break; > + } > + > + return NULL; > +} > + > +/* > + * The ACPI Generic Event Device (GED) is a hardware-reduced specific > + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, > + * including the hotplug ones. Platforms need to specify their own > + * GedEvent array to describe what kind of events they want to support > + * through GED. This routine uses a single interrupt for the GED device, > + * relying on IO memory region to communicate the type of device > + * affected by the interrupt. This way, we can support up to 32 events > + * with a unique interrupt. > + */ > +void build_ged_aml(Aml *table, const char *name, HotplugHandler *hotplug_dev, > + uint32_t ged_irq, AmlRegionSpace rs) > +{ > + AcpiGedState *s = ACPI_GED(hotplug_dev); > + GedEvent *ged_events = s->ged_events; > + Aml *crs = aml_resource_template(); > + Aml *evt, *field; > + Aml *dev = aml_device("%s", name); > + Aml *irq_sel = aml_local(0); > + Aml *isel = aml_name(AML_GED_IRQ_SEL); > + uint32_t i; > + > + if (!s->ged_base || !ged_events || !s->ged_events_size) { > + return; > + } > + > + /* _CRS interrupt */ > + aml_append(crs, aml_interrupt(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH, > + AML_EXCLUSIVE, &ged_irq, 1)); > + /* > + * For each GED event we: > + * - Add an interrupt to the CRS section. > + * - Add a conditional block for each event, inside a while loop. > + * This is semantically equivalent to a switch/case implementation. > + */ > + evt = aml_method("_EVT", 1, AML_SERIALIZED); > + { > + Aml *ged_aml; > + Aml *if_ctx; > + > + /* Local0 = ISEL */ > + aml_append(evt, aml_store(isel, irq_sel)); > + > + /* > + * Here we want to call a method for each supported GED event type. > + * The resulting ASL code looks like: > + * > + * Local0 = ISEL > + * If ((Local0 & irq0) == irq0) > + * { > + * MethodEvent0() > + * } > + * > + * If ((Local0 & irq1) == irq1) > + * { > + * MethodEvent1() > + * } > + * ... > + */ > + > + for (i = 0; i < s->ged_events_size; i++) { > + ged_aml = ged_event_aml(&ged_events[i]); > + if (!ged_aml) { > + continue; > + } > + > + /* If ((Local1 == irq))*/ > + if_ctx = aml_if(aml_equal(aml_and(irq_sel, > + aml_int(ged_events[i].selector), NULL), > + aml_int(ged_events[i].selector))); > + { > + /* AML for this specific type of event */ > + aml_append(if_ctx, ged_aml); > + } > + > + /* > + * We append the first "if" to the "while" context. > + * Other "if"s will be "elseif"s. Are we sure about that: in hw/acpi/nvdimm.c nvdimm_build_common_dsm() I can see an aml_else() is added inbetween aml_if's? > + */ > + aml_append(evt, if_ctx); > + } > + } > + > + aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0013"))); > + aml_append(dev, aml_name_decl("_UID", aml_string(GED_DEVICE))); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + > + /* Append IO region */ > + aml_append(dev, aml_operation_region(AML_GED_IRQ_REG, rs, > + aml_int(s->ged_base + ACPI_GED_IRQ_SEL_OFFSET), > + ACPI_GED_IRQ_SEL_LEN)); > + field = aml_field(AML_GED_IRQ_REG, AML_DWORD_ACC, AML_NOLOCK, > + AML_WRITE_AS_ZEROS); > + aml_append(field, aml_named_field(AML_GED_IRQ_SEL, > + ACPI_GED_IRQ_SEL_LEN * 8)); nit s/8/BITS_PER_BYTE as done in nvdimm.c. > + aml_append(dev, field); > + > + /* Append _EVT method */ > + aml_append(dev, evt); > + > + aml_append(table, dev); > +} > + > +/* Memory read by the GED _EVT AML dynamic method */ > +static uint64_t ged_read(void *opaque, hwaddr addr, unsigned size) > +{ > + uint64_t val = 0; > + GEDState *ged_st = opaque; > + > + switch (addr) { > + case ACPI_GED_IRQ_SEL_OFFSET: > + /* Read the selector value and reset it */ > + qemu_mutex_lock(&ged_st->lock); > + val = ged_st->sel; > + ged_st->sel = 0; > + qemu_mutex_unlock(&ged_st->lock); > + break; > + default: > + break; > + } > + > + return val; > +} > + > +/* Nothing is expected to be written to the GED memory region */ > +static void ged_write(void *opaque, hwaddr addr, uint64_t data, > + unsigned int size) > +{ > +} > + > +static const MemoryRegionOps ged_ops = { > + .read = ged_read, > + .write = ged_write, > + .endianness = DEVICE_LITTLE_ENDIAN, > + .valid = { > + .min_access_size = 4, > + .max_access_size = 4, > + }, > +}; > + > +static void acpi_ged_event(GEDState *ged_st, uint32_t ged_irq_sel) > +{ > + /* I would personally inline that code in void acpi_ged_send_event() > + * Set the GED IRQ selector to the expected device type value. This > + * way, the ACPI method will be able to trigger the right code based > + * on a unique IRQ. > + */ > + qemu_mutex_lock(&ged_st->lock); > + ged_st->sel = ged_irq_sel; > + qemu_mutex_unlock(&ged_st->lock); > + > + /* Trigger the event by sending an interrupt to the guest. */ > + qemu_irq_pulse(ged_st->gsi[ged_st->irq]); I don't really understand why we need this array. Doesn't The GED use a single IRQ. So why can't you store the precise qemu_irq pointer directly through the propery (DEFINE_PROP_PTR("gsi", AcpiGedState, gsi))? > +} > + > +static void acpi_ged_init(MemoryRegion *as, DeviceState *dev, GEDState *ged_st) > +{ > + AcpiGedState *s = ACPI_GED(dev); > + > + assert(s->ged_base); > + > + ged_st->irq = s->ged_irq; > + ged_st->gsi = s->gsi; > + qemu_mutex_init(&ged_st->lock); > + memory_region_init_io(&ged_st->io, OBJECT(dev), &ged_ops, ged_st, > + "acpi-ged-event", ACPI_GED_REG_LEN); > + memory_region_add_subregion(as, s->ged_base, &ged_st->io); > +} > + > +static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + AcpiGedState *s = ACPI_GED(hotplug_dev); > + > + if (s->memhp_state.is_enabled && > + object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > + acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, > + dev, errp); > + } else { > + error_setg(errp, "virt: device plug request for unsupported device" > + " type: %s", object_get_typename(OBJECT(dev))); > + } > +} > + > +static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev) > +{ > + AcpiGedState *s = ACPI_GED(adev); > + uint32_t sel; > + > + if (ev & ACPI_MEMORY_HOTPLUG_STATUS) { > + sel = ACPI_GED_IRQ_SEL_MEM; > + } else { > + /* Unknown event. Return without generating interrupt. */ > + return; > + } > + > + /* > + * We inject the hotplug interrupt. The IRQ selector will make > + * the difference from the ACPI table. > + */ > + acpi_ged_event(&s->ged_state, sel); > +} > + > +static void acpi_ged_device_realize(DeviceState *dev, Error **errp) > +{ > + AcpiGedState *s = ACPI_GED(dev); > + > + if (s->memhp_state.is_enabled) { > + acpi_memory_hotplug_init(get_system_memory(), OBJECT(dev), > + &s->memhp_state, > + s->memhp_base); > + acpi_ged_init(get_system_memory(), dev, &s->ged_state); > + } > +} > + > +static Property acpi_ged_properties[] = { > + /* > + * Memory hotplug base address is a property of GED here, > + * because GED handles memory hotplug event and MEMORY_HOTPLUG_DEVICE > + * gets initialized when GED device is realized. > + */ > + DEFINE_PROP_UINT64("memhp-base", AcpiGedState, memhp_base, 0), > + DEFINE_PROP_BOOL("memory-hotplug-support", AcpiGedState, > + memhp_state.is_enabled, true), > + DEFINE_PROP_PTR("gsi", AcpiGedState, gsi), > + DEFINE_PROP_UINT64("ged-base", AcpiGedState, ged_base, 0), > + DEFINE_PROP_UINT32("ged-irq", AcpiGedState, ged_irq, 0), > + DEFINE_PROP_PTR("ged-events", AcpiGedState, ged_events), > + DEFINE_PROP_UINT32("ged-events-size", AcpiGedState, ged_events_size, 0), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void acpi_ged_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 = acpi_ged_properties; > + dc->realize = acpi_ged_device_realize; > + > + /* Reason: pointer properties "gsi" and "ged_events" */ > + dc->user_creatable = false; > + > + hc->plug = acpi_ged_device_plug_cb; > + > + adevc->send_event = acpi_ged_send_event; > +} > + > +static const TypeInfo acpi_ged_info = { > + .name = TYPE_ACPI_GED, > + .parent = TYPE_DEVICE, > + .instance_size = sizeof(AcpiGedState), > + .class_init = acpi_ged_class_init, > + .interfaces = (InterfaceInfo[]) { > + { TYPE_HOTPLUG_HANDLER }, > + { TYPE_ACPI_DEVICE_IF }, > + { } > + } > +}; > + > +static void acpi_ged_register_types(void) > +{ > + type_register_static(&acpi_ged_info); > +} > + > +type_init(acpi_ged_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..9c840d8 > --- /dev/null > +++ b/include/hw/acpi/generic_event_device.h > @@ -0,0 +1,121 @@ > +/* > + * > + * 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/>. > + * > + * The ACPI Generic Event Device (GED) is a hardware-reduced specific > + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, > + * including the hotplug ones. Generic Event Device allows platforms > + * to handle interrupts in ACPI ASL statements. It follows a very > + * similar approach like the _EVT method from GPIO events. All > + * interrupts are listed in _CRS and the handler is written in _EVT> + * method. Here, we use a single interrupt for the GED device, relying > + * on IO memory region to communicate the type of device affected by > + * the interrupt. This way, we can support up to 32 events with a > + * unique interrupt. > + * > + * Here is an example. > + * > + * Device (\_SB.GED) > + * { > + * Name (_HID, "ACPI0013") > + * Name (_UID, Zero) > + * Name (_CRS, ResourceTemplate () > + * { > + * Interrupt (ResourceConsumer, Edge, ActiveHigh, Exclusive, ,, ) > + * { > + * 0x00000029, > + * } > + * }) > + * OperationRegion (IREG, SystemMemory, 0x09080000, 0x04) > + * Field (IREG, DWordAcc, NoLock, WriteAsZeros) > + * { > + * ISEL, 32 > + * } > + * > + * Method (_EVT, 1, Serialized) // _EVT: Event > + * { > + * Local0 = ISEL // ISEL = IO memory region which specifies the > + * // device type. > + * If (((Local0 & irq0) == irq0)) > + * { > + * MethodEvent0() > + * } > + * ElseIf ((Local0 & irq1) == irq1) > + * { > + * MethodEvent1() > + * } > + * ... > + * } > + * } > + * > + */ > + > +#ifndef HW_ACPI_GED_H > +#define HW_ACPI_GED_H > + > +#include "hw/acpi/memory_hotplug.h" > + > +#define TYPE_ACPI_GED "acpi-ged" > +#define ACPI_GED(obj) \ > + OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED) > + > +#define ACPI_GED_IRQ_SEL_OFFSET 0x0 > +#define ACPI_GED_IRQ_SEL_LEN 0x4 > +#define ACPI_GED_IRQ_SEL_MEM 0x1 > +#define ACPI_GED_REG_LEN 0x4 > + > +#define GED_DEVICE "GED" > +#define AML_GED_IRQ_REG "IREG" > +#define AML_GED_IRQ_SEL "ISEL" > + > +typedef enum { > + GED_MEMORY_HOTPLUG = 1, > +} GedEventType; > + > +/* > + * Platforms need to specify their own GedEvent array > + * to describe what kind of events they want to support > + * through GED. > + */ > +typedef struct GedEvent { > + uint32_t selector; > + GedEventType event; > +} GedEvent; > + > +typedef struct GEDState { > + MemoryRegion io; > + uint32_t sel; > + uint32_t irq; > + qemu_irq *gsi; so I am not sure why with need gsi + irq. > + QemuMutex lock; > +} GEDState; > + > + > +typedef struct AcpiGedState { > + DeviceClass parent_obj; > + MemHotplugState memhp_state; > + hwaddr memhp_base; > + void *gsi; > + hwaddr ged_base; > + GEDState ged_state; > + uint32_t ged_irq; > + void *ged_events; > + uint32_t ged_events_size; > +} AcpiGedState; > + > +void build_ged_aml(Aml *table, const char* name, HotplugHandler *hotplug_dev, > + uint32_t ged_irq, AmlRegionSpace rs); > + > +#endif > Besides, this looks good to me. Thanks Eric
Hi Eric, > -----Original Message----- > From: Auger Eric [mailto:eric.auger@redhat.com] > Sent: 30 April 2019 16:50 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; > qemu-devel@nongnu.org; qemu-arm@nongnu.org; imammedo@redhat.com > Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com; > sameo@linux.intel.com; sebastien.boeuf@intel.com; xuwei (O) > <xuwei5@huawei.com>; lersek@redhat.com; ard.biesheuvel@linaro.org; > Linuxarm <linuxarm@huawei.com> > Subject: Re: [PATCH v4 3/8] hw/acpi: Add ACPI Generic Event Device Support > > Hi Shameer, > > On 4/9/19 12:29 PM, Shameer Kolothum wrote: > > From: Samuel Ortiz <sameo@linux.intel.com> > > > > The ACPI Generic Event Device (GED) is a hardware-reduced specific > > device[ACPI v6.1 Section 5.6.9] that handles all platform events, > > including the hotplug ones.This patch generates the AML code that > > defines GEDs. > > > > Platforms need to specify their own GedEvent array to describe what > > kind of events they want to support through GED. Also this uses a > > a single interrupt for the GED device, relying on IO memory region > > to communicate the type of device affected by the interrupt. This > > way, we can support up to 32 events with a unique interrupt. > > > > This supports only memory hotplug for now. > > > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> > > Signed-off-by: Sebastien Boeuf <sebastien.boeuf@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 | 311 > +++++++++++++++++++++++++++++++++ > > include/hw/acpi/generic_event_device.h | 121 +++++++++++++ > > 4 files changed, 437 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..856ca04 > > --- /dev/null > > +++ b/hw/acpi/generic_event_device.c > > @@ -0,0 +1,311 @@ > > +/* > > + * > > + * 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. > I am not sure we need below statements: see hw/misc/armsse-mhu.c for a > recent added file. Ok. I will get rid of this. > > + * > > + * 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 "qapi/error.h" > > +#include "exec/address-spaces.h" > > +#include "hw/sysbus.h" > > +#include "hw/acpi/acpi.h" > > +#include "hw/acpi/generic_event_device.h" > > +#include "hw/mem/pc-dimm.h" > > + > > +static Aml *ged_event_aml(const GedEvent *event) > > +{ > > + > > + if (!event) { > > + return NULL; > > + } > > + > > + switch (event->event) { > > + case GED_MEMORY_HOTPLUG: > > + /* We run a complete memory SCAN when getting a memory > hotplug event */ > > + return aml_call0(MEMORY_DEVICES_CONTAINER "." > MEMORY_SLOT_SCAN_METHOD); > > + default: > > + break; > > + } > > + > > + return NULL; > > +} > > + > > +/* > > + * The ACPI Generic Event Device (GED) is a hardware-reduced specific > > + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, > > + * including the hotplug ones. Platforms need to specify their own > > + * GedEvent array to describe what kind of events they want to support > > + * through GED. This routine uses a single interrupt for the GED device, > > + * relying on IO memory region to communicate the type of device > > + * affected by the interrupt. This way, we can support up to 32 events > > + * with a unique interrupt. > > + */ > > +void build_ged_aml(Aml *table, const char *name, HotplugHandler > *hotplug_dev, > > + uint32_t ged_irq, AmlRegionSpace rs) > > +{ > > + AcpiGedState *s = ACPI_GED(hotplug_dev); > > + GedEvent *ged_events = s->ged_events; > > + Aml *crs = aml_resource_template(); > > + Aml *evt, *field; > > + Aml *dev = aml_device("%s", name); > > + Aml *irq_sel = aml_local(0); > > + Aml *isel = aml_name(AML_GED_IRQ_SEL); > > + uint32_t i; > > + > > + if (!s->ged_base || !ged_events || !s->ged_events_size) { > > + return; > > + } > > + > > + /* _CRS interrupt */ > > + aml_append(crs, aml_interrupt(AML_CONSUMER, AML_EDGE, > AML_ACTIVE_HIGH, > > + AML_EXCLUSIVE, &ged_irq, 1)); > > + /* > > + * For each GED event we: > > + * - Add an interrupt to the CRS section. > > + * - Add a conditional block for each event, inside a while loop. > > + * This is semantically equivalent to a switch/case implementation. > > + */ > > + evt = aml_method("_EVT", 1, AML_SERIALIZED); > > + { > > + Aml *ged_aml; > > + Aml *if_ctx; > > + > > + /* Local0 = ISEL */ > > + aml_append(evt, aml_store(isel, irq_sel)); > > + > > + /* > > + * Here we want to call a method for each supported GED event > type. > > + * The resulting ASL code looks like: > > + * > > + * Local0 = ISEL > > + * If ((Local0 & irq0) == irq0) > > + * { > > + * MethodEvent0() > > + * } > > + * > > + * If ((Local0 & irq1) == irq1) > > + * { > > + * MethodEvent1() > > + * } > > + * ... > > + */ > > + > > + for (i = 0; i < s->ged_events_size; i++) { > > + ged_aml = ged_event_aml(&ged_events[i]); > > + if (!ged_aml) { > > + continue; > > + } > > + > > + /* If ((Local1 == irq))*/ > > + if_ctx = aml_if(aml_equal(aml_and(irq_sel, > > + > aml_int(ged_events[i].selector), NULL), > > + > aml_int(ged_events[i].selector))); > > + { > > + /* AML for this specific type of event */ > > + aml_append(if_ctx, ged_aml); > > + } > > + > > + /* > > + * We append the first "if" to the "while" context. > > + * Other "if"s will be "elseif"s. > Are we sure about that: > in hw/acpi/nvdimm.c nvdimm_build_common_dsm() I can see an aml_else() is > added inbetween aml_if's? Right. I think the comment here is misleading. This will be as mentioned in the comment block above, If ((Local0 & irq0) == irq0) { MethodEvent0() } If ((Local0 & irq1) == irq1) { MethodEvent1() } And this should do the job. Do we really need ElseIf block here? > > + */ > > + aml_append(evt, if_ctx); > > + } > > + } > > + > > + aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0013"))); > > + aml_append(dev, aml_name_decl("_UID", aml_string(GED_DEVICE))); > > + aml_append(dev, aml_name_decl("_CRS", crs)); > > + > > + /* Append IO region */ > > + aml_append(dev, aml_operation_region(AML_GED_IRQ_REG, rs, > > + aml_int(s->ged_base + ACPI_GED_IRQ_SEL_OFFSET), > > + ACPI_GED_IRQ_SEL_LEN)); > > + field = aml_field(AML_GED_IRQ_REG, AML_DWORD_ACC, > AML_NOLOCK, > > + AML_WRITE_AS_ZEROS); > > + aml_append(field, aml_named_field(AML_GED_IRQ_SEL, > > + ACPI_GED_IRQ_SEL_LEN * > 8)); > nit s/8/BITS_PER_BYTE as done in nvdimm.c. Ok. > > + aml_append(dev, field); > > + > > + /* Append _EVT method */ > > + aml_append(dev, evt); > > + > > + aml_append(table, dev); > > +} > > + > > +/* Memory read by the GED _EVT AML dynamic method */ > > +static uint64_t ged_read(void *opaque, hwaddr addr, unsigned size) > > +{ > > + uint64_t val = 0; > > + GEDState *ged_st = opaque; > > + > > + switch (addr) { > > + case ACPI_GED_IRQ_SEL_OFFSET: > > + /* Read the selector value and reset it */ > > + qemu_mutex_lock(&ged_st->lock); > > + val = ged_st->sel; > > + ged_st->sel = 0; > > + qemu_mutex_unlock(&ged_st->lock); > > + break; > > + default: > > + break; > > + } > > + > > + return val; > > +} > > + > > +/* Nothing is expected to be written to the GED memory region */ > > +static void ged_write(void *opaque, hwaddr addr, uint64_t data, > > + unsigned int size) > > +{ > > +} > > + > > +static const MemoryRegionOps ged_ops = { > > + .read = ged_read, > > + .write = ged_write, > > + .endianness = DEVICE_LITTLE_ENDIAN, > > + .valid = { > > + .min_access_size = 4, > > + .max_access_size = 4, > > + }, > > +}; > > + > > +static void acpi_ged_event(GEDState *ged_st, uint32_t ged_irq_sel) > > +{ > > + /* > I would personally inline that code in void acpi_ged_send_event() Ok. > > + * Set the GED IRQ selector to the expected device type value. This > > + * way, the ACPI method will be able to trigger the right code based > > + * on a unique IRQ. > > + */ > > + qemu_mutex_lock(&ged_st->lock); > > + ged_st->sel = ged_irq_sel; > > + qemu_mutex_unlock(&ged_st->lock); > > + > > + /* Trigger the event by sending an interrupt to the guest. */ > > + qemu_irq_pulse(ged_st->gsi[ged_st->irq]); > I don't really understand why we need this array. Doesn't The GED use a > single IRQ. So why can't you store the precise qemu_irq pointer directly > through the propery (DEFINE_PROP_PTR("gsi", AcpiGedState, gsi))? Yes. You have raised this point previously as well. Sorry, I missed it when I reworked the GED type from SYS_BUS_DEVICE to TYPE_DEVICE. I think we can get rid of this array and the GED/gsi routing done in virt.c. > > +} > > + > > +static void acpi_ged_init(MemoryRegion *as, DeviceState *dev, GEDState > *ged_st) > > +{ > > + AcpiGedState *s = ACPI_GED(dev); > > + > > + assert(s->ged_base); > > + > > + ged_st->irq = s->ged_irq; > > + ged_st->gsi = s->gsi; > > + qemu_mutex_init(&ged_st->lock); > > + memory_region_init_io(&ged_st->io, OBJECT(dev), &ged_ops, ged_st, > > + "acpi-ged-event", ACPI_GED_REG_LEN); > > + memory_region_add_subregion(as, s->ged_base, &ged_st->io); > > +} > > + > > +static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev, > > + DeviceState *dev, Error **errp) > > +{ > > + AcpiGedState *s = ACPI_GED(hotplug_dev); > > + > > + if (s->memhp_state.is_enabled && > > + object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > > + acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, > > + dev, errp); > > + } else { > > + error_setg(errp, "virt: device plug request for unsupported > device" > > + " type: %s", object_get_typename(OBJECT(dev))); > > + } > > +} > > + > > +static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits > ev) > > +{ > > + AcpiGedState *s = ACPI_GED(adev); > > + uint32_t sel; > > + > > + if (ev & ACPI_MEMORY_HOTPLUG_STATUS) { > > + sel = ACPI_GED_IRQ_SEL_MEM; > > + } else { > > + /* Unknown event. Return without generating interrupt. */ > > + return; > > + } > > + > > + /* > > + * We inject the hotplug interrupt. The IRQ selector will make > > + * the difference from the ACPI table. > > + */ > > + acpi_ged_event(&s->ged_state, sel); > > +} > > + > > +static void acpi_ged_device_realize(DeviceState *dev, Error **errp) > > +{ > > + AcpiGedState *s = ACPI_GED(dev); > > + > > + if (s->memhp_state.is_enabled) { > > + acpi_memory_hotplug_init(get_system_memory(), OBJECT(dev), > > + &s->memhp_state, > > + s->memhp_base); > > + acpi_ged_init(get_system_memory(), dev, &s->ged_state); > > + } > > +} > > + > > +static Property acpi_ged_properties[] = { > > + /* > > + * Memory hotplug base address is a property of GED here, > > + * because GED handles memory hotplug event and > MEMORY_HOTPLUG_DEVICE > > + * gets initialized when GED device is realized. > > + */ > > + DEFINE_PROP_UINT64("memhp-base", AcpiGedState, memhp_base, > 0), > > + DEFINE_PROP_BOOL("memory-hotplug-support", AcpiGedState, > > + memhp_state.is_enabled, true), > > + DEFINE_PROP_PTR("gsi", AcpiGedState, gsi), > > + DEFINE_PROP_UINT64("ged-base", AcpiGedState, ged_base, 0), > > + DEFINE_PROP_UINT32("ged-irq", AcpiGedState, ged_irq, 0), > > + DEFINE_PROP_PTR("ged-events", AcpiGedState, ged_events), > > + DEFINE_PROP_UINT32("ged-events-size", AcpiGedState, > ged_events_size, 0), > > + DEFINE_PROP_END_OF_LIST(), > > +}; > > + > > +static void acpi_ged_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 = acpi_ged_properties; > > + dc->realize = acpi_ged_device_realize; > > + > > + /* Reason: pointer properties "gsi" and "ged_events" */ > > + dc->user_creatable = false; > > + > > + hc->plug = acpi_ged_device_plug_cb; > > + > > + adevc->send_event = acpi_ged_send_event; > > +} > > + > > +static const TypeInfo acpi_ged_info = { > > + .name = TYPE_ACPI_GED, > > + .parent = TYPE_DEVICE, > > + .instance_size = sizeof(AcpiGedState), > > + .class_init = acpi_ged_class_init, > > + .interfaces = (InterfaceInfo[]) { > > + { TYPE_HOTPLUG_HANDLER }, > > + { TYPE_ACPI_DEVICE_IF }, > > + { } > > + } > > +}; > > + > > +static void acpi_ged_register_types(void) > > +{ > > + type_register_static(&acpi_ged_info); > > +} > > + > > +type_init(acpi_ged_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..9c840d8 > > --- /dev/null > > +++ b/include/hw/acpi/generic_event_device.h > > @@ -0,0 +1,121 @@ > > +/* > > + * > > + * 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/>. > > + * > > + * The ACPI Generic Event Device (GED) is a hardware-reduced specific > > + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, > > + * including the hotplug ones. Generic Event Device allows platforms > > + * to handle interrupts in ACPI ASL statements. It follows a very > > + * similar approach like the _EVT method from GPIO events. All > > + * interrupts are listed in _CRS and the handler is written in _EVT> + * > method. Here, we use a single interrupt for the GED device, relying > > + * on IO memory region to communicate the type of device affected by > > + * the interrupt. This way, we can support up to 32 events with a > > + * unique interrupt. > > + * > > + * Here is an example. > > + * > > + * Device (\_SB.GED) > > + * { > > + * Name (_HID, "ACPI0013") > > + * Name (_UID, Zero) > > + * Name (_CRS, ResourceTemplate () > > + * { > > + * Interrupt (ResourceConsumer, Edge, ActiveHigh, Exclusive, ,, ) > > + * { > > + * 0x00000029, > > + * } > > + * }) > > + * OperationRegion (IREG, SystemMemory, 0x09080000, 0x04) > > + * Field (IREG, DWordAcc, NoLock, WriteAsZeros) > > + * { > > + * ISEL, 32 > > + * } > > + * > > + * Method (_EVT, 1, Serialized) // _EVT: Event > > + * { > > + * Local0 = ISEL // ISEL = IO memory region which specifies the > > + * // device type. > > + * If (((Local0 & irq0) == irq0)) > > + * { > > + * MethodEvent0() > > + * } > > + * ElseIf ((Local0 & irq1) == irq1) > > + * { > > + * MethodEvent1() > > + * } > > + * ... > > + * } > > + * } > > + * > > + */ > > + > > +#ifndef HW_ACPI_GED_H > > +#define HW_ACPI_GED_H > > + > > +#include "hw/acpi/memory_hotplug.h" > > + > > +#define TYPE_ACPI_GED "acpi-ged" > > +#define ACPI_GED(obj) \ > > + OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED) > > + > > +#define ACPI_GED_IRQ_SEL_OFFSET 0x0 > > +#define ACPI_GED_IRQ_SEL_LEN 0x4 > > +#define ACPI_GED_IRQ_SEL_MEM 0x1 > > +#define ACPI_GED_REG_LEN 0x4 > > + > > +#define GED_DEVICE "GED" > > +#define AML_GED_IRQ_REG "IREG" > > +#define AML_GED_IRQ_SEL "ISEL" > > + > > +typedef enum { > > + GED_MEMORY_HOTPLUG = 1, > > +} GedEventType; > > + > > +/* > > + * Platforms need to specify their own GedEvent array > > + * to describe what kind of events they want to support > > + * through GED. > > + */ > > +typedef struct GedEvent { > > + uint32_t selector; > > + GedEventType event; > > +} GedEvent; > > + > > +typedef struct GEDState { > > + MemoryRegion io; > > + uint32_t sel; > > + uint32_t irq; > > + qemu_irq *gsi; > so I am not sure why with need gsi + irq. > > + QemuMutex lock; > > +} GEDState; > > + > > + > > +typedef struct AcpiGedState { > > + DeviceClass parent_obj; > > + MemHotplugState memhp_state; > > + hwaddr memhp_base; > > + void *gsi; > > + hwaddr ged_base; > > + GEDState ged_state; > > + uint32_t ged_irq; > > + void *ged_events; > > + uint32_t ged_events_size; > > +} AcpiGedState; > > + > > +void build_ged_aml(Aml *table, const char* name, HotplugHandler > *hotplug_dev, > > + uint32_t ged_irq, AmlRegionSpace rs); > > + > > +#endif > > > Besides, this looks good to me. > Thanks, Shameer
On Tue, 9 Apr 2019 at 12:31, Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote: > > From: Samuel Ortiz <sameo@linux.intel.com> > > The ACPI Generic Event Device (GED) is a hardware-reduced specific > device[ACPI v6.1 Section 5.6.9] that handles all platform events, > including the hotplug ones.This patch generates the AML code that > defines GEDs. > > Platforms need to specify their own GedEvent array to describe what > kind of events they want to support through GED. Also this uses a > a single interrupt for the GED device, relying on IO memory region > to communicate the type of device affected by the interrupt. This > way, we can support up to 32 events with a unique interrupt. > > This supports only memory hotplug for now. > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> > Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com> > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> Apologies if this question has been raised before, but do we really need a separate device for this? We already handle the power button via _AEI/_Exx on the GPIO device, and I think we should be able to add additional events using that interface, rather than have two event signalling methods/devices on the same platform. > --- > hw/acpi/Kconfig | 4 + > hw/acpi/Makefile.objs | 1 + > hw/acpi/generic_event_device.c | 311 +++++++++++++++++++++++++++++++++ > include/hw/acpi/generic_event_device.h | 121 +++++++++++++ > 4 files changed, 437 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..856ca04 > --- /dev/null > +++ b/hw/acpi/generic_event_device.c > @@ -0,0 +1,311 @@ > +/* > + * > + * 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 "qapi/error.h" > +#include "exec/address-spaces.h" > +#include "hw/sysbus.h" > +#include "hw/acpi/acpi.h" > +#include "hw/acpi/generic_event_device.h" > +#include "hw/mem/pc-dimm.h" > + > +static Aml *ged_event_aml(const GedEvent *event) > +{ > + > + if (!event) { > + return NULL; > + } > + > + switch (event->event) { > + case GED_MEMORY_HOTPLUG: > + /* We run a complete memory SCAN when getting a memory hotplug event */ > + return aml_call0(MEMORY_DEVICES_CONTAINER "." MEMORY_SLOT_SCAN_METHOD); > + default: > + break; > + } > + > + return NULL; > +} > + > +/* > + * The ACPI Generic Event Device (GED) is a hardware-reduced specific > + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, > + * including the hotplug ones. Platforms need to specify their own > + * GedEvent array to describe what kind of events they want to support > + * through GED. This routine uses a single interrupt for the GED device, > + * relying on IO memory region to communicate the type of device > + * affected by the interrupt. This way, we can support up to 32 events > + * with a unique interrupt. > + */ > +void build_ged_aml(Aml *table, const char *name, HotplugHandler *hotplug_dev, > + uint32_t ged_irq, AmlRegionSpace rs) > +{ > + AcpiGedState *s = ACPI_GED(hotplug_dev); > + GedEvent *ged_events = s->ged_events; > + Aml *crs = aml_resource_template(); > + Aml *evt, *field; > + Aml *dev = aml_device("%s", name); > + Aml *irq_sel = aml_local(0); > + Aml *isel = aml_name(AML_GED_IRQ_SEL); > + uint32_t i; > + > + if (!s->ged_base || !ged_events || !s->ged_events_size) { > + return; > + } > + > + /* _CRS interrupt */ > + aml_append(crs, aml_interrupt(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH, > + AML_EXCLUSIVE, &ged_irq, 1)); > + /* > + * For each GED event we: > + * - Add an interrupt to the CRS section. > + * - Add a conditional block for each event, inside a while loop. > + * This is semantically equivalent to a switch/case implementation. > + */ > + evt = aml_method("_EVT", 1, AML_SERIALIZED); > + { > + Aml *ged_aml; > + Aml *if_ctx; > + > + /* Local0 = ISEL */ > + aml_append(evt, aml_store(isel, irq_sel)); > + > + /* > + * Here we want to call a method for each supported GED event type. > + * The resulting ASL code looks like: > + * > + * Local0 = ISEL > + * If ((Local0 & irq0) == irq0) > + * { > + * MethodEvent0() > + * } > + * > + * If ((Local0 & irq1) == irq1) > + * { > + * MethodEvent1() > + * } > + * ... > + */ > + > + for (i = 0; i < s->ged_events_size; i++) { > + ged_aml = ged_event_aml(&ged_events[i]); > + if (!ged_aml) { > + continue; > + } > + > + /* If ((Local1 == irq))*/ > + if_ctx = aml_if(aml_equal(aml_and(irq_sel, > + aml_int(ged_events[i].selector), NULL), > + aml_int(ged_events[i].selector))); > + { > + /* AML for this specific type of event */ > + aml_append(if_ctx, ged_aml); > + } > + > + /* > + * We append the first "if" to the "while" context. > + * Other "if"s will be "elseif"s. > + */ > + aml_append(evt, if_ctx); > + } > + } > + > + aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0013"))); > + aml_append(dev, aml_name_decl("_UID", aml_string(GED_DEVICE))); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + > + /* Append IO region */ > + aml_append(dev, aml_operation_region(AML_GED_IRQ_REG, rs, > + aml_int(s->ged_base + ACPI_GED_IRQ_SEL_OFFSET), > + ACPI_GED_IRQ_SEL_LEN)); > + field = aml_field(AML_GED_IRQ_REG, AML_DWORD_ACC, AML_NOLOCK, > + AML_WRITE_AS_ZEROS); > + aml_append(field, aml_named_field(AML_GED_IRQ_SEL, > + ACPI_GED_IRQ_SEL_LEN * 8)); > + aml_append(dev, field); > + > + /* Append _EVT method */ > + aml_append(dev, evt); > + > + aml_append(table, dev); > +} > + > +/* Memory read by the GED _EVT AML dynamic method */ > +static uint64_t ged_read(void *opaque, hwaddr addr, unsigned size) > +{ > + uint64_t val = 0; > + GEDState *ged_st = opaque; > + > + switch (addr) { > + case ACPI_GED_IRQ_SEL_OFFSET: > + /* Read the selector value and reset it */ > + qemu_mutex_lock(&ged_st->lock); > + val = ged_st->sel; > + ged_st->sel = 0; > + qemu_mutex_unlock(&ged_st->lock); > + break; > + default: > + break; > + } > + > + return val; > +} > + > +/* Nothing is expected to be written to the GED memory region */ > +static void ged_write(void *opaque, hwaddr addr, uint64_t data, > + unsigned int size) > +{ > +} > + > +static const MemoryRegionOps ged_ops = { > + .read = ged_read, > + .write = ged_write, > + .endianness = DEVICE_LITTLE_ENDIAN, > + .valid = { > + .min_access_size = 4, > + .max_access_size = 4, > + }, > +}; > + > +static void acpi_ged_event(GEDState *ged_st, uint32_t ged_irq_sel) > +{ > + /* > + * Set the GED IRQ selector to the expected device type value. This > + * way, the ACPI method will be able to trigger the right code based > + * on a unique IRQ. > + */ > + qemu_mutex_lock(&ged_st->lock); > + ged_st->sel = ged_irq_sel; > + qemu_mutex_unlock(&ged_st->lock); > + > + /* Trigger the event by sending an interrupt to the guest. */ > + qemu_irq_pulse(ged_st->gsi[ged_st->irq]); > +} > + > +static void acpi_ged_init(MemoryRegion *as, DeviceState *dev, GEDState *ged_st) > +{ > + AcpiGedState *s = ACPI_GED(dev); > + > + assert(s->ged_base); > + > + ged_st->irq = s->ged_irq; > + ged_st->gsi = s->gsi; > + qemu_mutex_init(&ged_st->lock); > + memory_region_init_io(&ged_st->io, OBJECT(dev), &ged_ops, ged_st, > + "acpi-ged-event", ACPI_GED_REG_LEN); > + memory_region_add_subregion(as, s->ged_base, &ged_st->io); > +} > + > +static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + AcpiGedState *s = ACPI_GED(hotplug_dev); > + > + if (s->memhp_state.is_enabled && > + object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > + acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, > + dev, errp); > + } else { > + error_setg(errp, "virt: device plug request for unsupported device" > + " type: %s", object_get_typename(OBJECT(dev))); > + } > +} > + > +static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev) > +{ > + AcpiGedState *s = ACPI_GED(adev); > + uint32_t sel; > + > + if (ev & ACPI_MEMORY_HOTPLUG_STATUS) { > + sel = ACPI_GED_IRQ_SEL_MEM; > + } else { > + /* Unknown event. Return without generating interrupt. */ > + return; > + } > + > + /* > + * We inject the hotplug interrupt. The IRQ selector will make > + * the difference from the ACPI table. > + */ > + acpi_ged_event(&s->ged_state, sel); > +} > + > +static void acpi_ged_device_realize(DeviceState *dev, Error **errp) > +{ > + AcpiGedState *s = ACPI_GED(dev); > + > + if (s->memhp_state.is_enabled) { > + acpi_memory_hotplug_init(get_system_memory(), OBJECT(dev), > + &s->memhp_state, > + s->memhp_base); > + acpi_ged_init(get_system_memory(), dev, &s->ged_state); > + } > +} > + > +static Property acpi_ged_properties[] = { > + /* > + * Memory hotplug base address is a property of GED here, > + * because GED handles memory hotplug event and MEMORY_HOTPLUG_DEVICE > + * gets initialized when GED device is realized. > + */ > + DEFINE_PROP_UINT64("memhp-base", AcpiGedState, memhp_base, 0), > + DEFINE_PROP_BOOL("memory-hotplug-support", AcpiGedState, > + memhp_state.is_enabled, true), > + DEFINE_PROP_PTR("gsi", AcpiGedState, gsi), > + DEFINE_PROP_UINT64("ged-base", AcpiGedState, ged_base, 0), > + DEFINE_PROP_UINT32("ged-irq", AcpiGedState, ged_irq, 0), > + DEFINE_PROP_PTR("ged-events", AcpiGedState, ged_events), > + DEFINE_PROP_UINT32("ged-events-size", AcpiGedState, ged_events_size, 0), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void acpi_ged_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 = acpi_ged_properties; > + dc->realize = acpi_ged_device_realize; > + > + /* Reason: pointer properties "gsi" and "ged_events" */ > + dc->user_creatable = false; > + > + hc->plug = acpi_ged_device_plug_cb; > + > + adevc->send_event = acpi_ged_send_event; > +} > + > +static const TypeInfo acpi_ged_info = { > + .name = TYPE_ACPI_GED, > + .parent = TYPE_DEVICE, > + .instance_size = sizeof(AcpiGedState), > + .class_init = acpi_ged_class_init, > + .interfaces = (InterfaceInfo[]) { > + { TYPE_HOTPLUG_HANDLER }, > + { TYPE_ACPI_DEVICE_IF }, > + { } > + } > +}; > + > +static void acpi_ged_register_types(void) > +{ > + type_register_static(&acpi_ged_info); > +} > + > +type_init(acpi_ged_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..9c840d8 > --- /dev/null > +++ b/include/hw/acpi/generic_event_device.h > @@ -0,0 +1,121 @@ > +/* > + * > + * 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/>. > + * > + * The ACPI Generic Event Device (GED) is a hardware-reduced specific > + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, > + * including the hotplug ones. Generic Event Device allows platforms > + * to handle interrupts in ACPI ASL statements. It follows a very > + * similar approach like the _EVT method from GPIO events. All > + * interrupts are listed in _CRS and the handler is written in _EVT > + * method. Here, we use a single interrupt for the GED device, relying > + * on IO memory region to communicate the type of device affected by > + * the interrupt. This way, we can support up to 32 events with a > + * unique interrupt. > + * > + * Here is an example. > + * > + * Device (\_SB.GED) > + * { > + * Name (_HID, "ACPI0013") > + * Name (_UID, Zero) > + * Name (_CRS, ResourceTemplate () > + * { > + * Interrupt (ResourceConsumer, Edge, ActiveHigh, Exclusive, ,, ) > + * { > + * 0x00000029, > + * } > + * }) > + * OperationRegion (IREG, SystemMemory, 0x09080000, 0x04) > + * Field (IREG, DWordAcc, NoLock, WriteAsZeros) > + * { > + * ISEL, 32 > + * } > + * > + * Method (_EVT, 1, Serialized) // _EVT: Event > + * { > + * Local0 = ISEL // ISEL = IO memory region which specifies the > + * // device type. > + * If (((Local0 & irq0) == irq0)) > + * { > + * MethodEvent0() > + * } > + * ElseIf ((Local0 & irq1) == irq1) > + * { > + * MethodEvent1() > + * } > + * ... > + * } > + * } > + * > + */ > + > +#ifndef HW_ACPI_GED_H > +#define HW_ACPI_GED_H > + > +#include "hw/acpi/memory_hotplug.h" > + > +#define TYPE_ACPI_GED "acpi-ged" > +#define ACPI_GED(obj) \ > + OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED) > + > +#define ACPI_GED_IRQ_SEL_OFFSET 0x0 > +#define ACPI_GED_IRQ_SEL_LEN 0x4 > +#define ACPI_GED_IRQ_SEL_MEM 0x1 > +#define ACPI_GED_REG_LEN 0x4 > + > +#define GED_DEVICE "GED" > +#define AML_GED_IRQ_REG "IREG" > +#define AML_GED_IRQ_SEL "ISEL" > + > +typedef enum { > + GED_MEMORY_HOTPLUG = 1, > +} GedEventType; > + > +/* > + * Platforms need to specify their own GedEvent array > + * to describe what kind of events they want to support > + * through GED. > + */ > +typedef struct GedEvent { > + uint32_t selector; > + GedEventType event; > +} GedEvent; > + > +typedef struct GEDState { > + MemoryRegion io; > + uint32_t sel; > + uint32_t irq; > + qemu_irq *gsi; > + QemuMutex lock; > +} GEDState; > + > + > +typedef struct AcpiGedState { > + DeviceClass parent_obj; > + MemHotplugState memhp_state; > + hwaddr memhp_base; > + void *gsi; > + hwaddr ged_base; > + GEDState ged_state; > + uint32_t ged_irq; > + void *ged_events; > + uint32_t ged_events_size; > +} AcpiGedState; > + > +void build_ged_aml(Aml *table, const char* name, HotplugHandler *hotplug_dev, > + uint32_t ged_irq, AmlRegionSpace rs); > + > +#endif > -- > 2.7.4 > >
Hi Ard, > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: 01 May 2019 12:10 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > Cc: QEMU Developers <qemu-devel@nongnu.org>; qemu-arm > <qemu-arm@nongnu.org>; Auger Eric <eric.auger@redhat.com>; Igor > Mammedov <imammedo@redhat.com>; Peter Maydell > <peter.maydell@linaro.org>; shannon.zhaosl@gmail.com; > sameo@linux.intel.com; sebastien.boeuf@intel.com; xuwei (O) > <xuwei5@huawei.com>; Laszlo Ersek <lersek@redhat.com>; Linuxarm > <linuxarm@huawei.com> > Subject: Re: [PATCH v4 3/8] hw/acpi: Add ACPI Generic Event Device Support > > On Tue, 9 Apr 2019 at 12:31, Shameer Kolothum > <shameerali.kolothum.thodi@huawei.com> wrote: > > > > From: Samuel Ortiz <sameo@linux.intel.com> > > > > The ACPI Generic Event Device (GED) is a hardware-reduced specific > > device[ACPI v6.1 Section 5.6.9] that handles all platform events, > > including the hotplug ones.This patch generates the AML code that > > defines GEDs. > > > > Platforms need to specify their own GedEvent array to describe what > > kind of events they want to support through GED. Also this uses a > > a single interrupt for the GED device, relying on IO memory region > > to communicate the type of device affected by the interrupt. This > > way, we can support up to 32 events with a unique interrupt. > > > > This supports only memory hotplug for now. > > > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> > > Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com> > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > > Apologies if this question has been raised before, but do we really > need a separate device for this? We already handle the power button > via _AEI/_Exx on the GPIO device, and I think we should be able to add > additional events using that interface, rather than have two event > signalling methods/devices on the same platform. Right. The initial RFC was based on GPIO device[1] and later Igor commented here[2] that, " ARM boards were first to use ACPI hw-reduced profile so they picked up available back then GPIO based way to deliver hotplug event, later spec introduced Generic Event Device for that means to use with hw-reduced profile, which NEMU implemented[1], so I'd use that rather than ad-hoc GPIO mapping. I'd guess it will more compatible with various contemporary guests and we could reuse the same code for both x86/arm virt boards) " Thanks, Shameer [1]. https://patchwork.kernel.org/cover/10783589/ [2] http://patchwork.ozlabs.org/cover/1045604/ > > > > --- > > hw/acpi/Kconfig | 4 + > > hw/acpi/Makefile.objs | 1 + > > hw/acpi/generic_event_device.c | 311 > +++++++++++++++++++++++++++++++++ > > include/hw/acpi/generic_event_device.h | 121 +++++++++++++ > > 4 files changed, 437 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..856ca04 > > --- /dev/null > > +++ b/hw/acpi/generic_event_device.c > > @@ -0,0 +1,311 @@ > > +/* > > + * > > + * 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 "qapi/error.h" > > +#include "exec/address-spaces.h" > > +#include "hw/sysbus.h" > > +#include "hw/acpi/acpi.h" > > +#include "hw/acpi/generic_event_device.h" > > +#include "hw/mem/pc-dimm.h" > > + > > +static Aml *ged_event_aml(const GedEvent *event) > > +{ > > + > > + if (!event) { > > + return NULL; > > + } > > + > > + switch (event->event) { > > + case GED_MEMORY_HOTPLUG: > > + /* We run a complete memory SCAN when getting a memory > hotplug event */ > > + return aml_call0(MEMORY_DEVICES_CONTAINER "." > MEMORY_SLOT_SCAN_METHOD); > > + default: > > + break; > > + } > > + > > + return NULL; > > +} > > + > > +/* > > + * The ACPI Generic Event Device (GED) is a hardware-reduced specific > > + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, > > + * including the hotplug ones. Platforms need to specify their own > > + * GedEvent array to describe what kind of events they want to support > > + * through GED. This routine uses a single interrupt for the GED device, > > + * relying on IO memory region to communicate the type of device > > + * affected by the interrupt. This way, we can support up to 32 events > > + * with a unique interrupt. > > + */ > > +void build_ged_aml(Aml *table, const char *name, HotplugHandler > *hotplug_dev, > > + uint32_t ged_irq, AmlRegionSpace rs) > > +{ > > + AcpiGedState *s = ACPI_GED(hotplug_dev); > > + GedEvent *ged_events = s->ged_events; > > + Aml *crs = aml_resource_template(); > > + Aml *evt, *field; > > + Aml *dev = aml_device("%s", name); > > + Aml *irq_sel = aml_local(0); > > + Aml *isel = aml_name(AML_GED_IRQ_SEL); > > + uint32_t i; > > + > > + if (!s->ged_base || !ged_events || !s->ged_events_size) { > > + return; > > + } > > + > > + /* _CRS interrupt */ > > + aml_append(crs, aml_interrupt(AML_CONSUMER, AML_EDGE, > AML_ACTIVE_HIGH, > > + AML_EXCLUSIVE, &ged_irq, 1)); > > + /* > > + * For each GED event we: > > + * - Add an interrupt to the CRS section. > > + * - Add a conditional block for each event, inside a while loop. > > + * This is semantically equivalent to a switch/case implementation. > > + */ > > + evt = aml_method("_EVT", 1, AML_SERIALIZED); > > + { > > + Aml *ged_aml; > > + Aml *if_ctx; > > + > > + /* Local0 = ISEL */ > > + aml_append(evt, aml_store(isel, irq_sel)); > > + > > + /* > > + * Here we want to call a method for each supported GED event > type. > > + * The resulting ASL code looks like: > > + * > > + * Local0 = ISEL > > + * If ((Local0 & irq0) == irq0) > > + * { > > + * MethodEvent0() > > + * } > > + * > > + * If ((Local0 & irq1) == irq1) > > + * { > > + * MethodEvent1() > > + * } > > + * ... > > + */ > > + > > + for (i = 0; i < s->ged_events_size; i++) { > > + ged_aml = ged_event_aml(&ged_events[i]); > > + if (!ged_aml) { > > + continue; > > + } > > + > > + /* If ((Local1 == irq))*/ > > + if_ctx = aml_if(aml_equal(aml_and(irq_sel, > > + > aml_int(ged_events[i].selector), NULL), > > + > aml_int(ged_events[i].selector))); > > + { > > + /* AML for this specific type of event */ > > + aml_append(if_ctx, ged_aml); > > + } > > + > > + /* > > + * We append the first "if" to the "while" context. > > + * Other "if"s will be "elseif"s. > > + */ > > + aml_append(evt, if_ctx); > > + } > > + } > > + > > + aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0013"))); > > + aml_append(dev, aml_name_decl("_UID", aml_string(GED_DEVICE))); > > + aml_append(dev, aml_name_decl("_CRS", crs)); > > + > > + /* Append IO region */ > > + aml_append(dev, aml_operation_region(AML_GED_IRQ_REG, rs, > > + aml_int(s->ged_base + ACPI_GED_IRQ_SEL_OFFSET), > > + ACPI_GED_IRQ_SEL_LEN)); > > + field = aml_field(AML_GED_IRQ_REG, AML_DWORD_ACC, > AML_NOLOCK, > > + AML_WRITE_AS_ZEROS); > > + aml_append(field, aml_named_field(AML_GED_IRQ_SEL, > > + ACPI_GED_IRQ_SEL_LEN * > 8)); > > + aml_append(dev, field); > > + > > + /* Append _EVT method */ > > + aml_append(dev, evt); > > + > > + aml_append(table, dev); > > +} > > + > > +/* Memory read by the GED _EVT AML dynamic method */ > > +static uint64_t ged_read(void *opaque, hwaddr addr, unsigned size) > > +{ > > + uint64_t val = 0; > > + GEDState *ged_st = opaque; > > + > > + switch (addr) { > > + case ACPI_GED_IRQ_SEL_OFFSET: > > + /* Read the selector value and reset it */ > > + qemu_mutex_lock(&ged_st->lock); > > + val = ged_st->sel; > > + ged_st->sel = 0; > > + qemu_mutex_unlock(&ged_st->lock); > > + break; > > + default: > > + break; > > + } > > + > > + return val; > > +} > > + > > +/* Nothing is expected to be written to the GED memory region */ > > +static void ged_write(void *opaque, hwaddr addr, uint64_t data, > > + unsigned int size) > > +{ > > +} > > + > > +static const MemoryRegionOps ged_ops = { > > + .read = ged_read, > > + .write = ged_write, > > + .endianness = DEVICE_LITTLE_ENDIAN, > > + .valid = { > > + .min_access_size = 4, > > + .max_access_size = 4, > > + }, > > +}; > > + > > +static void acpi_ged_event(GEDState *ged_st, uint32_t ged_irq_sel) > > +{ > > + /* > > + * Set the GED IRQ selector to the expected device type value. This > > + * way, the ACPI method will be able to trigger the right code based > > + * on a unique IRQ. > > + */ > > + qemu_mutex_lock(&ged_st->lock); > > + ged_st->sel = ged_irq_sel; > > + qemu_mutex_unlock(&ged_st->lock); > > + > > + /* Trigger the event by sending an interrupt to the guest. */ > > + qemu_irq_pulse(ged_st->gsi[ged_st->irq]); > > +} > > + > > +static void acpi_ged_init(MemoryRegion *as, DeviceState *dev, GEDState > *ged_st) > > +{ > > + AcpiGedState *s = ACPI_GED(dev); > > + > > + assert(s->ged_base); > > + > > + ged_st->irq = s->ged_irq; > > + ged_st->gsi = s->gsi; > > + qemu_mutex_init(&ged_st->lock); > > + memory_region_init_io(&ged_st->io, OBJECT(dev), &ged_ops, ged_st, > > + "acpi-ged-event", ACPI_GED_REG_LEN); > > + memory_region_add_subregion(as, s->ged_base, &ged_st->io); > > +} > > + > > +static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev, > > + DeviceState *dev, Error **errp) > > +{ > > + AcpiGedState *s = ACPI_GED(hotplug_dev); > > + > > + if (s->memhp_state.is_enabled && > > + object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > > + acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, > > + dev, errp); > > + } else { > > + error_setg(errp, "virt: device plug request for unsupported > device" > > + " type: %s", object_get_typename(OBJECT(dev))); > > + } > > +} > > + > > +static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits > ev) > > +{ > > + AcpiGedState *s = ACPI_GED(adev); > > + uint32_t sel; > > + > > + if (ev & ACPI_MEMORY_HOTPLUG_STATUS) { > > + sel = ACPI_GED_IRQ_SEL_MEM; > > + } else { > > + /* Unknown event. Return without generating interrupt. */ > > + return; > > + } > > + > > + /* > > + * We inject the hotplug interrupt. The IRQ selector will make > > + * the difference from the ACPI table. > > + */ > > + acpi_ged_event(&s->ged_state, sel); > > +} > > + > > +static void acpi_ged_device_realize(DeviceState *dev, Error **errp) > > +{ > > + AcpiGedState *s = ACPI_GED(dev); > > + > > + if (s->memhp_state.is_enabled) { > > + acpi_memory_hotplug_init(get_system_memory(), OBJECT(dev), > > + &s->memhp_state, > > + s->memhp_base); > > + acpi_ged_init(get_system_memory(), dev, &s->ged_state); > > + } > > +} > > + > > +static Property acpi_ged_properties[] = { > > + /* > > + * Memory hotplug base address is a property of GED here, > > + * because GED handles memory hotplug event and > MEMORY_HOTPLUG_DEVICE > > + * gets initialized when GED device is realized. > > + */ > > + DEFINE_PROP_UINT64("memhp-base", AcpiGedState, memhp_base, > 0), > > + DEFINE_PROP_BOOL("memory-hotplug-support", AcpiGedState, > > + memhp_state.is_enabled, true), > > + DEFINE_PROP_PTR("gsi", AcpiGedState, gsi), > > + DEFINE_PROP_UINT64("ged-base", AcpiGedState, ged_base, 0), > > + DEFINE_PROP_UINT32("ged-irq", AcpiGedState, ged_irq, 0), > > + DEFINE_PROP_PTR("ged-events", AcpiGedState, ged_events), > > + DEFINE_PROP_UINT32("ged-events-size", AcpiGedState, > ged_events_size, 0), > > + DEFINE_PROP_END_OF_LIST(), > > +}; > > + > > +static void acpi_ged_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 = acpi_ged_properties; > > + dc->realize = acpi_ged_device_realize; > > + > > + /* Reason: pointer properties "gsi" and "ged_events" */ > > + dc->user_creatable = false; > > + > > + hc->plug = acpi_ged_device_plug_cb; > > + > > + adevc->send_event = acpi_ged_send_event; > > +} > > + > > +static const TypeInfo acpi_ged_info = { > > + .name = TYPE_ACPI_GED, > > + .parent = TYPE_DEVICE, > > + .instance_size = sizeof(AcpiGedState), > > + .class_init = acpi_ged_class_init, > > + .interfaces = (InterfaceInfo[]) { > > + { TYPE_HOTPLUG_HANDLER }, > > + { TYPE_ACPI_DEVICE_IF }, > > + { } > > + } > > +}; > > + > > +static void acpi_ged_register_types(void) > > +{ > > + type_register_static(&acpi_ged_info); > > +} > > + > > +type_init(acpi_ged_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..9c840d8 > > --- /dev/null > > +++ b/include/hw/acpi/generic_event_device.h > > @@ -0,0 +1,121 @@ > > +/* > > + * > > + * 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/>. > > + * > > + * The ACPI Generic Event Device (GED) is a hardware-reduced specific > > + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, > > + * including the hotplug ones. Generic Event Device allows platforms > > + * to handle interrupts in ACPI ASL statements. It follows a very > > + * similar approach like the _EVT method from GPIO events. All > > + * interrupts are listed in _CRS and the handler is written in _EVT > > + * method. Here, we use a single interrupt for the GED device, relying > > + * on IO memory region to communicate the type of device affected by > > + * the interrupt. This way, we can support up to 32 events with a > > + * unique interrupt. > > + * > > + * Here is an example. > > + * > > + * Device (\_SB.GED) > > + * { > > + * Name (_HID, "ACPI0013") > > + * Name (_UID, Zero) > > + * Name (_CRS, ResourceTemplate () > > + * { > > + * Interrupt (ResourceConsumer, Edge, ActiveHigh, Exclusive, ,, ) > > + * { > > + * 0x00000029, > > + * } > > + * }) > > + * OperationRegion (IREG, SystemMemory, 0x09080000, 0x04) > > + * Field (IREG, DWordAcc, NoLock, WriteAsZeros) > > + * { > > + * ISEL, 32 > > + * } > > + * > > + * Method (_EVT, 1, Serialized) // _EVT: Event > > + * { > > + * Local0 = ISEL // ISEL = IO memory region which specifies the > > + * // device type. > > + * If (((Local0 & irq0) == irq0)) > > + * { > > + * MethodEvent0() > > + * } > > + * ElseIf ((Local0 & irq1) == irq1) > > + * { > > + * MethodEvent1() > > + * } > > + * ... > > + * } > > + * } > > + * > > + */ > > + > > +#ifndef HW_ACPI_GED_H > > +#define HW_ACPI_GED_H > > + > > +#include "hw/acpi/memory_hotplug.h" > > + > > +#define TYPE_ACPI_GED "acpi-ged" > > +#define ACPI_GED(obj) \ > > + OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED) > > + > > +#define ACPI_GED_IRQ_SEL_OFFSET 0x0 > > +#define ACPI_GED_IRQ_SEL_LEN 0x4 > > +#define ACPI_GED_IRQ_SEL_MEM 0x1 > > +#define ACPI_GED_REG_LEN 0x4 > > + > > +#define GED_DEVICE "GED" > > +#define AML_GED_IRQ_REG "IREG" > > +#define AML_GED_IRQ_SEL "ISEL" > > + > > +typedef enum { > > + GED_MEMORY_HOTPLUG = 1, > > +} GedEventType; > > + > > +/* > > + * Platforms need to specify their own GedEvent array > > + * to describe what kind of events they want to support > > + * through GED. > > + */ > > +typedef struct GedEvent { > > + uint32_t selector; > > + GedEventType event; > > +} GedEvent; > > + > > +typedef struct GEDState { > > + MemoryRegion io; > > + uint32_t sel; > > + uint32_t irq; > > + qemu_irq *gsi; > > + QemuMutex lock; > > +} GEDState; > > + > > + > > +typedef struct AcpiGedState { > > + DeviceClass parent_obj; > > + MemHotplugState memhp_state; > > + hwaddr memhp_base; > > + void *gsi; > > + hwaddr ged_base; > > + GEDState ged_state; > > + uint32_t ged_irq; > > + void *ged_events; > > + uint32_t ged_events_size; > > +} AcpiGedState; > > + > > +void build_ged_aml(Aml *table, const char* name, HotplugHandler > *hotplug_dev, > > + uint32_t ged_irq, AmlRegionSpace rs); > > + > > +#endif > > -- > > 2.7.4 > > > >
Hi Shameer, On 5/1/19 12:40 PM, Shameerali Kolothum Thodi wrote: > Hi Eric, > >> -----Original Message----- >> From: Auger Eric [mailto:eric.auger@redhat.com] >> Sent: 30 April 2019 16:50 >> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; >> qemu-devel@nongnu.org; qemu-arm@nongnu.org; imammedo@redhat.com >> Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com; >> sameo@linux.intel.com; sebastien.boeuf@intel.com; xuwei (O) >> <xuwei5@huawei.com>; lersek@redhat.com; ard.biesheuvel@linaro.org; >> Linuxarm <linuxarm@huawei.com> >> Subject: Re: [PATCH v4 3/8] hw/acpi: Add ACPI Generic Event Device Support >> >> Hi Shameer, >> >> On 4/9/19 12:29 PM, Shameer Kolothum wrote: >>> From: Samuel Ortiz <sameo@linux.intel.com> >>> >>> The ACPI Generic Event Device (GED) is a hardware-reduced specific >>> device[ACPI v6.1 Section 5.6.9] that handles all platform events, >>> including the hotplug ones.This patch generates the AML code that >>> defines GEDs. >>> >>> Platforms need to specify their own GedEvent array to describe what >>> kind of events they want to support through GED. Also this uses a >>> a single interrupt for the GED device, relying on IO memory region >>> to communicate the type of device affected by the interrupt. This >>> way, we can support up to 32 events with a unique interrupt. >>> >>> This supports only memory hotplug for now. >>> >>> Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> >>> Signed-off-by: Sebastien Boeuf <sebastien.boeuf@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 | 311 >> +++++++++++++++++++++++++++++++++ >>> include/hw/acpi/generic_event_device.h | 121 +++++++++++++ >>> 4 files changed, 437 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..856ca04 >>> --- /dev/null >>> +++ b/hw/acpi/generic_event_device.c >>> @@ -0,0 +1,311 @@ >>> +/* >>> + * >>> + * 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. >> I am not sure we need below statements: see hw/misc/armsse-mhu.c for a >> recent added file. > > Ok. I will get rid of this. > >>> + * >>> + * 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 "qapi/error.h" >>> +#include "exec/address-spaces.h" >>> +#include "hw/sysbus.h" >>> +#include "hw/acpi/acpi.h" >>> +#include "hw/acpi/generic_event_device.h" >>> +#include "hw/mem/pc-dimm.h" >>> + >>> +static Aml *ged_event_aml(const GedEvent *event) >>> +{ >>> + >>> + if (!event) { >>> + return NULL; >>> + } >>> + >>> + switch (event->event) { >>> + case GED_MEMORY_HOTPLUG: >>> + /* We run a complete memory SCAN when getting a memory >> hotplug event */ >>> + return aml_call0(MEMORY_DEVICES_CONTAINER "." >> MEMORY_SLOT_SCAN_METHOD); >>> + default: >>> + break; >>> + } >>> + >>> + return NULL; >>> +} >>> + >>> +/* >>> + * The ACPI Generic Event Device (GED) is a hardware-reduced specific >>> + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, >>> + * including the hotplug ones. Platforms need to specify their own >>> + * GedEvent array to describe what kind of events they want to support >>> + * through GED. This routine uses a single interrupt for the GED device, >>> + * relying on IO memory region to communicate the type of device >>> + * affected by the interrupt. This way, we can support up to 32 events >>> + * with a unique interrupt. >>> + */ >>> +void build_ged_aml(Aml *table, const char *name, HotplugHandler >> *hotplug_dev, >>> + uint32_t ged_irq, AmlRegionSpace rs) >>> +{ >>> + AcpiGedState *s = ACPI_GED(hotplug_dev); >>> + GedEvent *ged_events = s->ged_events; >>> + Aml *crs = aml_resource_template(); >>> + Aml *evt, *field; >>> + Aml *dev = aml_device("%s", name); >>> + Aml *irq_sel = aml_local(0); >>> + Aml *isel = aml_name(AML_GED_IRQ_SEL); >>> + uint32_t i; >>> + >>> + if (!s->ged_base || !ged_events || !s->ged_events_size) { >>> + return; >>> + } >>> + >>> + /* _CRS interrupt */ >>> + aml_append(crs, aml_interrupt(AML_CONSUMER, AML_EDGE, >> AML_ACTIVE_HIGH, >>> + AML_EXCLUSIVE, &ged_irq, 1)); >>> + /* >>> + * For each GED event we: >>> + * - Add an interrupt to the CRS section. >>> + * - Add a conditional block for each event, inside a while loop. >>> + * This is semantically equivalent to a switch/case implementation. >>> + */ >>> + evt = aml_method("_EVT", 1, AML_SERIALIZED); >>> + { >>> + Aml *ged_aml; >>> + Aml *if_ctx; >>> + >>> + /* Local0 = ISEL */ >>> + aml_append(evt, aml_store(isel, irq_sel)); >>> + >>> + /* >>> + * Here we want to call a method for each supported GED event >> type. >>> + * The resulting ASL code looks like: >>> + * >>> + * Local0 = ISEL >>> + * If ((Local0 & irq0) == irq0) >>> + * { >>> + * MethodEvent0() >>> + * } >>> + * >>> + * If ((Local0 & irq1) == irq1) >>> + * { >>> + * MethodEvent1() >>> + * } >>> + * ... >>> + */ >>> + >>> + for (i = 0; i < s->ged_events_size; i++) { >>> + ged_aml = ged_event_aml(&ged_events[i]); >>> + if (!ged_aml) { >>> + continue; >>> + } >>> + >>> + /* If ((Local1 == irq))*/ >>> + if_ctx = aml_if(aml_equal(aml_and(irq_sel, >>> + >> aml_int(ged_events[i].selector), NULL), >>> + >> aml_int(ged_events[i].selector))); >>> + { >>> + /* AML for this specific type of event */ >>> + aml_append(if_ctx, ged_aml); >>> + } >>> + >>> + /* >>> + * We append the first "if" to the "while" context. >>> + * Other "if"s will be "elseif"s. >> Are we sure about that: >> in hw/acpi/nvdimm.c nvdimm_build_common_dsm() I can see an aml_else() is >> added inbetween aml_if's? > > Right. I think the comment here is misleading. This will be as mentioned in the > comment block above, > > If ((Local0 & irq0) == irq0) > { > MethodEvent0() > } > > If ((Local0 & irq1) == irq1) > { > MethodEvent1() > } > > And this should do the job. Do we really need ElseIf block here? I don't think so, as long as handling several set bits is allowed. So let's remove the comment then. Thanks Eric > >>> + */ >>> + aml_append(evt, if_ctx); >>> + } >>> + } >>> + >>> + aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0013"))); >>> + aml_append(dev, aml_name_decl("_UID", aml_string(GED_DEVICE))); >>> + aml_append(dev, aml_name_decl("_CRS", crs)); >>> + >>> + /* Append IO region */ >>> + aml_append(dev, aml_operation_region(AML_GED_IRQ_REG, rs, >>> + aml_int(s->ged_base + ACPI_GED_IRQ_SEL_OFFSET), >>> + ACPI_GED_IRQ_SEL_LEN)); >>> + field = aml_field(AML_GED_IRQ_REG, AML_DWORD_ACC, >> AML_NOLOCK, >>> + AML_WRITE_AS_ZEROS); >>> + aml_append(field, aml_named_field(AML_GED_IRQ_SEL, >>> + ACPI_GED_IRQ_SEL_LEN * >> 8)); >> nit s/8/BITS_PER_BYTE as done in nvdimm.c. > > Ok. > >>> + aml_append(dev, field); >>> + >>> + /* Append _EVT method */ >>> + aml_append(dev, evt); >>> + >>> + aml_append(table, dev); >>> +} >>> + >>> +/* Memory read by the GED _EVT AML dynamic method */ >>> +static uint64_t ged_read(void *opaque, hwaddr addr, unsigned size) >>> +{ >>> + uint64_t val = 0; >>> + GEDState *ged_st = opaque; >>> + >>> + switch (addr) { >>> + case ACPI_GED_IRQ_SEL_OFFSET: >>> + /* Read the selector value and reset it */ >>> + qemu_mutex_lock(&ged_st->lock); >>> + val = ged_st->sel; >>> + ged_st->sel = 0; >>> + qemu_mutex_unlock(&ged_st->lock); >>> + break; >>> + default: >>> + break; >>> + } >>> + >>> + return val; >>> +} >>> + >>> +/* Nothing is expected to be written to the GED memory region */ >>> +static void ged_write(void *opaque, hwaddr addr, uint64_t data, >>> + unsigned int size) >>> +{ >>> +} >>> + >>> +static const MemoryRegionOps ged_ops = { >>> + .read = ged_read, >>> + .write = ged_write, >>> + .endianness = DEVICE_LITTLE_ENDIAN, >>> + .valid = { >>> + .min_access_size = 4, >>> + .max_access_size = 4, >>> + }, >>> +}; >>> + >>> +static void acpi_ged_event(GEDState *ged_st, uint32_t ged_irq_sel) >>> +{ >>> + /* >> I would personally inline that code in void acpi_ged_send_event() > > Ok. > >>> + * Set the GED IRQ selector to the expected device type value. This >>> + * way, the ACPI method will be able to trigger the right code based >>> + * on a unique IRQ. >>> + */ >>> + qemu_mutex_lock(&ged_st->lock); >>> + ged_st->sel = ged_irq_sel; >>> + qemu_mutex_unlock(&ged_st->lock); >>> + >>> + /* Trigger the event by sending an interrupt to the guest. */ >>> + qemu_irq_pulse(ged_st->gsi[ged_st->irq]); >> I don't really understand why we need this array. Doesn't The GED use a >> single IRQ. So why can't you store the precise qemu_irq pointer directly >> through the propery (DEFINE_PROP_PTR("gsi", AcpiGedState, gsi))? > > Yes. You have raised this point previously as well. Sorry, I missed it when I > reworked the GED type from SYS_BUS_DEVICE to TYPE_DEVICE. I think > we can get rid of this array and the GED/gsi routing done in virt.c. > > >>> +} >>> + >>> +static void acpi_ged_init(MemoryRegion *as, DeviceState *dev, GEDState >> *ged_st) >>> +{ >>> + AcpiGedState *s = ACPI_GED(dev); >>> + >>> + assert(s->ged_base); >>> + >>> + ged_st->irq = s->ged_irq; >>> + ged_st->gsi = s->gsi; >>> + qemu_mutex_init(&ged_st->lock); >>> + memory_region_init_io(&ged_st->io, OBJECT(dev), &ged_ops, ged_st, >>> + "acpi-ged-event", ACPI_GED_REG_LEN); >>> + memory_region_add_subregion(as, s->ged_base, &ged_st->io); >>> +} >>> + >>> +static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev, >>> + DeviceState *dev, Error **errp) >>> +{ >>> + AcpiGedState *s = ACPI_GED(hotplug_dev); >>> + >>> + if (s->memhp_state.is_enabled && >>> + object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { >>> + acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, >>> + dev, errp); >>> + } else { >>> + error_setg(errp, "virt: device plug request for unsupported >> device" >>> + " type: %s", object_get_typename(OBJECT(dev))); >>> + } >>> +} >>> + >>> +static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits >> ev) >>> +{ >>> + AcpiGedState *s = ACPI_GED(adev); >>> + uint32_t sel; >>> + >>> + if (ev & ACPI_MEMORY_HOTPLUG_STATUS) { >>> + sel = ACPI_GED_IRQ_SEL_MEM; >>> + } else { >>> + /* Unknown event. Return without generating interrupt. */ >>> + return; >>> + } >>> + >>> + /* >>> + * We inject the hotplug interrupt. The IRQ selector will make >>> + * the difference from the ACPI table. >>> + */ >>> + acpi_ged_event(&s->ged_state, sel); >>> +} >>> + >>> +static void acpi_ged_device_realize(DeviceState *dev, Error **errp) >>> +{ >>> + AcpiGedState *s = ACPI_GED(dev); >>> + >>> + if (s->memhp_state.is_enabled) { >>> + acpi_memory_hotplug_init(get_system_memory(), OBJECT(dev), >>> + &s->memhp_state, >>> + s->memhp_base); >>> + acpi_ged_init(get_system_memory(), dev, &s->ged_state); >>> + } >>> +} >>> + >>> +static Property acpi_ged_properties[] = { >>> + /* >>> + * Memory hotplug base address is a property of GED here, >>> + * because GED handles memory hotplug event and >> MEMORY_HOTPLUG_DEVICE >>> + * gets initialized when GED device is realized. >>> + */ >>> + DEFINE_PROP_UINT64("memhp-base", AcpiGedState, memhp_base, >> 0), >>> + DEFINE_PROP_BOOL("memory-hotplug-support", AcpiGedState, >>> + memhp_state.is_enabled, true), >>> + DEFINE_PROP_PTR("gsi", AcpiGedState, gsi), >>> + DEFINE_PROP_UINT64("ged-base", AcpiGedState, ged_base, 0), >>> + DEFINE_PROP_UINT32("ged-irq", AcpiGedState, ged_irq, 0), >>> + DEFINE_PROP_PTR("ged-events", AcpiGedState, ged_events), >>> + DEFINE_PROP_UINT32("ged-events-size", AcpiGedState, >> ged_events_size, 0), >>> + DEFINE_PROP_END_OF_LIST(), >>> +}; >>> + >>> +static void acpi_ged_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 = acpi_ged_properties; >>> + dc->realize = acpi_ged_device_realize; >>> + >>> + /* Reason: pointer properties "gsi" and "ged_events" */ >>> + dc->user_creatable = false; >>> + >>> + hc->plug = acpi_ged_device_plug_cb; >>> + >>> + adevc->send_event = acpi_ged_send_event; >>> +} >>> + >>> +static const TypeInfo acpi_ged_info = { >>> + .name = TYPE_ACPI_GED, >>> + .parent = TYPE_DEVICE, >>> + .instance_size = sizeof(AcpiGedState), >>> + .class_init = acpi_ged_class_init, >>> + .interfaces = (InterfaceInfo[]) { >>> + { TYPE_HOTPLUG_HANDLER }, >>> + { TYPE_ACPI_DEVICE_IF }, >>> + { } >>> + } >>> +}; >>> + >>> +static void acpi_ged_register_types(void) >>> +{ >>> + type_register_static(&acpi_ged_info); >>> +} >>> + >>> +type_init(acpi_ged_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..9c840d8 >>> --- /dev/null >>> +++ b/include/hw/acpi/generic_event_device.h >>> @@ -0,0 +1,121 @@ >>> +/* >>> + * >>> + * 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/>. >>> + * >>> + * The ACPI Generic Event Device (GED) is a hardware-reduced specific >>> + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, >>> + * including the hotplug ones. Generic Event Device allows platforms >>> + * to handle interrupts in ACPI ASL statements. It follows a very >>> + * similar approach like the _EVT method from GPIO events. All >>> + * interrupts are listed in _CRS and the handler is written in _EVT> + * >> method. Here, we use a single interrupt for the GED device, relying >>> + * on IO memory region to communicate the type of device affected by >>> + * the interrupt. This way, we can support up to 32 events with a >>> + * unique interrupt. >>> + * >>> + * Here is an example. >>> + * >>> + * Device (\_SB.GED) >>> + * { >>> + * Name (_HID, "ACPI0013") >>> + * Name (_UID, Zero) >>> + * Name (_CRS, ResourceTemplate () >>> + * { >>> + * Interrupt (ResourceConsumer, Edge, ActiveHigh, Exclusive, ,, ) >>> + * { >>> + * 0x00000029, >>> + * } >>> + * }) >>> + * OperationRegion (IREG, SystemMemory, 0x09080000, 0x04) >>> + * Field (IREG, DWordAcc, NoLock, WriteAsZeros) >>> + * { >>> + * ISEL, 32 >>> + * } >>> + * >>> + * Method (_EVT, 1, Serialized) // _EVT: Event >>> + * { >>> + * Local0 = ISEL // ISEL = IO memory region which specifies the >>> + * // device type. >>> + * If (((Local0 & irq0) == irq0)) >>> + * { >>> + * MethodEvent0() >>> + * } >>> + * ElseIf ((Local0 & irq1) == irq1) >>> + * { >>> + * MethodEvent1() >>> + * } >>> + * ... >>> + * } >>> + * } >>> + * >>> + */ >>> + >>> +#ifndef HW_ACPI_GED_H >>> +#define HW_ACPI_GED_H >>> + >>> +#include "hw/acpi/memory_hotplug.h" >>> + >>> +#define TYPE_ACPI_GED "acpi-ged" >>> +#define ACPI_GED(obj) \ >>> + OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED) >>> + >>> +#define ACPI_GED_IRQ_SEL_OFFSET 0x0 >>> +#define ACPI_GED_IRQ_SEL_LEN 0x4 >>> +#define ACPI_GED_IRQ_SEL_MEM 0x1 >>> +#define ACPI_GED_REG_LEN 0x4 >>> + >>> +#define GED_DEVICE "GED" >>> +#define AML_GED_IRQ_REG "IREG" >>> +#define AML_GED_IRQ_SEL "ISEL" >>> + >>> +typedef enum { >>> + GED_MEMORY_HOTPLUG = 1, >>> +} GedEventType; >>> + >>> +/* >>> + * Platforms need to specify their own GedEvent array >>> + * to describe what kind of events they want to support >>> + * through GED. >>> + */ >>> +typedef struct GedEvent { >>> + uint32_t selector; >>> + GedEventType event; >>> +} GedEvent; >>> + >>> +typedef struct GEDState { >>> + MemoryRegion io; >>> + uint32_t sel; >>> + uint32_t irq; >>> + qemu_irq *gsi; >> so I am not sure why with need gsi + irq. >>> + QemuMutex lock; >>> +} GEDState; >>> + >>> + >>> +typedef struct AcpiGedState { >>> + DeviceClass parent_obj; >>> + MemHotplugState memhp_state; >>> + hwaddr memhp_base; >>> + void *gsi; >>> + hwaddr ged_base; >>> + GEDState ged_state; >>> + uint32_t ged_irq; >>> + void *ged_events; >>> + uint32_t ged_events_size; >>> +} AcpiGedState; >>> + >>> +void build_ged_aml(Aml *table, const char* name, HotplugHandler >> *hotplug_dev, >>> + uint32_t ged_irq, AmlRegionSpace rs); >>> + >>> +#endif >>> >> Besides, this looks good to me. >> > > Thanks, > Shameer >
On Wed, 1 May 2019 at 13:25, Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote: > > Hi Ard, > > > -----Original Message----- > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > Sent: 01 May 2019 12:10 > > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > > Cc: QEMU Developers <qemu-devel@nongnu.org>; qemu-arm > > <qemu-arm@nongnu.org>; Auger Eric <eric.auger@redhat.com>; Igor > > Mammedov <imammedo@redhat.com>; Peter Maydell > > <peter.maydell@linaro.org>; shannon.zhaosl@gmail.com; > > sameo@linux.intel.com; sebastien.boeuf@intel.com; xuwei (O) > > <xuwei5@huawei.com>; Laszlo Ersek <lersek@redhat.com>; Linuxarm > > <linuxarm@huawei.com> > > Subject: Re: [PATCH v4 3/8] hw/acpi: Add ACPI Generic Event Device Support > > > > On Tue, 9 Apr 2019 at 12:31, Shameer Kolothum > > <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > > From: Samuel Ortiz <sameo@linux.intel.com> > > > > > > The ACPI Generic Event Device (GED) is a hardware-reduced specific > > > device[ACPI v6.1 Section 5.6.9] that handles all platform events, > > > including the hotplug ones.This patch generates the AML code that > > > defines GEDs. > > > > > > Platforms need to specify their own GedEvent array to describe what > > > kind of events they want to support through GED. Also this uses a > > > a single interrupt for the GED device, relying on IO memory region > > > to communicate the type of device affected by the interrupt. This > > > way, we can support up to 32 events with a unique interrupt. > > > > > > This supports only memory hotplug for now. > > > > > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> > > > Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com> > > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > > > > Apologies if this question has been raised before, but do we really > > need a separate device for this? We already handle the power button > > via _AEI/_Exx on the GPIO device, and I think we should be able to add > > additional events using that interface, rather than have two event > > signalling methods/devices on the same platform. > > Right. The initial RFC was based on GPIO device[1] and later Igor commented > here[2] that, > > " ARM boards were first to use ACPI hw-reduced profile so they picked up > available back then GPIO based way to deliver hotplug event, later spec > introduced Generic Event Device for that means to use with hw-reduced > profile, which NEMU implemented[1], so I'd use that rather than ad-hoc > GPIO mapping. I'd guess it will more compatible with various contemporary > guests and we could reuse the same code for both x86/arm virt boards) " > On mach-virt, we already use the GPIO controller for the ACPI event involving the power button, so by aligning with virt-x86, we end up in the opposite situation for mach-virt. The problem with the GED device is that it only supports GSI interrupts, while the GPIO device supports arbitrary interrupts (such as GPIO signalled ones). This means that mach-virt will be stuck with having two different methods of signalling ACPI events, unless we rewire the power button not to use a GPIO interrupt but use a GSI directly. In general, I think the ACPI event delivery mechanism doesn't really have to be aligned: the ACPI event is ultimately converted into a AML notification to the right device, and how we end up there can remain platform specific.
On Thu, 2 May 2019 09:22:35 +0200 Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On Wed, 1 May 2019 at 13:25, Shameerali Kolothum Thodi > <shameerali.kolothum.thodi@huawei.com> wrote: > > > > Hi Ard, > > > > > -----Original Message----- > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > > Sent: 01 May 2019 12:10 > > > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > > > Cc: QEMU Developers <qemu-devel@nongnu.org>; qemu-arm > > > <qemu-arm@nongnu.org>; Auger Eric <eric.auger@redhat.com>; Igor > > > Mammedov <imammedo@redhat.com>; Peter Maydell > > > <peter.maydell@linaro.org>; shannon.zhaosl@gmail.com; > > > sameo@linux.intel.com; sebastien.boeuf@intel.com; xuwei (O) > > > <xuwei5@huawei.com>; Laszlo Ersek <lersek@redhat.com>; Linuxarm > > > <linuxarm@huawei.com> > > > Subject: Re: [PATCH v4 3/8] hw/acpi: Add ACPI Generic Event Device Support > > > > > > On Tue, 9 Apr 2019 at 12:31, Shameer Kolothum > > > <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > > > > From: Samuel Ortiz <sameo@linux.intel.com> > > > > > > > > The ACPI Generic Event Device (GED) is a hardware-reduced specific > > > > device[ACPI v6.1 Section 5.6.9] that handles all platform events, > > > > including the hotplug ones.This patch generates the AML code that > > > > defines GEDs. > > > > > > > > Platforms need to specify their own GedEvent array to describe what > > > > kind of events they want to support through GED. Also this uses a > > > > a single interrupt for the GED device, relying on IO memory region > > > > to communicate the type of device affected by the interrupt. This > > > > way, we can support up to 32 events with a unique interrupt. > > > > > > > > This supports only memory hotplug for now. > > > > > > > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> > > > > Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com> > > > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > > > > > > Apologies if this question has been raised before, but do we really > > > need a separate device for this? We already handle the power button > > > via _AEI/_Exx on the GPIO device, and I think we should be able to add > > > additional events using that interface, rather than have two event > > > signalling methods/devices on the same platform. > > > > Right. The initial RFC was based on GPIO device[1] and later Igor commented > > here[2] that, > > > > " ARM boards were first to use ACPI hw-reduced profile so they picked up > > available back then GPIO based way to deliver hotplug event, later spec > > introduced Generic Event Device for that means to use with hw-reduced > > profile, which NEMU implemented[1], so I'd use that rather than ad-hoc > > GPIO mapping. I'd guess it will more compatible with various contemporary > > guests and we could reuse the same code for both x86/arm virt boards) " > > > > On mach-virt, we already use the GPIO controller for the ACPI event > involving the power button, so by aligning with virt-x86, we end up in > the opposite situation for mach-virt. > > The problem with the GED device is that it only supports GSI > interrupts, while the GPIO device supports arbitrary interrupts (such > as GPIO signalled ones). This means that mach-virt will be stuck with > having two different methods of signalling ACPI events, unless we > rewire the power button not to use a GPIO interrupt but use a GSI > directly. we can rewire power button then. > In general, I think the ACPI event delivery mechanism doesn't really > have to be aligned: the ACPI event is ultimately converted into a AML > notification to the right device, and how we end up there can remain > platform specific. Reasoning for using GED is to reduce code duplication with x86 and not creating zoo of different approached (if it could be avoided).
On Tue, 9 Apr 2019 11:29:30 +0100 Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote: > From: Samuel Ortiz <sameo@linux.intel.com> > > The ACPI Generic Event Device (GED) is a hardware-reduced specific > device[ACPI v6.1 Section 5.6.9] that handles all platform events, > including the hotplug ones.This patch generates the AML code that > defines GEDs. > > Platforms need to specify their own GedEvent array to describe what > kind of events they want to support through GED. Also this uses a > a single interrupt for the GED device, relying on IO memory region > to communicate the type of device affected by the interrupt. This > way, we can support up to 32 events with a unique interrupt. > > This supports only memory hotplug for now. > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> > Signed-off-by: Sebastien Boeuf <sebastien.boeuf@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 | 311 +++++++++++++++++++++++++++++++++ > include/hw/acpi/generic_event_device.h | 121 +++++++++++++ > 4 files changed, 437 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..856ca04 > --- /dev/null > +++ b/hw/acpi/generic_event_device.c > @@ -0,0 +1,311 @@ > +/* > + * > + * 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 "qapi/error.h" > +#include "exec/address-spaces.h" > +#include "hw/sysbus.h" > +#include "hw/acpi/acpi.h" > +#include "hw/acpi/generic_event_device.h" > +#include "hw/mem/pc-dimm.h" > + > +static Aml *ged_event_aml(const GedEvent *event) > +{ > + > + if (!event) { In general, I prefer to check condition for calling something before doing call. This way one can see in caller why and what is called, which is more clear. > + return NULL; > + } > + > + switch (event->event) { > + case GED_MEMORY_HOTPLUG: > + /* We run a complete memory SCAN when getting a memory hotplug event */ > + return aml_call0(MEMORY_DEVICES_CONTAINER "." MEMORY_SLOT_SCAN_METHOD); > + default: > + break; > + } > + > + return NULL; > +} > + > +/* > + * The ACPI Generic Event Device (GED) is a hardware-reduced specific > + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, > + * including the hotplug ones. Platforms need to specify their own > + * GedEvent array to describe what kind of events they want to support > + * through GED. This routine uses a single interrupt for the GED device, > + * relying on IO memory region to communicate the type of device > + * affected by the interrupt. This way, we can support up to 32 events > + * with a unique interrupt. > + */ > +void build_ged_aml(Aml *table, const char *name, HotplugHandler *hotplug_dev, > + uint32_t ged_irq, AmlRegionSpace rs) > +{ > + AcpiGedState *s = ACPI_GED(hotplug_dev); > + GedEvent *ged_events = s->ged_events; > + Aml *crs = aml_resource_template(); > + Aml *evt, *field; > + Aml *dev = aml_device("%s", name); > + Aml *irq_sel = aml_local(0); > + Aml *isel = aml_name(AML_GED_IRQ_SEL); > + uint32_t i; > + > + if (!s->ged_base || !ged_events || !s->ged_events_size) { I'd move it up to the caller, it would be obvious right there when function should be called. > + return; > + } > + > + /* _CRS interrupt */ > + aml_append(crs, aml_interrupt(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH, > + AML_EXCLUSIVE, &ged_irq, 1)); > + /* > + * For each GED event we: > + * - Add an interrupt to the CRS section. > + * - Add a conditional block for each event, inside a while loop. > + * This is semantically equivalent to a switch/case implementation. > + */ > + evt = aml_method("_EVT", 1, AML_SERIALIZED); > + { > + Aml *ged_aml; > + Aml *if_ctx; > + > + /* Local0 = ISEL */ > + aml_append(evt, aml_store(isel, irq_sel)); > + > + /* > + * Here we want to call a method for each supported GED event type. > + * The resulting ASL code looks like: > + * > + * Local0 = ISEL > + * If ((Local0 & irq0) == irq0) > + * { > + * MethodEvent0() > + * } > + * > + * If ((Local0 & irq1) == irq1) > + * { > + * MethodEvent1() > + * } > + * ... > + */ Well, I'm confused. do we actually use multiple IRQs or we use only one + MMIO for event type? > + for (i = 0; i < s->ged_events_size; i++) { > + ged_aml = ged_event_aml(&ged_events[i]); > + if (!ged_aml) { > + continue; > + } I'd get rid of ged_event_aml replace it with more 'switch': for (i,...) if_ctx = aml_if(...) switch (event) case GED_MEMORY_HOTPLUG: aml_append(if_ctx, aml_call0(MEMORY_DEVICES_CONTAINER "." MEMORY_SLOT_SCAN_METHOD)) break default: about(); // make sure that a newly added events have a handler > + > + /* If ((Local1 == irq))*/ > + if_ctx = aml_if(aml_equal(aml_and(irq_sel, > + aml_int(ged_events[i].selector), NULL), > + aml_int(ged_events[i].selector))); > + { > + /* AML for this specific type of event */ > + aml_append(if_ctx), ged_aml); > + } > + > + /* > + * We append the first "if" to the "while" context. > + * Other "if"s will be "elseif"s. > + */ > + aml_append(evt, if_ctx); > + } > + } > + > + aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0013"))); > + aml_append(dev, aml_name_decl("_UID", aml_string(GED_DEVICE))); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + > + /* Append IO region */ > + aml_append(dev, aml_operation_region(AML_GED_IRQ_REG, rs, > + aml_int(s->ged_base + ACPI_GED_IRQ_SEL_OFFSET), > + ACPI_GED_IRQ_SEL_LEN)); > + field = aml_field(AML_GED_IRQ_REG, AML_DWORD_ACC, AML_NOLOCK, > + AML_WRITE_AS_ZEROS); > + aml_append(field, aml_named_field(AML_GED_IRQ_SEL, > + ACPI_GED_IRQ_SEL_LEN * 8)); > + aml_append(dev, field); I'd move it up above EVT() method, so it would be clear from the begging for what device we are building AML > + /* Append _EVT method */ > + aml_append(dev, evt); > + > + aml_append(table, dev); > +} > + > +/* Memory read by the GED _EVT AML dynamic method */ > +static uint64_t ged_read(void *opaque, hwaddr addr, unsigned size) > +{ > + uint64_t val = 0; > + GEDState *ged_st = opaque; > + > + switch (addr) { > + case ACPI_GED_IRQ_SEL_OFFSET: > + /* Read the selector value and reset it */ > + qemu_mutex_lock(&ged_st->lock); > + val = ged_st->sel; > + ged_st->sel = 0; > + qemu_mutex_unlock(&ged_st->lock); > + break; > + default: > + break; > + } > + > + return val; > +} > + > +/* Nothing is expected to be written to the GED memory region */ > +static void ged_write(void *opaque, hwaddr addr, uint64_t data, > + unsigned int size) > +{ > +} > + > +static const MemoryRegionOps ged_ops = { > + .read = ged_read, > + .write = ged_write, > + .endianness = DEVICE_LITTLE_ENDIAN, > + .valid = { > + .min_access_size = 4, > + .max_access_size = 4, > + }, > +}; > + > +static void acpi_ged_event(GEDState *ged_st, uint32_t ged_irq_sel) > +{ > + /* > + * Set the GED IRQ selector to the expected device type value. This > + * way, the ACPI method will be able to trigger the right code based > + * on a unique IRQ. > + */ > + qemu_mutex_lock(&ged_st->lock); > + ged_st->sel = ged_irq_sel; what if there are 2 events with different sel value and 2nd event happens before guesr read the first one? > + qemu_mutex_unlock(&ged_st->lock); > + > + /* Trigger the event by sending an interrupt to the guest. */ > + qemu_irq_pulse(ged_st->gsi[ged_st->irq]); > +} > + > +static void acpi_ged_init(MemoryRegion *as, DeviceState *dev, GEDState *ged_st) > +{ > + AcpiGedState *s = ACPI_GED(dev); > + > + assert(s->ged_base); > + > + ged_st->irq = s->ged_irq; > + ged_st->gsi = s->gsi; > + qemu_mutex_init(&ged_st->lock); > + memory_region_init_io(&ged_st->io, OBJECT(dev), &ged_ops, ged_st, > + "acpi-ged-event", ACPI_GED_REG_LEN); > + memory_region_add_subregion(as, s->ged_base, &ged_st->io); > +} > + > +static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + AcpiGedState *s = ACPI_GED(hotplug_dev); > + > + if (s->memhp_state.is_enabled && > + object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > + acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, > + dev, errp); > + } else { > + error_setg(errp, "virt: device plug request for unsupported device" > + " type: %s", object_get_typename(OBJECT(dev))); > + } > +} > + > +static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev) > +{ > + AcpiGedState *s = ACPI_GED(adev); > + uint32_t sel; > + > + if (ev & ACPI_MEMORY_HOTPLUG_STATUS) { > + sel = ACPI_GED_IRQ_SEL_MEM; > + } else { > + /* Unknown event. Return without generating interrupt. */ > + return; > + } > + > + /* > + * We inject the hotplug interrupt. The IRQ selector will make > + * the difference from the ACPI table. > + */ > + acpi_ged_event(&s->ged_state, sel); > +} > + > +static void acpi_ged_device_realize(DeviceState *dev, Error **errp) > +{ > + AcpiGedState *s = ACPI_GED(dev); > + > + if (s->memhp_state.is_enabled) { > + acpi_memory_hotplug_init(get_system_memory(), OBJECT(dev), > + &s->memhp_state, > + s->memhp_base); > + acpi_ged_init(get_system_memory(), dev, &s->ged_state); > + } > +} > + > +static Property acpi_ged_properties[] = { > + /* > + * Memory hotplug base address is a property of GED here, > + * because GED handles memory hotplug event and MEMORY_HOTPLUG_DEVICE > + * gets initialized when GED device is realized. > + */ > + DEFINE_PROP_UINT64("memhp-base", AcpiGedState, memhp_base, 0), > + DEFINE_PROP_BOOL("memory-hotplug-support", AcpiGedState, > + memhp_state.is_enabled, true), > + DEFINE_PROP_PTR("gsi", AcpiGedState, gsi), PTR shouldn't be used in new code, look at object_property_add_link() & co > + DEFINE_PROP_UINT64("ged-base", AcpiGedState, ged_base, 0), > + DEFINE_PROP_UINT32("ged-irq", AcpiGedState, ged_irq, 0), > + DEFINE_PROP_PTR("ged-events", AcpiGedState, ged_events), > + DEFINE_PROP_UINT32("ged-events-size", AcpiGedState, ged_events_size, 0), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void acpi_ged_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 = acpi_ged_properties; > + dc->realize = acpi_ged_device_realize; > + > + /* Reason: pointer properties "gsi" and "ged_events" */ > + dc->user_creatable = false; > + > + hc->plug = acpi_ged_device_plug_cb; > + > + adevc->send_event = acpi_ged_send_event; > +} > + > +static const TypeInfo acpi_ged_info = { > + .name = TYPE_ACPI_GED, > + .parent = TYPE_DEVICE, > + .instance_size = sizeof(AcpiGedState), > + .class_init = acpi_ged_class_init, > + .interfaces = (InterfaceInfo[]) { > + { TYPE_HOTPLUG_HANDLER }, > + { TYPE_ACPI_DEVICE_IF }, > + { } > + } > +}; > + > +static void acpi_ged_register_types(void) > +{ > + type_register_static(&acpi_ged_info); > +} > + > +type_init(acpi_ged_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..9c840d8 > --- /dev/null > +++ b/include/hw/acpi/generic_event_device.h > @@ -0,0 +1,121 @@ > +/* > + * > + * 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/>. > + * > + * The ACPI Generic Event Device (GED) is a hardware-reduced specific > + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, > + * including the hotplug ones. Generic Event Device allows platforms > + * to handle interrupts in ACPI ASL statements. It follows a very > + * similar approach like the _EVT method from GPIO events. All > + * interrupts are listed in _CRS and the handler is written in _EVT > + * method. Here, we use a single interrupt for the GED device, relying > + * on IO memory region to communicate the type of device affected by > + * the interrupt. This way, we can support up to 32 events with a > + * unique interrupt. > + * > + * Here is an example. > + * > + * Device (\_SB.GED) > + * { > + * Name (_HID, "ACPI0013") > + * Name (_UID, Zero) > + * Name (_CRS, ResourceTemplate () > + * { > + * Interrupt (ResourceConsumer, Edge, ActiveHigh, Exclusive, ,, ) > + * { > + * 0x00000029, > + * } > + * }) > + * OperationRegion (IREG, SystemMemory, 0x09080000, 0x04) > + * Field (IREG, DWordAcc, NoLock, WriteAsZeros) > + * { > + * ISEL, 32 > + * } > + * > + * Method (_EVT, 1, Serialized) // _EVT: Event > + * { > + * Local0 = ISEL // ISEL = IO memory region which specifies the > + * // device type. > + * If (((Local0 & irq0) == irq0)) > + * { > + * MethodEvent0() > + * } > + * ElseIf ((Local0 & irq1) == irq1) > + * { > + * MethodEvent1() > + * } > + * ... > + * } > + * } > + * > + */ > + > +#ifndef HW_ACPI_GED_H > +#define HW_ACPI_GED_H > + > +#include "hw/acpi/memory_hotplug.h" > + > +#define TYPE_ACPI_GED "acpi-ged" > +#define ACPI_GED(obj) \ > + OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED) > + > +#define ACPI_GED_IRQ_SEL_OFFSET 0x0 > +#define ACPI_GED_IRQ_SEL_LEN 0x4 > +#define ACPI_GED_IRQ_SEL_MEM 0x1 > +#define ACPI_GED_REG_LEN 0x4 > + > +#define GED_DEVICE "GED" > +#define AML_GED_IRQ_REG "IREG" > +#define AML_GED_IRQ_SEL "ISEL" > + > +typedef enum { > + GED_MEMORY_HOTPLUG = 1, > +} GedEventType; > + > +/* > + * Platforms need to specify their own GedEvent array > + * to describe what kind of events they want to support > + * through GED. > + */ > +typedef struct GedEvent { > + uint32_t selector; > + GedEventType event; > +} GedEvent; > + > +typedef struct GEDState { > + MemoryRegion io; > + uint32_t sel; do we need to migrate this during migration? > + uint32_t irq; > + qemu_irq *gsi; > + QemuMutex lock; > +} GEDState; > + > + > +typedef struct AcpiGedState { > + DeviceClass parent_obj; > + MemHotplugState memhp_state; > + hwaddr memhp_base; > + void *gsi; > + hwaddr ged_base; > + GEDState ged_state; > + uint32_t ged_irq; > + void *ged_events; > + uint32_t ged_events_size; > +} AcpiGedState; > + > +void build_ged_aml(Aml *table, const char* name, HotplugHandler *hotplug_dev, > + uint32_t ged_irq, AmlRegionSpace rs); > + > +#endif
Hi Igor, > -----Original Message----- > From: Igor Mammedov [mailto:imammedo@redhat.com] > Sent: 02 May 2019 17:13 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > Cc: qemu-devel@nongnu.org; qemu-arm@nongnu.org; > eric.auger@redhat.com; peter.maydell@linaro.org; > shannon.zhaosl@gmail.com; sameo@linux.intel.com; > sebastien.boeuf@intel.com; xuwei (O) <xuwei5@huawei.com>; > lersek@redhat.com; ard.biesheuvel@linaro.org; Linuxarm > <linuxarm@huawei.com> > Subject: Re: [PATCH v4 3/8] hw/acpi: Add ACPI Generic Event Device Support > > On Tue, 9 Apr 2019 11:29:30 +0100 > Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote: > > > From: Samuel Ortiz <sameo@linux.intel.com> > > > > The ACPI Generic Event Device (GED) is a hardware-reduced specific > > device[ACPI v6.1 Section 5.6.9] that handles all platform events, > > including the hotplug ones.This patch generates the AML code that > > defines GEDs. > > > > Platforms need to specify their own GedEvent array to describe what > > kind of events they want to support through GED. Also this uses a > > a single interrupt for the GED device, relying on IO memory region > > to communicate the type of device affected by the interrupt. This > > way, we can support up to 32 events with a unique interrupt. > > > > This supports only memory hotplug for now. > > > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> > > Signed-off-by: Sebastien Boeuf <sebastien.boeuf@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 | 311 > +++++++++++++++++++++++++++++++++ > > include/hw/acpi/generic_event_device.h | 121 +++++++++++++ > > 4 files changed, 437 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..856ca04 > > --- /dev/null > > +++ b/hw/acpi/generic_event_device.c > > @@ -0,0 +1,311 @@ > > +/* > > + * > > + * 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 "qapi/error.h" > > +#include "exec/address-spaces.h" > > +#include "hw/sysbus.h" > > +#include "hw/acpi/acpi.h" > > +#include "hw/acpi/generic_event_device.h" > > +#include "hw/mem/pc-dimm.h" > > + > > +static Aml *ged_event_aml(const GedEvent *event) > > +{ > > + > > + if (!event) { > In general, I prefer to check condition for calling something before doing call. > This way one can see in caller why and what is called, which is more clear. Ok. I will move it then. > > > + return NULL; > > + } > > + > > + switch (event->event) { > > + case GED_MEMORY_HOTPLUG: > > + /* We run a complete memory SCAN when getting a memory > hotplug event */ > > + return aml_call0(MEMORY_DEVICES_CONTAINER "." > MEMORY_SLOT_SCAN_METHOD); > > + default: > > + break; > > + } > > + > > + return NULL; > > +} > > + > > +/* > > + * The ACPI Generic Event Device (GED) is a hardware-reduced specific > > + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, > > + * including the hotplug ones. Platforms need to specify their own > > + * GedEvent array to describe what kind of events they want to support > > + * through GED. This routine uses a single interrupt for the GED device, > > + * relying on IO memory region to communicate the type of device > > + * affected by the interrupt. This way, we can support up to 32 events > > + * with a unique interrupt. > > + */ > > +void build_ged_aml(Aml *table, const char *name, HotplugHandler > *hotplug_dev, > > + uint32_t ged_irq, AmlRegionSpace rs) > > +{ > > + AcpiGedState *s = ACPI_GED(hotplug_dev); > > + GedEvent *ged_events = s->ged_events; > > + Aml *crs = aml_resource_template(); > > + Aml *evt, *field; > > + Aml *dev = aml_device("%s", name); > > + Aml *irq_sel = aml_local(0); > > + Aml *isel = aml_name(AML_GED_IRQ_SEL); > > + uint32_t i; > > + > > + if (!s->ged_base || !ged_events || !s->ged_events_size) { > I'd move it up to the caller, it would be obvious right there when > function should be called. Ok. > > + return; > > + } > > + > > + /* _CRS interrupt */ > > + aml_append(crs, aml_interrupt(AML_CONSUMER, AML_EDGE, > AML_ACTIVE_HIGH, > > + AML_EXCLUSIVE, &ged_irq, 1)); > > + /* > > + * For each GED event we: > > + * - Add an interrupt to the CRS section. > > + * - Add a conditional block for each event, inside a while loop. > > + * This is semantically equivalent to a switch/case implementation. > > + */ > > + evt = aml_method("_EVT", 1, AML_SERIALIZED); > > + { > > + Aml *ged_aml; > > + Aml *if_ctx; > > + > > + /* Local0 = ISEL */ > > + aml_append(evt, aml_store(isel, irq_sel)); > > + > > + /* > > + * Here we want to call a method for each supported GED event > type. > > + * The resulting ASL code looks like: > > + * > > + * Local0 = ISEL > > + * If ((Local0 & irq0) == irq0) > > + * { > > + * MethodEvent0() > > + * } > > + * > > + * If ((Local0 & irq1) == irq1) > > + * { > > + * MethodEvent1() > > + * } > > + * ... > > + */ > Well, I'm confused. > do we actually use multiple IRQs or we use only one + MMIO for event type? It is one irq + MMIO. I will change the comment block something like this, Local0 = ISEL If ((Local0 & One) == One) { MethodEvent1() } If ((Local0 & 0x02) == 0x02) { MethodEvent2() } ... > > > + for (i = 0; i < s->ged_events_size; i++) { > > > + ged_aml = ged_event_aml(&ged_events[i]); > > + if (!ged_aml) { > > + continue; > > + } > I'd get rid of ged_event_aml replace it with more 'switch': > for (i,...) > if_ctx = aml_if(...) > switch (event) > case GED_MEMORY_HOTPLUG: > aml_append(if_ctx, > aml_call0(MEMORY_DEVICES_CONTAINER "." > MEMORY_SLOT_SCAN_METHOD)) > break > default: > about(); // make sure that a newly added events have a > handler Ok. I will change this. > > > + > > + /* If ((Local1 == irq))*/ > > + if_ctx = aml_if(aml_equal(aml_and(irq_sel, > > + > aml_int(ged_events[i].selector), NULL), > > + > aml_int(ged_events[i].selector))); > > + { > > + /* AML for this specific type of event */ > > + aml_append(if_ctx), ged_aml); > > + } > > + > > + /* > > + * We append the first "if" to the "while" context. > > + * Other "if"s will be "elseif"s. > > + */ > > + aml_append(evt, if_ctx); > > + } > > + } > > + > > > + aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0013"))); > > + aml_append(dev, aml_name_decl("_UID", aml_string(GED_DEVICE))); > > + aml_append(dev, aml_name_decl("_CRS", crs)); > > + > > + /* Append IO region */ > > + aml_append(dev, aml_operation_region(AML_GED_IRQ_REG, rs, > > + aml_int(s->ged_base + ACPI_GED_IRQ_SEL_OFFSET), > > + ACPI_GED_IRQ_SEL_LEN)); > > + field = aml_field(AML_GED_IRQ_REG, AML_DWORD_ACC, > AML_NOLOCK, > > + AML_WRITE_AS_ZEROS); > > + aml_append(field, aml_named_field(AML_GED_IRQ_SEL, > > + ACPI_GED_IRQ_SEL_LEN * > 8)); > > + aml_append(dev, field); > > I'd move it up above EVT() method, so it would be clear from the begging > for what device we are building AML Ok. > > > + /* Append _EVT method */ > > + aml_append(dev, evt); > > + > > + aml_append(table, dev); > > +} > > + > > +/* Memory read by the GED _EVT AML dynamic method */ > > +static uint64_t ged_read(void *opaque, hwaddr addr, unsigned size) > > +{ > > + uint64_t val = 0; > > + GEDState *ged_st = opaque; > > + > > + switch (addr) { > > + case ACPI_GED_IRQ_SEL_OFFSET: > > + /* Read the selector value and reset it */ > > + qemu_mutex_lock(&ged_st->lock); > > + val = ged_st->sel; > > + ged_st->sel = 0; > > + qemu_mutex_unlock(&ged_st->lock); > > + break; > > + default: > > + break; > > + } > > + > > + return val; > > +} > > + > > +/* Nothing is expected to be written to the GED memory region */ > > +static void ged_write(void *opaque, hwaddr addr, uint64_t data, > > + unsigned int size) > > +{ > > +} > > + > > +static const MemoryRegionOps ged_ops = { > > + .read = ged_read, > > + .write = ged_write, > > + .endianness = DEVICE_LITTLE_ENDIAN, > > + .valid = { > > + .min_access_size = 4, > > + .max_access_size = 4, > > + }, > > +}; > > + > > +static void acpi_ged_event(GEDState *ged_st, uint32_t ged_irq_sel) > > +{ > > + /* > > + * Set the GED IRQ selector to the expected device type value. This > > + * way, the ACPI method will be able to trigger the right code based > > + * on a unique IRQ. > > + */ > > + qemu_mutex_lock(&ged_st->lock); > > + ged_st->sel = ged_irq_sel; > what if there are 2 events with different sel value and 2nd event happens > before guesr read the first one? This was previously, ged_st->sel |= ged_irq_sel; and was changed based on the assumption that there won't be any multiple events. But I think the scenario above is right. I will change it back so that we won't overwrite. > > > + qemu_mutex_unlock(&ged_st->lock); > > + > > + /* Trigger the event by sending an interrupt to the guest. */ > > + qemu_irq_pulse(ged_st->gsi[ged_st->irq]); > > +} > > + > > +static void acpi_ged_init(MemoryRegion *as, DeviceState *dev, GEDState > *ged_st) > > +{ > > + AcpiGedState *s = ACPI_GED(dev); > > + > > + assert(s->ged_base); > > + > > + ged_st->irq = s->ged_irq; > > + ged_st->gsi = s->gsi; > > + qemu_mutex_init(&ged_st->lock); > > + memory_region_init_io(&ged_st->io, OBJECT(dev), &ged_ops, ged_st, > > + "acpi-ged-event", ACPI_GED_REG_LEN); > > + memory_region_add_subregion(as, s->ged_base, &ged_st->io); > > +} > > + > > +static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev, > > + DeviceState *dev, Error **errp) > > +{ > > + AcpiGedState *s = ACPI_GED(hotplug_dev); > > + > > + if (s->memhp_state.is_enabled && > > + object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > > + acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, > > + dev, errp); > > + } else { > > + error_setg(errp, "virt: device plug request for unsupported > device" > > + " type: %s", object_get_typename(OBJECT(dev))); > > + } > > +} > > + > > +static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits > ev) > > +{ > > + AcpiGedState *s = ACPI_GED(adev); > > + uint32_t sel; > > + > > + if (ev & ACPI_MEMORY_HOTPLUG_STATUS) { > > + sel = ACPI_GED_IRQ_SEL_MEM; > > + } else { > > + /* Unknown event. Return without generating interrupt. */ > > + return; > > + } > > + > > + /* > > + * We inject the hotplug interrupt. The IRQ selector will make > > + * the difference from the ACPI table. > > + */ > > + acpi_ged_event(&s->ged_state, sel); > > +} > > + > > +static void acpi_ged_device_realize(DeviceState *dev, Error **errp) > > +{ > > + AcpiGedState *s = ACPI_GED(dev); > > + > > + if (s->memhp_state.is_enabled) { > > + acpi_memory_hotplug_init(get_system_memory(), OBJECT(dev), > > + &s->memhp_state, > > + s->memhp_base); > > + acpi_ged_init(get_system_memory(), dev, &s->ged_state); > > + } > > +} > > + > > +static Property acpi_ged_properties[] = { > > + /* > > + * Memory hotplug base address is a property of GED here, > > + * because GED handles memory hotplug event and > MEMORY_HOTPLUG_DEVICE > > + * gets initialized when GED device is realized. > > + */ > > + DEFINE_PROP_UINT64("memhp-base", AcpiGedState, memhp_base, > 0), > > + DEFINE_PROP_BOOL("memory-hotplug-support", AcpiGedState, > > + memhp_state.is_enabled, true), > > + DEFINE_PROP_PTR("gsi", AcpiGedState, gsi), > > PTR shouldn't be used in new code, look at object_property_add_link() & co Ok. I will take a look at that. > > > + DEFINE_PROP_UINT64("ged-base", AcpiGedState, ged_base, 0), > > + DEFINE_PROP_UINT32("ged-irq", AcpiGedState, ged_irq, 0), > > + DEFINE_PROP_PTR("ged-events", AcpiGedState, ged_events), > > + DEFINE_PROP_UINT32("ged-events-size", AcpiGedState, > ged_events_size, 0), > > + DEFINE_PROP_END_OF_LIST(), > > +}; > > + > > +static void acpi_ged_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 = acpi_ged_properties; > > + dc->realize = acpi_ged_device_realize; > > + > > + /* Reason: pointer properties "gsi" and "ged_events" */ > > + dc->user_creatable = false; > > + > > + hc->plug = acpi_ged_device_plug_cb; > > + > > + adevc->send_event = acpi_ged_send_event; > > +} > > + > > +static const TypeInfo acpi_ged_info = { > > + .name = TYPE_ACPI_GED, > > + .parent = TYPE_DEVICE, > > + .instance_size = sizeof(AcpiGedState), > > + .class_init = acpi_ged_class_init, > > + .interfaces = (InterfaceInfo[]) { > > + { TYPE_HOTPLUG_HANDLER }, > > + { TYPE_ACPI_DEVICE_IF }, > > + { } > > + } > > +}; > > + > > +static void acpi_ged_register_types(void) > > +{ > > + type_register_static(&acpi_ged_info); > > +} > > + > > +type_init(acpi_ged_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..9c840d8 > > --- /dev/null > > +++ b/include/hw/acpi/generic_event_device.h > > @@ -0,0 +1,121 @@ > > +/* > > + * > > + * 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/>. > > + * > > + * The ACPI Generic Event Device (GED) is a hardware-reduced specific > > + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, > > + * including the hotplug ones. Generic Event Device allows platforms > > + * to handle interrupts in ACPI ASL statements. It follows a very > > + * similar approach like the _EVT method from GPIO events. All > > + * interrupts are listed in _CRS and the handler is written in _EVT > > + * method. Here, we use a single interrupt for the GED device, relying > > + * on IO memory region to communicate the type of device affected by > > + * the interrupt. This way, we can support up to 32 events with a > > + * unique interrupt. > > + * > > + * Here is an example. > > + * > > + * Device (\_SB.GED) > > + * { > > + * Name (_HID, "ACPI0013") > > + * Name (_UID, Zero) > > + * Name (_CRS, ResourceTemplate () > > + * { > > + * Interrupt (ResourceConsumer, Edge, ActiveHigh, Exclusive, ,, ) > > + * { > > + * 0x00000029, > > + * } > > + * }) > > + * OperationRegion (IREG, SystemMemory, 0x09080000, 0x04) > > + * Field (IREG, DWordAcc, NoLock, WriteAsZeros) > > + * { > > + * ISEL, 32 > > + * } > > + * > > + * Method (_EVT, 1, Serialized) // _EVT: Event > > + * { > > + * Local0 = ISEL // ISEL = IO memory region which specifies the > > + * // device type. > > + * If (((Local0 & irq0) == irq0)) > > + * { > > + * MethodEvent0() > > + * } > > + * ElseIf ((Local0 & irq1) == irq1) > > + * { > > + * MethodEvent1() > > + * } > > + * ... > > + * } > > + * } > > + * > > + */ > > + > > +#ifndef HW_ACPI_GED_H > > +#define HW_ACPI_GED_H > > + > > +#include "hw/acpi/memory_hotplug.h" > > + > > +#define TYPE_ACPI_GED "acpi-ged" > > +#define ACPI_GED(obj) \ > > + OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED) > > + > > +#define ACPI_GED_IRQ_SEL_OFFSET 0x0 > > +#define ACPI_GED_IRQ_SEL_LEN 0x4 > > +#define ACPI_GED_IRQ_SEL_MEM 0x1 > > +#define ACPI_GED_REG_LEN 0x4 > > + > > +#define GED_DEVICE "GED" > > +#define AML_GED_IRQ_REG "IREG" > > +#define AML_GED_IRQ_SEL "ISEL" > > + > > +typedef enum { > > + GED_MEMORY_HOTPLUG = 1, > > +} GedEventType; > > + > > +/* > > + * Platforms need to specify their own GedEvent array > > + * to describe what kind of events they want to support > > + * through GED. > > + */ > > +typedef struct GedEvent { > > + uint32_t selector; > > + GedEventType event; > > +} GedEvent; > > + > > +typedef struct GEDState { > > + MemoryRegion io; > > + uint32_t sel; > > do we need to migrate this during migration? TBH, I don't know and currently this series doesn't address migration as we don't have any VMStateDescription and friends. Is this something we can sort later? Thanks, Shameer > > + uint32_t irq; > > + qemu_irq *gsi; > > + QemuMutex lock; > > +} GEDState; > > + > > + > > +typedef struct AcpiGedState { > > + DeviceClass parent_obj; > > + MemHotplugState memhp_state; > > + hwaddr memhp_base; > > + void *gsi; > > + hwaddr ged_base; > > + GEDState ged_state; > > + uint32_t ged_irq; > > + void *ged_events; > > + uint32_t ged_events_size; > > +} AcpiGedState; > > + > > +void build_ged_aml(Aml *table, const char* name, HotplugHandler > *hotplug_dev, > > + uint32_t ged_irq, AmlRegionSpace rs); > > + > > +#endif
On Fri, 3 May 2019 12:45:52 +0000 Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote: > Hi Igor, > > > -----Original Message----- > > From: Igor Mammedov [mailto:imammedo@redhat.com] > > Sent: 02 May 2019 17:13 > > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > > Cc: qemu-devel@nongnu.org; qemu-arm@nongnu.org; > > eric.auger@redhat.com; peter.maydell@linaro.org; > > shannon.zhaosl@gmail.com; sameo@linux.intel.com; > > sebastien.boeuf@intel.com; xuwei (O) <xuwei5@huawei.com>; > > lersek@redhat.com; ard.biesheuvel@linaro.org; Linuxarm > > <linuxarm@huawei.com> > > Subject: Re: [PATCH v4 3/8] hw/acpi: Add ACPI Generic Event Device Support > > > > On Tue, 9 Apr 2019 11:29:30 +0100 > > Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > From: Samuel Ortiz <sameo@linux.intel.com> > > > > > > The ACPI Generic Event Device (GED) is a hardware-reduced specific > > > device[ACPI v6.1 Section 5.6.9] that handles all platform events, > > > including the hotplug ones.This patch generates the AML code that > > > defines GEDs. > > > > > > Platforms need to specify their own GedEvent array to describe what > > > kind of events they want to support through GED. Also this uses a > > > a single interrupt for the GED device, relying on IO memory region > > > to communicate the type of device affected by the interrupt. This > > > way, we can support up to 32 events with a unique interrupt. > > > > > > This supports only memory hotplug for now. > > > > > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> > > > Signed-off-by: Sebastien Boeuf <sebastien.boeuf@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 | 311 > > +++++++++++++++++++++++++++++++++ > > > include/hw/acpi/generic_event_device.h | 121 +++++++++++++ > > > 4 files changed, 437 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..856ca04 > > > --- /dev/null > > > +++ b/hw/acpi/generic_event_device.c > > > @@ -0,0 +1,311 @@ > > > +/* > > > + * > > > + * 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 "qapi/error.h" > > > +#include "exec/address-spaces.h" > > > +#include "hw/sysbus.h" > > > +#include "hw/acpi/acpi.h" > > > +#include "hw/acpi/generic_event_device.h" > > > +#include "hw/mem/pc-dimm.h" > > > + > > > +static Aml *ged_event_aml(const GedEvent *event) > > > +{ > > > + > > > + if (!event) { > > In general, I prefer to check condition for calling something before doing call. > > This way one can see in caller why and what is called, which is more clear. > > Ok. I will move it then. > > > > > > + return NULL; > > > + } > > > + > > > + switch (event->event) { > > > + case GED_MEMORY_HOTPLUG: > > > + /* We run a complete memory SCAN when getting a memory > > hotplug event */ > > > + return aml_call0(MEMORY_DEVICES_CONTAINER "." > > MEMORY_SLOT_SCAN_METHOD); > > > + default: > > > + break; > > > + } > > > + > > > + return NULL; > > > +} > > > + > > > +/* > > > + * The ACPI Generic Event Device (GED) is a hardware-reduced specific > > > + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, > > > + * including the hotplug ones. Platforms need to specify their own > > > + * GedEvent array to describe what kind of events they want to support > > > + * through GED. This routine uses a single interrupt for the GED device, > > > + * relying on IO memory region to communicate the type of device > > > + * affected by the interrupt. This way, we can support up to 32 events > > > + * with a unique interrupt. > > > + */ > > > +void build_ged_aml(Aml *table, const char *name, HotplugHandler > > *hotplug_dev, > > > + uint32_t ged_irq, AmlRegionSpace rs) > > > +{ > > > + AcpiGedState *s = ACPI_GED(hotplug_dev); > > > + GedEvent *ged_events = s->ged_events; > > > + Aml *crs = aml_resource_template(); > > > + Aml *evt, *field; > > > + Aml *dev = aml_device("%s", name); > > > + Aml *irq_sel = aml_local(0); > > > + Aml *isel = aml_name(AML_GED_IRQ_SEL); > > > + uint32_t i; > > > + > > > + if (!s->ged_base || !ged_events || !s->ged_events_size) { > > I'd move it up to the caller, it would be obvious right there when > > function should be called. > > Ok. > > > > + return; > > > + } > > > + > > > + /* _CRS interrupt */ > > > + aml_append(crs, aml_interrupt(AML_CONSUMER, AML_EDGE, > > AML_ACTIVE_HIGH, > > > + AML_EXCLUSIVE, &ged_irq, 1)); > > > + /* > > > + * For each GED event we: > > > + * - Add an interrupt to the CRS section. > > > + * - Add a conditional block for each event, inside a while loop. > > > + * This is semantically equivalent to a switch/case implementation. > > > + */ > > > + evt = aml_method("_EVT", 1, AML_SERIALIZED); > > > + { > > > + Aml *ged_aml; > > > + Aml *if_ctx; > > > + > > > + /* Local0 = ISEL */ > > > + aml_append(evt, aml_store(isel, irq_sel)); > > > + > > > + /* > > > + * Here we want to call a method for each supported GED event > > type. > > > + * The resulting ASL code looks like: > > > + * > > > + * Local0 = ISEL > > > + * If ((Local0 & irq0) == irq0) > > > + * { > > > + * MethodEvent0() > > > + * } > > > + * > > > + * If ((Local0 & irq1) == irq1) > > > + * { > > > + * MethodEvent1() > > > + * } > > > + * ... > > > + */ > > Well, I'm confused. > > do we actually use multiple IRQs or we use only one + MMIO for event type? > > It is one irq + MMIO. I will change the comment block something like this, change corresponding variable names as well > > Local0 = ISEL > If ((Local0 & One) == One) > { > MethodEvent1() > } > > If ((Local0 & 0x02) == 0x02) > { > MethodEvent2() > } > ... > > > > > > + for (i = 0; i < s->ged_events_size; i++) { > > > > > + ged_aml = ged_event_aml(&ged_events[i]); > > > + if (!ged_aml) { > > > + continue; > > > + } > > I'd get rid of ged_event_aml replace it with more 'switch': > > for (i,...) > > if_ctx = aml_if(...) > > switch (event) > > case GED_MEMORY_HOTPLUG: > > aml_append(if_ctx, > > aml_call0(MEMORY_DEVICES_CONTAINER "." > > MEMORY_SLOT_SCAN_METHOD)) > > break > > default: > > about(); // make sure that a newly added events have a > > handler > > Ok. I will change this. > > > > > > + > > > + /* If ((Local1 == irq))*/ > > > + if_ctx = aml_if(aml_equal(aml_and(irq_sel, > > > + > > aml_int(ged_events[i].selector), NULL), > > > + > > aml_int(ged_events[i].selector))); > > > + { > > > + /* AML for this specific type of event */ > > > + aml_append(if_ctx), ged_aml); > > > + } > > > + > > > + /* > > > + * We append the first "if" to the "while" context. > > > + * Other "if"s will be "elseif"s. > > > + */ > > > + aml_append(evt, if_ctx); > > > + } > > > + } > > > + > > > > > + aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0013"))); > > > + aml_append(dev, aml_name_decl("_UID", aml_string(GED_DEVICE))); > > > + aml_append(dev, aml_name_decl("_CRS", crs)); > > > + > > > + /* Append IO region */ > > > + aml_append(dev, aml_operation_region(AML_GED_IRQ_REG, rs, > > > + aml_int(s->ged_base + ACPI_GED_IRQ_SEL_OFFSET), > > > + ACPI_GED_IRQ_SEL_LEN)); > > > + field = aml_field(AML_GED_IRQ_REG, AML_DWORD_ACC, > > AML_NOLOCK, > > > + AML_WRITE_AS_ZEROS); > > > + aml_append(field, aml_named_field(AML_GED_IRQ_SEL, > > > + ACPI_GED_IRQ_SEL_LEN * > > 8)); > > > + aml_append(dev, field); > > > > I'd move it up above EVT() method, so it would be clear from the begging > > for what device we are building AML > > Ok. > > > > > > + /* Append _EVT method */ > > > + aml_append(dev, evt); > > > + > > > + aml_append(table, dev); > > > +} > > > + > > > +/* Memory read by the GED _EVT AML dynamic method */ > > > +static uint64_t ged_read(void *opaque, hwaddr addr, unsigned size) > > > +{ > > > + uint64_t val = 0; > > > + GEDState *ged_st = opaque; > > > + > > > + switch (addr) { > > > + case ACPI_GED_IRQ_SEL_OFFSET: > > > + /* Read the selector value and reset it */ > > > + qemu_mutex_lock(&ged_st->lock); > > > + val = ged_st->sel; > > > + ged_st->sel = 0; > > > + qemu_mutex_unlock(&ged_st->lock); > > > + break; > > > + default: > > > + break; > > > + } > > > + > > > + return val; > > > +} > > > + > > > +/* Nothing is expected to be written to the GED memory region */ > > > +static void ged_write(void *opaque, hwaddr addr, uint64_t data, > > > + unsigned int size) > > > +{ > > > +} > > > + > > > +static const MemoryRegionOps ged_ops = { > > > + .read = ged_read, > > > + .write = ged_write, > > > + .endianness = DEVICE_LITTLE_ENDIAN, > > > + .valid = { > > > + .min_access_size = 4, > > > + .max_access_size = 4, > > > + }, > > > +}; > > > + > > > +static void acpi_ged_event(GEDState *ged_st, uint32_t ged_irq_sel) > > > +{ > > > + /* > > > + * Set the GED IRQ selector to the expected device type value. This > > > + * way, the ACPI method will be able to trigger the right code based > > > + * on a unique IRQ. > > > + */ > > > + qemu_mutex_lock(&ged_st->lock); > > > + ged_st->sel = ged_irq_sel; > > what if there are 2 events with different sel value and 2nd event happens > > before guesr read the first one? > > This was previously, > ged_st->sel |= ged_irq_sel; > > and was changed based on the assumption that there won't be any multiple > events. But I think the scenario above is right. I will change it back so that > we won't overwrite. so it was bitmap, then handling it as a DWORD in AML could be wrong and AML part probably should be changed to accommodate it. > > > > > > + qemu_mutex_unlock(&ged_st->lock); > > > + > > > + /* Trigger the event by sending an interrupt to the guest. */ > > > + qemu_irq_pulse(ged_st->gsi[ged_st->irq]); > > > +} > > > + > > > +static void acpi_ged_init(MemoryRegion *as, DeviceState *dev, GEDState > > *ged_st) > > > +{ > > > + AcpiGedState *s = ACPI_GED(dev); > > > + > > > + assert(s->ged_base); > > > + > > > + ged_st->irq = s->ged_irq; > > > + ged_st->gsi = s->gsi; > > > + qemu_mutex_init(&ged_st->lock); > > > + memory_region_init_io(&ged_st->io, OBJECT(dev), &ged_ops, ged_st, > > > + "acpi-ged-event", ACPI_GED_REG_LEN); > > > + memory_region_add_subregion(as, s->ged_base, &ged_st->io); > > > +} > > > + > > > +static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev, > > > + DeviceState *dev, Error **errp) > > > +{ > > > + AcpiGedState *s = ACPI_GED(hotplug_dev); > > > + > > > + if (s->memhp_state.is_enabled && > > > + object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > > > + acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, > > > + dev, errp); > > > + } else { > > > + error_setg(errp, "virt: device plug request for unsupported > > device" > > > + " type: %s", object_get_typename(OBJECT(dev))); > > > + } > > > +} > > > + > > > +static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits > > ev) > > > +{ > > > + AcpiGedState *s = ACPI_GED(adev); > > > + uint32_t sel; > > > + > > > + if (ev & ACPI_MEMORY_HOTPLUG_STATUS) { > > > + sel = ACPI_GED_IRQ_SEL_MEM; > > > + } else { > > > + /* Unknown event. Return without generating interrupt. */ > > > + return; > > > + } > > > + > > > + /* > > > + * We inject the hotplug interrupt. The IRQ selector will make > > > + * the difference from the ACPI table. > > > + */ > > > + acpi_ged_event(&s->ged_state, sel); > > > +} > > > + > > > +static void acpi_ged_device_realize(DeviceState *dev, Error **errp) > > > +{ > > > + AcpiGedState *s = ACPI_GED(dev); > > > + > > > + if (s->memhp_state.is_enabled) { > > > + acpi_memory_hotplug_init(get_system_memory(), OBJECT(dev), > > > + &s->memhp_state, > > > + s->memhp_base); > > > + acpi_ged_init(get_system_memory(), dev, &s->ged_state); > > > + } > > > +} > > > + > > > +static Property acpi_ged_properties[] = { > > > + /* > > > + * Memory hotplug base address is a property of GED here, > > > + * because GED handles memory hotplug event and > > MEMORY_HOTPLUG_DEVICE > > > + * gets initialized when GED device is realized. > > > + */ > > > + DEFINE_PROP_UINT64("memhp-base", AcpiGedState, memhp_base, > > 0), > > > + DEFINE_PROP_BOOL("memory-hotplug-support", AcpiGedState, > > > + memhp_state.is_enabled, true), > > > + DEFINE_PROP_PTR("gsi", AcpiGedState, gsi), > > > > PTR shouldn't be used in new code, look at object_property_add_link() & co > > Ok. I will take a look at that. > > > > > > + DEFINE_PROP_UINT64("ged-base", AcpiGedState, ged_base, 0), > > > + DEFINE_PROP_UINT32("ged-irq", AcpiGedState, ged_irq, 0), > > > + DEFINE_PROP_PTR("ged-events", AcpiGedState, ged_events), > > > + DEFINE_PROP_UINT32("ged-events-size", AcpiGedState, > > ged_events_size, 0), > > > + DEFINE_PROP_END_OF_LIST(), > > > +}; > > > + > > > +static void acpi_ged_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 = acpi_ged_properties; > > > + dc->realize = acpi_ged_device_realize; > > > + > > > + /* Reason: pointer properties "gsi" and "ged_events" */ > > > + dc->user_creatable = false; > > > + > > > + hc->plug = acpi_ged_device_plug_cb; > > > + > > > + adevc->send_event = acpi_ged_send_event; > > > +} > > > + > > > +static const TypeInfo acpi_ged_info = { > > > + .name = TYPE_ACPI_GED, > > > + .parent = TYPE_DEVICE, > > > + .instance_size = sizeof(AcpiGedState), > > > + .class_init = acpi_ged_class_init, > > > + .interfaces = (InterfaceInfo[]) { > > > + { TYPE_HOTPLUG_HANDLER }, > > > + { TYPE_ACPI_DEVICE_IF }, > > > + { } > > > + } > > > +}; > > > + > > > +static void acpi_ged_register_types(void) > > > +{ > > > + type_register_static(&acpi_ged_info); > > > +} > > > + > > > +type_init(acpi_ged_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..9c840d8 > > > --- /dev/null > > > +++ b/include/hw/acpi/generic_event_device.h > > > @@ -0,0 +1,121 @@ > > > +/* > > > + * > > > + * 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/>. > > > + * > > > + * The ACPI Generic Event Device (GED) is a hardware-reduced specific > > > + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, > > > + * including the hotplug ones. Generic Event Device allows platforms > > > + * to handle interrupts in ACPI ASL statements. It follows a very > > > + * similar approach like the _EVT method from GPIO events. All > > > + * interrupts are listed in _CRS and the handler is written in _EVT > > > + * method. Here, we use a single interrupt for the GED device, relying > > > + * on IO memory region to communicate the type of device affected by > > > + * the interrupt. This way, we can support up to 32 events with a > > > + * unique interrupt. > > > + * > > > + * Here is an example. > > > + * > > > + * Device (\_SB.GED) > > > + * { > > > + * Name (_HID, "ACPI0013") > > > + * Name (_UID, Zero) > > > + * Name (_CRS, ResourceTemplate () > > > + * { > > > + * Interrupt (ResourceConsumer, Edge, ActiveHigh, Exclusive, ,, ) > > > + * { > > > + * 0x00000029, > > > + * } > > > + * }) > > > + * OperationRegion (IREG, SystemMemory, 0x09080000, 0x04) > > > + * Field (IREG, DWordAcc, NoLock, WriteAsZeros) > > > + * { > > > + * ISEL, 32 > > > + * } > > > + * > > > + * Method (_EVT, 1, Serialized) // _EVT: Event > > > + * { > > > + * Local0 = ISEL // ISEL = IO memory region which specifies the > > > + * // device type. > > > + * If (((Local0 & irq0) == irq0)) > > > + * { > > > + * MethodEvent0() > > > + * } > > > + * ElseIf ((Local0 & irq1) == irq1) > > > + * { > > > + * MethodEvent1() > > > + * } > > > + * ... > > > + * } > > > + * } > > > + * > > > + */ > > > + > > > +#ifndef HW_ACPI_GED_H > > > +#define HW_ACPI_GED_H > > > + > > > +#include "hw/acpi/memory_hotplug.h" > > > + > > > +#define TYPE_ACPI_GED "acpi-ged" > > > +#define ACPI_GED(obj) \ > > > + OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED) > > > + > > > +#define ACPI_GED_IRQ_SEL_OFFSET 0x0 > > > +#define ACPI_GED_IRQ_SEL_LEN 0x4 > > > +#define ACPI_GED_IRQ_SEL_MEM 0x1 > > > +#define ACPI_GED_REG_LEN 0x4 > > > + > > > +#define GED_DEVICE "GED" > > > +#define AML_GED_IRQ_REG "IREG" > > > +#define AML_GED_IRQ_SEL "ISEL" > > > + > > > +typedef enum { > > > + GED_MEMORY_HOTPLUG = 1, > > > +} GedEventType; > > > + > > > +/* > > > + * Platforms need to specify their own GedEvent array > > > + * to describe what kind of events they want to support > > > + * through GED. > > > + */ > > > +typedef struct GedEvent { > > > + uint32_t selector; > > > + GedEventType event; > > > +} GedEvent; > > > + > > > +typedef struct GEDState { > > > + MemoryRegion io; > > > + uint32_t sel; > > > > do we need to migrate this during migration? > > TBH, I don't know and currently this series doesn't address migration > as we don't have any VMStateDescription and friends. Is this something > we can sort later? It probably should be implemented, otherwise we will have to deal with hard to debug bug reports when users will try to migrate. Alternative hack could be that, enabling memory hotplug should put migration blocker, but that's probably the same effort as adding proper VMStateDescription > > Thanks, > Shameer > > > > + uint32_t irq; > > > + qemu_irq *gsi; > > > + QemuMutex lock; > > > +} GEDState; > > > + > > > + > > > +typedef struct AcpiGedState { > > > + DeviceClass parent_obj; > > > + MemHotplugState memhp_state; > > > + hwaddr memhp_base; > > > + void *gsi; > > > + hwaddr ged_base; > > > + GEDState ged_state; > > > + uint32_t ged_irq; > > > + void *ged_events; > > > + uint32_t ged_events_size; > > > +} AcpiGedState; > > > + > > > +void build_ged_aml(Aml *table, const char* name, HotplugHandler > > *hotplug_dev, > > > + uint32_t ged_irq, AmlRegionSpace rs); > > > + > > > +#endif >
Hi Igor, > -----Original Message----- > From: Igor Mammedov [mailto:imammedo@redhat.com] > Sent: 03 May 2019 16:10 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > Cc: qemu-devel@nongnu.org; qemu-arm@nongnu.org; > eric.auger@redhat.com; peter.maydell@linaro.org; > shannon.zhaosl@gmail.com; sameo@linux.intel.com; > sebastien.boeuf@intel.com; xuwei (O) <xuwei5@huawei.com>; > lersek@redhat.com; ard.biesheuvel@linaro.org; Linuxarm > <linuxarm@huawei.com> > Subject: Re: [PATCH v4 3/8] hw/acpi: Add ACPI Generic Event Device Support [...] > > > type. > > > > + * The resulting ASL code looks like: > > > > + * > > > > + * Local0 = ISEL > > > > + * If ((Local0 & irq0) == irq0) > > > > + * { > > > > + * MethodEvent0() > > > > + * } > > > > + * > > > > + * If ((Local0 & irq1) == irq1) > > > > + * { > > > > + * MethodEvent1() > > > > + * } > > > > + * ... > > > > + */ > > > Well, I'm confused. > > > do we actually use multiple IRQs or we use only one + MMIO for event > type? > > > > It is one irq + MMIO. I will change the comment block something like > > this, > > change corresponding variable names as well Ok. > > > > Local0 = ISEL > > If ((Local0 & One) == One) > > { > > MethodEvent1() > > } > > > > If ((Local0 & 0x02) == 0x02) > > { > > MethodEvent2() > > } > > ... > > > > > > > > > + for (i = 0; i < s->ged_events_size; i++) { > > > > > > > + ged_aml = ged_event_aml(&ged_events[i]); > > > > + if (!ged_aml) { > > > > + continue; > > > > + } > > > I'd get rid of ged_event_aml replace it with more 'switch': > > > for (i,...) > > > if_ctx = aml_if(...) > > > switch (event) > > > case GED_MEMORY_HOTPLUG: > > > aml_append(if_ctx, > > > aml_call0(MEMORY_DEVICES_CONTAINER "." > > > MEMORY_SLOT_SCAN_METHOD)) > > > break > > > default: > > > about(); // make sure that a newly added events have > > > a handler > > > > Ok. I will change this. > > > > > > > > > + > > > > + /* If ((Local1 == irq))*/ > > > > + if_ctx = aml_if(aml_equal(aml_and(irq_sel, > > > > + > > > aml_int(ged_events[i].selector), NULL), > > > > + > > > aml_int(ged_events[i].selector))); > > > > + { > > > > + /* AML for this specific type of event */ > > > > + aml_append(if_ctx), ged_aml); > > > > + } > > > > + > > > > + /* > > > > + * We append the first "if" to the "while" context. > > > > + * Other "if"s will be "elseif"s. > > > > + */ > > > > + aml_append(evt, if_ctx); > > > > + } > > > > + } > > > > + > > > > > > > + aml_append(dev, aml_name_decl("_HID", > aml_string("ACPI0013"))); > > > > + aml_append(dev, aml_name_decl("_UID", > aml_string(GED_DEVICE))); > > > > + aml_append(dev, aml_name_decl("_CRS", crs)); > > > > + > > > > + /* Append IO region */ > > > > + aml_append(dev, aml_operation_region(AML_GED_IRQ_REG, rs, > > > > + aml_int(s->ged_base + ACPI_GED_IRQ_SEL_OFFSET), > > > > + ACPI_GED_IRQ_SEL_LEN)); > > > > + field = aml_field(AML_GED_IRQ_REG, AML_DWORD_ACC, > > > AML_NOLOCK, > > > > + AML_WRITE_AS_ZEROS); > > > > + aml_append(field, aml_named_field(AML_GED_IRQ_SEL, > > > > + ACPI_GED_IRQ_SEL_LEN > * > > > 8)); > > > > + aml_append(dev, field); > > > > > > I'd move it up above EVT() method, so it would be clear from the > > > begging for what device we are building AML > > > > Ok. > > > > > > > > > + /* Append _EVT method */ > > > > + aml_append(dev, evt); > > > > + > > > > + aml_append(table, dev); > > > > +} > > > > + > > > > +/* Memory read by the GED _EVT AML dynamic method */ static > > > > +uint64_t ged_read(void *opaque, hwaddr addr, unsigned size) { > > > > + uint64_t val = 0; > > > > + GEDState *ged_st = opaque; > > > > + > > > > + switch (addr) { > > > > + case ACPI_GED_IRQ_SEL_OFFSET: > > > > + /* Read the selector value and reset it */ > > > > + qemu_mutex_lock(&ged_st->lock); > > > > + val = ged_st->sel; > > > > + ged_st->sel = 0; > > > > + qemu_mutex_unlock(&ged_st->lock); > > > > + break; > > > > + default: > > > > + break; > > > > + } > > > > + > > > > + return val; > > > > +} > > > > + > > > > +/* Nothing is expected to be written to the GED memory region */ > > > > +static void ged_write(void *opaque, hwaddr addr, uint64_t data, > > > > + unsigned int size) { } > > > > + > > > > +static const MemoryRegionOps ged_ops = { > > > > + .read = ged_read, > > > > + .write = ged_write, > > > > + .endianness = DEVICE_LITTLE_ENDIAN, > > > > + .valid = { > > > > + .min_access_size = 4, > > > > + .max_access_size = 4, > > > > + }, > > > > +}; > > > > + > > > > +static void acpi_ged_event(GEDState *ged_st, uint32_t > > > > +ged_irq_sel) { > > > > + /* > > > > + * Set the GED IRQ selector to the expected device type value. This > > > > + * way, the ACPI method will be able to trigger the right code based > > > > + * on a unique IRQ. > > > > + */ > > > > + qemu_mutex_lock(&ged_st->lock); > > > > + ged_st->sel = ged_irq_sel; > > > what if there are 2 events with different sel value and 2nd event > > > happens before guesr read the first one? > > > > This was previously, > > ged_st->sel |= ged_irq_sel; > > > > and was changed based on the assumption that there won't be any > > multiple events. But I think the scenario above is right. I will > > change it back so that we won't overwrite. > so it was bitmap, then handling it as a DWORD in AML could be wrong and AML > part probably should be changed to accommodate it. Hmm..I am slightly confused as the AML code already uses the len as 32 bits for this register field, /* Append IO region */ aml_append(dev, aml_operation_region(AML_GED_IRQ_REG, rs, aml_int(s->ged_base + ACPI_GED_IRQ_SEL_OFFSET), ACPI_GED_IRQ_SEL_LEN)); field = aml_field(AML_GED_IRQ_REG, AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS); aml_append(field, aml_named_field(AML_GED_IRQ_SEL, ACPI_GED_IRQ_SEL_LEN * 8)); aml_append(dev, field); and event check logic is, if_ctx = aml_if(aml_equal(aml_and(irq_sel, aml_int(ged_events[i].selector), NULL), aml_int(ged_events[i].selector))); Not sure what is the concern here. Please let me know. > > > > > > > > > + qemu_mutex_unlock(&ged_st->lock); > > > > + > > > > + /* Trigger the event by sending an interrupt to the guest. */ > > > > + qemu_irq_pulse(ged_st->gsi[ged_st->irq]); > > > > +} > > > > + > > > > +static void acpi_ged_init(MemoryRegion *as, DeviceState *dev, > > > > +GEDState > > > *ged_st) > > > > +{ > > > > + AcpiGedState *s = ACPI_GED(dev); > > > > + > > > > + assert(s->ged_base); > > > > + > > > > + ged_st->irq = s->ged_irq; > > > > + ged_st->gsi = s->gsi; > > > > + qemu_mutex_init(&ged_st->lock); > > > > + memory_region_init_io(&ged_st->io, OBJECT(dev), &ged_ops, > ged_st, > > > > + "acpi-ged-event", ACPI_GED_REG_LEN); > > > > + memory_region_add_subregion(as, s->ged_base, &ged_st->io); } > > > > + > > > > +static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev, > > > > + DeviceState *dev, Error > > > > +**errp) { > > > > + AcpiGedState *s = ACPI_GED(hotplug_dev); > > > > + > > > > + if (s->memhp_state.is_enabled && > > > > + object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > > > > + acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, > > > > + dev, errp); > > > > + } else { > > > > + error_setg(errp, "virt: device plug request for > > > > + unsupported > > > device" > > > > + " type: %s", > object_get_typename(OBJECT(dev))); > > > > + } > > > > +} > > > > + > > > > +static void acpi_ged_send_event(AcpiDeviceIf *adev, > > > > +AcpiEventStatusBits > > > ev) > > > > +{ > > > > + AcpiGedState *s = ACPI_GED(adev); > > > > + uint32_t sel; > > > > + > > > > + if (ev & ACPI_MEMORY_HOTPLUG_STATUS) { > > > > + sel = ACPI_GED_IRQ_SEL_MEM; > > > > + } else { > > > > + /* Unknown event. Return without generating interrupt. */ > > > > + return; > > > > + } > > > > + > > > > + /* > > > > + * We inject the hotplug interrupt. The IRQ selector will make > > > > + * the difference from the ACPI table. > > > > + */ > > > > + acpi_ged_event(&s->ged_state, sel); } > > > > + > > > > +static void acpi_ged_device_realize(DeviceState *dev, Error > > > > +**errp) { > > > > + AcpiGedState *s = ACPI_GED(dev); > > > > + > > > > + if (s->memhp_state.is_enabled) { > > > > + acpi_memory_hotplug_init(get_system_memory(), > OBJECT(dev), > > > > + &s->memhp_state, > > > > + s->memhp_base); > > > > + acpi_ged_init(get_system_memory(), dev, &s->ged_state); > > > > + } > > > > +} > > > > + > > > > +static Property acpi_ged_properties[] = { > > > > + /* > > > > + * Memory hotplug base address is a property of GED here, > > > > + * because GED handles memory hotplug event and > > > MEMORY_HOTPLUG_DEVICE > > > > + * gets initialized when GED device is realized. > > > > + */ > > > > + DEFINE_PROP_UINT64("memhp-base", AcpiGedState, > memhp_base, > > > 0), > > > > + DEFINE_PROP_BOOL("memory-hotplug-support", AcpiGedState, > > > > + memhp_state.is_enabled, true), > > > > + DEFINE_PROP_PTR("gsi", AcpiGedState, gsi), > > > > > > PTR shouldn't be used in new code, look at > > > object_property_add_link() & co > > > > Ok. I will take a look at that. > > > > > > > > > + DEFINE_PROP_UINT64("ged-base", AcpiGedState, ged_base, 0), > > > > + DEFINE_PROP_UINT32("ged-irq", AcpiGedState, ged_irq, 0), > > > > + DEFINE_PROP_PTR("ged-events", AcpiGedState, ged_events), > > > > + DEFINE_PROP_UINT32("ged-events-size", AcpiGedState, > > > ged_events_size, 0), > > > > + DEFINE_PROP_END_OF_LIST(), > > > > +}; > > > > + > > > > +static void acpi_ged_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 = acpi_ged_properties; > > > > + dc->realize = acpi_ged_device_realize; > > > > + > > > > + /* Reason: pointer properties "gsi" and "ged_events" */ > > > > + dc->user_creatable = false; > > > > + > > > > + hc->plug = acpi_ged_device_plug_cb; > > > > + > > > > + adevc->send_event = acpi_ged_send_event; } > > > > + > > > > +static const TypeInfo acpi_ged_info = { > > > > + .name = TYPE_ACPI_GED, > > > > + .parent = TYPE_DEVICE, > > > > + .instance_size = sizeof(AcpiGedState), > > > > + .class_init = acpi_ged_class_init, > > > > + .interfaces = (InterfaceInfo[]) { > > > > + { TYPE_HOTPLUG_HANDLER }, > > > > + { TYPE_ACPI_DEVICE_IF }, > > > > + { } > > > > + } > > > > +}; > > > > + > > > > +static void acpi_ged_register_types(void) { > > > > + type_register_static(&acpi_ged_info); > > > > +} > > > > + > > > > +type_init(acpi_ged_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..9c840d8 > > > > --- /dev/null > > > > +++ b/include/hw/acpi/generic_event_device.h > > > > @@ -0,0 +1,121 @@ > > > > +/* > > > > + * > > > > + * 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/>. > > > > + * > > > > + * The ACPI Generic Event Device (GED) is a hardware-reduced > > > > + specific > > > > + * device[ACPI v6.1 Section 5.6.9] that handles all platform > > > > + events, > > > > + * including the hotplug ones. Generic Event Device allows > > > > + platforms > > > > + * to handle interrupts in ACPI ASL statements. It follows a very > > > > + * similar approach like the _EVT method from GPIO events. All > > > > + * interrupts are listed in _CRS and the handler is written in > > > > + _EVT > > > > + * method. Here, we use a single interrupt for the GED device, > > > > + relying > > > > + * on IO memory region to communicate the type of device affected > > > > + by > > > > + * the interrupt. This way, we can support up to 32 events with a > > > > + * unique interrupt. > > > > + * > > > > + * Here is an example. > > > > + * > > > > + * Device (\_SB.GED) > > > > + * { > > > > + * Name (_HID, "ACPI0013") > > > > + * Name (_UID, Zero) > > > > + * Name (_CRS, ResourceTemplate () > > > > + * { > > > > + * Interrupt (ResourceConsumer, Edge, ActiveHigh, > Exclusive, ,, ) > > > > + * { > > > > + * 0x00000029, > > > > + * } > > > > + * }) > > > > + * OperationRegion (IREG, SystemMemory, 0x09080000, 0x04) > > > > + * Field (IREG, DWordAcc, NoLock, WriteAsZeros) > > > > + * { > > > > + * ISEL, 32 > > > > + * } > > > > + * > > > > + * Method (_EVT, 1, Serialized) // _EVT: Event > > > > + * { > > > > + * Local0 = ISEL // ISEL = IO memory region which specifies the > > > > + * // device type. > > > > + * If (((Local0 & irq0) == irq0)) > > > > + * { > > > > + * MethodEvent0() > > > > + * } > > > > + * ElseIf ((Local0 & irq1) == irq1) > > > > + * { > > > > + * MethodEvent1() > > > > + * } > > > > + * ... > > > > + * } > > > > + * } > > > > + * > > > > + */ > > > > + > > > > +#ifndef HW_ACPI_GED_H > > > > +#define HW_ACPI_GED_H > > > > + > > > > +#include "hw/acpi/memory_hotplug.h" > > > > + > > > > +#define TYPE_ACPI_GED "acpi-ged" > > > > +#define ACPI_GED(obj) \ > > > > + OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED) > > > > + > > > > +#define ACPI_GED_IRQ_SEL_OFFSET 0x0 > > > > +#define ACPI_GED_IRQ_SEL_LEN 0x4 > > > > +#define ACPI_GED_IRQ_SEL_MEM 0x1 > > > > +#define ACPI_GED_REG_LEN 0x4 > > > > + > > > > +#define GED_DEVICE "GED" > > > > +#define AML_GED_IRQ_REG "IREG" > > > > +#define AML_GED_IRQ_SEL "ISEL" > > > > + > > > > +typedef enum { > > > > + GED_MEMORY_HOTPLUG = 1, > > > > +} GedEventType; > > > > + > > > > +/* > > > > + * Platforms need to specify their own GedEvent array > > > > + * to describe what kind of events they want to support > > > > + * through GED. > > > > + */ > > > > +typedef struct GedEvent { > > > > + uint32_t selector; > > > > + GedEventType event; > > > > +} GedEvent; > > > > + > > > > +typedef struct GEDState { > > > > + MemoryRegion io; > > > > + uint32_t sel; > > > > > > do we need to migrate this during migration? > > > > TBH, I don't know and currently this series doesn't address migration > > as we don't have any VMStateDescription and friends. Is this something > > we can sort later? > > It probably should be implemented, otherwise we will have to deal with hard to > debug bug reports when users will try to migrate. > > Alternative hack could be that, enabling memory hotplug should put migration > blocker, but that's probably the same effort as adding proper > VMStateDescription Ok. I will take a look into VMStateDescription then. Thanks, Shameer > > > > Thanks, > > Shameer > > > > > > + uint32_t irq; > > > > + qemu_irq *gsi; > > > > + QemuMutex lock; > > > > +} GEDState; > > > > + > > > > + > > > > +typedef struct AcpiGedState { > > > > + DeviceClass parent_obj; > > > > + MemHotplugState memhp_state; > > > > + hwaddr memhp_base; > > > > + void *gsi; > > > > + hwaddr ged_base; > > > > + GEDState ged_state; > > > > + uint32_t ged_irq; > > > > + void *ged_events; > > > > + uint32_t ged_events_size; > > > > +} AcpiGedState; > > > > + > > > > +void build_ged_aml(Aml *table, const char* name, HotplugHandler > > > *hotplug_dev, > > > > + uint32_t ged_irq, AmlRegionSpace rs); > > > > + > > > > +#endif > >
On Tue, 7 May 2019 09:01:43 +0000 Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote: > Hi Igor, > > > -----Original Message----- > > From: Igor Mammedov [mailto:imammedo@redhat.com] > > Sent: 03 May 2019 16:10 > > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > > Cc: qemu-devel@nongnu.org; qemu-arm@nongnu.org; > > eric.auger@redhat.com; peter.maydell@linaro.org; > > shannon.zhaosl@gmail.com; sameo@linux.intel.com; > > sebastien.boeuf@intel.com; xuwei (O) <xuwei5@huawei.com>; > > lersek@redhat.com; ard.biesheuvel@linaro.org; Linuxarm > > <linuxarm@huawei.com> > > Subject: Re: [PATCH v4 3/8] hw/acpi: Add ACPI Generic Event Device Support > > [...] > > > > > type. > > > > > + * The resulting ASL code looks like: > > > > > + * > > > > > + * Local0 = ISEL > > > > > + * If ((Local0 & irq0) == irq0) > > > > > + * { > > > > > + * MethodEvent0() > > > > > + * } > > > > > + * > > > > > + * If ((Local0 & irq1) == irq1) > > > > > + * { > > > > > + * MethodEvent1() > > > > > + * } > > > > > + * ... > > > > > + */ > > > > Well, I'm confused. > > > > do we actually use multiple IRQs or we use only one + MMIO for event > > type? > > > > > > It is one irq + MMIO. I will change the comment block something like > > > this, > > > > change corresponding variable names as well > > Ok. > > > > > > > Local0 = ISEL > > > If ((Local0 & One) == One) > > > { > > > MethodEvent1() > > > } > > > > > > If ((Local0 & 0x02) == 0x02) > > > { > > > MethodEvent2() > > > } > > > ... > > > > > > > > > > > > + for (i = 0; i < s->ged_events_size; i++) { > > > > > > > > > + ged_aml = ged_event_aml(&ged_events[i]); > > > > > + if (!ged_aml) { > > > > > + continue; > > > > > + } > > > > I'd get rid of ged_event_aml replace it with more 'switch': > > > > for (i,...) > > > > if_ctx = aml_if(...) > > > > switch (event) > > > > case GED_MEMORY_HOTPLUG: > > > > aml_append(if_ctx, > > > > aml_call0(MEMORY_DEVICES_CONTAINER "." > > > > MEMORY_SLOT_SCAN_METHOD)) > > > > break > > > > default: > > > > about(); // make sure that a newly added events have > > > > a handler > > > > > > Ok. I will change this. > > > > > > > > > > > > + > > > > > + /* If ((Local1 == irq))*/ > > > > > + if_ctx = aml_if(aml_equal(aml_and(irq_sel, > > > > > + > > > > aml_int(ged_events[i].selector), NULL), > > > > > + > > > > aml_int(ged_events[i].selector))); > > > > > + { > > > > > + /* AML for this specific type of event */ > > > > > + aml_append(if_ctx), ged_aml); > > > > > + } > > > > > + > > > > > + /* > > > > > + * We append the first "if" to the "while" context. > > > > > + * Other "if"s will be "elseif"s. > > > > > + */ > > > > > + aml_append(evt, if_ctx); > > > > > + } > > > > > + } > > > > > + > > > > > > > > > + aml_append(dev, aml_name_decl("_HID", > > aml_string("ACPI0013"))); > > > > > + aml_append(dev, aml_name_decl("_UID", > > aml_string(GED_DEVICE))); > > > > > + aml_append(dev, aml_name_decl("_CRS", crs)); > > > > > + > > > > > + /* Append IO region */ > > > > > + aml_append(dev, aml_operation_region(AML_GED_IRQ_REG, rs, > > > > > + aml_int(s->ged_base + ACPI_GED_IRQ_SEL_OFFSET), > > > > > + ACPI_GED_IRQ_SEL_LEN)); > > > > > + field = aml_field(AML_GED_IRQ_REG, AML_DWORD_ACC, > > > > AML_NOLOCK, > > > > > + AML_WRITE_AS_ZEROS); > > > > > + aml_append(field, aml_named_field(AML_GED_IRQ_SEL, > > > > > + ACPI_GED_IRQ_SEL_LEN > > * > > > > 8)); > > > > > + aml_append(dev, field); > > > > > > > > I'd move it up above EVT() method, so it would be clear from the > > > > begging for what device we are building AML > > > > > > Ok. > > > > > > > > > > > > + /* Append _EVT method */ > > > > > + aml_append(dev, evt); > > > > > + > > > > > + aml_append(table, dev); > > > > > +} > > > > > + > > > > > +/* Memory read by the GED _EVT AML dynamic method */ static > > > > > +uint64_t ged_read(void *opaque, hwaddr addr, unsigned size) { > > > > > + uint64_t val = 0; > > > > > + GEDState *ged_st = opaque; > > > > > + > > > > > + switch (addr) { > > > > > + case ACPI_GED_IRQ_SEL_OFFSET: > > > > > + /* Read the selector value and reset it */ > > > > > + qemu_mutex_lock(&ged_st->lock); > > > > > + val = ged_st->sel; > > > > > + ged_st->sel = 0; > > > > > + qemu_mutex_unlock(&ged_st->lock); > > > > > + break; > > > > > + default: > > > > > + break; > > > > > + } > > > > > + > > > > > + return val; > > > > > +} > > > > > + > > > > > +/* Nothing is expected to be written to the GED memory region */ > > > > > +static void ged_write(void *opaque, hwaddr addr, uint64_t data, > > > > > + unsigned int size) { } > > > > > + > > > > > +static const MemoryRegionOps ged_ops = { > > > > > + .read = ged_read, > > > > > + .write = ged_write, > > > > > + .endianness = DEVICE_LITTLE_ENDIAN, > > > > > + .valid = { > > > > > + .min_access_size = 4, > > > > > + .max_access_size = 4, > > > > > + }, > > > > > +}; > > > > > + > > > > > +static void acpi_ged_event(GEDState *ged_st, uint32_t > > > > > +ged_irq_sel) { > > > > > + /* > > > > > + * Set the GED IRQ selector to the expected device type value. This > > > > > + * way, the ACPI method will be able to trigger the right code based > > > > > + * on a unique IRQ. > > > > > + */ > > > > > + qemu_mutex_lock(&ged_st->lock); > > > > > + ged_st->sel = ged_irq_sel; > > > > what if there are 2 events with different sel value and 2nd event > > > > happens before guesr read the first one? > > > > > > This was previously, > > > ged_st->sel |= ged_irq_sel; > > > > > > and was changed based on the assumption that there won't be any > > > multiple events. But I think the scenario above is right. I will > > > change it back so that we won't overwrite. > > so it was bitmap, then handling it as a DWORD in AML could be wrong and AML > > part probably should be changed to accommodate it. > > Hmm..I am slightly confused as the AML code already uses the len as 32 bits for this > register field, > > /* Append IO region */ > aml_append(dev, aml_operation_region(AML_GED_IRQ_REG, rs, > aml_int(s->ged_base + ACPI_GED_IRQ_SEL_OFFSET), > ACPI_GED_IRQ_SEL_LEN)); > field = aml_field(AML_GED_IRQ_REG, AML_DWORD_ACC, AML_NOLOCK, > AML_WRITE_AS_ZEROS); > aml_append(field, aml_named_field(AML_GED_IRQ_SEL, > ACPI_GED_IRQ_SEL_LEN * 8)); > aml_append(dev, field); > > and event check logic is, > > if_ctx = aml_if(aml_equal(aml_and(irq_sel, > aml_int(ged_events[i].selector), NULL), > aml_int(ged_events[i].selector))); > > Not sure what is the concern here. Please let me know. Looking at it second time, it seems I've been mistaken. that 'if' should work. one suggestion how to improve AML side, instead of using AML_GED_IRQ_REG DWORD + AND masking you can use bit fields directly. See how "aml_named_field(...,1)" is used for inspiration. > > > > > > > > > > > > > + qemu_mutex_unlock(&ged_st->lock); > > > > > + > > > > > + /* Trigger the event by sending an interrupt to the guest. */ > > > > > + qemu_irq_pulse(ged_st->gsi[ged_st->irq]); > > > > > +} > > > > > + > > > > > +static void acpi_ged_init(MemoryRegion *as, DeviceState *dev, > > > > > +GEDState > > > > *ged_st) > > > > > +{ > > > > > + AcpiGedState *s = ACPI_GED(dev); > > > > > + > > > > > + assert(s->ged_base); > > > > > + > > > > > + ged_st->irq = s->ged_irq; > > > > > + ged_st->gsi = s->gsi; > > > > > + qemu_mutex_init(&ged_st->lock); > > > > > + memory_region_init_io(&ged_st->io, OBJECT(dev), &ged_ops, > > ged_st, > > > > > + "acpi-ged-event", ACPI_GED_REG_LEN); > > > > > + memory_region_add_subregion(as, s->ged_base, &ged_st->io); } > > > > > + > > > > > +static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev, > > > > > + DeviceState *dev, Error > > > > > +**errp) { > > > > > + AcpiGedState *s = ACPI_GED(hotplug_dev); > > > > > + > > > > > + if (s->memhp_state.is_enabled && > > > > > + object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > > > > > + acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, > > > > > + dev, errp); > > > > > + } else { > > > > > + error_setg(errp, "virt: device plug request for > > > > > + unsupported > > > > device" > > > > > + " type: %s", > > object_get_typename(OBJECT(dev))); > > > > > + } > > > > > +} > > > > > + > > > > > +static void acpi_ged_send_event(AcpiDeviceIf *adev, > > > > > +AcpiEventStatusBits > > > > ev) > > > > > +{ > > > > > + AcpiGedState *s = ACPI_GED(adev); > > > > > + uint32_t sel; > > > > > + > > > > > + if (ev & ACPI_MEMORY_HOTPLUG_STATUS) { > > > > > + sel = ACPI_GED_IRQ_SEL_MEM; > > > > > + } else { > > > > > + /* Unknown event. Return without generating interrupt. */ > > > > > + return; > > > > > + } > > > > > + > > > > > + /* > > > > > + * We inject the hotplug interrupt. The IRQ selector will make > > > > > + * the difference from the ACPI table. > > > > > + */ > > > > > + acpi_ged_event(&s->ged_state, sel); } > > > > > + > > > > > +static void acpi_ged_device_realize(DeviceState *dev, Error > > > > > +**errp) { > > > > > + AcpiGedState *s = ACPI_GED(dev); > > > > > + > > > > > + if (s->memhp_state.is_enabled) { > > > > > + acpi_memory_hotplug_init(get_system_memory(), > > OBJECT(dev), > > > > > + &s->memhp_state, > > > > > + s->memhp_base); > > > > > + acpi_ged_init(get_system_memory(), dev, &s->ged_state); > > > > > + } > > > > > +} > > > > > + > > > > > +static Property acpi_ged_properties[] = { > > > > > + /* > > > > > + * Memory hotplug base address is a property of GED here, > > > > > + * because GED handles memory hotplug event and > > > > MEMORY_HOTPLUG_DEVICE > > > > > + * gets initialized when GED device is realized. > > > > > + */ > > > > > + DEFINE_PROP_UINT64("memhp-base", AcpiGedState, > > memhp_base, > > > > 0), > > > > > + DEFINE_PROP_BOOL("memory-hotplug-support", AcpiGedState, > > > > > + memhp_state.is_enabled, true), > > > > > + DEFINE_PROP_PTR("gsi", AcpiGedState, gsi), > > > > > > > > PTR shouldn't be used in new code, look at > > > > object_property_add_link() & co > > > > > > Ok. I will take a look at that. > > > > > > > > > > > > + DEFINE_PROP_UINT64("ged-base", AcpiGedState, ged_base, 0), > > > > > + DEFINE_PROP_UINT32("ged-irq", AcpiGedState, ged_irq, 0), > > > > > + DEFINE_PROP_PTR("ged-events", AcpiGedState, ged_events), > > > > > + DEFINE_PROP_UINT32("ged-events-size", AcpiGedState, > > > > ged_events_size, 0), > > > > > + DEFINE_PROP_END_OF_LIST(), > > > > > +}; > > > > > + > > > > > +static void acpi_ged_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 = acpi_ged_properties; > > > > > + dc->realize = acpi_ged_device_realize; > > > > > + > > > > > + /* Reason: pointer properties "gsi" and "ged_events" */ > > > > > + dc->user_creatable = false; > > > > > + > > > > > + hc->plug = acpi_ged_device_plug_cb; > > > > > + > > > > > + adevc->send_event = acpi_ged_send_event; } > > > > > + > > > > > +static const TypeInfo acpi_ged_info = { > > > > > + .name = TYPE_ACPI_GED, > > > > > + .parent = TYPE_DEVICE, > > > > > + .instance_size = sizeof(AcpiGedState), > > > > > + .class_init = acpi_ged_class_init, > > > > > + .interfaces = (InterfaceInfo[]) { > > > > > + { TYPE_HOTPLUG_HANDLER }, > > > > > + { TYPE_ACPI_DEVICE_IF }, > > > > > + { } > > > > > + } > > > > > +}; > > > > > + > > > > > +static void acpi_ged_register_types(void) { > > > > > + type_register_static(&acpi_ged_info); > > > > > +} > > > > > + > > > > > +type_init(acpi_ged_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..9c840d8 > > > > > --- /dev/null > > > > > +++ b/include/hw/acpi/generic_event_device.h > > > > > @@ -0,0 +1,121 @@ > > > > > +/* > > > > > + * > > > > > + * 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/>. > > > > > + * > > > > > + * The ACPI Generic Event Device (GED) is a hardware-reduced > > > > > + specific > > > > > + * device[ACPI v6.1 Section 5.6.9] that handles all platform > > > > > + events, > > > > > + * including the hotplug ones. Generic Event Device allows > > > > > + platforms > > > > > + * to handle interrupts in ACPI ASL statements. It follows a very > > > > > + * similar approach like the _EVT method from GPIO events. All > > > > > + * interrupts are listed in _CRS and the handler is written in > > > > > + _EVT > > > > > + * method. Here, we use a single interrupt for the GED device, > > > > > + relying > > > > > + * on IO memory region to communicate the type of device affected > > > > > + by > > > > > + * the interrupt. This way, we can support up to 32 events with a > > > > > + * unique interrupt. > > > > > + * > > > > > + * Here is an example. > > > > > + * > > > > > + * Device (\_SB.GED) > > > > > + * { > > > > > + * Name (_HID, "ACPI0013") > > > > > + * Name (_UID, Zero) > > > > > + * Name (_CRS, ResourceTemplate () > > > > > + * { > > > > > + * Interrupt (ResourceConsumer, Edge, ActiveHigh, > > Exclusive, ,, ) > > > > > + * { > > > > > + * 0x00000029, > > > > > + * } > > > > > + * }) > > > > > + * OperationRegion (IREG, SystemMemory, 0x09080000, 0x04) > > > > > + * Field (IREG, DWordAcc, NoLock, WriteAsZeros) > > > > > + * { > > > > > + * ISEL, 32 > > > > > + * } > > > > > + * > > > > > + * Method (_EVT, 1, Serialized) // _EVT: Event > > > > > + * { > > > > > + * Local0 = ISEL // ISEL = IO memory region which specifies the > > > > > + * // device type. > > > > > + * If (((Local0 & irq0) == irq0)) > > > > > + * { > > > > > + * MethodEvent0() > > > > > + * } > > > > > + * ElseIf ((Local0 & irq1) == irq1) > > > > > + * { > > > > > + * MethodEvent1() > > > > > + * } > > > > > + * ... > > > > > + * } > > > > > + * } > > > > > + * > > > > > + */ > > > > > + > > > > > +#ifndef HW_ACPI_GED_H > > > > > +#define HW_ACPI_GED_H > > > > > + > > > > > +#include "hw/acpi/memory_hotplug.h" > > > > > + > > > > > +#define TYPE_ACPI_GED "acpi-ged" > > > > > +#define ACPI_GED(obj) \ > > > > > + OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED) > > > > > + > > > > > +#define ACPI_GED_IRQ_SEL_OFFSET 0x0 > > > > > +#define ACPI_GED_IRQ_SEL_LEN 0x4 > > > > > +#define ACPI_GED_IRQ_SEL_MEM 0x1 > > > > > +#define ACPI_GED_REG_LEN 0x4 > > > > > + > > > > > +#define GED_DEVICE "GED" > > > > > +#define AML_GED_IRQ_REG "IREG" > > > > > +#define AML_GED_IRQ_SEL "ISEL" > > > > > + > > > > > +typedef enum { > > > > > + GED_MEMORY_HOTPLUG = 1, > > > > > +} GedEventType; > > > > > + > > > > > +/* > > > > > + * Platforms need to specify their own GedEvent array > > > > > + * to describe what kind of events they want to support > > > > > + * through GED. > > > > > + */ > > > > > +typedef struct GedEvent { > > > > > + uint32_t selector; > > > > > + GedEventType event; > > > > > +} GedEvent; > > > > > + > > > > > +typedef struct GEDState { > > > > > + MemoryRegion io; > > > > > + uint32_t sel; > > > > > > > > do we need to migrate this during migration? > > > > > > TBH, I don't know and currently this series doesn't address migration > > > as we don't have any VMStateDescription and friends. Is this something > > > we can sort later? > > > > It probably should be implemented, otherwise we will have to deal with hard to > > debug bug reports when users will try to migrate. > > > > Alternative hack could be that, enabling memory hotplug should put migration > > blocker, but that's probably the same effort as adding proper > > VMStateDescription > > Ok. I will take a look into VMStateDescription then. > > Thanks, > Shameer > > > > > > > Thanks, > > > Shameer > > > > > > > > + uint32_t irq; > > > > > + qemu_irq *gsi; > > > > > + QemuMutex lock; > > > > > +} GEDState; > > > > > + > > > > > + > > > > > +typedef struct AcpiGedState { > > > > > + DeviceClass parent_obj; > > > > > + MemHotplugState memhp_state; > > > > > + hwaddr memhp_base; > > > > > + void *gsi; > > > > > + hwaddr ged_base; > > > > > + GEDState ged_state; > > > > > + uint32_t ged_irq; > > > > > + void *ged_events; > > > > > + uint32_t ged_events_size; > > > > > +} AcpiGedState; > > > > > + > > > > > +void build_ged_aml(Aml *table, const char* name, HotplugHandler > > > > *hotplug_dev, > > > > > + uint32_t ged_irq, AmlRegionSpace rs); > > > > > + > > > > > +#endif > > > > >
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..856ca04 --- /dev/null +++ b/hw/acpi/generic_event_device.c @@ -0,0 +1,311 @@ +/* + * + * 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 "qapi/error.h" +#include "exec/address-spaces.h" +#include "hw/sysbus.h" +#include "hw/acpi/acpi.h" +#include "hw/acpi/generic_event_device.h" +#include "hw/mem/pc-dimm.h" + +static Aml *ged_event_aml(const GedEvent *event) +{ + + if (!event) { + return NULL; + } + + switch (event->event) { + case GED_MEMORY_HOTPLUG: + /* We run a complete memory SCAN when getting a memory hotplug event */ + return aml_call0(MEMORY_DEVICES_CONTAINER "." MEMORY_SLOT_SCAN_METHOD); + default: + break; + } + + return NULL; +} + +/* + * The ACPI Generic Event Device (GED) is a hardware-reduced specific + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, + * including the hotplug ones. Platforms need to specify their own + * GedEvent array to describe what kind of events they want to support + * through GED. This routine uses a single interrupt for the GED device, + * relying on IO memory region to communicate the type of device + * affected by the interrupt. This way, we can support up to 32 events + * with a unique interrupt. + */ +void build_ged_aml(Aml *table, const char *name, HotplugHandler *hotplug_dev, + uint32_t ged_irq, AmlRegionSpace rs) +{ + AcpiGedState *s = ACPI_GED(hotplug_dev); + GedEvent *ged_events = s->ged_events; + Aml *crs = aml_resource_template(); + Aml *evt, *field; + Aml *dev = aml_device("%s", name); + Aml *irq_sel = aml_local(0); + Aml *isel = aml_name(AML_GED_IRQ_SEL); + uint32_t i; + + if (!s->ged_base || !ged_events || !s->ged_events_size) { + return; + } + + /* _CRS interrupt */ + aml_append(crs, aml_interrupt(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH, + AML_EXCLUSIVE, &ged_irq, 1)); + /* + * For each GED event we: + * - Add an interrupt to the CRS section. + * - Add a conditional block for each event, inside a while loop. + * This is semantically equivalent to a switch/case implementation. + */ + evt = aml_method("_EVT", 1, AML_SERIALIZED); + { + Aml *ged_aml; + Aml *if_ctx; + + /* Local0 = ISEL */ + aml_append(evt, aml_store(isel, irq_sel)); + + /* + * Here we want to call a method for each supported GED event type. + * The resulting ASL code looks like: + * + * Local0 = ISEL + * If ((Local0 & irq0) == irq0) + * { + * MethodEvent0() + * } + * + * If ((Local0 & irq1) == irq1) + * { + * MethodEvent1() + * } + * ... + */ + + for (i = 0; i < s->ged_events_size; i++) { + ged_aml = ged_event_aml(&ged_events[i]); + if (!ged_aml) { + continue; + } + + /* If ((Local1 == irq))*/ + if_ctx = aml_if(aml_equal(aml_and(irq_sel, + aml_int(ged_events[i].selector), NULL), + aml_int(ged_events[i].selector))); + { + /* AML for this specific type of event */ + aml_append(if_ctx, ged_aml); + } + + /* + * We append the first "if" to the "while" context. + * Other "if"s will be "elseif"s. + */ + aml_append(evt, if_ctx); + } + } + + aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0013"))); + aml_append(dev, aml_name_decl("_UID", aml_string(GED_DEVICE))); + aml_append(dev, aml_name_decl("_CRS", crs)); + + /* Append IO region */ + aml_append(dev, aml_operation_region(AML_GED_IRQ_REG, rs, + aml_int(s->ged_base + ACPI_GED_IRQ_SEL_OFFSET), + ACPI_GED_IRQ_SEL_LEN)); + field = aml_field(AML_GED_IRQ_REG, AML_DWORD_ACC, AML_NOLOCK, + AML_WRITE_AS_ZEROS); + aml_append(field, aml_named_field(AML_GED_IRQ_SEL, + ACPI_GED_IRQ_SEL_LEN * 8)); + aml_append(dev, field); + + /* Append _EVT method */ + aml_append(dev, evt); + + aml_append(table, dev); +} + +/* Memory read by the GED _EVT AML dynamic method */ +static uint64_t ged_read(void *opaque, hwaddr addr, unsigned size) +{ + uint64_t val = 0; + GEDState *ged_st = opaque; + + switch (addr) { + case ACPI_GED_IRQ_SEL_OFFSET: + /* Read the selector value and reset it */ + qemu_mutex_lock(&ged_st->lock); + val = ged_st->sel; + ged_st->sel = 0; + qemu_mutex_unlock(&ged_st->lock); + break; + default: + break; + } + + return val; +} + +/* Nothing is expected to be written to the GED memory region */ +static void ged_write(void *opaque, hwaddr addr, uint64_t data, + unsigned int size) +{ +} + +static const MemoryRegionOps ged_ops = { + .read = ged_read, + .write = ged_write, + .endianness = DEVICE_LITTLE_ENDIAN, + .valid = { + .min_access_size = 4, + .max_access_size = 4, + }, +}; + +static void acpi_ged_event(GEDState *ged_st, uint32_t ged_irq_sel) +{ + /* + * Set the GED IRQ selector to the expected device type value. This + * way, the ACPI method will be able to trigger the right code based + * on a unique IRQ. + */ + qemu_mutex_lock(&ged_st->lock); + ged_st->sel = ged_irq_sel; + qemu_mutex_unlock(&ged_st->lock); + + /* Trigger the event by sending an interrupt to the guest. */ + qemu_irq_pulse(ged_st->gsi[ged_st->irq]); +} + +static void acpi_ged_init(MemoryRegion *as, DeviceState *dev, GEDState *ged_st) +{ + AcpiGedState *s = ACPI_GED(dev); + + assert(s->ged_base); + + ged_st->irq = s->ged_irq; + ged_st->gsi = s->gsi; + qemu_mutex_init(&ged_st->lock); + memory_region_init_io(&ged_st->io, OBJECT(dev), &ged_ops, ged_st, + "acpi-ged-event", ACPI_GED_REG_LEN); + memory_region_add_subregion(as, s->ged_base, &ged_st->io); +} + +static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) +{ + AcpiGedState *s = ACPI_GED(hotplug_dev); + + if (s->memhp_state.is_enabled && + object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { + acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, + dev, errp); + } else { + error_setg(errp, "virt: device plug request for unsupported device" + " type: %s", object_get_typename(OBJECT(dev))); + } +} + +static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev) +{ + AcpiGedState *s = ACPI_GED(adev); + uint32_t sel; + + if (ev & ACPI_MEMORY_HOTPLUG_STATUS) { + sel = ACPI_GED_IRQ_SEL_MEM; + } else { + /* Unknown event. Return without generating interrupt. */ + return; + } + + /* + * We inject the hotplug interrupt. The IRQ selector will make + * the difference from the ACPI table. + */ + acpi_ged_event(&s->ged_state, sel); +} + +static void acpi_ged_device_realize(DeviceState *dev, Error **errp) +{ + AcpiGedState *s = ACPI_GED(dev); + + if (s->memhp_state.is_enabled) { + acpi_memory_hotplug_init(get_system_memory(), OBJECT(dev), + &s->memhp_state, + s->memhp_base); + acpi_ged_init(get_system_memory(), dev, &s->ged_state); + } +} + +static Property acpi_ged_properties[] = { + /* + * Memory hotplug base address is a property of GED here, + * because GED handles memory hotplug event and MEMORY_HOTPLUG_DEVICE + * gets initialized when GED device is realized. + */ + DEFINE_PROP_UINT64("memhp-base", AcpiGedState, memhp_base, 0), + DEFINE_PROP_BOOL("memory-hotplug-support", AcpiGedState, + memhp_state.is_enabled, true), + DEFINE_PROP_PTR("gsi", AcpiGedState, gsi), + DEFINE_PROP_UINT64("ged-base", AcpiGedState, ged_base, 0), + DEFINE_PROP_UINT32("ged-irq", AcpiGedState, ged_irq, 0), + DEFINE_PROP_PTR("ged-events", AcpiGedState, ged_events), + DEFINE_PROP_UINT32("ged-events-size", AcpiGedState, ged_events_size, 0), + DEFINE_PROP_END_OF_LIST(), +}; + +static void acpi_ged_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 = acpi_ged_properties; + dc->realize = acpi_ged_device_realize; + + /* Reason: pointer properties "gsi" and "ged_events" */ + dc->user_creatable = false; + + hc->plug = acpi_ged_device_plug_cb; + + adevc->send_event = acpi_ged_send_event; +} + +static const TypeInfo acpi_ged_info = { + .name = TYPE_ACPI_GED, + .parent = TYPE_DEVICE, + .instance_size = sizeof(AcpiGedState), + .class_init = acpi_ged_class_init, + .interfaces = (InterfaceInfo[]) { + { TYPE_HOTPLUG_HANDLER }, + { TYPE_ACPI_DEVICE_IF }, + { } + } +}; + +static void acpi_ged_register_types(void) +{ + type_register_static(&acpi_ged_info); +} + +type_init(acpi_ged_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..9c840d8 --- /dev/null +++ b/include/hw/acpi/generic_event_device.h @@ -0,0 +1,121 @@ +/* + * + * 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/>. + * + * The ACPI Generic Event Device (GED) is a hardware-reduced specific + * device[ACPI v6.1 Section 5.6.9] that handles all platform events, + * including the hotplug ones. Generic Event Device allows platforms + * to handle interrupts in ACPI ASL statements. It follows a very + * similar approach like the _EVT method from GPIO events. All + * interrupts are listed in _CRS and the handler is written in _EVT + * method. Here, we use a single interrupt for the GED device, relying + * on IO memory region to communicate the type of device affected by + * the interrupt. This way, we can support up to 32 events with a + * unique interrupt. + * + * Here is an example. + * + * Device (\_SB.GED) + * { + * Name (_HID, "ACPI0013") + * Name (_UID, Zero) + * Name (_CRS, ResourceTemplate () + * { + * Interrupt (ResourceConsumer, Edge, ActiveHigh, Exclusive, ,, ) + * { + * 0x00000029, + * } + * }) + * OperationRegion (IREG, SystemMemory, 0x09080000, 0x04) + * Field (IREG, DWordAcc, NoLock, WriteAsZeros) + * { + * ISEL, 32 + * } + * + * Method (_EVT, 1, Serialized) // _EVT: Event + * { + * Local0 = ISEL // ISEL = IO memory region which specifies the + * // device type. + * If (((Local0 & irq0) == irq0)) + * { + * MethodEvent0() + * } + * ElseIf ((Local0 & irq1) == irq1) + * { + * MethodEvent1() + * } + * ... + * } + * } + * + */ + +#ifndef HW_ACPI_GED_H +#define HW_ACPI_GED_H + +#include "hw/acpi/memory_hotplug.h" + +#define TYPE_ACPI_GED "acpi-ged" +#define ACPI_GED(obj) \ + OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED) + +#define ACPI_GED_IRQ_SEL_OFFSET 0x0 +#define ACPI_GED_IRQ_SEL_LEN 0x4 +#define ACPI_GED_IRQ_SEL_MEM 0x1 +#define ACPI_GED_REG_LEN 0x4 + +#define GED_DEVICE "GED" +#define AML_GED_IRQ_REG "IREG" +#define AML_GED_IRQ_SEL "ISEL" + +typedef enum { + GED_MEMORY_HOTPLUG = 1, +} GedEventType; + +/* + * Platforms need to specify their own GedEvent array + * to describe what kind of events they want to support + * through GED. + */ +typedef struct GedEvent { + uint32_t selector; + GedEventType event; +} GedEvent; + +typedef struct GEDState { + MemoryRegion io; + uint32_t sel; + uint32_t irq; + qemu_irq *gsi; + QemuMutex lock; +} GEDState; + + +typedef struct AcpiGedState { + DeviceClass parent_obj; + MemHotplugState memhp_state; + hwaddr memhp_base; + void *gsi; + hwaddr ged_base; + GEDState ged_state; + uint32_t ged_irq; + void *ged_events; + uint32_t ged_events_size; +} AcpiGedState; + +void build_ged_aml(Aml *table, const char* name, HotplugHandler *hotplug_dev, + uint32_t ged_irq, AmlRegionSpace rs); + +#endif