On Thu, 19 Aug 2021, Philippe Mathieu-Daudé wrote:
> On 8/12/21 9:14 AM, Ani Sinha wrote: > > 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> > > --- > > configs/devices/mips-softmmu/common.mak | 5 +-- > > hw/acpi/Kconfig | 10 +++++ > > 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/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 > > > > diff --git a/configs/devices/mips-softmmu/common.mak > > b/configs/devices/mips-softmmu/common.mak > > index ea78fe7275..752b62b1e6 100644 > > --- a/configs/devices/mips-softmmu/common.mak > > +++ b/configs/devices/mips-softmmu/common.mak > > @@ -18,10 +18,7 @@ CONFIG_PCSPK=y > > CONFIG_PCKBD=y > > CONFIG_FDC=y > > CONFIG_ACPI=y > > -CONFIG_ACPI_X86=y > > -CONFIG_ACPI_MEMORY_HOTPLUG=y > > -CONFIG_ACPI_NVDIMM=y > > -CONFIG_ACPI_CPU_HOTPLUG=y > > +CONFIG_ACPI_PIIX4=y > > CONFIG_APM=y > > CONFIG_I8257=y > > CONFIG_PIIX4=y > > diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig > > index cfc4ede8d9..3b5e118c54 100644 > > --- a/hw/acpi/Kconfig > > +++ b/hw/acpi/Kconfig > > @@ -8,6 +8,8 @@ config ACPI_X86 > > select ACPI_CPU_HOTPLUG > > select ACPI_MEMORY_HOTPLUG > > select ACPI_HMAT > > + select ACPI_PIIX4 > > + select ACPI_PCIHP > > > > config ACPI_X86_ICH > > bool > > @@ -24,6 +26,14 @@ config ACPI_NVDIMM > > bool > > depends on ACPI > > > > +config ACPI_PIIX4 > > + bool > > + depends on ACPI > > + > > +config ACPI_PCIHP > > + bool > > + depends on ACPI > > + > > config ACPI_HMAT > > bool > > depends on ACPI > > diff --git a/hw/acpi/acpi-cpu-hotplug-stub.c > > b/hw/acpi/acpi-cpu-hotplug-stub.c > > new file mode 100644 > > index 0000000000..3fc4b14c26 > > --- /dev/null > > +++ b/hw/acpi/acpi-cpu-hotplug-stub.c > > @@ -0,0 +1,50 @@ > > +#include "qemu/osdep.h" > > +#include "hw/acpi/cpu_hotplug.h" > > +#include "migration/vmstate.h" > > + > > + > > +/* Following stubs are all related to ACPI cpu hotplug */ > > +const VMStateDescription vmstate_cpu_hotplug; > > + > > +void acpi_switch_to_modern_cphp(AcpiCpuHotplug *gpe_cpu, > > + CPUHotplugState *cpuhp_state, > > + uint16_t io_port) > > +{ > > + return; > > I suppose if you replace all 'return' by 'g_assert_not_reached()' > both issues reproducers crash? yes, I presume so. For example, with the following change : diff --git a/hw/acpi/acpi-cpu-hotplug-stub.c b/hw/acpi/acpi-cpu-hotplug-stub.c index 3fc4b14c26..9725e4a81b 100644 --- a/hw/acpi/acpi-cpu-hotplug-stub.c +++ b/hw/acpi/acpi-cpu-hotplug-stub.c @@ -16,7 +16,7 @@ void acpi_switch_to_modern_cphp(AcpiCpuHotplug *gpe_cpu, void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner, AcpiCpuHotplug *gpe_cpu, uint16_t base) { - return; + g_assert_not_reached(); } void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list) I get the following crash : ./qemu-system-mips -machine malta -bios /dev/null -nodefaults -monitor stdio -S QEMU 6.0.93 monitor - type 'help' for more information (qemu) ** ERROR:../hw/acpi/acpi-cpu-hotplug-stub.c:19:legacy_acpi_cpu_hotplug_init: code should not be reached Bail out! ERROR:../hw/acpi/acpi-cpu-hotplug-stub.c:19:legacy_acpi_cpu_hotplug_init: code should not be reached Aborted (core dumped)
