Re: [Qemu-devel] [PATCH-for-4.2 v8 3/9] hw/acpi: Add ACPI Generic Event Device Support

[Igor Mammedov]

On Fri, 26 Jul 2019 11:45:13 +0100
Shameer Kolothum <address@hidden> wrote:

> From: Samuel Ortiz <address@hidden>
> 
> 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 GED Event bitmap 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.
> 

> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> new file mode 100644
> index 0000000000..7902e9d706
> --- /dev/null
> +++ b/hw/acpi/generic_event_device.c
[...]
> +void build_ged_aml(Aml *table, const char *name, HotplugHandler *hotplug_dev,
> +                   uint32_t ged_irq, AmlRegionSpace rs)
> +{
[...]
> +
> +        if (ged_events) {
> +            error_report("GED: Unsupported events specified");
> +            exit(1);
I'd use error_abort instead, since it's programing error, if you have to respin 
series.

> +        }
> +    }
> +
> +    /* Append _EVT method */
> +    aml_append(dev, evt);
> +
> +    aml_append(table, dev);
> +}
> +
[...]
> +static void acpi_ged_device_realize(DeviceState *dev, Error **errp)
> +{
> +    AcpiGedState *s = ACPI_GED(dev);
> +
> +    assert(s->ged_base);
> +    acpi_ged_init(get_system_memory(), dev, &s->ged_state);

calling get_system_memory() from device code used to be a reason for rejecting 
patch,
I'm not sure what suggest though. 

Maybe Paolo could suggest something.

> +
> +    if (s->memhp_state.is_enabled) {
> +        assert(s->memhp_base);
> +        acpi_memory_hotplug_init(get_system_memory(), OBJECT(dev),
                                     ^^^^
ditto

> +                                 &s->memhp_state,
> +                                 s->memhp_base);
> +    }
> +}
[...]

> +
> +#endif