On Sat, 4 Sept 2021 at 22:36, Michael S. Tsirkin <m...@redhat.com> wrote: > > From: Ani Sinha <a...@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 > > Signed-off-by: Ani Sinha <a...@anisinha.ca> > Message-Id: <20210812071409.492299-1-...@anisinha.ca> > Reviewed-by: Michael S. Tsirkin <m...@redhat.com> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
Hi; I'm trying to track down the fix for a bug that I think was introduced by this change. Specifically, if you configure with a target list of '--target-list=mips-linux-user,mips64-linux-user,mipsel-linux-user,mipsn32-linux-user,mipsn32el-linux-user,mips-softmmu,mipsel-softmmu,mips64-softmmu,mips64el-softmmu' (ie "just mips"), then run 'make check', the iotest 267 fails because QEMU segfaults trying to do a VM save/restore on qemu-system-mips. (You can run just that iotest by cd'ing into the build dir's tests/qemu-iotests/ subdir and running ./check -qcow2 -gdb 267 if you like). This is because hw/acpi/piix4.c (used by the MIPS malta board) has a vmstate that includes use of the VMSTATE_PCI_HOTPLUG() macro. This macro uses the vmstate_acpi_pcihp_pci_status vmstate struct. If the MIPS target is built along with some other targets which require CONFIG_ACPI_PCIHP then we correctly get the real definition of that vmstate struct from pcihp.c. However, if we are only building the MIPS targets then CONFIG_ACPI_PCIHP is false, and we get the dummy definition of the struct from acpi-pci-hotplug-stub.c. The dummy definition obviously doesn't actually work for migrating anything, and in fact the migration code ends up segfaulting because the 'name' field in the struct is NULL. (MIPS builds and uses hw/acpi/piix4.c because configs/devices/mips-softmmu/common.mak sets CONFIG_ACPI_PIIX4=y, and it needs this because piix4_init() unconditionally creates a TYPE_PIIX4_PM device. It's possible this is a bug revealed/introduced by the recent piix4 refactoring, but I had a look at the code at the time this change was committed and afaict back then it also created the PIIX4_PM device, just in a different place. Indeed it is this commit which adds the CONFIG_ACPI_PIIX4=y to the config!) How is this intended to work? The obvious fix from my point of view would just be to say "piix4.c requires pcihp.c" and cause CONFIG_ACPI_PIIX4 to pull in CONFIG_ACPI_PCIHP, but that seems like it would be rather undoing the point of this change. But if Malta requires ACPI_PIIX4 and it creates the PIIX4_PM device, it needs the real pcihp.c and not the stubs, doesn't it ? thanks -- PMM
