[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH] hw/arm/Kconfig: no need to enable ACPI_MEMORY_HOTPLUG explic
From: |
Shameerali Kolothum Thodi |
Subject: |
RE: [PATCH] hw/arm/Kconfig: no need to enable ACPI_MEMORY_HOTPLUG explicitly |
Date: |
Thu, 19 Aug 2021 15:21:37 +0000 |
> -----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