Linux IOMMU Development
 help / color / mirror / Atom feed
* 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-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

* 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

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