Re: [PATCH 2/7] guest_memfd: Introduce an object to manage the guest-memfd with RamDiscardManager

[Xu Yilun]

On Wed, Jan 22, 2025 at 11:43:01AM -0500, Peter Xu wrote:
> On Wed, Jan 22, 2025 at 05:41:31PM +0800, Xu Yilun wrote:
> > On Wed, Jan 22, 2025 at 03:30:05PM +1100, Alexey Kardashevskiy wrote:
> > > 
> > > 
> > > On 22/1/25 02:18, Peter Xu wrote:
> > > > On Tue, Jun 25, 2024 at 12:31:13AM +0800, Xu Yilun wrote:
> > > > > On Mon, Jan 20, 2025 at 03:46:15PM -0500, Peter Xu wrote:
> > > > > > On Mon, Jan 20, 2025 at 09:22:50PM +1100, Alexey Kardashevskiy 
> > > > > > wrote:
> > > > > > > > It is still uncertain how to implement the private MMIO. Our 
> > > > > > > > assumption
> > > > > > > > is the private MMIO would also create a memory region with
> > > > > > > > guest_memfd-like backend. Its mr->ram is true and should be 
> > > > > > > > managed by
> > > > > > > > RamdDiscardManager which can skip doing DMA_MAP in VFIO's 
> > > > > > > > region_add
> > > > > > > > listener.
> > > > > > > 
> > > > > > > My current working approach is to leave it as is in QEMU and VFIO.
> > > > > > 
> > > > > > Agreed.  Setting ram=true to even private MMIO sounds hackish, at 
> > > > > > least
> > > > > 
> > > > > The private MMIO refers to assigned MMIO, not emulated MMIO. IIUC,
> > > > > normal assigned MMIO is always set ram=true,
> > > > > 
> > > > > void memory_region_init_ram_device_ptr(MemoryRegion *mr,
> > > > >                                         Object *owner,
> > > > >                                         const char *name,
> > > > >                                         uint64_t size,
> > > > >                                         void *ptr)
> 
> [1]
> 
> > > > > {
> > > > >      memory_region_init(mr, owner, name, size);
> > > > >      mr->ram = true;
> > > > > 
> > > > > 
> > > > > So I don't think ram=true is a problem here.
> > > > 
> > > > I see.  If there's always a host pointer then it looks valid.  So it 
> > > > means
> > > > the device private MMIOs are always mappable since the start?
> > > 
> > > Yes. VFIO owns the mapping and does not treat shared/private MMIO any
> > > different at the moment. Thanks,
> > 
> > mm.. I'm actually expecting private MMIO not have a host pointer, just
> > as private memory do.
> > 
> > But I'm not sure why having host pointer correlates mr->ram == true.
> 
> If there is no host pointer, what would you pass into "ptr" as referenced
> at [1] above when creating the private MMIO memory region?

Sorry for confusion. I mean existing MMIO region use set mr->ram = true,
and unmappable region (gmem) also set mr->ram = true. So don't know why
mr->ram = true for private MMIO is hackish.

I think We could add another helper to create memory region for private
MMIO.

> 
> OTOH, IIUC guest private memory finally can also have a host pointer (aka,
> mmap()-able), it's just that even if it exists, accessing it may crash QEMU
> if it's private.

Not sure if I get it correct: when memory will be converted to private, QEMU
should firstly unmap the host ptr, which means host ptr doesn't alway exist.

Thanks,
Yilun

> 
> Thanks,
> 
> -- 
> Peter Xu
>