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 <chenhua...@kernel.org>
Cc: "Philippe Mathieu-Daudé" <f4...@amsat.org>
Cc: Jiaxun Yang <jiaxun.y...@flygoat.com>
Reported-by: Peter Maydell <peter.may...@linaro.org>
Signed-off-by: Isaku Yamahata <isaku.yamah...@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));


Reply via email to