[RFC PATCH] ntb core: copy device archdata on create

[RFC PATCH] ntb core: copy device archdata on create

[Alexander Fomichev]

On non-x86 platforms, e.g. PowerPC, struct device's archdata field is used to
store DMA-related sensitive information. Meanwhile NTB subsystem doesn't copy
that field when creates a child device. Therefore dma_alloc_coherent() fails on
PowerPC. As a result ntb_tool doesn't work.
This is the possible solution.

---
 drivers/ntb/ntb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/ntb/ntb.c b/drivers/ntb/ntb.c
index 2581ab7..e690987 100644
--- a/drivers/ntb/ntb.c
+++ b/drivers/ntb/ntb.c
@@ -114,6 +114,7 @@ int ntb_register_device(struct ntb_dev *ntb)
 	ntb->dev.bus = &ntb_bus;
 	ntb->dev.parent = &ntb->pdev->dev;
 	ntb->dev.release = ntb_dev_release;
+	ntb->dev.archdata = ntb->pdev->dev.archdata;
 	dev_set_name(&ntb->dev, "%s", pci_name(ntb->pdev));
 
 	ntb->ctx = NULL;
-- 
2.7.4

Re: [RFC PATCH] ntb core: copy device archdata on create

[Logan Gunthorpe]

On 2019-01-30 4:20 a.m., Alexander Fomichev wrote:
> On non-x86 platforms, e.g. PowerPC, struct device's archdata field is used to
> store DMA-related sensitive information. Meanwhile NTB subsystem doesn't copy
> that field when creates a child device. Therefore dma_alloc_coherent() fails on
> PowerPC. As a result ntb_tool doesn't work.
> This is the possible solution.

I think you're just playing wack-a-mole at this point and this is not a
good solution as the IOMMU implementations still may not know exactly
what device this is or which IOMMU group we are in. So unless we want to
audit all the IOMMU drivers we shouldn't bother.

We should revert back to using the pci device for DMA allocations and I
already posted a patch for that here [1].

Logan

[1]
https://lore.kernel.org/lkml/20190109192233.5752-3-logang@deltatee.com/T/#u

> ---
>  drivers/ntb/ntb.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/ntb/ntb.c b/drivers/ntb/ntb.c
> index 2581ab7..e690987 100644
> --- a/drivers/ntb/ntb.c
> +++ b/drivers/ntb/ntb.c
> @@ -114,6 +114,7 @@ int ntb_register_device(struct ntb_dev *ntb)
>  	ntb->dev.bus = &ntb_bus;
>  	ntb->dev.parent = &ntb->pdev->dev;
>  	ntb->dev.release = ntb_dev_release;
> +	ntb->dev.archdata = ntb->pdev->dev.archdata;
>  	dev_set_name(&ntb->dev, "%s", pci_name(ntb->pdev));
>  
>  	ntb->ctx = NULL;
> 

Re: [RFC PATCH] ntb core: copy device archdata on create

[Alexander Fomichev]

On Wed, Jan 30, 2019 at 10:25:27AM -0700, Logan Gunthorpe wrote:
> 
> I think you're just playing wack-a-mole at this point and this is not a
> good solution as the IOMMU implementations still may not know exactly
> what device this is or which IOMMU group we are in. So unless we want to
> audit all the IOMMU drivers we shouldn't bother.
> 
> We should revert back to using the pci device for DMA allocations and I
> already posted a patch for that here [1].
> 
> Logan
> 
> [1]
> https://lore.kernel.org/lkml/20190109192233.5752-3-logang@deltatee.com/T/#u
> 

Thanks, Logan. I'll test your solution ASAP.
However I think it's better to separate your patch into "use pci device
struct" part and "move dma_coerce_mask_and_coherent() to ntb core" part.
Especially since the latter is still under discussion.

-- 
Regards,
  Alexander

Re: [RFC PATCH] ntb core: copy device archdata on create

[Logan Gunthorpe]

On 2019-01-31 2:54 a.m., Alexander Fomichev wrote:
> On Wed, Jan 30, 2019 at 10:25:27AM -0700, Logan Gunthorpe wrote:
>>
>> I think you're just playing wack-a-mole at this point and this is not a
>> good solution as the IOMMU implementations still may not know exactly
>> what device this is or which IOMMU group we are in. So unless we want to
>> audit all the IOMMU drivers we shouldn't bother.
>>
>> We should revert back to using the pci device for DMA allocations and I
>> already posted a patch for that here [1].
>>
>> Logan
>>
>> [1]
>> https://lore.kernel.org/lkml/20190109192233.5752-3-logang@deltatee.com/T/#u
>>
> 
> Thanks, Logan. I'll test your solution ASAP.
> However I think it's better to separate your patch into "use pci device
> struct" part and "move dma_coerce_mask_and_coherent() to ntb core" part.
> Especially since the latter is still under discussion.

With that patch we are not moving dma_coerce_mask_and_coherent(); we are
removing it.

If we do DMA operations on the PCI device (as is pretty much universal
in other drivers[1]), then we don't need to set the DMA mask for the
virtual device anymore and thus that patch removes that code -- it does
not move it.

Logan

[1] Just grep the kernel for dma_alloc_coherent; you'll find a lot of
users that reference a 'pdev':

$ kgrep dma_alloc_  | grep pdev | wc -l
568

Re: [RFC PATCH] ntb core: copy device archdata on create

[Alexander Fomichev]

On Thu, Jan 31, 2019 at 10:07:22AM -0700, Logan Gunthorpe wrote:
> 
> 
> > On Wed, Jan 30, 2019 at 10:25:27AM -0700, Logan Gunthorpe wrote:
> >>
> >>
> >> We should revert back to using the pci device for DMA allocations and I
> >> already posted a patch for that here [1].
> >>
> >> Logan
> >>
> >> [1]
> >> https://lore.kernel.org/lkml/20190109192233.5752-3-logang@deltatee.com/T/#u
> >>
> > 
> 
> With that patch we are not moving dma_coerce_mask_and_coherent(); we are
> removing it.
> 
> If we do DMA operations on the PCI device (as is pretty much universal
> in other drivers[1]), then we don't need to set the DMA mask for the
> virtual device anymore and thus that patch removes that code -- it does
> not move it.
> 

Sorry, my bad.
It seems to me that using PCI device struct for DMA-related operations is a
preferred solution. No need to play with dma_mask. And also dma_alloc_* works on
powerpc systems as good as on x86.
If Logan's patchset is applied into mainline kernel then my patch will
be unnecessary.

-- 
Regards,
  Alexander