| Message ID | 00c07067c1c8700bea48407cbec6d854e87de742.1616519655.git.isaku.yamahata@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | reinitialize ACPI PM device on reset | expand |
Hi Isaku, On 3/23/21 6:24 PM, Isaku Yamahata wrote: > Without this patch, the following patch will triger clan runtime > sanitizer warnings as follows. This patch proactively works around it. > I let v582c686.c maintainer address a correct fix as I'm not sure > about fuloong2e device model. > >> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} >> QTEST_QEMU_IMG=./qemu-img >> G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh >> QTEST_QEMU_BINARY=./qemu-system-mips64el tests/qtest/qom-test --tap -k >> PASS 1 qtest-mips64el/qom-test /mips64el/qom/loongson3-virt >> PASS 2 qtest-mips64el/qom-test /mips64el/qom/none >> PASS 3 qtest-mips64el/qom-test /mips64el/qom/magnum >> PASS 4 qtest-mips64el/qom-test /mips64el/qom/mipssim >> PASS 5 qtest-mips64el/qom-test /mips64el/qom/malta >> ../../hw/pci/pci.c:252:30: runtime error: shift exponent -1 is negative >> PASS 6 qtest-mips64el/qom-test /mips64el/qom/fuloong2e >> PASS 7 qtest-mips64el/qom-test /mips64el/qom/boston >> PASS 8 qtest-mips64el/qom-test /mips64el/qom/pica61 >> >> and similarly for eg >> >> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} >> QTEST_QEMU_IMG=./qemu-img >> G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh >> QTEST_QEMU_BINARY=./qemu-system-mips64el tests/qtest/endianness-test >> --tap -k >> ../../hw/pci/pci.c:252:30: runtime error: shift exponent -1 is negative >> PASS 1 qtest-mips64el/endianness-test /mips64el/endianness/fuloong2e >> ../../hw/pci/pci.c:252:30: runtime error: shift exponent -1 is negative >> PASS 2 qtest-mips64el/endianness-test /mips64el/endianness/split/fuloong2e >> ../../hw/pci/pci.c:252:30: runtime error: shift exponent -1 is negative >> PASS 3 qtest-mips64el/endianness-test /mips64el/endianness/combine/fuloong2e > > Cc: Huacai Chen <chenhuacai@kernel.org> > Cc: "Philippe Mathieu-Daudé" <f4bug@amsat.org> > Cc: Jiaxun Yang <jiaxun.yang@flygoat.com> > Reported-by: Peter Maydell <Peter.maydel@linaro.org> > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> > --- > hw/isa/vt82c686.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c > index 05d084f698..f0fb309f12 100644 > --- a/hw/isa/vt82c686.c > +++ b/hw/isa/vt82c686.c > @@ -144,7 +144,18 @@ static void pm_update_sci(ViaPMState *s) > ACPI_BITMASK_POWER_BUTTON_ENABLE | > ACPI_BITMASK_GLOBAL_LOCK_ENABLE | > ACPI_BITMASK_TIMER_ENABLE)) != 0); > - pci_set_irq(&s->dev, sci_level); > + if (pci_get_byte(s->dev.config + PCI_INTERRUPT_PIN)) { > + /* > + * FIXME: > + * Fix device model that realizes this PM device and remove > + * this work around. > + * The device model should wire SCI and setup > + * PCI_INTERRUPT_PIN properly. > + * If PIN# = 0(interrupt pin isn't used), don't raise SCI as > + * work around. > + */ > + pci_set_irq(&s->dev, sci_level); I'll defer this to Zoltan. Personally I wouldn't care about SCI_EN on the vt82c686, as it is not used by x86 machines (IOW, I'd not modify via_pm_reset and KISS). > + } > /* schedule a timer interruption if needed */ > acpi_pm_tmr_update(&s->ar, (s->ar.pm1.evt.en & ACPI_BITMASK_TIMER_ENABLE) && > !(pmsts & ACPI_BITMASK_TIMER_STATUS)); >
On Tue, 23 Mar 2021, Philippe Mathieu-Daudé wrote: > Hi Isaku, > > On 3/23/21 6:24 PM, Isaku Yamahata wrote: >> Without this patch, the following patch will triger clan runtime >> sanitizer warnings as follows. This patch proactively works around it. >> I let v582c686.c maintainer address a correct fix as I'm not sure >> about fuloong2e device model. >> >>> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} >>> QTEST_QEMU_IMG=./qemu-img >>> G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh >>> QTEST_QEMU_BINARY=./qemu-system-mips64el tests/qtest/qom-test --tap -k >>> PASS 1 qtest-mips64el/qom-test /mips64el/qom/loongson3-virt >>> PASS 2 qtest-mips64el/qom-test /mips64el/qom/none >>> PASS 3 qtest-mips64el/qom-test /mips64el/qom/magnum >>> PASS 4 qtest-mips64el/qom-test /mips64el/qom/mipssim >>> PASS 5 qtest-mips64el/qom-test /mips64el/qom/malta >>> ../../hw/pci/pci.c:252:30: runtime error: shift exponent -1 is negative >>> PASS 6 qtest-mips64el/qom-test /mips64el/qom/fuloong2e >>> PASS 7 qtest-mips64el/qom-test /mips64el/qom/boston >>> PASS 8 qtest-mips64el/qom-test /mips64el/qom/pica61 >>> >>> and similarly for eg >>> >>> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} >>> QTEST_QEMU_IMG=./qemu-img >>> G_TEST_DBUS_DAEMON=/home/petmay01/linaro/qemu-for-merges/tests/dbus-vmstate-daemon.sh >>> QTEST_QEMU_BINARY=./qemu-system-mips64el tests/qtest/endianness-test >>> --tap -k >>> ../../hw/pci/pci.c:252:30: runtime error: shift exponent -1 is negative >>> PASS 1 qtest-mips64el/endianness-test /mips64el/endianness/fuloong2e >>> ../../hw/pci/pci.c:252:30: runtime error: shift exponent -1 is negative >>> PASS 2 qtest-mips64el/endianness-test /mips64el/endianness/split/fuloong2e >>> ../../hw/pci/pci.c:252:30: runtime error: shift exponent -1 is negative >>> PASS 3 qtest-mips64el/endianness-test /mips64el/endianness/combine/fuloong2e >> >> Cc: Huacai Chen <chenhuacai@kernel.org> >> Cc: "Philippe Mathieu-Daudé" <f4bug@amsat.org> >> Cc: Jiaxun Yang <jiaxun.yang@flygoat.com> >> Reported-by: Peter Maydell <Peter.maydel@linaro.org> >> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> >> --- >> hw/isa/vt82c686.c | 13 ++++++++++++- >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c >> index 05d084f698..f0fb309f12 100644 >> --- a/hw/isa/vt82c686.c >> +++ b/hw/isa/vt82c686.c >> @@ -144,7 +144,18 @@ static void pm_update_sci(ViaPMState *s) >> ACPI_BITMASK_POWER_BUTTON_ENABLE | >> ACPI_BITMASK_GLOBAL_LOCK_ENABLE | >> ACPI_BITMASK_TIMER_ENABLE)) != 0); >> - pci_set_irq(&s->dev, sci_level); >> + if (pci_get_byte(s->dev.config + PCI_INTERRUPT_PIN)) { >> + /* >> + * FIXME: >> + * Fix device model that realizes this PM device and remove >> + * this work around. >> + * The device model should wire SCI and setup >> + * PCI_INTERRUPT_PIN properly. >> + * If PIN# = 0(interrupt pin isn't used), don't raise SCI as >> + * work around. >> + */ >> + pci_set_irq(&s->dev, sci_level); > > I'll defer this to Zoltan. I don't know anything about this, this was there well before I've touched this device model: https://git.qemu.org/?p=qemu.git;a=blame;f=hw/isa/vt82c686.c;hb=8063396bf3459a810d24e3efd6110b8480f0de5b > Personally I wouldn't care about SCI_EN on the vt82c686, as > it is not used by x86 machines (IOW, I'd not modify via_pm_reset > and KISS). I'm not sure but maybe then you could also just remove the PM parts from the device model as it probably does not work correctly anyway at the moment as it may not be correctly wired up to config registers. I'm not sure if it's needed by any guests but it was there for some reason and maybe better to fix it if possible than dropping it. As a workaround I'm OK with the proposed patch, I don't think it would break anything but haven't tested it. Regards, BALATON Zoltan >> + } >> /* schedule a timer interruption if needed */ >> acpi_pm_tmr_update(&s->ar, (s->ar.pm1.evt.en & ACPI_BITMASK_TIMER_ENABLE) && >> !(pmsts & ACPI_BITMASK_TIMER_STATUS)); >> > >
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c index 05d084f698..f0fb309f12 100644 --- a/hw/isa/vt82c686.c +++ b/hw/isa/vt82c686.c @@ -144,7 +144,18 @@ static void pm_update_sci(ViaPMState *s) ACPI_BITMASK_POWER_BUTTON_ENABLE | ACPI_BITMASK_GLOBAL_LOCK_ENABLE | ACPI_BITMASK_TIMER_ENABLE)) != 0); - pci_set_irq(&s->dev, sci_level); + if (pci_get_byte(s->dev.config + PCI_INTERRUPT_PIN)) { + /* + * FIXME: + * Fix device model that realizes this PM device and remove + * this work around. + * The device model should wire SCI and setup + * PCI_INTERRUPT_PIN properly. + * If PIN# = 0(interrupt pin isn't used), don't raise SCI as + * work around. + */ + pci_set_irq(&s->dev, sci_level); + } /* schedule a timer interruption if needed */ acpi_pm_tmr_update(&s->ar, (s->ar.pm1.evt.en & ACPI_BITMASK_TIMER_ENABLE) && !(pmsts & ACPI_BITMASK_TIMER_STATUS));