diff mbox

[dpdk-dev,v2,2/3] eal: PCI domain should be 32 bits

Message ID 20170622155641.25916-3-stephen@networkplumber.org (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Stephen Hemminger June 22, 2017, 3:56 p.m. UTC 
  In some environments, the PCI domain can be larger than 16 bits.
For example, a PCI device passed through in Azure gets a synthetic domain
id  which is internally generated based on GUID. The PCI standard does
not restrict domain to be 16 bits.

This change breaks ABI for API's that expose PCI address structure.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
v2 -- need to expand string size and shifts in compare function

 lib/librte_eal/common/include/rte_pci.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Thomas Monjalon June 22, 2017, 4:05 p.m. UTC | #1 
22/06/2017 17:56, Stephen Hemminger:
> In some environments, the PCI domain can be larger than 16 bits.
> For example, a PCI device passed through in Azure gets a synthetic domain
> id  which is internally generated based on GUID. The PCI standard does
> not restrict domain to be 16 bits.
> 
> This change breaks ABI for API's that expose PCI address structure.
> 
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
[...]
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -63,7 +63,7 @@ const char *pci_get_sysfs_path(void);
>  
>  /** Formatting string for PCI device identifier: Ex: 0000:00:01.0 */
>  #define PCI_PRI_FMT "%.4" PRIx16 ":%.2" PRIx8 ":%.2" PRIx8 ".%" PRIx8
> -#define PCI_PRI_STR_SIZE sizeof("XXXX:XX:XX.X")
> +#define PCI_PRI_STR_SIZE sizeof("XXXXXXXX:XX:XX.X")

I think you need to change PCI_PRI_FMT accordingly.
Stephen Hemminger June 22, 2017, 4:23 p.m. UTC | #2 
On Thu, 22 Jun 2017 18:05:42 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 22/06/2017 17:56, Stephen Hemminger:
> > In some environments, the PCI domain can be larger than 16 bits.
> > For example, a PCI device passed through in Azure gets a synthetic domain
> > id  which is internally generated based on GUID. The PCI standard does
> > not restrict domain to be 16 bits.
> > 
> > This change breaks ABI for API's that expose PCI address structure.
> > 
> > Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> > ---  
> [...]
> > --- a/lib/librte_eal/common/include/rte_pci.h
> > +++ b/lib/librte_eal/common/include/rte_pci.h
> > @@ -63,7 +63,7 @@ const char *pci_get_sysfs_path(void);
> >  
> >  /** Formatting string for PCI device identifier: Ex: 0000:00:01.0 */
> >  #define PCI_PRI_FMT "%.4" PRIx16 ":%.2" PRIx8 ":%.2" PRIx8 ".%" PRIx8
> > -#define PCI_PRI_STR_SIZE sizeof("XXXX:XX:XX.X")
> > +#define PCI_PRI_STR_SIZE sizeof("XXXXXXXX:XX:XX.X")  
> 
> I think you need to change PCI_PRI_FMT accordingly.

No. I don't want all outputs to have extra leading zeros on other platforms
The existing format works:

Example:
--- cut here ---

struct rte_pci_addr {
	uint32_t domain;                /**< Device domain */
	uint8_t bus;                    /**< Device bus */
	uint8_t devid;                  /**< Device ID */
	uint8_t function;               /**< Device function. */
};

#define PCI_PRI_FMT "%.4" PRIx16 ":%.2" PRIx8 ":%.2" PRIx8 ".%" PRIx8

int main(void)
{
	struct rte_pci_addr pci_addr = { 0, 5, 0, 0 };

	printf(PCI_PRI_FMT "\n",
		       pci_addr.domain,
		       pci_addr.bus, pci_addr.devid,
		       pci_addr.function);

	pci_addr.domain = 0xdeadbeef;
	
	printf(PCI_PRI_FMT "\n",
		       pci_addr.domain,
		       pci_addr.bus, pci_addr.devid,
		       pci_addr.function);
	
	return 0;
}

--- output ---
0000:05:00.0
deadbeef:05:00.0
diff mbox

Patch

diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 0284a6208aa5..e416714b32ff 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -63,7 +63,7 @@  const char *pci_get_sysfs_path(void);
 
 /** Formatting string for PCI device identifier: Ex: 0000:00:01.0 */
 #define PCI_PRI_FMT "%.4" PRIx16 ":%.2" PRIx8 ":%.2" PRIx8 ".%" PRIx8
-#define PCI_PRI_STR_SIZE sizeof("XXXX:XX:XX.X")
+#define PCI_PRI_STR_SIZE sizeof("XXXXXXXX:XX:XX.X")
 
 /** Short formatting string, without domain, for PCI device: Ex: 00:01.0 */
 #define PCI_SHORT_PRI_FMT "%.2" PRIx8 ":%.2" PRIx8 ".%" PRIx8
@@ -112,7 +112,7 @@  struct rte_pci_id {
  * A structure describing the location of a PCI device.
  */
 struct rte_pci_addr {
-	uint16_t domain;                /**< Device domain */
+	uint32_t domain;                /**< Device domain */
 	uint8_t bus;                    /**< Device bus */
 	uint8_t devid;                  /**< Device ID */
 	uint8_t function;               /**< Device function. */
@@ -346,10 +346,10 @@  rte_eal_compare_pci_addr(const struct rte_pci_addr *addr,
 	if ((addr == NULL) || (addr2 == NULL))
 		return -1;
 
-	dev_addr = (addr->domain << 24) | (addr->bus << 16) |
-				(addr->devid << 8) | addr->function;
-	dev_addr2 = (addr2->domain << 24) | (addr2->bus << 16) |
-				(addr2->devid << 8) | addr2->function;
+	dev_addr = ((uint64_t)addr->domain << 24) |
+		(addr->bus << 16) | (addr->devid << 8) | addr->function;
+	dev_addr2 = ((uint64_t)addr2->domain << 24) |
+		(addr2->bus << 16) | (addr2->devid << 8) | addr2->function;
 
 	if (dev_addr > dev_addr2)
 		return 1;