Re: [PATCH v2 1/3] vfio: Find DMA available capability

[Cornelia Huck <cohuck@redhat.com>]

On Tue, 15 Sep 2020 08:14:24 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Hi Matthew,
> 
> On 9/15/20 12:29 AM, Matthew Rosato wrote:
> > The underlying host may be limiting the number of outstanding DMA
> > requests for type 1 IOMMU.  Add helper functions to check for the
> > DMA available capability and retrieve the current number of DMA
> > mappings allowed.
> > 
> > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> > ---
> >  hw/vfio/common.c              | 37 +++++++++++++++++++++++++++++++++++++
> >  include/hw/vfio/vfio-common.h |  2 ++
> >  2 files changed, 39 insertions(+)

(...)

> > +bool vfio_get_info_dma_avail(struct vfio_iommu_type1_info *info,
> > +                             unsigned int *avail)
> > +{
> > +    struct vfio_info_cap_header *hdr;
> > +    struct vfio_iommu_type1_info_dma_avail *cap;
> > +
> > +    /* If the capability cannot be found, assume no DMA limiting */
> > +    hdr = vfio_get_iommu_type1_info_cap(info,
> > +                                        VFIO_IOMMU_TYPE1_INFO_DMA_AVAIL);
> > +    if (hdr == NULL || avail == NULL) {  
> 
> If you expect the caller to use avail=NULL, then why
> return false when there is available information?

I agree; if the purpose of this function is to check if limiting is in
place and only optionally return the actual limit, we should return
true for hdr != NULL and avail == NULL.

> 
> > +        return false;
> > +    }
> > +
> > +    cap = (void *) hdr;
> > +    *avail = cap->avail;
> > +    return true;
> > +}
> > +
> >  static int vfio_setup_region_sparse_mmaps(VFIORegion *region,
> >                                            struct vfio_region_info *info)
> >  {

(...)