[dpdk-dev,v2,2/3] eal: PCI domain should be 32 bits
Checks
Commit Message
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
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.
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
@@ -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;