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

[Philippe Mathieu-Daudé]

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.

>> 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