Message ID | 1446179790-11505-1-git-send-email-ruscur@russell.cc |
---|---|
State | Superseded |
Headers | show |
Russell, A few minor comments below. Mainly we need to add a version field to the vendor specific capability so that Nvidia can detect that this bit is meant to be present. I would suggest using the upper byte of the vendor specific capability like so: +-----------------+----------------+----------------+----------------+ | Version (0x1) | Cap Length | Next Cap Ptr | Cap ID (0x09) | +-----------------+----------------+----------------+----------------+ On Fri, 30 Oct 2015 15:36:30 Russell Currey wrote: > Previously, it was difficult to determine from userspace whether a NVLink > emulated PCI device was associated with a real PCI device. A bit in > the vendor capabilities section of the config space is now set if a > device was bound. The documentation has been updated to reflect this. > > In addition, the config space offsets are refactored into #defines. > > Signed-off-by: Russell Currey <ruscur@russell.cc> > --- > doc/nvlink.txt | 7 ++++++- > hw/npu.c | 16 ++++++++++++++-- > 2 files changed, 20 insertions(+), 3 deletions(-) > > diff --git a/doc/nvlink.txt b/doc/nvlink.txt > index 5673de4..d5f96c1 100644 > --- a/doc/nvlink.txt > +++ b/doc/nvlink.txt > @@ -66,7 +66,7 @@ Vendor Specific Capabilities > +--------------------------------------------------------------------+ > | Procedure Control Register | > +---------------------------------------------------+----------------+ > -| Reserved | Link Number | > +| Reserved | PCI Dev Flag | Link Number | > +---------------------------------------------------+----------------+ > > Procedure Control Register > @@ -121,6 +121,11 @@ Procedure Status Register > 3 - Procedure aborted. > 4 - Unsupported procedure. > > +PCI Device Flag > + > + This bit is set only if an actual PCI device was bound to this > + emulated device. It would be good if we could call out the actual bit here (ie. bit 0) - we may define other PCI device flag bits in future. Thanks. - Alistair > Link Number > > Physical link number this emulated PCI device is assoicated > diff --git a/hw/npu.c b/hw/npu.c > index c9bc12b..daf3d1f 100644 > --- a/hw/npu.c > +++ b/hw/npu.c > @@ -111,6 +111,13 @@ > static struct npu_dev_cap *npu_dev_find_capability(struct npu_dev *dev, > uint16_t id); > > +#define PCIE_CAP_START 0x40 > +#define PCIE_CAP_END 0x80 > +#define VENDOR_CAP_START 0x80 > +#define VENDOR_CAP_END 0x90 > + > +#define VENDOR_CAP_PCI_DEV_OFFSET 0x0d > + > /* PCI config raw accessors */ > #define NPU_DEV_CFG_NORMAL_RD(d, o, s, v) \ > npu_dev_cfg_read_raw(d, NPU_DEV_CFG_NORMAL, o, s, v) > @@ -546,6 +553,9 @@ static void npu_dev_bind_pci_dev(struct npu_dev *dev) > dev->pd = pci_walk_dev(phb, __npu_dev_bind_pci_dev, dev); > if (dev->pd) { > dev->phb = phb; > + /* Found the device, set the bit in config space */ > + NPU_DEV_CFG_INIT_RO(dev, VENDOR_CAP_START + > + VENDOR_CAP_PCI_DEV_OFFSET, 1, 0x01); > return; > } > } > @@ -1352,11 +1362,13 @@ static void npu_dev_create_capabilities(struct npu_dev *dev) > > /* PCI express capability */ > npu_dev_create_capability(dev, npu_dev_populate_pcie_cap, > - PCI_CFG_CAP_ID_EXP, 0x40, 0x80); > + PCI_CFG_CAP_ID_EXP, PCIE_CAP_START, > + PCIE_CAP_END); > > /* Vendor specific capability */ > npu_dev_create_capability(dev, npu_dev_populate_vendor_cap, > - PCI_CFG_CAP_ID_VENDOR, 0x80, 0x90); > + PCI_CFG_CAP_ID_VENDOR, VENDOR_CAP_START, > + VENDOR_CAP_END); > } > > static void npu_dev_create_cfg(struct npu_dev *dev) >
Russell Currey <ruscur@russell.cc> writes: > Previously, it was difficult to determine from userspace whether a NVLink > emulated PCI device was associated with a real PCI device. A bit in > the vendor capabilities section of the config space is now set if a > device was bound. The documentation has been updated to reflect this. > > In addition, the config space offsets are refactored into #defines. > > Signed-off-by: Russell Currey <ruscur@russell.cc> > --- > doc/nvlink.txt | 7 ++++++- > hw/npu.c | 16 ++++++++++++++-- > 2 files changed, 20 insertions(+), 3 deletions(-) Thanks! Merged to master as of d67d79a.
diff --git a/doc/nvlink.txt b/doc/nvlink.txt index 5673de4..d5f96c1 100644 --- a/doc/nvlink.txt +++ b/doc/nvlink.txt @@ -66,7 +66,7 @@ Vendor Specific Capabilities +--------------------------------------------------------------------+ | Procedure Control Register | +---------------------------------------------------+----------------+ -| Reserved | Link Number | +| Reserved | PCI Dev Flag | Link Number | +---------------------------------------------------+----------------+ Procedure Control Register @@ -121,6 +121,11 @@ Procedure Status Register 3 - Procedure aborted. 4 - Unsupported procedure. +PCI Device Flag + + This bit is set only if an actual PCI device was bound to this + emulated device. + Link Number Physical link number this emulated PCI device is assoicated diff --git a/hw/npu.c b/hw/npu.c index c9bc12b..daf3d1f 100644 --- a/hw/npu.c +++ b/hw/npu.c @@ -111,6 +111,13 @@ static struct npu_dev_cap *npu_dev_find_capability(struct npu_dev *dev, uint16_t id); +#define PCIE_CAP_START 0x40 +#define PCIE_CAP_END 0x80 +#define VENDOR_CAP_START 0x80 +#define VENDOR_CAP_END 0x90 + +#define VENDOR_CAP_PCI_DEV_OFFSET 0x0d + /* PCI config raw accessors */ #define NPU_DEV_CFG_NORMAL_RD(d, o, s, v) \ npu_dev_cfg_read_raw(d, NPU_DEV_CFG_NORMAL, o, s, v) @@ -546,6 +553,9 @@ static void npu_dev_bind_pci_dev(struct npu_dev *dev) dev->pd = pci_walk_dev(phb, __npu_dev_bind_pci_dev, dev); if (dev->pd) { dev->phb = phb; + /* Found the device, set the bit in config space */ + NPU_DEV_CFG_INIT_RO(dev, VENDOR_CAP_START + + VENDOR_CAP_PCI_DEV_OFFSET, 1, 0x01); return; } } @@ -1352,11 +1362,13 @@ static void npu_dev_create_capabilities(struct npu_dev *dev) /* PCI express capability */ npu_dev_create_capability(dev, npu_dev_populate_pcie_cap, - PCI_CFG_CAP_ID_EXP, 0x40, 0x80); + PCI_CFG_CAP_ID_EXP, PCIE_CAP_START, + PCIE_CAP_END); /* Vendor specific capability */ npu_dev_create_capability(dev, npu_dev_populate_vendor_cap, - PCI_CFG_CAP_ID_VENDOR, 0x80, 0x90); + PCI_CFG_CAP_ID_VENDOR, VENDOR_CAP_START, + VENDOR_CAP_END); } static void npu_dev_create_cfg(struct npu_dev *dev)
Previously, it was difficult to determine from userspace whether a NVLink emulated PCI device was associated with a real PCI device. A bit in the vendor capabilities section of the config space is now set if a device was bound. The documentation has been updated to reflect this. In addition, the config space offsets are refactored into #defines. Signed-off-by: Russell Currey <ruscur@russell.cc> --- doc/nvlink.txt | 7 ++++++- hw/npu.c | 16 ++++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-)