2021-02-11 09:31:10

by Gustavo Pimentel

[permalink] [raw]
Subject: [PATCH v5 04/15] PCI: Add pci_find_vsec_capability() to find a specific VSEC

Add pci_find_vsec_capability() that crawls through the device config
space searching in all Vendor-Specific Extended Capabilities for a
particular capability ID.

Vendor-Specific Extended Capability (VSEC) is a PCIe capability (acts
like a wrapper) specified by PCI-SIG that allows the vendor to create
their own and specific capability in the device config space.

Signed-off-by: Gustavo Pimentel <[email protected]>
---
drivers/pci/pci.c | 34 ++++++++++++++++++++++++++++++++++
include/linux/pci.h | 2 ++
include/uapi/linux/pci_regs.h | 6 ++++++
3 files changed, 42 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b9fecc2..1307af6 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -693,6 +693,40 @@ u8 pci_find_ht_capability(struct pci_dev *dev, int ht_cap)
EXPORT_SYMBOL_GPL(pci_find_ht_capability);

/**
+ * pci_find_vsec_capability - Find a vendor-specific extended capability
+ * @dev: PCI device to query
+ * @cap: vendor-specific capability ID code
+ *
+ * Typically this function will be called by the pci driver, which passes
+ * through argument the 'struct pci_dev *' already pointing for the device
+ * config space that is associated with the vendor and device ID which will
+ * know which ID to search and what to do with it, however, it might be
+ * cases that this function could be called outside of this scope and
+ * therefore is the caller responsibility to check the vendor and/or
+ * device ID first.
+ *
+ * Returns the address of the vendor-specific structure that matches the
+ * requested capability ID code within the device's PCI configuration space
+ * or 0 if it does not find a match.
+ */
+u16 pci_find_vsec_capability(struct pci_dev *dev, int vsec_cap_id)
+{
+ u16 vsec = 0;
+ u32 header;
+
+ while ((vsec = pci_find_next_ext_capability(dev, vsec,
+ PCI_EXT_CAP_ID_VNDR))) {
+ if (pci_read_config_dword(dev, vsec + PCI_VSEC_HDR,
+ &header) == PCIBIOS_SUCCESSFUL &&
+ PCI_VSEC_CAP_ID(header) == vsec_cap_id)
+ return vsec;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pci_find_vsec_capability);
+
+/**
* pci_find_parent_resource - return resource region of parent bus of given
* region
* @dev: PCI device structure contains resources to be searched
diff --git a/include/linux/pci.h b/include/linux/pci.h
index b32126d..da6ab6a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1080,6 +1080,8 @@ struct pci_bus *pci_find_next_bus(const struct pci_bus *from);

u64 pci_get_dsn(struct pci_dev *dev);

+u16 pci_find_vsec_capability(struct pci_dev *dev, int vsec_cap_id);
+
struct pci_dev *pci_get_device(unsigned int vendor, unsigned int device,
struct pci_dev *from);
struct pci_dev *pci_get_subsys(unsigned int vendor, unsigned int device,
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index e709ae8..deae275 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -983,6 +983,12 @@
#define PCI_VSEC_HDR 4 /* extended cap - vendor-specific */
#define PCI_VSEC_HDR_LEN_SHIFT 20 /* shift for length field */

+/* Vendor-Specific Extended Capabilities */
+#define PCI_VSEC_HEADER 4 /* Vendor-Specific Header */
+#define PCI_VSEC_CAP_ID(x) ((x) & 0xffff)
+#define PCI_VSEC_CAP_REV(x) (((x) >> 16) & 0xf)
+#define PCI_VSEC_CAP_LEN(x) (((x) >> 20) & 0xfff)
+
/* SATA capability */
#define PCI_SATA_REGS 4 /* SATA REGs specifier */
#define PCI_SATA_REGS_MASK 0xF /* location - BAR#/inline */
--
2.7.4


2021-02-11 13:12:53

by Krzysztof Wilczyński

[permalink] [raw]
Subject: Re: [PATCH v5 04/15] PCI: Add pci_find_vsec_capability() to find a specific VSEC

Hi Gustavo,

[...]
> + * Typically this function will be called by the pci driver, which passes

It would be "PCI" here.

> + * through argument the 'struct pci_dev *' already pointing for the device
> + * config space that is associated with the vendor and device ID which will
> + * know which ID to search and what to do with it, however, it might be

Probably "there might be".

> + * cases that this function could be called outside of this scope and
> + * therefore is the caller responsibility to check the vendor and/or
[...]

A suggestion. This commit message is a little hard to read and could be
improved.

It might just be me (by and large, and I am not a native English
speaker), but it's actually easier to figure out what the function does
after reading the implementation that from the comment. :)

Krzysztof

2021-02-11 19:48:10

by Gustavo Pimentel

[permalink] [raw]
Subject: RE: [PATCH v5 04/15] PCI: Add pci_find_vsec_capability() to find a specific VSEC

On Thu, Feb 11, 2021 at 12:50:46, Krzysztof Wilczyński <[email protected]>
wrote:

> Hi Gustavo,
>
> [...]
> > + * Typically this function will be called by the pci driver, which passes
>
> It would be "PCI" here.

Nicely catch.

>
> > + * through argument the 'struct pci_dev *' already pointing for the device
> > + * config space that is associated with the vendor and device ID which will
> > + * know which ID to search and what to do with it, however, it might be
>
> Probably "there might be".

I've rephrased it.

>
> > + * cases that this function could be called outside of this scope and
> > + * therefore is the caller responsibility to check the vendor and/or
> [...]
>
> A suggestion. This commit message is a little hard to read and could be
> improved.
>
> It might just be me (by and large, and I am not a native English
> speaker), but it's actually easier to figure out what the function does
> after reading the implementation that from the comment. :)

I have also rephrased it. I hope it would be better, but I don't see how.

"Adds another helper to ones that already exist called
pci_find_vsec_capability. This helper crawls through the device PCI
config space searching for a specific ID on the Vendor-Specific Extended
Capabilities section.

The Vendor-Specific Extended Capability (VSEC) is a special PCI
capability (acts like container) defined by PCI-SIG that allows the one
or more proprietary capabilities defined by the vendor which aren't
standard or shared between the manufactures.

-Gustavo

>
> Krzysztof