qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 11/20] vfio/common: Add device dirty page tracking start/s


From: Alex Williamson
Subject: Re: [PATCH v2 11/20] vfio/common: Add device dirty page tracking start/stop
Date: Thu, 23 Feb 2023 12:27:23 -0700

On Wed, 22 Feb 2023 22:02:24 -0400
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Wed, Feb 22, 2023 at 03:40:43PM -0700, Alex Williamson wrote:
> > > +    /*
> > > +     * DMA logging uAPI guarantees to support at least num_ranges that 
> > > fits into
> > > +     * a single host kernel page. To be on the safe side, use this as a 
> > > limit
> > > +     * from which to merge to a single range.
> > > +     */
> > > +    max_ranges = qemu_real_host_page_size() / sizeof(*ranges);
> > > +    cur_ranges = iova_tree_nnodes(container->mappings);
> > > +    control->num_ranges = (cur_ranges <= max_ranges) ? cur_ranges : 1;  
> > 
> > This makes me suspicious that we're implementing to the characteristics
> > of a specific device rather than strictly to the vfio migration API.
> > Are we just trying to avoid the error handling to support the try and
> > fall back to a single range behavior?  
> 
> This was what we agreed to when making the kernel patches. Userspace
> is restricted to send one page of range list to the kernel, and the
> kernel will always adjust that to whatever smaller list the device needs.
> 
> We added this limit only because we don't want to have a way for
> userspace to consume a lot of kernel memory.
> 
> See LOG_MAX_RANGES in vfio_main.c
> 
> If qemu is viommu mode and it has a huge number of ranges then it must
> cut it down before passing things to the kernel.

Ok, that's the kernel implementation, but the uAPI states:

 * The core kernel code guarantees to support by minimum num_ranges that fit
 * into a single kernel page. User space can try higher values but should give
 * up if the above can't be achieved as of some driver limitations.

So again, I think I'm just looking for a better comment that doesn't
add FUD to the reasoning behind switching to a single range, ie. a)
it's easier to deal with given the kernel guarantee and b) the current
kernel implementation imposes a hard limit at page size anyway.  Thanks,

Alex




reply via email to

[Prev in Thread] Current Thread [Next in Thread]