Re: [PATCH v4 03/13] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming

2022-01-03 Thread Lu Baolu

On 1/4/22 3:53 AM, Jason Gunthorpe wrote:

On Fri, Dec 31, 2021 at 09:10:43AM +0800, Lu Baolu wrote:


We still need to call iommu_device_use_dma_api() in bus dma_configure()
callback. But we can call iommu_device_unuse_dma_api() in the .probe()
of vfio (and vfio-approved) drivers, so that we don't need the new flag
anymore.


No, we can't. The action that iommu_device_use_dma_api() takes is to
not call probe, it obviously cannot be undone by code inside probe.


Yes. Agreed.


Jason



Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 03/13] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming

2022-01-03 Thread Jason Gunthorpe via iommu
On Fri, Dec 31, 2021 at 09:10:43AM +0800, Lu Baolu wrote:

> We still need to call iommu_device_use_dma_api() in bus dma_configure()
> callback. But we can call iommu_device_unuse_dma_api() in the .probe()
> of vfio (and vfio-approved) drivers, so that we don't need the new flag
> anymore.

No, we can't. The action that iommu_device_use_dma_api() takes is to
not call probe, it obviously cannot be undone by code inside probe.

Jason
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 03/13] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming

2021-12-30 Thread Lu Baolu

On 12/31/21 9:10 AM, Lu Baolu wrote:


On 12/31/21 8:40 AM, Jason Gunthorpe wrote:

On Thu, Dec 30, 2021 at 04:24:14PM -0600, Bjorn Helgaas wrote:


I was speculating that maybe the DMA ownership claiming must be done
*before* the driver's .probe() method?


This is correct.


If DMA ownership could be claimed by the .probe() method, we
wouldn't need the new flag in struct device_driver.


The other requirement is that every existing driver must claim
ownership, so pushing this into the device driver's probe op would
require revising almost every driver in Linux...

In effect the new flag indicates if the driver will do the DMA
ownership claim in it's probe, or should use the default claim the
core code does.

In almost every case a driver should do a claim. A driver like
pci-stub, or a bridge, that doesn't actually operate MMIO on the
device would be the exception.


We still need to call iommu_device_use_dma_api() in bus dma_configure()
callback. But we can call iommu_device_unuse_dma_api() in the .probe()
of vfio (and vfio-approved) drivers, so that we don't need the new flag
anymore.


Oh, wait. I didn't think about the hot-plug case. If we call
iommu_device_use_dma_api() in bus dma_configure() anyway, we can't bind
any (no matter vfio or none-vfio) driver to a device if it's group has
already been assigned to user space. It seems that we can't omit this
flag.

Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 03/13] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming

2021-12-30 Thread Lu Baolu

Hi Jason,

On 12/31/21 8:40 AM, Jason Gunthorpe wrote:

On Thu, Dec 30, 2021 at 04:24:14PM -0600, Bjorn Helgaas wrote:


I was speculating that maybe the DMA ownership claiming must be done
*before* the driver's .probe() method?


This is correct.


If DMA ownership could be claimed by the .probe() method, we
wouldn't need the new flag in struct device_driver.


The other requirement is that every existing driver must claim
ownership, so pushing this into the device driver's probe op would
require revising almost every driver in Linux...

In effect the new flag indicates if the driver will do the DMA
ownership claim in it's probe, or should use the default claim the
core code does.

In almost every case a driver should do a claim. A driver like
pci-stub, or a bridge, that doesn't actually operate MMIO on the
device would be the exception.


We still need to call iommu_device_use_dma_api() in bus dma_configure()
callback. But we can call iommu_device_unuse_dma_api() in the .probe()
of vfio (and vfio-approved) drivers, so that we don't need the new flag
anymore.



Jason



Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 03/13] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming

2021-12-30 Thread Lu Baolu

On 12/31/21 6:24 AM, Bjorn Helgaas wrote:

On Thu, Dec 30, 2021 at 01:34:27PM +0800, Lu Baolu wrote:

Hi Bjorn,

On 12/30/21 4:42 AM, Bjorn Helgaas wrote:

On Fri, Dec 17, 2021 at 02:36:58PM +0800, Lu Baolu wrote:

The pci_dma_configure() marks the iommu_group as containing only devices
with kernel drivers that manage DMA.


I'm looking at pci_dma_configure(), and I don't see the connection to
iommu_groups.


The 2nd patch "driver core: Set DMA ownership during driver bind/unbind"
sets all drivers' DMA to be kernel-managed by default except a few ones
which has a driver flag set. So by default, all iommu groups contains
only devices with kernel drivers managing DMA.


It looks like that happens in device_dma_configure(), not
pci_dma_configure().


Avoid this default behavior for the
pci_stub because it does not program any DMA itself.  This allows the
pci_stub still able to be used by the admin to block driver binding after
applying the DMA ownership to vfio.




Signed-off-by: Lu Baolu 
---
   drivers/pci/pci-stub.c | 3 +++
   1 file changed, 3 insertions(+)

diff --git a/drivers/pci/pci-stub.c b/drivers/pci/pci-stub.c
index e408099fea52..6324c68602b4 100644
--- a/drivers/pci/pci-stub.c
+++ b/drivers/pci/pci-stub.c
@@ -36,6 +36,9 @@ static struct pci_driver stub_driver = {
.name   = "pci-stub",
.id_table   = NULL, /* only dynamic id's */
.probe  = pci_stub_probe,
+   .driver = {
+   .suppress_auto_claim_dma_owner = true,


The new .suppress_auto_claim_dma_owner controls whether we call
iommu_device_set_dma_owner().  I guess you added
.suppress_auto_claim_dma_owner because iommu_device_set_dma_owner()
must be done *before* we call the driver's .probe() method?


As explained above, all drivers are set to kernel-managed dma by
default. For those vfio and vfio-approved drivers,
suppress_auto_claim_dma_owner is used to tell the driver core that "this
driver is attached to device for userspace assignment purpose, do not
claim it for kernel-management dma".


Otherwise, we could call some new interface from .probe() instead of
adding the flag to struct device_driver.


Most device drivers are of the kernel-managed DMA type. Only a few vfio
and vfio-approved drivers need to use this flag. That's the reason why
we claim kernel-managed DMA by default.


Yes.  But you didn't answer the question of whether this must be done
by a new flag in struct device_driver, or whether it could be done by
having these few VFIO and "VFIO-approved" (whatever that means)
drivers call a new interface.

I was speculating that maybe the DMA ownership claiming must be done
*before* the driver's .probe() method?  If so, that would require a
new flag.  But I don't know whether that's the case.  If DMA
ownership could be claimed by the .probe() method, we wouldn't need
the new flag in struct device_driver.


Yes. It's feasible. Hence we can remove the suppress flag which is only
for some special drivers. I will come up with a new version so that you
can further comment with the real code. Thank you!



Bjorn



Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 03/13] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming

2021-12-30 Thread Jason Gunthorpe via iommu
On Thu, Dec 30, 2021 at 04:24:14PM -0600, Bjorn Helgaas wrote:

> I was speculating that maybe the DMA ownership claiming must be done
> *before* the driver's .probe() method?  

This is correct.

> If DMA ownership could be claimed by the .probe() method, we
> wouldn't need the new flag in struct device_driver.

The other requirement is that every existing driver must claim
ownership, so pushing this into the device driver's probe op would
require revising almost every driver in Linux...

In effect the new flag indicates if the driver will do the DMA
ownership claim in it's probe, or should use the default claim the
core code does.

In almost every case a driver should do a claim. A driver like
pci-stub, or a bridge, that doesn't actually operate MMIO on the
device would be the exception.

Jason
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 03/13] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming

2021-12-30 Thread Bjorn Helgaas
On Thu, Dec 30, 2021 at 01:34:27PM +0800, Lu Baolu wrote:
> Hi Bjorn,
> 
> On 12/30/21 4:42 AM, Bjorn Helgaas wrote:
> > On Fri, Dec 17, 2021 at 02:36:58PM +0800, Lu Baolu wrote:
> > > The pci_dma_configure() marks the iommu_group as containing only devices
> > > with kernel drivers that manage DMA.
> > 
> > I'm looking at pci_dma_configure(), and I don't see the connection to
> > iommu_groups.
> 
> The 2nd patch "driver core: Set DMA ownership during driver bind/unbind"
> sets all drivers' DMA to be kernel-managed by default except a few ones
> which has a driver flag set. So by default, all iommu groups contains
> only devices with kernel drivers managing DMA.

It looks like that happens in device_dma_configure(), not
pci_dma_configure().

> > > Avoid this default behavior for the
> > > pci_stub because it does not program any DMA itself.  This allows the
> > > pci_stub still able to be used by the admin to block driver binding after
> > > applying the DMA ownership to vfio.
> > 
> > > 
> > > Signed-off-by: Lu Baolu 
> > > ---
> > >   drivers/pci/pci-stub.c | 3 +++
> > >   1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/pci/pci-stub.c b/drivers/pci/pci-stub.c
> > > index e408099fea52..6324c68602b4 100644
> > > --- a/drivers/pci/pci-stub.c
> > > +++ b/drivers/pci/pci-stub.c
> > > @@ -36,6 +36,9 @@ static struct pci_driver stub_driver = {
> > >   .name   = "pci-stub",
> > >   .id_table   = NULL, /* only dynamic id's */
> > >   .probe  = pci_stub_probe,
> > > + .driver = {
> > > + .suppress_auto_claim_dma_owner = true,
> > 
> > The new .suppress_auto_claim_dma_owner controls whether we call
> > iommu_device_set_dma_owner().  I guess you added
> > .suppress_auto_claim_dma_owner because iommu_device_set_dma_owner()
> > must be done *before* we call the driver's .probe() method?
> 
> As explained above, all drivers are set to kernel-managed dma by
> default. For those vfio and vfio-approved drivers,
> suppress_auto_claim_dma_owner is used to tell the driver core that "this
> driver is attached to device for userspace assignment purpose, do not
> claim it for kernel-management dma".
> 
> > Otherwise, we could call some new interface from .probe() instead of
> > adding the flag to struct device_driver.
> 
> Most device drivers are of the kernel-managed DMA type. Only a few vfio
> and vfio-approved drivers need to use this flag. That's the reason why
> we claim kernel-managed DMA by default.

Yes.  But you didn't answer the question of whether this must be done
by a new flag in struct device_driver, or whether it could be done by
having these few VFIO and "VFIO-approved" (whatever that means)
drivers call a new interface.

I was speculating that maybe the DMA ownership claiming must be done
*before* the driver's .probe() method?  If so, that would require a
new flag.  But I don't know whether that's the case.  If DMA
ownership could be claimed by the .probe() method, we wouldn't need
the new flag in struct device_driver.

Bjorn
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 03/13] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming

2021-12-29 Thread Lu Baolu

Hi Bjorn,

On 12/30/21 4:42 AM, Bjorn Helgaas wrote:

On Fri, Dec 17, 2021 at 02:36:58PM +0800, Lu Baolu wrote:

The pci_dma_configure() marks the iommu_group as containing only devices
with kernel drivers that manage DMA.


I'm looking at pci_dma_configure(), and I don't see the connection to
iommu_groups.


The 2nd patch "driver core: Set DMA ownership during driver bind/unbind"
sets all drivers' DMA to be kernel-managed by default except a few ones
which has a driver flag set. So by default, all iommu groups contains
only devices with kernel drivers managing DMA.




Avoid this default behavior for the
pci_stub because it does not program any DMA itself.  This allows the
pci_stub still able to be used by the admin to block driver binding after
applying the DMA ownership to vfio.




Signed-off-by: Lu Baolu 
---
  drivers/pci/pci-stub.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/pci/pci-stub.c b/drivers/pci/pci-stub.c
index e408099fea52..6324c68602b4 100644
--- a/drivers/pci/pci-stub.c
+++ b/drivers/pci/pci-stub.c
@@ -36,6 +36,9 @@ static struct pci_driver stub_driver = {
.name   = "pci-stub",
.id_table   = NULL, /* only dynamic id's */
.probe  = pci_stub_probe,
+   .driver = {
+   .suppress_auto_claim_dma_owner = true,


The new .suppress_auto_claim_dma_owner controls whether we call
iommu_device_set_dma_owner().  I guess you added
.suppress_auto_claim_dma_owner because iommu_device_set_dma_owner()
must be done *before* we call the driver's .probe() method?


As explained above, all drivers are set to kernel-managed dma by
default. For those vfio and vfio-approved drivers,
suppress_auto_claim_dma_owner is used to tell the driver core that "this
driver is attached to device for userspace assignment purpose, do not
claim it for kernel-management dma".



Otherwise, we could call some new interface from .probe() instead of
adding the flag to struct device_driver.


Most device drivers are of the kernel-managed DMA type. Only a few vfio
and vfio-approved drivers need to use this flag. That's the reason why
we claim kernel-managed DMA by default.




+   },
  };
  
  static int __init pci_stub_init(void)

--
2.25.1



Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 03/13] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming

2021-12-29 Thread Bjorn Helgaas
On Fri, Dec 17, 2021 at 02:36:58PM +0800, Lu Baolu wrote:
> The pci_dma_configure() marks the iommu_group as containing only devices
> with kernel drivers that manage DMA.

I'm looking at pci_dma_configure(), and I don't see the connection to
iommu_groups.

> Avoid this default behavior for the
> pci_stub because it does not program any DMA itself.  This allows the
> pci_stub still able to be used by the admin to block driver binding after
> applying the DMA ownership to vfio.

> 
> Signed-off-by: Lu Baolu 
> ---
>  drivers/pci/pci-stub.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pci/pci-stub.c b/drivers/pci/pci-stub.c
> index e408099fea52..6324c68602b4 100644
> --- a/drivers/pci/pci-stub.c
> +++ b/drivers/pci/pci-stub.c
> @@ -36,6 +36,9 @@ static struct pci_driver stub_driver = {
>   .name   = "pci-stub",
>   .id_table   = NULL, /* only dynamic id's */
>   .probe  = pci_stub_probe,
> + .driver = {
> + .suppress_auto_claim_dma_owner = true,

The new .suppress_auto_claim_dma_owner controls whether we call
iommu_device_set_dma_owner().  I guess you added
.suppress_auto_claim_dma_owner because iommu_device_set_dma_owner()
must be done *before* we call the driver's .probe() method?

Otherwise, we could call some new interface from .probe() instead of
adding the flag to struct device_driver.

> + },
>  };
>  
>  static int __init pci_stub_init(void)
> -- 
> 2.25.1
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v4 03/13] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming

2021-12-16 Thread Lu Baolu
The pci_dma_configure() marks the iommu_group as containing only devices
with kernel drivers that manage DMA. Avoid this default behavior for the
pci_stub because it does not program any DMA itself.  This allows the
pci_stub still able to be used by the admin to block driver binding after
applying the DMA ownership to vfio.

Signed-off-by: Lu Baolu 
---
 drivers/pci/pci-stub.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/pci-stub.c b/drivers/pci/pci-stub.c
index e408099fea52..6324c68602b4 100644
--- a/drivers/pci/pci-stub.c
+++ b/drivers/pci/pci-stub.c
@@ -36,6 +36,9 @@ static struct pci_driver stub_driver = {
.name   = "pci-stub",
.id_table   = NULL, /* only dynamic id's */
.probe  = pci_stub_probe,
+   .driver = {
+   .suppress_auto_claim_dma_owner = true,
+   },
 };
 
 static int __init pci_stub_init(void)
-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu