stephend@silicom-usa.com has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34620 )
Change subject: pci: Add support for assigning resources to SR-IOV VF BARs ......................................................................
pci: Add support for assigning resources to SR-IOV VF BARs
This ensures that bridge windows allocate enough space to cover SR-IOV BARs. Without this Linux will print messages like:
pci 0000:03:00.0: BAR 7: no space for [mem size 0x00100000 64bit] pci 0000:03:00.0: BAR 7: failed to assign [mem size 0x00100000 64bit]
Tested on Camelback Mountain, and Harcuvar.
Change-Id: Ib169efe5a6b998a8342a895f1456a280669c719d Signed-off-by: Stephen Douthit stephend@silicom-usa.com --- M src/device/pci_device.c M src/device/pciexp_device.c M src/include/device/pci_def.h M src/include/device/pciexp.h 4 files changed, 112 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/34620/1
diff --git a/src/device/pci_device.c b/src/device/pci_device.c index 7786043..c96fc8d 100644 --- a/src/device/pci_device.c +++ b/src/device/pci_device.c @@ -439,6 +439,11 @@ { pci_read_bases(dev, 6); pci_get_rom_resource(dev, PCI_ROM_ADDRESS); + +#if CONFIG(PCIEXP_PLUGIN_SUPPORT) + /* Check for SR-IOV BARs if we have PCIe support */ + pciexp_dev_read_resources(dev); +#endif }
void pci_bus_read_resources(struct device *dev) diff --git a/src/device/pciexp_device.c b/src/device/pciexp_device.c index c209816..8d660b1 100644 --- a/src/device/pciexp_device.c +++ b/src/device/pciexp_device.c @@ -442,6 +442,82 @@ pciexp_enable_aspm(root, root_cap, dev, cap); }
+/* + * Check if this is an SR-IOV capable device and add resources for all VF bars + * + * @param dev Pointer to the dev structure. + */ +void pciexp_dev_read_resources(struct device *dev) +{ + unsigned long sriovpos; + u16 numvfs, i; + + sriovpos = pci_find_capability(dev, PCI_CAP_ID_PCIE); + if (!sriovpos) { + return; + } + + sriovpos = pciexp_find_extended_cap(dev, PCI_EXT_CAP_ID_SRIOV); + if (!sriovpos) { + return; + } + + numvfs = pci_read_config16(dev, sriovpos + PCI_SRIOV_TOT_VFS); + printk(BIOS_DEBUG, "%s: supports %d SR-IOV VFs\n", dev_path(dev), numvfs); + + /* The spec allows this to be 0 for some reason. Nothing to do. */ + if (numvfs == 0) { + return; + } + + for (int off = 0; off < 6; off++) { + unsigned long res_ix = sriovpos + PCI_SRIOV_VFBAR0 + off * 4; + struct resource *resource; + + resource = pci_get_resource(dev, res_ix); + + /* VF BARs aren't necessarily contiguous, skip the unused ones */ + if (resource->size == 0) { + continue; + } + + printk(BIOS_DEBUG, "%s: found %dbit SR-IOV BAR, size 0x%llx @ index %lx\n", + dev_path(dev), (resource->flags & IORESOURCE_PCI64) ? 64 : 32, + resource->size, resource->index); + + if (resource->flags & IORESOURCE_PCI64) { + off++; + } + + /* + * SR-IOV BARs break the resource allocator assumption for PCI + * dev resources that size = gran = alignment. + * + * alignment = gran = pci_get_resource() result, but... + * size is pci_get_resource()->size * numvfs, and there's no + * power of two guarantee on size either since numvfs is just + * an integer. + * + * Rather than add code to handle this as a special case in the + * resource allocator, just round up the size. In practice + * MaxVfs tends to be 2^n or 2^n - 1, so the holes produced + * should only be the size of a single VF BAR + */ + for (i = 1; i < numvfs; i <<= 1) { + resource->size <<= 1; + resource->align += 1; + resource->gran += 1; + } + + if (i != numvfs) { + printk(BIOS_DEBUG, "%s: VFs != 2^n, wasting MMIO space...\n", + dev_path(dev)); + } + } + + compact_resources(dev); +} + void pciexp_scan_bus(struct bus *bus, unsigned int min_devfn, unsigned int max_devfn) { diff --git a/src/include/device/pci_def.h b/src/include/device/pci_def.h index bc5bc79..39a6137 100644 --- a/src/include/device/pci_def.h +++ b/src/include/device/pci_def.h @@ -453,6 +453,7 @@ #define PCI_EXT_CAP_ID_VC 2 #define PCI_EXT_CAP_ID_DSN 3 #define PCI_EXT_CAP_ID_PWR 4 +#define PCI_EXT_CAP_ID_SRIOV 0x0010
/* Extended Capability lists*/ #define PCIE_EXT_CAP_OFFSET 0x100 @@ -518,6 +519,34 @@ #define PCI_PWR_CAP 12 /* Capability */ #define PCI_PWR_CAP_BUDGET(x) ((x) & 1) /* Included in system budget */
+/* SR-IOV */ +#define PCI_SRIOV_CAPS 0x04 /* SR-IOV capabilities */ +#define PCI_SRIOV_VF_MIG (1 << 0) /* VF Migration capable */ +#define PCI_SRIOV_ARI_CAP (1 << 1) /* ARI Capable Hierarchy Preserved */ +#define PCI_SRIOV_MIG_INT(x) (((x) >> 21) & 0x7ff) +#define PCI_SRIOV_CTRL 0x08 /* SR-IOV Control */ +#define PCI_SRIOV_VF_EN (1 << 0) +#define PCI_SRIOV_MIG_EN (1 << 1) +#define PCI_SRIOV_MIG_INT_EN (1 << 2) +#define PCI_SRIOV_MSE (1 << 3) +#define PCI_SRIOV_ARI_CAP_EN (1 << 4) +#define PCI_SRIOV_STATUS 0x0A /* SR-IOV Status */ +#define PCI_SRIOV_INIT_VFS 0x0C +#define PCI_SRIOV_TOT_VFS 0x0E +#define PCI_SRIOV_NUM_VFS 0x10 +#define PCI_SRIOV_FN_DEP_LINK 0x12 +#define PCI_SRIOV_VF_OFF 0x14 +#define PCI_SRIOV_VF_STRIDE 0x16 +#define PCI_SRIOV_VF_DEVID 0x1A +#define PCI_SRIOV_SUP_PAGE_SIZE 0x1C +#define PCI_SRIOV_PAGE_SIZE 0x20 +#define PCI_SRIOV_VFBAR0 0x24 +#define PCI_SRIOV_VFBAR1 0x28 +#define PCI_SRIOV_VFBAR2 0x2C +#define PCI_SRIOV_VFBAR3 0x30 +#define PCI_SRIOV_VFBAR4 0x34 +#define PCI_SRIOV_VFBAR5 0x38 +#define PCI_SRIOV_MIG_ARRAY 0x3C
/* * The PCI interface treats multi-function devices as independent diff --git a/src/include/device/pciexp.h b/src/include/device/pciexp.h index 3a9825d..94a80bf 100644 --- a/src/include/device/pciexp.h +++ b/src/include/device/pciexp.h @@ -19,6 +19,8 @@ /* Latency tolerance reporting, max snoop latency value 3.14ms */ #define PCIE_LTR_MAX_SNOOP_LATENCY_3146US 0x1003
+void pciexp_dev_read_resources(struct device *dev); + void pciexp_scan_bus(struct bus *bus, unsigned int min_devfn, unsigned int max_devfn);
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34620 )
Change subject: pci: Add support for assigning resources to SR-IOV VF BARs ......................................................................
Patch Set 1:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34620/1/src/device/pciexp_device.c File src/device/pciexp_device.c:
https://review.coreboot.org/c/coreboot/+/34620/1/src/device/pciexp_device.c@... PS1, Line 456: if (!sriovpos) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/c/coreboot/+/34620/1/src/device/pciexp_device.c@... PS1, Line 461: if (!sriovpos) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/c/coreboot/+/34620/1/src/device/pciexp_device.c@... PS1, Line 469: if (numvfs == 0) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/c/coreboot/+/34620/1/src/device/pciexp_device.c@... PS1, Line 480: if (resource->size == 0) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/c/coreboot/+/34620/1/src/device/pciexp_device.c@... PS1, Line 488: if (resource->flags & IORESOURCE_PCI64) { braces {} are not necessary for single statement blocks
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34620
to look at the new patch set (#2).
Change subject: pci: Add support for assigning resources to SR-IOV VF BARs ......................................................................
pci: Add support for assigning resources to SR-IOV VF BARs
This ensures that bridge windows allocate enough space to cover SR-IOV BARs. Without this Linux will print messages like:
pci 0000:03:00.0: BAR 7: no space for [mem size 0x00100000 64bit] pci 0000:03:00.0: BAR 7: failed to assign [mem size 0x00100000 64bit]
Tested on Camelback Mountain, and Harcuvar.
Change-Id: Ib169efe5a6b998a8342a895f1456a280669c719d Signed-off-by: Stephen Douthit stephend@silicom-usa.com --- M src/device/pci_device.c M src/device/pciexp_device.c M src/include/device/pci_def.h M src/include/device/pciexp.h 4 files changed, 107 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/34620/2
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34620 )
Change subject: pci: Add support for assigning resources to SR-IOV VF BARs ......................................................................
Patch Set 2: Code-Review+1
Looks good to me at least from the code perspective
Attention is currently required from: Stephen Douthit.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34620 )
Change subject: pci: Add support for assigning resources to SR-IOV VF BARs ......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/34620/comment/bb55a8d8_ee0e9165 PS2, Line 10: Linux Please mention the/a version.
Attention is currently required from: Stephen Douthit.
Harrie Paijmans has posted comments on this change by Stephen Douthit. ( https://review.coreboot.org/c/coreboot/+/34620?usp=email )
Change subject: pci: Add support for assigning resources to SR-IOV VF BARs ......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2: Hi,
We see this same problem on our Raptorlake P platform.
I have rebased this patch on the latest main branch and updated it to accommodate changes made after this patchset, it solves the issues described above.
I want to ask if I can update this patchset with my updated version?
-
build bot (Jenkins) (Code Review) -
Harrie Paijmans (Code Review)
-
Name of user not set (Code Review)
-
Paul Menzel (Code Review)
-
Philipp Deppenwiese (Code Review)