Re: [PATCH v2 0/3] Remove iommu_group_remove_device() from fsl

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, May 30, 2023 at 10:03:53PM +1000, Michael Ellerman wrote:
> Jason Gunthorpe <jgg@nvidia.com> writes:
> > On Tue, May 23, 2023 at 08:26:32AM +0200, Joerg Roedel wrote:
> >> On Tue, May 16, 2023 at 09:35:25PM -0300, Jason Gunthorpe wrote:
> >> > With POWER SPAPR now having a real iommu driver and using the normal group
> >> > lifecycle stuff fixing FSL will leave only VFIO's no-iommu support as a
> >> > user for the iommu_group_add/remove_device() calls. This will help
> >> > simplify the understanding of what the core code should be doing for these
> >> > functions.
> >> > 
> >> > Fix FSL to not need to call iommu_group_remove_device() at all.
> >> > 
> >> > v2:
> >> >  - Change the approach to use driver_managed_dma
> >> >  - Really simplify fsl_pamu_device_group() and just put everything in one
> >> >    function
> >> >  - New patch to make missing OF properties a probe failure
> >> > v1: https://lore.kernel.org/r/0-v1-1421774b874b+167-ppc_device_group_jgg@nvidia.com
> >> > 
> >> > Jason Gunthorpe (3):
> >> >   iommu/fsl: Always allocate a group for non-pci devices
> >> >   iommu/fsl: Move ENODEV to fsl_pamu_probe_device()
> >> >   iommu/fsl: Use driver_managed_dma to allow VFIO to work
> >> > 
> >> >  arch/powerpc/sysdev/fsl_pci.c   |   1 +
> >> >  drivers/iommu/fsl_pamu_domain.c | 123 +++++++++-----------------------
> >> >  2 files changed, 36 insertions(+), 88 deletions(-)
> >> 
> >> Any chance someone can test this on real hardware?
> >
> > There isn't even a MAINTAINERS entry for this, and the git log looks
> > pretty dead for a long time. I tried to cc people who might care,
> > but I'm not so optimistic - unless Li says something.
> 
> I guess it falls under LINUX FOR POWERPC EMBEDDED PPC83XX AND PPC85XX,
> but that's basically orphaned these days. Basically all the FSL/NXP
> powerpc code is orphaned, although there are still some users.

:\

> But looks like this driver is powerpc only.

Yes
 
> I do see some changes in dmesg, eg:
> 
> -fsl-pci ffe270000.pcie: Removing from iommu group 61
> -pci 0003:00:00.0: Adding to iommu group 60
> +pci 0003:00:00.0: Adding to iommu group 64
> 
> And lots more like that.

Yes, we expected that the groups would renumber.

> Anything else I can check easily?

Wow Great, I think that is a Tested-by. :) Honestly booting at all is
99% of the battle..

This system looks like it has "partitionable end points" so I expect
it to all work, all this does is create a group for the controller
itself, which you saw in the boot with this diff:

  -fsl-pci ffe270000.pcie: Removing from iommu group 61

Which is harmless as long as its group is singleton.

So, I don't think there is more you can do with this system.

Joerg, this seems like enough, lets go ahead please :)

Thanks,
Jason