Re: [QEMU][PATCH v3 5/7] memory: add MemoryRegion map and unmap callbacks

[Peter Xu <peterx@redhat.com>]

On Wed, Apr 10, 2024 at 06:44:38PM +0200, Edgar E. Iglesias wrote:
> On Tue, Feb 27, 2024 at 11:37 PM Vikram Garhwal <vikram.garhwal@amd.com>
> wrote:
> 
> > From: Juergen Gross <jgross@suse.com>
> >
> > In order to support mapping and unmapping guest memory dynamically to
> > and from qemu during address_space_[un]map() operations add the map()
> > and unmap() callbacks to MemoryRegionOps.
> >
> > Those will be used e.g. for Xen grant mappings when performing guest
> > I/Os.
> >
> > Signed-off-by: Juergen Gross <jgross@suse.com>
> > Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com>
> >
> 
> 
> Paolo, Peter, David, Phiippe, do you guys have any concerns with this patch?

This introduces a 3rd memory type afaict, neither direct nor !direct.

What happens if someone does address_space_write() to it?  I didn't see it
covered here..

OTOH, the cover letter didn't mention too much either on the big picture:

https://lore.kernel.org/all/20240227223501.28475-1-vikram.garhwal@amd.com/

I want to have a quick grasp on whether it's justified worthwhile we should
introduce this complexity to qemu memory core.

Could I request a better cover letter when repost?  It'll be great to
mention things like:

  - what is grant mapping, why it needs to be used, when it can be used (is
    it only relevant to vIOMMU=on)?  Some more information on the high
    level design using this type or MR would be great.

  - why a 3rd memory type is required?  Do we have other alternatives?

    So it's all based on my very limited understanding of reading this:
    https://xenbits.xenproject.org/docs/4.3-testing/misc/grant-tables.txt

    If it's about cross-vm sharing memory, does it mean that in reality
    there are RAMs allocated, but it's only about permission management?
    In that case, is there any option we implement it with direct access
    mode (however with some extra dynamic permissions applied on top using
    some mechanism)?

  - perhaps sold old links would be great too so people can read about the
    context when it's not obvious, without a need to copy-paste.

Thanks,

-- 
Peter Xu