05.06.2013 17:42, Hervé Poussineau wrote: > If that's not the case, QEMU will may during execution. > This has recently been fixed for: > - acpi (2d3b989529727ccace243b953a181fbae04a30d1) > - kvmapic (0c1cd0ae2a4faabeb948b9a07ea1696e853de174) > - xhci (6d3bc22e31bcee74dc1e05a5370cabb33b7c3fda)
And apparently we also have alot of other cases like this, which will trigger this new assert: hw/ssi/xilinx_spips.c:static const MemoryRegionOps lqspi_ops = { hw/ssi/xilinx_spips.c- .read = lqspi_read, hw/ssi/xilinx_spips.c- .endianness = DEVICE_NATIVE_ENDIAN, hw/ssi/xilinx_spips.c- .valid = { hw/ssi/xilinx_spips.c- .min_access_size = 1, hw/ssi/xilinx_spips.c- .max_access_size = 4 hw/ssi/xilinx_spips.c- } hw/usb/hcd-ehci.c:static const MemoryRegionOps ehci_mmio_caps_ops = { hw/usb/hcd-ehci.c- .read = ehci_caps_read, hw/usb/hcd-ehci.c- .valid.min_access_size = 1, hw/usb/hcd-ehci.c- .valid.max_access_size = 4, hw/usb/hcd-ehci.c- .impl.min_access_size = 1, hw/usb/hcd-ehci.c- .impl.max_access_size = 1, hw/usb/hcd-ehci.c- .endianness = DEVICE_LITTLE_ENDIAN, hw/usb/hcd-ehci.c-}; hw/scsi/megasas.c:static const MemoryRegionOps megasas_queue_ops = { hw/scsi/megasas.c- .read = megasas_queue_read, hw/scsi/megasas.c- .endianness = DEVICE_LITTLE_ENDIAN, hw/scsi/megasas.c- .impl = { hw/scsi/megasas.c- .min_access_size = 8, hw/scsi/megasas.c- .max_access_size = 8, hw/scsi/megasas.c- } hw/scsi/megasas.c-}; hw/pci/msix.c:static const MemoryRegionOps msix_pba_mmio_ops = { hw/pci/msix.c- .read = msix_pba_mmio_read, hw/pci/msix.c- .endianness = DEVICE_LITTLE_ENDIAN, hw/pci/msix.c- .valid = { hw/pci/msix.c- .min_access_size = 4, hw/pci/msix.c- .max_access_size = 4, hw/pci/msix.c- }, hw/pci/msix.c-}; hw/misc/lm32_sys.c:static const MemoryRegionOps sys_ops = { hw/misc/lm32_sys.c- .write = sys_write, hw/misc/lm32_sys.c- .valid.accepts = sys_ops_accepts, hw/misc/lm32_sys.c- .endianness = DEVICE_NATIVE_ENDIAN, hw/misc/lm32_sys.c-}; hw/misc/vfio.c:static const MemoryRegionOps vfio_ati_3c3_quirk = { hw/misc/vfio.c- .read = vfio_ati_3c3_quirk_read, hw/misc/vfio.c- .endianness = DEVICE_LITTLE_ENDIAN, hw/misc/vfio.c-}; hw/misc/debugexit.c:static const MemoryRegionOps debug_exit_ops = { hw/misc/debugexit.c- .write = debug_exit_write, hw/misc/debugexit.c- .valid.min_access_size = 1, hw/misc/debugexit.c- .valid.max_access_size = 4, hw/misc/debugexit.c- .endianness = DEVICE_LITTLE_ENDIAN, hw/misc/debugexit.c-}; hw/misc/pc-testdev.c:static const MemoryRegionOps test_irq_ops = { hw/misc/pc-testdev.c- .write = test_irq_line, hw/misc/pc-testdev.c- .valid.min_access_size = 1, hw/misc/pc-testdev.c- .valid.max_access_size = 1, hw/misc/pc-testdev.c- .endianness = DEVICE_LITTLE_ENDIAN, hw/misc/pc-testdev.c-}; hw/misc/pc-testdev.c:static const MemoryRegionOps test_flush_ops = { hw/misc/pc-testdev.c- .write = test_flush_page, hw/misc/pc-testdev.c- .valid.min_access_size = 4, hw/misc/pc-testdev.c- .valid.max_access_size = 4, hw/misc/pc-testdev.c- .endianness = DEVICE_LITTLE_ENDIAN, hw/misc/pc-testdev.c-}; Maybe instead (or in addition to), we should provide a dummy read or write functions -- instead of fixing each such occurence to use its own dummy function -- and maybe even a function to map old_mmio.{read,write} into {read,write} (so we'll have less ifs in there). Adding some Cc's. I don't think this is "trivial enough" having in mind all the above :) Thanks, /mjt > Signed-off-by: Hervé Poussineau <hpous...@reactos.org> > --- > I started all current QEMU system emulations with > qemu-system-{arch} -M {machine} , and none broke on these > additionnal asserts. > However, lots of them exited for other reasons, like not having the > right number of CPUs, no -kernel argument, or fetching invalid > instructions from RAM. > > ioport.c | 1 + > memory.c | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/ioport.c b/ioport.c > index a0ac2a0..8dd9d50 100644 > --- a/ioport.c > +++ b/ioport.c > @@ -337,6 +337,7 @@ void portio_list_init(PortioList *piolist, > unsigned n = 0; > > while (callbacks[n].size) { > + assert(callbacks[n].read && callbacks[n].write); > ++n; > } > > diff --git a/memory.c b/memory.c > index 5cb8f4a..654d1ce 100644 > --- a/memory.c > +++ b/memory.c > @@ -1008,6 +1008,8 @@ void memory_region_init_io(MemoryRegion *mr, > uint64_t size) > { > memory_region_init(mr, name, size); > + assert(ops->read || ops->old_mmio.read); > + assert(ops->write || ops->old_mmio.write); > mr->ops = ops; > mr->opaque = opaque; > mr->terminates = true; >