Re: [PULL 06/35] hw/acpi: refactor acpi hp modules so that targets can just use what they need

From: Ani Sinha <ani@anisinha.ca>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	qemu-devel@nongnu.org, Ani Sinha <ani@anisinha.ca>,
	Igor Mammedov <imammedo@redhat.com>,
	Aurelien Jarno <aurelien@aurel32.net>
Subject: Re: [PULL 06/35] hw/acpi: refactor acpi hp modules so that targets can just use what they need
Date: Mon, 6 Sep 2021 15:33:31 +0530 (IST)	[thread overview]
Message-ID: <alpine.DEB.2.22.394.2109061531570.2070064@anisinha-lenovo> (raw)
In-Reply-To: <6a9c2fde-317d-bd31-5a8c-135446b881dc@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 3102 bytes --]

On Mon, 6 Sep 2021, Philippe Mathieu-Daudé wrote:

> Hi Ani,
>
> On 9/4/21 11:36 PM, Michael S. Tsirkin wrote:
> > From: Ani Sinha <ani@anisinha.ca>
> >
> > Currently various acpi hotplug modules like cpu hotplug, memory hotplug, pci
> > hotplug, nvdimm hotplug are all pulled in when CONFIG_ACPI_X86 is turned on.
> > This brings in support for whole lot of subsystems that some targets like
> > mips does not need. They are added just to satisfy symbol dependencies. This
> > is ugly and should be avoided. Targets should be able to pull in just what they
> > need and no more. For example, mips only needs support for PIIX4 and does not
> > need acpi pci hotplug support or cpu hotplug support or memory hotplug support
> > etc. This change is an effort to clean this up.
> > In this change, new config variables are added for various acpi hotplug
> > subsystems. Targets like mips can only enable PIIX4 support and not the rest
> > of all the other modules which were being previously pulled in as a part of
> > CONFIG_ACPI_X86. Function stubs make sure that symbols which piix4 needs but
> > are not required by mips (for example, symbols specific to pci hotplug etc)
> > are available to satisfy the dependencies.
> >
> > Currently, this change only addresses issues with mips malta targets. In future
> > we might be able to clean up other targets which are similarly pulling in lot
> > of unnecessary hotplug modules by enabling ACPI_X86.
> >
> > This change should also address issues such as the following:
> > https://gitlab.com/qemu-project/qemu/-/issues/221
> > https://gitlab.com/qemu-project/qemu/-/issues/193
>
> FYI per https://docs.gitlab.com/ee/administration/issue_closing_pattern.html
> this should have been:
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/193
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/221
>

Ah my apologies. Will do this next time.

> Can we close these issues manually?

Since both you and I have verified that those issues gets fixed with my
change, yes we can close them. I do not have a gitlab account. Should I
have one? Is there special permissions needed to handle these tickets?

>
> > Signed-off-by: Ani Sinha <ani@anisinha.ca>
> > Message-Id: <20210812071409.492299-1-ani@anisinha.ca>
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  configs/devices/mips-softmmu/common.mak |  5 +--
> >  hw/acpi/acpi-cpu-hotplug-stub.c         | 50 +++++++++++++++++++++++++
> >  hw/acpi/acpi-mem-hotplug-stub.c         | 35 +++++++++++++++++
> >  hw/acpi/acpi-nvdimm-stub.c              |  8 ++++
> >  hw/acpi/acpi-pci-hotplug-stub.c         | 47 +++++++++++++++++++++++
> >  hw/acpi/Kconfig                         | 10 +++++
> >  hw/acpi/meson.build                     | 14 +++++--
> >  7 files changed, 161 insertions(+), 8 deletions(-)
> >  create mode 100644 hw/acpi/acpi-cpu-hotplug-stub.c
> >  create mode 100644 hw/acpi/acpi-mem-hotplug-stub.c
> >  create mode 100644 hw/acpi/acpi-nvdimm-stub.c
> >  create mode 100644 hw/acpi/acpi-pci-hotplug-stub.c
>
>