Re: [Qemu-devel] [PATCH] memory region: check the old.mmio.read status

[Li Qiang]

Peter Maydell <address@hidden> 于2018年9月12日周三 下午8:55写道：

> On 12 September 2018 at 13:32, Li Qiang <address@hidden> wrote:
> > To avoid NULL-deref for the devices without read callbacks
> >
> > Signed-off-by: Li Qiang <address@hidden>
> > ---
> >  memory.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/memory.c b/memory.c
> > index 9b73892768..48d025426b 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -406,6 +406,10 @@ static MemTxResult
> memory_region_oldmmio_read_accessor(MemoryRegion *mr,
> >  {
> >      uint64_t tmp;
> >
> > +    if (!mr->ops->old_mmio.read[ctz32(size)]) {
> > +        return MEMTX_DECODE_ERROR;
> > +    }
> > +
> >      tmp = mr->ops->old_mmio.read[ctz32(size)](mr->opaque, addr);
> >      if (mr->subpage) {
> >          trace_memory_region_subpage_read(get_cpu_index(), mr, addr,
> tmp, size);
> > --
> > 2.11.0
> >
>
> There's patches on-list which drop the old_mmio field from the MemoryRegion
> struct entirely, so I think this patch as it stands is obsolete.
>
> Currently our semantics are "you must provide both read and write, even
> if one of them just always returns 0 / does nothing / returns an error".
> We could probably reasonably assert this at the point when the
> MemoryRegionOps is registered.
>

This patch is sent as the results of this thread:
-->https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg01332.html

So I think  I should send a path set to add all the missing read function
as Laszlo Ersek points
in the above thread discussion, right?

Thanks,
Li Qiang




>
> thanks
> -- PMM
>