All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] PCI/DPC: Fix TLP Prefix register reading offset
@ 2024-01-18 11:08 Ilpo Järvinen
  2024-01-19 22:59   ` Bjorn Helgaas
  2024-01-22 18:16   ` Bjorn Helgaas
  0 siblings, 2 replies; 7+ messages in thread
From: Ilpo Järvinen @ 2024-01-18 11:08 UTC (permalink / raw)
  To: Mahesh J Salgaonkar, Oliver O'Halloran, Bjorn Helgaas,
	Keith Busch, Dongdong Liu, linuxppc-dev, linux-pci, linux-kernel
  Cc: Ilpo Järvinen

The TLP Prefix Log Register consists of multiple DWORDs (PCIe r6.1 sec
7.9.14.13) but the loop in dpc_process_rp_pio_error() keeps reading
from the first DWORD. Add the iteration count based offset calculation
into the config read.

Fixes: f20c4ea49ec4 ("PCI/DPC: Add eDPC support")
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/pcie/dpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index 94111e438241..e5d7c12854fa 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -234,7 +234,7 @@ static void dpc_process_rp_pio_error(struct pci_dev *pdev)
 
 	for (i = 0; i < pdev->dpc_rp_log_size - 5; i++) {
 		pci_read_config_dword(pdev,
-			cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG, &prefix);
+			cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG + i * 4, &prefix);
 		pci_err(pdev, "TLP Prefix Header: dw%d, %#010x\n", i, prefix);
 	}
  clear_status:
-- 
2.39.2


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

* Re: [PATCH 1/1] PCI/DPC: Fix TLP Prefix register reading offset
  2024-01-18 11:08 [PATCH 1/1] PCI/DPC: Fix TLP Prefix register reading offset Ilpo Järvinen
@ 2024-01-19 22:59   ` Bjorn Helgaas
  2024-01-22 18:16   ` Bjorn Helgaas
  1 sibling, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2024-01-19 22:59 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Mahesh J Salgaonkar, Oliver O'Halloran, Bjorn Helgaas,
	Keith Busch, Dongdong Liu, linuxppc-dev, linux-pci, linux-kernel

On Thu, Jan 18, 2024 at 01:08:15PM +0200, Ilpo Järvinen wrote:
> The TLP Prefix Log Register consists of multiple DWORDs (PCIe r6.1 sec
> 7.9.14.13) but the loop in dpc_process_rp_pio_error() keeps reading
> from the first DWORD. Add the iteration count based offset calculation
> into the config read.

So IIUC the user-visible bug is that we print only the first PIO TLP
Prefix (duplicated several times), and we never print the second,
third, etc Prefixes, right?

I wish we could print them all in a single pci_err(), as we do for the
TLP Header Log, instead of dribbling them out one by one.

> Fixes: f20c4ea49ec4 ("PCI/DPC: Add eDPC support")
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  drivers/pci/pcie/dpc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index 94111e438241..e5d7c12854fa 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -234,7 +234,7 @@ static void dpc_process_rp_pio_error(struct pci_dev *pdev)
>  
>  	for (i = 0; i < pdev->dpc_rp_log_size - 5; i++) {
>  		pci_read_config_dword(pdev,
> -			cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG, &prefix);
> +			cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG + i * 4, &prefix);
>  		pci_err(pdev, "TLP Prefix Header: dw%d, %#010x\n", i, prefix);
>  	}
>   clear_status:
> -- 
> 2.39.2
> 

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

* Re: [PATCH 1/1] PCI/DPC: Fix TLP Prefix register reading offset
@ 2024-01-19 22:59   ` Bjorn Helgaas
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2024-01-19 22:59 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: linux-pci, Mahesh J Salgaonkar, linux-kernel, Keith Busch,
	Oliver O'Halloran, Dongdong Liu, Bjorn Helgaas, linuxppc-dev

On Thu, Jan 18, 2024 at 01:08:15PM +0200, Ilpo Järvinen wrote:
> The TLP Prefix Log Register consists of multiple DWORDs (PCIe r6.1 sec
> 7.9.14.13) but the loop in dpc_process_rp_pio_error() keeps reading
> from the first DWORD. Add the iteration count based offset calculation
> into the config read.

So IIUC the user-visible bug is that we print only the first PIO TLP
Prefix (duplicated several times), and we never print the second,
third, etc Prefixes, right?

I wish we could print them all in a single pci_err(), as we do for the
TLP Header Log, instead of dribbling them out one by one.

> Fixes: f20c4ea49ec4 ("PCI/DPC: Add eDPC support")
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  drivers/pci/pcie/dpc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index 94111e438241..e5d7c12854fa 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -234,7 +234,7 @@ static void dpc_process_rp_pio_error(struct pci_dev *pdev)
>  
>  	for (i = 0; i < pdev->dpc_rp_log_size - 5; i++) {
>  		pci_read_config_dword(pdev,
> -			cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG, &prefix);
> +			cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG + i * 4, &prefix);
>  		pci_err(pdev, "TLP Prefix Header: dw%d, %#010x\n", i, prefix);
>  	}
>   clear_status:
> -- 
> 2.39.2
> 

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

* Re: [PATCH 1/1] PCI/DPC: Fix TLP Prefix register reading offset
  2024-01-19 22:59   ` Bjorn Helgaas
@ 2024-01-22 12:20     ` Ilpo Järvinen
  -1 siblings, 0 replies; 7+ messages in thread
From: Ilpo Järvinen @ 2024-01-22 12:20 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Mahesh J Salgaonkar, Oliver O'Halloran, Bjorn Helgaas,
	Keith Busch, Dongdong Liu, linuxppc-dev, linux-pci, LKML

[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]

On Fri, 19 Jan 2024, Bjorn Helgaas wrote:

> On Thu, Jan 18, 2024 at 01:08:15PM +0200, Ilpo Järvinen wrote:
> > The TLP Prefix Log Register consists of multiple DWORDs (PCIe r6.1 sec
> > 7.9.14.13) but the loop in dpc_process_rp_pio_error() keeps reading
> > from the first DWORD. Add the iteration count based offset calculation
> > into the config read.
> 
> So IIUC the user-visible bug is that we print only the first PIO TLP
> Prefix (duplicated several times), and we never print the second,
> third, etc Prefixes, right?

Yes.

> I wish we could print them all in a single pci_err(), as we do for the
> TLP Header Log, instead of dribbling them out one by one.

I've also done some work towards consolidating AER and DPC TLP 
Header/Prefix Log handling which is when I found this bug (the reading 
side is already done but printing is still pending).

> > Fixes: f20c4ea49ec4 ("PCI/DPC: Add eDPC support")
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

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

* Re: [PATCH 1/1] PCI/DPC: Fix TLP Prefix register reading offset
@ 2024-01-22 12:20     ` Ilpo Järvinen
  0 siblings, 0 replies; 7+ messages in thread
From: Ilpo Järvinen @ 2024-01-22 12:20 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, Mahesh J Salgaonkar, LKML, Keith Busch,
	Oliver O'Halloran, Dongdong Liu, Bjorn Helgaas, linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]

On Fri, 19 Jan 2024, Bjorn Helgaas wrote:

> On Thu, Jan 18, 2024 at 01:08:15PM +0200, Ilpo Järvinen wrote:
> > The TLP Prefix Log Register consists of multiple DWORDs (PCIe r6.1 sec
> > 7.9.14.13) but the loop in dpc_process_rp_pio_error() keeps reading
> > from the first DWORD. Add the iteration count based offset calculation
> > into the config read.
> 
> So IIUC the user-visible bug is that we print only the first PIO TLP
> Prefix (duplicated several times), and we never print the second,
> third, etc Prefixes, right?

Yes.

> I wish we could print them all in a single pci_err(), as we do for the
> TLP Header Log, instead of dribbling them out one by one.

I've also done some work towards consolidating AER and DPC TLP 
Header/Prefix Log handling which is when I found this bug (the reading 
side is already done but printing is still pending).

> > Fixes: f20c4ea49ec4 ("PCI/DPC: Add eDPC support")
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

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

* Re: [PATCH 1/1] PCI/DPC: Fix TLP Prefix register reading offset
  2024-01-18 11:08 [PATCH 1/1] PCI/DPC: Fix TLP Prefix register reading offset Ilpo Järvinen
@ 2024-01-22 18:16   ` Bjorn Helgaas
  2024-01-22 18:16   ` Bjorn Helgaas
  1 sibling, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2024-01-22 18:16 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Mahesh J Salgaonkar, Oliver O'Halloran, Bjorn Helgaas,
	Keith Busch, Dongdong Liu, linuxppc-dev, linux-pci, linux-kernel

On Thu, Jan 18, 2024 at 01:08:15PM +0200, Ilpo Järvinen wrote:
> The TLP Prefix Log Register consists of multiple DWORDs (PCIe r6.1 sec
> 7.9.14.13) but the loop in dpc_process_rp_pio_error() keeps reading
> from the first DWORD. Add the iteration count based offset calculation
> into the config read.
> 
> Fixes: f20c4ea49ec4 ("PCI/DPC: Add eDPC support")
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

Applied to pci/dpc for v6.9 with commit log below, thanks!

    PCI/DPC: Print all TLP Prefixes, not just the first
    
    The TLP Prefix Log Register consists of multiple DWORDs (PCIe r6.1 sec
    7.9.14.13) but the loop in dpc_process_rp_pio_error() keeps reading from
    the first DWORD, so we print only the first PIO TLP Prefix (duplicated
    several times), and we never print the second, third, etc., Prefixes.
    
    Add the iteration count based offset calculation into the config read.
    
    Fixes: f20c4ea49ec4 ("PCI/DPC: Add eDPC support")
    Link: https://lore.kernel.org/r/20240118110815.3867-1-ilpo.jarvinen@linux.intel.com
    Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
    [bhelgaas: add user-visible details to commit log]
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/pci/pcie/dpc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index 94111e438241..e5d7c12854fa 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -234,7 +234,7 @@ static void dpc_process_rp_pio_error(struct pci_dev *pdev)
>  
>  	for (i = 0; i < pdev->dpc_rp_log_size - 5; i++) {
>  		pci_read_config_dword(pdev,
> -			cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG, &prefix);
> +			cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG + i * 4, &prefix);
>  		pci_err(pdev, "TLP Prefix Header: dw%d, %#010x\n", i, prefix);
>  	}
>   clear_status:
> -- 
> 2.39.2
> 

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

* Re: [PATCH 1/1] PCI/DPC: Fix TLP Prefix register reading offset
@ 2024-01-22 18:16   ` Bjorn Helgaas
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2024-01-22 18:16 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: linux-pci, Mahesh J Salgaonkar, linux-kernel, Keith Busch,
	Oliver O'Halloran, Dongdong Liu, Bjorn Helgaas, linuxppc-dev

On Thu, Jan 18, 2024 at 01:08:15PM +0200, Ilpo Järvinen wrote:
> The TLP Prefix Log Register consists of multiple DWORDs (PCIe r6.1 sec
> 7.9.14.13) but the loop in dpc_process_rp_pio_error() keeps reading
> from the first DWORD. Add the iteration count based offset calculation
> into the config read.
> 
> Fixes: f20c4ea49ec4 ("PCI/DPC: Add eDPC support")
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

Applied to pci/dpc for v6.9 with commit log below, thanks!

    PCI/DPC: Print all TLP Prefixes, not just the first
    
    The TLP Prefix Log Register consists of multiple DWORDs (PCIe r6.1 sec
    7.9.14.13) but the loop in dpc_process_rp_pio_error() keeps reading from
    the first DWORD, so we print only the first PIO TLP Prefix (duplicated
    several times), and we never print the second, third, etc., Prefixes.
    
    Add the iteration count based offset calculation into the config read.
    
    Fixes: f20c4ea49ec4 ("PCI/DPC: Add eDPC support")
    Link: https://lore.kernel.org/r/20240118110815.3867-1-ilpo.jarvinen@linux.intel.com
    Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
    [bhelgaas: add user-visible details to commit log]
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/pci/pcie/dpc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index 94111e438241..e5d7c12854fa 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -234,7 +234,7 @@ static void dpc_process_rp_pio_error(struct pci_dev *pdev)
>  
>  	for (i = 0; i < pdev->dpc_rp_log_size - 5; i++) {
>  		pci_read_config_dword(pdev,
> -			cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG, &prefix);
> +			cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG + i * 4, &prefix);
>  		pci_err(pdev, "TLP Prefix Header: dw%d, %#010x\n", i, prefix);
>  	}
>   clear_status:
> -- 
> 2.39.2
> 

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

end of thread, other threads:[~2024-01-22 18:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-18 11:08 [PATCH 1/1] PCI/DPC: Fix TLP Prefix register reading offset Ilpo Järvinen
2024-01-19 22:59 ` Bjorn Helgaas
2024-01-19 22:59   ` Bjorn Helgaas
2024-01-22 12:20   ` Ilpo Järvinen
2024-01-22 12:20     ` Ilpo Järvinen
2024-01-22 18:16 ` Bjorn Helgaas
2024-01-22 18:16   ` Bjorn Helgaas

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.