On Thu, 5 Aug 2021, Igor Mammedov wrote:
> On Mon, 26 Jul 2021 22:27:43 +0530
> Ani Sinha <a...@anisinha.ca> wrote:
>
> > All existing code using acpi_get_i386_pci_host() checks for a non-null
> > return value from this function call. Instead of returning early when the
> > value
> > returned is NULL, assert instead. Since there are only two possible host
> > buses
> > for i386 - q35 and i440fx, a null value return from the function does not
> > make
> > sense in most cases and is likely an error situation.
> >
> > Fixes: c0e427d6eb5fef ("hw/acpi/ich9: Enable ACPI PCI hot-plug")
> >
> > Signed-off-by: Ani Sinha <a...@anisinha.ca>
> > ---
> > hw/acpi/pcihp.c | 8 ++++++++
> > hw/i386/acpi-build.c | 15 ++++++---------
> > 2 files changed, 14 insertions(+), 9 deletions(-)
> >
> > changelog:
> > v1: initial patch
> > v2: removed comment addition - that can be sent as a separate patch.
> > v3: added assertion for null host values for all cases except one.
> >
> > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> > index f4d706e47d..054ee8cbc5 100644
> > --- a/hw/acpi/pcihp.c
> > +++ b/hw/acpi/pcihp.c
> > @@ -116,6 +116,12 @@ static void acpi_set_pci_info(void)
> > bsel_is_set = true;
> >
> > if (!host) {
> > + /*
> > + * This function can be eventually called from
> > + * qemu_devices_reset() -> acpi_pcihp_reset() even
> > + * for architectures other than i386. Hence, we need
> > + * to ignore null values for host here.
> > + */
> > return;
> > }
>
> I suspect it's a MIPS target that call this code unnecessarily.
> It would be better to get rid of this condition altogether.
> Frr that I can suggest to make acpi_pcihp_reset() stub and
> replace pcihp.c with stub (perhaps use acpi-x86-stub.c) when building
> for MIPS.
>
> then a bunch of asserts/ifs won't be necessary,
> just one in acpi_get_i386_pci_host() will be sufficient.
>
OK this is a good idea.
I can see that mips-softmmu-config-devices.h has
CONFIG_ACPI_X86 turned on for mips. This does not seem right.
The issue here is:
$ grep -R CONFIG_ACPI_X86 *
devices/mips-softmmu/common.mak:CONFIG_ACPI_X86=y
So after
-CONFIG_ACPI_X86=y
-CONFIG_PIIX4=y
(the second one is needed because after removing first one we get:
/usr/bin/ld: libcommon.fa.p/hw_isa_piix4.c.o: in function `piix4_create':
/home/anisinha/workspace/qemu/build/../hw/isa/piix4.c:269: undefined
reference to `piix4_pm_init'
This is because in hw/acpi/meson.build, piix4.c is conditional on
CONFIG_ACPI_X86. )
/usr/bin/ld: libqemu-mips-softmmu.fa.p/hw_mips_gt64xxx_pci.c.o: in
function `gt64120_pci_set_irq':
/home/anisinha/workspace/qemu/build/../hw/mips/gt64xxx_pci.c:1020:
undefined reference to `piix4_dev'
/usr/bin/ld: libqemu-mips-softmmu.fa.p/hw_mips_malta.c.o: in function
`mips_malta_init':
/home/anisinha/workspace/qemu/build/../hw/mips/malta.c:1404: undefined
reference to `piix4_create'
So should mips be doing piix stuff anyway? Is Piix4 etc not x86 specific?
How to handle this?
