Re: [PATCH] softmmu/memory: Validate {read, write}_with_attrs before calling

[Peter Maydell]

On Sun, 5 Sept 2021 at 16:40, Bin Meng <bmeng.cn@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.cn@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