[PATCH part1 v6 0/7] Introduce PCIe Device Serial Number capability support

[PATCH part1 v6 0/7] Introduce PCIe Device Serial Number capability support

[Yijing Wang]

v5->v6: Add is_frozen flag to protect pci bus from double frozen.

This series is based on Bjorn's pci-next branch.
Currently, more and more PCIe devices support PCIe DSN(Device Serial Number)
capability, we can use this cap to identify device. In some platform,
when we hotplug PCIe device, no interrupts will be received in system.
So we can not identify the device whether is changed during suspend.
PCIe DSN can help to identify this. 

Legacy PCI device can achieve it by PCI Vital Product Data capability.
We can use its SN keyword to report the unique number.
But in my platform, PCI devices which support VPD SN cap report the
meaningless same string "0123456789". 
Rework PCI VPD code to support device identification is not an easy work.
Plan to do it in part2.

Yijing Wang (7):
  PCI: rework pci_find_next_ext_capability()
  PCI: introduce pci_bus_find_ext_capability()
  PCI: Add support for Device Serial Number capability
  PCI: Introduce pci_serial_number_changed()
  PCI: Add pci_dummy_ops to isolate pci device temporarily
  PCI: Check pci device serial number when scan device
  PCI: pciehp: Don't enable/disable slot on resume unless status
    changed

 drivers/pci/hotplug/pciehp_core.c |   10 ++-
 drivers/pci/pci.c                 |  169 +++++++++++++++++++++++++++++++++++--
 drivers/pci/pci.h                 |    2 +-
 drivers/pci/probe.c               |   15 +++-
 include/linux/pci.h               |   10 ++-
 5 files changed, 192 insertions(+), 14 deletions(-)

[PATCH part1 v6 1/7] PCI: rework pci_find_next_ext_capability()

[Yijing Wang]

Rework pci_find_next_ext_capability(), use
pci_bus_read_config_xxx() instead of
pci_read_config_xxx(). So we can use this
function before pci_dev setup.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/pci.c   |   17 +++++++++--------
 include/linux/pci.h |    2 +-
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 1febe90..a263c0a 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -256,7 +256,8 @@ int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap)
 
 /**
  * pci_find_next_ext_capability - Find an extended capability
- * @dev: PCI device to query
+ * @bus: the PCI bus to query
+ * @devfn: PCI device to query
  * @start: address at which to start looking (0 to start at beginning of list)
  * @cap: capability code
  *
@@ -265,7 +266,7 @@ int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap)
  * not support it.  Some capabilities can occur several times, e.g., the
  * vendor-specific capability, and this provides a way to find them all.
  */
-int pci_find_next_ext_capability(struct pci_dev *dev, int start, int cap)
+int pci_find_next_ext_capability(struct pci_bus *bus, int devfn, int start, int cap)
 {
 	u32 header;
 	int ttl;
@@ -274,13 +275,10 @@ int pci_find_next_ext_capability(struct pci_dev *dev, int start, int cap)
 	/* minimum 8 bytes per capability */
 	ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;
 
-	if (dev->cfg_size <= PCI_CFG_SPACE_SIZE)
-		return 0;
-
 	if (start)
 		pos = start;
 
-	if (pci_read_config_dword(dev, pos, &header) != PCIBIOS_SUCCESSFUL)
+	if (pci_bus_read_config_dword(bus, devfn, pos, &header) != PCIBIOS_SUCCESSFUL)
 		return 0;
 
 	/*
@@ -298,7 +296,7 @@ int pci_find_next_ext_capability(struct pci_dev *dev, int start, int cap)
 		if (pos < PCI_CFG_SPACE_SIZE)
 			break;
 
-		if (pci_read_config_dword(dev, pos, &header) != PCIBIOS_SUCCESSFUL)
+		if (pci_bus_read_config_dword(bus, devfn, pos, &header) != PCIBIOS_SUCCESSFUL)
 			break;
 	}
 
@@ -322,7 +320,10 @@ EXPORT_SYMBOL_GPL(pci_find_next_ext_capability);
  */
 int pci_find_ext_capability(struct pci_dev *dev, int cap)
 {
-	return pci_find_next_ext_capability(dev, 0, cap);
+	if (dev->cfg_size <= PCI_CFG_SPACE_SIZE)
+		return 0;
+
+	return pci_find_next_ext_capability(dev->bus, dev->devfn, 0, cap);
 }
 EXPORT_SYMBOL_GPL(pci_find_ext_capability);
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index fb57c89..df495e9 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -796,7 +796,7 @@ enum pci_lost_interrupt_reason pci_lost_interrupt(struct pci_dev *dev);
 int pci_find_capability(struct pci_dev *dev, int cap);
 int pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap);
 int pci_find_ext_capability(struct pci_dev *dev, int cap);
-int pci_find_next_ext_capability(struct pci_dev *dev, int pos, int cap);
+int pci_find_next_ext_capability(struct pci_bus *bus, int devfn, int pos, int cap);
 int pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
 int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap);
 struct pci_bus *pci_find_next_bus(const struct pci_bus *from);
-- 
1.7.1

[PATCH part1 v6 2/7] PCI: introduce pci_bus_find_ext_capability()

[Yijing Wang]

Sometimes we need to find PCI EXT CAP before
pci_dev set up. So introduce pci_bus_find_ext_capability(),
it will be used to get PCI EXT DSN before pci_dev set up.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/pci.c   |   32 ++++++++++++++++++++++++++++++++
 include/linux/pci.h |    1 +
 2 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index a263c0a..4502e6f 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -327,6 +327,38 @@ int pci_find_ext_capability(struct pci_dev *dev, int cap)
 }
 EXPORT_SYMBOL_GPL(pci_find_ext_capability);
 
+/**
+ * pci_bus_find_ext_capability - Find an extended capability
+ * @bus: the PCI bus to query
+ * @devfn: PCI device to query
+ * @cap: capability code
+ *
+ * Like pci_find_ext_capability() but works for pci devices that do not have a
+ * pci_dev structure set up yet.
+ *
+ * Returns the address of the requested capability structure within the
+ * device's PCI configuration space or 0 in case the device does not
+ * support it.
+ *
+ * */
+int pci_bus_find_ext_capability(struct pci_bus *bus, int devfn, int cap)
+{
+	int pos;
+	u32 status;
+
+	pos = pci_bus_find_capability(bus, devfn, PCI_CAP_ID_EXP);
+	if (!pos)
+		return 0;
+
+	if (pci_bus_read_config_dword(bus, devfn, PCI_CFG_SPACE_SIZE,
+			&status) !=
+			PCIBIOS_SUCCESSFUL || status == 0xffffffff)
+		return 0;
+
+	return pci_find_next_ext_capability(bus, devfn, 0, cap);
+}
+
+
 static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap)
 {
 	int rc, ttl = PCI_FIND_CAP_TTL;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index df495e9..470de02 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -797,6 +797,7 @@ int pci_find_capability(struct pci_dev *dev, int cap);
 int pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap);
 int pci_find_ext_capability(struct pci_dev *dev, int cap);
 int pci_find_next_ext_capability(struct pci_bus *bus, int devfn, int pos, int cap);
+int pci_bus_find_ext_capability(struct pci_bus *bus, int devfn, int cap);
 int pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
 int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap);
 struct pci_bus *pci_find_next_bus(const struct pci_bus *from);
-- 
1.7.1

[PATCH part1 v6 3/7] PCI: Add support for Device Serial Number capability

[Yijing Wang]

Add support for the Device Serial Number capability, so we can use the
unique device serial number to identify the physical device.  This helps
determine whether a device was replaced while the system was suspended.

[bhelgaas: changelog, drop pci_dsn_init(), spell out "serial_number"]
Reviewed-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Paul Bolle <pebolle@tiscali.nl>
Cc: Oliver Neukum <oneukum@suse.de>
Cc: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
 drivers/pci/pci.c   |   30 ++++++++++++++++++++++++++++++
 drivers/pci/pci.h   |    2 +-
 drivers/pci/probe.c |    2 ++
 include/linux/pci.h |    1 +
 4 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 4502e6f..a8c1c2d 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2169,6 +2169,36 @@ void pci_free_cap_save_buffers(struct pci_dev *dev)
 }
 
 /**
+ * pci_device_serial_number - get device serial number
+ * @bus: PCI bus the device on
+ * @devfn: the PCI device
+ *
+ * return the device serial number if device support,
+ * otherwise return 0.
+ */
+static u64 pci_device_serial_number(struct pci_bus *bus, int devfn)
+{
+	int pos;
+	u32 lo, hi;
+
+	if (!pci_bus_find_capability(bus, devfn, PCI_CAP_ID_EXP))
+		return 0;
+
+	pos = pci_bus_find_ext_capability(bus, devfn, PCI_EXT_CAP_ID_DSN);
+	if (!pos)
+		return 0;
+
+	pci_bus_read_config_dword(bus, devfn, pos + 4, &lo);
+	pci_bus_read_config_dword(bus, devfn, pos + 8, &hi);
+	return ((u64)hi << 32) | lo;
+}
+
+void pci_dsn_init(struct pci_dev *dev)
+{
+	dev->sn = pci_device_serial_number(dev->bus, dev->devfn);
+}
+
+/**
  * pci_configure_ari - enable or disable ARI forwarding
  * @dev: the PCI device
  *
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4df38df..685301a 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -208,7 +208,7 @@ void __ref __pci_bus_size_bridges(struct pci_bus *bus,
 void __ref __pci_bus_assign_resources(const struct pci_bus *bus,
 				      struct list_head *realloc_head,
 				      struct list_head *fail_head);
-
+void pci_dsn_init(struct pci_dev *dev);
 /**
  * pci_ari_enabled - query ARI forwarding status
  * @bus: the PCI bus
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6e34498..27d3e6f 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1324,6 +1324,8 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	/* Vital Product Data */
 	pci_vpd_pci22_init(dev);
 
+	pci_dsn_init(dev);
+
 	/* Alternative Routing-ID Forwarding */
 	pci_configure_ari(dev);
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 470de02..3631859 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -354,6 +354,7 @@ struct pci_dev {
 	struct list_head msi_list;
 	const struct attribute_group **msi_irq_groups;
 #endif
+	u64 sn;		/* device serial number, 0 if not support */
 	struct pci_vpd *vpd;
 #ifdef CONFIG_PCI_ATS
 	union {
-- 
1.7.1

[PATCH part1 v6 4/7] PCI: Introduce pci_serial_number_changed()

[Yijing Wang]

Sometimes OS do not know the physical device swap,
for instance, some device hotplug during system suspend.
Interrupt can not deliver to OS in some platform.
So we can use pci serial number capability to detect this
issue if device supports serial number.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Cc: Paul Bolle <pebolle@tiscali.nl>
Cc: Oliver Neukum <oneukum@suse.de>
Cc: Gu Zheng <guz.fnst@cn.fujitsu.com>
Cc: linux-pci@vger.kernel.org
---
 drivers/pci/pci.c   |   28 ++++++++++++++++++++++++++++
 include/linux/pci.h |    2 ++
 2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index a8c1c2d..af34057 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2199,6 +2199,34 @@ void pci_dsn_init(struct pci_dev *dev)
 }
 
 /**
+ * pci_serial_number_changed - check the device SN is changed
+ * @pdev: the PCI device
+ *
+ * check the device serial number is changed.
+ * if device does not support device serial number,
+ * return false.
+ */
+bool pci_serial_number_changed(struct pci_dev *pdev)
+{
+	u64 old, new;
+	old = pdev->sn;
+
+	if (!pci_is_pcie(pdev))
+		return false;
+
+	new = pci_device_serial_number(pdev->bus,
+			pdev->devfn);
+
+	if (old != new) {
+		pr_info("%s: Device Serial Number Changed!\n",
+				pci_name(pdev));
+		return true;
+	} else
+		return false;
+}
+EXPORT_SYMBOL(pci_serial_number_changed);
+
+/**
  * pci_configure_ari - enable or disable ARI forwarding
  * @dev: the PCI device
  *
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 3631859..d60c0b6 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1025,6 +1025,8 @@ void pci_unlock_rescan_remove(void);
 ssize_t pci_read_vpd(struct pci_dev *dev, loff_t pos, size_t count, void *buf);
 ssize_t pci_write_vpd(struct pci_dev *dev, loff_t pos, size_t count, const void *buf);
 
+bool pci_serial_number_changed(struct pci_dev *pdev);
+
 /* Helper functions for low-level code (drivers/pci/setup-[bus,res].c) */
 resource_size_t pcibios_retrieve_fw_addr(struct pci_dev *dev, int idx);
 void pci_bus_assign_resources(const struct pci_bus *bus);
-- 
1.7.1

[PATCH part1 v6 5/7] PCI: Add pci_dummy_ops to isolate pci device temporarily

[Yijing Wang]

Pci_dummy_ops does nothing when we use it to read/write
pci_device. So we can isolate pci device by replace its
bus pci_ops by pci_dummy_ops. This is preparation for
the later patch.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/pci.c   |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h |    4 +++
 2 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index af34057..283e82e 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2226,6 +2226,68 @@ bool pci_serial_number_changed(struct pci_dev *pdev)
 }
 EXPORT_SYMBOL(pci_serial_number_changed);
 
+static int pci_dummy_read(struct pci_bus *bus, unsigned int devfn,
+		int where, int size, u32 *val)
+{
+	return PCIBIOS_DEVICE_NOT_FOUND;
+}
+
+static int pci_dummy_write(struct pci_bus *bus, unsigned int devfn,
+		int reg, int size, u32 val)
+{
+	return PCIBIOS_DEVICE_NOT_FOUND;
+}
+
+static struct pci_ops pci_dummy_ops = {
+	.read = pci_dummy_read,
+	.write = pci_dummy_write,
+};
+
+static DEFINE_SPINLOCK(pci_freeze_lock);
+/**
+ * pci_bus_freeze_device - freeze pci bus to access pci device
+ * @bus: the pci bus to freeze
+ *
+ * Replace pci bus ops by pci_dummy_ops, protect system from
+ * accessing pci devices.
+ */
+void pci_bus_freeze_device(struct pci_bus *bus)
+{
+	struct pci_ops *ops;
+	unsigned long flags;
+
+	spin_lock_irqsave(&pci_freeze_lock, flags);
+	if (bus->is_frozen)
+		goto out;
+	bus->is_frozen = 1;
+	ops = pci_bus_set_ops(bus, &pci_dummy_ops);
+	bus->save_ops = ops;
+out:
+	spin_unlock_irqrestore(&pci_freeze_lock, flags);
+}
+
+/**
+ * pci_bus_unfreeze_device - unfreeze pci bus to acess pci devices
+ * @bus: the pci bus to unfreeze
+ *
+ * Restore pci bus original ops, so pci bus can access
+ * pci devices normally.
+ */
+void pci_bus_unfreeze_device(struct pci_bus *bus)
+{
+	unsigned long flags;
+
+	BUG_ON(!bus->save_ops);
+	spin_lock_irqsave(&pci_freeze_lock, flags);
+	if (!bus->is_frozen)
+		goto out;
+	pci_bus_set_ops(bus, bus->save_ops);
+	bus->save_ops = NULL;
+	bus->is_frozen = 0;
+out:
+	spin_unlock_irqrestore(&pci_freeze_lock, flags);
+}
+
 /**
  * pci_configure_ari - enable or disable ARI forwarding
  * @dev: the PCI device
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d60c0b6..e3dd4ea 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -442,6 +442,7 @@ struct pci_bus {
 	struct resource busn_res;	/* bus numbers routed to this bus */
 
 	struct pci_ops	*ops;		/* configuration access functions */
+	struct pci_ops	*save_ops; /* save configuration access functions */
 	struct msi_chip	*msi;		/* MSI controller */
 	void		*sysdata;	/* hook for sys-specific extension */
 	struct proc_dir_entry *procdir;	/* directory entry in /proc/bus/pci */
@@ -460,6 +461,7 @@ struct pci_bus {
 	struct bin_attribute	*legacy_io; /* legacy I/O for this bus */
 	struct bin_attribute	*legacy_mem; /* legacy mem */
 	unsigned int		is_added:1;
+	unsigned int		is_frozen:1;
 };
 
 #define pci_bus_b(n)	list_entry(n, struct pci_bus, node)
@@ -1026,6 +1028,8 @@ ssize_t pci_read_vpd(struct pci_dev *dev, loff_t pos, size_t count, void *buf);
 ssize_t pci_write_vpd(struct pci_dev *dev, loff_t pos, size_t count, const void *buf);
 
 bool pci_serial_number_changed(struct pci_dev *pdev);
+void pci_bus_freeze_device(struct pci_bus *bus);
+void pci_bus_unfreeze_device(struct pci_bus *bus);
 
 /* Helper functions for low-level code (drivers/pci/setup-[bus,res].c) */
 resource_size_t pcibios_retrieve_fw_addr(struct pci_dev *dev, int idx);
-- 
1.7.1

[PATCH part1 v6 6/7] PCI: Check pci device serial number when scan device

[Yijing Wang]

Sometimes pci device will be removed and reinsert
while suspended. In this case system can not identify
the device has been changed. Now PCIe support Device
Serial Number Capability which has the unique number.
So make system check pci device DSN during scanning
device if support.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/probe.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 27d3e6f..370b25c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1383,11 +1383,22 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 struct pci_dev *__ref pci_scan_single_device(struct pci_bus *bus, int devfn)
 {
 	struct pci_dev *dev;
+	int rescan = 0;
 
 	dev = pci_get_slot(bus, devfn);
 	if (dev) {
+		/* only check func 0 device */
+		if (PCI_FUNC(devfn) == 0) {
+			if (pci_serial_number_changed(dev)) {
+				pci_bus_freeze_device(bus);
+				pci_stop_and_remove_bus_device(dev);
+				pci_bus_unfreeze_device(bus);
+				rescan = 1;
+			}
+		}
 		pci_dev_put(dev);
-		return dev;
+		if (!rescan)
+			return dev;
 	}
 
 	dev = pci_scan_device(bus, devfn);
-- 
1.7.1

[PATCH part1 v6 7/7] PCI: pciehp: Don't enable/disable slot on resume unless status changed

[Yijing Wang]

Currently pciehp_resume() always enables the slot if it is occupied.  But
often the slot was already occupied before the suspend, so we complain like
this:

    pciehp 0000:00:1c.1:pcie04: Device 0000:03:00.0 already exists at 0000:03:00, cannot hot-add
    pciehp 0000:00:1c.1:pcie04: Cannot add device at 0000:03:00

This patch only enables the slot if it was empty before the suspend and is
now occupied, i.e., a card was inserted while suspended.

Similarly, we only disable the slot if a card was removed while suspended.
If it was already empty before the suspend, we don't need to do anything.

[bhelgaas: changelog]
Tested-by: Paul Bolle <pebolle@tiscali.nl>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Oliver Neukum <oneukum@suse.de>
Cc: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
 drivers/pci/hotplug/pciehp_core.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
index 53b58de..551137f 100644
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -317,6 +317,7 @@ static int pciehp_resume (struct pcie_device *dev)
 {
 	struct controller *ctrl;
 	struct slot *slot;
+	struct pci_bus *pbus = dev->port->subordinate;
 	u8 status;
 
 	ctrl = get_service_data(dev);
@@ -328,10 +329,13 @@ static int pciehp_resume (struct pcie_device *dev)
 
 	/* Check if slot is occupied */
 	pciehp_get_adapter_status(slot, &status);
-	if (status)
-		pciehp_enable_slot(slot);
-	else
+	if (status) {
+		if (list_empty(&pbus->devices))
+			pciehp_enable_slot(slot);
+	} else if (!list_empty(&pbus->devices)) {
 		pciehp_disable_slot(slot);
+	}
+
 	return 0;
 }
 #endif /* PM */
-- 
1.7.1

Re: [PATCH part1 v6 4/7] PCI: Introduce pci_serial_number_changed()

[Gu Zheng]

Hi Yijing,
On 02/12/2014 10:52 AM, Yijing Wang wrote:

> Sometimes OS do not know the physical device swap,
> for instance, some device hotplug during system suspend.
> Interrupt can not deliver to OS in some platform.
> So we can use pci serial number capability to detect this
> issue if device supports serial number.
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> Cc: Paul Bolle <pebolle@tiscali.nl>
> Cc: Oliver Neukum <oneukum@suse.de>
> Cc: Gu Zheng <guz.fnst@cn.fujitsu.com>
> Cc: linux-pci@vger.kernel.org
> ---
>  drivers/pci/pci.c   |   28 ++++++++++++++++++++++++++++
>  include/linux/pci.h |    2 ++
>  2 files changed, 30 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index a8c1c2d..af34057 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2199,6 +2199,34 @@ void pci_dsn_init(struct pci_dev *dev)
>  }
>  
>  /**
> + * pci_serial_number_changed - check the device SN is changed

How about the name “pci_device_changed”? I think it's more readable,
though the judgement is according to SN.

Regards,
Gu

> + * @pdev: the PCI device
> + *
> + * check the device serial number is changed.
> + * if device does not support device serial number,
> + * return false.
> + */
> +bool pci_serial_number_changed(struct pci_dev *pdev)
> +{
> +	u64 old, new;
> +	old = pdev->sn;
> +
> +	if (!pci_is_pcie(pdev))
> +		return false;
> +
> +	new = pci_device_serial_number(pdev->bus,
> +			pdev->devfn);
> +
> +	if (old != new) {
> +		pr_info("%s: Device Serial Number Changed!\n",
> +				pci_name(pdev));
> +		return true;
> +	} else
> +		return false;
> +}
> +EXPORT_SYMBOL(pci_serial_number_changed);
> +
> +/**
>   * pci_configure_ari - enable or disable ARI forwarding
>   * @dev: the PCI device
>   *
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 3631859..d60c0b6 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1025,6 +1025,8 @@ void pci_unlock_rescan_remove(void);
>  ssize_t pci_read_vpd(struct pci_dev *dev, loff_t pos, size_t count, void *buf);
>  ssize_t pci_write_vpd(struct pci_dev *dev, loff_t pos, size_t count, const void *buf);
>  
> +bool pci_serial_number_changed(struct pci_dev *pdev);
> +
>  /* Helper functions for low-level code (drivers/pci/setup-[bus,res].c) */
>  resource_size_t pcibios_retrieve_fw_addr(struct pci_dev *dev, int idx);
>  void pci_bus_assign_resources(const struct pci_bus *bus);

Re: [PATCH part1 v6 5/7] PCI: Add pci_dummy_ops to isolate pci device temporarily

[Gu Zheng]

Hi Yijing,
On 02/12/2014 10:52 AM, Yijing Wang wrote:

> Pci_dummy_ops does nothing when we use it to read/write
> pci_device. So we can isolate pci device by replace its
> bus pci_ops by pci_dummy_ops. This is preparation for
> the later patch.
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
>  drivers/pci/pci.c   |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h |    4 +++
>  2 files changed, 66 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index af34057..283e82e 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2226,6 +2226,68 @@ bool pci_serial_number_changed(struct pci_dev *pdev)
>  }
>  EXPORT_SYMBOL(pci_serial_number_changed);
>  
> +static int pci_dummy_read(struct pci_bus *bus, unsigned int devfn,
> +		int where, int size, u32 *val)
> +{
> +	return PCIBIOS_DEVICE_NOT_FOUND;
> +}
> +
> +static int pci_dummy_write(struct pci_bus *bus, unsigned int devfn,
> +		int reg, int size, u32 val)
> +{
> +	return PCIBIOS_DEVICE_NOT_FOUND;
> +}
> +
> +static struct pci_ops pci_dummy_ops = {
> +	.read = pci_dummy_read,
> +	.write = pci_dummy_write,
> +};
> +
> +static DEFINE_SPINLOCK(pci_freeze_lock);
> +/**
> + * pci_bus_freeze_device - freeze pci bus to access pci device
> + * @bus: the pci bus to freeze
> + *
> + * Replace pci bus ops by pci_dummy_ops, protect system from
> + * accessing pci devices.
> + */
> +void pci_bus_freeze_device(struct pci_bus *bus)
> +{
> +	struct pci_ops *ops;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&pci_freeze_lock, flags);
> +	if (bus->is_frozen)
> +		goto out;
> +	bus->is_frozen = 1;
> +	ops = pci_bus_set_ops(bus, &pci_dummy_ops);
> +	bus->save_ops = ops;
> +out:
> +	spin_unlock_irqrestore(&pci_freeze_lock, flags);
> +}
> +
> +/**
> + * pci_bus_unfreeze_device - unfreeze pci bus to acess pci devices
> + * @bus: the pci bus to unfreeze
> + *
> + * Restore pci bus original ops, so pci bus can access
> + * pci devices normally.
> + */
> +void pci_bus_unfreeze_device(struct pci_bus *bus)
> +{
> +	unsigned long flags;
> +
> +	BUG_ON(!bus->save_ops);
> +	spin_lock_irqsave(&pci_freeze_lock, flags);
> +	if (!bus->is_frozen)
> +		goto out;

It seems that the BUG_ON() should placed here, otherwise it will be
triggered if we try to unfreeze the same bus multiply.

Regards,
Gu

> +	pci_bus_set_ops(bus, bus->save_ops);
> +	bus->save_ops = NULL;
> +	bus->is_frozen = 0;
> +out:
> +	spin_unlock_irqrestore(&pci_freeze_lock, flags);
> +}
> +
>  /**
>   * pci_configure_ari - enable or disable ARI forwarding
>   * @dev: the PCI device
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index d60c0b6..e3dd4ea 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -442,6 +442,7 @@ struct pci_bus {
>  	struct resource busn_res;	/* bus numbers routed to this bus */
>  
>  	struct pci_ops	*ops;		/* configuration access functions */
> +	struct pci_ops	*save_ops; /* save configuration access functions */
>  	struct msi_chip	*msi;		/* MSI controller */
>  	void		*sysdata;	/* hook for sys-specific extension */
>  	struct proc_dir_entry *procdir;	/* directory entry in /proc/bus/pci */
> @@ -460,6 +461,7 @@ struct pci_bus {
>  	struct bin_attribute	*legacy_io; /* legacy I/O for this bus */
>  	struct bin_attribute	*legacy_mem; /* legacy mem */
>  	unsigned int		is_added:1;
> +	unsigned int		is_frozen:1;
>  };
>  
>  #define pci_bus_b(n)	list_entry(n, struct pci_bus, node)
> @@ -1026,6 +1028,8 @@ ssize_t pci_read_vpd(struct pci_dev *dev, loff_t pos, size_t count, void *buf);
>  ssize_t pci_write_vpd(struct pci_dev *dev, loff_t pos, size_t count, const void *buf);
>  
>  bool pci_serial_number_changed(struct pci_dev *pdev);
> +void pci_bus_freeze_device(struct pci_bus *bus);
> +void pci_bus_unfreeze_device(struct pci_bus *bus);
>  
>  /* Helper functions for low-level code (drivers/pci/setup-[bus,res].c) */
>  resource_size_t pcibios_retrieve_fw_addr(struct pci_dev *dev, int idx);

Re: [PATCH part1 v6 4/7] PCI: Introduce pci_serial_number_changed()

[Yijing Wang]

>> + * pci_serial_number_changed - check the device SN is changed
> 
> How about the name “pci_device_changed”? I think it's more readable,
> though the judgement is according to SN.

Ah, it's nice, will update in next version.

Thanks!
Yijing.

> 
> Regards,
> Gu
> 
>> + * @pdev: the PCI device
>> + *
>> + * check the device serial number is changed.
>> + * if device does not support device serial number,
>> + * return false.
>> + */
>> +bool pci_serial_number_changed(struct pci_dev *pdev)
>> +{
>> +	u64 old, new;

Re: [PATCH part1 v6 5/7] PCI: Add pci_dummy_ops to isolate pci device temporarily

[Yijing Wang]

>> +void pci_bus_unfreeze_device(struct pci_bus *bus)
>> +{
>> +	unsigned long flags;
>> +
>> +	BUG_ON(!bus->save_ops);
>> +	spin_lock_irqsave(&pci_freeze_lock, flags);
>> +	if (!bus->is_frozen)
>> +		goto out;
> 
> It seems that the BUG_ON() should placed here, otherwise it will be
> triggered if we try to unfreeze the same bus multiply.
> 

Oh, right, I should move it down, thanks for the good catch.


> 
>> +	pci_bus_set_ops(bus, bus->save_ops);
>> +	bus->save_ops = NULL;
>> +	bus->is_frozen = 0;
>> +out:
>> +	spin_unlock_irqrestore(&pci_freeze_lock, flags);
>> +}
>> +
>>  /**
>>   * pci_configure_ari - enable or disable ARI forwarding
>>   * @dev: the PCI device
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index d60c0b6..e3dd4ea 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -442,6 +442,7 @@ struct pci_bus {
>>  	struct resource busn_res;	/* bus numbers routed to this bus */
>>  
>>  	struct pci_ops	*ops;		/* configuration access functions */
>> +	struct pci_ops	*save_ops; /* save configuration access functions */
>>  	struct msi_chip	*msi;		/* MSI controller */
>>  	void		*sysdata;	/* hook for sys-specific extension */
>>  	struct proc_dir_entry *procdir;	/* directory entry in /proc/bus/pci */
>> @@ -460,6 +461,7 @@ struct pci_bus {
>>  	struct bin_attribute	*legacy_io; /* legacy I/O for this bus */
>>  	struct bin_attribute	*legacy_mem; /* legacy mem */
>>  	unsigned int		is_added:1;
>> +	unsigned int		is_frozen:1;
>>  };
>>  
>>  #define pci_bus_b(n)	list_entry(n, struct pci_bus, node)
>> @@ -1026,6 +1028,8 @@ ssize_t pci_read_vpd(struct pci_dev *dev, loff_t pos, size_t count, void *buf);
>>  ssize_t pci_write_vpd(struct pci_dev *dev, loff_t pos, size_t count, const void *buf);
>>  
>>  bool pci_serial_number_changed(struct pci_dev *pdev);
>> +void pci_bus_freeze_device(struct pci_bus *bus);
>> +void pci_bus_unfreeze_device(struct pci_bus *bus);
>>  
>>  /* Helper functions for low-level code (drivers/pci/setup-[bus,res].c) */
>>  resource_size_t pcibios_retrieve_fw_addr(struct pci_dev *dev, int idx);
> 
> 
> 
> .
> 


-- 
Thanks!
Yijing

Re: [PATCH part1 v6 0/7] Introduce PCIe Device Serial Number capability support

[Yijing Wang]

Hi Bjorn,
   Sorry to bother you, any comment about this series?

Thanks!
Yijing.

On 2014/2/12 10:52, Yijing Wang wrote:
> v5->v6: Add is_frozen flag to protect pci bus from double frozen.
> 
> This series is based on Bjorn's pci-next branch.
> Currently, more and more PCIe devices support PCIe DSN(Device Serial Number)
> capability, we can use this cap to identify device. In some platform,
> when we hotplug PCIe device, no interrupts will be received in system.
> So we can not identify the device whether is changed during suspend.
> PCIe DSN can help to identify this. 
> 
> Legacy PCI device can achieve it by PCI Vital Product Data capability.
> We can use its SN keyword to report the unique number.
> But in my platform, PCI devices which support VPD SN cap report the
> meaningless same string "0123456789". 
> Rework PCI VPD code to support device identification is not an easy work.
> Plan to do it in part2.
> 
> Yijing Wang (7):
>   PCI: rework pci_find_next_ext_capability()
>   PCI: introduce pci_bus_find_ext_capability()
>   PCI: Add support for Device Serial Number capability
>   PCI: Introduce pci_serial_number_changed()
>   PCI: Add pci_dummy_ops to isolate pci device temporarily
>   PCI: Check pci device serial number when scan device
>   PCI: pciehp: Don't enable/disable slot on resume unless status
>     changed
> 
>  drivers/pci/hotplug/pciehp_core.c |   10 ++-
>  drivers/pci/pci.c                 |  169 +++++++++++++++++++++++++++++++++++--
>  drivers/pci/pci.h                 |    2 +-
>  drivers/pci/probe.c               |   15 +++-
>  include/linux/pci.h               |   10 ++-
>  5 files changed, 192 insertions(+), 14 deletions(-)
> 
> 
> 
> .
> 


-- 
Thanks!
Yijing