[Skiboot] [PATCH] nvlink: Set a bit in config space to indicate a real PCI device was bound

[Alistair Popple]

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 at 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)
>