On 6/17/20 4:05 PM, Alex Bennée wrote: > > Philippe Mathieu-Daudé <phi...@redhat.com> writes: > >> On 6/17/20 3:06 PM, Alex Williamson wrote: >>> On Wed, 17 Jun 2020 16:39:56 +1000 >>> David Gibson <da...@gibson.dropbear.id.au> wrote: >>> >>>> On Wed, Jun 17, 2020 at 11:09:27AM +0530, P J P wrote: >>>>> From: Prasad J Pandit <p...@fedoraproject.org> >>>>> >>>>> Hello, >>>>> >>>>> This series adds various undefined MMIO read/write functions >>>>> to avoid potential guest crash via a NULL pointer dereference. >>>> >>>> Hrm. If this is such a common problem, maybe we should just add a >>>> NULL check in the common paths. >>> >>> +1, clearly the behavior is already expected. Thanks, >> >> 20 months ago Peter suggested: >> >> "assert that every MemoryRegionOps has pointers to callbacks >> in it, when it is registered in memory_region_init_io() and >> memory_region_init_rom_device_nomigrate()." >> >> https://www.mail-archive.com/qemu-devel@nongnu.org/msg573310.html >> >> Li Qiang refers to this post from Paolo: >> >>> static const MemoryRegionOps notdirty_mem_ops = { >>> + .read = notdirty_mem_read, >>> .write = notdirty_mem_write, >>> .valid.accepts = notdirty_mem_accepts, >>> .endianness = DEVICE_NATIVE_ENDIAN, >> >> "This cannot happen, since TLB_NOTDIRTY is only added >> to the addr_write member (see accel/tcg/cputlb.c)." >> >> https://www.mail-archive.com/qemu-devel@nongnu.org/msg561345.html > > What about catching it in memory_region_dispatch_write: > > if (mr->ops->write) { > return access_with_adjusted_size(addr, &data, size, > mr->ops->impl.min_access_size, > mr->ops->impl.max_access_size, > memory_region_write_accessor, mr, > attrs); > } else if (mr->ops->write_with_attrs) { > return > access_with_adjusted_size(addr, &data, size, > mr->ops->impl.min_access_size, > mr->ops->impl.max_access_size, > memory_region_write_with_attrs_accessor, > mr, attrs); > } else { > qemu_log_mask(LOG_UNIMP|LOG_GUEST_ERROR, "%s: %s un-handled write\n", > __func__, mr->name);
The problem is what return value to return... MEMTX_OK/MEMTX_ERROR/MEMTX_DECODE_ERROR? This is very device-specific and can't be decided here for all the cases. Better to abort() and fix each device? > } > >
