| Message ID | 20190321104745.28068-10-shameerali.kolothum.thodi@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | ARM virt: ACPI memory hotplug support | expand |
Hi Shameer, On 3/21/19 11:47 AM, Shameer Kolothum wrote: > From: Samuel Ortiz <sameo@linux.intel.com> > > The ACPI Generic Event Device (GED) is a hardware-reduced specific > device 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 is in preparation for making use of GED for ARM/virt > platform and for now supports only memory hotplug. Personally I would squash this with PATCH 3. > > 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/generic_event_device.c | 200 +++++++++++++++++++++++++++++++++ > include/hw/acpi/generic_event_device.h | 34 ++++++ > 2 files changed, 234 insertions(+) > > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c > index 0b32fc9..9deaa33 100644 > --- a/hw/acpi/generic_event_device.c > +++ b/hw/acpi/generic_event_device.c > @@ -23,6 +23,183 @@ > #include "hw/acpi/generic_event_device.h" > #include "hw/mem/pc-dimm.h" > > +static hwaddr ged_io_base; > +static GedEvent *ged_events; > +static uint32_t ged_events_size; > + > +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. During last review I asked the question herefater. I may have missed your answer but just in case. 5.6.9.1 says: " The platform declare its support for the GED, and query whether an OS supports it, via the _OSC method " Is it something done? > + */ > +void build_ged_aml(Aml *table, const char *name, uint32_t ged_irq, > + AmlRegionSpace rs) > +{ > + 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 (!ged_io_base || !ged_events || !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() > + * } I think we could have stopped here ;-) with a ../.. > + * > + * If ((Local0 & irq2) == irq2) > + * { > + * MethodEvent2() > + * } > + */ > + > + for (i = 0; i < 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))); doesn't check_patch complain here? > + { > + /* AML for this specific type of event */ > + aml_append(if_ctx, ged_aml); > + } > + > + /* > + * We append the first "if" to the "while" context. > + * Other "ifs" will be "elseifs". > + */ > + 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(ged_io_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]); I don't get this. The devices uses a single irq, right? Why can't we do like other sysbus devices, sysbus_init_irq(dev, &s->irq); and use sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic); in virt? > +} > + > static void virt_device_plug_cb(HotplugHandler *hotplug_dev, > DeviceState *dev, Error **errp) > { > @@ -40,6 +217,21 @@ static void virt_device_plug_cb(HotplugHandler *hotplug_dev, > > static void virt_send_ged(AcpiDeviceIf *adev, AcpiEventStatusBits ev) > { > + VirtAcpiState *s = VIRT_ACPI(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 virt_device_realize(DeviceState *dev, Error **errp) > @@ -57,6 +249,11 @@ static Property virt_acpi_properties[] = { > DEFINE_PROP_UINT64("memhp_base", VirtAcpiState, memhp_base, 0), > DEFINE_PROP_BOOL("memory-hotplug-support", VirtAcpiState, > memhp_state.is_enabled, true), It may be worth to explain why the GED device owns the MEMORY_HOTPLUG_MMIO region. This was and still is confusing me I acknowledge. > + DEFINE_PROP_PTR("gsi", VirtAcpiState, gsi), see the comment abour irq above. > + DEFINE_PROP_UINT64("ged_base", VirtAcpiState, ged_base, 0), > + DEFINE_PROP_UINT32("ged_irq", VirtAcpiState, ged_irq, 0), > + DEFINE_PROP_PTR("ged_events", VirtAcpiState, ged_events), > + DEFINE_PROP_UINT32("ged_events_size", VirtAcpiState, ged_events_size, 0), > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -70,6 +267,9 @@ static void virt_acpi_class_init(ObjectClass *class, void *data) > dc->props = virt_acpi_properties; > dc->realize = virt_device_realize; > > + /* Reason: pointer properties "gsi" and "gde_events" */ ged_events > + dc->user_creatable = false; > + > hc->plug = virt_device_plug_cb; > > adevc->send_event = virt_send_ged; > diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h > index 262ca7d..7f130f3 100644 > --- a/include/hw/acpi/generic_event_device.h > +++ b/include/hw/acpi/generic_event_device.h > @@ -24,11 +24,45 @@ > #define VIRT_ACPI(obj) \ > OBJECT_CHECK(VirtAcpiState, (obj), TYPE_VIRT_ACPI) > > +#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; > + > +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 VirtAcpiState { > SysBusDevice 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; > } VirtAcpiState; > > +void build_ged_aml(Aml *table, const char* name, uint32_t ged_irq, > + AmlRegionSpace rs); > > #endif > Thanks Eric
Hi Eric, > -----Original Message----- > From: Auger Eric [mailto:eric.auger@redhat.com] > Sent: 29 March 2019 13:09 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; > qemu-devel@nongnu.org; qemu-arm@nongnu.org; imammedo@redhat.com; > peter.maydell@linaro.org; shannon.zhaosl@gmail.com; > sameo@linux.intel.com; sebastien.boeuf@intel.com > Cc: Linuxarm <linuxarm@huawei.com>; xuwei (O) <xuwei5@huawei.com> > Subject: Re: [PATCH v3 09/10] hw/acpi: Add ACPI Generic Event Device Support > > Hi Shameer, > > On 3/21/19 11:47 AM, Shameer Kolothum wrote: > > From: Samuel Ortiz <sameo@linux.intel.com> > > > > The ACPI Generic Event Device (GED) is a hardware-reduced specific > > device 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 is in preparation for making use of GED for ARM/virt > > platform and for now supports only memory hotplug. > > Personally I would squash this with PATCH 3. Ok. > > 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/generic_event_device.c | 200 > +++++++++++++++++++++++++++++++++ > > include/hw/acpi/generic_event_device.h | 34 ++++++ > > 2 files changed, 234 insertions(+) > > > > diff --git a/hw/acpi/generic_event_device.c > b/hw/acpi/generic_event_device.c > > index 0b32fc9..9deaa33 100644 > > --- a/hw/acpi/generic_event_device.c > > +++ b/hw/acpi/generic_event_device.c > > @@ -23,6 +23,183 @@ > > #include "hw/acpi/generic_event_device.h" > > #include "hw/mem/pc-dimm.h" > > > > +static hwaddr ged_io_base; > > +static GedEvent *ged_events; > > +static uint32_t ged_events_size; > > + > > +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. > > During last review I asked the question herefater. I may have missed > your answer but just in case. > > 5.6.9.1 says: > " > The platform declare its support for the GED, and query whether an OS > supports it, via the _OSC method > " > Is it something done? Yes you did raise this earlier and I had replied as well :). https://patchwork.kernel.org/patch/10844557/ " Right. _OSC is not defined and I don’t see Qemu ever defined platform wide OSPM capabilities. I can see that it does that for PCI/PCIe hierarchies. Moreover looking at the Linux code, it doesn’t seems to care about GED definition either, https://elixir.bootlin.com/linux/v5.0/source/include/linux/acpi.h#L490 I can try adding _OSC declaring just the GED bit for future, but not sure It makes much difference as of now. Please let me know if there is a strong argument for it. " > > + */ > > +void build_ged_aml(Aml *table, const char *name, uint32_t ged_irq, > > + AmlRegionSpace rs) > > +{ > > + 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 (!ged_io_base || !ged_events || !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() > > + * } > I think we could have stopped here ;-) with a ../.. Ok > > + * > > + * If ((Local0 & irq2) == irq2) > > + * { > > + * MethodEvent2() > > + * } > > + */ > > + > > + for (i = 0; i < 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))); > doesn't check_patch complain here? It did warn but thought this has better readability. I will address this. > > + { > > + /* AML for this specific type of event */ > > + aml_append(if_ctx, ged_aml); > > + } > > + > > + /* > > + * We append the first "if" to the "while" context. > > + * Other "ifs" will be "elseifs". > > + */ > > + 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(ged_io_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]); > I don't get this. The devices uses a single irq, right? Yes it uses single irq. > Why can't we do like other sysbus devices, sysbus_init_irq(dev, &s->irq); > and use > sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic); in virt? I have to take a look at this. > > +} > > + > > static void virt_device_plug_cb(HotplugHandler *hotplug_dev, > > DeviceState *dev, Error **errp) > > { > > @@ -40,6 +217,21 @@ static void virt_device_plug_cb(HotplugHandler > *hotplug_dev, > > > > static void virt_send_ged(AcpiDeviceIf *adev, AcpiEventStatusBits ev) > > { > > + VirtAcpiState *s = VIRT_ACPI(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 virt_device_realize(DeviceState *dev, Error **errp) > > @@ -57,6 +249,11 @@ static Property virt_acpi_properties[] = { > > DEFINE_PROP_UINT64("memhp_base", VirtAcpiState, memhp_base, > 0), > > DEFINE_PROP_BOOL("memory-hotplug-support", VirtAcpiState, > > memhp_state.is_enabled, true), > It may be worth to explain why the GED device owns the > MEMORY_HOTPLUG_MMIO region. This was and still is confusing me I > acknowledge. I will try to add the comment here saying this is the hotplug handler/ acpi interface device and needs to initialize the memory hotplug base region. > > + DEFINE_PROP_PTR("gsi", VirtAcpiState, gsi), > see the comment abour irq above. > > + DEFINE_PROP_UINT64("ged_base", VirtAcpiState, ged_base, 0), > > + DEFINE_PROP_UINT32("ged_irq", VirtAcpiState, ged_irq, 0), > > + DEFINE_PROP_PTR("ged_events", VirtAcpiState, ged_events), > > + DEFINE_PROP_UINT32("ged_events_size", VirtAcpiState, > ged_events_size, 0), > > DEFINE_PROP_END_OF_LIST(), > > }; > > > > @@ -70,6 +267,9 @@ static void virt_acpi_class_init(ObjectClass *class, > void *data) > > dc->props = virt_acpi_properties; > > dc->realize = virt_device_realize; > > > > + /* Reason: pointer properties "gsi" and "gde_events" */ > ged_events Thanks, Shameer > > + dc->user_creatable = false; > > + > > hc->plug = virt_device_plug_cb; > > > > adevc->send_event = virt_send_ged; > > diff --git a/include/hw/acpi/generic_event_device.h > b/include/hw/acpi/generic_event_device.h > > index 262ca7d..7f130f3 100644 > > --- a/include/hw/acpi/generic_event_device.h > > +++ b/include/hw/acpi/generic_event_device.h > > @@ -24,11 +24,45 @@ > > #define VIRT_ACPI(obj) \ > > OBJECT_CHECK(VirtAcpiState, (obj), TYPE_VIRT_ACPI) > > > > +#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; > > + > > +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 VirtAcpiState { > > SysBusDevice 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; > > } VirtAcpiState; > > > > +void build_ged_aml(Aml *table, const char* name, uint32_t ged_irq, > > + AmlRegionSpace rs); > > > > #endif > > > > Thanks > > Eric
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c index 0b32fc9..9deaa33 100644 --- a/hw/acpi/generic_event_device.c +++ b/hw/acpi/generic_event_device.c @@ -23,6 +23,183 @@ #include "hw/acpi/generic_event_device.h" #include "hw/mem/pc-dimm.h" +static hwaddr ged_io_base; +static GedEvent *ged_events; +static uint32_t ged_events_size; + +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, uint32_t ged_irq, + AmlRegionSpace rs) +{ + 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 (!ged_io_base || !ged_events || !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() + * } + * + * If ((Local0 & irq2) == irq2) + * { + * MethodEvent2() + * } + */ + + for (i = 0; i < 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 "ifs" will be "elseifs". + */ + 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(ged_io_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 virt_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { @@ -40,6 +217,21 @@ static void virt_device_plug_cb(HotplugHandler *hotplug_dev, static void virt_send_ged(AcpiDeviceIf *adev, AcpiEventStatusBits ev) { + VirtAcpiState *s = VIRT_ACPI(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 virt_device_realize(DeviceState *dev, Error **errp) @@ -57,6 +249,11 @@ static Property virt_acpi_properties[] = { DEFINE_PROP_UINT64("memhp_base", VirtAcpiState, memhp_base, 0), DEFINE_PROP_BOOL("memory-hotplug-support", VirtAcpiState, memhp_state.is_enabled, true), + DEFINE_PROP_PTR("gsi", VirtAcpiState, gsi), + DEFINE_PROP_UINT64("ged_base", VirtAcpiState, ged_base, 0), + DEFINE_PROP_UINT32("ged_irq", VirtAcpiState, ged_irq, 0), + DEFINE_PROP_PTR("ged_events", VirtAcpiState, ged_events), + DEFINE_PROP_UINT32("ged_events_size", VirtAcpiState, ged_events_size, 0), DEFINE_PROP_END_OF_LIST(), }; @@ -70,6 +267,9 @@ static void virt_acpi_class_init(ObjectClass *class, void *data) dc->props = virt_acpi_properties; dc->realize = virt_device_realize; + /* Reason: pointer properties "gsi" and "gde_events" */ + dc->user_creatable = false; + hc->plug = virt_device_plug_cb; adevc->send_event = virt_send_ged; diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h index 262ca7d..7f130f3 100644 --- a/include/hw/acpi/generic_event_device.h +++ b/include/hw/acpi/generic_event_device.h @@ -24,11 +24,45 @@ #define VIRT_ACPI(obj) \ OBJECT_CHECK(VirtAcpiState, (obj), TYPE_VIRT_ACPI) +#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; + +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 VirtAcpiState { SysBusDevice 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; } VirtAcpiState; +void build_ged_aml(Aml *table, const char* name, uint32_t ged_irq, + AmlRegionSpace rs); #endif