[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] docs/devel: memory: Document MemoryRegionOps requirement
From: |
Peter Xu |
Subject: |
Re: [PATCH] docs/devel: memory: Document MemoryRegionOps requirement |
Date: |
Wed, 8 Sep 2021 14:50:31 -0400 |
On Mon, Sep 06, 2021 at 03:01:54PM +0200, Philippe Mathieu-Daudé wrote:
> On 9/6/21 2:20 PM, Bin Meng wrote:
> > It's been a requirement that at least one function pointer for read
> > and one for write are provided ever since the MemoryRegion APIs were
> > introduced in 2012.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
> >
> > docs/devel/memory.rst | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/docs/devel/memory.rst b/docs/devel/memory.rst
> > index 5dc8a12682..7b589b21d2 100644
> > --- a/docs/devel/memory.rst
> > +++ b/docs/devel/memory.rst
> > @@ -344,6 +344,11 @@ based on the attributes used for the memory
> > transaction, or need
> > to be able to respond that the access should provoke a bus error
> > rather than completing successfully; those devices can use the
> > ->read_with_attrs() and ->write_with_attrs() callbacks instead.
> > +The requirement for a device's MemoryRegionOps is that at least
> > +one callback for read and one for write are provided. If both
> > +->read() and ->read_with_attrs() are provided, the plain ->read()
> > +version takes precedence over the with_attrs() version. So does
> > +the write callback.
>
> What about also adding a runtime check?
>
> -- >8 --
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index bfedaf9c4df..8ab602d3379 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -1516,6 +1516,17 @@ MemTxResult
> memory_region_dispatch_write(MemoryRegion *mr,
> }
> }
>
> +static void memory_region_set_ops(MemoryRegion *mr, const
> MemoryRegionOps *ops)
> +{
> + if (ops) {
> + assert(ops->valid.accepts || (ops->read || ops->read_with_attrs));
> + assert(ops->valid.accepts || (ops->write ||
> ops->write_with_attrs));
Curious why accepts() matters.. Say, if there's only accepts() provided and it
returned true, then I think we still can't avoid the coredump when read/write?
I'm also curious what's the issue that Paolo mentioned here:
8da074de-7dff-6505-5180-720cf2f47c70@redhat.com/">https://lore.kernel.org/qemu-devel/8da074de-7dff-6505-5180-720cf2f47c70@redhat.com/
I believe Paolo was referring to this series from Prasad:
https://lore.kernel.org/qemu-devel/20200811114133.672647-10-ppandit@redhat.com/
We may need to solve that issue then maybe we can consider revive Prasad's
patchset?
--
Peter Xu