Re: [PATCH-for-5.0 1/2] hw/acpi/piix4: Add 'system-hotplug-support' property

[Igor Mammedov]

On Wed, 18 Mar 2020 23:15:30 +0100
Philippe Mathieu-Daudé <address@hidden> wrote:

> The I/O ranges registered by the piix4_acpi_system_hot_add_init()
> function are not documented in the PIIX4 datasheet.
> This appears to be a PC-only feature added in commit 5e3cb5347e
> ("initialize hot add system / acpi gpe") which was then moved
> to the PIIX4 device model in commit 9d5e77a22f ("make
> qemu_system_device_hot_add piix independent")
> Add a property (default enabled, to not modify the current
> behavior) to allow machines wanting to model a simple PIIX4
> to disable this feature.
> 
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>

it's already pretty twisted code and adding one more knob
to workaround other compat knobs makes it worse.

Even though it's not really welcomed approach,
we can ifdef all hotplug parts and compile them out for mips
dropping along the way linking with not needed dependencies
or
more often used, make stubs from hotplug parts for mips
and link with them.

> ---
> Should I squash this with the next patch and start with
> default=false, which is closer to the hardware model?
> ---
>  hw/acpi/piix4.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 964d6f5990..9c970336ac 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -78,6 +78,7 @@ typedef struct PIIX4PMState {
>  
>      AcpiPciHpState acpi_pci_hotplug;
>      bool use_acpi_pci_hotplug;
> +    bool use_acpi_system_hotplug;
>  
>      uint8_t disable_s3;
>      uint8_t disable_s4;
> @@ -503,8 +504,10 @@ static void piix4_pm_realize(PCIDevice *dev, Error 
> **errp)
>      s->machine_ready.notify = piix4_pm_machine_ready;
>      qemu_add_machine_init_done_notifier(&s->machine_ready);
>  
> -    piix4_acpi_system_hot_add_init(pci_address_space_io(dev),
> -                                   pci_get_bus(dev), s);
> +    if (s->use_acpi_system_hotplug) {
> +        piix4_acpi_system_hot_add_init(pci_address_space_io(dev),
> +                                       pci_get_bus(dev), s);
> +    }
>      qbus_set_hotplug_handler(BUS(pci_get_bus(dev)), OBJECT(s), &error_abort);
>  
>      piix4_pm_add_propeties(s);
> @@ -635,6 +638,8 @@ static Property piix4_pm_properties[] = {
>                       use_acpi_pci_hotplug, true),
>      DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
>                       acpi_memory_hotplug.is_enabled, true),
> +    DEFINE_PROP_BOOL("system-hotplug-support", PIIX4PMState,
> +                     use_acpi_system_hotplug, true),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>