On 14/11/2024 12.29, Mark Cave-Ayland wrote:
On 14/11/2024 10:46, Thomas Huth wrote:
When compiling QEMU with --enable-cfi, the "q800" m68k machine
currently crashes very early, when the q800_machine_init() function
tries to wire the interrupts of the "via1" device.
This happens because TYPE_MOS6522_Q800_VIA1 is supposed to be a
proper SysBus device, but its parent (TYPE_MOS6522) has a mistake
in its class definition where it is only derived from DeviceClass,
and not from SysBusDeviceClass, so we end up in funny memory access
issues here. Using the right class hierarchy for the MOS6522 device
fixes the problem.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2675
Signed-off-by: Thomas Huth <th...@redhat.com>
---
include/hw/misc/mos6522.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h
index fba45668ab..920871a598 100644
--- a/include/hw/misc/mos6522.h
+++ b/include/hw/misc/mos6522.h
@@ -154,7 +154,7 @@ struct MOS6522State {
OBJECT_DECLARE_TYPE(MOS6522State, MOS6522DeviceClass, MOS6522)
struct MOS6522DeviceClass {
- DeviceClass parent_class;
+ SysBusDeviceClass parent_class;
ResettablePhases parent_phases;
void (*portB_write)(MOS6522State *dev);
Ooof. I suspect I started using DeviceClass first before switching to
SysBusDeviceClass later to implement reset functionality. Anyhow the patch
looks good: thanks Thomas!
Reviewed-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
Thanks!
By the way, fun fact why this problem was likely only triggering with CFI
enabled: SysBusDeviceClass just extends DeviceClass by two function
pointers, second one is connect_irq_notifier. This one gets called from
sysbus_connect_irq() (which is called by q800_machine_init()).
But MOS6522DeviceClass is directly derived from DeviceClass, that memory
space is filled by the ResettablePhases struct instead. So instead of
calling an irq notifier, the sysbus_connect_irq() likely called the
ResettableHoldPhase() function instead! It likely did not crash without CFI,
since the parameters are similar enough, but with CFI, this got successfully
flagged as an illegal call via function pointer :-)
Thomas
