* Re: [RFC PATCH 1/5] PCI: Refuse function reset of an SR-IOV PF with enabled VFs [not found] ` <20260818140304.GC5482@ziepe.ca> @ 2026-08-18 20:39 ` Alex Williamson 2026-08-20 22:38 ` Samiullah Khawaja 2026-08-27 5:35 ` Tian, Kevin 0 siblings, 2 replies; 6+ messages in thread From: Alex Williamson @ 2026-08-18 20:39 UTC (permalink / raw) To: Jason Gunthorpe Cc: Tian, Kevin, Samiullah Khawaja, kvm, Alex Williamson, Bjorn Helgaas, linux-kernel, linux-pci, iommu On Tue, 18 Aug 2026 11:03:04 -0300 Jason Gunthorpe <jgg@ziepe.ca> wrote: > On Tue, Aug 18, 2026 at 07:07:20AM +0000, Tian, Kevin wrote: > > 3) resetting a PF due to ad-hoc FW error > > > > e.g. pdsc_check_pci_health() calls pci_reset_function() when FW > > reports broken PCI. Sounds a similar condition to AER. > > > > but as it calls pci_reset_function() this series causes a regression > > in case of active VFs. Is it desired? > > mlx5 does something similar. RAS flows require detecting the PF has > blown up and resetting it to bring it back. > > This has to work while VFs exist and bound to kernel drivers or vfio. > > In the VFIO KVM case I would expect the VM to experiance a RAS error > on its vPCI device and trigger a VF flr This would need to be a device level RAS flow as we're sorely lacking on forwarding reset notification to VF drivers or providing any mechanism for recovery from the vfio error eventfd. > But we have to sort all this out through the hypervisor to not crash > the machine or otherwise :) In general guarding the high level pci_reset_function() family on (pci_num_vfs() == 0) seems to have more pros than cons and the restriction matches the scoped intent of the interface. The low level __pci_reset_function_locked() interface remains untouched, and as the prefix implies, relies on the caller to do necessary verification and preparation. A "reset the PF regardless of VFs" seems like an underscore prefix sort of operation. We'll need to export the pci_dev_save_and_disable() and pci_dev_restore() functions to provide handlers the full suite of tools they need to replace the existing function and port those we find (maybe also questioning the author's intent on VF reset and recovery). Of course making the guard parameterized with wrappers to expand this family of calls is also an option, pci_reset_function_locked_with_vfs()... The mlx5 driver doesn't actually seem to be affected by these changes, doing its own link toggle or a PCI bus reset, not a pci_reset_function() call. On the other improvements, triggering a pci_reset_function_locked() on unbind after SR-IOV is disabled, when needs_reset is indicated, looks pretty trivial. The blocking domain operation looks like it might be simplest to implement in the IOMMU core. We can set a flag for a default blocking domain on the IOMMU group when we take_dma_ownership of the group. Then release_dma_ownership picks the blocking rather than default domain. This is then unwound in use_default_domain, called via dma_configure, attaching the device to the default domain in probe of the next driver. Therefore until probe by another driver, a device used by vfio would remain in a blocking domain even while unused and unbound. Does that seem reasonable (Cc +iommu)? Thanks, Alex ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/5] PCI: Refuse function reset of an SR-IOV PF with enabled VFs 2026-08-18 20:39 ` [RFC PATCH 1/5] PCI: Refuse function reset of an SR-IOV PF with enabled VFs Alex Williamson @ 2026-08-20 22:38 ` Samiullah Khawaja 2026-08-20 23:37 ` Jason Gunthorpe 2026-08-27 5:35 ` Tian, Kevin 1 sibling, 1 reply; 6+ messages in thread From: Samiullah Khawaja @ 2026-08-20 22:38 UTC (permalink / raw) To: Alex Williamson Cc: Jason Gunthorpe, Tian, Kevin, kvm, Alex Williamson, Bjorn Helgaas, linux-kernel, linux-pci, iommu On Tue, Aug 18, 2026 at 02:39:06PM -0600, Alex Williamson wrote: >On Tue, 18 Aug 2026 11:03:04 -0300 >Jason Gunthorpe <jgg@ziepe.ca> wrote: > >> On Tue, Aug 18, 2026 at 07:07:20AM +0000, Tian, Kevin wrote: >> > 3) resetting a PF due to ad-hoc FW error >> > >> > e.g. pdsc_check_pci_health() calls pci_reset_function() when FW >> > reports broken PCI. Sounds a similar condition to AER. >> > >> > but as it calls pci_reset_function() this series causes a regression >> > in case of active VFs. Is it desired? >> >> mlx5 does something similar. RAS flows require detecting the PF has >> blown up and resetting it to bring it back. >> >> This has to work while VFs exist and bound to kernel drivers or vfio. >> >> In the VFIO KVM case I would expect the VM to experiance a RAS error >> on its vPCI device and trigger a VF flr > >This would need to be a device level RAS flow as we're sorely lacking >on forwarding reset notification to VF drivers or providing any >mechanism for recovery from the vfio error eventfd. > >> But we have to sort all this out through the hypervisor to not crash >> the machine or otherwise :) > >In general guarding the high level pci_reset_function() family on >(pci_num_vfs() == 0) seems to have more pros than cons and the >restriction matches the scoped intent of the interface. > >The low level __pci_reset_function_locked() interface remains >untouched, and as the prefix implies, relies on the caller to do >necessary verification and preparation. A "reset the PF regardless of >VFs" seems like an underscore prefix sort of operation. We'll need to >export the pci_dev_save_and_disable() and pci_dev_restore() functions >to provide handlers the full suite of tools they need to replace the >existing function and port those we find (maybe also questioning the >author's intent on VF reset and recovery). > >Of course making the guard parameterized with wrappers to expand this >family of calls is also an option, >pci_reset_function_locked_with_vfs()... > >The mlx5 driver doesn't actually seem to be affected by these changes, >doing its own link toggle or a PCI bus reset, not a >pci_reset_function() call. > >On the other improvements, triggering a pci_reset_function_locked() on >unbind after SR-IOV is disabled, when needs_reset is indicated, looks >pretty trivial. > >The blocking domain operation looks like it might be simplest to >implement in the IOMMU core. We can set a flag for a default blocking >domain on the IOMMU group when we take_dma_ownership of the group. Then >release_dma_ownership picks the blocking rather than default domain. > >This is then unwound in use_default_domain, called via dma_configure, >attaching the device to the default domain in probe of the next driver. >Therefore until probe by another driver, a device used by vfio would >remain in a blocking domain even while unused and unbound. The devices are expected to be attached to the default_domain even when these are unbound and the use_default_domain assumes that, and it only checks the ownership and doesn't switch the domain to default_domain. I guess we should add a WARN in use_default_domain() if that is not true. I will probably send out a patch for that separately. I think we can move the device back to default_domain after reset after unbind, maybe it can be done in pci_dma_cleanup() based on driver_managed_dma? > >Does that seem reasonable (Cc +iommu)? Thanks, > >Alex > Thanks, Sami ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/5] PCI: Refuse function reset of an SR-IOV PF with enabled VFs 2026-08-20 22:38 ` Samiullah Khawaja @ 2026-08-20 23:37 ` Jason Gunthorpe 2026-08-21 2:20 ` Samiullah Khawaja 0 siblings, 1 reply; 6+ messages in thread From: Jason Gunthorpe @ 2026-08-20 23:37 UTC (permalink / raw) To: Samiullah Khawaja, Nicolin Chen Cc: Alex Williamson, Tian, Kevin, kvm, Alex Williamson, Bjorn Helgaas, linux-kernel, linux-pci, iommu On Thu, Aug 20, 2026 at 10:38:07PM +0000, Samiullah Khawaja wrote: > > The blocking domain operation looks like it might be simplest to > > implement in the IOMMU core. We can set a flag for a default blocking > > domain on the IOMMU group when we take_dma_ownership of the group. Then > > release_dma_ownership picks the blocking rather than default domain. > > > > This is then unwound in use_default_domain, called via dma_configure, > > attaching the device to the default domain in probe of the next driver. > > Therefore until probe by another driver, a device used by vfio would > > remain in a blocking domain even while unused and unbound. > > The devices are expected to be attached to the default_domain even when > these are unbound and the use_default_domain assumes that, and it only > checks the ownership and doesn't switch the domain to default_domain. I > guess we should add a WARN in use_default_domain() if that is not true. > I will probably send out a patch for that separately. > > I think we can move the device back to default_domain after reset after > unbind, maybe it can be done in pci_dma_cleanup() based on > driver_managed_dma? This blocking domain stuff sounds very similar to what Nicolin implemented for the per-function ATS issue? Broadly we must setup a blocking domain in the iommu if ATS is available across reset or you get these ATS related issues. I think at the time he looked at doing SRIOV as well but it was tricky.. Jason ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/5] PCI: Refuse function reset of an SR-IOV PF with enabled VFs 2026-08-20 23:37 ` Jason Gunthorpe @ 2026-08-21 2:20 ` Samiullah Khawaja 2026-09-04 19:06 ` Jason Gunthorpe 0 siblings, 1 reply; 6+ messages in thread From: Samiullah Khawaja @ 2026-08-21 2:20 UTC (permalink / raw) To: Jason Gunthorpe Cc: Nicolin Chen, Alex Williamson, Tian, Kevin, kvm, Alex Williamson, Bjorn Helgaas, linux-kernel, linux-pci, iommu On Thu, Aug 20, 2026 at 08:37:22PM -0300, Jason Gunthorpe wrote: >On Thu, Aug 20, 2026 at 10:38:07PM +0000, Samiullah Khawaja wrote: > >> > The blocking domain operation looks like it might be simplest to >> > implement in the IOMMU core. We can set a flag for a default blocking >> > domain on the IOMMU group when we take_dma_ownership of the group. Then >> > release_dma_ownership picks the blocking rather than default domain. >> > >> > This is then unwound in use_default_domain, called via dma_configure, >> > attaching the device to the default domain in probe of the next driver. >> > Therefore until probe by another driver, a device used by vfio would >> > remain in a blocking domain even while unused and unbound. >> >> The devices are expected to be attached to the default_domain even when >> these are unbound and the use_default_domain assumes that, and it only >> checks the ownership and doesn't switch the domain to default_domain. I >> guess we should add a WARN in use_default_domain() if that is not true. >> I will probably send out a patch for that separately. >> >> I think we can move the device back to default_domain after reset after >> unbind, maybe it can be done in pci_dma_cleanup() based on >> driver_managed_dma? > >This blocking domain stuff sounds very similar to what Nicolin >implemented for the per-function ATS issue? I see you are talking about this invalidation stuff: https://lore.kernel.org/all/348c50ab6e95b5ec6d48ee3fa05d529a784a34c3.1765834788.git.nicolinc@nvidia.com/ But this was the case where the device is going to be reset and to prevent ATS issues, we attach it to blocking domain before doing the reset. But of course, with the PF reset here, it induces the same kind of ATS issues on the VFs. > >Broadly we must setup a blocking domain in the iommu if ATS is >available across reset or you get these ATS related issues. > >I think at the time he looked at doing SRIOV as well but it was >tricky.. Hmm... doing that for SRIOV also, by allowing PF reset but attaching the VFs to blocking domains before the PF is reset, will only resolve the ATS issues. But the software state is still out of sync with hardware state. > >Jason Sami ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/5] PCI: Refuse function reset of an SR-IOV PF with enabled VFs 2026-08-21 2:20 ` Samiullah Khawaja @ 2026-09-04 19:06 ` Jason Gunthorpe 0 siblings, 0 replies; 6+ messages in thread From: Jason Gunthorpe @ 2026-09-04 19:06 UTC (permalink / raw) To: Samiullah Khawaja Cc: Nicolin Chen, Alex Williamson, Tian, Kevin, kvm, Alex Williamson, Bjorn Helgaas, linux-kernel, linux-pci, iommu On Fri, Aug 21, 2026 at 02:20:55AM +0000, Samiullah Khawaja wrote: > On Thu, Aug 20, 2026 at 08:37:22PM -0300, Jason Gunthorpe wrote: > > On Thu, Aug 20, 2026 at 10:38:07PM +0000, Samiullah Khawaja wrote: > > > > > > The blocking domain operation looks like it might be simplest to > > > > implement in the IOMMU core. We can set a flag for a default blocking > > > > domain on the IOMMU group when we take_dma_ownership of the group. Then > > > > release_dma_ownership picks the blocking rather than default domain. > > > > > > > > This is then unwound in use_default_domain, called via dma_configure, > > > > attaching the device to the default domain in probe of the next driver. > > > > Therefore until probe by another driver, a device used by vfio would > > > > remain in a blocking domain even while unused and unbound. > > > > > > The devices are expected to be attached to the default_domain even when > > > these are unbound and the use_default_domain assumes that, and it only > > > checks the ownership and doesn't switch the domain to default_domain. I > > > guess we should add a WARN in use_default_domain() if that is not true. > > > I will probably send out a patch for that separately. > > > > > > I think we can move the device back to default_domain after reset after > > > unbind, maybe it can be done in pci_dma_cleanup() based on > > > driver_managed_dma? > > > > This blocking domain stuff sounds very similar to what Nicolin > > implemented for the per-function ATS issue? > > I see you are talking about this invalidation stuff: > > https://lore.kernel.org/all/348c50ab6e95b5ec6d48ee3fa05d529a784a34c3.1765834788.git.nicolinc@nvidia.com/ > > But this was the case where the device is going to be reset and to > prevent ATS issues, we attach it to blocking domain before doing the > reset. But of course, with the PF reset here, it induces the same kind > of ATS issues on the VFs. Yes, we need to attach a blocking domain before we make the function unable to respond to ATC invalidation. PF or VF reset both trigger this condition. > Hmm... doing that for SRIOV also, by allowing PF reset but attaching the > VFs to blocking domains before the PF is reset, will only resolve the > ATS issues. But the software state is still out of sync with hardware > state. Sure, it doesn't fix everything, but it is something you'd need to do to avoid problems in the iommu driver side.. Jason ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [RFC PATCH 1/5] PCI: Refuse function reset of an SR-IOV PF with enabled VFs 2026-08-18 20:39 ` [RFC PATCH 1/5] PCI: Refuse function reset of an SR-IOV PF with enabled VFs Alex Williamson 2026-08-20 22:38 ` Samiullah Khawaja @ 2026-08-27 5:35 ` Tian, Kevin 1 sibling, 0 replies; 6+ messages in thread From: Tian, Kevin @ 2026-08-27 5:35 UTC (permalink / raw) To: Alex Williamson, Jason Gunthorpe, Nicolin Chen Cc: Samiullah Khawaja, kvm, Alex Williamson, Bjorn Helgaas, linux-kernel, linux-pci, iommu@lists.linux.dev > From: Alex Williamson <alex.williamson@nvidia.com> > Sent: Wednesday, August 19, 2026 4:39 AM > > On Tue, 18 Aug 2026 11:03:04 -0300 > Jason Gunthorpe <jgg@ziepe.ca> wrote: > > > On Tue, Aug 18, 2026 at 07:07:20AM +0000, Tian, Kevin wrote: > > > 3) resetting a PF due to ad-hoc FW error > > > > > > e.g. pdsc_check_pci_health() calls pci_reset_function() when FW > > > reports broken PCI. Sounds a similar condition to AER. > > > > > > but as it calls pci_reset_function() this series causes a regression > > > in case of active VFs. Is it desired? > > > > mlx5 does something similar. RAS flows require detecting the PF has > > blown up and resetting it to bring it back. > > > > This has to work while VFs exist and bound to kernel drivers or vfio. > > > > In the VFIO KVM case I would expect the VM to experiance a RAS error > > on its vPCI device and trigger a VF flr > > This would need to be a device level RAS flow as we're sorely lacking > on forwarding reset notification to VF drivers or providing any > mechanism for recovery from the vfio error eventfd. > > > But we have to sort all this out through the hypervisor to not crash > > the machine or otherwise :) > > In general guarding the high level pci_reset_function() family on > (pci_num_vfs() == 0) seems to have more pros than cons and the > restriction matches the scoped intent of the interface. > > The low level __pci_reset_function_locked() interface remains > untouched, and as the prefix implies, relies on the caller to do > necessary verification and preparation. A "reset the PF regardless of > VFs" seems like an underscore prefix sort of operation. We'll need to > export the pci_dev_save_and_disable() and pci_dev_restore() functions > to provide handlers the full suite of tools they need to replace the > existing function and port those we find (maybe also questioning the > author's intent on VF reset and recovery). > > Of course making the guard parameterized with wrappers to expand this > family of calls is also an option, > pci_reset_function_locked_with_vfs()... +1 for wrappers. Maybe call it pci_force_reset_function_locked() which is slightly forward-looking if in future other conditions may reject the reset request using the base function. > > The mlx5 driver doesn't actually seem to be affected by these changes, > doing its own link toggle or a PCI bus reset, not a > pci_reset_function() call. > > On the other improvements, triggering a pci_reset_function_locked() on > unbind after SR-IOV is disabled, when needs_reset is indicated, looks > pretty trivial. > > The blocking domain operation looks like it might be simplest to > implement in the IOMMU core. We can set a flag for a default blocking > domain on the IOMMU group when we take_dma_ownership of the group. > Then > release_dma_ownership picks the blocking rather than default domain. IMHO we should change release_dma_ownership() to always picks the blocking. No new flag. this take/release interfaces are invented for drivers which manage dma on their own, so it sounds reasonable to not assume the operated device is in a good shape after the ownership is released. Then only a clean reset can unwind it. > > This is then unwound in use_default_domain, called via dma_configure, > attaching the device to the default domain in probe of the next driver. > Therefore until probe by another driver, a device used by vfio would > remain in a blocking domain even while unused and unbound. > so unwinding it blindly at dma_configure time is also problematic, as the device may still have a bad state to hurt the kernel. Instead use_default_domain should fail in such case (in blocked with owner_cnt being '0') then a reset is expected to unwind it (Nicolin's work will carry the resetting status to pci_dev_reset_iommu_done()) Once we follow Jason's suggestion to do reset on vfio unbind, user vfio experience is intact across binding/unbinding drivers. Then when a 2nd driver claiming dma comes but it forgets to do reset on unbind, only manual reset can unblock the device to bind to a new driver. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-04 19:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260812045325.2733631-1-alex.williamson@nvidia.com>
[not found] ` <20260812045325.2733631-2-alex.williamson@nvidia.com>
[not found] ` <an5JbIzz40svS3xe@google.com>
[not found] ` <20260814083737.66bb83fb@nvidia.com>
[not found] ` <20260817121810.GC933791@ziepe.ca>
[not found] ` <CO1PR11MB48354AC2854C4D8E9984FD8E8CA62@CO1PR11MB4835.namprd11.prod.outlook.com>
[not found] ` <20260818140304.GC5482@ziepe.ca>
2026-08-18 20:39 ` [RFC PATCH 1/5] PCI: Refuse function reset of an SR-IOV PF with enabled VFs Alex Williamson
2026-08-20 22:38 ` Samiullah Khawaja
2026-08-20 23:37 ` Jason Gunthorpe
2026-08-21 2:20 ` Samiullah Khawaja
2026-09-04 19:06 ` Jason Gunthorpe
2026-08-27 5:35 ` Tian, Kevin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox