[PATCH v5 0/4] PCI: Add support for PCI Enhanced Allocation "BARs"

[PATCH v5 0/4] PCI: Add support for PCI Enhanced Allocation "BARs"

[David Daney]

From: David Daney <david.daney@cavium.com>

The original patches are from Sean O. Stalley. I made a few tweaks,
but feel that it is substancially Sean's work, so I am keeping the
patch set version numbering scheme going.

Tested on Cavium ThunderX system with 4 Root Complexes containing 50
devices/bridges provisioned with EA.

Here is Sean's description of the patches:

PCI Enhanced Allocation is a new method of allocating MMIO & IO
resources for PCI devices & bridges. It can be used instead
of the traditional PCI method of using BARs.

EA entries are hardware-initialized to a fixed address.
Unlike BARs, regions described by EA are cannot be moved.
Because of this, only devices which are permanently connected to
the PCI bus can use EA. A removable PCI card must not use EA.

This patchset adds support for using EA entries instead of BARs
on Root Complex Integrated Endpoints.

The Enhanced Allocation ECN is publicly available here:
https://www.pcisig.com/specifications/conventional/ECN_Enhanced_Allocation_23_Oct_2014_Final.pdf


Changes from V1:
	- Use generic PCI resource claim functions (instead of EA-specific functions)
	- Only add support for RCiEPs (instead of all devices).
	- Removed some debugging messages leftover from early testing.

Changes from V2 (By David Daney):
	- Add ea_cap to struct pci_device, to aid in finding the EA capability.
	- Factored EA entity decoding into a separate function.
	- Add functions to find EA entities by BEI or Property.
	- Add handling of EA provisioned bridges.
	- Add handling of EA SRIOV BARs.
	- Try to assign proper resource parent so that SRIOV device creation can occur.

Changes from V3 (By David Daney):
	- Discarded V3 changes and started over fresh based on Sean's V2.
	- Add more support/checking for Entry Properties.
	- Allow EA behind bridges.
	- Rewrite some error messages.
	- Add patch 3/5 to prevent resizing, and better handle
          assigning, of fixed EA resources.
	- Add patch 4/5 to handle EA provisioned SRIOV devices.
	- Add patch 5/5 to handle EA provisioned bridges.

Changes from V4 (By David Daney):
	- Drop patch 5/5 to handle EA provisioned bridges.
	- Drop cases for bridge resources in 2/5.
	- Drop unnecessary fallback resource parent handling in 3/5
	- Small code formatting improvements.

David Daney (2):
  PCI: Handle IORESOURCE_PCI_FIXED when sizing and assigning resources.
  PCI: Handle Enhanced Allocation (EA) capability for SRIOV devices.

Sean O. Stalley (2):
  PCI: Add Enhanced Allocation register entries
  PCI: Add support for Enhanced Allocation devices

 drivers/pci/iov.c             |  11 ++-
 drivers/pci/pci.c             | 189 ++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h             |   1 +
 drivers/pci/probe.c           |   3 +
 drivers/pci/setup-bus.c       |  50 ++++++++++-
 include/uapi/linux/pci_regs.h |  44 +++++++++-
 6 files changed, 292 insertions(+), 6 deletions(-)

-- 
1.9.1

[PATCH v5 1/4] PCI: Add Enhanced Allocation register entries

[David Daney]

From: "Sean O. Stalley" <sean.stalley@intel.com>

Add registers defined in PCI-SIG's Enhanced allocation ECN.

Signed-off-by: Sean O. Stalley <sean.stalley@intel.com>
[david.daney@cavium.com: Added more definitions for PCI_EA_BEI_*]
Signed-off-by: Signed-off-by: David Daney <david.daney@cavium.com>
---
 include/uapi/linux/pci_regs.h | 44 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 413417f..352e418 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -216,7 +216,8 @@
 #define  PCI_CAP_ID_MSIX	0x11	/* MSI-X */
 #define  PCI_CAP_ID_SATA	0x12	/* SATA Data/Index Conf. */
 #define  PCI_CAP_ID_AF		0x13	/* PCI Advanced Features */
-#define  PCI_CAP_ID_MAX		PCI_CAP_ID_AF
+#define  PCI_CAP_ID_EA		0x14	/* PCI Enhanced Allocation */
+#define  PCI_CAP_ID_MAX		PCI_CAP_ID_EA
 #define PCI_CAP_LIST_NEXT	1	/* Next capability in the list */
 #define PCI_CAP_FLAGS		2	/* Capability defined flags (16 bits) */
 #define PCI_CAP_SIZEOF		4
@@ -353,6 +354,47 @@
 #define  PCI_AF_STATUS_TP	0x01
 #define PCI_CAP_AF_SIZEOF	6	/* size of AF registers */
 
+/* PCI Enhanced Allocation registers */
+
+#define PCI_EA_NUM_ENT		2	/* Number of Capability Entries */
+#define PCI_EA_NUM_ENT_MASK	0x3f	/* Num Entries Mask */
+#define PCI_EA_FIRST_ENT	4	/* First EA Entry in List */
+#define PCI_EA_FIRST_ENT_BRIDGE	8	/* First EA Entry for Bridges */
+#define PCI_EA_ES		0x7	/* Entry Size */
+#define PCI_EA_BEI(x)	(((x) >> 4) & 0xf) /* BAR Equivalent Indicator */
+/* 0-5 map to BARs 0-5 respectively */
+#define  PCI_EA_BEI_BAR0	0
+#define  PCI_EA_BEI_BAR5	5
+#define  PCI_EA_BEI_BRIDGE	6	/* Resource behind bridge */
+#define  PCI_EA_BEI_ENI		7	/* Equivalent Not Indicated */
+#define  PCI_EA_BEI_ROM		8	/* Expansion ROM */
+/* 9-14 map to VF BARs 0-5 respectively */
+#define  PCI_EA_BEI_VF_BAR0	9
+#define  PCI_EA_BEI_VF_BAR5	14
+#define  PCI_EA_BEI_RESERVED	15	/* Reserved - Treat like ENI */
+
+#define PCI_EA_PP(x)	(((x) >>  8) & 0xff)	/* Primary Properties */
+#define PCI_EA_SP(x)	(((x) >> 16) & 0xff)	/* Secondary Properties */
+#define  PCI_EA_P_MEM			0x00	/* Non-Prefetch Memory */
+#define  PCI_EA_P_MEM_PREFETCH		0x01	/* Prefetchable Memory */
+#define  PCI_EA_P_IO			0x02	/* I/O Space */
+#define  PCI_EA_P_VIRT_MEM_PREFETCH	0x03	/* VF Prefetchable Memory */
+#define  PCI_EA_P_VIRT_MEM		0x04	/* VF Non-Prefetch Memory */
+#define  PCI_EA_P_BRIDGE_MEM		0x05	/* Bridge Non-Prefetch Memory */
+#define  PCI_EA_P_BRIDGE_MEM_PREFETCH	0x06	/* Bridge Prefetchable Memory */
+#define  PCI_EA_P_BRIDGE_IO		0x07	/* Bridge I/O Space */
+/* 0x08-0xfc reserved */
+#define  PCI_EA_P_MEM_RESERVED		0xfd	/* Reserved Memory */
+#define  PCI_EA_P_IO_RESERVED		0xfe	/* Reserved I/O Space */
+#define  PCI_EA_P_UNAVAILABLE		0xff	/* Entry Unavailable */
+#define PCI_EA_WRITEABLE	BIT(30) /* Writable, 1 = RW, 0 = HwInit */
+#define PCI_EA_ENABLE		BIT(31) /* Enable for this entry */
+#define PCI_EA_BASE		4	/* Base Address Offset */
+#define PCI_EA_MAX_OFFSET	8	/* MaxOffset (resource length) */
+/* bit 0 is reserved */
+#define PCI_EA_IS_64		BIT(1)	/* 64-bit field flag */
+#define PCI_EA_FIELD_MASK	0xfffffffc	/* For Base & Max Offset */
+
 /* PCI-X registers (Type 0 (non-bridge) devices) */
 
 #define PCI_X_CMD		2	/* Modes & Features */
-- 
1.9.1

[PATCH v5 2/4] PCI: Add support for Enhanced Allocation devices

[David Daney]

From: "Sean O. Stalley" <sean.stalley@intel.com>

Add support for devices using Enhanced Allocation entries instead of BARs.
This patch allows the kernel to parse the EA Extended Capability structure
in PCI configspace and claim the BAR-equivalent resources.

Signed-off-by: Sean O. Stalley <sean.stalley@intel.com>
[david.daney@cavium.com: Add more support/checking for Entry Properties,
allow EA behind bridges, rewrite some error messages.]
Signed-off-by: David Daney <david.daney@cavium.com>
---
 drivers/pci/pci.c   | 182 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h   |   1 +
 drivers/pci/probe.c |   3 +
 3 files changed, 186 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 6a9a111..30a90d1 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2148,6 +2148,188 @@ void pci_pm_init(struct pci_dev *dev)
 	}
 }
 
+static unsigned long pci_ea_set_flags(struct pci_dev *dev, u8 prop)
+{
+	unsigned long flags = IORESOURCE_PCI_FIXED;
+
+	switch (prop) {
+	case PCI_EA_P_MEM:
+	case PCI_EA_P_VIRT_MEM:
+		flags |= IORESOURCE_MEM;
+		break;
+	case PCI_EA_P_MEM_PREFETCH:
+	case PCI_EA_P_VIRT_MEM_PREFETCH:
+		flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH;
+		break;
+	case PCI_EA_P_IO:
+		flags |= IORESOURCE_IO;
+		break;
+	default:
+		return 0;
+	}
+
+	return flags;
+}
+
+static struct resource *pci_ea_get_resource(struct pci_dev *dev, u8 bei,
+					    u8 prop)
+{
+	if (bei <= PCI_EA_BEI_BAR5 && prop <= PCI_EA_P_IO)
+		return &dev->resource[bei];
+	else if (bei == PCI_EA_BEI_ROM)
+		return &dev->resource[PCI_ROM_RESOURCE];
+	else
+		return NULL;
+}
+
+/* Read an Enhanced Allocation (EA) entry */
+static int pci_ea_read(struct pci_dev *dev, int offset)
+{
+	struct resource *res;
+	int ent_offset = offset;
+	int ent_size;
+	resource_size_t start;
+	resource_size_t end;
+	unsigned long flags;
+	u32 dw0;
+	u32 base;
+	u32 max_offset;
+	u8 prop;
+	bool support_64 = (sizeof(resource_size_t) >= 8);
+
+	pci_read_config_dword(dev, ent_offset, &dw0);
+	ent_offset += 4;
+
+	/* Entry size field indicates DWORDs after 1st */
+	ent_size = ((dw0 & PCI_EA_ES) + 1) << 2;
+
+	if (!(dw0 & PCI_EA_ENABLE)) /* Entry not enabled */
+		goto out;
+
+	prop = PCI_EA_PP(dw0);
+	/*
+	 * If the Property is in the reserved range, try the Secondary
+	 * Property instead.
+	 */
+	if (prop > PCI_EA_P_BRIDGE_IO && prop < PCI_EA_P_MEM_RESERVED)
+		prop = PCI_EA_SP(dw0);
+	if (prop > PCI_EA_P_BRIDGE_IO)
+		goto out;
+
+	res = pci_ea_get_resource(dev, PCI_EA_BEI(dw0), prop);
+	if (!res) {
+		dev_err(&dev->dev, "Unsupported EA entry BEI: %u\n",
+			PCI_EA_BEI(dw0));
+		goto out;
+	}
+
+	flags = pci_ea_set_flags(dev, prop);
+	if (!flags) {
+		dev_err(&dev->dev, "Unsupported EA properties: %u\n", prop);
+		goto out;
+	}
+
+	/* Read Base */
+	pci_read_config_dword(dev, ent_offset, &base);
+	start = (base & PCI_EA_FIELD_MASK);
+	ent_offset += 4;
+
+	/* Read MaxOffset */
+	pci_read_config_dword(dev, ent_offset, &max_offset);
+	ent_offset += 4;
+
+	/* Read Base MSBs (if 64-bit entry) */
+	if (base & PCI_EA_IS_64) {
+		u32 base_upper;
+
+		pci_read_config_dword(dev, ent_offset, &base_upper);
+		ent_offset += 4;
+
+		flags |= IORESOURCE_MEM_64;
+
+		/* entry starts above 32-bit boundary, can't use */
+		if (!support_64 && base_upper)
+			goto out;
+
+		if (support_64)
+			start |= ((u64)base_upper << 32);
+	}
+
+	dev_dbg(&dev->dev,
+		"EA (%u,%u) start = %pa\n", PCI_EA_BEI(dw0), prop, &start);
+
+	end = start + (max_offset | 0x03);
+
+	/* Read MaxOffset MSBs (if 64-bit entry) */
+	if (max_offset & PCI_EA_IS_64) {
+		u32 max_offset_upper;
+
+		pci_read_config_dword(dev, ent_offset, &max_offset_upper);
+		ent_offset += 4;
+
+		flags |= IORESOURCE_MEM_64;
+
+		/* entry too big, can't use */
+		if (!support_64 && max_offset_upper)
+			goto out;
+
+		if (support_64)
+			end += ((u64)max_offset_upper << 32);
+	}
+
+	dev_dbg(&dev->dev,
+		"EA (%u,%u) end = %pa\n", PCI_EA_BEI(dw0), prop, &end);
+
+	if (end < start) {
+		dev_err(&dev->dev, "EA Entry crosses address boundary\n");
+		goto out;
+	}
+
+	if (ent_size != ent_offset - offset) {
+		dev_err(&dev->dev,
+			"EA Entry Size (%d) does not match length read (%d)\n",
+			ent_size, ent_offset - offset);
+		goto out;
+	}
+
+	res->name = pci_name(dev);
+	res->start = start;
+	res->end = end;
+	res->flags = flags;
+
+out:
+	return offset + ent_size;
+}
+
+/* Enhanced Allocation Initalization */
+void pci_ea_init(struct pci_dev *dev)
+{
+	int ea;
+	u8 num_ent;
+	int offset;
+	int i;
+
+	/* find PCI EA capability in list */
+	ea = pci_find_capability(dev, PCI_CAP_ID_EA);
+	if (!ea)
+		return;
+
+	/* determine the number of entries */
+	pci_bus_read_config_byte(dev->bus, dev->devfn, ea + PCI_EA_NUM_ENT,
+					&num_ent);
+	num_ent &= PCI_EA_NUM_ENT_MASK;
+
+	offset = ea + PCI_EA_FIRST_ENT;
+
+	/* Skip DWORD 2 for type 1 functions */
+	if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE)
+		offset += 4;
+
+	/* parse each EA entry */
+	for (i = 0; i < num_ent; ++i)
+		offset = pci_ea_read(dev, offset);
+}
+
 static void pci_add_saved_cap(struct pci_dev *pci_dev,
 	struct pci_cap_saved_state *new_cap)
 {
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 24ba9dc..a160733 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -78,6 +78,7 @@ bool pci_dev_keep_suspended(struct pci_dev *dev);
 void pci_config_pm_runtime_get(struct pci_dev *dev);
 void pci_config_pm_runtime_put(struct pci_dev *dev);
 void pci_pm_init(struct pci_dev *dev);
+void pci_ea_init(struct pci_dev *dev);
 void pci_allocate_cap_save_buffers(struct pci_dev *dev);
 void pci_free_cap_save_buffers(struct pci_dev *dev);
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8361d27..4c4af78 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1597,6 +1597,9 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
 
 static void pci_init_capabilities(struct pci_dev *dev)
 {
+	/* Enhanced Allocation */
+	pci_ea_init(dev);
+
 	/* MSI/MSI-X list */
 	pci_msi_init_pci_dev(dev);
 
-- 
1.9.1

[PATCH v5 3/4] PCI: Handle IORESOURCE_PCI_FIXED when sizing and assigning resources.

[David Daney]

From: David Daney <david.daney@cavium.com>

The new Enhanced Allocation (EA) capability support creates resources
with the IORESOURCE_PCI_FIXED set.  This causes a couple of problems:

1) Since these resources cannot be relocated or resized, their
   alignment is not really defined, and it is therefore not specified.
   This causes a problem in pbus_size_mem() where resources with
   unspecified alignment are disabled.

2) During resource assignment in pci_bus_assign_resources(),
   IORESOURCE_PCI_FIXED resources are not given a parent.  This, in
   turn, causes pci_enable_resources() to fail with a "not claimed"
   error.

So, in pbus_size_mem() skip IORESOURCE_PCI_FIXED resources, instead of
disabling them.

In __pci_bus_assign_resources(), for IORESOURCE_PCI_FIXED resources,
try to request the resource from a parent bus.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 drivers/pci/setup-bus.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 47 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 508cc56..7239a2c 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1037,9 +1037,10 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			struct resource *r = &dev->resource[i];
 			resource_size_t r_size;
 
-			if (r->parent || ((r->flags & mask) != type &&
-					  (r->flags & mask) != type2 &&
-					  (r->flags & mask) != type3))
+			if (r->parent || (r->flags | IORESOURCE_PCI_FIXED) ||
+			    ((r->flags & mask) != type &&
+			     (r->flags & mask) != type2 &&
+			     (r->flags & mask) != type3))
 				continue;
 			r_size = resource_size(r);
 #ifdef CONFIG_PCI_IOV
@@ -1340,6 +1341,47 @@ void pci_bus_size_bridges(struct pci_bus *bus)
 }
 EXPORT_SYMBOL(pci_bus_size_bridges);
 
+static void assign_fixed_resource_on_bus(struct pci_bus *b, struct resource *r)
+{
+	int i;
+	struct resource *parent_r;
+	unsigned long mask = IORESOURCE_IO | IORESOURCE_MEM |
+			     IORESOURCE_PREFETCH;
+
+	pci_bus_for_each_resource(b, parent_r, i) {
+		if (!parent_r)
+			continue;
+
+		if ((r->flags & mask) == (parent_r->flags & mask) &&
+		    resource_contains(parent_r, r))
+			request_resource(parent_r, r);
+	}
+}
+
+/*
+ * Try to assign any resources marked as IORESOURCE_PCI_FIXED, as they
+ * are skipped by pbus_assign_resources_sorted().
+ */
+static void pdev_assign_fixed_resources(struct pci_dev *dev)
+{
+	int i;
+
+	for (i = 0; i <  PCI_NUM_RESOURCES; i++) {
+		struct pci_bus *b;
+		struct resource *r = &dev->resource[i];
+
+		if (r->parent || !(r->flags & IORESOURCE_PCI_FIXED) ||
+		    !(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
+			continue;
+
+		b = dev->bus;
+		while (b && !r->parent) {
+			assign_fixed_resource_on_bus(b, r);
+			b = b->parent;
+		}
+	}
+}
+
 void __pci_bus_assign_resources(const struct pci_bus *bus,
 				struct list_head *realloc_head,
 				struct list_head *fail_head)
@@ -1350,6 +1392,8 @@ void __pci_bus_assign_resources(const struct pci_bus *bus,
 	pbus_assign_resources_sorted(bus, realloc_head, fail_head);
 
 	list_for_each_entry(dev, &bus->devices, bus_list) {
+		pdev_assign_fixed_resources(dev);
+
 		b = dev->subordinate;
 		if (!b)
 			continue;
-- 
1.9.1

[PATCH v5 4/4] PCI: Handle Enhanced Allocation (EA) capability for SRIOV devices.

[David Daney]

From: David Daney <david.daney@cavium.com>

SRIOV BARs can be specified via EA entries.  Extend the EA parser to
extract the SRIOV BAR resources, and modify sriov_init() to use
resources previously obtained via EA.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 drivers/pci/iov.c | 11 +++++++++--
 drivers/pci/pci.c |  7 +++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index ee0ebff..c789e68 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -436,8 +436,15 @@ found:
 	nres = 0;
 	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
 		res = &dev->resource[i + PCI_IOV_RESOURCES];
-		bar64 = __pci_read_base(dev, pci_bar_unknown, res,
-					pos + PCI_SRIOV_BAR + i * 4);
+		/*
+		 * If it is already FIXED, don't change it, something
+		 * (perhaps EA or header fixups) wants it this way.
+		 */
+		if (res->flags & IORESOURCE_PCI_FIXED)
+			bar64 = (res->flags & IORESOURCE_MEM_64) ? 1 : 0;
+		else
+			bar64 = __pci_read_base(dev, pci_bar_unknown, res,
+						pos + PCI_SRIOV_BAR + i * 4);
 		if (!res->flags)
 			continue;
 		if (resource_size(res) & (PAGE_SIZE - 1)) {
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 30a90d1..6c41585 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2176,6 +2176,13 @@ static struct resource *pci_ea_get_resource(struct pci_dev *dev, u8 bei,
 {
 	if (bei <= PCI_EA_BEI_BAR5 && prop <= PCI_EA_P_IO)
 		return &dev->resource[bei];
+#ifdef CONFIG_PCI_IOV
+	else if (bei >= PCI_EA_BEI_VF_BAR0 && bei <= PCI_EA_BEI_VF_BAR5 &&
+		 (prop == PCI_EA_P_VIRT_MEM ||
+		  prop == PCI_EA_P_VIRT_MEM_PREFETCH))
+		return &dev->resource[PCI_IOV_RESOURCES +
+				      bei - PCI_EA_BEI_VF_BAR0];
+#endif
 	else if (bei == PCI_EA_BEI_ROM)
 		return &dev->resource[PCI_ROM_RESOURCE];
 	else
-- 
1.9.1

RE: [PATCH v5 0/4] PCI: Add support for PCI Enhanced Allocation "BARs"

[Stalley, Sean]

[PATCH 3/4 & 4/4] Acked-by: Sean O. Stalley <sean.stalley@intel.com>

I won't be able to test it out until next week, but I like how it looks :)

Thanks Again,
Sean

> -----Original Message-----
> From: David Daney [mailto:ddaney.cavm@gmail.com]
> Sent: Tuesday, October 06, 2015 4:51 PM
> To: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; Bjorn Helgaas;
> Michael S. Tsirkin; Rafał Miłecki; linux-api@vger.kernel.org; Stalley, Sean;
> yinghai@kernel.org; rajatxjain@gmail.com; gong.chen@linux.intel.com
> Cc: David Daney
> Subject: [PATCH v5 0/4] PCI: Add support for PCI Enhanced Allocation
> "BARs"
> 
> From: David Daney <david.daney@cavium.com>
> 
> The original patches are from Sean O. Stalley. I made a few tweaks, but feel
> that it is substancially Sean's work, so I am keeping the patch set version
> numbering scheme going.
> 
> Tested on Cavium ThunderX system with 4 Root Complexes containing 50
> devices/bridges provisioned with EA.
> 
> Here is Sean's description of the patches:
> 
> PCI Enhanced Allocation is a new method of allocating MMIO & IO
> resources for PCI devices & bridges. It can be used instead of the traditional
> PCI method of using BARs.
> 
> EA entries are hardware-initialized to a fixed address.
> Unlike BARs, regions described by EA are cannot be moved.
> Because of this, only devices which are permanently connected to the PCI
> bus can use EA. A removable PCI card must not use EA.
> 
> This patchset adds support for using EA entries instead of BARs on Root
> Complex Integrated Endpoints.
> 
> The Enhanced Allocation ECN is publicly available here:
> https://www.pcisig.com/specifications/conventional/ECN_Enhanced_Alloca
> tion_23_Oct_2014_Final.pdf
> 
> 
> Changes from V1:
> 	- Use generic PCI resource claim functions (instead of EA-specific
> functions)
> 	- Only add support for RCiEPs (instead of all devices).
> 	- Removed some debugging messages leftover from early testing.
> 
> Changes from V2 (By David Daney):
> 	- Add ea_cap to struct pci_device, to aid in finding the EA capability.
> 	- Factored EA entity decoding into a separate function.
> 	- Add functions to find EA entities by BEI or Property.
> 	- Add handling of EA provisioned bridges.
> 	- Add handling of EA SRIOV BARs.
> 	- Try to assign proper resource parent so that SRIOV device creation
> can occur.
> 
> Changes from V3 (By David Daney):
> 	- Discarded V3 changes and started over fresh based on Sean's V2.
> 	- Add more support/checking for Entry Properties.
> 	- Allow EA behind bridges.
> 	- Rewrite some error messages.
> 	- Add patch 3/5 to prevent resizing, and better handle
>           assigning, of fixed EA resources.
> 	- Add patch 4/5 to handle EA provisioned SRIOV devices.
> 	- Add patch 5/5 to handle EA provisioned bridges.
> 
> Changes from V4 (By David Daney):
> 	- Drop patch 5/5 to handle EA provisioned bridges.
> 	- Drop cases for bridge resources in 2/5.
> 	- Drop unnecessary fallback resource parent handling in 3/5
> 	- Small code formatting improvements.
> 
> David Daney (2):
>   PCI: Handle IORESOURCE_PCI_FIXED when sizing and assigning resources.
>   PCI: Handle Enhanced Allocation (EA) capability for SRIOV devices.
> 
> Sean O. Stalley (2):
>   PCI: Add Enhanced Allocation register entries
>   PCI: Add support for Enhanced Allocation devices
> 
>  drivers/pci/iov.c             |  11 ++-
>  drivers/pci/pci.c             | 189
> ++++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/pci.h             |   1 +
>  drivers/pci/probe.c           |   3 +
>  drivers/pci/setup-bus.c       |  50 ++++++++++-
>  include/uapi/linux/pci_regs.h |  44 +++++++++-
>  6 files changed, 292 insertions(+), 6 deletions(-)
> 
> --
> 1.9.1

Re: [PATCH v5 0/4] PCI: Add support for PCI Enhanced Allocation "BARs"

[Sean O. Stalley]

Signed-off-by: Sean O. Stalley <sean.stalley@intel.com>

I tested it out with the QEMU EA Patches here:
	[https://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg00348.html]

Also, I found 1 trivial typo in the commit message of PATCH 1/4:
	"Signed-off-by: Signed-off-by: David Daney <david.daney@cavium.com>"

-Sean

On Wed, Oct 07, 2015 at 06:44:52AM -0700, Stalley, Sean wrote:
> [PATCH 3/4 & 4/4] Acked-by: Sean O. Stalley <sean.stalley@intel.com>
> 
> I won't be able to test it out until next week, but I like how it looks :)
> 
> Thanks Again,
> Sean
> 
> > -----Original Message-----
> > From: David Daney [mailto:ddaney.cavm@gmail.com]
> > Sent: Tuesday, October 06, 2015 4:51 PM
> > To: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; Bjorn Helgaas;
> > Michael S. Tsirkin; Rafał Miłecki; linux-api@vger.kernel.org; Stalley, Sean;
> > yinghai@kernel.org; rajatxjain@gmail.com; gong.chen@linux.intel.com
> > Cc: David Daney
> > Subject: [PATCH v5 0/4] PCI: Add support for PCI Enhanced Allocation
> > "BARs"
> > 
> > From: David Daney <david.daney@cavium.com>
> > 
> > The original patches are from Sean O. Stalley. I made a few tweaks, but feel
> > that it is substancially Sean's work, so I am keeping the patch set version
> > numbering scheme going.
> > 
> > Tested on Cavium ThunderX system with 4 Root Complexes containing 50
> > devices/bridges provisioned with EA.
> > 
> > Here is Sean's description of the patches:
> > 
> > PCI Enhanced Allocation is a new method of allocating MMIO & IO
> > resources for PCI devices & bridges. It can be used instead of the traditional
> > PCI method of using BARs.
> > 
> > EA entries are hardware-initialized to a fixed address.
> > Unlike BARs, regions described by EA are cannot be moved.
> > Because of this, only devices which are permanently connected to the PCI
> > bus can use EA. A removable PCI card must not use EA.
> > 
> > This patchset adds support for using EA entries instead of BARs on Root
> > Complex Integrated Endpoints.
> > 
> > The Enhanced Allocation ECN is publicly available here:
> > https://www.pcisig.com/specifications/conventional/ECN_Enhanced_Alloca
> > tion_23_Oct_2014_Final.pdf
> > 
> > 
> > Changes from V1:
> > 	- Use generic PCI resource claim functions (instead of EA-specific
> > functions)
> > 	- Only add support for RCiEPs (instead of all devices).
> > 	- Removed some debugging messages leftover from early testing.
> > 
> > Changes from V2 (By David Daney):
> > 	- Add ea_cap to struct pci_device, to aid in finding the EA capability.
> > 	- Factored EA entity decoding into a separate function.
> > 	- Add functions to find EA entities by BEI or Property.
> > 	- Add handling of EA provisioned bridges.
> > 	- Add handling of EA SRIOV BARs.
> > 	- Try to assign proper resource parent so that SRIOV device creation
> > can occur.
> > 
> > Changes from V3 (By David Daney):
> > 	- Discarded V3 changes and started over fresh based on Sean's V2.
> > 	- Add more support/checking for Entry Properties.
> > 	- Allow EA behind bridges.
> > 	- Rewrite some error messages.
> > 	- Add patch 3/5 to prevent resizing, and better handle
> >           assigning, of fixed EA resources.
> > 	- Add patch 4/5 to handle EA provisioned SRIOV devices.
> > 	- Add patch 5/5 to handle EA provisioned bridges.
> > 
> > Changes from V4 (By David Daney):
> > 	- Drop patch 5/5 to handle EA provisioned bridges.
> > 	- Drop cases for bridge resources in 2/5.
> > 	- Drop unnecessary fallback resource parent handling in 3/5
> > 	- Small code formatting improvements.
> > 
> > David Daney (2):
> >   PCI: Handle IORESOURCE_PCI_FIXED when sizing and assigning resources.
> >   PCI: Handle Enhanced Allocation (EA) capability for SRIOV devices.
> > 
> > Sean O. Stalley (2):
> >   PCI: Add Enhanced Allocation register entries
> >   PCI: Add support for Enhanced Allocation devices
> > 
> >  drivers/pci/iov.c             |  11 ++-
> >  drivers/pci/pci.c             | 189
> > ++++++++++++++++++++++++++++++++++++++++++
> >  drivers/pci/pci.h             |   1 +
> >  drivers/pci/probe.c           |   3 +
> >  drivers/pci/setup-bus.c       |  50 ++++++++++-
> >  include/uapi/linux/pci_regs.h |  44 +++++++++-
> >  6 files changed, 292 insertions(+), 6 deletions(-)
> > 
> > --
> > 1.9.1
> 

Re: [PATCH v5 0/4] PCI: Add support for PCI Enhanced Allocation "BARs"

[Sean O. Stalley]

Signed-off-by: Sean O. Stalley <sean.stalley-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

I tested it out with the QEMU EA Patches here:
	[https://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg00348.html]

Also, I found 1 trivial typo in the commit message of PATCH 1/4:
	"Signed-off-by: Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>"

-Sean

On Wed, Oct 07, 2015 at 06:44:52AM -0700, Stalley, Sean wrote:
> [PATCH 3/4 & 4/4] Acked-by: Sean O. Stalley <sean.stalley-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> I won't be able to test it out until next week, but I like how it looks :)
> 
> Thanks Again,
> Sean
> 
> > -----Original Message-----
> > From: David Daney [mailto:ddaney.cavm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > Sent: Tuesday, October 06, 2015 4:51 PM
> > To: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Bjorn Helgaas;
> > Michael S. Tsirkin; Rafał Miłecki; linux-api-u79uwXL29TasMV2rI37PzA@public.gmane.orgorg; Stalley, Sean;
> > yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; rajatxjain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; gong.chen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org
> > Cc: David Daney
> > Subject: [PATCH v5 0/4] PCI: Add support for PCI Enhanced Allocation
> > "BARs"
> > 
> > From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> > 
> > The original patches are from Sean O. Stalley. I made a few tweaks, but feel
> > that it is substancially Sean's work, so I am keeping the patch set version
> > numbering scheme going.
> > 
> > Tested on Cavium ThunderX system with 4 Root Complexes containing 50
> > devices/bridges provisioned with EA.
> > 
> > Here is Sean's description of the patches:
> > 
> > PCI Enhanced Allocation is a new method of allocating MMIO & IO
> > resources for PCI devices & bridges. It can be used instead of the traditional
> > PCI method of using BARs.
> > 
> > EA entries are hardware-initialized to a fixed address.
> > Unlike BARs, regions described by EA are cannot be moved.
> > Because of this, only devices which are permanently connected to the PCI
> > bus can use EA. A removable PCI card must not use EA.
> > 
> > This patchset adds support for using EA entries instead of BARs on Root
> > Complex Integrated Endpoints.
> > 
> > The Enhanced Allocation ECN is publicly available here:
> > https://www.pcisig.com/specifications/conventional/ECN_Enhanced_Alloca
> > tion_23_Oct_2014_Final.pdf
> > 
> > 
> > Changes from V1:
> > 	- Use generic PCI resource claim functions (instead of EA-specific
> > functions)
> > 	- Only add support for RCiEPs (instead of all devices).
> > 	- Removed some debugging messages leftover from early testing.
> > 
> > Changes from V2 (By David Daney):
> > 	- Add ea_cap to struct pci_device, to aid in finding the EA capability.
> > 	- Factored EA entity decoding into a separate function.
> > 	- Add functions to find EA entities by BEI or Property.
> > 	- Add handling of EA provisioned bridges.
> > 	- Add handling of EA SRIOV BARs.
> > 	- Try to assign proper resource parent so that SRIOV device creation
> > can occur.
> > 
> > Changes from V3 (By David Daney):
> > 	- Discarded V3 changes and started over fresh based on Sean's V2.
> > 	- Add more support/checking for Entry Properties.
> > 	- Allow EA behind bridges.
> > 	- Rewrite some error messages.
> > 	- Add patch 3/5 to prevent resizing, and better handle
> >           assigning, of fixed EA resources.
> > 	- Add patch 4/5 to handle EA provisioned SRIOV devices.
> > 	- Add patch 5/5 to handle EA provisioned bridges.
> > 
> > Changes from V4 (By David Daney):
> > 	- Drop patch 5/5 to handle EA provisioned bridges.
> > 	- Drop cases for bridge resources in 2/5.
> > 	- Drop unnecessary fallback resource parent handling in 3/5
> > 	- Small code formatting improvements.
> > 
> > David Daney (2):
> >   PCI: Handle IORESOURCE_PCI_FIXED when sizing and assigning resources.
> >   PCI: Handle Enhanced Allocation (EA) capability for SRIOV devices.
> > 
> > Sean O. Stalley (2):
> >   PCI: Add Enhanced Allocation register entries
> >   PCI: Add support for Enhanced Allocation devices
> > 
> >  drivers/pci/iov.c             |  11 ++-
> >  drivers/pci/pci.c             | 189
> > ++++++++++++++++++++++++++++++++++++++++++
> >  drivers/pci/pci.h             |   1 +
> >  drivers/pci/probe.c           |   3 +
> >  drivers/pci/setup-bus.c       |  50 ++++++++++-
> >  include/uapi/linux/pci_regs.h |  44 +++++++++-
> >  6 files changed, 292 insertions(+), 6 deletions(-)
> > 
> > --
> > 1.9.1
> 

Re: [PATCH v5 0/4] PCI: Add support for PCI Enhanced Allocation "BARs"

[David Daney]

On 10/14/2015 09:17 AM, Sean O. Stalley wrote:
> Signed-off-by: Sean O. Stalley <sean.stalley@intel.com>
>

Thanks a lot Sean.

I think you cannot SOB if the patches are not flowing through you (as 
may be the case for my two additions).  Perhaps a Tested-by: or 
Acked-by: would be more appropriate.

> I tested it out with the QEMU EA Patches here:
> 	[https://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg00348.html]
>
> Also, I found 1 trivial typo in the commit message of PATCH 1/4:
> 	"Signed-off-by: Signed-off-by: David Daney <david.daney@cavium.com>"

Aargh!  I need to be more careful.

In any case, what should be the next course of action?

   A) I receive Tested-by/Acked-by from Sean, and resend the four patches?

   B) Bjorn takes these as is, but fixes the headers as needed.

Bjorn, what do you think?

Thanks,
David Daney


>
> -Sean
>
> On Wed, Oct 07, 2015 at 06:44:52AM -0700, Stalley, Sean wrote:
>> [PATCH 3/4 & 4/4] Acked-by: Sean O. Stalley <sean.stalley@intel.com>
>>
>> I won't be able to test it out until next week, but I like how it looks :)
>>
>> Thanks Again,
>> Sean
>>
>>> -----Original Message-----
>>> From: David Daney [mailto:ddaney.cavm@gmail.com]
>>> Sent: Tuesday, October 06, 2015 4:51 PM
>>> To: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; Bjorn Helgaas;
>>> Michael S. Tsirkin; Rafał Miłecki; linux-api@vger.kernel.org; Stalley, Sean;
>>> yinghai@kernel.org; rajatxjain@gmail.com; gong.chen@linux.intel.com
>>> Cc: David Daney
>>> Subject: [PATCH v5 0/4] PCI: Add support for PCI Enhanced Allocation
>>> "BARs"
>>>
>>> From: David Daney <david.daney@cavium.com>
>>>
>>> The original patches are from Sean O. Stalley. I made a few tweaks, but feel
>>> that it is substancially Sean's work, so I am keeping the patch set version
>>> numbering scheme going.
>>>
>>> Tested on Cavium ThunderX system with 4 Root Complexes containing 50
>>> devices/bridges provisioned with EA.
>>>
>>> Here is Sean's description of the patches:
>>>
>>> PCI Enhanced Allocation is a new method of allocating MMIO & IO
>>> resources for PCI devices & bridges. It can be used instead of the traditional
>>> PCI method of using BARs.
>>>
>>> EA entries are hardware-initialized to a fixed address.
>>> Unlike BARs, regions described by EA are cannot be moved.
>>> Because of this, only devices which are permanently connected to the PCI
>>> bus can use EA. A removable PCI card must not use EA.
>>>
>>> This patchset adds support for using EA entries instead of BARs on Root
>>> Complex Integrated Endpoints.
>>>
>>> The Enhanced Allocation ECN is publicly available here:
>>> https://www.pcisig.com/specifications/conventional/ECN_Enhanced_Alloca
>>> tion_23_Oct_2014_Final.pdf
>>>
>>>
>>> Changes from V1:
>>> 	- Use generic PCI resource claim functions (instead of EA-specific
>>> functions)
>>> 	- Only add support for RCiEPs (instead of all devices).
>>> 	- Removed some debugging messages leftover from early testing.
>>>
>>> Changes from V2 (By David Daney):
>>> 	- Add ea_cap to struct pci_device, to aid in finding the EA capability.
>>> 	- Factored EA entity decoding into a separate function.
>>> 	- Add functions to find EA entities by BEI or Property.
>>> 	- Add handling of EA provisioned bridges.
>>> 	- Add handling of EA SRIOV BARs.
>>> 	- Try to assign proper resource parent so that SRIOV device creation
>>> can occur.
>>>
>>> Changes from V3 (By David Daney):
>>> 	- Discarded V3 changes and started over fresh based on Sean's V2.
>>> 	- Add more support/checking for Entry Properties.
>>> 	- Allow EA behind bridges.
>>> 	- Rewrite some error messages.
>>> 	- Add patch 3/5 to prevent resizing, and better handle
>>>            assigning, of fixed EA resources.
>>> 	- Add patch 4/5 to handle EA provisioned SRIOV devices.
>>> 	- Add patch 5/5 to handle EA provisioned bridges.
>>>
>>> Changes from V4 (By David Daney):
>>> 	- Drop patch 5/5 to handle EA provisioned bridges.
>>> 	- Drop cases for bridge resources in 2/5.
>>> 	- Drop unnecessary fallback resource parent handling in 3/5
>>> 	- Small code formatting improvements.
>>>
>>> David Daney (2):
>>>    PCI: Handle IORESOURCE_PCI_FIXED when sizing and assigning resources.
>>>    PCI: Handle Enhanced Allocation (EA) capability for SRIOV devices.
>>>
>>> Sean O. Stalley (2):
>>>    PCI: Add Enhanced Allocation register entries
>>>    PCI: Add support for Enhanced Allocation devices
>>>
>>>   drivers/pci/iov.c             |  11 ++-
>>>   drivers/pci/pci.c             | 189
>>> ++++++++++++++++++++++++++++++++++++++++++
>>>   drivers/pci/pci.h             |   1 +
>>>   drivers/pci/probe.c           |   3 +
>>>   drivers/pci/setup-bus.c       |  50 ++++++++++-
>>>   include/uapi/linux/pci_regs.h |  44 +++++++++-
>>>   6 files changed, 292 insertions(+), 6 deletions(-)
>>>
>>> --
>>> 1.9.1
>>

Re: [PATCH v5 0/4] PCI: Add support for PCI Enhanced Allocation "BARs"

[David Daney]

On 10/14/2015 09:17 AM, Sean O. Stalley wrote:
> Signed-off-by: Sean O. Stalley <sean.stalley-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>

Thanks a lot Sean.

I think you cannot SOB if the patches are not flowing through you (as 
may be the case for my two additions).  Perhaps a Tested-by: or 
Acked-by: would be more appropriate.

> I tested it out with the QEMU EA Patches here:
> 	[https://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg00348.html]
>
> Also, I found 1 trivial typo in the commit message of PATCH 1/4:
> 	"Signed-off-by: Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>"

Aargh!  I need to be more careful.

In any case, what should be the next course of action?

   A) I receive Tested-by/Acked-by from Sean, and resend the four patches?

   B) Bjorn takes these as is, but fixes the headers as needed.

Bjorn, what do you think?

Thanks,
David Daney


>
> -Sean
>
> On Wed, Oct 07, 2015 at 06:44:52AM -0700, Stalley, Sean wrote:
>> [PATCH 3/4 & 4/4] Acked-by: Sean O. Stalley <sean.stalley-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>>
>> I won't be able to test it out until next week, but I like how it looks :)
>>
>> Thanks Again,
>> Sean
>>
>>> -----Original Message-----
>>> From: David Daney [mailto:ddaney.cavm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
>>> Sent: Tuesday, October 06, 2015 4:51 PM
>>> To: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Bjorn Helgaas;
>>> Michael S. Tsirkin; Rafał Miłecki; linux-api-u79uwXL29TasMV2rI37PzA@public.gmane.orgorg; Stalley, Sean;
>>> yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; rajatxjain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; gong.chen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org
>>> Cc: David Daney
>>> Subject: [PATCH v5 0/4] PCI: Add support for PCI Enhanced Allocation
>>> "BARs"
>>>
>>> From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>>>
>>> The original patches are from Sean O. Stalley. I made a few tweaks, but feel
>>> that it is substancially Sean's work, so I am keeping the patch set version
>>> numbering scheme going.
>>>
>>> Tested on Cavium ThunderX system with 4 Root Complexes containing 50
>>> devices/bridges provisioned with EA.
>>>
>>> Here is Sean's description of the patches:
>>>
>>> PCI Enhanced Allocation is a new method of allocating MMIO & IO
>>> resources for PCI devices & bridges. It can be used instead of the traditional
>>> PCI method of using BARs.
>>>
>>> EA entries are hardware-initialized to a fixed address.
>>> Unlike BARs, regions described by EA are cannot be moved.
>>> Because of this, only devices which are permanently connected to the PCI
>>> bus can use EA. A removable PCI card must not use EA.
>>>
>>> This patchset adds support for using EA entries instead of BARs on Root
>>> Complex Integrated Endpoints.
>>>
>>> The Enhanced Allocation ECN is publicly available here:
>>> https://www.pcisig.com/specifications/conventional/ECN_Enhanced_Alloca
>>> tion_23_Oct_2014_Final.pdf
>>>
>>>
>>> Changes from V1:
>>> 	- Use generic PCI resource claim functions (instead of EA-specific
>>> functions)
>>> 	- Only add support for RCiEPs (instead of all devices).
>>> 	- Removed some debugging messages leftover from early testing.
>>>
>>> Changes from V2 (By David Daney):
>>> 	- Add ea_cap to struct pci_device, to aid in finding the EA capability.
>>> 	- Factored EA entity decoding into a separate function.
>>> 	- Add functions to find EA entities by BEI or Property.
>>> 	- Add handling of EA provisioned bridges.
>>> 	- Add handling of EA SRIOV BARs.
>>> 	- Try to assign proper resource parent so that SRIOV device creation
>>> can occur.
>>>
>>> Changes from V3 (By David Daney):
>>> 	- Discarded V3 changes and started over fresh based on Sean's V2.
>>> 	- Add more support/checking for Entry Properties.
>>> 	- Allow EA behind bridges.
>>> 	- Rewrite some error messages.
>>> 	- Add patch 3/5 to prevent resizing, and better handle
>>>            assigning, of fixed EA resources.
>>> 	- Add patch 4/5 to handle EA provisioned SRIOV devices.
>>> 	- Add patch 5/5 to handle EA provisioned bridges.
>>>
>>> Changes from V4 (By David Daney):
>>> 	- Drop patch 5/5 to handle EA provisioned bridges.
>>> 	- Drop cases for bridge resources in 2/5.
>>> 	- Drop unnecessary fallback resource parent handling in 3/5
>>> 	- Small code formatting improvements.
>>>
>>> David Daney (2):
>>>    PCI: Handle IORESOURCE_PCI_FIXED when sizing and assigning resources.
>>>    PCI: Handle Enhanced Allocation (EA) capability for SRIOV devices.
>>>
>>> Sean O. Stalley (2):
>>>    PCI: Add Enhanced Allocation register entries
>>>    PCI: Add support for Enhanced Allocation devices
>>>
>>>   drivers/pci/iov.c             |  11 ++-
>>>   drivers/pci/pci.c             | 189
>>> ++++++++++++++++++++++++++++++++++++++++++
>>>   drivers/pci/pci.h             |   1 +
>>>   drivers/pci/probe.c           |   3 +
>>>   drivers/pci/setup-bus.c       |  50 ++++++++++-
>>>   include/uapi/linux/pci_regs.h |  44 +++++++++-
>>>   6 files changed, 292 insertions(+), 6 deletions(-)
>>>
>>> --
>>> 1.9.1
>>

Re: [PATCH v5 0/4] PCI: Add support for PCI Enhanced Allocation "BARs"

[Bjorn Helgaas]

On Wed, Oct 14, 2015 at 09:26:09AM -0700, David Daney wrote:
> On 10/14/2015 09:17 AM, Sean O. Stalley wrote:
> >Signed-off-by: Sean O. Stalley <sean.stalley@intel.com>
> >
> 
> Thanks a lot Sean.
> 
> I think you cannot SOB if the patches are not flowing through you
> (as may be the case for my two additions).  Perhaps a Tested-by: or
> Acked-by: would be more appropriate.
> 
> >I tested it out with the QEMU EA Patches here:
> >	[https://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg00348.html]
> >
> >Also, I found 1 trivial typo in the commit message of PATCH 1/4:
> >	"Signed-off-by: Signed-off-by: David Daney <david.daney@cavium.com>"
> 
> Aargh!  I need to be more careful.
> 
> In any case, what should be the next course of action?
> 
>   A) I receive Tested-by/Acked-by from Sean, and resend the four patches?
> 
>   B) Bjorn takes these as is, but fixes the headers as needed.
> 
> Bjorn, what do you think?

You do not need to repost patches just to add Tested-by/Acked-by/etc.

I can also fix trivial things like your Signed-off-by typo.

Bjorn

Re: [PATCH v5 1/4] PCI: Add Enhanced Allocation register entries

[Bjorn Helgaas]

Hi David,

On Tue, Oct 06, 2015 at 04:50:35PM -0700, David Daney wrote:
> From: "Sean O. Stalley" <sean.stalley@intel.com>
> 
> Add registers defined in PCI-SIG's Enhanced allocation ECN.
> 
> Signed-off-by: Sean O. Stalley <sean.stalley@intel.com>
> [david.daney@cavium.com: Added more definitions for PCI_EA_BEI_*]
> Signed-off-by: Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  include/uapi/linux/pci_regs.h | 44 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 413417f..352e418 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -216,7 +216,8 @@
>  #define  PCI_CAP_ID_MSIX	0x11	/* MSI-X */
>  #define  PCI_CAP_ID_SATA	0x12	/* SATA Data/Index Conf. */
>  #define  PCI_CAP_ID_AF		0x13	/* PCI Advanced Features */
> -#define  PCI_CAP_ID_MAX		PCI_CAP_ID_AF
> +#define  PCI_CAP_ID_EA		0x14	/* PCI Enhanced Allocation */
> +#define  PCI_CAP_ID_MAX		PCI_CAP_ID_EA
>  #define PCI_CAP_LIST_NEXT	1	/* Next capability in the list */
>  #define PCI_CAP_FLAGS		2	/* Capability defined flags (16 bits) */
>  #define PCI_CAP_SIZEOF		4
> @@ -353,6 +354,47 @@
>  #define  PCI_AF_STATUS_TP	0x01
>  #define PCI_CAP_AF_SIZEOF	6	/* size of AF registers */
>  
> +/* PCI Enhanced Allocation registers */
> +
> +#define PCI_EA_NUM_ENT		2	/* Number of Capability Entries */
> +#define PCI_EA_NUM_ENT_MASK	0x3f	/* Num Entries Mask */

Can you tweak this a bit to match the style of the rest of the file a
bit closer?

  - no indent (one space) for register offsets
  - two spaces before masks for register fields
  - write out masks showing register width
  - use explicit mask, not BIT()

> +#define PCI_EA_FIRST_ENT	4	/* First EA Entry in List */
> +#define PCI_EA_FIRST_ENT_BRIDGE	8	/* First EA Entry for Bridges */
> +#define PCI_EA_ES		0x7	/* Entry Size */

  #define  PCI_EA_ES            0x00000007   /* Entry Size */

> +#define PCI_EA_BEI(x)	(((x) >> 4) & 0xf) /* BAR Equivalent Indicator */
> +/* 0-5 map to BARs 0-5 respectively */
> +#define  PCI_EA_BEI_BAR0	0
> +#define  PCI_EA_BEI_BAR5	5
> +#define  PCI_EA_BEI_BRIDGE	6	/* Resource behind bridge */
> +#define  PCI_EA_BEI_ENI		7	/* Equivalent Not Indicated */
> +#define  PCI_EA_BEI_ROM		8	/* Expansion ROM */
> +/* 9-14 map to VF BARs 0-5 respectively */
> +#define  PCI_EA_BEI_VF_BAR0	9
> +#define  PCI_EA_BEI_VF_BAR5	14
> +#define  PCI_EA_BEI_RESERVED	15	/* Reserved - Treat like ENI */
> +
> +#define PCI_EA_PP(x)	(((x) >>  8) & 0xff)	/* Primary Properties */
> +#define PCI_EA_SP(x)	(((x) >> 16) & 0xff)	/* Secondary Properties */
> +#define  PCI_EA_P_MEM			0x00	/* Non-Prefetch Memory */

What does the "_P_" stand for?  Maybe it could be dropped?  Oh, I
suppose it stands for "property."

> +#define  PCI_EA_P_MEM_PREFETCH		0x01	/* Prefetchable Memory */
> +#define  PCI_EA_P_IO			0x02	/* I/O Space */
> +#define  PCI_EA_P_VIRT_MEM_PREFETCH	0x03	/* VF Prefetchable Memory */
> +#define  PCI_EA_P_VIRT_MEM		0x04	/* VF Non-Prefetch Memory */

_VIRT_MEM suggests "virtual memory"; maybe you could use "VF_MEM"
instead?

> +#define  PCI_EA_P_BRIDGE_MEM		0x05	/* Bridge Non-Prefetch Memory */
> +#define  PCI_EA_P_BRIDGE_MEM_PREFETCH	0x06	/* Bridge Prefetchable Memory */
> +#define  PCI_EA_P_BRIDGE_IO		0x07	/* Bridge I/O Space */
> +/* 0x08-0xfc reserved */
> +#define  PCI_EA_P_MEM_RESERVED		0xfd	/* Reserved Memory */
> +#define  PCI_EA_P_IO_RESERVED		0xfe	/* Reserved I/O Space */
> +#define  PCI_EA_P_UNAVAILABLE		0xff	/* Entry Unavailable */
> +#define PCI_EA_WRITEABLE	BIT(30) /* Writable, 1 = RW, 0 = HwInit */
> +#define PCI_EA_ENABLE		BIT(31) /* Enable for this entry */

  #define  PCI_EA_WRITABLE      0x40000000
  #define  PCI_EA_ENABLE        0x80000000

(note s/WRITEABLE/WRITABLE/)

> +#define PCI_EA_BASE		4	/* Base Address Offset */
> +#define PCI_EA_MAX_OFFSET	8	/* MaxOffset (resource length) */
> +/* bit 0 is reserved */
> +#define PCI_EA_IS_64		BIT(1)	/* 64-bit field flag */
> +#define PCI_EA_FIELD_MASK	0xfffffffc	/* For Base & Max Offset */
> +
>  /* PCI-X registers (Type 0 (non-bridge) devices) */
>  
>  #define PCI_X_CMD		2	/* Modes & Features */
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 1/4] PCI: Add Enhanced Allocation register entries

[Bjorn Helgaas]

Hi David,

On Tue, Oct 06, 2015 at 04:50:35PM -0700, David Daney wrote:
> From: "Sean O. Stalley" <sean.stalley-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> Add registers defined in PCI-SIG's Enhanced allocation ECN.
> 
> Signed-off-by: Sean O. Stalley <sean.stalley-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> [david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org: Added more definitions for PCI_EA_BEI_*]
> Signed-off-by: Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> ---
>  include/uapi/linux/pci_regs.h | 44 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 413417f..352e418 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -216,7 +216,8 @@
>  #define  PCI_CAP_ID_MSIX	0x11	/* MSI-X */
>  #define  PCI_CAP_ID_SATA	0x12	/* SATA Data/Index Conf. */
>  #define  PCI_CAP_ID_AF		0x13	/* PCI Advanced Features */
> -#define  PCI_CAP_ID_MAX		PCI_CAP_ID_AF
> +#define  PCI_CAP_ID_EA		0x14	/* PCI Enhanced Allocation */
> +#define  PCI_CAP_ID_MAX		PCI_CAP_ID_EA
>  #define PCI_CAP_LIST_NEXT	1	/* Next capability in the list */
>  #define PCI_CAP_FLAGS		2	/* Capability defined flags (16 bits) */
>  #define PCI_CAP_SIZEOF		4
> @@ -353,6 +354,47 @@
>  #define  PCI_AF_STATUS_TP	0x01
>  #define PCI_CAP_AF_SIZEOF	6	/* size of AF registers */
>  
> +/* PCI Enhanced Allocation registers */
> +
> +#define PCI_EA_NUM_ENT		2	/* Number of Capability Entries */
> +#define PCI_EA_NUM_ENT_MASK	0x3f	/* Num Entries Mask */

Can you tweak this a bit to match the style of the rest of the file a
bit closer?

  - no indent (one space) for register offsets
  - two spaces before masks for register fields
  - write out masks showing register width
  - use explicit mask, not BIT()

> +#define PCI_EA_FIRST_ENT	4	/* First EA Entry in List */
> +#define PCI_EA_FIRST_ENT_BRIDGE	8	/* First EA Entry for Bridges */
> +#define PCI_EA_ES		0x7	/* Entry Size */

  #define  PCI_EA_ES            0x00000007   /* Entry Size */

> +#define PCI_EA_BEI(x)	(((x) >> 4) & 0xf) /* BAR Equivalent Indicator */
> +/* 0-5 map to BARs 0-5 respectively */
> +#define  PCI_EA_BEI_BAR0	0
> +#define  PCI_EA_BEI_BAR5	5
> +#define  PCI_EA_BEI_BRIDGE	6	/* Resource behind bridge */
> +#define  PCI_EA_BEI_ENI		7	/* Equivalent Not Indicated */
> +#define  PCI_EA_BEI_ROM		8	/* Expansion ROM */
> +/* 9-14 map to VF BARs 0-5 respectively */
> +#define  PCI_EA_BEI_VF_BAR0	9
> +#define  PCI_EA_BEI_VF_BAR5	14
> +#define  PCI_EA_BEI_RESERVED	15	/* Reserved - Treat like ENI */
> +
> +#define PCI_EA_PP(x)	(((x) >>  8) & 0xff)	/* Primary Properties */
> +#define PCI_EA_SP(x)	(((x) >> 16) & 0xff)	/* Secondary Properties */
> +#define  PCI_EA_P_MEM			0x00	/* Non-Prefetch Memory */

What does the "_P_" stand for?  Maybe it could be dropped?  Oh, I
suppose it stands for "property."

> +#define  PCI_EA_P_MEM_PREFETCH		0x01	/* Prefetchable Memory */
> +#define  PCI_EA_P_IO			0x02	/* I/O Space */
> +#define  PCI_EA_P_VIRT_MEM_PREFETCH	0x03	/* VF Prefetchable Memory */
> +#define  PCI_EA_P_VIRT_MEM		0x04	/* VF Non-Prefetch Memory */

_VIRT_MEM suggests "virtual memory"; maybe you could use "VF_MEM"
instead?

> +#define  PCI_EA_P_BRIDGE_MEM		0x05	/* Bridge Non-Prefetch Memory */
> +#define  PCI_EA_P_BRIDGE_MEM_PREFETCH	0x06	/* Bridge Prefetchable Memory */
> +#define  PCI_EA_P_BRIDGE_IO		0x07	/* Bridge I/O Space */
> +/* 0x08-0xfc reserved */
> +#define  PCI_EA_P_MEM_RESERVED		0xfd	/* Reserved Memory */
> +#define  PCI_EA_P_IO_RESERVED		0xfe	/* Reserved I/O Space */
> +#define  PCI_EA_P_UNAVAILABLE		0xff	/* Entry Unavailable */
> +#define PCI_EA_WRITEABLE	BIT(30) /* Writable, 1 = RW, 0 = HwInit */
> +#define PCI_EA_ENABLE		BIT(31) /* Enable for this entry */

  #define  PCI_EA_WRITABLE      0x40000000
  #define  PCI_EA_ENABLE        0x80000000

(note s/WRITEABLE/WRITABLE/)

> +#define PCI_EA_BASE		4	/* Base Address Offset */
> +#define PCI_EA_MAX_OFFSET	8	/* MaxOffset (resource length) */
> +/* bit 0 is reserved */
> +#define PCI_EA_IS_64		BIT(1)	/* 64-bit field flag */
> +#define PCI_EA_FIELD_MASK	0xfffffffc	/* For Base & Max Offset */
> +
>  /* PCI-X registers (Type 0 (non-bridge) devices) */
>  
>  #define PCI_X_CMD		2	/* Modes & Features */
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 2/4] PCI: Add support for Enhanced Allocation devices

[Bjorn Helgaas]

On Tue, Oct 06, 2015 at 04:50:36PM -0700, David Daney wrote:
> From: "Sean O. Stalley" <sean.stalley@intel.com>
> 
> Add support for devices using Enhanced Allocation entries instead of BARs.
> This patch allows the kernel to parse the EA Extended Capability structure
> in PCI configspace and claim the BAR-equivalent resources.
> 
> Signed-off-by: Sean O. Stalley <sean.stalley@intel.com>
> [david.daney@cavium.com: Add more support/checking for Entry Properties,
> allow EA behind bridges, rewrite some error messages.]
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  drivers/pci/pci.c   | 182 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/pci.h   |   1 +
>  drivers/pci/probe.c |   3 +
>  3 files changed, 186 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 6a9a111..30a90d1 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2148,6 +2148,188 @@ void pci_pm_init(struct pci_dev *dev)
>  	}
>  }
>  
> +static unsigned long pci_ea_set_flags(struct pci_dev *dev, u8 prop)
> +{
> +	unsigned long flags = IORESOURCE_PCI_FIXED;
> +
> +	switch (prop) {
> +	case PCI_EA_P_MEM:
> +	case PCI_EA_P_VIRT_MEM:
> +		flags |= IORESOURCE_MEM;
> +		break;
> +	case PCI_EA_P_MEM_PREFETCH:
> +	case PCI_EA_P_VIRT_MEM_PREFETCH:
> +		flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH;
> +		break;
> +	case PCI_EA_P_IO:
> +		flags |= IORESOURCE_IO;
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	return flags;
> +}
> +
> +static struct resource *pci_ea_get_resource(struct pci_dev *dev, u8 bei,
> +					    u8 prop)
> +{
> +	if (bei <= PCI_EA_BEI_BAR5 && prop <= PCI_EA_P_IO)
> +		return &dev->resource[bei];
> +	else if (bei == PCI_EA_BEI_ROM)
> +		return &dev->resource[PCI_ROM_RESOURCE];
> +	else
> +		return NULL;
> +}
> +
> +/* Read an Enhanced Allocation (EA) entry */
> +static int pci_ea_read(struct pci_dev *dev, int offset)
> +{
> +	struct resource *res;
> +	int ent_offset = offset;
> +	int ent_size;
> +	resource_size_t start;
> +	resource_size_t end;
> +	unsigned long flags;
> +	u32 dw0;
> +	u32 base;
> +	u32 max_offset;
> +	u8 prop;
> +	bool support_64 = (sizeof(resource_size_t) >= 8);
> +
> +	pci_read_config_dword(dev, ent_offset, &dw0);
> +	ent_offset += 4;
> +
> +	/* Entry size field indicates DWORDs after 1st */
> +	ent_size = ((dw0 & PCI_EA_ES) + 1) << 2;
> +
> +	if (!(dw0 & PCI_EA_ENABLE)) /* Entry not enabled */
> +		goto out;
> +
> +	prop = PCI_EA_PP(dw0);
> +	/*
> +	 * If the Property is in the reserved range, try the Secondary
> +	 * Property instead.
> +	 */
> +	if (prop > PCI_EA_P_BRIDGE_IO && prop < PCI_EA_P_MEM_RESERVED)
> +		prop = PCI_EA_SP(dw0);
> +	if (prop > PCI_EA_P_BRIDGE_IO)
> +		goto out;
> +
> +	res = pci_ea_get_resource(dev, PCI_EA_BEI(dw0), prop);
> +	if (!res) {
> +		dev_err(&dev->dev, "Unsupported EA entry BEI: %u\n",
> +			PCI_EA_BEI(dw0));
> +		goto out;
> +	}
> +
> +	flags = pci_ea_set_flags(dev, prop);
> +	if (!flags) {
> +		dev_err(&dev->dev, "Unsupported EA properties: %u\n", prop);
> +		goto out;
> +	}
> +
> +	/* Read Base */
> +	pci_read_config_dword(dev, ent_offset, &base);
> +	start = (base & PCI_EA_FIELD_MASK);
> +	ent_offset += 4;
> +
> +	/* Read MaxOffset */
> +	pci_read_config_dword(dev, ent_offset, &max_offset);
> +	ent_offset += 4;
> +
> +	/* Read Base MSBs (if 64-bit entry) */
> +	if (base & PCI_EA_IS_64) {
> +		u32 base_upper;
> +
> +		pci_read_config_dword(dev, ent_offset, &base_upper);
> +		ent_offset += 4;
> +
> +		flags |= IORESOURCE_MEM_64;
> +
> +		/* entry starts above 32-bit boundary, can't use */
> +		if (!support_64 && base_upper)
> +			goto out;
> +
> +		if (support_64)
> +			start |= ((u64)base_upper << 32);
> +	}
> +
> +	dev_dbg(&dev->dev,
> +		"EA (%u,%u) start = %pa\n", PCI_EA_BEI(dw0), prop, &start);
> +
> +	end = start + (max_offset | 0x03);
> +
> +	/* Read MaxOffset MSBs (if 64-bit entry) */
> +	if (max_offset & PCI_EA_IS_64) {
> +		u32 max_offset_upper;
> +
> +		pci_read_config_dword(dev, ent_offset, &max_offset_upper);
> +		ent_offset += 4;
> +
> +		flags |= IORESOURCE_MEM_64;
> +
> +		/* entry too big, can't use */
> +		if (!support_64 && max_offset_upper)
> +			goto out;
> +
> +		if (support_64)
> +			end += ((u64)max_offset_upper << 32);
> +	}
> +
> +	dev_dbg(&dev->dev,
> +		"EA (%u,%u) end = %pa\n", PCI_EA_BEI(dw0), prop, &end);
> +
> +	if (end < start) {
> +		dev_err(&dev->dev, "EA Entry crosses address boundary\n");
> +		goto out;
> +	}
> +
> +	if (ent_size != ent_offset - offset) {
> +		dev_err(&dev->dev,
> +			"EA Entry Size (%d) does not match length read (%d)\n",
> +			ent_size, ent_offset - offset);
> +		goto out;
> +	}
> +
> +	res->name = pci_name(dev);
> +	res->start = start;
> +	res->end = end;
> +	res->flags = flags;

This is similar to reading a BAR from a normal device; can you print what
we found so it looks similar in dmesg, e.g., similar to what
__pci_read_base() does?  Note that "dev_dbg" is not equivalent to
dev_printk(KERN_DEBUG).  I want the output in dmesg all the time, without
having to enable something via dyndbg.

> +
> +out:
> +	return offset + ent_size;
> +}
> +
> +/* Enhanced Allocation Initalization */
> +void pci_ea_init(struct pci_dev *dev)
> +{
> +	int ea;
> +	u8 num_ent;
> +	int offset;
> +	int i;
> +
> +	/* find PCI EA capability in list */
> +	ea = pci_find_capability(dev, PCI_CAP_ID_EA);
> +	if (!ea)
> +		return;
> +
> +	/* determine the number of entries */
> +	pci_bus_read_config_byte(dev->bus, dev->devfn, ea + PCI_EA_NUM_ENT,
> +					&num_ent);
> +	num_ent &= PCI_EA_NUM_ENT_MASK;
> +
> +	offset = ea + PCI_EA_FIRST_ENT;
> +
> +	/* Skip DWORD 2 for type 1 functions */
> +	if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE)
> +		offset += 4;
> +
> +	/* parse each EA entry */
> +	for (i = 0; i < num_ent; ++i)
> +		offset = pci_ea_read(dev, offset);
> +}
> +
>  static void pci_add_saved_cap(struct pci_dev *pci_dev,
>  	struct pci_cap_saved_state *new_cap)
>  {
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 24ba9dc..a160733 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -78,6 +78,7 @@ bool pci_dev_keep_suspended(struct pci_dev *dev);
>  void pci_config_pm_runtime_get(struct pci_dev *dev);
>  void pci_config_pm_runtime_put(struct pci_dev *dev);
>  void pci_pm_init(struct pci_dev *dev);
> +void pci_ea_init(struct pci_dev *dev);
>  void pci_allocate_cap_save_buffers(struct pci_dev *dev);
>  void pci_free_cap_save_buffers(struct pci_dev *dev);
>  
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 8361d27..4c4af78 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1597,6 +1597,9 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
>  
>  static void pci_init_capabilities(struct pci_dev *dev)
>  {
> +	/* Enhanced Allocation */
> +	pci_ea_init(dev);
> +
>  	/* MSI/MSI-X list */
>  	pci_msi_init_pci_dev(dev);
>  
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 3/4] PCI: Handle IORESOURCE_PCI_FIXED when sizing and assigning resources.

[Bjorn Helgaas]

On Tue, Oct 06, 2015 at 04:50:37PM -0700, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> The new Enhanced Allocation (EA) capability support creates resources
> with the IORESOURCE_PCI_FIXED set.  This causes a couple of problems:
> 
> 1) Since these resources cannot be relocated or resized, their
>    alignment is not really defined, and it is therefore not specified.
>    This causes a problem in pbus_size_mem() where resources with
>    unspecified alignment are disabled.
> 
> 2) During resource assignment in pci_bus_assign_resources(),
>    IORESOURCE_PCI_FIXED resources are not given a parent.  This, in
>    turn, causes pci_enable_resources() to fail with a "not claimed"
>    error.
> 
> So, in pbus_size_mem() skip IORESOURCE_PCI_FIXED resources, instead of
> disabling them.
> 
> In __pci_bus_assign_resources(), for IORESOURCE_PCI_FIXED resources,
> try to request the resource from a parent bus.

This is fixing two problems; can you just split this into two patches?
I think the changelogs will read more easily then.

It seems like maybe these fixes should also precede the "add support
for EA devices" patch.  We already use IORESOURCE_PCI_FIXED in a few
cases, and these are probably applicable to them, and if you have the
fixes in first, we'll have less of a bisection hole when we add EA
support.

> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  drivers/pci/setup-bus.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 508cc56..7239a2c 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1037,9 +1037,10 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>  			struct resource *r = &dev->resource[i];
>  			resource_size_t r_size;
>  
> -			if (r->parent || ((r->flags & mask) != type &&
> -					  (r->flags & mask) != type2 &&
> -					  (r->flags & mask) != type3))
> +			if (r->parent || (r->flags | IORESOURCE_PCI_FIXED) ||
> +			    ((r->flags & mask) != type &&
> +			     (r->flags & mask) != type2 &&
> +			     (r->flags & mask) != type3))
>  				continue;
>  			r_size = resource_size(r);
>  #ifdef CONFIG_PCI_IOV
> @@ -1340,6 +1341,47 @@ void pci_bus_size_bridges(struct pci_bus *bus)
>  }
>  EXPORT_SYMBOL(pci_bus_size_bridges);
>  
> +static void assign_fixed_resource_on_bus(struct pci_bus *b, struct resource *r)
> +{
> +	int i;
> +	struct resource *parent_r;
> +	unsigned long mask = IORESOURCE_IO | IORESOURCE_MEM |
> +			     IORESOURCE_PREFETCH;
> +
> +	pci_bus_for_each_resource(b, parent_r, i) {
> +		if (!parent_r)
> +			continue;
> +
> +		if ((r->flags & mask) == (parent_r->flags & mask) &&
> +		    resource_contains(parent_r, r))
> +			request_resource(parent_r, r);

If request_resource() fails, it seems like it'd useful to know about
it.  Can you use request_resource_conflict() and add a diagnostic
similar to what's in pci_claim_resource()?

> +	}
> +}
> +
> +/*
> + * Try to assign any resources marked as IORESOURCE_PCI_FIXED, as they
> + * are skipped by pbus_assign_resources_sorted().
> + */
> +static void pdev_assign_fixed_resources(struct pci_dev *dev)

"assign" seems like the wrong verb here (and above).  I think of
"assign" as the process where we might change the addresses to which
the device responds, seting res->start accordingly.  But here, we
already know those addresses, and we're merely telling the resource
manager what we're using.

> +{
> +	int i;
> +
> +	for (i = 0; i <  PCI_NUM_RESOURCES; i++) {
> +		struct pci_bus *b;
> +		struct resource *r = &dev->resource[i];
> +
> +		if (r->parent || !(r->flags & IORESOURCE_PCI_FIXED) ||
> +		    !(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
> +			continue;
> +
> +		b = dev->bus;
> +		while (b && !r->parent) {
> +			assign_fixed_resource_on_bus(b, r);
> +			b = b->parent;
> +		}
> +	}
> +}
> +
>  void __pci_bus_assign_resources(const struct pci_bus *bus,
>  				struct list_head *realloc_head,
>  				struct list_head *fail_head)
> @@ -1350,6 +1392,8 @@ void __pci_bus_assign_resources(const struct pci_bus *bus,
>  	pbus_assign_resources_sorted(bus, realloc_head, fail_head);
>  
>  	list_for_each_entry(dev, &bus->devices, bus_list) {
> +		pdev_assign_fixed_resources(dev);
> +
>  		b = dev->subordinate;
>  		if (!b)
>  			continue;
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 3/4] PCI: Handle IORESOURCE_PCI_FIXED when sizing and assigning resources.

[Bjorn Helgaas]

On Tue, Oct 06, 2015 at 04:50:37PM -0700, David Daney wrote:
> From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> 
> The new Enhanced Allocation (EA) capability support creates resources
> with the IORESOURCE_PCI_FIXED set.  This causes a couple of problems:
> 
> 1) Since these resources cannot be relocated or resized, their
>    alignment is not really defined, and it is therefore not specified.
>    This causes a problem in pbus_size_mem() where resources with
>    unspecified alignment are disabled.
> 
> 2) During resource assignment in pci_bus_assign_resources(),
>    IORESOURCE_PCI_FIXED resources are not given a parent.  This, in
>    turn, causes pci_enable_resources() to fail with a "not claimed"
>    error.
> 
> So, in pbus_size_mem() skip IORESOURCE_PCI_FIXED resources, instead of
> disabling them.
> 
> In __pci_bus_assign_resources(), for IORESOURCE_PCI_FIXED resources,
> try to request the resource from a parent bus.

This is fixing two problems; can you just split this into two patches?
I think the changelogs will read more easily then.

It seems like maybe these fixes should also precede the "add support
for EA devices" patch.  We already use IORESOURCE_PCI_FIXED in a few
cases, and these are probably applicable to them, and if you have the
fixes in first, we'll have less of a bisection hole when we add EA
support.

> Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/pci/setup-bus.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 508cc56..7239a2c 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1037,9 +1037,10 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>  			struct resource *r = &dev->resource[i];
>  			resource_size_t r_size;
>  
> -			if (r->parent || ((r->flags & mask) != type &&
> -					  (r->flags & mask) != type2 &&
> -					  (r->flags & mask) != type3))
> +			if (r->parent || (r->flags | IORESOURCE_PCI_FIXED) ||
> +			    ((r->flags & mask) != type &&
> +			     (r->flags & mask) != type2 &&
> +			     (r->flags & mask) != type3))
>  				continue;
>  			r_size = resource_size(r);
>  #ifdef CONFIG_PCI_IOV
> @@ -1340,6 +1341,47 @@ void pci_bus_size_bridges(struct pci_bus *bus)
>  }
>  EXPORT_SYMBOL(pci_bus_size_bridges);
>  
> +static void assign_fixed_resource_on_bus(struct pci_bus *b, struct resource *r)
> +{
> +	int i;
> +	struct resource *parent_r;
> +	unsigned long mask = IORESOURCE_IO | IORESOURCE_MEM |
> +			     IORESOURCE_PREFETCH;
> +
> +	pci_bus_for_each_resource(b, parent_r, i) {
> +		if (!parent_r)
> +			continue;
> +
> +		if ((r->flags & mask) == (parent_r->flags & mask) &&
> +		    resource_contains(parent_r, r))
> +			request_resource(parent_r, r);

If request_resource() fails, it seems like it'd useful to know about
it.  Can you use request_resource_conflict() and add a diagnostic
similar to what's in pci_claim_resource()?

> +	}
> +}
> +
> +/*
> + * Try to assign any resources marked as IORESOURCE_PCI_FIXED, as they
> + * are skipped by pbus_assign_resources_sorted().
> + */
> +static void pdev_assign_fixed_resources(struct pci_dev *dev)

"assign" seems like the wrong verb here (and above).  I think of
"assign" as the process where we might change the addresses to which
the device responds, seting res->start accordingly.  But here, we
already know those addresses, and we're merely telling the resource
manager what we're using.

> +{
> +	int i;
> +
> +	for (i = 0; i <  PCI_NUM_RESOURCES; i++) {
> +		struct pci_bus *b;
> +		struct resource *r = &dev->resource[i];
> +
> +		if (r->parent || !(r->flags & IORESOURCE_PCI_FIXED) ||
> +		    !(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
> +			continue;
> +
> +		b = dev->bus;
> +		while (b && !r->parent) {
> +			assign_fixed_resource_on_bus(b, r);
> +			b = b->parent;
> +		}
> +	}
> +}
> +
>  void __pci_bus_assign_resources(const struct pci_bus *bus,
>  				struct list_head *realloc_head,
>  				struct list_head *fail_head)
> @@ -1350,6 +1392,8 @@ void __pci_bus_assign_resources(const struct pci_bus *bus,
>  	pbus_assign_resources_sorted(bus, realloc_head, fail_head);
>  
>  	list_for_each_entry(dev, &bus->devices, bus_list) {
> +		pdev_assign_fixed_resources(dev);
> +
>  		b = dev->subordinate;
>  		if (!b)
>  			continue;
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/