Skip to contentSkip to conversation

PCI: aer_inject cleanups and verbosity

PATCH Applied Jean Delvare 2 participants 6 messages Feb 18, 2016 – Mar 8, 2016 PCI

lore
Share & tools
lore

Jean Delvare

author
#0 lore raw

cover PCI: aer_inject cleanups and verbosity

As discussed earlier, here are a few cleanups and verbosity adjustments to the aer_inject driver.

[PATCH 1/4] PCI: aer_inject: Fix error codes [PATCH 2/4] PCI: aer_inject: Use dev_warn() [PATCH 3/4] PCI: aer_inject: Log actual error causes [PATCH 4/4] PCI: aer_inject: Log error injections

Signature · 2 lines
Jean Delvare
SUSE L3 Support
author
#1 lore raw

PATCH 1/4 PCI: aer_inject: Fix error codes

EPERM means "Operation not permitted", which doesn't reflect the lack of support for AER. EPROTONOSUPPORT (Protocol not supported) is a better choice or error code if the device or its root port lack support for AER.

Likewise, EINVAL means "Invalid argument", which is not suitable for cases where the AER error device is missing or unusable. ENODEV and EPROTONOSUPPORT, respectively, fit better.

Suggested-by Borislav Petkov <[email protected]>
Signed-off-by Jean Delvare <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Prarit Bhargava <[email protected]>
---
 drivers/pci/pcie/aer/aer_inject.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/pci/pcie/aer/aer_inject.c +4 -4
@@ -340,7 +340,7 @@ static int aer_inject(struct aer_error_i
 
 	pos_cap_err = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
 	if (!pos_cap_err) {
-		ret = -EPERM;
+		ret = -EPROTONOSUPPORT;
 		goto out_put;
 	}
 	pci_read_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_SEVER, &sever);

@@ -350,7 +350,7 @@ static int aer_inject(struct aer_error_i
 
 	rp_pos_cap_err = pci_find_ext_capability(rpdev, PCI_EXT_CAP_ID_ERR);
 	if (!rp_pos_cap_err) {
-		ret = -EPERM;
+		ret = -EPROTONOSUPPORT;
 		goto out_put;
 	}
 

@@ -458,12 +458,12 @@ static int aer_inject(struct aer_error_i
 	if (find_aer_device(rpdev, &edev)) {
 		if (!get_service_data(edev)) {
 			printk(KERN_WARNING "AER service is not initialized\n");
-			ret = -EINVAL;
+			ret = -EPROTONOSUPPORT;
 			goto out_put;
 		}
 		aer_irq(-1, edev);
 	} else
-		ret = -EINVAL;
+		ret = -ENODEV;
 out_put:
 	kfree(err_alloc);
 	kfree(rperr_alloc);
Signature · 2 lines
Jean Delvare
SUSE L3 Support
author
#2 lore raw

PATCH 2/4 PCI: aer_inject: Use dev_warn()

In reply to Jean Delvare

dev_warn() is better than printk(LOG_WARNING...) as it records which device the message relates to. Also add a prefix "aer_inject:" to help differentiate real errors from injected errors.

Signed-off-by Jean Delvare <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Bjorn Helgaas <[email protected]>
---
 drivers/pci/pcie/aer/aer_inject.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
drivers/pci/pcie/aer/aer_inject.c +7 -3
@@ -25,6 +25,7 @@
 #include <linux/fs.h>
 #include <linux/uaccess.h>
 #include <linux/stddef.h>
+#include <linux/device.h>
 #include "aerdrv.h"
 
 /* Override the existing corrected and uncorrected error masks */

@@ -397,14 +398,16 @@ static int aer_inject(struct aer_error_i
 	if (!aer_mask_override && einj->cor_status &&
 	    !(einj->cor_status & ~cor_mask)) {
 		ret = -EINVAL;
-		printk(KERN_WARNING "The correctable error(s) is masked by device\n");
+		dev_warn(&dev->dev,
+			 "aer_inject: The correctable error(s) is masked by device\n");
 		spin_unlock_irqrestore(&inject_lock, flags);
 		goto out_put;
 	}
 	if (!aer_mask_override && einj->uncor_status &&
 	    !(einj->uncor_status & ~uncor_mask)) {
 		ret = -EINVAL;
-		printk(KERN_WARNING "The uncorrectable error(s) is masked by device\n");
+		dev_warn(&dev->dev,
+			 "aer_inject: The uncorrectable error(s) is masked by device\n");
 		spin_unlock_irqrestore(&inject_lock, flags);
 		goto out_put;
 	}

@@ -457,7 +460,8 @@ static int aer_inject(struct aer_error_i
 
 	if (find_aer_device(rpdev, &edev)) {
 		if (!get_service_data(edev)) {
-			printk(KERN_WARNING "AER service is not initialized\n");
+			dev_warn(&edev->device,
+				 "aer_inject: AER service is not initialized\n");
 			ret = -EPROTONOSUPPORT;
 			goto out_put;
 		}
Signature · 2 lines
Jean Delvare
SUSE L3 Support
author
#3 lore raw

PATCH 3/4 PCI: aer_inject: Log actual error causes

In reply to Jean Delvare

The aer_inject driver is very quiet. In most cases, it merely returns an error code to user-space, leaving the user with little clue about the actual reason for the failure.

So, log error messages for 4 of the most frequent causes of failure:
* Can't find the root port of the specified device.
* Device doesn't support AER.
* Root port doesn't support AER.
* AER device not found.
This gives the user a chance to understand why aer-inject failed.

Based on a preliminary patch by Thomas Renninger.

Signed-off-by Jean Delvare <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Thomas Renninger <[email protected]>
Cc Bjorn Helgaas <[email protected]>
---
 drivers/pci/pcie/aer/aer_inject.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/pci/pcie/aer/aer_inject.c +7 -1
@@ -335,12 +335,14 @@ static int aer_inject(struct aer_error_i
 		return -ENODEV;
 	rpdev = pcie_find_root_port(dev);
 	if (!rpdev) {
+		dev_err(&dev->dev, "aer_inject: Root port not found\n");
 		ret = -ENODEV;
 		goto out_put;
 	}
 
 	pos_cap_err = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
 	if (!pos_cap_err) {
+		dev_err(&dev->dev, "aer_inject: Device doesn't support AER\n");
 		ret = -EPROTONOSUPPORT;
 		goto out_put;
 	}

@@ -351,6 +353,8 @@ static int aer_inject(struct aer_error_i
 
 	rp_pos_cap_err = pci_find_ext_capability(rpdev, PCI_EXT_CAP_ID_ERR);
 	if (!rp_pos_cap_err) {
+		dev_err(&rpdev->dev,
+			"aer_inject: Root port doesn't support AER\n");
 		ret = -EPROTONOSUPPORT;
 		goto out_put;
 	}

@@ -466,8 +470,10 @@ static int aer_inject(struct aer_error_i
 			goto out_put;
 		}
 		aer_irq(-1, edev);
-	} else
+	} else {
+		dev_err(&rpdev->dev, "aer_inject: AER device not found\n");
 		ret = -ENODEV;
+	}
 out_put:
 	kfree(err_alloc);
 	kfree(rperr_alloc);
Signature · 2 lines
Jean Delvare
SUSE L3 Support
author
#4 lore raw

PATCH 4/4 PCI: aer_inject: Log error injections

In reply to Jean Delvare

Log successful error injections so that injected errors can be differentiated from real errors.

Suggested-by Bjorn Helgaas <[email protected]>
Signed-off-by Jean Delvare <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Bjorn Helgaas <[email protected]>

---
Note: none of my systems support AER injection so this is untested.

 drivers/pci/pcie/aer/aer_inject.c |    3 +++
 1 file changed, 3 insertions(+)
drivers/pci/pcie/aer/aer_inject.c +3 -0
@@ -469,6 +469,9 @@ static int aer_inject(struct aer_error_i
 			ret = -EPROTONOSUPPORT;
 			goto out_put;
 		}
+		dev_info(&edev->device,
+			 "aer_inject: Injecting errors %08x/%08x into device %s\n",
+			 einj->cor_status, einj->uncor_status, pci_name(dev));
 		aer_irq(-1, edev);
 	} else {
 		dev_err(&rpdev->dev, "aer_inject: AER device not found\n");
Signature · 2 lines
Jean Delvare
SUSE L3 Support
Applied
#5 lore raw
In reply to Jean Delvare
Quoted 7 lines · Jean Delvare

As discussed earlier, here are a few cleanups and verbosity adjustments to the aer_inject driver.

[PATCH 1/4] PCI: aer_inject: Fix error codes [PATCH 2/4] PCI: aer_inject: Use dev_warn() [PATCH 3/4] PCI: aer_inject: Log actual error causes [PATCH 4/4] PCI: aer_inject: Log error injections

Applied all four to pci/aer for v4.6, thanks, Jean!