> From: Robin Murphy <robin.murphy@xxxxxxx>
> Sent: Friday, July 1, 2022 6:34 PM
>
> On 2022-07-01 07:17, Alexey Kardashevskiy wrote:
> > VFIO on POWER does not implement iommu_ops and therefore
> iommu_capable()
> > always returns false and __iommu_group_alloc_blocking_domain() always
> > fails.
> >
> > iommu_group_claim_dma_owner() in setting container fails for the same
> > reason - it cannot allocate a domain.
> >
> > This skips the check for platforms supporting VFIO without implementing
> > iommu_ops which to my best knowledge is POWER only.
> >
> > This also allows setting container in absence of iommu_ops.
> >
> > Fixes: 70693f470848 ("vfio: Set DMA ownership for VFIO devices")
> > Fixes: e8ae0e140c05 ("vfio: Require that devices support DMA cache
> coherence")
> > Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxxxx>
> > ---
> >
> > Not quite sure what the proper small fix is and implementing iommu_ops
> > on POWER is not going to happen any time soon or ever :-/
>
> FWIW I did wonder about this when writing [1]. Is it appropriate to have
> any IOMMU API specifics outside the type1 code, or should these bits be
> abstracted behind vfio_iommu_driver_ops methods?
>
That is a good point. But an abstraction approach may not work as in
set_container() there may be no iommu driver attached to the container
yet. Probably a better way is just to do cache coherency check and
dma ownership claim both in type1 attach_group w/o adding new op.
But a bigger problem to me is how dma ownership is managed now on
POWER. Previously it was guarded by BUG_ON and vfio_group_viable().
Now with that removed while POWER doesn't support dma claim, does
it imply a regression on POWER platform now?
e.g. what should be returned for POWER in VFIO_GROUP_GET_STATUS:
if (group->container)
status.flags |= VFIO_GROUP_FLAGS_CONTAINER_SET |
VFIO_GROUP_FLAGS_VIABLE;
else if (!iommu_group_dma_owner_claimed(group->iommu_group))
status.flags |= VFIO_GROUP_FLAGS_VIABLE;
Thanks
Kevin
