On Sun, 5 Sept 2021 at 16:40, Bin Meng <bmeng...@gmail.com> wrote:
>
> {read,write}_with_attrs might be missing, and the codes currently do
> not validate them before calling, which will cause segment fault.
>
> Fixes: 62a0db942dec ("memory: Remove old_mmio accessors")
> Signed-off-by: Bin Meng <bmeng...@gmail.com>This 'fixes' tag doesn't seem quite right. Before that commit 62a0db942dec, we still required that a MemoryRegionOps provided some form of both read and write. It was never valid to leave all of the possible read function pointers NULL. What are the examples of devices which are deliberately leaving these pointers set to NULL? Last time this came up, we discussed the other option, which is to have memory_region_init assert that the MemoryRegionOps defines at least one valid read and one valid write pointer, so that these bugs get caught quickly rather than only if the guest accesses the device in the wrong way. I tend to think that that is better, because for any particular device it's not necessarily the right thing to return MEMTX_ERROR (maybe the right behaviour is "return 0 for reads, ignore writes" -- the point is that there is no single default behaviour that works for every device and architecture). Forcing the device model author to explicitly write the code means they have to think about what the behaviour they want is. If we do choose to support allowing MemoryRegionOps structs to leave all the pointers NULL, we should document that, including what the default behaviour is. thanks -- PMM
