| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20190823104415.GC14582@e119886-lin.cambridge.arm.com>
Date: Fri, 23 Aug 2019 11:44:15 +0100
From: Andrew Murray <andrew.murray@....com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: linux-pci@...r.kernel.org,
Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Keith Busch <keith.busch@...el.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Rafael J . Wysocki" <rjw@...ysocki.net>,
Mika Westerberg <mika.westerberg@...ux.intel.com>,
linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
Bjorn Helgaas <bhelgaas@...gle.com>,
"Rafael J . Wysocki" <rafael.j.wysocki@...el.com>
Subject: Re: [PATCH 1/3] PCI: Add PCI_ERROR_RESPONSE definition
On Thu, Aug 22, 2019 at 03:05:49PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@...gle.com>
>
> 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.
>
> Note that successful reads *also* may return ~0 data, so additional
> information (e.g., knowledge that ~0 is not a valid register value) is
> needed to reliably identify errors.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@...gle.com>
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
> drivers/pci/access.c | 13 ++++++------
> .../pci/controller/dwc/pcie-designware-host.c | 2 +-
> drivers/pci/controller/pci-aardvark.c | 2 +-
> drivers/pci/controller/pci-mvebu.c | 4 ++--
> drivers/pci/controller/pci-thunder-ecam.c | 20 +++++++++----------
> drivers/pci/controller/pci-thunder-pem.c | 2 +-
> drivers/pci/controller/pcie-altera.c | 2 +-
> drivers/pci/controller/pcie-iproc.c | 2 +-
> drivers/pci/controller/pcie-mediatek.c | 4 ++--
> drivers/pci/controller/pcie-rcar.c | 2 +-
> drivers/pci/controller/pcie-rockchip-host.c | 2 +-
> drivers/pci/controller/vmd.c | 2 +-
> drivers/pci/hotplug/cpqphp_ctrl.c | 12 +++++------
> drivers/pci/hotplug/cpqphp_pci.c | 20 +++++++++----------
> drivers/pci/hotplug/pciehp_hpc.c | 6 +++---
> drivers/pci/pci.c | 8 ++++----
> drivers/pci/pcie/dpc.c | 3 ++-
> drivers/pci/pcie/pme.c | 4 ++--
> drivers/pci/probe.c | 4 ++--
> drivers/pci/quirks.c | 2 +-
> include/linux/pci.h | 7 +++++++
> 21 files changed, 65 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/pci/access.c b/drivers/pci/access.c
> index 544922f097c0..02186f3471d8 100644
> --- a/drivers/pci/access.c
> +++ b/drivers/pci/access.c
> @@ -81,7 +81,7 @@ int pci_generic_config_read(struct pci_bus *bus, unsigned int devfn,
>
> addr = bus->ops->map_bus(bus, devfn, where);
> if (!addr) {
> - *val = ~0;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
>
> @@ -123,7 +123,7 @@ int pci_generic_config_read32(struct pci_bus *bus, unsigned int devfn,
>
> addr = bus->ops->map_bus(bus, devfn, where & ~0x3);
> if (!addr) {
> - *val = ~0;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
>
> @@ -536,7 +536,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;
> + *val = (u8) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
> return pci_bus_read_config_byte(dev->bus, dev->devfn, where, val);
> @@ -546,18 +546,17 @@ 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;
> + *val = (u16) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
> return pci_bus_read_config_word(dev->bus, dev->devfn, where, val);
> }
> EXPORT_SYMBOL(pci_read_config_word);
>
> -int pci_read_config_dword(const struct pci_dev *dev, int where,
> - u32 *val)
> +int pci_read_config_dword(const struct pci_dev *dev, int where, u32 *val)
> {
> if (pci_dev_is_disconnected(dev)) {
> - *val = ~0;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
> return pci_bus_read_config_dword(dev->bus, dev->devfn, where, val);
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index f93252d0da5b..eb2e8b080a7d 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -594,7 +594,7 @@ static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
> struct pcie_port *pp = bus->sysdata;
>
> if (!dw_pcie_valid_device(pp, bus, PCI_SLOT(devfn))) {
> - *val = 0xffffffff;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
>
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index fc0fe4d4de49..77ad25ed7d38 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -520,7 +520,7 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn,
> int ret;
>
> if (!advk_pcie_valid_device(pcie, bus, devfn)) {
> - *val = 0xffffffff;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
>
> diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
> index d3a0419e42f2..a45e45226790 100644
> --- a/drivers/pci/controller/pci-mvebu.c
> +++ b/drivers/pci/controller/pci-mvebu.c
> @@ -648,7 +648,7 @@ static int mvebu_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>
> port = mvebu_pcie_find_port(pcie, bus, devfn);
> if (!port) {
> - *val = 0xffffffff;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
>
> @@ -658,7 +658,7 @@ static int mvebu_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
> size, val);
>
> if (!mvebu_pcie_link_up(port)) {
> - *val = 0xffffffff;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
>
> diff --git a/drivers/pci/controller/pci-thunder-ecam.c b/drivers/pci/controller/pci-thunder-ecam.c
> index 32d1d7b81ef4..b0332f349863 100644
> --- a/drivers/pci/controller/pci-thunder-ecam.c
> +++ b/drivers/pci/controller/pci-thunder-ecam.c
> @@ -42,7 +42,7 @@ 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;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
> v = readl(addr);
> @@ -57,7 +57,7 @@ static int handle_ea_bar(u32 e0, int bar, struct pci_bus *bus,
>
> addr = bus->ops->map_bus(bus, devfn, bar); /* BAR 0 */
> if (!addr) {
> - *val = ~0;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
> barl_orig = readl(addr + 0);
> @@ -73,7 +73,7 @@ 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;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
> v = readl(addr); /* EA entry-3. Base-H */
> @@ -105,7 +105,7 @@ 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;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
>
> @@ -136,7 +136,7 @@ static int thunder_ecam_config_read(struct pci_bus *bus, unsigned int devfn,
>
> addr = bus->ops->map_bus(bus, devfn, 0xc);
> if (!addr) {
> - *val = ~0;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
>
> @@ -147,7 +147,7 @@ static int thunder_ecam_config_read(struct pci_bus *bus, unsigned int devfn,
>
> addr = bus->ops->map_bus(bus, devfn, 8);
> if (!addr) {
> - *val = ~0;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
>
> @@ -177,7 +177,7 @@ 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;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
>
> @@ -197,7 +197,7 @@ static int thunder_ecam_config_read(struct pci_bus *bus, unsigned int devfn,
>
> addr = bus->ops->map_bus(bus, devfn, 0x70);
> if (!addr) {
> - *val = ~0;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
> /* E_CAP */
> @@ -212,7 +212,7 @@ 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;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
> v = readl(addr);
> @@ -269,7 +269,7 @@ 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;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
> v = readl(addr);
> diff --git a/drivers/pci/controller/pci-thunder-pem.c b/drivers/pci/controller/pci-thunder-pem.c
> index f127ce8bd4ef..115e2c94d968 100644
> --- a/drivers/pci/controller/pci-thunder-pem.c
> +++ b/drivers/pci/controller/pci-thunder-pem.c
> @@ -31,7 +31,7 @@ static int thunder_pem_bridge_read(struct pci_bus *bus, unsigned int devfn,
> struct thunder_pem_pci *pem_pci = (struct thunder_pem_pci *)cfg->priv;
>
> if (devfn != 0 || where >= 2048) {
> - *val = ~0;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
>
> diff --git a/drivers/pci/controller/pcie-altera.c b/drivers/pci/controller/pcie-altera.c
> index d2497ca43828..3bd63b507eb1 100644
> --- a/drivers/pci/controller/pcie-altera.c
> +++ b/drivers/pci/controller/pcie-altera.c
> @@ -512,7 +512,7 @@ static int altera_pcie_cfg_read(struct pci_bus *bus, unsigned int devfn,
> return PCIBIOS_BAD_REGISTER_NUMBER;
>
> if (!altera_pcie_valid_device(pcie, bus, PCI_SLOT(devfn))) {
> - *value = 0xffffffff;
> + *value = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
>
> diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c
> index 2d457bfdaf66..1fafedb39c00 100644
> --- a/drivers/pci/controller/pcie-iproc.c
> +++ b/drivers/pci/controller/pcie-iproc.c
> @@ -668,7 +668,7 @@ static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie,
>
> addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
> if (!addr) {
> - *val = ~0;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
>
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index 80601e1b939e..38f15a5e18ad 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -361,13 +361,13 @@ static int mtk_pcie_config_read(struct pci_bus *bus, unsigned int devfn,
>
> port = mtk_pcie_find_port(bus, devfn);
> if (!port) {
> - *val = ~0;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
>
> ret = mtk_pcie_hw_rd_cfg(port, bn, devfn, where, size, val);
> if (ret)
> - *val = ~0;
> + *val = (u32) PCI_ERROR_RESPONSE;
>
> return ret;
> }
> diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c
> index f6a669a9af41..ae8d95146351 100644
> --- a/drivers/pci/controller/pcie-rcar.c
> +++ b/drivers/pci/controller/pcie-rcar.c
> @@ -277,7 +277,7 @@ static int rcar_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
> ret = rcar_pcie_config_access(pcie, RCAR_PCI_ACCESS_READ,
> bus, devfn, where, val);
> if (ret != PCIBIOS_SUCCESSFUL) {
> - *val = 0xffffffff;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return ret;
> }
>
> diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> index 8d20f1793a61..48d85f648d79 100644
> --- a/drivers/pci/controller/pcie-rockchip-host.c
> +++ b/drivers/pci/controller/pcie-rockchip-host.c
> @@ -226,7 +226,7 @@ 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;
> + *val = (u32) PCI_ERROR_RESPONSE;
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
>
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index 4575e0c6dc4b..2af43b9e85db 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -578,7 +578,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>
> membar2_offset = 0x2018;
> ret = pci_read_config_dword(vmd->dev, PCI_REG_VMLOCK, &vmlock);
> - if (ret || vmlock == ~0)
> + if (ret || vmlock == (u32) PCI_ERROR_RESPONSE)
> return -ENODEV;
>
> if (MB2_SHADOW_EN(vmlock)) {
> diff --git a/drivers/pci/hotplug/cpqphp_ctrl.c b/drivers/pci/hotplug/cpqphp_ctrl.c
> index b7f4e1f099d9..e9d2bf4eeeef 100644
> --- a/drivers/pci/hotplug/cpqphp_ctrl.c
> +++ b/drivers/pci/hotplug/cpqphp_ctrl.c
> @@ -1497,7 +1497,7 @@ static u32 board_added(struct pci_func *func, struct controller *ctrl)
> /* Check for a power fault */
> if (func->status == 0xFF) {
> /* power fault occurred, but it was benign */
> - temp_register = 0xFFFFFFFF;
> + temp_register = (u32) PCI_ERROR_RESPONSE;
> dbg("%s: temp register set to %x by power fault\n", __func__, temp_register);
> rc = POWER_FAILURE;
> func->status = 0;
> @@ -1510,7 +1510,7 @@ static u32 board_added(struct pci_func *func, struct controller *ctrl)
>
> if (rc != 0) {
> /* Something's wrong here */
> - temp_register = 0xFFFFFFFF;
> + temp_register = (u32) PCI_ERROR_RESPONSE;
> dbg("%s: temp register set to %x by error\n", __func__, temp_register);
> }
> /* Preset return code. It will be changed later if things go okay. */
> @@ -1518,7 +1518,7 @@ static u32 board_added(struct pci_func *func, struct controller *ctrl)
> }
>
> /* All F's is an empty slot or an invalid board */
> - if (temp_register != 0xFFFFFFFF) {
> + if (temp_register != (u32) PCI_ERROR_RESPONSE) {
> res_lists.io_head = ctrl->io_head;
> res_lists.mem_head = ctrl->mem_head;
> res_lists.p_mem_head = ctrl->p_mem_head;
> @@ -2277,7 +2277,7 @@ static u32 configure_new_device(struct controller *ctrl, struct pci_func *func
> while ((function < max_functions) && (!stop_it)) {
> pci_bus_read_config_dword(ctrl->pci_bus, PCI_DEVFN(func->device, function), 0x00, &ID);
>
> - if (ID == 0xFFFFFFFF) {
> + if (ID == (u32) PCI_ERROR_RESPONSE) {
> function++;
> } else {
> /* Setup slot structure. */
> @@ -2516,12 +2516,12 @@ static int configure_new_function(struct controller *ctrl, struct pci_func *func
> for (device = 0; (device <= 0x1F) && !rc; device++) {
> irqs.barber_pole = (irqs.barber_pole + 1) & 0x03;
>
> - ID = 0xFFFFFFFF;
> + ID = (u32) PCI_ERROR_RESPONSE;
> pci_bus->number = hold_bus_node->base;
> pci_bus_read_config_dword(pci_bus, PCI_DEVFN(device, 0), 0x00, &ID);
> pci_bus->number = func->bus;
>
> - if (ID != 0xFFFFFFFF) { /* device present */
> + if (ID != (u32) PCI_ERROR_RESPONSE) { /* device present */
> /* Setup slot structure. */
> new_slot = cpqhp_slot_create(hold_bus_node->base);
>
> diff --git a/drivers/pci/hotplug/cpqphp_pci.c b/drivers/pci/hotplug/cpqphp_pci.c
> index 1b2b3f3b648b..d6ab73b9b280 100644
> --- a/drivers/pci/hotplug/cpqphp_pci.c
> +++ b/drivers/pci/hotplug/cpqphp_pci.c
> @@ -138,7 +138,7 @@ static int PCI_RefinedAccessConfig(struct pci_bus *bus, unsigned int devfn, u8 o
>
> if (pci_bus_read_config_dword(bus, devfn, PCI_VENDOR_ID, &vendID) == -1)
> return -1;
> - if (vendID == 0xffffffff)
> + if (vendID == (u32) PCI_ERROR_RESPONSE)
> return -1;
> return pci_bus_read_config_dword(bus, devfn, offset, value);
> }
> @@ -251,7 +251,7 @@ static int PCI_GetBusDevHelper(struct controller *ctrl, u8 *bus_num, u8 *dev_num
> *dev_num = tdevice;
> ctrl->pci_bus->number = tbus;
> pci_bus_read_config_dword(ctrl->pci_bus, *dev_num, PCI_VENDOR_ID, &work);
> - if (!nobridge || (work == 0xffffffff))
> + if (!nobridge || (work == (u32) PCI_ERROR_RESPONSE))
> return 0;
>
> dbg("bus_num %d devfn %d\n", *bus_num, *dev_num);
> @@ -330,10 +330,10 @@ int cpqhp_save_config(struct controller *ctrl, int busnumber, int is_hot_plug)
> /* Save PCI configuration space for all devices in supported slots */
> ctrl->pci_bus->number = busnumber;
> for (device = FirstSupported; device <= LastSupported; device++) {
> - ID = 0xFFFFFFFF;
> + ID = (u32) PCI_ERROR_RESPONSE;
> rc = pci_bus_read_config_dword(ctrl->pci_bus, PCI_DEVFN(device, 0), PCI_VENDOR_ID, &ID);
>
> - if (ID == 0xFFFFFFFF) {
> + if (ID == (u32) PCI_ERROR_RESPONSE) {
> if (is_hot_plug) {
> /* Setup slot structure with entry for empty
> * slot
> @@ -431,7 +431,7 @@ int cpqhp_save_config(struct controller *ctrl, int busnumber, int is_hot_plug)
> */
> while ((function < max_functions) && (!stop_it)) {
> rc = pci_bus_read_config_dword(ctrl->pci_bus, PCI_DEVFN(device, function), PCI_VENDOR_ID, &ID);
> - if (ID == 0xFFFFFFFF) {
> + if (ID == (u32) PCI_ERROR_RESPONSE) {
> function++;
> continue;
> }
> @@ -474,12 +474,12 @@ int cpqhp_save_slot_config(struct controller *ctrl, struct pci_func *new_slot)
> int cloop = 0;
> int stop_it;
>
> - ID = 0xFFFFFFFF;
> + ID = (u32) PCI_ERROR_RESPONSE;
>
> ctrl->pci_bus->number = new_slot->bus;
> pci_bus_read_config_dword(ctrl->pci_bus, PCI_DEVFN(new_slot->device, 0), PCI_VENDOR_ID, &ID);
>
> - if (ID == 0xFFFFFFFF)
> + if (ID == (u32) PCI_ERROR_RESPONSE)
> return 2;
>
> pci_bus_read_config_byte(ctrl->pci_bus, PCI_DEVFN(new_slot->device, 0), 0x0B, &class_code);
> @@ -522,7 +522,7 @@ int cpqhp_save_slot_config(struct controller *ctrl, struct pci_func *new_slot)
> while ((function < max_functions) && (!stop_it)) {
> pci_bus_read_config_dword(ctrl->pci_bus, PCI_DEVFN(new_slot->device, function), PCI_VENDOR_ID, &ID);
>
> - if (ID == 0xFFFFFFFF)
> + if (ID == (u32) PCI_ERROR_RESPONSE)
> function++;
> else {
> pci_bus_read_config_byte(ctrl->pci_bus, PCI_DEVFN(new_slot->device, function), 0x0B, &class_code);
> @@ -1049,7 +1049,7 @@ int cpqhp_valid_replace(struct controller *ctrl, struct pci_func *func)
> pci_bus_read_config_dword(pci_bus, devfn, PCI_VENDOR_ID, &temp_register);
>
> /* No adapter present */
> - if (temp_register == 0xFFFFFFFF)
> + if (temp_register == (u32) PCI_ERROR_RESPONSE)
> return(NO_ADAPTER_PRESENT);
>
> if (temp_register != func->config_space[0])
> @@ -1267,7 +1267,7 @@ int cpqhp_find_available_resources(struct controller *ctrl, void __iomem *rom_st
> pci_bus_read_config_dword(ctrl->pci_bus, dev_func, PCI_VENDOR_ID, &temp_dword);
> dbg("temp_D_word = %x\n", temp_dword);
>
> - if (temp_dword != 0xFFFFFFFF) {
> + if (temp_dword != (u32) PCI_ERROR_RESPONSE) {
> index = 0;
> func = cpqhp_slot_find(primary_bus, dev_func >> 3, 0);
>
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index bd990e3371e3..f0489f305ad7 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -70,7 +70,7 @@ static int pcie_poll_cmd(struct controller *ctrl, int timeout)
>
> while (true) {
> pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
> - if (slot_status == (u16) ~0) {
> + if (slot_status == (u16) PCI_ERROR_RESPONSE) {
> ctrl_info(ctrl, "%s: no response from device\n",
> __func__);
> return 0;
> @@ -148,7 +148,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 (slot_ctrl == (u16) PCI_ERROR_RESPONSE) {
> ctrl_info(ctrl, "%s: no response from device\n", __func__);
> goto out;
> }
> @@ -543,7 +543,7 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id)
> }
>
> pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &status);
> - if (status == (u16) ~0) {
> + if (status == (u16) PCI_ERROR_RESPONSE) {
> ctrl_info(ctrl, "%s: no response from device\n", __func__);
> if (parent)
> pm_runtime_put(parent);
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 29ed5ec1ac27..bfc739dc6ada 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4434,16 +4434,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 data (PCI_ERROR_RESPONSE) 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).
There are some other areas in drivers/pci where comments also refer to ~0
and similar:
$ git grep -i ffffffff drivers/pci/ | grep \*
drivers/pci/access.c: * have been written as 0xFFFFFFFF if hardware error happens
drivers/pci/controller/dwc/pci-keystone.c: * bus error instead of returning 0xffffffff. This handler always returns 0
drivers/pci/controller/pci-xgene.c: * ready") instead of 0xFFFFFFFF ("device does not exist"). This
drivers/pci/controller/pcie-iproc.c: * eventually return the wrong data (0xffffffff).
drivers/pci/pci.c: * FFFFFFFFs on the command line.)
I've removed anything in the above list that doesn't look like a good candidate
for PCI_ERROR_RESPONSE.
Perhaps there is some value for replacing "~0" with "~0 (PCI_ERROR_RESPONSE)"
in the comments too?
> *
> * 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 (id == (u32) PCI_ERROR_RESPONSE) {
> if (delay > timeout) {
> pci_warn(dev, "not ready %dms after %s; giving up\n",
> delay - 1, reset_type);
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index a32ec3487a8d..96b6b87a0af3 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -272,7 +272,8 @@ 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) ||
> + status == (u16)(PCI_ERROR_RESPONSE))
> return IRQ_NONE;
>
> pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS,
> diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c
> index f38e6c19dd50..3c46622e6c3f 100644
> --- a/drivers/pci/pcie/pme.c
> +++ b/drivers/pci/pcie/pme.c
> @@ -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 (rtsta == (u32) PCI_ERROR_RESPONSE)
> 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 (rtsta == (u32) PCI_ERROR_RESPONSE || !(rtsta & PCI_EXP_RTSTA_PME)) {
> spin_unlock_irqrestore(&data->lock, flags);
> return IRQ_NONE;
> }
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index a3c7338fad86..fb953f2666b9 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1549,7 +1549,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 (status == (u32) PCI_ERROR_RESPONSE || pci_ext_cfg_is_aliased(dev))
> return PCI_CFG_SPACE_SIZE;
>
> return PCI_CFG_SPACE_EXP_SIZE;
> @@ -2488,7 +2488,7 @@ bool pci_bus_generic_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
> return false;
>
> /* Some broken boards return 0 or ~0 if a slot is empty: */
> - if (*l == 0xffffffff || *l == 0x00000000 ||
> + if (*l == (u32) PCI_ERROR_RESPONSE || *l == 0x00000000 ||
> *l == 0x0000ffff || *l == 0xffff0000)
> return false;
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 208aacf39329..3d5c92b53310 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -4854,7 +4854,7 @@ static void quirk_intel_qat_vf_cap(struct pci_dev *pdev)
>
> pdev->cfg_size = PCI_CFG_SPACE_EXP_SIZE;
> if (pci_read_config_dword(pdev, PCI_CFG_SPACE_SIZE, &status) !=
> - PCIBIOS_SUCCESSFUL || (status == 0xffffffff))
> + PCIBIOS_SUCCESSFUL || (status == (u32) PCI_ERROR_RESPONSE))
The casts are necessary but slightly annoying. Have you considered something
like:
#define SET_PCI_ERROR_RESPONSE(val) (val = ((typeof(val))(~0ULL)))
#define RESPONSE_IS_PCI_ERROR(val) (val == ((typeof(val))(~0ULL)))
Thanks,
Andrew Murray
> pdev->cfg_size = PCI_CFG_SPACE_SIZE;
>
> if (pci_find_saved_cap(pdev, PCI_CAP_ID_EXP))
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 9e700d9f9f28..d64fd3788061 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -123,6 +123,13 @@ 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)
> +
> /*
> * pci_power_t values must match the bits in the Capabilities PME_Support
> * and Control/Status PowerState fields in the Power Management capability.
> --
> 2.23.0.187.g17f5b7556c-goog
>
Powered by blists - more mailing lists