On Mon, Sep 6, 2021 at 12:54 AM Peter Maydell <peter.mayd...@linaro.org> wrote: > > On Sun, 5 Sept 2021 at 17:49, Bin Meng <bmeng...@gmail.com> wrote: > > > > On Mon, Sep 6, 2021 at 12:29 AM Peter Maydell <peter.mayd...@linaro.org> > > wrote: > > > > > > 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. > > > > Did you mean before commit 62a0db942dec leaving MemoryRegionOps NULL > > was not allowed, but things changed so that now it's possible to have > > NULL MemoryRegionOps? If yes, then the commit id of "Fixes" should > > probably go to the changes that allowed NULL memory ops. If not, I > > don't think "Fixes" tag is wrong. > > I mean that before commit 62a0db942dec leaving the pointers all > NULL was not allowed, and after that commit leaving the pointers all > NULL was still not allowed. It's been a requirement that you > provide at least one function pointer for read and one for > write ever since the MemoryRegion APIs were introduced in 2012. > You're proposing an API change; it might be a good one, but it > isn't a 'Fixes' to anything.
Where is the requirement documented? I don't see anything mentioned in docs/devel/memory.rst If it's a requirement since 2012, then I agree "Fixes" can be dropped. But a doc fix should be made to document the "requirement". Regards, Bin