On Thu, 30 Nov 2017 12:57:56 +0000
Jean-Philippe Brucker <jean-philippe.brucker@xxxxxxx> wrote:
> Hello,
>
> On 30/11/17 11:34, Pierre Morel wrote:
> [...]
> > +/**
> > + * 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;
>
> I wonder if the default values should also reflect what the VFIO
> implementation actually supports. Looking at vfio_dma_do_map, a 32-bit
> host will reject any iova greater than 32 bits. In addition,
> vfio_dma_do_unmap doesn't support unmapping the last page of a 64-bit
> address space (existing IOMMUs would probably reject map requests with
> IOVA > 52 bits anyway, but if they don't report a domain aperture, VFIO
> can't guess it).
>
> I think it's convenient to use VFIO_IOMMU_UNMAP_DMA on the full address
> space when an unmap-all is needed, maybe we could provide default aperture
> values that help doing this? (~0U for 32-bit and (~0ULL - PAGE_SIZE) for
> 64-bit)
Hmm, I'm not a huge fan of that. Looks like there are two bugs, one
that I've known about but we can't fix[1] in vfio_iommu_type1_dma_unmap
using start and size rather than start and end, therefore the user can
only express a range of (U64_MAX - 1). That's part of the API though,
live and learn.
The other is the check for iova wrap that doesn't consider the boundary
case. Seems like this could simply be fixed. In any case, I'd hate
for the apertures to impose that one-off if the hardware otherwise
doesn't. Thanks,
Alex
[1] Really the vfio api is flexible enough to solve this, a flag bit in
vfio_iommu_type1_info could indicate support for an alternate map/unmap
mode where size becomes end and a flag bit on the map and unmap structs
could indicate use of that mode.
