[PATCH 1/5] RAS, trace: Update error definition format

[PATCH 1/5] RAS, trace: Update error definition format

[Chen, Gong]

Previous format definition uses MACRO BIT(...), which is not very
maintainable. Use unified MACRO as substitution.

Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
---
 include/ras/ras_event.h | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
index 47da53c..0f2cca4 100644
--- a/include/ras/ras_event.h
+++ b/include/ras/ras_event.h
@@ -8,6 +8,7 @@
 #include <linux/tracepoint.h>
 #include <linux/edac.h>
 #include <linux/ktime.h>
+#include <linux/pci.h>
 #include <linux/aer.h>
 #include <linux/cper.h>
 
@@ -174,24 +175,24 @@ TRACE_EVENT(mc_event,
  */
 
 #define aer_correctable_errors		\
-	{BIT(0),	"Receiver Error"},		\
-	{BIT(6),	"Bad TLP"},			\
-	{BIT(7),	"Bad DLLP"},			\
-	{BIT(8),	"RELAY_NUM Rollover"},		\
-	{BIT(12),	"Replay Timer Timeout"},	\
-	{BIT(13),	"Advisory Non-Fatal"}
+	{PCI_ERR_COR_RCVR,	"Receiver Error"},	\
+	{PCI_ERR_COR_BAD_TLP,	"Bad TLP"},		\
+	{PCI_ERR_COR_BAD_DLLP,	"Bad DLLP"},		\
+	{PCI_ERR_COR_REP_ROLL,	"RELAY_NUM Rollover"},	\
+	{PCI_ERR_COR_REP_TIMER,	"Replay Timer Timeout"},\
+	{PCI_ERR_COR_ADV_NFAT,	"Advisory Non-Fatal"}
 
 #define aer_uncorrectable_errors		\
-	{BIT(4),	"Data Link Protocol"},		\
-	{BIT(12),	"Poisoned TLP"},		\
-	{BIT(13),	"Flow Control Protocol"},	\
-	{BIT(14),	"Completion Timeout"},		\
-	{BIT(15),	"Completer Abort"},		\
-	{BIT(16),	"Unexpected Completion"},	\
-	{BIT(17),	"Receiver Overflow"},		\
-	{BIT(18),	"Malformed TLP"},		\
-	{BIT(19),	"ECRC"},			\
-	{BIT(20),	"Unsupported Request"}
+	{PCI_ERR_UNC_DLP,	"Data Link Protocol"},		\
+	{PCI_ERR_UNC_POISON_TLP,"Poisoned TLP"},		\
+	{PCI_ERR_UNC_FCP,	"Flow Control Protocol"},	\
+	{PCI_ERR_UNC_COMP_TIME,	"Completion Timeout"},		\
+	{PCI_ERR_UNC_COMP_ABORT,"Completer Abort"},		\
+	{PCI_ERR_UNC_UNX_COMP,	"Unexpected Completion"},	\
+	{PCI_ERR_UNC_RX_OVER,	"Receiver Overflow"},		\
+	{PCI_ERR_UNC_MALF_TLP,	"Malformed TLP"},		\
+	{PCI_ERR_UNC_ECRC,	"ECRC"},			\
+	{PCI_ERR_UNC_UNSUP,	"Unsupported Request"}
 
 TRACE_EVENT(aer_event,
 	TP_PROTO(const char *dev_name,
-- 
2.0.0.rc2

[PATCH 2/5] PCI-e, AER: Replenish missed AER status bits for AER driver

[Chen, Gong]

Since commit 6c2b374d is commited, the capability of PCI-e AER
has changed a lot. This patch adds all missed CE/UC error bits
existed in PCI-e SPEC r3.0. Meanwhile, adjust the code format
to make it simpler to read/maintain.

Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
---
 drivers/pci/pcie/aer/aerdrv_errprint.c | 60 ++++++++++++++--------------------
 1 file changed, 25 insertions(+), 35 deletions(-)

diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c
index 35d06e1..5c4f7e2 100644
--- a/drivers/pci/pcie/aer/aerdrv_errprint.c
+++ b/drivers/pci/pcie/aer/aerdrv_errprint.c
@@ -75,44 +75,34 @@ static const char *aer_error_layer[] = {
 };
 
 static const char *aer_correctable_error_string[] = {
-	"Receiver Error",		/* Bit Position 0	*/
-	NULL,
-	NULL,
-	NULL,
-	NULL,
-	NULL,
-	"Bad TLP",			/* Bit Position 6	*/
-	"Bad DLLP",			/* Bit Position 7	*/
-	"RELAY_NUM Rollover",		/* Bit Position 8	*/
-	NULL,
-	NULL,
-	NULL,
-	"Replay Timer Timeout",		/* Bit Position 12	*/
-	"Advisory Non-Fatal",		/* Bit Position 13	*/
+	[0] = "Receiver Error",
+	[6] = "Bad TLP",
+	[7] = "Bad DLLP",
+	[8] = "RELAY_NUM Rollover",
+	[12] = "Replay Timer Timeout",
+	[13] = "Advisory Non-Fatal Error",
+	[14] = "Corrected Internal Error",
+	[15] = "Header Log Overflow",
 };
 
 static const char *aer_uncorrectable_error_string[] = {
-	NULL,
-	NULL,
-	NULL,
-	NULL,
-	"Data Link Protocol",		/* Bit Position 4	*/
-	NULL,
-	NULL,
-	NULL,
-	NULL,
-	NULL,
-	NULL,
-	NULL,
-	"Poisoned TLP",			/* Bit Position 12	*/
-	"Flow Control Protocol",	/* Bit Position 13	*/
-	"Completion Timeout",		/* Bit Position 14	*/
-	"Completer Abort",		/* Bit Position 15	*/
-	"Unexpected Completion",	/* Bit Position 16	*/
-	"Receiver Overflow",		/* Bit Position 17	*/
-	"Malformed TLP",		/* Bit Position 18	*/
-	"ECRC",				/* Bit Position 19	*/
-	"Unsupported Request",		/* Bit Position 20	*/
+	[0] = "Undefined",
+	[4] = "Data Link Protocol Error",
+	[5] = "Surprise Down Error",
+	[12] = "Poisoned TLP",
+	[13] = "Flow Control Protocol Error",
+	[14] = "Completion Timeout",
+	[15] = "Completer Abort",
+	[16] = "Unexpected Completion",
+	[17] = "Receiver Overflow",
+	[18] = "Malformed TLP",
+	[19] = "ECRC Error",
+	[20] = "Unsupported Request Error",
+	[21] = "ACS Violation",
+	[22] = "Uncorrectable Internal Error",
+	[23] = "MC Blocked TLP",
+	[24] = "AtomicOp Egress Blocked",
+	[25] = "TLP Prefix Blocked Error",
 };
 
 static const char *aer_agent_string[] = {
-- 
2.0.0.rc2

[PATCH 3/5] PCI-e, trace: Replenish missed AER status bits for PCIE trace I/F

[Chen, Gong]

This patch adds all missed AER error bits(CE & UC) existed in PCI-e
SPEC r3.0 for trace interface.

Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
---
 include/ras/ras_event.h | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
index 0f2cca4..0f04a97 100644
--- a/include/ras/ras_event.h
+++ b/include/ras/ras_event.h
@@ -174,25 +174,34 @@ TRACE_EVENT(mc_event,
  * u8 severity -	error severity 0:NONFATAL 1:FATAL 2:CORRECTED
  */
 
-#define aer_correctable_errors		\
-	{PCI_ERR_COR_RCVR,	"Receiver Error"},	\
-	{PCI_ERR_COR_BAD_TLP,	"Bad TLP"},		\
-	{PCI_ERR_COR_BAD_DLLP,	"Bad DLLP"},		\
-	{PCI_ERR_COR_REP_ROLL,	"RELAY_NUM Rollover"},	\
-	{PCI_ERR_COR_REP_TIMER,	"Replay Timer Timeout"},\
-	{PCI_ERR_COR_ADV_NFAT,	"Advisory Non-Fatal"}
-
-#define aer_uncorrectable_errors		\
-	{PCI_ERR_UNC_DLP,	"Data Link Protocol"},		\
+#define aer_correctable_errors					\
+	{PCI_ERR_COR_RCVR,	"Receiver Error"},		\
+	{PCI_ERR_COR_BAD_TLP,	"Bad TLP"},			\
+	{PCI_ERR_COR_BAD_DLLP,	"Bad DLLP"},			\
+	{PCI_ERR_COR_REP_ROLL,	"RELAY_NUM Rollover"},		\
+	{PCI_ERR_COR_REP_TIMER,	"Replay Timer Timeout"},	\
+	{PCI_ERR_COR_ADV_NFAT,	"Advisory Non-Fatal Error"},	\
+	{PCI_ERR_COR_INTERNAL,	"Corrected Internal Error"},	\
+	{PCI_ERR_COR_LOG_OVER,	"Header Log Overflow"}
+
+#define aer_uncorrectable_errors				\
+	{PCI_ERR_UNC_TRAIN,	"Undefined"},			\
+	{PCI_ERR_UNC_DLP,	"Data Link Protocol Error"},	\
+	{PCI_ERR_UNC_SURPDN,	"Surprise Down Error"},		\
 	{PCI_ERR_UNC_POISON_TLP,"Poisoned TLP"},		\
-	{PCI_ERR_UNC_FCP,	"Flow Control Protocol"},	\
+	{PCI_ERR_UNC_FCP,	"Flow Control Protocol Error"},	\
 	{PCI_ERR_UNC_COMP_TIME,	"Completion Timeout"},		\
 	{PCI_ERR_UNC_COMP_ABORT,"Completer Abort"},		\
 	{PCI_ERR_UNC_UNX_COMP,	"Unexpected Completion"},	\
 	{PCI_ERR_UNC_RX_OVER,	"Receiver Overflow"},		\
 	{PCI_ERR_UNC_MALF_TLP,	"Malformed TLP"},		\
-	{PCI_ERR_UNC_ECRC,	"ECRC"},			\
-	{PCI_ERR_UNC_UNSUP,	"Unsupported Request"}
+	{PCI_ERR_UNC_ECRC,	"ECRC Error"},			\
+	{PCI_ERR_UNC_UNSUP,	"Unsupported Request Error"},	\
+	{PCI_ERR_UNC_ACSV,	"ACS Violation"},		\
+	{PCI_ERR_UNC_INTN,	"Uncorrectable Internal Error"},\
+	{PCI_ERR_UNC_MCBTLP,	"MC Blocked TLP"},		\
+	{PCI_ERR_UNC_ATOMEG,	"AtomicOp Egress Blocked"},	\
+	{PCI_ERR_UNC_TLPPRE,	"TLP Prefix Blocked Error"}
 
 TRACE_EVENT(aer_event,
 	TP_PROTO(const char *dev_name,
-- 
2.0.0.rc2

[PATCH 4/5] PCI-e, AER: Make AER UC status naming clearer

[Chen, Gong]

In PCI-e SPEC r3.0, BIT 0 of Uncorrectable Error Status Register
has been redefined for a different purpose.

BIT 0:
    Undefined – The value read from this bit is undefined. In
		previous versions of this specification, this
		bit was used to mask a Link Training Error. 
		System software must ignore the value read from 
		this bit. System software must only write a value
		of 1b to this bit.

Update related MACRO definition to reflect this change.

Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
---
 drivers/vfio/pci/vfio_pci_config.c | 2 +-
 include/ras/ras_event.h            | 2 +-
 include/uapi/linux/pci_regs.h      | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
index e50790e..1de3f94 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -727,7 +727,7 @@ static int __init init_pci_ext_cap_err_perm(struct perm_bits *perm)
 	p_setd(perm, 0, ALL_VIRT, NO_WRITE);
 
 	/* Writable bits mask */
-	mask =	PCI_ERR_UNC_TRAIN |		/* Training */
+	mask =	PCI_ERR_UNC_UND |		/* Undefined */
 		PCI_ERR_UNC_DLP |		/* Data Link Protocol */
 		PCI_ERR_UNC_SURPDN |		/* Surprise Down */
 		PCI_ERR_UNC_POISON_TLP |	/* Poisoned TLP */
diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
index 0f04a97..79abb9c 100644
--- a/include/ras/ras_event.h
+++ b/include/ras/ras_event.h
@@ -185,7 +185,7 @@ TRACE_EVENT(mc_event,
 	{PCI_ERR_COR_LOG_OVER,	"Header Log Overflow"}
 
 #define aer_uncorrectable_errors				\
-	{PCI_ERR_UNC_TRAIN,	"Undefined"},			\
+	{PCI_ERR_UNC_UND,	"Undefined"},			\
 	{PCI_ERR_UNC_DLP,	"Data Link Protocol Error"},	\
 	{PCI_ERR_UNC_SURPDN,	"Surprise Down Error"},		\
 	{PCI_ERR_UNC_POISON_TLP,"Poisoned TLP"},		\
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 30db069..99e3182 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -630,7 +630,7 @@
 
 /* Advanced Error Reporting */
 #define PCI_ERR_UNCOR_STATUS	4	/* Uncorrectable Error Status */
-#define  PCI_ERR_UNC_TRAIN	0x00000001	/* Training */
+#define  PCI_ERR_UNC_UND	0x00000001	/* Undefined */
 #define  PCI_ERR_UNC_DLP	0x00000010	/* Data Link Protocol */
 #define  PCI_ERR_UNC_SURPDN	0x00000020	/* Surprise Down */
 #define  PCI_ERR_UNC_POISON_TLP	0x00001000	/* Poisoned TLP */
-- 
2.0.0.rc2

[RFC PATCH 5/5] PCI, aer: Update initial value of UC error mask

[Chen, Gong]

In PCI-e SPEC r3.0, BIT 0 of Uncorrectable Error Status Register
is redefined and it has an explicit requirement that when writing
this field, a value of 1b is the only choice. So change previous
initial maks from 0 to 1.

Signed-off-by: Chen, Gong <gong.chen@linux.intel.com>
---
NOTE: After scratching all use cases, this is the most obvious use
case to violate the SPEC. Most of use cases just read first and
then overwrite for clear purpose. Even so, such fix is obvious to
not compatiable with previous SPEC definition. Do we need a dirty
hack?

 arch/mips/pci/pci-octeon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/pci/pci-octeon.c b/arch/mips/pci/pci-octeon.c
index 59cccd9..f1bfdc2 100644
--- a/arch/mips/pci/pci-octeon.c
+++ b/arch/mips/pci/pci-octeon.c
@@ -134,7 +134,7 @@ int pcibios_plat_dev_init(struct pci_dev *dev)
 				       dconfig);
 		/* Enable reporting of all uncorrectable errors */
 		/* Uncorrectable Error Mask - turned on bits disable errors */
-		pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_MASK, 0);
+		pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_MASK, 1);
 		/*
 		 * Leave severity at HW default. This only controls if
 		 * errors are reported as uncorrectable or
-- 
2.0.0.rc2

Re: [PATCH 1/5] RAS, trace: Update error definition format

[Borislav Petkov]

On Wed, Aug 06, 2014 at 04:32:40AM -0400, Chen, Gong wrote:
> Previous format definition uses MACRO BIT(...), which is not very
> maintainable.

What does that "not very maintainable" mean?

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH 1/5] RAS, trace: Update error definition format

[Chen, Gong]

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

On Wed, Aug 06, 2014 at 11:32:23AM +0200, Borislav Petkov wrote:
> Date: Wed, 6 Aug 2014 11:32:23 +0200
> From: Borislav Petkov <bp@alien8.de>
> To: "Chen, Gong" <gong.chen@linux.intel.com>
> Cc: bhelgaas@google.com, rdunlap@infradead.org, tony.luck@intel.com,
>  linux-pci@vger.kernel.org
> Subject: Re: [PATCH 1/5] RAS, trace: Update error definition format
> User-Agent: Mutt/1.5.23 (2014-03-12)
> 
> On Wed, Aug 06, 2014 at 04:32:40AM -0400, Chen, Gong wrote:
> > Previous format definition uses MACRO BIT(...), which is not very
> > maintainable.
> 
> What does that "not very maintainable" mean?
> 
Bjorn ever mentioned for this:
"I'd like to see all those "BIT(...)" things changed to use the #defines
that already exist in include/uapi/linux/pci_regs.h, e.g.,
PCI_ERR_COR_RCVR.  That way grep will find these uses, which will make
maintenance easier."

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/5] RAS, trace: Update error definition format

[Borislav Petkov]

On Wed, Aug 06, 2014 at 05:08:54AM -0400, Chen, Gong wrote:
> Bjorn ever mentioned for this:
> "I'd like to see all those "BIT(...)" things changed to use the #defines
> that already exist in include/uapi/linux/pci_regs.h, e.g.,
> PCI_ERR_COR_RCVR.  That way grep will find these uses, which will make
> maintenance easier."

So explain that in the commit message but don't use some bits out of
context.

In general, when you read your own commit message, always ask yourself
whether other people will be able to understand it, long time from now
and out of context.

If yes, only then send out the patch.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH 1/5] RAS, trace: Update error definition format

[Chen, Gong]

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

On Wed, Aug 06, 2014 at 11:59:39AM +0200, Borislav Petkov wrote:
> > Bjorn ever mentioned for this:
> > "I'd like to see all those "BIT(...)" things changed to use the #defines
> > that already exist in include/uapi/linux/pci_regs.h, e.g.,
> > PCI_ERR_COR_RCVR.  That way grep will find these uses, which will make
> > maintenance easier."
> 
> So explain that in the commit message but don't use some bits out of
> context.
> 
> In general, when you read your own commit message, always ask yourself
> whether other people will be able to understand it, long time from now
> and out of context.
> 
> If yes, only then send out the patch.
> 
Copy that. Thx a lot!

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/5] RAS, trace: Update error definition format

[Chen, Gong]

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

On Wed, Aug 06, 2014 at 04:32:40AM -0400, Chen, Gong wrote:
> Previous format definition uses MACRO BIT(...), which is not very
> maintainable. Use unified MACRO as substitution.
> 
Hi, Bjorn

Any comments for this series?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]