All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources
@ 2020-08-19 13:07 Frederic Barrat
  2020-08-19 13:15 ` Oliver O'Halloran
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Frederic Barrat @ 2020-08-19 13:07 UTC (permalink / raw)
  To: linuxppc-dev, oohall

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 <fbarrat@linux.ibm.com>
---
 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(&pe->table_group, 0);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources
  2020-08-19 13:07 [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources Frederic Barrat
@ 2020-08-19 13:15 ` Oliver O'Halloran
  2020-08-19 13:31   ` Frederic Barrat
  2020-08-20  4:18 ` Michael Ellerman
  2020-08-20 13:31 ` Michael Ellerman
  2 siblings, 1 reply; 6+ messages in thread
From: Oliver O'Halloran @ 2020-08-19 13:15 UTC (permalink / raw)
  To: Frederic Barrat; +Cc: linuxppc-dev

On Wed, Aug 19, 2020 at 11:07 PM Frederic Barrat <fbarrat@linux.ibm.com> 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 <oohall@gmail.com>

> 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 <fbarrat@linux.ibm.com>
> ---
>  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(&pe->table_group, 0);
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources
  2020-08-19 13:15 ` Oliver O'Halloran
@ 2020-08-19 13:31   ` Frederic Barrat
  0 siblings, 0 replies; 6+ messages in thread
From: Frederic Barrat @ 2020-08-19 13:31 UTC (permalink / raw)
  To: Oliver O'Halloran; +Cc: linuxppc-dev



Le 19/08/2020 à 15:15, Oliver O'Halloran a écrit :
> On Wed, Aug 19, 2020 at 11:07 PM Frederic Barrat <fbarrat@linux.ibm.com> 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 <oohall@gmail.com>
> 
>> 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 <fbarrat@linux.ibm.com>
>> ---
>>   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(&pe->table_group, 0);
>> --
>> 2.26.2
>>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources
  2020-08-19 13:07 [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources Frederic Barrat
  2020-08-19 13:15 ` Oliver O'Halloran
@ 2020-08-20  4:18 ` Michael Ellerman
  2020-08-20  7:39   ` Frederic Barrat
  2020-08-20 13:31 ` Michael Ellerman
  2 siblings, 1 reply; 6+ messages in thread
From: Michael Ellerman @ 2020-08-20  4:18 UTC (permalink / raw)
  To: Frederic Barrat, linuxppc-dev, oohall

Frederic Barrat <fbarrat@linux.ibm.com> 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 <fbarrat@linux.ibm.com>

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(&pe->table_group, 0);
> -- 
> 2.26.2

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources
  2020-08-20  4:18 ` Michael Ellerman
@ 2020-08-20  7:39   ` Frederic Barrat
  0 siblings, 0 replies; 6+ messages in thread
From: Frederic Barrat @ 2020-08-20  7:39 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev, oohall



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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources
  2020-08-19 13:07 [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources Frederic Barrat
  2020-08-19 13:15 ` Oliver O'Halloran
  2020-08-20  4:18 ` Michael Ellerman
@ 2020-08-20 13:31 ` Michael Ellerman
  2 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2020-08-20 13:31 UTC (permalink / raw)
  To: oohall, linuxppc-dev, Frederic Barrat

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-08-20 13:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-19 13:07 [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources Frederic Barrat
2020-08-19 13:15 ` Oliver O'Halloran
2020-08-19 13:31   ` Frederic Barrat
2020-08-20  4:18 ` Michael Ellerman
2020-08-20  7:39   ` Frederic Barrat
2020-08-20 13:31 ` Michael Ellerman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.