RE: [PATCH] hw/arm/Kconfig: no need to enable ACPI_MEMORY_HOTPLUG explicitly

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

> -----Original Message-----
> From: Philippe Mathieu-Daudé [mailto:philmd@redhat.com]
> Sent: 19 August 2021 15:50
> To: Ani Sinha <ani@anisinha.ca>
> Cc: Peter Maydell <peter.maydell@linaro.org>; QEMU Developers
> <qemu-devel@nongnu.org>; qemu-arm <qemu-arm@nongnu.org>; Michael S.
> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>;
> Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Subject: Re: [PATCH] hw/arm/Kconfig: no need to enable
> ACPI_MEMORY_HOTPLUG explicitly
> 
> Cc'ing Shameer Kolothum.
> 
> On 8/19/21 3:36 PM, Ani Sinha wrote:
> > On Thu, 19 Aug 2021, Ani Sinha wrote:
> >> On Thu, 19 Aug 2021, Peter Maydell wrote:
> >>> On Tue, 17 Aug 2021 at 05:45, Ani Sinha <ani@anisinha.ca> wrote:
> 
> >>> Is it intended that ACPI_HW_REDUCED must always imply
> >>> ACPI_MEMORY_HOTPLUG, or is it just a coincidence that the virt board
> >>> happens to want both, and so we select both ?
> 
> The ACPI dependency was missing (see commit 36b79e3219d,
> "hw/acpi/Kconfig: Add missing Kconfig dependencies (build error)", now we
> don't need it explicitly.

Yes. And it looks like ACPI_NVDIMM also can be removed now.

Regards,
Shameer

> >> From a purely code inspection point of view, I noticed that
> >> generic_event_device.c depends on CONFIG_ACPI_HW_REDUCED. The GED
> >> does use memory hotplug apis - for example acpi_ged_device_plug_cb()
> >> uses
> >> acpi_memory_plug_cb() etc.
> >>
> >> Hence, as it stands today, CONFIG_ACPI_HW_REDUCED will need to select
> >> ACPI memory hotplug. Unless we remove the GED device's dependence on
> >> ACPI_HW_REDUCED that is. I cannot comment whether that would be wise
> >> or if we should reorg the code in some other way.
> >
> > The other question we should ask is whether arm platform requires
> > ACPI_MEMORY_HOTPLUG independent of ACPI_HW_REDUCED/GED device?
> If that
> > is the case, then maybe we should keep that config option as is.
> > Maybe @qemu-arm can answer that?
> 
> Or git-log:
> 
> commit cff51ac978c4fa0b3d0de0fd62d772d9003f123e
> Author: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Date:   Wed Sep 18 14:06:27 2019 +0100
> 
>     hw/arm/virt: Enable device memory cold/hot plug with ACPI boot
> 
>     This initializes the GED device with base memory and irq, configures
>     ged memory hotplug event and builds the corresponding aml code. With
>     this, both hot and cold plug of device memory is enabled now for
>     Guest with ACPI boot. Memory cold plug support with Guest DT boot is
>     not yet supported.
> 
> >>>> On Thu, 12 Aug 2021, Ani Sinha wrote:
> >>>>
> 
> Please prepend here 'Since commit 36b79e3219d ("hw/acpi/Kconfig: Add
> missing Kconfig dependencies"),'
> 
> With it:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> >>>>> ACPI_MEMORY_HOTPLUG is implicitly turned on when
> ACPI_HW_REDUCED is selected.
> >>>>> ACPI_HW_REDUCED is already enabled. No need to turn on
> >>>>> ACPI_MEMORY_HOTPLUG explicitly. This is a minor cleanup.
> >>>>>
> >>>>> Signed-off-by: Ani Sinha <ani@anisinha.ca>
> >>>>> ---
> >>>>>  hw/arm/Kconfig | 1 -
> >>>>>  1 file changed, 1 deletion(-)
> >>>>>
> >>>>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index
> >>>>> 4ba0aca067..38cf9f44e2 100644
> >>>>> --- a/hw/arm/Kconfig
> >>>>> +++ b/hw/arm/Kconfig
> >>>>> @@ -25,7 +25,6 @@ config ARM_VIRT
> >>>>>      select ACPI_PCI
> >>>>>      select MEM_DEVICE
> >>>>>      select DIMM
> >>>>> -    select ACPI_MEMORY_HOTPLUG
> >>>>>      select ACPI_HW_REDUCED
> >>>>>      select ACPI_NVDIMM
> >>>>>      select ACPI_APEI
> >>>>> --
> >>>>> 2.25.1