Unify PCI error response checking
Subsystems (22)
v3 cover Unify PCI error response checking
An MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
This patch series adds PCI_ERROR_RESPONSE definition and other helper definition SET_PCI_ERROR_RESPONSE and RESPONSE_IS_PCI_ERROR and uses it where appropriate to make these checks consistent and easier to find.
This helps unify PCI error response checking and make error check consistent and easier to find.
This series also ensures that the error response fabrication now happens in the PCI_OP_READ and PCI_USER_READ_CONFIG. This removes the responsibility from controller drivers to do the error response setting.
Patch 1:
- Adds the PCI_ERROR_RESPONSE and other related defintions
- All other patches are dependent on this patch. This patch needs to be applied first, before the others
Patch 2:
- Error fabrication happens in PCI_OP_READ and PCI_USER_READ_CONFIG whenever the data read via the controller driver fails.
- This patch needs to be applied before, Patch 4/24 to Patch 15/24 are applied.
Patch 3:
- Uses SET_PCI_ERROR_RESPONSE() when device is not found
Patch 4 - 15:
- Removes redundant error fabrication that happens in controller drivers when the read from a PCI device fails.
- These patches are dependent on Patch 2/24 of the series.
- These can be applied in any order.
Patch 16 - 22:
- Uses RESPONSE_IS_PCI_ERROR() to check the reads from hardware
- Patches can be applied in any order.
Patch 23 - 25:
- Edits the comments to include PCI_ERROR_RESPONSE alsong with 0xFFFFFFFF, so that it becomes easier to grep for faulty hardware reads.
Changelog
=========
v3:
- Change RESPONSE_IS_PCI_ERROR macro definition
- Fix the macros, Add () around macro parameters
- Fix alignment issue in Patch 2/24
- Add proper receipients for all the patches
v2:
- Instead of using SET_PCI_ERROR_RESPONSE in all controller drivers to fabricate error response, only use them in PCI_OP_READ and PCI_USER_READ_CONFIG
Naveen Naidu (25):
[Patch 1/25] PCI: Add PCI_ERROR_RESPONSE and it's related definitions [Patch 2/25] PCI: Set error response in config access defines when ops->read() fails [Patch 3/25] PCI: Use SET_PCI_ERROR_RESPONSE() when device not found [Patch 4/25] PCI: Remove redundant error fabrication when device read fails [Patch 5/25] PCI: thunder: Remove redundant error fabrication when device read fails [Patch 6/25] PCI: iproc: Remove redundant error fabrication when device read fails [Patch 7/25] PCI: mediatek: Remove redundant error fabrication when device read fails [Patch 8/25] PCI: exynos: Remove redundant error fabrication when device read fails [Patch 9/25] PCI: histb: Remove redundant error fabrication when device read fails [Patch 10/25] PCI: kirin: Remove redundant error fabrication when device read fails [Patch 11/25] PCI: aardvark: Remove redundant error fabrication when device read fails [Patch 12/25] PCI: mvebu: Remove redundant error fabrication when device read fails [Patch 13/25] PCI: altera: Remove redundant error fabrication when device read fails [Patch 14/25] PCI: rcar: Remove redundant error fabrication when device read fails [Patch 15/25] PCI: rockchip: Remove redundant error fabrication when device read fails [Patch 16/25] PCI/ERR: Use RESPONSE_IS_PCI_ERROR() to check read from hardware [Patch 17/25] PCI: vmd: Use RESPONSE_IS_PCI_ERROR() to check read from hardware [Patch 18/25] PCI: pciehp: Use RESPONSE_IS_PCI_ERROR() to check read from hardware [Patch 19/25] PCI/DPC: Use RESPONSE_IS_PCI_ERROR() to check read from hardware [Patch 20/25] PCI/PME: Use RESPONSE_IS_PCI_ERROR() to check read from hardware [Patch 21/25] PCI: cpqphp: Use RESPONSE_IS_PCI_ERROR() to check read from hardware [Patch 22/25] PCI: Use PCI_ERROR_RESPONSE to specify hardware error [Patch 23/25] PCI: keystone: Use PCI_ERROR_RESPONSE to specify hardware error [Patch 24/25] PCI: hv: Use PCI_ERROR_RESPONSE to specify hardware read error [Patch 25/25] PCI: xgene: Use PCI_ERROR_RESPONSE to specify hardware error
drivers/pci/access.c | 32 +++++++------- drivers/pci/controller/dwc/pci-exynos.c | 4 +- drivers/pci/controller/dwc/pci-keystone.c | 4 +- drivers/pci/controller/dwc/pcie-histb.c | 4 +- drivers/pci/controller/dwc/pcie-kirin.c | 4 +- drivers/pci/controller/pci-aardvark.c | 10 +---- drivers/pci/controller/pci-hyperv.c | 2 +- drivers/pci/controller/pci-mvebu.c | 8 +--- drivers/pci/controller/pci-thunder-ecam.c | 46 +++++++-------------- drivers/pci/controller/pci-thunder-pem.c | 4 +- drivers/pci/controller/pci-xgene.c | 8 ++-- drivers/pci/controller/pcie-altera.c | 4 +- drivers/pci/controller/pcie-iproc.c | 4 +- drivers/pci/controller/pcie-mediatek.c | 11 +---- drivers/pci/controller/pcie-rcar-host.c | 4 +- drivers/pci/controller/pcie-rockchip-host.c | 4 +- drivers/pci/controller/vmd.c | 2 +- drivers/pci/hotplug/cpqphp_ctrl.c | 4 +- drivers/pci/hotplug/pciehp_hpc.c | 10 ++--- drivers/pci/pci.c | 10 ++--- drivers/pci/pcie/dpc.c | 4 +- drivers/pci/pcie/pme.c | 4 +- drivers/pci/probe.c | 10 ++--- include/linux/pci.h | 9 ++++ 24 files changed, 85 insertions(+), 121 deletions(-)
Signature · 1 line
2.25.1
PATCH v3 1/25 PCI: Add PCI_ERROR_RESPONSE and it's related definitions
An MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
Add a PCI_ERROR_RESPONSE definition for that and use it where appropriate to make these checks consistent and easier to find.
Also add helper definitions SET_PCI_ERROR_RESPONSE and RESPONSE_IS_PCI_ERROR to make the code more readable.
--- include/linux/pci.h | 9 +++++++++ 1 file changed, 9 insertions(+)
include/linux/pci.h +9 -0
@@ -154,6 +154,15 @@ enum pci_interrupt_pin {/* The number of legacy PCI INTx interrupts */ #define PCI_NUM_INTX 4 +/* +* Reading from a device that doesn't respond typically returns ~0. A +* successful read from a device may also return ~0, so you need additional +* information to reliably identify errors. +*/ +#define PCI_ERROR_RESPONSE (~0ULL) +#define SET_PCI_ERROR_RESPONSE (val) (*(val) = ((typeof(*(val))) PCI_ERROR_RESPONSE)) +#define RESPONSE_IS_PCI_ERROR (val) ((val) == ((typeof(val)) PCI_ERROR_RESPONSE)) +/* * pci_power_t values must match the bits in the Capabilities PME_Support * and Control/Status PowerState fields in the Power Management capability.
Signature · 1 line
2.25.1
Re: [PATCH v3 01/25] PCI: Add PCI_ERROR_RESPONSE and it's related definitions
Quoted 4 lines · On Thursday 21 October 2021 20:37:26 Naveen Naidu wrote:
An MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
Quoted 8 lines
Add a
PCI_ERROR_RESPONSEdefinition for that and use it where appropriate to make these checks consistent and easier to find.Also add helper definitions
SET_PCI_ERROR_RESPONSEandRESPONSE_IS_PCI_ERRORto make the code more readable.Suggested-by Bjorn Helgaas <[email protected]>Signed-off-by Naveen Naidu <[email protected]>
Quoted 27 lines
--- include/linux/pci.h | 9 +++++++++ 1 file changed, 9 insertions(+)include/linux/pci.h +9
-0@@ -154,6 +154,15 @@ enum pci_interrupt_pin {/* The number of legacy PCI INTx interrupts */ #define PCI_NUM_INTX 4 +/* +* Reading from a device that doesn't respond typically returns ~0. A +* successful read from a device may also return ~0, so you need additional +* information to reliably identify errors. +*/ +#define PCI_ERROR_RESPONSE (~0ULL) +#define SET_PCI_ERROR_RESPONSE (val) (*(val) = ((typeof(*(val))) PCI_ERROR_RESPONSE)) +#define RESPONSE_IS_PCI_ERROR (val) ((val) == ((typeof(val)) PCI_ERROR_RESPONSE)) +/* * pci_power_t values must match the bits in the Capabilities PME_Support * and Control/Status PowerState fields in the Power Management capability. Signature · 1 line
2.25.1
Re: [PATCH v3 01/25] PCI: Add PCI_ERROR_RESPONSE and it's related definitions
In reply to Naveen NaiduQuoted 22 lines · Naveen Naidu
An MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
Add a
PCI_ERROR_RESPONSEdefinition for that and use it where appropriate to make these checks consistent and easier to find.Also add helper definitions
SET_PCI_ERROR_RESPONSEandRESPONSE_IS_PCI_ERRORto make the code more readable.Suggested-by Bjorn Helgaas <[email protected]>Signed-off-by Naveen Naidu <[email protected]>--- include/linux/pci.h | 9 +++++++++ 1 file changed, 9 insertions(+)include/linux/pci.h +0
-0@@ -154,6 +154,15 @@ enum pci_interrupt_pin {/* The number of legacy PCI INTx interrupts */
include/linux/pci.h enum pci_interrupt_pin { · line 164
Quoted 10 lines
include/linux/pci.h +8
-0#define PCI_NUM_INTX 4 +/* +* Reading from a device that doesn't respond typically returns ~0. A +* successful read from a device may also return ~0, so you need additional +* information to reliably identify errors. +*/ +#define PCI_ERROR_RESPONSE (~0ULL) +#define SET_PCI_ERROR_RESPONSE (val) (*(val) = ((typeof(*(val))) PCI_ERROR_RESPONSE)) +#define RESPONSE_IS_PCI_ERROR (val) ((val) == ((typeof(val)) PCI_ERROR_RESPONSE))
Beautiful! I really like this.
I would prefer the macros to start with "PCI_", e.g., PCI_SET_ERROR_RESPONSE().
I think "RESPONSE_IS_PCI_ERROR()" is too strong because (as the comment says), ~0 *may* indicate an error. Or it may be a successful read of a register that happens to contain ~0.
Possibilities to convey the idea that this isn't definitive:
PCI_POSSIBLE_ERROR_RESPONSE(val) # a little long PCI_LIKELY_ERROR(val) # we really have no idea whether PCI_PROBABLE_ERROR(val) # likely or probable PCI_POSSIBLE_ERROR(val) # promising?
Can you rebase to my "main" branch (v5.16-rc1), tweak the above, and collect up the acks/reviews?
We should also browse drivers outside drivers/pci for places we could use these. Not necessarily as part of this series, although if authors there object, it would be good to learn that earlier than later.
Drivers that implement pci_error_handlers might be a fruitful place to start. But you've done a great job finding users of ~0 and 0xffff... in drivers/pci/, too.
Quoted 11 lines
include/linux/pci.h +1
-0+/* * pci_power_t values must match the bits in the Capabilities PME_Support * and Control/Status PowerState fields in the Power Management capability. Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Re: [PATCH v3 01/25] PCI: Add PCI_ERROR_RESPONSE and it's related definitions
Quoted 25 lines · Bjorn Helgaas
Quoted 24 lines · Naveen Naidu
An MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
Add a
PCI_ERROR_RESPONSEdefinition for that and use it where appropriate to make these checks consistent and easier to find.Also add helper definitions
SET_PCI_ERROR_RESPONSEandRESPONSE_IS_PCI_ERRORto make the code more readable.Suggested-by Bjorn Helgaas <[email protected]>Signed-off-by Naveen Naidu <[email protected]>--- include/linux/pci.h | 9 +++++++++ 1 file changed, 9 insertions(+)include/linux/pci.h +0
-0@@ -154,6 +154,15 @@ enum pci_interrupt_pin {/* The number of legacy PCI INTx interrupts */ #define PCI_NUM_INTX 4
Quoted 11 lines
include/linux/pci.h enum pci_interrupt_pin { · line 164
Quoted 8 lines
include/linux/pci.h +8
-0+/* +* Reading from a device that doesn't respond typically returns ~0. A +* successful read from a device may also return ~0, so you need additional +* information to reliably identify errors. +*/ +#define PCI_ERROR_RESPONSE (~0ULL) +#define SET_PCI_ERROR_RESPONSE (val) (*(val) = ((typeof(*(val))) PCI_ERROR_RESPONSE)) +#define RESPONSE_IS_PCI_ERROR (val) ((val) == ((typeof(val)) PCI_ERROR_RESPONSE))Beautiful! I really like this.
Thank you very much for the review ^^
Quoted 3 lines
I would prefer the macros to start with "
PCI_", e.g.,PCI_SET_ERROR_RESPONSE().
ACK
Quoted 11 lines
I think "
RESPONSE_IS_PCI_ERROR()" is too strong because (as the comment says), ~0 *may* indicate an error. Or it may be a successful read of a register that happens to contain ~0.Possibilities to convey the idea that this isn't definitive:
PCI_POSSIBLE_ERROR_RESPONSE(val) # a little long PCI_LIKELY_ERROR(val) # we really have no idea whether PCI_PROBABLE_ERROR(val) # likely or probable PCI_POSSIBLE_ERROR(val) # promising?
ACK. Will use PCI_POSSIBLE_ERROR()
Quoted 3 lines
Can you rebase to my "main" branch (v5.16-rc1), tweak the above, and collect up the acks/reviews?
ACK
Quoted 9 lines
We should also browse drivers outside
drivers/pcifor places we could use these. Not necessarily as part of this series, although if authors there object, it would be good to learn that earlier than later.Drivers that implement
pci_error_handlersmight be a fruitful place to start. But you've done a great job finding users of ~0 and0xffff... indrivers/pci/, too.
A quick grep showed that there are around 80 drivers which have pci_error_handlers. I was thinking that it would be better if we handle these drivers in another patch series since the current patch series is itself 25 patches long. And in my short tenure reading LKML, I gathered that folks generally are not so kind to a long list of patches in a single patch series ^^' (I might be wrong though, Apologies)
The consensus on the patch series does seem slightly positive so ideally, I was hoping that we would not have the case where a author does not like the way we are handling this patch. Then again, I'm pretty sure that I might be wrong ^^'
I hope it would be okay that I send in a new patch series with the suggested changes and handle the other changes in another patch series ^^
Thanks,
Naveen
Quoted 11 lines
Quoted 11 lines
include/linux/pci.h +1
-0+/* * pci_power_t values must match the bits in the Capabilities PME_Support * and Control/Status PowerState fields in the Power Management capability. Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 2/25 PCI: Set error response in config access defines when ops->read() fails
In reply to Naveen NaiduMake PCI_OP_READ and PCI_USER_READ_CONFIG set the data value with error response (~0), when the PCI device read by a host controller fails.
This ensures that the controller drivers no longer need to fabricate (~0) value when they detect error. It also gurantees that the error response (~0) is always set when the controller drivers fails to read a config register from a device.
This makes error response fabrication consistent and helps in removal of a lot of repeated code.
--- drivers/pci/access.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/pci/access.c +8 -2
@@ -42,7 +42,10 @@ int noinline pci_bus_read_config_##size \ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \pci_lock_config (flags); \ res = bus->ops->read (bus, devfn, pos, len, &data); \ - *value = (type )data; \ +if (res) \ +SET_PCI_ERROR_RESPONSE (value); \ +else \ + *value = (type )data; \pci_unlock_config (flags); \return res; \ }
@@ -228,7 +231,10 @@ int pci_user_read_config_##size \ ret = dev->bus->ops->read (dev->bus, dev->devfn, \ pos,sizeof (type), &data); \raw_spin_unlock_irq (&pci_lock); \ - *val = (type )data; \ +if (ret) \ +SET_PCI_ERROR_RESPONSE (val); \ +else \ + *val = (type )data; \return pcibios_err_to_errno (ret); \ } \ EXPORT_SYMBOL_GPL(pci_user_read_config_##size);
Signature · 1 line
2.25.1
Re: [PATCH v3 02/25] PCI: Set error response in config access defines when ops->read() fails
Quoted 8 lines · On Thursday 21 October 2021 20:37:27 Naveen Naidu wrote:
Make
PCI_OP_READandPCI_USER_READ_CONFIGset the data value with error response (~0), when the PCI device read by a host controller fails.This ensures that the controller drivers no longer need to fabricate (~0) value when they detect error. It also gurantees that the error response (~0) is always set when the controller drivers fails to read a config register from a device.
Quoted 6 lines
This makes error response fabrication consistent and helps in removal of a lot of repeated code.
Suggested-by Rob Herring <[email protected]>Reviewed-by Rob Herring <[email protected]>Signed-off-by Naveen Naidu <[email protected]>
Quoted 35 lines
--- drivers/pci/access.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)drivers/pci/access.c +8
-2@@ -42,7 +42,10 @@ int noinline pci_bus_read_config_##size \ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \pci_lock_config (flags); \ res = bus->ops->read (bus, devfn, pos, len, &data); \ - *value = (type )data; \ +if (res) \ +SET_PCI_ERROR_RESPONSE (value); \ +else \ + *value = (type )data; \pci_unlock_config (flags); \return res; \ }@@ -228,7 +231,10 @@ int pci_user_read_config_##size \ ret = dev->bus->ops->read (dev->bus, dev->devfn, \ pos,sizeof (type), &data); \raw_spin_unlock_irq (&pci_lock); \ - *val = (type )data; \ +if (ret) \ +SET_PCI_ERROR_RESPONSE (val); \ +else \ + *val = (type )data; \return pcibios_err_to_errno (ret); \ } \ EXPORT_SYMBOL_GPL(pci_user_read_config_##size);Signature · 1 line
2.25.1
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 3/25 PCI: Use SET_PCI_ERROR_RESPONSE() when device not found
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
Use SET_PCI_ERROR_RESPONSE() to set the error response, when a faulty read occurs.
This helps unify PCI error response checking and make error check consistent and easier to find.
--- drivers/pci/access.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/pci/access.c +3 -3
@@ -529,7 +529,7 @@ EXPORT_SYMBOL(pcie_capability_clear_and_set_dword);int pci_read_config_byte (const struct pci_dev *dev,int where,u8 *val) {if (pci_dev_is_disconnected (dev)) { - *val = ~0 ; +SET_PCI_ERROR_RESPONSE (val);return PCIBIOS_DEVICE_NOT_FOUND ; }return pci_bus_read_config_byte (dev->bus, dev->devfn, where, val);
@@ -539,7 +539,7 @@ EXPORT_SYMBOL(pci_read_config_byte);int pci_read_config_word (const struct pci_dev *dev,int where,u16 *val) {if (pci_dev_is_disconnected (dev)) { - *val = ~0 ; +SET_PCI_ERROR_RESPONSE (val);return PCIBIOS_DEVICE_NOT_FOUND ; }return pci_bus_read_config_word (dev->bus, dev->devfn, where, val);
@@ -550,7 +550,7 @@ int pci_read_config_dword(const struct pci_dev *dev, int where, u32 *val) {if (pci_dev_is_disconnected (dev)) { - *val = ~0 ; +SET_PCI_ERROR_RESPONSE (val);return PCIBIOS_DEVICE_NOT_FOUND ; }return pci_bus_read_config_dword (dev->bus, dev->devfn, where, val);
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 4/25 PCI: Remove redundant error fabrication when device read fails
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
The host controller drivers sets the error response values (~0) and returns an error when faulty hardware read occurs. But the error response value (~0) is already being set in PCI_OP_READ and PCI_USER_READ_CONFIG whenever a read by host controller driver fails.
Thus, it's no longer necessary for the host controller drivers to fabricate any error response.
This helps unify PCI error response checking and make error check consistent and easier to find.
--- drivers/pci/access.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
drivers/pci/access.c +2 -6
@@ -83,10 +83,8 @@ int pci_generic_config_read(struct pci_bus *bus, unsigned int devfn,void __iomem *addr; addr = bus->ops->map_bus (bus, devfn, where); -if (!addr) { - *val = ~0 ; +if (!addr)return PCIBIOS_DEVICE_NOT_FOUND ; - }if (size ==1 ) *val =readb (addr);
@@ -125,10 +123,8 @@ int pci_generic_config_read32(struct pci_bus *bus, unsigned int devfn,void __iomem *addr; addr = bus->ops->map_bus (bus, devfn, where & ~0x3 ); -if (!addr) { - *val = ~0 ; +if (!addr)return PCIBIOS_DEVICE_NOT_FOUND ; - } *val =readl (addr);
Signature · 1 line
2.25.1
PATCH v3 5/25 PCI: thunder: Remove redundant error fabrication when device read fails
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
The host controller drivers sets the error response values (~0) and returns an error when faulty hardware read occurs. But the error response value (~0) is already being set in PCI_OP_READ and PCI_USER_READ_CONFIG whenever a read by host controller driver fails.
Thus, it's no longer necessary for the host controller drivers to fabricate any error response.
This helps unify PCI error response checking and make error check consistent and easier to find.
--- drivers/pci/controller/pci-thunder-ecam.c | 46 ++++++++--------------- drivers/pci/controller/pci-thunder-pem.c | 4 +- 2 files changed, 17 insertions(+), 33 deletions(-)
drivers/pci/controller/pci-thunder-ecam.c +16 -30
@@ -41,10 +41,9 @@ static int handle_ea_bar(u32 e0, int bar, struct pci_bus *bus, }if (where_a ==0x4 ) { addr = bus->ops->map_bus (bus, devfn, bar);/* BAR 0 */ -if (!addr) { - *val = ~0 ; +if (!addr)return PCIBIOS_DEVICE_NOT_FOUND ; - } + v =readl (addr); v &= ~0xf ; v |=2 ;/* EA entry-1. Base-L */
@@ -56,10 +55,9 @@ static int handle_ea_bar(u32 e0, int bar, struct pci_bus *bus,u32 barl_rb; addr = bus->ops->map_bus (bus, devfn, bar);/* BAR 0 */ -if (!addr) { - *val = ~0 ; +if (!addr)return PCIBIOS_DEVICE_NOT_FOUND ; - } + barl_orig =readl (addr +0 );writel (0xffffffff , addr +0 ); barl_rb =readl (addr +0 );
@@ -72,10 +70,9 @@ static int handle_ea_bar(u32 e0, int bar, struct pci_bus *bus, }if (where_a ==0xc ) { addr = bus->ops->map_bus (bus, devfn, bar +4 );/* BAR 1 */ -if (!addr) { - *val = ~0 ; +if (!addr)return PCIBIOS_DEVICE_NOT_FOUND ; - } + v =readl (addr);/* EA entry-3. Base-H */ set_val (v, where, size, val);return PCIBIOS_SUCCESSFUL ;
@@ -104,10 +101,8 @@ static int thunder_ecam_p2_config_read(struct pci_bus *bus, unsigned int devfn, } addr = bus->ops->map_bus (bus, devfn, where_a); -if (!addr) { - *val = ~0 ; +if (!addr)return PCIBIOS_DEVICE_NOT_FOUND ; - } v =readl (addr);
@@ -135,10 +130,8 @@ static int thunder_ecam_config_read(struct pci_bus *bus, unsigned int devfn,int where_a = where & ~3 ; addr = bus->ops->map_bus (bus, devfn,0xc ); -if (!addr) { - *val = ~0 ; +if (!addr)return PCIBIOS_DEVICE_NOT_FOUND ; - } v =readl (addr);
@@ -146,10 +139,8 @@ static int thunder_ecam_config_read(struct pci_bus *bus, unsigned int devfn, cfg_type = (v >>16 ) &0x7f ; addr = bus->ops->map_bus (bus, devfn,8 ); -if (!addr) { - *val = ~0 ; +if (!addr)return PCIBIOS_DEVICE_NOT_FOUND ; - } class_rev =readl (addr);if (class_rev ==0xffffffff )
@@ -176,10 +167,8 @@ static int thunder_ecam_config_read(struct pci_bus *bus, unsigned int devfn, } addr = bus->ops->map_bus (bus, devfn,0 ); -if (!addr) { - *val = ~0 ; +if (!addr)return PCIBIOS_DEVICE_NOT_FOUND ; - } vendor_device =readl (addr);if (vendor_device ==0xffffffff )
@@ -196,10 +185,9 @@ static int thunder_ecam_config_read(struct pci_bus *bus, unsigned int devfn,bool is_tns = (vendor_device ==0xa01f177d ); addr = bus->ops->map_bus (bus, devfn,0x70 ); -if (!addr) { - *val = ~0 ; +if (!addr)return PCIBIOS_DEVICE_NOT_FOUND ; - } +/* E_CAP */ v =readl (addr); has_msix = (v &0xff00 ) !=0 ;
@@ -211,10 +199,9 @@ static int thunder_ecam_config_read(struct pci_bus *bus, unsigned int devfn, }if (where_a ==0xb0 ) { addr = bus->ops->map_bus (bus, devfn, where_a); -if (!addr) { - *val = ~0 ; +if (!addr)return PCIBIOS_DEVICE_NOT_FOUND ; - } + v =readl (addr);if (v &0xff00 )pr_err ("Bad MSIX cap header: %08x\n" , v);
@@ -268,10 +255,9 @@ static int thunder_ecam_config_read(struct pci_bus *bus, unsigned int devfn,if (where_a ==0x70 ) { addr = bus->ops->map_bus (bus, devfn, where_a); -if (!addr) { - *val = ~0 ; +if (!addr)return PCIBIOS_DEVICE_NOT_FOUND ; - } + v =readl (addr);if (v &0xff00 )pr_err ("Bad PCIe cap header: %08x\n" , v);
drivers/pci/controller/pci-thunder-pem.c +1 -3
@@ -41,10 +41,8 @@ static int thunder_pem_bridge_read(struct pci_bus *bus, unsigned int devfn,struct pci_config_window *cfg = bus->sysdata;struct thunder_pem_pci *pem_pci = (struct thunder_pem_pci *)cfg->priv; -if (devfn !=0 || where >=2048 ) { - *val = ~0 ; +if (devfn !=0 || where >=2048 )return PCIBIOS_DEVICE_NOT_FOUND ; - }/* * 32-bit accesses only. Write the address to the low order
Signature · 1 line
2.25.1
PATCH v3 6/25 PCI: iproc: Remove redundant error fabrication when device read fails
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
The host controller drivers sets the error response values (~0) and returns an error when faulty hardware read occurs. But the error response value (~0) is already being set in PCI_OP_READ and PCI_USER_READ_CONFIG whenever a read by host controller driver fails.
Thus, it's no longer necessary for the host controller drivers to fabricate any error response.
This helps unify PCI error response checking and make error check consistent and easier to find.
--- drivers/pci/controller/pcie-iproc.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/pci/controller/pcie-iproc.c +1 -3
@@ -659,10 +659,8 @@ static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie,void __iomem *addr; addr =iproc_pcie_map_cfg_bus (pcie,0 , devfn, where & ~0x3 ); -if (!addr) { - *val = ~0 ; +if (!addr)return PCIBIOS_DEVICE_NOT_FOUND ; - } *val =readl (addr);
Signature · 1 line
2.25.1
PATCH v3 7/25 PCI: mediatek: Remove redundant error fabrication when device read fails
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
The host controller drivers sets the error response values (~0) and returns an error when faulty hardware read occurs. But the error response value (~0) is already being set in PCI_OP_READ and PCI_USER_READ_CONFIG whenever a read by host controller driver fails.
Thus, it's no longer necessary for the host controller drivers to fabricate any error response.
This helps unify PCI error response checking and make error check consistent and easier to find.
--- drivers/pci/controller/pcie-mediatek.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)
drivers/pci/controller/pcie-mediatek.c +2 -9
@@ -365,19 +365,12 @@ static int mtk_pcie_config_read(struct pci_bus *bus, unsigned int devfn, {struct mtk_pcie_port *port;u32 bn = bus->number; -int ret; port =mtk_pcie_find_port (bus, devfn); -if (!port) { - *val = ~0 ; +if (!port)return PCIBIOS_DEVICE_NOT_FOUND ; - } - - ret =mtk_pcie_hw_rd_cfg (port, bn, devfn, where, size, val); -if (ret) - *val = ~0 ; -return ret; +return mtk_pcie_hw_rd_cfg (port, bn, devfn, where, size, val); } static int mtk_pcie_config_write(struct pci_bus *bus, unsigned int devfn,
Signature · 1 line
2.25.1
PATCH v3 8/25 PCI: exynos: Remove redundant error fabrication when device read fails
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
The host controller drivers sets the error response values (~0) and returns an error when faulty hardware read occurs. But the error response value (~0) is already being set in PCI_OP_READ and PCI_USER_READ_CONFIG whenever a read by host controller driver fails.
Thus, it's no longer necessary for the host controller drivers to fabricate any error response.
This helps unify PCI error response checking and make error check consistent and easier to find.
--- drivers/pci/controller/dwc/pci-exynos.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/pci/controller/dwc/pci-exynos.c +1 -3
@@ -216,10 +216,8 @@ static int exynos_pcie_rd_own_conf(struct pci_bus *bus, unsigned int devfn, {struct dw_pcie *pci =to_dw_pcie_from_pp (bus->sysdata); -if (PCI_SLOT (devfn)) { - *val = ~0 ; +if (PCI_SLOT (devfn))return PCIBIOS_DEVICE_NOT_FOUND ; - } *val =dw_pcie_read_dbi (pci, where, size);return PCIBIOS_SUCCESSFUL ;
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 9/25 PCI: histb: Remove redundant error fabrication when device read fails
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
The host controller drivers sets the error response values (~0) and returns an error when faulty hardware read occurs. But the error response value (~0) is already being set in PCI_OP_READ and PCI_USER_READ_CONFIG whenever a read by host controller driver fails.
Thus, it's no longer necessary for the host controller drivers to fabricate any error response.
This helps unify PCI error response checking and make error check consistent and easier to find.
--- drivers/pci/controller/dwc/pcie-histb.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/pci/controller/dwc/pcie-histb.c +1 -3
@@ -127,10 +127,8 @@ static int histb_pcie_rd_own_conf(struct pci_bus *bus, unsigned int devfn, {struct dw_pcie *pci =to_dw_pcie_from_pp (bus->sysdata); -if (PCI_SLOT (devfn)) { - *val = ~0 ; +if (PCI_SLOT (devfn))return PCIBIOS_DEVICE_NOT_FOUND ; - } *val =dw_pcie_read_dbi (pci, where, size);return PCIBIOS_SUCCESSFUL ;
Signature · 1 line
2.25.1
PATCH v3 10/25 PCI: kirin: Remove redundant error fabrication when device read fails
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
The host controller drivers sets the error response values (~0) and returns an error when faulty hardware read occurs. But the error response value (~0) is already being set in PCI_OP_READ and PCI_USER_READ_CONFIG whenever a read by host controller driver fails.
Thus, it's no longer necessary for the host controller drivers to fabricate any error response.
This helps unify PCI error response checking and make error check consistent and easier to find.
--- drivers/pci/controller/dwc/pcie-kirin.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/pci/controller/dwc/pcie-kirin.c +1 -3
@@ -330,10 +330,8 @@ static int kirin_pcie_rd_own_conf(struct pci_bus *bus, unsigned int devfn, {struct dw_pcie *pci =to_dw_pcie_from_pp (bus->sysdata); -if (PCI_SLOT (devfn)) { - *val = ~0 ; +if (PCI_SLOT (devfn))return PCIBIOS_DEVICE_NOT_FOUND ; - } *val =dw_pcie_read_dbi (pci, where, size);return PCIBIOS_SUCCESSFUL ;
Signature · 1 line
2.25.1
PATCH v3 11/25 PCI: aardvark: Remove redundant error fabrication when device read fails
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
The host controller drivers sets the error response values (~0) and returns an error when faulty hardware read occurs. But the error response value (~0) is already being set in PCI_OP_READ and PCI_USER_READ_CONFIG whenever a read by host controller driver fails.
Thus, it's no longer necessary for the host controller drivers to fabricate any error response.
This helps unify PCI error response checking and make error check consistent and easier to find.
--- drivers/pci/controller/pci-aardvark.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
drivers/pci/controller/pci-aardvark.c +2 -8
@@ -893,10 +893,8 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn,u32 reg;int ret; -if (!advk_pcie_valid_device (pcie, bus, devfn)) { - *val =0xffffffff ; +if (!advk_pcie_valid_device (pcie, bus, devfn))return PCIBIOS_DEVICE_NOT_FOUND ; - }if (pci_is_root_bus (bus)) return pci_bridge_emul_conf_read(&pcie->bridge, where,
@@ -920,7 +918,6 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn, *val =CFG_RD_CRS_VAL ;return PCIBIOS_SUCCESSFUL ; } - *val =0xffffffff ;return PCIBIOS_SET_FAILED ; }
@@ -955,16 +952,13 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn, *val =CFG_RD_CRS_VAL ;return PCIBIOS_SUCCESSFUL ; } - *val =0xffffffff ;return PCIBIOS_SET_FAILED ; }/* Check PIO status and get the read result */ ret =advk_pcie_check_pio_status (pcie, allow_crs, val); -if (ret <0 ) { - *val =0xffffffff ; +if (ret <0 )return PCIBIOS_SET_FAILED ; - }if (size ==1 ) *val = (*val >> (8 * (where &3 ))) &0xff ;
Signature · 1 line
2.25.1
Re: [PATCH v3 11/25] PCI: aardvark: Remove redundant error fabrication when device read fails
Quoted 9 lines · On Thursday 21 October 2021 20:37:36 Naveen Naidu wrote:
An MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
The host controller drivers sets the error response values (~0) and returns an error when faulty hardware read occurs. But the error response value (~0) is already being set in
PCI_OP_READandPCI_USER_READ_CONFIGwhenever a read by host controller driver fails.
Quoted 7 lines
Thus, it's no longer necessary for the host controller drivers to fabricate any error response.
This helps unify PCI error response checking and make error check consistent and easier to find.
Signed-off-by Naveen Naidu <[email protected]>
Quoted 49 lines
--- drivers/pci/controller/pci-aardvark.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)drivers/pci/controller/pci-aardvark.c +2
-8@@ -893,10 +893,8 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn,u32 reg;int ret; -if (!advk_pcie_valid_device (pcie, bus, devfn)) { - *val =0xffffffff ; +if (!advk_pcie_valid_device (pcie, bus, devfn))return PCIBIOS_DEVICE_NOT_FOUND ; - }if (pci_is_root_bus (bus)) return pci_bridge_emul_conf_read(&pcie->bridge, where,@@ -920,7 +918,6 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn, *val =CFG_RD_CRS_VAL ;return PCIBIOS_SUCCESSFUL ; } - *val =0xffffffff ;return PCIBIOS_SET_FAILED ; }@@ -955,16 +952,13 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn, *val =CFG_RD_CRS_VAL ;return PCIBIOS_SUCCESSFUL ; } - *val =0xffffffff ;return PCIBIOS_SET_FAILED ; }/* Check PIO status and get the read result */ ret =advk_pcie_check_pio_status (pcie, allow_crs, val); -if (ret <0 ) { - *val =0xffffffff ; +if (ret <0 )return PCIBIOS_SET_FAILED ; - }if (size ==1 ) *val = (*val >> (8 * (where &3 ))) &0xff ;Signature · 1 line
2.25.1
PATCH v3 12/25 PCI: mvebu: Remove redundant error fabrication when device read fails
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
The host controller drivers sets the error response values (~0) and returns an error when faulty hardware read occurs. But the error response value (~0) is already being set in PCI_OP_READ and PCI_USER_READ_CONFIG whenever a read by host controller driver fails.
Thus, it's no longer necessary for the host controller drivers to fabricate any error response.
This helps unify PCI error response checking and make error check consistent and easier to find.
--- drivers/pci/controller/pci-mvebu.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
drivers/pci/controller/pci-mvebu.c +2 -6
@@ -653,20 +653,16 @@ static int mvebu_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,int ret; port =mvebu_pcie_find_port (pcie, bus, devfn); -if (!port) { - *val =0xffffffff ; +if (!port)return PCIBIOS_DEVICE_NOT_FOUND ; - }/* Access the emulated PCI-to-PCI bridge */ if (bus->number ==0 )return pci_bridge_emul_conf_read (&port->bridge, where, size, val); -if (!mvebu_pcie_link_up (port)) { - *val =0xffffffff ; +if (!mvebu_pcie_link_up (port))return PCIBIOS_DEVICE_NOT_FOUND ; - }/* Access the real PCIe interface */ ret = mvebu_pcie_hw_rd_conf(port, bus, devfn,
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 13/25 PCI: altera: Remove redundant error fabrication when device read fails
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
The host controller drivers sets the error response values (~0) and returns an error when faulty hardware read occurs. But the error response value (~0) is already being set in PCI_OP_READ and PCI_USER_READ_CONFIG whenever a read by host controller driver fails.
Thus, it's no longer necessary for the host controller drivers to fabricate any error response.
This helps unify PCI error response checking and make error check consistent and easier to find.
--- drivers/pci/controller/pcie-altera.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/pci/controller/pcie-altera.c +1 -3
@@ -510,10 +510,8 @@ static int altera_pcie_cfg_read(struct pci_bus *bus, unsigned int devfn,if (altera_pcie_hide_rc_bar (bus, devfn, where))return PCIBIOS_BAD_REGISTER_NUMBER ; -if (!altera_pcie_valid_device (pcie, bus,PCI_SLOT (devfn))) { - *value =0xffffffff ; +if (!altera_pcie_valid_device (pcie, bus,PCI_SLOT (devfn)))return PCIBIOS_DEVICE_NOT_FOUND ; - }return _altera_pcie_cfg_read (pcie, bus->number, devfn, where, size, value);
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 14/25 PCI: rcar: Remove redundant error fabrication when device read fails
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
The host controller drivers sets the error response values (~0) and returns an error when faulty hardware read occurs. But the error response value (~0) is already being set in PCI_OP_READ and PCI_USER_READ_CONFIG whenever a read by host controller driver fails.
Thus, it's no longer necessary for the host controller drivers to fabricate any error response.
This helps unify PCI error response checking and make error check consistent and easier to find.
--- drivers/pci/controller/pcie-rcar-host.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/pci/controller/pcie-rcar-host.c +1 -3
@@ -161,10 +161,8 @@ static int rcar_pcie_read_conf(struct pci_bus *bus, unsigned int devfn, ret =rcar_pcie_config_access (host,RCAR_PCI_ACCESS_READ , bus, devfn, where, val); -if (ret !=PCIBIOS_SUCCESSFUL ) { - *val =0xffffffff ; +if (ret !=PCIBIOS_SUCCESSFUL )return ret; - }if (size ==1 ) *val = (*val >> (BITS_PER_BYTE * (where &3 ))) &0xff ;
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 15/25 PCI: rockchip: Remove redundant error fabrication when device read fails
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
The host controller drivers sets the error response values (~0) and returns an error when faulty hardware read occurs. But the error response value (~0) is already being set in PCI_OP_READ and PCI_USER_READ_CONFIG whenever a read by host controller driver fails.
Thus, it's no longer necessary for the host controller drivers to fabricate any error response.
This helps unify PCI error response checking and make error check consistent and easier to find.
--- drivers/pci/controller/pcie-rockchip-host.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/pci/controller/pcie-rockchip-host.c +1 -3
@@ -221,10 +221,8 @@ static int rockchip_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where, {struct rockchip_pcie *rockchip = bus->sysdata; -if (!rockchip_pcie_valid_device (rockchip, bus,PCI_SLOT (devfn))) { - *val =0xffffffff ; +if (!rockchip_pcie_valid_device (rockchip, bus,PCI_SLOT (devfn)))return PCIBIOS_DEVICE_NOT_FOUND ; - }if (pci_is_root_bus (bus))return rockchip_pcie_rd_own_conf (rockchip, where, size, val);
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 16/25 PCI/ERR: Use RESPONSE_IS_PCI_ERROR() to check read from hardware
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
Use RESPONSE_IS_PCI_ERROR() to check the response we get when we read data from hardware.
This unifies PCI error response checking and make error checks consistent and easier to find.
--- drivers/pci/pci.c | 10 +++++----- drivers/pci/probe.c | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-)
drivers/pci/pci.c +5 -5
@@ -1077,7 +1077,7 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state)return -EIO ;pci_read_config_word (dev, dev->pm_cap +PCI_PM_CTRL , &pmcsr); -if (pmcsr == (u16 ) ~0 ) { +if (RESPONSE_IS_PCI_ERROR (pmcsr)) {pci_err (dev,"can't change power state from %s to %s (config space inaccessible)\n" ,pci_power_name (dev->current_state),pci_power_name (state));
@@ -1239,16 +1239,16 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout)* After reset, the device should not silently discard config * requests, but it may still indicate that it needs more time by * responding to them with CRS completions. The Root Port will -* generally synthesize ~0 data to complete the read (except when -* CRS SV is enabled and the read was for the Vendor ID; in that -* case it synthesizes 0x0001 data). +* generally synthesize ~0 (PCI_ERROR_RESPONSE) data to complete +* the read (except when CRS SV is enabled and the read was for the +* Vendor ID; in that case it synthesizes 0x0001 data). * * Wait for the device to return a non-CRS completion. Read the * Command register instead of Vendor ID so we don't have to * contend with the CRS SV value. */ pci_read_config_dword (dev,PCI_COMMAND , &id); -while (id == ~0 ) { +while (RESPONSE_IS_PCI_ERROR (id)) {if (delay > timeout) {pci_warn (dev,"not ready %dms after %s; giving up\n" , delay -1 , reset_type);
drivers/pci/probe.c +5 -5
@@ -206,14 +206,14 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,* memory BAR or a ROM, bit 0 must be clear; if it's an io BAR, bit * 1 must be clear. */ -if (sz ==0xffffffff ) +if (RESPONSE_IS_PCI_ERROR (sz)) sz =0 ;/* * I don't know how l can have all bits set. Copied from old code. * Maybe it fixes a bug on some ancient platform. */ -if (l ==0xffffffff ) +if (RESPONSE_IS_PCI_ERROR (l)) l =0 ;if (type == pci_bar_unknown) {
@@ -1660,7 +1660,7 @@ static int pci_cfg_space_size_ext(struct pci_dev *dev)if (pci_read_config_dword (dev, pos, &status) !=PCIBIOS_SUCCESSFUL )return PCI_CFG_SPACE_SIZE ; -if (status ==0xffffffff ||pci_ext_cfg_is_aliased (dev)) +if (RESPONSE_IS_PCI_ERROR (status) ||pci_ext_cfg_is_aliased (dev))return PCI_CFG_SPACE_SIZE ;return PCI_CFG_SPACE_EXP_SIZE ;
@@ -2336,8 +2336,8 @@ bool pci_bus_generic_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,if (pci_bus_read_config_dword (bus, devfn,PCI_VENDOR_ID , l))return false; -/* Some broken boards return 0 or ~0 if a slot is empty: */ -if (*l ==0xffffffff || *l ==0x00000000 || +/* Some broken boards return 0 or ~0 (PCI_ERROR_RESPONSE) if a slot is empty: */ +if (RESPONSE_IS_PCI_ERROR (*l) || *l ==0x00000000 || *l ==0x0000ffff || *l ==0xffff0000 )return false;
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 17/25 PCI: vmd: Use RESPONSE_IS_PCI_ERROR() to check read from hardware
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
Use RESPONSE_IS_PCI_ERROR() to check the response we get when we read data from hardware.
This helps unify PCI error response checking and make error checks consistent and easier to find.
--- drivers/pci/controller/vmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pci/controller/vmd.c +1 -1
@@ -538,7 +538,7 @@ static int vmd_get_phys_offsets(struct vmd_dev *vmd, bool native_hint,int ret; ret =pci_read_config_dword (dev,PCI_REG_VMLOCK , &vmlock); -if (ret || vmlock == ~0 ) +if (ret ||RESPONSE_IS_PCI_ERROR (vmlock))return -ENODEV ;if (MB2_SHADOW_EN (vmlock)) {
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Re: [PATCH v3 17/25] PCI: vmd: Use RESPONSE_IS_PCI_ERROR() to check read from hardware
Quoted 31 lines · Naveen Naidu
An MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
Use
RESPONSE_IS_PCI_ERROR()to check the response we get when we read data from hardware.This helps unify PCI error response checking and make error checks consistent and easier to find.
Signed-off-by Naveen Naidu <[email protected]>--- drivers/pci/controller/vmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/pci/controller/vmd.c +1
-1@@ -538,7 +538,7 @@ static int vmd_get_phys_offsets(struct vmd_dev *vmd, bool native_hint,int ret; ret =pci_read_config_dword (dev,PCI_REG_VMLOCK , &vmlock); -if (ret || vmlock == ~0 ) +if (ret ||RESPONSE_IS_PCI_ERROR (vmlock))return -ENODEV ;if (MB2_SHADOW_EN (vmlock)) {Signature · 1 line
2.25.1
Jonathan, I haven't added your Reviewed-by tag to this patch which you gave in v1 of the series [1] , because the macro definition of the RESPONSE_IS_PCI_ERROR changed slightly, and I was not sure if you would be okay with it. I hope it was right thing to do, if not I apologize for that.
[1]:
https://lore.kernel.org/linux-pci/[email protected]/T/#u
Thanks,
Naveen
PATCH v3 18/25 PCI: pciehp: Use RESPONSE_IS_PCI_ERROR() to check read from hardware
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
Use RESPONSE_IS_PCI_ERROR() to check the response we get when we read data from hardware.
This helps unify PCI error response checking and make error checks consistent and easier to find.
Compile tested only.
--- drivers/pci/hotplug/pciehp_hpc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/pci/hotplug/pciehp_hpc.c +5 -5
@@ -89,7 +89,7 @@ static int pcie_poll_cmd(struct controller *ctrl, int timeout) do { pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status); - if (slot_status == (u16) ~0) { + if (RESPONSE_IS_PCI_ERROR(slot_status)) { ctrl_info(ctrl, "%s: no response from device\n", __func__); return 0;
@@ -165,7 +165,7 @@ static void pcie_do_write_cmd(struct controller *ctrl, u16 cmd,pcie_wait_cmd (ctrl);pcie_capability_read_word (pdev,PCI_EXP_SLTCTL , &slot_ctrl); -if (slot_ctrl == (u16 ) ~0 ) { +if (RESPONSE_IS_PCI_ERROR (slot_ctrl)) {ctrl_info (ctrl,"%s: no response from device\n" , __func__); goto out; }
@@ -236,7 +236,7 @@ int pciehp_check_link_active(struct controller *ctrl)int ret; ret =pcie_capability_read_word (pdev,PCI_EXP_LNKSTA , &lnk_status); -if (ret ==PCIBIOS_DEVICE_NOT_FOUND || lnk_status == (u16 )~0 ) +if (ret ==PCIBIOS_DEVICE_NOT_FOUND ||RESPONSE_IS_PCI_ERROR (lnk_status))return -ENODEV ; ret = !!(lnk_status &PCI_EXP_LNKSTA_DLLLA );
@@ -443,7 +443,7 @@ int pciehp_card_present(struct controller *ctrl)int ret; ret =pcie_capability_read_word (pdev,PCI_EXP_SLTSTA , &slot_status); -if (ret ==PCIBIOS_DEVICE_NOT_FOUND || slot_status == (u16 )~0 ) +if (ret ==PCIBIOS_DEVICE_NOT_FOUND ||RESPONSE_IS_PCI_ERROR (slot_status))return -ENODEV ;return !!(slot_status &PCI_EXP_SLTSTA_PDS );
@@ -621,7 +621,7 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id) read_status:pcie_capability_read_word (pdev,PCI_EXP_SLTSTA , &status); -if (status == (u16 ) ~0 ) { +if (RESPONSE_IS_PCI_ERROR (status)) {ctrl_info (ctrl,"%s: no response from device\n" , __func__);if (parent)pm_runtime_put (parent);
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Re: [PATCH v3 18/25] PCI: pciehp: Use RESPONSE_IS_PCI_ERROR() to check read from hardware
Quoted 69 lines · Naveen Naidu
An MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
Use
RESPONSE_IS_PCI_ERROR()to check the response we get when we read data from hardware.This helps unify PCI error response checking and make error checks consistent and easier to find.
Compile tested only.
Signed-off-by Naveen Naidu <[email protected]>--- drivers/pci/hotplug/pciehp_hpc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)drivers/pci/hotplug/pciehp_hpc.c +5
-5@@ -89,7 +89,7 @@ static int pcie_poll_cmd(struct controller *ctrl, int timeout) do { pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status); - if (slot_status == (u16) ~0) { + if (RESPONSE_IS_PCI_ERROR(slot_status)) { ctrl_info(ctrl, "%s: no response from device\n", __func__); return 0;@@ -165,7 +165,7 @@ static void pcie_do_write_cmd(struct controller *ctrl, u16 cmd,pcie_wait_cmd (ctrl);pcie_capability_read_word (pdev,PCI_EXP_SLTCTL , &slot_ctrl); -if (slot_ctrl == (u16 ) ~0 ) { +if (RESPONSE_IS_PCI_ERROR (slot_ctrl)) {ctrl_info (ctrl,"%s: no response from device\n" , __func__); goto out; }@@ -236,7 +236,7 @@ int pciehp_check_link_active(struct controller *ctrl)int ret; ret =pcie_capability_read_word (pdev,PCI_EXP_LNKSTA , &lnk_status); -if (ret ==PCIBIOS_DEVICE_NOT_FOUND || lnk_status == (u16 )~0 ) +if (ret ==PCIBIOS_DEVICE_NOT_FOUND ||RESPONSE_IS_PCI_ERROR (lnk_status))return -ENODEV ; ret = !!(lnk_status &PCI_EXP_LNKSTA_DLLLA );@@ -443,7 +443,7 @@ int pciehp_card_present(struct controller *ctrl)int ret; ret =pcie_capability_read_word (pdev,PCI_EXP_SLTSTA , &slot_status); -if (ret ==PCIBIOS_DEVICE_NOT_FOUND || slot_status == (u16 )~0 ) +if (ret ==PCIBIOS_DEVICE_NOT_FOUND ||RESPONSE_IS_PCI_ERROR (slot_status))return -ENODEV ;return !!(slot_status &PCI_EXP_SLTSTA_PDS );@@ -621,7 +621,7 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id) read_status:pcie_capability_read_word (pdev,PCI_EXP_SLTSTA , &status); -if (status == (u16 ) ~0 ) { +if (RESPONSE_IS_PCI_ERROR (status)) {ctrl_info (ctrl,"%s: no response from device\n" , __func__);if (parent)pm_runtime_put (parent);Signature · 1 line
2.25.1
Lukas, I have not added your Acked-by tag from the v1 [1] of the patch series, since the RESPONSE_IS_PCI_ERROR macro definition slightly changed. I hope this was the right thing to do.
[1]: https://lore.kernel.org/linux-pci/[email protected]/
Also, regarding your comments from v1 patch series [1] about re-naming the RESPONSE_IS_PCI_ERROR to RESPONSE_IS_PCI_TIMEOUT. We could indeed change the change to RESPONSE_IS_PCI_TIMEOUT for pciehp, but then I'm afraid that picehp would be the odd one out. I mean, since in all the other places we are using RESPONE_IS_PCI_TIMEOUT to see if any error occured while reading from a device.
RESPONSE_IS_PCI_ERROR stills gives an idea to the readers that some PCI error occured. It was my understanding that timeout is also a kind of PCI error (I might be horribly wrong here, given my very less experience with PCI subsystem) so it would be okay to use RESPONSE_IS_PCI_ERROR here.
If that is not the case please let me know. But I am not sure what to do here? If RESPONSE_IS_PCI_ERROR does not fit here, should the right option would be to revert/remove this patch from the series?
Thanks,
Naveen
Re: [PATCH v3 18/25] PCI: pciehp: Use RESPONSE_IS_PCI_ERROR() to check read from hardware
Quoted 3 lines · Naveen Naidu
Lukas, I have not added your Acked-by tag from the v1 [1] of the patch series, since the
RESPONSE_IS_PCI_ERRORmacro definition slightly changed. I hope this was the right thing to do.
[...]
Quoted 3 lines
If that is not the case please let me know. But I am not sure what to do here? If
RESPONSE_IS_PCI_ERRORdoes not fit here, should the right option would be to revert/remove this patch from the series?
My Acked-by still stands. As for the macro name, I'm fine with whatever Bjorn and the community settle on. :)
Thanks,
Lukas
PATCH v3 19/25 PCI/DPC: Use RESPONSE_IS_PCI_ERROR() to check read from hardware
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
Use RESPONSE_IS_PCI_ERROR() to check the response we get when we read data from hardware.
This helps unify PCI error response checking and make error checks consistent and easier to find.
Compile tested only.
--- drivers/pci/pcie/dpc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/pci/pcie/dpc.c +2 -2
@@ -79,7 +79,7 @@ static bool dpc_completed(struct pci_dev *pdev)u16 status;pci_read_config_word (pdev, pdev->dpc_cap +PCI_EXP_DPC_STATUS , &status); -if ((status !=0xffff ) && (status &PCI_EXP_DPC_STATUS_TRIGGER )) +if ((!RESPONSE_IS_PCI_ERROR (status)) && (status &PCI_EXP_DPC_STATUS_TRIGGER ))return false;if (test_bit (PCI_DPC_RECOVERING , &pdev->priv_flags))
@@ -312,7 +312,7 @@ static irqreturn_t dpc_irq(int irq, void *context)pci_read_config_word (pdev, cap +PCI_EXP_DPC_STATUS , &status); -if (!(status &PCI_EXP_DPC_STATUS_INTERRUPT ) || status == (u16)(~0 )) +if (!(status &PCI_EXP_DPC_STATUS_INTERRUPT ) ||RESPONSE_IS_PCI_ERROR (status))return IRQ_NONE ; pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS,
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 20/25 PCI/PME: Use RESPONSE_IS_PCI_ERROR() to check read from hardware
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
Use RESPONSE_IS_PCI_ERROR() to check the response we get when we read data from hardware.
This helps unify PCI error response checking and make error checks consistent and easier to find.
Compile tested only.
--- drivers/pci/pcie/pme.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/pci/pcie/pme.c +2 -2
@@ -224,7 +224,7 @@ static void pcie_pme_work_fn(struct work_struct *work)break ;pcie_capability_read_dword (port,PCI_EXP_RTSTA , &rtsta); -if (rtsta == (u32 ) ~0 ) +if (RESPONSE_IS_PCI_ERROR (rtsta))break ;if (rtsta &PCI_EXP_RTSTA_PME ) {
@@ -274,7 +274,7 @@ static irqreturn_t pcie_pme_irq(int irq, void *context)spin_lock_irqsave (&data->lock, flags);pcie_capability_read_dword (port,PCI_EXP_RTSTA , &rtsta); -if (rtsta == (u32 ) ~0 || !(rtsta &PCI_EXP_RTSTA_PME )) { +if (RESPONSE_IS_PCI_ERROR (rtsta) || !(rtsta &PCI_EXP_RTSTA_PME )) {spin_unlock_irqrestore (&data->lock, flags);return IRQ_NONE ; }
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 21/25 PCI: cpqphp: Use RESPONSE_IS_PCI_ERROR() to check read from hardware
In reply to Naveen NaiduAn MMIO read from a PCI device that doesn't exist or doesn't respond causes a PCI error. There's no real data to return to satisfy the CPU read, so most hardware fabricates ~0 data.
Use RESPONSE_IS_PCI_ERROR() to check the response we get when we read data from hardware.
This helps unify PCI error response checking and make error checks consistent and easier to find.
--- drivers/pci/hotplug/cpqphp_ctrl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/pci/hotplug/cpqphp_ctrl.c +2 -2
@@ -2273,7 +2273,7 @@ static u32 configure_new_device(struct controller *ctrl, struct pci_func *funcwhile ((function < max_functions) && (!stop_it)) {pci_bus_read_config_dword (ctrl->pci_bus,PCI_DEVFN (func->device, function),0x00 , &ID ); -if (ID ==0xFFFFFFFF ) { +if (RESPONSE_IS_PCI_ERROR (ID )) { function++; }else {/* Setup slot structure. */
@@ -2517,7 +2517,7 @@ static int configure_new_function(struct controller *ctrl, struct pci_func *funcpci_bus_read_config_dword (pci_bus,PCI_DEVFN (device,0 ),0x00 , &ID ); pci_bus->number = func->bus; -if (ID !=0xFFFFFFFF ) {/* device present */ +if (!RESPONSE_IS_PCI_ERROR (ID )) {/* device present */ /* Setup slot structure. */ new_slot =cpqhp_slot_create (hold_bus_node->base);
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 22/25 PCI: Use PCI_ERROR_RESPONSE to specify hardware error
In reply to Naveen NaiduInclude PCI_ERROR_RESPONSE along with 0xFFFF and 0xFFFFFFFF in the comment to specify a hardware error. This makes MMIO read errors easier to find.
--- drivers/pci/access.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/pci/access.c +4 -4
@@ -413,8 +413,8 @@ int pcie_capability_read_word(struct pci_dev *dev, int pos, u16 *val) ret =pci_read_config_word (dev,pci_pcie_cap (dev) + pos, val);/* * Reset *val to 0 if pci_read_config_word() fails, it may -* have been written as 0xFFFF if hardware error happens -* during pci_read_config_word(). +* have been written as 0xFFFF (PCI_ERROR_RESPONSE) if hardware error +* happens during pci_read_config_word(). */ if (ret) *val =0 ;
@@ -448,8 +448,8 @@ int pcie_capability_read_dword(struct pci_dev *dev, int pos, u32 *val) ret =pci_read_config_dword (dev,pci_pcie_cap (dev) + pos, val);/* * Reset *val to 0 if pci_read_config_dword() fails, it may -* have been written as 0xFFFFFFFF if hardware error happens -* during pci_read_config_dword(). +* have been written as 0xFFFFFFFF (PCI_ERROR_RESPONSE) if hardware +* error happens during pci_read_config_dword(). */ if (ret) *val =0 ;
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 23/25 PCI: keystone: Use PCI_ERROR_RESPONSE to specify hardware error
In reply to Naveen NaiduInclude PCI_ERROR_RESPONSE along with 0xffffffff in the comment to specify a hardware error. This makes MMIO read errors easier to find.
--- drivers/pci/controller/dwc/pci-keystone.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/pci/controller/dwc/pci-keystone.c +2 -2
@@ -748,8 +748,8 @@ static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)#ifdef CONFIG_ARM /* * When a PCI device does not exist during config cycles, keystone host gets a -* bus error instead of returning 0xffffffff. This handler always returns 0 -* for this kind of faults. +* bus error instead of returning 0xffffffff (PCI_ERROR_RESPONSE). +* This handler always returns 0 for this kind of faults. */ static int ks_pcie_fault (unsigned long addr,unsigned int fsr,struct pt_regs *regs)
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 24/25 PCI: hv: Use PCI_ERROR_RESPONSE to specify hardware read error
In reply to Naveen NaiduInclude PCI_ERROR_RESPONSE along with 0xFFFFFFFF in the comment to specify a hardware error. This makes MMIO read errors easier to find.
--- drivers/pci/controller/pci-hyperv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pci/controller/pci-hyperv.c +1 -1
@@ -1774,7 +1774,7 @@ static void prepopulate_bars(struct hv_pcibus_device *hbus)* If the memory enable bit is already set, Hyper-V silently ignores * the below BAR updates, and the related PCI device driver can not * work, because reading from the device register(s) always returns -* 0xFFFFFFFF. +* 0xFFFFFFFF (PCI_ERROR_RESPONSE). */ list_for_each_entry (hpdev, &hbus->children, list_entry) {_hv_pcifront_read_config (hpdev,PCI_COMMAND ,2 , &command);
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 25/25 PCI: xgene: Use PCI_ERROR_RESPONSE to specify hardware error
In reply to Naveen NaiduInclude PCI_ERROR_RESPONSE along with 0xffffffff in the comment to specify a hardware error. This makes MMIO read errors easier to find.
--- drivers/pci/controller/pci-xgene.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/pci/controller/pci-xgene.c +4 -4
@@ -176,10 +176,10 @@ static int xgene_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,* Retry Status (CRS) logic: when CRS Software Visibility is * enabled and we read the Vendor and Device ID of a non-existent * device, the controller fabricates return data of 0xFFFF0001 -* ("device exists but is not ready") instead of 0xFFFFFFFF -* ("device does not exist"). This causes the PCI core to retry -* the read until it times out. Avoid this by not claiming to -* support CRS SV. +* ("device exists but is not ready") instead of +* 0xFFFFFFFF (PCI_ERROR_RESPONSE) ("device does not exist"). This causes +* the PCI core to retry the read until it times out. +* Avoid this by not claiming to support CRS SV. */ if (pci_is_root_bus (bus) && (port->version ==XGENE_PCIE_IP_VER_1 ) && ((where & ~0x3 ) ==XGENE_V1_PCI_EXP_CAP +PCI_EXP_RTCTL ))
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees