| Message ID | 0-v2-ce71068deeec+4cf6-fsl_rm_groups_jgg@nvidia.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Remove iommu_group_remove_device() from fsl | expand |
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? Regards, Joerg
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 do feel good that if there is a problem and someone does come forward it can be fixed up without a big trouble. Certainly without going back to mis-using iommu_grou_add/remove_device.. Thanks, Jason
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. And things are somewhat complicated because some of the drivers are also used on their ARM SOCs, so those still get maintained from the ARM side. But looks like this driver is powerpc only. Turns out I do have a machine that will probe this driver. AFAICS this series doesn't regress it, but that's just booting. I don't have it setup to test KVM/VFIO etc. 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. Anything else I can check easily? cheers
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
Jason Gunthorpe <jgg@nvidia.com> writes: > 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. >> ... > >> Anything else I can check easily? > > Wow Great, I think that is a Tested-by. :) Honestly booting at all is > 99% of the battle.. Great, yep consider it: Tested-by: Michael Ellerman <mpe@ellerman.id.au> cheers
On Wed, May 31, 2023 at 05:04:04PM +1000, Michael Ellerman wrote: > Great, yep consider it: > > Tested-by: Michael Ellerman <mpe@ellerman.id.au> Alright, applied them for 6.5.