[PATCH kernel] powerpc/iommu: Set default DMA offset in dma_dev_setup

[PATCH kernel] powerpc/iommu: Set default DMA offset in dma_dev_setup

[Alexey Kardashevskiy]

"powerpc/iommu: Cleanup setting of DMA base/offset" expects that
the default DMA offset is set from pnv_ioda_setup_bus_dma() which
is correct unless it is SRIOV where the code flow is different - at
the moment when pnv_ioda_setup_bus_dma() is called, there is no PE for
the just created VF.

This adds missing set_dma_offset() to pnv_pci_ioda_dma_dev_setup()
to cover the case of SRIOV.

Note that we still need set_dma_offset() in pnv_ioda_setup_bus_dma()
as at the boot time pnv_pci_ioda_dma_dev_setup() is called when no PE
was created yet, this happens at the PHB fixup stage.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---

Makes sense to squash it to "powerpc/iommu: Cleanup setting of DMA base/offset".


---
 arch/powerpc/platforms/powernv/pci-ioda.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 2c286b57..af875c1 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1590,6 +1590,7 @@ static void pnv_pci_ioda_dma_dev_setup(struct pnv_phb *phb, struct pci_dev *pdev
 
 	pe = &phb->ioda.pe_array[pdn->pe_number];
 	WARN_ON(get_dma_ops(&pdev->dev) != &dma_iommu_ops);
+	set_dma_offset(&pdev->dev, pe->tce_bypass_base);
 	set_iommu_table_base(&pdev->dev, pe->table_group.tables[0]);
 	/*
 	 * Note: iommu_add_device() will fail here as
-- 
2.4.0.rc3.8.gfb3e7d5

Re: [PATCH kernel] powerpc/iommu: Set default DMA offset in dma_dev_setup

[Gavin Shan]

On Thu, Aug 27, 2015 at 04:01:16PM +1000, Alexey Kardashevskiy wrote:
>"powerpc/iommu: Cleanup setting of DMA base/offset" expects that
>the default DMA offset is set from pnv_ioda_setup_bus_dma() which
>is correct unless it is SRIOV where the code flow is different - at
>the moment when pnv_ioda_setup_bus_dma() is called, there is no PE for
>the just created VF.
>

The PEs for VFs are ready at the moment. We don't have pci_dev for
VFs actually.

>This adds missing set_dma_offset() to pnv_pci_ioda_dma_dev_setup()
>to cover the case of SRIOV.
>
>Note that we still need set_dma_offset() in pnv_ioda_setup_bus_dma()
>as at the boot time pnv_pci_ioda_dma_dev_setup() is called when no PE
>was created yet, this happens at the PHB fixup stage.
>
>Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

Thanks,
Gavin

>---
>
>Makes sense to squash it to "powerpc/iommu: Cleanup setting of DMA base/offset".
>
>
>---
> arch/powerpc/platforms/powernv/pci-ioda.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>index 2c286b57..af875c1 100644
>--- a/arch/powerpc/platforms/powernv/pci-ioda.c
>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>@@ -1590,6 +1590,7 @@ static void pnv_pci_ioda_dma_dev_setup(struct pnv_phb *phb, struct pci_dev *pdev
>
> 	pe = &phb->ioda.pe_array[pdn->pe_number];
> 	WARN_ON(get_dma_ops(&pdev->dev) != &dma_iommu_ops);
>+	set_dma_offset(&pdev->dev, pe->tce_bypass_base);
> 	set_iommu_table_base(&pdev->dev, pe->table_group.tables[0]);
> 	/*
> 	 * Note: iommu_add_device() will fail here as
>-- 
>2.4.0.rc3.8.gfb3e7d5
>

Re: [PATCH kernel] powerpc/iommu: Set default DMA offset in dma_dev_setup

[Alexey Kardashevskiy]

On 08/27/2015 04:16 PM, Gavin Shan wrote:
> On Thu, Aug 27, 2015 at 04:01:16PM +1000, Alexey Kardashevskiy wrote:
>> "powerpc/iommu: Cleanup setting of DMA base/offset" expects that
>> the default DMA offset is set from pnv_ioda_setup_bus_dma() which
>> is correct unless it is SRIOV where the code flow is different - at
>> the moment when pnv_ioda_setup_bus_dma() is called, there is no PE for
>> the just created VF.
>>
>
> The PEs for VFs are ready at the moment. We don't have pci_dev for
> VFs actually.


Ah, you are right. Should have been:

====
"powerpc/iommu: Cleanup setting of DMA base/offset" expects that
the default DMA offset is set from pnv_ioda_setup_bus_dma() which
is correct unless it is SRIOV where the code flow is different - at
the moment when pnv_ioda_setup_bus_dma() is called, PCI devices
for VFs are not created yet.
====


>> This adds missing set_dma_offset() to pnv_pci_ioda_dma_dev_setup()
>> to cover the case of SRIOV.
>>
>> Note that we still need set_dma_offset() in pnv_ioda_setup_bus_dma()
>> as at the boot time pnv_pci_ioda_dma_dev_setup() is called when no PE
>> was created yet, this happens at the PHB fixup stage.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>
> Thanks,
> Gavin
>
>> ---
>>
>> Makes sense to squash it to "powerpc/iommu: Cleanup setting of DMA base/offset".
>>
>>
>> ---
>> arch/powerpc/platforms/powernv/pci-ioda.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>> index 2c286b57..af875c1 100644
>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>> @@ -1590,6 +1590,7 @@ static void pnv_pci_ioda_dma_dev_setup(struct pnv_phb *phb, struct pci_dev *pdev
>>
>> 	pe = &phb->ioda.pe_array[pdn->pe_number];
>> 	WARN_ON(get_dma_ops(&pdev->dev) != &dma_iommu_ops);
>> +	set_dma_offset(&pdev->dev, pe->tce_bypass_base);
>> 	set_iommu_table_base(&pdev->dev, pe->table_group.tables[0]);
>> 	/*
>> 	 * Note: iommu_add_device() will fail here as
>> --
>> 2.4.0.rc3.8.gfb3e7d5
>>
>


-- 
Alexey

Re: [kernel] powerpc/iommu: Set default DMA offset in dma_dev_setup

[Michael Ellerman]

On Thu, 2015-27-08 at 06:01:16 UTC, Alexey Kardashevskiy wrote:
> "powerpc/iommu: Cleanup setting of DMA base/offset" expects that

This should be:

Commit e91c25111aa3 "powerpc/iommu: Cleanup setting of DMA base/offset" ...

> the default DMA offset is set from pnv_ioda_setup_bus_dma() which
> is correct unless it is SRIOV where the code flow is different - at
> the moment when pnv_ioda_setup_bus_dma() is called, there is no PE for
> the just created VF.
> 
> This adds missing set_dma_offset() to pnv_pci_ioda_dma_dev_setup()
> to cover the case of SRIOV.
> 
> Note that we still need set_dma_offset() in pnv_ioda_setup_bus_dma()
> as at the boot time pnv_pci_ioda_dma_dev_setup() is called when no PE
> was created yet, this happens at the PHB fixup stage.

And here we should have:

Fixes: e91c25111aa3 ("powerpc/iommu: Cleanup setting of DMA base/offset")

> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
> 
> Makes sense to squash it to "powerpc/iommu: Cleanup setting of DMA base/offset".

We don't squash things that are already in next.

I've fixed the change log up with my comments above and your reworded
explanation.

cheers

Re: [kernel] powerpc/iommu: Set default DMA offset in dma_dev_setup

[Alexey Kardashevskiy]

On 08/27/2015 07:37 PM, Michael Ellerman wrote:
> On Thu, 2015-27-08 at 06:01:16 UTC, Alexey Kardashevskiy wrote:
>> "powerpc/iommu: Cleanup setting of DMA base/offset" expects that
>
> This should be:
>
> Commit e91c25111aa3 "powerpc/iommu: Cleanup setting of DMA base/offset" ...

Is not this format for reverts (which this patch is not)?

>
>> the default DMA offset is set from pnv_ioda_setup_bus_dma() which
>> is correct unless it is SRIOV where the code flow is different - at
>> the moment when pnv_ioda_setup_bus_dma() is called, there is no PE for
>> the just created VF.
>>
>> This adds missing set_dma_offset() to pnv_pci_ioda_dma_dev_setup()
>> to cover the case of SRIOV.
>>
>> Note that we still need set_dma_offset() in pnv_ioda_setup_bus_dma()
>> as at the boot time pnv_pci_ioda_dma_dev_setup() is called when no PE
>> was created yet, this happens at the PHB fixup stage.
>
> And here we should have:
>
> Fixes: e91c25111aa3 ("powerpc/iommu: Cleanup setting of DMA base/offset")
>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> ---
>>
>> Makes sense to squash it to "powerpc/iommu: Cleanup setting of DMA base/offset".
>
> We don't squash things that are already in next.


imho this screams for rebase. There is no point in having broken patches in 
non merged tree, rebase is a very cheap operation.


>
> I've fixed the change log up with my comments above and your reworded
> explanation.

Thanks!

>
> cheers
>


-- 
Alexey

Re: [kernel] powerpc/iommu: Set default DMA offset in dma_dev_setup

[Michael Ellerman]

On Thu, 2015-27-08 at 06:01:16 UTC, Alexey Kardashevskiy wrote:
> "powerpc/iommu: Cleanup setting of DMA base/offset" expects that
> the default DMA offset is set from pnv_ioda_setup_bus_dma() which
> is correct unless it is SRIOV where the code flow is different - at
> the moment when pnv_ioda_setup_bus_dma() is called, there is no PE for
> the just created VF.
> 
> This adds missing set_dma_offset() to pnv_pci_ioda_dma_dev_setup()
> to cover the case of SRIOV.
> 
> Note that we still need set_dma_offset() in pnv_ioda_setup_bus_dma()
> as at the boot time pnv_pci_ioda_dma_dev_setup() is called when no PE
> was created yet, this happens at the PHB fixup stage.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/0e1ffef02cf94e46f95957af

cheers

Re: [kernel] powerpc/iommu: Set default DMA offset in dma_dev_setup

[Michael Ellerman]

On Thu, 2015-08-27 at 23:07 +1000, Alexey Kardashevskiy wrote:
> On 08/27/2015 07:37 PM, Michael Ellerman wrote:
> > On Thu, 2015-27-08 at 06:01:16 UTC, Alexey Kardashevskiy wrote:
> >> "powerpc/iommu: Cleanup setting of DMA base/offset" expects that
> >
> > This should be:
> >
> > Commit e91c25111aa3 "powerpc/iommu: Cleanup setting of DMA base/offset" ...
> 
> Is not this format for reverts (which this patch is not)?

No, it's for any time you reference a commit in a change log.

> >> Makes sense to squash it to "powerpc/iommu: Cleanup setting of DMA base/offset".
> >
> > We don't squash things that are already in next.
> 
> imho this screams for rebase. There is no point in having broken patches in 
> non merged tree, rebase is a very cheap operation.

You can scream all you like.

Scott has already forked my next as the basis for his next, so if I rebased my
tree we'd get duplicate copies of all of that, and conflicts everywhere.

cheers