Re: [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources
On Wed, 19 Aug 2020 15:07:41 +0200, Frederic Barrat wrote: > Fix typo introduced during recent code cleanup, which could lead to > silently not freeing resources or oops message (on PCI hotplug or CAPI > reset). > Only impacts ioda2, the code path for ioda1 is correct. Applied to powerpc/fixes. [1/1] powerpc/powernv/pci: Fix possible crash when releasing DMA resources https://git.kernel.org/powerpc/c/e17a7c0e0aebb956719ce2a8465f649859c2da7d cheers
Re: [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources
Le 20/08/2020 à 06:18, Michael Ellerman a écrit : I changed the subject to: powerpc/powernv/pci: Fix possible crash when releasing DMA resources Much better, thanks! Fred
Re: [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources
Frederic Barrat writes:
> Fix typo introduced during recent code cleanup, which could lead to
> silently not freeing resources or oops message (on PCI hotplug or CAPI
> reset).
> Only impacts ioda2, the code path for ioda1 is correct.
>
> Fixes: 01e12629af4e ("powerpc/powernv/pci: Add explicit tracking of the DMA
> setup state")
> Signed-off-by: Frederic Barrat
I changed the subject to:
powerpc/powernv/pci: Fix possible crash when releasing DMA resources
To hopefully better convey that it's a moderately serious bug, even if
the root cause is just a typo. Otherwise folks scanning the commit log
might think it's just a harmless typo.
cheers
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
> b/arch/powerpc/platforms/powernv/pci-ioda.c
> index c9c25fb0783c..023a4f987bb2 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -2705,7 +2705,7 @@ void pnv_pci_ioda2_release_pe_dma(struct pnv_ioda_pe
> *pe)
> struct iommu_table *tbl = pe->table_group.tables[0];
> int64_t rc;
>
> - if (pe->dma_setup_done)
> + if (!pe->dma_setup_done)
> return;
>
> rc = pnv_pci_ioda2_unset_window(>table_group, 0);
> --
> 2.26.2
Re: [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources
Le 19/08/2020 à 15:15, Oliver O'Halloran a écrit :
On Wed, Aug 19, 2020 at 11:07 PM Frederic Barrat wrote:
Fix typo introduced during recent code cleanup, which could lead to
silently not freeing resources or oops message (on PCI hotplug or CAPI
reset).
oof
Did you actually hit that oops on CAPI reset? Including the stack
trace is helpful for avoiding this sort of problem in the future.
Anyway,
yeah, I found it with capi reset. It's actually not a oops, we hit the
WARN_ON in iommu_tce_table_put(), when we try to release the DMA
resource for a PE where it was never configured to start with. So that
was easy to track.
Fred
Reviewed-by: Oliver O'Halloran
Only impacts ioda2, the code path for ioda1 is correct.
yeah but no ones uses ioda1
Fixes: 01e12629af4e ("powerpc/powernv/pci: Add explicit tracking of the DMA setup
state")
Signed-off-by: Frederic Barrat
---
arch/powerpc/platforms/powernv/pci-ioda.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index c9c25fb0783c..023a4f987bb2 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -2705,7 +2705,7 @@ void pnv_pci_ioda2_release_pe_dma(struct pnv_ioda_pe *pe)
struct iommu_table *tbl = pe->table_group.tables[0];
int64_t rc;
- if (pe->dma_setup_done)
+ if (!pe->dma_setup_done)
return;
rc = pnv_pci_ioda2_unset_window(>table_group, 0);
--
2.26.2
Re: [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources
On Wed, Aug 19, 2020 at 11:07 PM Frederic Barrat wrote:
>
> Fix typo introduced during recent code cleanup, which could lead to
> silently not freeing resources or oops message (on PCI hotplug or CAPI
> reset).
oof
Did you actually hit that oops on CAPI reset? Including the stack
trace is helpful for avoiding this sort of problem in the future.
Anyway,
Reviewed-by: Oliver O'Halloran
> Only impacts ioda2, the code path for ioda1 is correct.
yeah but no ones uses ioda1
> Fixes: 01e12629af4e ("powerpc/powernv/pci: Add explicit tracking of the DMA
> setup state")
> Signed-off-by: Frederic Barrat
> ---
> arch/powerpc/platforms/powernv/pci-ioda.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
> b/arch/powerpc/platforms/powernv/pci-ioda.c
> index c9c25fb0783c..023a4f987bb2 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -2705,7 +2705,7 @@ void pnv_pci_ioda2_release_pe_dma(struct pnv_ioda_pe
> *pe)
> struct iommu_table *tbl = pe->table_group.tables[0];
> int64_t rc;
>
> - if (pe->dma_setup_done)
> + if (!pe->dma_setup_done)
> return;
>
> rc = pnv_pci_ioda2_unset_window(>table_group, 0);
> --
> 2.26.2
>
[PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources
Fix typo introduced during recent code cleanup, which could lead to
silently not freeing resources or oops message (on PCI hotplug or CAPI
reset).
Only impacts ioda2, the code path for ioda1 is correct.
Fixes: 01e12629af4e ("powerpc/powernv/pci: Add explicit tracking of the DMA
setup state")
Signed-off-by: Frederic Barrat
---
arch/powerpc/platforms/powernv/pci-ioda.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index c9c25fb0783c..023a4f987bb2 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -2705,7 +2705,7 @@ void pnv_pci_ioda2_release_pe_dma(struct pnv_ioda_pe *pe)
struct iommu_table *tbl = pe->table_group.tables[0];
int64_t rc;
- if (pe->dma_setup_done)
+ if (!pe->dma_setup_done)
return;
rc = pnv_pci_ioda2_unset_window(>table_group, 0);
--
2.26.2
