On Wed, 4 Sep 2019 09:56:23 +0100
Shameer Kolothum <shameerali.kolothum.th...@huawei.com> wrote:
[...]
> @@ -730,6 +733,19 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> vms->highmem, vms->highmem_ecam);
> acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
> (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
> + if (vms->acpi_dev) {
> + build_ged_aml(scope, "\\_SB."GED_DEVICE,
> + HOTPLUG_HANDLER(vms->acpi_dev),
> + irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE,
> AML_SYSTEM_MEMORY,
> + memmap[VIRT_ACPI_GED].base);
> + }
> +
> + if (vms->acpi_dev && ms->ram_slots) {
> + build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB", NULL,
> + AML_SYSTEM_MEMORY,
> + memmap[VIRT_PCDIMM_ACPI].base);
> + }
one more thing (though non critical), if ms->ram_slots == 0 ^^^^
makes IASL spew a warning
External (_SB_.MHPC.MSCN, MethodObj) // Warning: Unknown method,
guessing 0 arguments
In general non-existing references within methods are fine if they aren't ever
used.
however we can be a little bit less sloppy.
Below you advertise "event = ACPI_GED_MEM_HOTPLUG_EVT", and then here suddenly
don't generate essential AML part for it here.
For consistency if above is conditioned on ms->ram_slots != 0, probably
it would be better to move that condition where you set 'event' value and
check property value above instead of ms->ram_slots
[...]
> +static inline DeviceState *create_acpi_ged(VirtMachineState *vms, qemu_irq
> *pic)
> +{
> + DeviceState *dev;
> + int irq = vms->irqmap[VIRT_ACPI_GED];
> + uint32_t event = ACPI_GED_MEM_HOTPLUG_EVT;
> +
> + dev = qdev_create(NULL, TYPE_ACPI_GED);
> + qdev_prop_set_uint32(dev, "ged-event", event);
> + qdev_init_nofail(dev);
> +
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vms->memmap[VIRT_ACPI_GED].base);
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1,
> vms->memmap[VIRT_PCDIMM_ACPI].base);
> +
> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
> +
> + return dev;
> +}
> +
[...]
