Re: [PATCH] vfio/iommu_type1: report the IOMMU aperture info

[Pierre Morel]

On 30/11/2017 14:22, Auger Eric wrote:
> Hi Pierre,
> On 30/11/17 12:34, Pierre Morel wrote:
> > When userland VFIO defines a new IOMMU for a guest it may
> > want to specify to the guest the physical limits of
> > the underlying host IOMMU to avoid access to forbidden
> > memory ranges.
> >
> > Currently, the vfio_iommu_type1 driver does not report this
> > information to userland.
> >
> > Let's extend the vfio_iommu_type1_info structure reported
> > by the ioctl VFIO_IOMMU_GET_INFO command to report the
> > IOMMU limits as new uint64_t entries aperture_start and
> > aperture_end.
> >
> > Let's also extend the flags bit map to add a flag specifying
> > if this extension of the info structure is reported or not.
> >
> > Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxxxxxxx>
> > ---
> >   drivers/vfio/vfio_iommu_type1.c | 42 +++++++++++++++++++++++++++++++++++++++++
> >   include/uapi/linux/vfio.h       |  3 +++
> >   2 files changed, 45 insertions(+)
> >
> > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> > index 8549cb1..7da5fe0 100644
> > --- a/drivers/vfio/vfio_iommu_type1.c
> > +++ b/drivers/vfio/vfio_iommu_type1.c
> > @@ -1526,6 +1526,40 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu)
> >   	return ret;
> >   }
> >   
> > +/**
> > + * vfio_get_aperture - report minimal aperture of a vfio_iommu
> > + * @iommu: the current vfio_iommu
> > + * @start: a pointer to the aperture start
> > + * @end  : a pointer to the aperture end
> > + *
> > + * This function iterate on the domains using the given vfio_iommu
> > + * and restrict the aperture to the minimal aperture common
> > + * to all domains sharing this vfio_iommu.
> > + */
> > +static void vfio_get_aperture(struct vfio_iommu *iommu, uint64_t *start,
> > +				uint64_t *end)
> > +{
> > +	struct iommu_domain_geometry geometry;
> > +	struct vfio_domain *domain;
> > +
> > +	*start = 0;
> > +	*end = U64_MAX;
> > +
> > +	mutex_lock(&iommu->lock);
> > +	/* loop on all domains using this vfio_iommu */
> > +	list_for_each_entry(domain, &iommu->domain_list, next) {
> > +		iommu_domain_get_attr(domain->domain, DOMAIN_ATTR_GEOMETRY,
> > +					&geometry);
> > +		if (geometry.force_aperture) {
> > +			if (geometry.aperture_start > *start)
> > +				*start = geometry.aperture_start;
> > +			if (geometry.aperture_end < *end)
> > +				*end = geometry.aperture_end;
> > +		}
> > +	}
> > +	mutex_unlock(&iommu->lock);
> > +}
> > +
> >   static long vfio_iommu_type1_ioctl(void *iommu_data,
> >   				   unsigned int cmd, unsigned long arg)
> >   {
> > @@ -1560,6 +1594,14 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
> >   
> >   		info.iova_pgsizes = vfio_pgsize_bitmap(iommu);
> >   
> > +		minsz = min_t(size_t, info.argsz, sizeof(info));
> > +		if (minsz >= offsetofend(struct vfio_iommu_type1_info,
> > +					 aperture_end)) {
> > +			info.flags |= VFIO_IOMMU_INFO_APERTURE;
> > +			vfio_get_aperture(iommu, &info.aperture_start,
> > +					  &info.aperture_end);
> > +		}
> > +
> >   		return copy_to_user((void __user *)arg, &info, minsz) ?
> >   			-EFAULT : 0;
> >   
> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > index 0fb25fb..780d909 100644
> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -519,6 +519,9 @@ struct vfio_iommu_type1_info {
> >   	__u32	flags;
> >   #define VFIO_IOMMU_INFO_PGSIZES (1 << 0)	/* supported page sizes info */
> >   	__u64	iova_pgsizes;		/* Bitmap of supported page sizes */
> > +#define VFIO_IOMMU_INFO_APERTURE (1 << 1)	/* supported aperture info */
> > +	__u64   aperture_start;		/* start of DMA aperture */
> > +	__u64   aperture_end;		/* end of DMA aperture */
> >   };
> >   
> >   #define VFIO_IOMMU_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
>
> In the past I was told by Alex that using the capability chain
> extensions to add some new capabilities was a better approach:
>
> https://lkml.org/lkml/2016/4/6/751
>
> I did that kind of exercise in
> [PATCH v9 7/7] vfio/type1: return MSI geometry through
> VFIO_IOMMU_GET_INFO capability chains
>
> https://lists.linuxfoundation.org/pipermail/iommu/2016-May/016892.html
>
> The patch was not used eventually but maybe that can help.
>
> Thanks
>
> Eric

indeed interesting, I will go this way.
Thanks and also thanks for the pointers.

Pierre

--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany