Re: [Qemu-devel] [PATCH-for-4.2 v9 01/12] hw/acpi: Make ACPI IO address space configurable

[Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>]

> -----Original Message-----
> From: Igor Mammedov [mailto:imammedo@redhat.com]
> Sent: 29 August 2019 13:38
> 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; sameo@linux.intel.com;
> ard.biesheuvel@linaro.org; Linuxarm <linuxarm@huawei.com>;
> shannon.zhaosl@gmail.com; sebastien.boeuf@intel.com; lersek@redhat.com
> Subject: Re: [PATCH-for-4.2 v9 01/12] hw/acpi: Make ACPI IO address space
> configurable
> 
> On Thu, 29 Aug 2019 11:04:27 +0000
> Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote:

[...]

> >
> > I think what happens is since we are now passing the memhp_io_base
> directly into the
> > build_memory_hotplug_aml() and removed the "static uint16_t
> memhp_io_base", on
> > x86, memory hotplug aml code is always built by default irrespective of
> whether
> > acpi_memory_hotplug_init() is invoked or not.
> >
> > I could either reintroduce a check in build_memory_hotplug_aml() to make
> sure
> > acpi_memory_hotplug_init() is called, or could do something like below,

> fix looks fine to me, see minor comment below

Ok
 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 3995f9a40f..17756c2191 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1873,9 +1873,12 @@ build_dsdt(GArray *table_data, BIOSLinker
> *linker,
> >          build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
> >                         "\\_SB.PCI0", "\\_GPE._E02");
> >      }
> > -    build_memory_hotplug_aml(dsdt, nr_mem, "\\_SB.PCI0",
> > -                             "\\_GPE._E03", AML_SYSTEM_IO,
> > -                             pcms->memhp_io_base);
> > +
> > +    if (acpi_enabled && pcms->acpi_dev && nr_mem) {
> double-check call path and see if
>   acpi_enabled && pcms->acpi_dev
> is really necessary

Right, looks like those are always true. I will remove those.

Also appreciate if you could take a look at rest of the series and then I can
re-spin along with this.

Thanks,
Shameer