[kernel,3/3] vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] [10de:1db1] subdriver

POWER9 Witherspoon machines come with 4 or 6 V100 GPUs which are not
pluggable PCIe devices but implement PCIe links for config space and MMIO.
In addition to that the GPUs are interconnected to each other and also
have direct links to the P9 CPU. The links are NVLink2 and provide direct
access to the system RAM for GPUs via NPU (an NVLink2 "proxy" on P9 chip).
These systems also support ATS (address translation services) which is
a part of the NVLink2 prototol. Such GPUs also share on-board RAM
(16GB in tested config) to the system via the same NVLink2 so a CPU has
cache-coherent access to a GPU RAM.

This exports GPU RAM to the userspace as a new PCI region. This
preregisters the new memory as device memory as it might be used for DMA.
This inserts pfns from the fault handler as the GPU memory is not onlined
until the NVIDIA driver is loaded and trained the links so doing this
earlier produces low level errors which we fence in the firmware so
it does not hurt the host system but still better to avoid.

This exports ATSD (Address Translation Shootdown) register of NPU which
allows the guest to invalidate TLB. The register conviniently occupies
a single 64k page. Since NPU maps the GPU memory, it has a "tgt" property
(which is an abbreviated host system bus address). This exports the "tgt"
as a capability so the guest can program it into the GPU so the GPU can
know how to route DMA trafic.

For ATS to work, the nest MMU (an NVIDIA block in a P9 CPU) needs to
know LPID (a logical partition ID or a KVM guest hardware ID in other
words) and PID (a memory context ID of an userspace process, not to be
confused with a linux pid). This assigns a GPU to LPID in the NPU and
this is why this adds a listener for KVM on an IOMMU group. A PID comes
via NVLink from a GPU and NPU uses a PID wildcard to pass it through.

This requires coherent memory and ATSD to be available on the host as
the GPU vendor only supports configurations with both features enabled
and other configurations are known not to work. Because of this and
because of the ways the features are advertised to the host system
(which is a device tree with very platform specific properties),
this requires enabled POWERNV platform.

This hardcodes the NVLink2 support for specific vendor and device IDs
as there is no reliable way of knowing about coherent memory and ATS
support. The GPU has an unique vendor PCIe capability 0x23 but it was
confirmed that it does not provide required information (and it is still
undisclosed what it actually does).

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 drivers/vfio/pci/Makefile           |   1 +
 drivers/vfio/pci/vfio_pci_private.h |   2 +
 include/uapi/linux/vfio.h           |  18 ++
 drivers/vfio/pci/vfio_pci.c         |  37 +++-
 drivers/vfio/pci/vfio_pci_nvlink2.c | 409 ++++++++++++++++++++++++++++++++++++
 drivers/vfio/pci/Kconfig            |   4 +
 6 files changed, 469 insertions(+), 2 deletions(-)
 create mode 100644 drivers/vfio/pci/vfio_pci_nvlink2.c

On Mon, 15 Oct 2018 20:42:33 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> POWER9 Witherspoon machines come with 4 or 6 V100 GPUs which are not
> pluggable PCIe devices but implement PCIe links for config space and MMIO.
> In addition to that the GPUs are interconnected to each other and also
> have direct links to the P9 CPU. The links are NVLink2 and provide direct
> access to the system RAM for GPUs via NPU (an NVLink2 "proxy" on P9 chip).
> These systems also support ATS (address translation services) which is
> a part of the NVLink2 prototol. Such GPUs also share on-board RAM
> (16GB in tested config) to the system via the same NVLink2 so a CPU has
> cache-coherent access to a GPU RAM.
> 
> This exports GPU RAM to the userspace as a new PCI region. This
> preregisters the new memory as device memory as it might be used for DMA.
> This inserts pfns from the fault handler as the GPU memory is not onlined
> until the NVIDIA driver is loaded and trained the links so doing this
> earlier produces low level errors which we fence in the firmware so
> it does not hurt the host system but still better to avoid.
> 
> This exports ATSD (Address Translation Shootdown) register of NPU which
> allows the guest to invalidate TLB. The register conviniently occupies
> a single 64k page. Since NPU maps the GPU memory, it has a "tgt" property
> (which is an abbreviated host system bus address). This exports the "tgt"
> as a capability so the guest can program it into the GPU so the GPU can
> know how to route DMA trafic.

I'm not really following what "tgt" is and why it's needed.  Is the GPU
memory here different than the GPU RAM region above?  Why does the user
need the host system bus address of this "tgt" thing?  Are we not able
to relocate it in guest physical address space, does this shootdown
only work in the host physical address space and therefore we need this
offset?  Please explain, I'm confused.
 
> For ATS to work, the nest MMU (an NVIDIA block in a P9 CPU) needs to
> know LPID (a logical partition ID or a KVM guest hardware ID in other
> words) and PID (a memory context ID of an userspace process, not to be
> confused with a linux pid). This assigns a GPU to LPID in the NPU and
> this is why this adds a listener for KVM on an IOMMU group. A PID comes
> via NVLink from a GPU and NPU uses a PID wildcard to pass it through.
> 
> This requires coherent memory and ATSD to be available on the host as
> the GPU vendor only supports configurations with both features enabled
> and other configurations are known not to work. Because of this and
> because of the ways the features are advertised to the host system
> (which is a device tree with very platform specific properties),
> this requires enabled POWERNV platform.
> 
> This hardcodes the NVLink2 support for specific vendor and device IDs
> as there is no reliable way of knowing about coherent memory and ATS
> support. The GPU has an unique vendor PCIe capability 0x23 but it was
> confirmed that it does not provide required information (and it is still
> undisclosed what it actually does).
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  drivers/vfio/pci/Makefile           |   1 +
>  drivers/vfio/pci/vfio_pci_private.h |   2 +
>  include/uapi/linux/vfio.h           |  18 ++
>  drivers/vfio/pci/vfio_pci.c         |  37 +++-
>  drivers/vfio/pci/vfio_pci_nvlink2.c | 409 ++++++++++++++++++++++++++++++++++++
>  drivers/vfio/pci/Kconfig            |   4 +
>  6 files changed, 469 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/vfio/pci/vfio_pci_nvlink2.c
> 
> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
> index 76d8ec0..9662c06 100644
> --- a/drivers/vfio/pci/Makefile
> +++ b/drivers/vfio/pci/Makefile
> @@ -1,5 +1,6 @@
>  
>  vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
>  vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o
> +vfio-pci-$(CONFIG_VFIO_PCI_NVLINK2) += vfio_pci_nvlink2.o
>  
>  obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
> diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
> index 93c1738..7639241 100644
> --- a/drivers/vfio/pci/vfio_pci_private.h
> +++ b/drivers/vfio/pci/vfio_pci_private.h
> @@ -163,4 +163,6 @@ static inline int vfio_pci_igd_init(struct vfio_pci_device *vdev)
>  	return -ENODEV;
>  }
>  #endif
> +extern int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev);
> +extern int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev);
>  #endif /* VFIO_PCI_PRIVATE_H */
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index f378b98..9e9a8d3 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -303,6 +303,12 @@ struct vfio_region_info_cap_type {
>  #define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG	(2)
>  #define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG	(3)
>  
> +/* NVIDIA GPU NVlink2 RAM */
> +#define VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM	(1)
> +
> +/* IBM NPU NVlink2 ATSD */
> +#define VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD	(1)
> +

Please include some of the description in the commitlog here for
reference.  Also please be explicit that these are vendor defined
regions and note the numerical vendor ID associated with them.

>  /*
>   * The MSIX mappable capability informs that MSIX data of a BAR can be mmapped
>   * which allows direct access to non-MSIX registers which happened to be within
> @@ -313,6 +319,18 @@ struct vfio_region_info_cap_type {
>   */
>  #define VFIO_REGION_INFO_CAP_MSIX_MAPPABLE	3
>  
> +/*
> + * Capability with compressed real address (aka SSA - small system address)
> + * where GPU RAM is mapped on a system bus. Used by a GPU for DMA routing.
> + */
> +#define VFIO_REGION_INFO_CAP_NPU2		4
> +
> +struct vfio_region_info_cap_npu2 {
> +	struct vfio_info_cap_header header;
> +	__u64 tgt;
> +	/* size is defined in VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM */

But this is a capability for the IBM_NVLINK2_ATSD?  What is the
relevance of this comment?  Is this capability relevant to the RAM or
ATSD?

> +};
> +
>  /**
>   * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
>   *				    struct vfio_irq_info)
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 4a3b93e..e9afd43 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -224,6 +224,16 @@ static bool vfio_pci_nointx(struct pci_dev *pdev)
>  	return false;
>  }
>  
> +int __weak vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev)
> +{
> +	return -ENODEV;
> +}
> +
> +int __weak vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev)
> +{
> +	return -ENODEV;
> +}
> +
>  static int vfio_pci_enable(struct vfio_pci_device *vdev)
>  {
>  	struct pci_dev *pdev = vdev->pdev;
> @@ -302,14 +312,37 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
>  		if (ret) {
>  			dev_warn(&vdev->pdev->dev,
>  				 "Failed to setup Intel IGD regions\n");
> -			vfio_pci_disable(vdev);
> -			return ret;
> +			goto disable_exit;
> +		}
> +	}
> +
> +	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
> +	    pdev->device == 0x1db1) {
> +		ret = vfio_pci_nvdia_v100_nvlink2_init(vdev);
> +		if (ret) {
> +			dev_warn(&vdev->pdev->dev,
> +				 "Failed to setup NVIDIA NV2 RAM region\n");
> +			goto disable_exit;
> +		}
> +	}

This device ID is not unique to POWER9 Witherspoon systems, I see your
comment in the commitlog, but this is clearly going to generate a
dev_warn and failure on an x86 system with the same hardware.  Perhaps
this could be masked off with IS_ENABLED(CONFIG_VFIO_PCI_NVLINK2) like
the IGD code above this chunk does?

> +
> +	if (pdev->vendor == PCI_VENDOR_ID_IBM &&
> +			pdev->device == 0x04ea) {
> +		ret = vfio_pci_ibm_npu2_init(vdev);
> +		if (ret) {
> +			dev_warn(&vdev->pdev->dev,
> +					"Failed to setup NVIDIA NV2 ATSD region\n");
> +			goto disable_exit;
>  		}

So the NPU is also actually owned by vfio-pci and assigned to the VM?

>  	}
>  
>  	vfio_pci_probe_mmaps(vdev);
>  
>  	return 0;
> +
> +disable_exit:
> +	vfio_pci_disable(vdev);
> +	return ret;
>  }
>  
>  static void vfio_pci_disable(struct vfio_pci_device *vdev)
> diff --git a/drivers/vfio/pci/vfio_pci_nvlink2.c b/drivers/vfio/pci/vfio_pci_nvlink2.c
> new file mode 100644
> index 0000000..c9d2b55
> --- /dev/null
> +++ b/drivers/vfio/pci/vfio_pci_nvlink2.c
> @@ -0,0 +1,409 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * VFIO PCI NVIDIA Whitherspoon GPU support a.k.a. NVLink2.
> + *
> + * Copyright (C) 2018 IBM Corp.  All rights reserved.
> + *     Author: Alexey Kardashevskiy <aik@ozlabs.ru>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Register an on-GPU RAM region for cacheable access.
> + *
> + * Derived from original vfio_pci_igd.c:
> + * Copyright (C) 2016 Red Hat, Inc.  All rights reserved.
> + *	Author: Alex Williamson <alex.williamson@redhat.com>
> + */
> +
> +#include <linux/io.h>
> +#include <linux/pci.h>
> +#include <linux/uaccess.h>
> +#include <linux/vfio.h>
> +#include <linux/sched/mm.h>
> +#include <linux/mmu_context.h>
> +#include <asm/kvm_ppc.h>
> +#include "vfio_pci_private.h"
> +

Please steal some of your description in the commitlog to document
inline what this is all about.

> +struct vfio_pci_nvgpu_data {
> +	unsigned long gpu_hpa;
> +	unsigned long useraddr;
> +	unsigned long size;
> +	void *base;
> +	struct mm_struct *mm;
> +	struct mm_iommu_table_group_mem_t *mem;
> +	struct pci_dev *gpdev;
> +	struct notifier_block group_notifier;
> +};
> +
> +static size_t vfio_pci_nvgpu_rw(struct vfio_pci_device *vdev,
> +		char __user *buf, size_t count, loff_t *ppos, bool iswrite)
> +{
> +	unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) - VFIO_PCI_NUM_REGIONS;
> +	struct vfio_pci_nvgpu_data *data = vdev->region[i].data;
> +	loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> +
> +	if (pos >= vdev->region[i].size)
> +		return -EINVAL;
> +
> +	count = min(count, (size_t)(vdev->region[i].size - pos));
> +
> +	if (iswrite) {
> +		if (copy_from_user(data->base + pos, buf, count))
> +			return -EFAULT;
> +	} else {
> +		if (copy_to_user(buf, data->base + pos, count))
> +			return -EFAULT;
> +	}
> +	*ppos += count;
> +
> +	return count;
> +}
> +
> +static void vfio_pci_nvgpu_release(struct vfio_pci_device *vdev,
> +		struct vfio_pci_region *region)
> +{
> +	struct vfio_pci_nvgpu_data *data = region->data;
> +	long ret;
> +	struct pci_controller *hose;
> +	struct pci_dev *npdev;
> +
> +	/* If there were any mappings at all... */
> +	if (data->mm) {
> +		ret = mm_iommu_put(data->mm, data->mem);
> +		WARN_ON(ret);
> +
> +		mmdrop(data->mm);
> +	}
> +
> +	vfio_unregister_notifier(&data->gpdev->dev, VFIO_GROUP_NOTIFY,
> +			&data->group_notifier);
> +
> +	npdev = pnv_pci_get_npu_dev(data->gpdev, 0);
> +	hose = pci_bus_to_host(npdev->bus);
> +
> +	pnv_npu2_map_lpar_dev(hose, data->gpdev, 0, MSR_DR | MSR_PR | MSR_HV);
> +
> +	memunmap(data->base);
> +	kfree(data);
> +}
> +
> +static int vfio_pci_nvgpu_mmap_fault(struct vm_fault *vmf)
> +{
> +	int ret;
> +	struct vm_area_struct *vma = vmf->vma;
> +	struct vfio_pci_region *region = vma->vm_private_data;
> +	struct vfio_pci_nvgpu_data *data = region->data;
> +	unsigned long vmf_off = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
> +	unsigned long nv2pg = data->gpu_hpa >> PAGE_SHIFT;
> +	unsigned long vm_pgoff = vma->vm_pgoff &
> +		((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1);
> +	unsigned long pfn = nv2pg + vm_pgoff + vmf_off;
> +
> +	ret = vm_insert_pfn(vma, vmf->address, pfn);
> +	pr_debug("NVLink2: vmf=%lx hpa=%lx ret=%d\n",
> +		 vmf->address, pfn << PAGE_SHIFT, ret);

Tracing would probably be a better option if you intend to keep this.

> +	if (ret)
> +		return VM_FAULT_SIGSEGV;
> +
> +	return VM_FAULT_NOPAGE;
> +}
> +
> +static const struct vm_operations_struct vfio_pci_nvgpu_mmap_vmops = {
> +	.fault = vfio_pci_nvgpu_mmap_fault,
> +};
> +
> +static int vfio_pci_nvgpu_mmap(struct vfio_pci_device *vdev,
> +		struct vfio_pci_region *region, struct vm_area_struct *vma)
> +{
> +	long ret;
> +	struct vfio_pci_nvgpu_data *data = region->data;
> +
> +	if (data->useraddr)
> +		return -EPERM;
> +
> +	if (vma->vm_end - vma->vm_start > data->size)
> +		return -EINVAL;
> +
> +	vma->vm_private_data = region;
> +	vma->vm_flags |= VM_PFNMAP;
> +	vma->vm_ops = &vfio_pci_nvgpu_mmap_vmops;
> +
> +	/*
> +	 * Calling mm_iommu_newdev() here once as the region is not
> +	 * registered yet and therefore right initialization will happen now.
> +	 * Other places will use mm_iommu_find() which returns
> +	 * registered @mem and does not go gup().
> +	 */
> +	data->useraddr = vma->vm_start;
> +	data->mm = current->mm;
> +
> +	atomic_inc(&data->mm->mm_count);
> +	ret = mm_iommu_newdev(data->mm, data->useraddr,
> +			(vma->vm_end - vma->vm_start) >> PAGE_SHIFT,
> +			data->gpu_hpa, &data->mem);
> +
> +	pr_debug("VFIO NVLINK2 mmap: useraddr=%lx hpa=%lx size=%lx ret=%ld\n",
> +			data->useraddr, data->gpu_hpa,
> +			vma->vm_end - vma->vm_start, ret);

Same

> +
> +	return ret;
> +}
> +
> +static const struct vfio_pci_regops vfio_pci_nvgpu_regops = {
> +	.rw = vfio_pci_nvgpu_rw,
> +	.release = vfio_pci_nvgpu_release,
> +	.mmap = vfio_pci_nvgpu_mmap,
> +};
> +
> +static int vfio_pci_nvgpu_group_notifier(struct notifier_block *nb,
> +		unsigned long action, void *opaque)
> +{
> +	struct kvm *kvm = opaque;
> +	struct vfio_pci_nvgpu_data *data = container_of(nb,
> +			struct vfio_pci_nvgpu_data,
> +			group_notifier);
> +
> +	if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
> +		struct pci_controller *hose;
> +		struct pci_dev *npdev;
> +		struct pnv_phb *nphb;
> +
> +		npdev = pnv_pci_get_npu_dev(data->gpdev, 0);
> +		hose = pci_bus_to_host(npdev->bus);
> +		nphb = hose->private_data;
> +
> +		if (!kvm) {
> +			if (pnv_npu2_map_lpar_dev(hose, data->gpdev, 0,
> +					MSR_DR | MSR_PR | MSR_HV))
> +				return NOTIFY_BAD;
> +		} else {
> +			if (pnv_npu2_map_lpar_dev(hose, data->gpdev,
> +					kvm->arch.lpid, MSR_DR | MSR_PR))
> +				return NOTIFY_BAD;
> +		}
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
> +int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev)
> +{
> +	int ret;
> +	u64 reg[2];
> +	struct device_node *npu_node, *mem_node;
> +	struct pci_dev *npu_dev;
> +	struct vfio_pci_nvgpu_data *data;
> +	uint32_t mem_phandle = 0;
> +	unsigned long events = VFIO_GROUP_NOTIFY_SET_KVM;
> +
> +	npu_dev = pnv_pci_get_npu_dev(vdev->pdev, 0);
> +	if (!npu_dev)
> +		return -EINVAL;
> +
> +	npu_node = pci_device_to_OF_node(npu_dev);
> +	if (!npu_node)
> +		return -EINVAL;
> +
> +	if (of_property_read_u32(npu_node, "memory-region", &mem_phandle))
> +		return -EINVAL;
> +
> +	mem_node = of_find_node_by_phandle(mem_phandle);
> +	if (!mem_node)
> +		return -EINVAL;
> +
> +	if (of_property_read_variable_u64_array(mem_node, "reg", reg,
> +				ARRAY_SIZE(reg), ARRAY_SIZE(reg)) !=
> +			ARRAY_SIZE(reg))
> +		return -EINVAL;
> +
> +	data = kzalloc(sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->gpu_hpa = reg[0];
> +	data->size = reg[1];
> +	data->base = memremap(data->gpu_hpa, data->size, MEMREMAP_WB);
> +	if (!data->base) {
> +		ret = -ENOMEM;
> +		goto free_exit;
> +	}
> +
> +	dev_dbg(&vdev->pdev->dev, "%lx..%lx\n", data->gpu_hpa,
> +			data->gpu_hpa + data->size - 1);
> +
> +	data->gpdev = vdev->pdev;
> +	data->group_notifier.notifier_call = vfio_pci_nvgpu_group_notifier;
> +
> +	ret = vfio_register_notifier(&data->gpdev->dev, VFIO_GROUP_NOTIFY,
> +			&events, &data->group_notifier);
> +	if (ret)
> +		goto free_exit;
> +
> +	ret = vfio_pci_register_dev_region(vdev,
> +			PCI_VENDOR_ID_NVIDIA | VFIO_REGION_TYPE_PCI_VENDOR_TYPE,
> +			VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM,
> +			&vfio_pci_nvgpu_regops, data->size,
> +			VFIO_REGION_INFO_FLAG_READ, data);

Clearly WRITE and MMAP flags are supported above as well as READ.

> +	if (ret)
> +		goto unreg_exit;
> +
> +	return 0;
> +unreg_exit:
> +	vfio_unregister_notifier(&data->gpdev->dev, VFIO_GROUP_NOTIFY,
> +			&data->group_notifier);
> +free_exit:
> +	kfree(data);
> +
> +	return ret;
> +}
> +
> +/*
> + * IBM NPU2 bridge
> + */
> +struct vfio_pci_npu2_data {
> +	void *base;
> +	unsigned long mmio_atsd;
> +	unsigned long gpu_tgt;
> +};
> +
> +static size_t vfio_pci_npu2_rw(struct vfio_pci_device *vdev,
> +		char __user *buf, size_t count, loff_t *ppos, bool iswrite)
> +{
> +	unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) - VFIO_PCI_NUM_REGIONS;
> +	struct vfio_pci_npu2_data *data = vdev->region[i].data;
> +	loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> +
> +	if (pos >= vdev->region[i].size)
> +		return -EINVAL;
> +
> +	count = min(count, (size_t)(vdev->region[i].size - pos));
> +
> +	if (iswrite) {
> +		if (copy_from_user(data->base + pos, buf, count))
> +			return -EFAULT;
> +	} else {
> +		if (copy_to_user(buf, data->base + pos, count))
> +			return -EFAULT;
> +	}
> +	*ppos += count;
> +
> +	return count;
> +}
> +
> +static int vfio_pci_npu2_mmap(struct vfio_pci_device *vdev,
> +		struct vfio_pci_region *region, struct vm_area_struct *vma)
> +{
> +	int ret;
> +	struct vfio_pci_npu2_data *data = region->data;
> +	unsigned long req_len = vma->vm_end - vma->vm_start;
> +
> +	if (req_len != PAGE_SIZE)
> +		return -EINVAL;
> +
> +	vma->vm_flags |= VM_PFNMAP;
> +	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> +
> +	ret = remap_pfn_range(vma, vma->vm_start, data->mmio_atsd >> PAGE_SHIFT,
> +			req_len, vma->vm_page_prot);
> +	pr_debug("VFIO NPU2 mmap: %lx %lx size=%lx ret=%d\n",
> +			vma->vm_start, data->mmio_atsd,
> +			vma->vm_end - vma->vm_start, ret);

For commit or convert to tracing?

> +
> +	return ret;
> +}
> +
> +static void vfio_pci_npu2_release(struct vfio_pci_device *vdev,
> +		struct vfio_pci_region *region)
> +{
> +	struct vfio_pci_npu2_data *data = region->data;
> +
> +	memunmap(data->base);
> +	kfree(data);
> +}
> +
> +static int vfio_pci_npu2_add_capability(struct vfio_pci_device *vdev,
> +		struct vfio_pci_region *region, struct vfio_info_cap *caps)
> +{
> +	struct vfio_pci_npu2_data *data = region->data;
> +	struct vfio_region_info_cap_npu2 cap;
> +
> +	cap.header.id = VFIO_REGION_INFO_CAP_NPU2;
> +	cap.header.version = 1;
> +	cap.tgt = data->gpu_tgt;
> +
> +	return vfio_info_add_capability(caps, &cap.header, sizeof(cap));
> +}
> +
> +static const struct vfio_pci_regops vfio_pci_npu2_regops = {
> +	.rw = vfio_pci_npu2_rw,
> +	.mmap = vfio_pci_npu2_mmap,
> +	.release = vfio_pci_npu2_release,
> +	.add_capability = vfio_pci_npu2_add_capability,
> +};
> +
> +int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev)
> +{
> +	int ret;
> +	struct vfio_pci_npu2_data *data;
> +	struct device_node *nvlink_dn;
> +	u32 nvlink_index = 0;
> +	struct pci_dev *npdev = vdev->pdev;
> +	struct device_node *npu_node = pci_device_to_OF_node(npdev);
> +	struct pci_controller *hose = pci_bus_to_host(npdev->bus);
> +	u64 mmio_atsd = 0;
> +	u64 tgt = 0;
> +
> +	/*
> +	 * NPU2 normally has 8 ATSD registers (for concurrency) and 6 links
> +	 * so we can allocate one register per link.
> +	 * Since skiboot only exposes one (a bug), use this as a fallback
> +	 * which is safe as we do not split GPUs attached to the same NPU.
> +	 */
> +	nvlink_dn = of_parse_phandle(npdev->dev.of_node, "ibm,nvlink", 0);
> +	if (WARN_ON(of_property_read_u32(nvlink_dn, "ibm,npu-link-index",
> +			&nvlink_index)))
> +		return -ENODEV;
> +
> +	if (of_property_read_u64_index(hose->dn, "ibm,mmio-atsd", nvlink_index,
> +			&mmio_atsd)) {
> +		if (of_property_read_u64_index(hose->dn, "ibm,mmio-atsd", 0,
> +					&mmio_atsd)) {
> +			dev_warn(&vdev->pdev->dev, "No ATSD found\n");
> +			return -EFAULT;
> +		}
> +		dev_warn(&vdev->pdev->dev, "Fallback to ATSD#0\n");
> +	}
> +
> +	if (of_property_read_u64(npu_node, "ibm,device-tgt-addr", &tgt)) {
> +		dev_warn(&vdev->pdev->dev, "No ibm,device-tgt-addr found\n");
> +		return -EFAULT;
> +	}
> +
> +	data = kzalloc(sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->mmio_atsd = mmio_atsd;
> +	data->gpu_tgt = tgt;
> +	data->base = memremap(data->mmio_atsd, SZ_64K, MEMREMAP_WT);
> +	if (!data->base) {
> +		ret = -ENOMEM;
> +		goto free_exit;
> +	}
> +
> +	ret = vfio_pci_register_dev_region(vdev,
> +			PCI_VENDOR_ID_IBM | VFIO_REGION_TYPE_PCI_VENDOR_TYPE,
> +			VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD,
> +			&vfio_pci_npu2_regops, PAGE_SIZE,
> +			VFIO_REGION_INFO_FLAG_READ, data);

READ|WRITE|MMAP

> +	if (ret)
> +		goto free_exit;
> +
> +	return 0;
> +
> +free_exit:
> +	kfree(data);
> +
> +	return ret;
> +}
> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
> index 42dc1d3..1a58979 100644
> --- a/drivers/vfio/pci/Kconfig
> +++ b/drivers/vfio/pci/Kconfig
> @@ -38,3 +38,7 @@ config VFIO_PCI_IGD
>  	  and LPC bridge config space.
>  
>  	  To enable Intel IGD assignment through vfio-pci, say Y.
> +
> +config VFIO_PCI_NVLINK2
> +	bool "VFIO PCI support for P9 Witherspoon machine with NVIDIA V100 GPUs"
> +	depends on VFIO_PCI && PPC_POWERNV

On 17/10/2018 06:08, Alex Williamson wrote:
> On Mon, 15 Oct 2018 20:42:33 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
>> POWER9 Witherspoon machines come with 4 or 6 V100 GPUs which are not
>> pluggable PCIe devices but implement PCIe links for config space and MMIO.
>> In addition to that the GPUs are interconnected to each other and also
>> have direct links to the P9 CPU. The links are NVLink2 and provide direct
>> access to the system RAM for GPUs via NPU (an NVLink2 "proxy" on P9 chip).
>> These systems also support ATS (address translation services) which is
>> a part of the NVLink2 prototol. Such GPUs also share on-board RAM
>> (16GB in tested config) to the system via the same NVLink2 so a CPU has
>> cache-coherent access to a GPU RAM.
>>
>> This exports GPU RAM to the userspace as a new PCI region. This
>> preregisters the new memory as device memory as it might be used for DMA.
>> This inserts pfns from the fault handler as the GPU memory is not onlined
>> until the NVIDIA driver is loaded and trained the links so doing this
>> earlier produces low level errors which we fence in the firmware so
>> it does not hurt the host system but still better to avoid.
>>
>> This exports ATSD (Address Translation Shootdown) register of NPU which
>> allows the guest to invalidate TLB. The register conviniently occupies
>> a single 64k page. Since NPU maps the GPU memory, it has a "tgt" property
>> (which is an abbreviated host system bus address). This exports the "tgt"
>> as a capability so the guest can program it into the GPU so the GPU can
>> know how to route DMA trafic.
> 
> I'm not really following what "tgt" is and why it's needed.  Is the GPU
> memory here different than the GPU RAM region above?  Why does the user
> need the host system bus address of this "tgt" thing?  Are we not able
> to relocate it in guest physical address space, does this shootdown
> only work in the host physical address space and therefore we need this
> offset?  Please explain, I'm confused.


This "tgt" is made of:
- "memory select" (bits 45, 46)
- "group select" (bits 43, 44)
- "chip select" (bit 42)
- chip internal address (bits 0..41)

These are internal to GPU and this is where GPU RAM is mapped into the
GPU's real space, this fits 46 bits.

On POWER9 CPU the bits are different and higher so the same memory is
mapped higher on P9 CPU. Just because we can map it higher, I guess.

So it is not exactly the address but this provides the exact physical
location of the memory.

We have a group of 3 interconnected GPUs, they got their own
memory/group/chip numbers. The GPUs use ATS service to translate
userspace to physical (host or guest) addresses. Now a GPU needs to know
which specific link to use for a specific physical address, in other
words what this physical address belongs to - a CPU or one of GPUs. This
is when "tgt" is used by the GPU hardware.

A GPU could run all the DMA trafic via the system bus indeed, just not
as fast.

I am also struggling here and adding an Nvidia person in cc: (I should
have done that when I posted the patches, my bad) to correct when/if I
am wrong.



>  
>> For ATS to work, the nest MMU (an NVIDIA block in a P9 CPU) needs to
>> know LPID (a logical partition ID or a KVM guest hardware ID in other
>> words) and PID (a memory context ID of an userspace process, not to be
>> confused with a linux pid). This assigns a GPU to LPID in the NPU and
>> this is why this adds a listener for KVM on an IOMMU group. A PID comes
>> via NVLink from a GPU and NPU uses a PID wildcard to pass it through.
>>
>> This requires coherent memory and ATSD to be available on the host as
>> the GPU vendor only supports configurations with both features enabled
>> and other configurations are known not to work. Because of this and
>> because of the ways the features are advertised to the host system
>> (which is a device tree with very platform specific properties),
>> this requires enabled POWERNV platform.
>>
>> This hardcodes the NVLink2 support for specific vendor and device IDs
>> as there is no reliable way of knowing about coherent memory and ATS
>> support. The GPU has an unique vendor PCIe capability 0x23 but it was
>> confirmed that it does not provide required information (and it is still
>> undisclosed what it actually does).
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>  drivers/vfio/pci/Makefile           |   1 +
>>  drivers/vfio/pci/vfio_pci_private.h |   2 +
>>  include/uapi/linux/vfio.h           |  18 ++
>>  drivers/vfio/pci/vfio_pci.c         |  37 +++-
>>  drivers/vfio/pci/vfio_pci_nvlink2.c | 409 ++++++++++++++++++++++++++++++++++++
>>  drivers/vfio/pci/Kconfig            |   4 +
>>  6 files changed, 469 insertions(+), 2 deletions(-)
>>  create mode 100644 drivers/vfio/pci/vfio_pci_nvlink2.c
>>
>> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
>> index 76d8ec0..9662c06 100644
>> --- a/drivers/vfio/pci/Makefile
>> +++ b/drivers/vfio/pci/Makefile
>> @@ -1,5 +1,6 @@
>>  
>>  vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
>>  vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o
>> +vfio-pci-$(CONFIG_VFIO_PCI_NVLINK2) += vfio_pci_nvlink2.o
>>  
>>  obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
>> diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
>> index 93c1738..7639241 100644
>> --- a/drivers/vfio/pci/vfio_pci_private.h
>> +++ b/drivers/vfio/pci/vfio_pci_private.h
>> @@ -163,4 +163,6 @@ static inline int vfio_pci_igd_init(struct vfio_pci_device *vdev)
>>  	return -ENODEV;
>>  }
>>  #endif
>> +extern int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev);
>> +extern int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev);
>>  #endif /* VFIO_PCI_PRIVATE_H */
>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>> index f378b98..9e9a8d3 100644
>> --- a/include/uapi/linux/vfio.h
>> +++ b/include/uapi/linux/vfio.h
>> @@ -303,6 +303,12 @@ struct vfio_region_info_cap_type {
>>  #define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG	(2)
>>  #define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG	(3)
>>  
>> +/* NVIDIA GPU NVlink2 RAM */
>> +#define VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM	(1)
>> +
>> +/* IBM NPU NVlink2 ATSD */
>> +#define VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD	(1)
>> +
> 
> Please include some of the description in the commitlog here for
> reference.  Also please be explicit that these are vendor defined
> regions and note the numerical vendor ID associated with them.

These are PCI region subtypes which are not from any PCI spec and which
we define as we like, are not they all "vendor"?


> 
>>  /*
>>   * The MSIX mappable capability informs that MSIX data of a BAR can be mmapped
>>   * which allows direct access to non-MSIX registers which happened to be within
>> @@ -313,6 +319,18 @@ struct vfio_region_info_cap_type {
>>   */
>>  #define VFIO_REGION_INFO_CAP_MSIX_MAPPABLE	3
>>  
>> +/*
>> + * Capability with compressed real address (aka SSA - small system address)
>> + * where GPU RAM is mapped on a system bus. Used by a GPU for DMA routing.
>> + */
>> +#define VFIO_REGION_INFO_CAP_NPU2		4
>> +
>> +struct vfio_region_info_cap_npu2 {
>> +	struct vfio_info_cap_header header;
>> +	__u64 tgt;
>> +	/* size is defined in VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM */
> 
> But this is a capability for the IBM_NVLINK2_ATSD?  What is the
> relevance of this comment?  Is this capability relevant to the RAM or
> ATSD?

It is relevant to NPU (NVLink host bus adapter of POWER9) which maps the
GPU RAM to the system bus and acts as a proxy to nestMMU (NVIDIA's unit
in POWER9 CPU) for ATS/ATSD services so it is a property of NPU. But
then one might ask "wait, here is the address, where is the size then",
 hence the comment...


> 
>> +};
>> +
>>  /**
>>   * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
>>   *				    struct vfio_irq_info)
>> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
>> index 4a3b93e..e9afd43 100644
>> --- a/drivers/vfio/pci/vfio_pci.c
>> +++ b/drivers/vfio/pci/vfio_pci.c
>> @@ -224,6 +224,16 @@ static bool vfio_pci_nointx(struct pci_dev *pdev)
>>  	return false;
>>  }
>>  
>> +int __weak vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev)
>> +{
>> +	return -ENODEV;
>> +}
>> +
>> +int __weak vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev)
>> +{
>> +	return -ENODEV;
>> +}
>> +
>>  static int vfio_pci_enable(struct vfio_pci_device *vdev)
>>  {
>>  	struct pci_dev *pdev = vdev->pdev;
>> @@ -302,14 +312,37 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
>>  		if (ret) {
>>  			dev_warn(&vdev->pdev->dev,
>>  				 "Failed to setup Intel IGD regions\n");
>> -			vfio_pci_disable(vdev);
>> -			return ret;
>> +			goto disable_exit;
>> +		}
>> +	}
>> +
>> +	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
>> +	    pdev->device == 0x1db1) {
>> +		ret = vfio_pci_nvdia_v100_nvlink2_init(vdev);
>> +		if (ret) {
>> +			dev_warn(&vdev->pdev->dev,
>> +				 "Failed to setup NVIDIA NV2 RAM region\n");
>> +			goto disable_exit;
>> +		}
>> +	}
> 
> This device ID is not unique to POWER9 Witherspoon systems, I see your
> comment in the commitlog, but this is clearly going to generate a
> dev_warn and failure on an x86 system with the same hardware.  Perhaps
> this could be masked off with IS_ENABLED(CONFIG_VFIO_PCI_NVLINK2) like
> the IGD code above this chunk does?

Right, will fix.


>> +
>> +	if (pdev->vendor == PCI_VENDOR_ID_IBM &&
>> +			pdev->device == 0x04ea) {
>> +		ret = vfio_pci_ibm_npu2_init(vdev);
>> +		if (ret) {
>> +			dev_warn(&vdev->pdev->dev,
>> +					"Failed to setup NVIDIA NV2 ATSD region\n");
>> +			goto disable_exit;
>>  		}
> 
> So the NPU is also actually owned by vfio-pci and assigned to the VM?

Yes. On a running system it looks like:

0007:00:00.0 Bridge: IBM Device 04ea (rev 01)
0007:00:00.1 Bridge: IBM Device 04ea (rev 01)
0007:00:01.0 Bridge: IBM Device 04ea (rev 01)
0007:00:01.1 Bridge: IBM Device 04ea (rev 01)
0007:00:02.0 Bridge: IBM Device 04ea (rev 01)
0007:00:02.1 Bridge: IBM Device 04ea (rev 01)
0035:00:00.0 PCI bridge: IBM Device 04c1
0035:01:00.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
0035:02:04.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
0035:02:05.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
0035:02:0d.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
0035:03:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
(rev a1
0035:04:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
(rev a1)
0035:05:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
(rev a1)

One "IBM Device" bridge represents one NVLink2, i.e. a piece of NPU.
They all and 3 GPUs go to the same IOMMU group and get passed through to
a guest.

The entire NPU does not have representation via sysfs as a whole though.

> 
>>  	}
>>  
>>  	vfio_pci_probe_mmaps(vdev);
>>  
>>  	return 0;
>> +
>> +disable_exit:
>> +	vfio_pci_disable(vdev);
>> +	return ret;
>>  }
>>  
>>  static void vfio_pci_disable(struct vfio_pci_device *vdev)
>> diff --git a/drivers/vfio/pci/vfio_pci_nvlink2.c b/drivers/vfio/pci/vfio_pci_nvlink2.c
>> new file mode 100644
>> index 0000000..c9d2b55
>> --- /dev/null
>> +++ b/drivers/vfio/pci/vfio_pci_nvlink2.c
>> @@ -0,0 +1,409 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * VFIO PCI NVIDIA Whitherspoon GPU support a.k.a. NVLink2.
>> + *
>> + * Copyright (C) 2018 IBM Corp.  All rights reserved.
>> + *     Author: Alexey Kardashevskiy <aik@ozlabs.ru>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * Register an on-GPU RAM region for cacheable access.
>> + *
>> + * Derived from original vfio_pci_igd.c:
>> + * Copyright (C) 2016 Red Hat, Inc.  All rights reserved.
>> + *	Author: Alex Williamson <alex.williamson@redhat.com>
>> + */
>> +
>> +#include <linux/io.h>
>> +#include <linux/pci.h>
>> +#include <linux/uaccess.h>
>> +#include <linux/vfio.h>
>> +#include <linux/sched/mm.h>
>> +#include <linux/mmu_context.h>
>> +#include <asm/kvm_ppc.h>
>> +#include "vfio_pci_private.h"
>> +
> 
> Please steal some of your description in the commitlog to document
> inline what this is all about.
> 
>> +struct vfio_pci_nvgpu_data {
>> +	unsigned long gpu_hpa;
>> +	unsigned long useraddr;
>> +	unsigned long size;
>> +	void *base;
>> +	struct mm_struct *mm;
>> +	struct mm_iommu_table_group_mem_t *mem;
>> +	struct pci_dev *gpdev;
>> +	struct notifier_block group_notifier;
>> +};
>> +
>> +static size_t vfio_pci_nvgpu_rw(struct vfio_pci_device *vdev,
>> +		char __user *buf, size_t count, loff_t *ppos, bool iswrite)
>> +{
>> +	unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) - VFIO_PCI_NUM_REGIONS;
>> +	struct vfio_pci_nvgpu_data *data = vdev->region[i].data;
>> +	loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
>> +
>> +	if (pos >= vdev->region[i].size)
>> +		return -EINVAL;
>> +
>> +	count = min(count, (size_t)(vdev->region[i].size - pos));
>> +
>> +	if (iswrite) {
>> +		if (copy_from_user(data->base + pos, buf, count))
>> +			return -EFAULT;
>> +	} else {
>> +		if (copy_to_user(buf, data->base + pos, count))
>> +			return -EFAULT;
>> +	}
>> +	*ppos += count;
>> +
>> +	return count;
>> +}
>> +
>> +static void vfio_pci_nvgpu_release(struct vfio_pci_device *vdev,
>> +		struct vfio_pci_region *region)
>> +{
>> +	struct vfio_pci_nvgpu_data *data = region->data;
>> +	long ret;
>> +	struct pci_controller *hose;
>> +	struct pci_dev *npdev;
>> +
>> +	/* If there were any mappings at all... */
>> +	if (data->mm) {
>> +		ret = mm_iommu_put(data->mm, data->mem);
>> +		WARN_ON(ret);
>> +
>> +		mmdrop(data->mm);
>> +	}
>> +
>> +	vfio_unregister_notifier(&data->gpdev->dev, VFIO_GROUP_NOTIFY,
>> +			&data->group_notifier);
>> +
>> +	npdev = pnv_pci_get_npu_dev(data->gpdev, 0);
>> +	hose = pci_bus_to_host(npdev->bus);
>> +
>> +	pnv_npu2_map_lpar_dev(hose, data->gpdev, 0, MSR_DR | MSR_PR | MSR_HV);
>> +
>> +	memunmap(data->base);
>> +	kfree(data);
>> +}
>> +
>> +static int vfio_pci_nvgpu_mmap_fault(struct vm_fault *vmf)
>> +{
>> +	int ret;
>> +	struct vm_area_struct *vma = vmf->vma;
>> +	struct vfio_pci_region *region = vma->vm_private_data;
>> +	struct vfio_pci_nvgpu_data *data = region->data;
>> +	unsigned long vmf_off = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
>> +	unsigned long nv2pg = data->gpu_hpa >> PAGE_SHIFT;
>> +	unsigned long vm_pgoff = vma->vm_pgoff &
>> +		((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1);
>> +	unsigned long pfn = nv2pg + vm_pgoff + vmf_off;
>> +
>> +	ret = vm_insert_pfn(vma, vmf->address, pfn);
>> +	pr_debug("NVLink2: vmf=%lx hpa=%lx ret=%d\n",
>> +		 vmf->address, pfn << PAGE_SHIFT, ret);
> 
> Tracing would probably be a better option if you intend to keep this.


Ok, tracing it will be here and later.


> 
>> +	if (ret)
>> +		return VM_FAULT_SIGSEGV;
>> +
>> +	return VM_FAULT_NOPAGE;
>> +}
>> +
>> +static const struct vm_operations_struct vfio_pci_nvgpu_mmap_vmops = {
>> +	.fault = vfio_pci_nvgpu_mmap_fault,
>> +};
>> +
>> +static int vfio_pci_nvgpu_mmap(struct vfio_pci_device *vdev,
>> +		struct vfio_pci_region *region, struct vm_area_struct *vma)
>> +{
>> +	long ret;
>> +	struct vfio_pci_nvgpu_data *data = region->data;
>> +
>> +	if (data->useraddr)
>> +		return -EPERM;
>> +
>> +	if (vma->vm_end - vma->vm_start > data->size)
>> +		return -EINVAL;
>> +
>> +	vma->vm_private_data = region;
>> +	vma->vm_flags |= VM_PFNMAP;
>> +	vma->vm_ops = &vfio_pci_nvgpu_mmap_vmops;
>> +
>> +	/*
>> +	 * Calling mm_iommu_newdev() here once as the region is not
>> +	 * registered yet and therefore right initialization will happen now.
>> +	 * Other places will use mm_iommu_find() which returns
>> +	 * registered @mem and does not go gup().
>> +	 */
>> +	data->useraddr = vma->vm_start;
>> +	data->mm = current->mm;
>> +
>> +	atomic_inc(&data->mm->mm_count);
>> +	ret = mm_iommu_newdev(data->mm, data->useraddr,
>> +			(vma->vm_end - vma->vm_start) >> PAGE_SHIFT,
>> +			data->gpu_hpa, &data->mem);
>> +
>> +	pr_debug("VFIO NVLINK2 mmap: useraddr=%lx hpa=%lx size=%lx ret=%ld\n",
>> +			data->useraddr, data->gpu_hpa,
>> +			vma->vm_end - vma->vm_start, ret);
> 
> Same
> 
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct vfio_pci_regops vfio_pci_nvgpu_regops = {
>> +	.rw = vfio_pci_nvgpu_rw,
>> +	.release = vfio_pci_nvgpu_release,
>> +	.mmap = vfio_pci_nvgpu_mmap,
>> +};
>> +
>> +static int vfio_pci_nvgpu_group_notifier(struct notifier_block *nb,
>> +		unsigned long action, void *opaque)
>> +{
>> +	struct kvm *kvm = opaque;
>> +	struct vfio_pci_nvgpu_data *data = container_of(nb,
>> +			struct vfio_pci_nvgpu_data,
>> +			group_notifier);
>> +
>> +	if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
>> +		struct pci_controller *hose;
>> +		struct pci_dev *npdev;
>> +		struct pnv_phb *nphb;
>> +
>> +		npdev = pnv_pci_get_npu_dev(data->gpdev, 0);
>> +		hose = pci_bus_to_host(npdev->bus);
>> +		nphb = hose->private_data;
>> +
>> +		if (!kvm) {
>> +			if (pnv_npu2_map_lpar_dev(hose, data->gpdev, 0,
>> +					MSR_DR | MSR_PR | MSR_HV))
>> +				return NOTIFY_BAD;
>> +		} else {
>> +			if (pnv_npu2_map_lpar_dev(hose, data->gpdev,
>> +					kvm->arch.lpid, MSR_DR | MSR_PR))
>> +				return NOTIFY_BAD;
>> +		}
>> +	}
>> +
>> +	return NOTIFY_OK;
>> +}
>> +
>> +int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev)
>> +{
>> +	int ret;
>> +	u64 reg[2];
>> +	struct device_node *npu_node, *mem_node;
>> +	struct pci_dev *npu_dev;
>> +	struct vfio_pci_nvgpu_data *data;
>> +	uint32_t mem_phandle = 0;
>> +	unsigned long events = VFIO_GROUP_NOTIFY_SET_KVM;
>> +
>> +	npu_dev = pnv_pci_get_npu_dev(vdev->pdev, 0);
>> +	if (!npu_dev)
>> +		return -EINVAL;
>> +
>> +	npu_node = pci_device_to_OF_node(npu_dev);
>> +	if (!npu_node)
>> +		return -EINVAL;
>> +
>> +	if (of_property_read_u32(npu_node, "memory-region", &mem_phandle))
>> +		return -EINVAL;
>> +
>> +	mem_node = of_find_node_by_phandle(mem_phandle);
>> +	if (!mem_node)
>> +		return -EINVAL;
>> +
>> +	if (of_property_read_variable_u64_array(mem_node, "reg", reg,
>> +				ARRAY_SIZE(reg), ARRAY_SIZE(reg)) !=
>> +			ARRAY_SIZE(reg))
>> +		return -EINVAL;
>> +
>> +	data = kzalloc(sizeof(*data), GFP_KERNEL);
>> +	if (!data)
>> +		return -ENOMEM;
>> +
>> +	data->gpu_hpa = reg[0];
>> +	data->size = reg[1];
>> +	data->base = memremap(data->gpu_hpa, data->size, MEMREMAP_WB);
>> +	if (!data->base) {
>> +		ret = -ENOMEM;
>> +		goto free_exit;
>> +	}
>> +
>> +	dev_dbg(&vdev->pdev->dev, "%lx..%lx\n", data->gpu_hpa,
>> +			data->gpu_hpa + data->size - 1);
>> +
>> +	data->gpdev = vdev->pdev;
>> +	data->group_notifier.notifier_call = vfio_pci_nvgpu_group_notifier;
>> +
>> +	ret = vfio_register_notifier(&data->gpdev->dev, VFIO_GROUP_NOTIFY,
>> +			&events, &data->group_notifier);
>> +	if (ret)
>> +		goto free_exit;
>> +
>> +	ret = vfio_pci_register_dev_region(vdev,
>> +			PCI_VENDOR_ID_NVIDIA | VFIO_REGION_TYPE_PCI_VENDOR_TYPE,
>> +			VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM,
>> +			&vfio_pci_nvgpu_regops, data->size,
>> +			VFIO_REGION_INFO_FLAG_READ, data);
> 
> Clearly WRITE and MMAP flags are supported above as well as READ.


Ah, right. These are informational flags so I did not hit problems
before. I should probably check for these in "[PATCH kernel 1/3]
vfio_pci: Allow mapping extra regions" and block mmap if MMAP is not set
then.

> 
>> +	if (ret)
>> +		goto unreg_exit;
>> +
>> +	return 0;
>> +unreg_exit:
>> +	vfio_unregister_notifier(&data->gpdev->dev, VFIO_GROUP_NOTIFY,
>> +			&data->group_notifier);
>> +free_exit:
>> +	kfree(data);
>> +
>> +	return ret;
>> +}
>> +
>> +/*
>> + * IBM NPU2 bridge
>> + */
>> +struct vfio_pci_npu2_data {
>> +	void *base;
>> +	unsigned long mmio_atsd;
>> +	unsigned long gpu_tgt;
>> +};
>> +
>> +static size_t vfio_pci_npu2_rw(struct vfio_pci_device *vdev,
>> +		char __user *buf, size_t count, loff_t *ppos, bool iswrite)
>> +{
>> +	unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) - VFIO_PCI_NUM_REGIONS;
>> +	struct vfio_pci_npu2_data *data = vdev->region[i].data;
>> +	loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
>> +
>> +	if (pos >= vdev->region[i].size)
>> +		return -EINVAL;
>> +
>> +	count = min(count, (size_t)(vdev->region[i].size - pos));
>> +
>> +	if (iswrite) {
>> +		if (copy_from_user(data->base + pos, buf, count))
>> +			return -EFAULT;
>> +	} else {
>> +		if (copy_to_user(buf, data->base + pos, count))
>> +			return -EFAULT;
>> +	}
>> +	*ppos += count;
>> +
>> +	return count;
>> +}
>> +
>> +static int vfio_pci_npu2_mmap(struct vfio_pci_device *vdev,
>> +		struct vfio_pci_region *region, struct vm_area_struct *vma)
>> +{
>> +	int ret;
>> +	struct vfio_pci_npu2_data *data = region->data;
>> +	unsigned long req_len = vma->vm_end - vma->vm_start;
>> +
>> +	if (req_len != PAGE_SIZE)
>> +		return -EINVAL;
>> +
>> +	vma->vm_flags |= VM_PFNMAP;
>> +	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
>> +
>> +	ret = remap_pfn_range(vma, vma->vm_start, data->mmio_atsd >> PAGE_SHIFT,
>> +			req_len, vma->vm_page_prot);
>> +	pr_debug("VFIO NPU2 mmap: %lx %lx size=%lx ret=%d\n",
>> +			vma->vm_start, data->mmio_atsd,
>> +			vma->vm_end - vma->vm_start, ret);
> 
> For commit or convert to tracing?
> 
>> +
>> +	return ret;
>> +}
>> +
>> +static void vfio_pci_npu2_release(struct vfio_pci_device *vdev,
>> +		struct vfio_pci_region *region)
>> +{
>> +	struct vfio_pci_npu2_data *data = region->data;
>> +
>> +	memunmap(data->base);
>> +	kfree(data);
>> +}
>> +
>> +static int vfio_pci_npu2_add_capability(struct vfio_pci_device *vdev,
>> +		struct vfio_pci_region *region, struct vfio_info_cap *caps)
>> +{
>> +	struct vfio_pci_npu2_data *data = region->data;
>> +	struct vfio_region_info_cap_npu2 cap;
>> +
>> +	cap.header.id = VFIO_REGION_INFO_CAP_NPU2;
>> +	cap.header.version = 1;
>> +	cap.tgt = data->gpu_tgt;
>> +
>> +	return vfio_info_add_capability(caps, &cap.header, sizeof(cap));
>> +}
>> +
>> +static const struct vfio_pci_regops vfio_pci_npu2_regops = {
>> +	.rw = vfio_pci_npu2_rw,
>> +	.mmap = vfio_pci_npu2_mmap,
>> +	.release = vfio_pci_npu2_release,
>> +	.add_capability = vfio_pci_npu2_add_capability,
>> +};
>> +
>> +int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev)
>> +{
>> +	int ret;
>> +	struct vfio_pci_npu2_data *data;
>> +	struct device_node *nvlink_dn;
>> +	u32 nvlink_index = 0;
>> +	struct pci_dev *npdev = vdev->pdev;
>> +	struct device_node *npu_node = pci_device_to_OF_node(npdev);
>> +	struct pci_controller *hose = pci_bus_to_host(npdev->bus);
>> +	u64 mmio_atsd = 0;
>> +	u64 tgt = 0;
>> +
>> +	/*
>> +	 * NPU2 normally has 8 ATSD registers (for concurrency) and 6 links
>> +	 * so we can allocate one register per link.
>> +	 * Since skiboot only exposes one (a bug), use this as a fallback
>> +	 * which is safe as we do not split GPUs attached to the same NPU.
>> +	 */
>> +	nvlink_dn = of_parse_phandle(npdev->dev.of_node, "ibm,nvlink", 0);
>> +	if (WARN_ON(of_property_read_u32(nvlink_dn, "ibm,npu-link-index",
>> +			&nvlink_index)))
>> +		return -ENODEV;
>> +
>> +	if (of_property_read_u64_index(hose->dn, "ibm,mmio-atsd", nvlink_index,
>> +			&mmio_atsd)) {
>> +		if (of_property_read_u64_index(hose->dn, "ibm,mmio-atsd", 0,
>> +					&mmio_atsd)) {
>> +			dev_warn(&vdev->pdev->dev, "No ATSD found\n");
>> +			return -EFAULT;
>> +		}
>> +		dev_warn(&vdev->pdev->dev, "Fallback to ATSD#0\n");
>> +	}
>> +
>> +	if (of_property_read_u64(npu_node, "ibm,device-tgt-addr", &tgt)) {
>> +		dev_warn(&vdev->pdev->dev, "No ibm,device-tgt-addr found\n");
>> +		return -EFAULT;
>> +	}
>> +
>> +	data = kzalloc(sizeof(*data), GFP_KERNEL);
>> +	if (!data)
>> +		return -ENOMEM;
>> +
>> +	data->mmio_atsd = mmio_atsd;
>> +	data->gpu_tgt = tgt;
>> +	data->base = memremap(data->mmio_atsd, SZ_64K, MEMREMAP_WT);
>> +	if (!data->base) {
>> +		ret = -ENOMEM;
>> +		goto free_exit;
>> +	}
>> +
>> +	ret = vfio_pci_register_dev_region(vdev,
>> +			PCI_VENDOR_ID_IBM | VFIO_REGION_TYPE_PCI_VENDOR_TYPE,
>> +			VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD,
>> +			&vfio_pci_npu2_regops, PAGE_SIZE,
>> +			VFIO_REGION_INFO_FLAG_READ, data);
> 
> READ|WRITE|MMAP
> 
>> +	if (ret)
>> +		goto free_exit;
>> +
>> +	return 0;
>> +
>> +free_exit:
>> +	kfree(data);
>> +
>> +	return ret;
>> +}
>> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
>> index 42dc1d3..1a58979 100644
>> --- a/drivers/vfio/pci/Kconfig
>> +++ b/drivers/vfio/pci/Kconfig
>> @@ -38,3 +38,7 @@ config VFIO_PCI_IGD
>>  	  and LPC bridge config space.
>>  
>>  	  To enable Intel IGD assignment through vfio-pci, say Y.
>> +
>> +config VFIO_PCI_NVLINK2
>> +	bool "VFIO PCI support for P9 Witherspoon machine with NVIDIA V100 GPUs"
>> +	depends on VFIO_PCI && PPC_POWERNV
>

On Wed, 17 Oct 2018 12:19:20 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> On 17/10/2018 06:08, Alex Williamson wrote:
> > On Mon, 15 Oct 2018 20:42:33 +1100
> > Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> >   
> >> POWER9 Witherspoon machines come with 4 or 6 V100 GPUs which are not
> >> pluggable PCIe devices but implement PCIe links for config space and MMIO.
> >> In addition to that the GPUs are interconnected to each other and also
> >> have direct links to the P9 CPU. The links are NVLink2 and provide direct
> >> access to the system RAM for GPUs via NPU (an NVLink2 "proxy" on P9 chip).
> >> These systems also support ATS (address translation services) which is
> >> a part of the NVLink2 prototol. Such GPUs also share on-board RAM
> >> (16GB in tested config) to the system via the same NVLink2 so a CPU has
> >> cache-coherent access to a GPU RAM.
> >>
> >> This exports GPU RAM to the userspace as a new PCI region. This
> >> preregisters the new memory as device memory as it might be used for DMA.
> >> This inserts pfns from the fault handler as the GPU memory is not onlined
> >> until the NVIDIA driver is loaded and trained the links so doing this
> >> earlier produces low level errors which we fence in the firmware so
> >> it does not hurt the host system but still better to avoid.
> >>
> >> This exports ATSD (Address Translation Shootdown) register of NPU which
> >> allows the guest to invalidate TLB. The register conviniently occupies
> >> a single 64k page. Since NPU maps the GPU memory, it has a "tgt" property
> >> (which is an abbreviated host system bus address). This exports the "tgt"
> >> as a capability so the guest can program it into the GPU so the GPU can
> >> know how to route DMA trafic.  
> > 
> > I'm not really following what "tgt" is and why it's needed.  Is the GPU
> > memory here different than the GPU RAM region above?  Why does the user
> > need the host system bus address of this "tgt" thing?  Are we not able
> > to relocate it in guest physical address space, does this shootdown
> > only work in the host physical address space and therefore we need this
> > offset?  Please explain, I'm confused.  
> 
> 
> This "tgt" is made of:
> - "memory select" (bits 45, 46)
> - "group select" (bits 43, 44)
> - "chip select" (bit 42)
> - chip internal address (bits 0..41)
> 
> These are internal to GPU and this is where GPU RAM is mapped into the
> GPU's real space, this fits 46 bits.
> 
> On POWER9 CPU the bits are different and higher so the same memory is
> mapped higher on P9 CPU. Just because we can map it higher, I guess.
> 
> So it is not exactly the address but this provides the exact physical
> location of the memory.
> 
> We have a group of 3 interconnected GPUs, they got their own
> memory/group/chip numbers. The GPUs use ATS service to translate
> userspace to physical (host or guest) addresses. Now a GPU needs to know
> which specific link to use for a specific physical address, in other
> words what this physical address belongs to - a CPU or one of GPUs. This
> is when "tgt" is used by the GPU hardware.

Clear as mud ;)  So tgt, provided by the npu2 capability of the ATSD
region of the NPU tells the GPU (a completely separate device) how to
route it its own RAM via its NVLink interface?  How can one tgt
indicate the routing for multiple interfaces?

> A GPU could run all the DMA trafic via the system bus indeed, just not
> as fast.
> 
> I am also struggling here and adding an Nvidia person in cc: (I should
> have done that when I posted the patches, my bad) to correct when/if I
> am wrong.
> 
> 
> 
> >    
> >> For ATS to work, the nest MMU (an NVIDIA block in a P9 CPU) needs to
> >> know LPID (a logical partition ID or a KVM guest hardware ID in other
> >> words) and PID (a memory context ID of an userspace process, not to be
> >> confused with a linux pid). This assigns a GPU to LPID in the NPU and
> >> this is why this adds a listener for KVM on an IOMMU group. A PID comes
> >> via NVLink from a GPU and NPU uses a PID wildcard to pass it through.
> >>
> >> This requires coherent memory and ATSD to be available on the host as
> >> the GPU vendor only supports configurations with both features enabled
> >> and other configurations are known not to work. Because of this and
> >> because of the ways the features are advertised to the host system
> >> (which is a device tree with very platform specific properties),
> >> this requires enabled POWERNV platform.
> >>
> >> This hardcodes the NVLink2 support for specific vendor and device IDs
> >> as there is no reliable way of knowing about coherent memory and ATS
> >> support. The GPU has an unique vendor PCIe capability 0x23 but it was
> >> confirmed that it does not provide required information (and it is still
> >> undisclosed what it actually does).
> >>
> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >> ---
> >>  drivers/vfio/pci/Makefile           |   1 +
> >>  drivers/vfio/pci/vfio_pci_private.h |   2 +
> >>  include/uapi/linux/vfio.h           |  18 ++
> >>  drivers/vfio/pci/vfio_pci.c         |  37 +++-
> >>  drivers/vfio/pci/vfio_pci_nvlink2.c | 409 ++++++++++++++++++++++++++++++++++++
> >>  drivers/vfio/pci/Kconfig            |   4 +
> >>  6 files changed, 469 insertions(+), 2 deletions(-)
> >>  create mode 100644 drivers/vfio/pci/vfio_pci_nvlink2.c
> >>
> >> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
> >> index 76d8ec0..9662c06 100644
> >> --- a/drivers/vfio/pci/Makefile
> >> +++ b/drivers/vfio/pci/Makefile
> >> @@ -1,5 +1,6 @@
> >>  
> >>  vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
> >>  vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o
> >> +vfio-pci-$(CONFIG_VFIO_PCI_NVLINK2) += vfio_pci_nvlink2.o
> >>  
> >>  obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
> >> diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
> >> index 93c1738..7639241 100644
> >> --- a/drivers/vfio/pci/vfio_pci_private.h
> >> +++ b/drivers/vfio/pci/vfio_pci_private.h
> >> @@ -163,4 +163,6 @@ static inline int vfio_pci_igd_init(struct vfio_pci_device *vdev)
> >>  	return -ENODEV;
> >>  }
> >>  #endif
> >> +extern int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev);
> >> +extern int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev);
> >>  #endif /* VFIO_PCI_PRIVATE_H */
> >> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> >> index f378b98..9e9a8d3 100644
> >> --- a/include/uapi/linux/vfio.h
> >> +++ b/include/uapi/linux/vfio.h
> >> @@ -303,6 +303,12 @@ struct vfio_region_info_cap_type {
> >>  #define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG	(2)
> >>  #define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG	(3)
> >>  
> >> +/* NVIDIA GPU NVlink2 RAM */
> >> +#define VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM	(1)
> >> +
> >> +/* IBM NPU NVlink2 ATSD */
> >> +#define VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD	(1)
> >> +  
> > 
> > Please include some of the description in the commitlog here for
> > reference.  Also please be explicit that these are vendor defined
> > regions and note the numerical vendor ID associated with them.  
> 
> These are PCI region subtypes which are not from any PCI spec and which
> we define as we like, are not they all "vendor"?

No, it's not entirely obvious that they're vendor regions, I had to
look at the usage later in the patch.  See linux-next where Gerd has
added an EDID region which is not a vendor region.  You make use of
these by masking VFIO_REGION_TYPE_PCI_VENDOR_TYPE into the type, which
indicates they are a PCI vendor type region, which means they are
associated to a specific vendor.  That vendor should be explicitly,
numerically, indicated (some hardware vendors own multiple PCI vendor
IDs or may acquire more).

> >>  /*
> >>   * The MSIX mappable capability informs that MSIX data of a BAR can be mmapped
> >>   * which allows direct access to non-MSIX registers which happened to be within
> >> @@ -313,6 +319,18 @@ struct vfio_region_info_cap_type {
> >>   */
> >>  #define VFIO_REGION_INFO_CAP_MSIX_MAPPABLE	3
> >>  
> >> +/*
> >> + * Capability with compressed real address (aka SSA - small system address)
> >> + * where GPU RAM is mapped on a system bus. Used by a GPU for DMA routing.
> >> + */
> >> +#define VFIO_REGION_INFO_CAP_NPU2		4
> >> +
> >> +struct vfio_region_info_cap_npu2 {
> >> +	struct vfio_info_cap_header header;
> >> +	__u64 tgt;
> >> +	/* size is defined in VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM */  
> > 
> > But this is a capability for the IBM_NVLINK2_ATSD?  What is the
> > relevance of this comment?  Is this capability relevant to the RAM or
> > ATSD?  
> 
> It is relevant to NPU (NVLink host bus adapter of POWER9) which maps the
> GPU RAM to the system bus and acts as a proxy to nestMMU (NVIDIA's unit
> in POWER9 CPU) for ATS/ATSD services so it is a property of NPU. But
> then one might ask "wait, here is the address, where is the size then",
>  hence the comment...

So the tgt field within the npu2 capability of the ATSD region on the
NPU device describes the GPU internal address of the GPU RAM which is
described by the NVLINK2 RAM region on the GPU device... *twitch*  What
business does the NPU have exposing this piece of information and how
is it related to the ATSD region/register?  Is this tgt base used in
the process of doing address translation shootdowns?
 
> >> +};
> >> +
> >>  /**
> >>   * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
> >>   *				    struct vfio_irq_info)
> >> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> >> index 4a3b93e..e9afd43 100644
> >> --- a/drivers/vfio/pci/vfio_pci.c
> >> +++ b/drivers/vfio/pci/vfio_pci.c
> >> @@ -224,6 +224,16 @@ static bool vfio_pci_nointx(struct pci_dev *pdev)
> >>  	return false;
> >>  }
> >>  
> >> +int __weak vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev)
> >> +{
> >> +	return -ENODEV;
> >> +}
> >> +
> >> +int __weak vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev)
> >> +{
> >> +	return -ENODEV;
> >> +}
> >> +
> >>  static int vfio_pci_enable(struct vfio_pci_device *vdev)
> >>  {
> >>  	struct pci_dev *pdev = vdev->pdev;
> >> @@ -302,14 +312,37 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
> >>  		if (ret) {
> >>  			dev_warn(&vdev->pdev->dev,
> >>  				 "Failed to setup Intel IGD regions\n");
> >> -			vfio_pci_disable(vdev);
> >> -			return ret;
> >> +			goto disable_exit;
> >> +		}
> >> +	}
> >> +
> >> +	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
> >> +	    pdev->device == 0x1db1) {
> >> +		ret = vfio_pci_nvdia_v100_nvlink2_init(vdev);
> >> +		if (ret) {
> >> +			dev_warn(&vdev->pdev->dev,
> >> +				 "Failed to setup NVIDIA NV2 RAM region\n");
> >> +			goto disable_exit;
> >> +		}
> >> +	}  
> > 
> > This device ID is not unique to POWER9 Witherspoon systems, I see your
> > comment in the commitlog, but this is clearly going to generate a
> > dev_warn and failure on an x86 system with the same hardware.  Perhaps
> > this could be masked off with IS_ENABLED(CONFIG_VFIO_PCI_NVLINK2) like
> > the IGD code above this chunk does?  
> 
> Right, will fix.
> 
> 
> >> +
> >> +	if (pdev->vendor == PCI_VENDOR_ID_IBM &&
> >> +			pdev->device == 0x04ea) {
> >> +		ret = vfio_pci_ibm_npu2_init(vdev);
> >> +		if (ret) {
> >> +			dev_warn(&vdev->pdev->dev,
> >> +					"Failed to setup NVIDIA NV2 ATSD region\n");
> >> +			goto disable_exit;
> >>  		}  
> > 
> > So the NPU is also actually owned by vfio-pci and assigned to the VM?  
> 
> Yes. On a running system it looks like:
> 
> 0007:00:00.0 Bridge: IBM Device 04ea (rev 01)
> 0007:00:00.1 Bridge: IBM Device 04ea (rev 01)
> 0007:00:01.0 Bridge: IBM Device 04ea (rev 01)
> 0007:00:01.1 Bridge: IBM Device 04ea (rev 01)
> 0007:00:02.0 Bridge: IBM Device 04ea (rev 01)
> 0007:00:02.1 Bridge: IBM Device 04ea (rev 01)
> 0035:00:00.0 PCI bridge: IBM Device 04c1
> 0035:01:00.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> 0035:02:04.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> 0035:02:05.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> 0035:02:0d.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> 0035:03:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
> (rev a1
> 0035:04:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
> (rev a1)
> 0035:05:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
> (rev a1)
> 
> One "IBM Device" bridge represents one NVLink2, i.e. a piece of NPU.
> They all and 3 GPUs go to the same IOMMU group and get passed through to
> a guest.
> 
> The entire NPU does not have representation via sysfs as a whole though.

So the NPU is a bridge, but it uses a normal header type so vfio-pci
will bind to it?  And the ATSD register that we need on it is not
accessible through these PCI representations of the sub-pieces of the
NPU?  Thanks,

Alex

On 10/17/18 2:52 PM, Alex Williamson wrote:
> On Wed, 17 Oct 2018 12:19:20 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
>> On 17/10/2018 06:08, Alex Williamson wrote:
>>> On Mon, 15 Oct 2018 20:42:33 +1100
>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>    
>>>> POWER9 Witherspoon machines come with 4 or 6 V100 GPUs which are not
>>>> pluggable PCIe devices but implement PCIe links for config space and MMIO.
>>>> In addition to that the GPUs are interconnected to each other and also
>>>> have direct links to the P9 CPU. The links are NVLink2 and provide direct
>>>> access to the system RAM for GPUs via NPU (an NVLink2 "proxy" on P9 chip).
>>>> These systems also support ATS (address translation services) which is
>>>> a part of the NVLink2 prototol. Such GPUs also share on-board RAM
>>>> (16GB in tested config) to the system via the same NVLink2 so a CPU has
>>>> cache-coherent access to a GPU RAM.
>>>>
>>>> This exports GPU RAM to the userspace as a new PCI region. This
>>>> preregisters the new memory as device memory as it might be used for DMA.
>>>> This inserts pfns from the fault handler as the GPU memory is not onlined
>>>> until the NVIDIA driver is loaded and trained the links so doing this
>>>> earlier produces low level errors which we fence in the firmware so
>>>> it does not hurt the host system but still better to avoid.
>>>>
>>>> This exports ATSD (Address Translation Shootdown) register of NPU which
>>>> allows the guest to invalidate TLB. The register conviniently occupies
>>>> a single 64k page. Since NPU maps the GPU memory, it has a "tgt" property
>>>> (which is an abbreviated host system bus address). This exports the "tgt"
>>>> as a capability so the guest can program it into the GPU so the GPU can
>>>> know how to route DMA trafic.
>>>
>>> I'm not really following what "tgt" is and why it's needed.  Is the GPU
>>> memory here different than the GPU RAM region above?  Why does the user
>>> need the host system bus address of this "tgt" thing?  Are we not able
>>> to relocate it in guest physical address space, does this shootdown
>>> only work in the host physical address space and therefore we need this
>>> offset?  Please explain, I'm confused.
>>
>>
>> This "tgt" is made of:
>> - "memory select" (bits 45, 46)
>> - "group select" (bits 43, 44)
>> - "chip select" (bit 42)
>> - chip internal address (bits 0..41)
>>
>> These are internal to GPU and this is where GPU RAM is mapped into the
>> GPU's real space, this fits 46 bits.
>>
>> On POWER9 CPU the bits are different and higher so the same memory is
>> mapped higher on P9 CPU. Just because we can map it higher, I guess.
>>
>> So it is not exactly the address but this provides the exact physical
>> location of the memory.
>>
>> We have a group of 3 interconnected GPUs, they got their own
>> memory/group/chip numbers. The GPUs use ATS service to translate
>> userspace to physical (host or guest) addresses. Now a GPU needs to know
>> which specific link to use for a specific physical address, in other
>> words what this physical address belongs to - a CPU or one of GPUs. This
>> is when "tgt" is used by the GPU hardware.
> 
> Clear as mud ;)  So tgt, provided by the npu2 capability of the ATSD
> region of the NPU tells the GPU (a completely separate device) how to
> route it its own RAM via its NVLink interface?  How can one tgt
> indicate the routing for multiple interfaces?

The tgt addresses are read by the GPU driver for each GPU from the 
device tree properties and are used to program routing for all the GPUs 
in the VM. Each GPU needs to know:
1) Its own address range so that it can route ATS accesses to it 
directly to its RAM.
2) All direct peer GPUs (connected with nvlink directly) address ranges 
so that it can route accesses to peers through links going directly to 
those peers.
3) Everything else gets routed to the links going to the CPU.

Anticipating a question about the security implications of allowing the 
guest to configure this routing, no, this is not a problem:
1) If a range gets misprogrammed causing an access to be routed to the 
CPU nvlink incorrectly, the CPU still receives the full physical address 
of the access and can drop or re-route it correctly.
2) If a range gets misprogrammed causing an access to go to a peer GPU 
incorrectly, the guest can corrupt memory of that peer GPU, but it fully 
owns that GPU anyway.
3) If a range gets misprogrammed causing an access to go to local GPU 
memory incorrectly, the guest can corrupt that memory, but it fully owns 
it anyway.

Thanks,
Piotr


> 
>> A GPU could run all the DMA trafic via the system bus indeed, just not
>> as fast.
>>
>> I am also struggling here and adding an Nvidia person in cc: (I should
>> have done that when I posted the patches, my bad) to correct when/if I
>> am wrong.
>>
>>
>>
>>>     
>>>> For ATS to work, the nest MMU (an NVIDIA block in a P9 CPU) needs to
>>>> know LPID (a logical partition ID or a KVM guest hardware ID in other
>>>> words) and PID (a memory context ID of an userspace process, not to be
>>>> confused with a linux pid). This assigns a GPU to LPID in the NPU and
>>>> this is why this adds a listener for KVM on an IOMMU group. A PID comes
>>>> via NVLink from a GPU and NPU uses a PID wildcard to pass it through.
>>>>
>>>> This requires coherent memory and ATSD to be available on the host as
>>>> the GPU vendor only supports configurations with both features enabled
>>>> and other configurations are known not to work. Because of this and
>>>> because of the ways the features are advertised to the host system
>>>> (which is a device tree with very platform specific properties),
>>>> this requires enabled POWERNV platform.
>>>>
>>>> This hardcodes the NVLink2 support for specific vendor and device IDs
>>>> as there is no reliable way of knowing about coherent memory and ATS
>>>> support. The GPU has an unique vendor PCIe capability 0x23 but it was
>>>> confirmed that it does not provide required information (and it is still
>>>> undisclosed what it actually does).
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>> ---
>>>>   drivers/vfio/pci/Makefile           |   1 +
>>>>   drivers/vfio/pci/vfio_pci_private.h |   2 +
>>>>   include/uapi/linux/vfio.h           |  18 ++
>>>>   drivers/vfio/pci/vfio_pci.c         |  37 +++-
>>>>   drivers/vfio/pci/vfio_pci_nvlink2.c | 409 ++++++++++++++++++++++++++++++++++++
>>>>   drivers/vfio/pci/Kconfig            |   4 +
>>>>   6 files changed, 469 insertions(+), 2 deletions(-)
>>>>   create mode 100644 drivers/vfio/pci/vfio_pci_nvlink2.c
>>>>
>>>> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
>>>> index 76d8ec0..9662c06 100644
>>>> --- a/drivers/vfio/pci/Makefile
>>>> +++ b/drivers/vfio/pci/Makefile
>>>> @@ -1,5 +1,6 @@
>>>>   
>>>>   vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
>>>>   vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o
>>>> +vfio-pci-$(CONFIG_VFIO_PCI_NVLINK2) += vfio_pci_nvlink2.o
>>>>   
>>>>   obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
>>>> diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
>>>> index 93c1738..7639241 100644
>>>> --- a/drivers/vfio/pci/vfio_pci_private.h
>>>> +++ b/drivers/vfio/pci/vfio_pci_private.h
>>>> @@ -163,4 +163,6 @@ static inline int vfio_pci_igd_init(struct vfio_pci_device *vdev)
>>>>   	return -ENODEV;
>>>>   }
>>>>   #endif
>>>> +extern int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev);
>>>> +extern int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev);
>>>>   #endif /* VFIO_PCI_PRIVATE_H */
>>>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>>>> index f378b98..9e9a8d3 100644
>>>> --- a/include/uapi/linux/vfio.h
>>>> +++ b/include/uapi/linux/vfio.h
>>>> @@ -303,6 +303,12 @@ struct vfio_region_info_cap_type {
>>>>   #define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG	(2)
>>>>   #define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG	(3)
>>>>   
>>>> +/* NVIDIA GPU NVlink2 RAM */
>>>> +#define VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM	(1)
>>>> +
>>>> +/* IBM NPU NVlink2 ATSD */
>>>> +#define VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD	(1)
>>>> +
>>>
>>> Please include some of the description in the commitlog here for
>>> reference.  Also please be explicit that these are vendor defined
>>> regions and note the numerical vendor ID associated with them.
>>
>> These are PCI region subtypes which are not from any PCI spec and which
>> we define as we like, are not they all "vendor"?
> 
> No, it's not entirely obvious that they're vendor regions, I had to
> look at the usage later in the patch.  See linux-next where Gerd has
> added an EDID region which is not a vendor region.  You make use of
> these by masking VFIO_REGION_TYPE_PCI_VENDOR_TYPE into the type, which
> indicates they are a PCI vendor type region, which means they are
> associated to a specific vendor.  That vendor should be explicitly,
> numerically, indicated (some hardware vendors own multiple PCI vendor
> IDs or may acquire more).
> 
>>>>   /*
>>>>    * The MSIX mappable capability informs that MSIX data of a BAR can be mmapped
>>>>    * which allows direct access to non-MSIX registers which happened to be within
>>>> @@ -313,6 +319,18 @@ struct vfio_region_info_cap_type {
>>>>    */
>>>>   #define VFIO_REGION_INFO_CAP_MSIX_MAPPABLE	3
>>>>   
>>>> +/*
>>>> + * Capability with compressed real address (aka SSA - small system address)
>>>> + * where GPU RAM is mapped on a system bus. Used by a GPU for DMA routing.
>>>> + */
>>>> +#define VFIO_REGION_INFO_CAP_NPU2		4
>>>> +
>>>> +struct vfio_region_info_cap_npu2 {
>>>> +	struct vfio_info_cap_header header;
>>>> +	__u64 tgt;
>>>> +	/* size is defined in VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM */
>>>
>>> But this is a capability for the IBM_NVLINK2_ATSD?  What is the
>>> relevance of this comment?  Is this capability relevant to the RAM or
>>> ATSD?
>>
>> It is relevant to NPU (NVLink host bus adapter of POWER9) which maps the
>> GPU RAM to the system bus and acts as a proxy to nestMMU (NVIDIA's unit
>> in POWER9 CPU) for ATS/ATSD services so it is a property of NPU. But
>> then one might ask "wait, here is the address, where is the size then",
>>   hence the comment...
> 
> So the tgt field within the npu2 capability of the ATSD region on the
> NPU device describes the GPU internal address of the GPU RAM which is
> described by the NVLINK2 RAM region on the GPU device... *twitch*  What
> business does the NPU have exposing this piece of information and how
> is it related to the ATSD region/register?  Is this tgt base used in
> the process of doing address translation shootdowns?
>   
>>>> +};
>>>> +
>>>>   /**
>>>>    * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
>>>>    *				    struct vfio_irq_info)
>>>> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
>>>> index 4a3b93e..e9afd43 100644
>>>> --- a/drivers/vfio/pci/vfio_pci.c
>>>> +++ b/drivers/vfio/pci/vfio_pci.c
>>>> @@ -224,6 +224,16 @@ static bool vfio_pci_nointx(struct pci_dev *pdev)
>>>>   	return false;
>>>>   }
>>>>   
>>>> +int __weak vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev)
>>>> +{
>>>> +	return -ENODEV;
>>>> +}
>>>> +
>>>> +int __weak vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev)
>>>> +{
>>>> +	return -ENODEV;
>>>> +}
>>>> +
>>>>   static int vfio_pci_enable(struct vfio_pci_device *vdev)
>>>>   {
>>>>   	struct pci_dev *pdev = vdev->pdev;
>>>> @@ -302,14 +312,37 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
>>>>   		if (ret) {
>>>>   			dev_warn(&vdev->pdev->dev,
>>>>   				 "Failed to setup Intel IGD regions\n");
>>>> -			vfio_pci_disable(vdev);
>>>> -			return ret;
>>>> +			goto disable_exit;
>>>> +		}
>>>> +	}
>>>> +
>>>> +	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
>>>> +	    pdev->device == 0x1db1) {
>>>> +		ret = vfio_pci_nvdia_v100_nvlink2_init(vdev);
>>>> +		if (ret) {
>>>> +			dev_warn(&vdev->pdev->dev,
>>>> +				 "Failed to setup NVIDIA NV2 RAM region\n");
>>>> +			goto disable_exit;
>>>> +		}
>>>> +	}
>>>
>>> This device ID is not unique to POWER9 Witherspoon systems, I see your
>>> comment in the commitlog, but this is clearly going to generate a
>>> dev_warn and failure on an x86 system with the same hardware.  Perhaps
>>> this could be masked off with IS_ENABLED(CONFIG_VFIO_PCI_NVLINK2) like
>>> the IGD code above this chunk does?
>>
>> Right, will fix.
>>
>>
>>>> +
>>>> +	if (pdev->vendor == PCI_VENDOR_ID_IBM &&
>>>> +			pdev->device == 0x04ea) {
>>>> +		ret = vfio_pci_ibm_npu2_init(vdev);
>>>> +		if (ret) {
>>>> +			dev_warn(&vdev->pdev->dev,
>>>> +					"Failed to setup NVIDIA NV2 ATSD region\n");
>>>> +			goto disable_exit;
>>>>   		}
>>>
>>> So the NPU is also actually owned by vfio-pci and assigned to the VM?
>>
>> Yes. On a running system it looks like:
>>
>> 0007:00:00.0 Bridge: IBM Device 04ea (rev 01)
>> 0007:00:00.1 Bridge: IBM Device 04ea (rev 01)
>> 0007:00:01.0 Bridge: IBM Device 04ea (rev 01)
>> 0007:00:01.1 Bridge: IBM Device 04ea (rev 01)
>> 0007:00:02.0 Bridge: IBM Device 04ea (rev 01)
>> 0007:00:02.1 Bridge: IBM Device 04ea (rev 01)
>> 0035:00:00.0 PCI bridge: IBM Device 04c1
>> 0035:01:00.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>> 0035:02:04.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>> 0035:02:05.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>> 0035:02:0d.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>> 0035:03:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>> (rev a1
>> 0035:04:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>> (rev a1)
>> 0035:05:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>> (rev a1)
>>
>> One "IBM Device" bridge represents one NVLink2, i.e. a piece of NPU.
>> They all and 3 GPUs go to the same IOMMU group and get passed through to
>> a guest.
>>
>> The entire NPU does not have representation via sysfs as a whole though.
> 
> So the NPU is a bridge, but it uses a normal header type so vfio-pci
> will bind to it?  And the ATSD register that we need on it is not
> accessible through these PCI representations of the sub-pieces of the
> NPU?  Thanks,
> 
> Alex
>

On 18/10/2018 08:52, Alex Williamson wrote:
> On Wed, 17 Oct 2018 12:19:20 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
>> On 17/10/2018 06:08, Alex Williamson wrote:
>>> On Mon, 15 Oct 2018 20:42:33 +1100
>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>   
>>>> POWER9 Witherspoon machines come with 4 or 6 V100 GPUs which are not
>>>> pluggable PCIe devices but implement PCIe links for config space and MMIO.
>>>> In addition to that the GPUs are interconnected to each other and also
>>>> have direct links to the P9 CPU. The links are NVLink2 and provide direct
>>>> access to the system RAM for GPUs via NPU (an NVLink2 "proxy" on P9 chip).
>>>> These systems also support ATS (address translation services) which is
>>>> a part of the NVLink2 prototol. Such GPUs also share on-board RAM
>>>> (16GB in tested config) to the system via the same NVLink2 so a CPU has
>>>> cache-coherent access to a GPU RAM.
>>>>
>>>> This exports GPU RAM to the userspace as a new PCI region. This
>>>> preregisters the new memory as device memory as it might be used for DMA.
>>>> This inserts pfns from the fault handler as the GPU memory is not onlined
>>>> until the NVIDIA driver is loaded and trained the links so doing this
>>>> earlier produces low level errors which we fence in the firmware so
>>>> it does not hurt the host system but still better to avoid.
>>>>
>>>> This exports ATSD (Address Translation Shootdown) register of NPU which
>>>> allows the guest to invalidate TLB. The register conviniently occupies
>>>> a single 64k page. Since NPU maps the GPU memory, it has a "tgt" property
>>>> (which is an abbreviated host system bus address). This exports the "tgt"
>>>> as a capability so the guest can program it into the GPU so the GPU can
>>>> know how to route DMA trafic.  
>>>
>>> I'm not really following what "tgt" is and why it's needed.  Is the GPU
>>> memory here different than the GPU RAM region above?  Why does the user
>>> need the host system bus address of this "tgt" thing?  Are we not able
>>> to relocate it in guest physical address space, does this shootdown
>>> only work in the host physical address space and therefore we need this
>>> offset?  Please explain, I'm confused.  
>>
>>
>> This "tgt" is made of:
>> - "memory select" (bits 45, 46)
>> - "group select" (bits 43, 44)
>> - "chip select" (bit 42)
>> - chip internal address (bits 0..41)
>>
>> These are internal to GPU and this is where GPU RAM is mapped into the
>> GPU's real space, this fits 46 bits.
>>
>> On POWER9 CPU the bits are different and higher so the same memory is
>> mapped higher on P9 CPU. Just because we can map it higher, I guess.
>>
>> So it is not exactly the address but this provides the exact physical
>> location of the memory.
>>
>> We have a group of 3 interconnected GPUs, they got their own
>> memory/group/chip numbers. The GPUs use ATS service to translate
>> userspace to physical (host or guest) addresses. Now a GPU needs to know
>> which specific link to use for a specific physical address, in other
>> words what this physical address belongs to - a CPU or one of GPUs. This
>> is when "tgt" is used by the GPU hardware.
> 
> Clear as mud ;) 

/me is sad. I hope Piotr explained it better...


> So tgt, provided by the npu2 capability of the ATSD
> region of the NPU tells the GPU (a completely separate device) how to
> route it its own RAM via its NVLink interface?  How can one tgt
> indicate the routing for multiple interfaces?

This NVLink DMA is using direct host physical addresses (no IOMMU, no
filtering) which come from ATS. So unless we tell the GPU its own
address range on the host CPU, it will route trafic via CPU. And the
driver can also discover the NVLink topology and tell each GPU physical
addresses of peer GPUs.



> 
>> A GPU could run all the DMA trafic via the system bus indeed, just not
>> as fast.
>>
>> I am also struggling here and adding an Nvidia person in cc: (I should
>> have done that when I posted the patches, my bad) to correct when/if I
>> am wrong.
>>
>>
>>
>>>    
>>>> For ATS to work, the nest MMU (an NVIDIA block in a P9 CPU) needs to
>>>> know LPID (a logical partition ID or a KVM guest hardware ID in other
>>>> words) and PID (a memory context ID of an userspace process, not to be
>>>> confused with a linux pid). This assigns a GPU to LPID in the NPU and
>>>> this is why this adds a listener for KVM on an IOMMU group. A PID comes
>>>> via NVLink from a GPU and NPU uses a PID wildcard to pass it through.
>>>>
>>>> This requires coherent memory and ATSD to be available on the host as
>>>> the GPU vendor only supports configurations with both features enabled
>>>> and other configurations are known not to work. Because of this and
>>>> because of the ways the features are advertised to the host system
>>>> (which is a device tree with very platform specific properties),
>>>> this requires enabled POWERNV platform.
>>>>
>>>> This hardcodes the NVLink2 support for specific vendor and device IDs
>>>> as there is no reliable way of knowing about coherent memory and ATS
>>>> support. The GPU has an unique vendor PCIe capability 0x23 but it was
>>>> confirmed that it does not provide required information (and it is still
>>>> undisclosed what it actually does).
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>> ---
>>>>  drivers/vfio/pci/Makefile           |   1 +
>>>>  drivers/vfio/pci/vfio_pci_private.h |   2 +
>>>>  include/uapi/linux/vfio.h           |  18 ++
>>>>  drivers/vfio/pci/vfio_pci.c         |  37 +++-
>>>>  drivers/vfio/pci/vfio_pci_nvlink2.c | 409 ++++++++++++++++++++++++++++++++++++
>>>>  drivers/vfio/pci/Kconfig            |   4 +
>>>>  6 files changed, 469 insertions(+), 2 deletions(-)
>>>>  create mode 100644 drivers/vfio/pci/vfio_pci_nvlink2.c
>>>>
>>>> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
>>>> index 76d8ec0..9662c06 100644
>>>> --- a/drivers/vfio/pci/Makefile
>>>> +++ b/drivers/vfio/pci/Makefile
>>>> @@ -1,5 +1,6 @@
>>>>  
>>>>  vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
>>>>  vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o
>>>> +vfio-pci-$(CONFIG_VFIO_PCI_NVLINK2) += vfio_pci_nvlink2.o
>>>>  
>>>>  obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
>>>> diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
>>>> index 93c1738..7639241 100644
>>>> --- a/drivers/vfio/pci/vfio_pci_private.h
>>>> +++ b/drivers/vfio/pci/vfio_pci_private.h
>>>> @@ -163,4 +163,6 @@ static inline int vfio_pci_igd_init(struct vfio_pci_device *vdev)
>>>>  	return -ENODEV;
>>>>  }
>>>>  #endif
>>>> +extern int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev);
>>>> +extern int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev);
>>>>  #endif /* VFIO_PCI_PRIVATE_H */
>>>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>>>> index f378b98..9e9a8d3 100644
>>>> --- a/include/uapi/linux/vfio.h
>>>> +++ b/include/uapi/linux/vfio.h
>>>> @@ -303,6 +303,12 @@ struct vfio_region_info_cap_type {
>>>>  #define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG	(2)
>>>>  #define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG	(3)
>>>>  
>>>> +/* NVIDIA GPU NVlink2 RAM */
>>>> +#define VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM	(1)
>>>> +
>>>> +/* IBM NPU NVlink2 ATSD */
>>>> +#define VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD	(1)
>>>> +  
>>>
>>> Please include some of the description in the commitlog here for
>>> reference.  Also please be explicit that these are vendor defined
>>> regions and note the numerical vendor ID associated with them.  
>>
>> These are PCI region subtypes which are not from any PCI spec and which
>> we define as we like, are not they all "vendor"?
> 
> No, it's not entirely obvious that they're vendor regions, I had to
> look at the usage later in the patch.  See linux-next where Gerd has
> added an EDID region which is not a vendor region.  You make use of
> these by masking VFIO_REGION_TYPE_PCI_VENDOR_TYPE into the type, which
> indicates they are a PCI vendor type region, which means they are
> associated to a specific vendor.  That vendor should be explicitly,
> numerically, indicated (some hardware vendors own multiple PCI vendor
> IDs or may acquire more).

Ah, I see now. ok.


>>>>  /*
>>>>   * The MSIX mappable capability informs that MSIX data of a BAR can be mmapped
>>>>   * which allows direct access to non-MSIX registers which happened to be within
>>>> @@ -313,6 +319,18 @@ struct vfio_region_info_cap_type {
>>>>   */
>>>>  #define VFIO_REGION_INFO_CAP_MSIX_MAPPABLE	3
>>>>  
>>>> +/*
>>>> + * Capability with compressed real address (aka SSA - small system address)
>>>> + * where GPU RAM is mapped on a system bus. Used by a GPU for DMA routing.
>>>> + */
>>>> +#define VFIO_REGION_INFO_CAP_NPU2		4
>>>> +
>>>> +struct vfio_region_info_cap_npu2 {
>>>> +	struct vfio_info_cap_header header;
>>>> +	__u64 tgt;
>>>> +	/* size is defined in VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM */  
>>>
>>> But this is a capability for the IBM_NVLINK2_ATSD?  What is the
>>> relevance of this comment?  Is this capability relevant to the RAM or
>>> ATSD?  
>>
>> It is relevant to NPU (NVLink host bus adapter of POWER9) which maps the
>> GPU RAM to the system bus and acts as a proxy to nestMMU (NVIDIA's unit
>> in POWER9 CPU) for ATS/ATSD services so it is a property of NPU. But
>> then one might ask "wait, here is the address, where is the size then",
>>  hence the comment...
> 
> So the tgt field within the npu2 capability of the ATSD region on the
> NPU device describes the GPU internal address of the GPU RAM which is
> described by the NVLINK2 RAM region on the GPU device... *twitch*  What
> business does the NPU have exposing this piece of information and how
> is it related to the ATSD region/register? 

NPU is the source of the information on the host. ATSD is just another
feature of NPU.

> Is this tgt base used in
> the process of doing address translation shootdowns?

No, only routing.


>>>> +};
>>>> +
>>>>  /**
>>>>   * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
>>>>   *				    struct vfio_irq_info)
>>>> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
>>>> index 4a3b93e..e9afd43 100644
>>>> --- a/drivers/vfio/pci/vfio_pci.c
>>>> +++ b/drivers/vfio/pci/vfio_pci.c
>>>> @@ -224,6 +224,16 @@ static bool vfio_pci_nointx(struct pci_dev *pdev)
>>>>  	return false;
>>>>  }
>>>>  
>>>> +int __weak vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev)
>>>> +{
>>>> +	return -ENODEV;
>>>> +}
>>>> +
>>>> +int __weak vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev)
>>>> +{
>>>> +	return -ENODEV;
>>>> +}
>>>> +
>>>>  static int vfio_pci_enable(struct vfio_pci_device *vdev)
>>>>  {
>>>>  	struct pci_dev *pdev = vdev->pdev;
>>>> @@ -302,14 +312,37 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
>>>>  		if (ret) {
>>>>  			dev_warn(&vdev->pdev->dev,
>>>>  				 "Failed to setup Intel IGD regions\n");
>>>> -			vfio_pci_disable(vdev);
>>>> -			return ret;
>>>> +			goto disable_exit;
>>>> +		}
>>>> +	}
>>>> +
>>>> +	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
>>>> +	    pdev->device == 0x1db1) {
>>>> +		ret = vfio_pci_nvdia_v100_nvlink2_init(vdev);
>>>> +		if (ret) {
>>>> +			dev_warn(&vdev->pdev->dev,
>>>> +				 "Failed to setup NVIDIA NV2 RAM region\n");
>>>> +			goto disable_exit;
>>>> +		}
>>>> +	}  
>>>
>>> This device ID is not unique to POWER9 Witherspoon systems, I see your
>>> comment in the commitlog, but this is clearly going to generate a
>>> dev_warn and failure on an x86 system with the same hardware.  Perhaps
>>> this could be masked off with IS_ENABLED(CONFIG_VFIO_PCI_NVLINK2) like
>>> the IGD code above this chunk does?  
>>
>> Right, will fix.
>>
>>
>>>> +
>>>> +	if (pdev->vendor == PCI_VENDOR_ID_IBM &&
>>>> +			pdev->device == 0x04ea) {
>>>> +		ret = vfio_pci_ibm_npu2_init(vdev);
>>>> +		if (ret) {
>>>> +			dev_warn(&vdev->pdev->dev,
>>>> +					"Failed to setup NVIDIA NV2 ATSD region\n");
>>>> +			goto disable_exit;
>>>>  		}  
>>>
>>> So the NPU is also actually owned by vfio-pci and assigned to the VM?  
>>
>> Yes. On a running system it looks like:
>>
>> 0007:00:00.0 Bridge: IBM Device 04ea (rev 01)
>> 0007:00:00.1 Bridge: IBM Device 04ea (rev 01)
>> 0007:00:01.0 Bridge: IBM Device 04ea (rev 01)
>> 0007:00:01.1 Bridge: IBM Device 04ea (rev 01)
>> 0007:00:02.0 Bridge: IBM Device 04ea (rev 01)
>> 0007:00:02.1 Bridge: IBM Device 04ea (rev 01)
>> 0035:00:00.0 PCI bridge: IBM Device 04c1
>> 0035:01:00.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>> 0035:02:04.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>> 0035:02:05.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>> 0035:02:0d.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>> 0035:03:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>> (rev a1
>> 0035:04:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>> (rev a1)
>> 0035:05:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>> (rev a1)
>>
>> One "IBM Device" bridge represents one NVLink2, i.e. a piece of NPU.
>> They all and 3 GPUs go to the same IOMMU group and get passed through to
>> a guest.
>>
>> The entire NPU does not have representation via sysfs as a whole though.
> 
> So the NPU is a bridge, but it uses a normal header type so vfio-pci
> will bind to it? 

An NPU is a NVLink bridge, it is not PCI in any sense. We (the host
powerpc firmware known as "skiboot" or "opal") have chosen to emulate a
virtual bridge per 1 NVLink on the firmware level. So for each physical
NPU there are 6 virtual bridges. So the NVIDIA driver does not need to
know much about NPUs.

> And the ATSD register that we need on it is not
> accessible through these PCI representations of the sub-pieces of the
> NPU?  Thanks,

No, only via the device tree. The skiboot puts the ATSD register address
to the PHB's DT property called 'ibm,mmio-atsd' of these virtual bridges.

On Thu, 18 Oct 2018 11:31:33 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> On 18/10/2018 08:52, Alex Williamson wrote:
> > On Wed, 17 Oct 2018 12:19:20 +1100
> > Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> >   
> >> On 17/10/2018 06:08, Alex Williamson wrote:  
> >>> On Mon, 15 Oct 2018 20:42:33 +1100
> >>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> >>>     
> >>>> POWER9 Witherspoon machines come with 4 or 6 V100 GPUs which are not
> >>>> pluggable PCIe devices but implement PCIe links for config space and MMIO.
> >>>> In addition to that the GPUs are interconnected to each other and also
> >>>> have direct links to the P9 CPU. The links are NVLink2 and provide direct
> >>>> access to the system RAM for GPUs via NPU (an NVLink2 "proxy" on P9 chip).
> >>>> These systems also support ATS (address translation services) which is
> >>>> a part of the NVLink2 prototol. Such GPUs also share on-board RAM
> >>>> (16GB in tested config) to the system via the same NVLink2 so a CPU has
> >>>> cache-coherent access to a GPU RAM.
> >>>>
> >>>> This exports GPU RAM to the userspace as a new PCI region. This
> >>>> preregisters the new memory as device memory as it might be used for DMA.
> >>>> This inserts pfns from the fault handler as the GPU memory is not onlined
> >>>> until the NVIDIA driver is loaded and trained the links so doing this
> >>>> earlier produces low level errors which we fence in the firmware so
> >>>> it does not hurt the host system but still better to avoid.
> >>>>
> >>>> This exports ATSD (Address Translation Shootdown) register of NPU which
> >>>> allows the guest to invalidate TLB. The register conviniently occupies
> >>>> a single 64k page. Since NPU maps the GPU memory, it has a "tgt" property
> >>>> (which is an abbreviated host system bus address). This exports the "tgt"
> >>>> as a capability so the guest can program it into the GPU so the GPU can
> >>>> know how to route DMA trafic.    
> >>>
> >>> I'm not really following what "tgt" is and why it's needed.  Is the GPU
> >>> memory here different than the GPU RAM region above?  Why does the user
> >>> need the host system bus address of this "tgt" thing?  Are we not able
> >>> to relocate it in guest physical address space, does this shootdown
> >>> only work in the host physical address space and therefore we need this
> >>> offset?  Please explain, I'm confused.    
> >>
> >>
> >> This "tgt" is made of:
> >> - "memory select" (bits 45, 46)
> >> - "group select" (bits 43, 44)
> >> - "chip select" (bit 42)
> >> - chip internal address (bits 0..41)
> >>
> >> These are internal to GPU and this is where GPU RAM is mapped into the
> >> GPU's real space, this fits 46 bits.
> >>
> >> On POWER9 CPU the bits are different and higher so the same memory is
> >> mapped higher on P9 CPU. Just because we can map it higher, I guess.
> >>
> >> So it is not exactly the address but this provides the exact physical
> >> location of the memory.
> >>
> >> We have a group of 3 interconnected GPUs, they got their own
> >> memory/group/chip numbers. The GPUs use ATS service to translate
> >> userspace to physical (host or guest) addresses. Now a GPU needs to know
> >> which specific link to use for a specific physical address, in other
> >> words what this physical address belongs to - a CPU or one of GPUs. This
> >> is when "tgt" is used by the GPU hardware.  
> > 
> > Clear as mud ;)   
> 
> /me is sad. I hope Piotr explained it better...

It's starting to be a bit more clear, and Piotr anticipated the
security questions I was mulling over.

> > So tgt, provided by the npu2 capability of the ATSD
> > region of the NPU tells the GPU (a completely separate device) how to
> > route it its own RAM via its NVLink interface?  How can one tgt
> > indicate the routing for multiple interfaces?  
> 
> This NVLink DMA is using direct host physical addresses (no IOMMU, no
> filtering) which come from ATS. So unless we tell the GPU its own
> address range on the host CPU, it will route trafic via CPU. And the
> driver can also discover the NVLink topology and tell each GPU physical
> addresses of peer GPUs.

I think this is a key point too, the tgt seems to only identify the
routing for the GPU local RAM, but the guest driver is able to
conglomerate this information so each GPU knows how to get directly to
the other GPUs.

> >> A GPU could run all the DMA trafic via the system bus indeed, just not
> >> as fast.
> >>
> >> I am also struggling here and adding an Nvidia person in cc: (I should
> >> have done that when I posted the patches, my bad) to correct when/if I
> >> am wrong.
> >>
> >>
> >>  
> >>>      
> >>>> For ATS to work, the nest MMU (an NVIDIA block in a P9 CPU) needs to
> >>>> know LPID (a logical partition ID or a KVM guest hardware ID in other
> >>>> words) and PID (a memory context ID of an userspace process, not to be
> >>>> confused with a linux pid). This assigns a GPU to LPID in the NPU and
> >>>> this is why this adds a listener for KVM on an IOMMU group. A PID comes
> >>>> via NVLink from a GPU and NPU uses a PID wildcard to pass it through.
> >>>>
> >>>> This requires coherent memory and ATSD to be available on the host as
> >>>> the GPU vendor only supports configurations with both features enabled
> >>>> and other configurations are known not to work. Because of this and
> >>>> because of the ways the features are advertised to the host system
> >>>> (which is a device tree with very platform specific properties),
> >>>> this requires enabled POWERNV platform.
> >>>>
> >>>> This hardcodes the NVLink2 support for specific vendor and device IDs
> >>>> as there is no reliable way of knowing about coherent memory and ATS
> >>>> support. The GPU has an unique vendor PCIe capability 0x23 but it was
> >>>> confirmed that it does not provide required information (and it is still
> >>>> undisclosed what it actually does).
> >>>>
> >>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >>>> ---
> >>>>  drivers/vfio/pci/Makefile           |   1 +
> >>>>  drivers/vfio/pci/vfio_pci_private.h |   2 +
> >>>>  include/uapi/linux/vfio.h           |  18 ++
> >>>>  drivers/vfio/pci/vfio_pci.c         |  37 +++-
> >>>>  drivers/vfio/pci/vfio_pci_nvlink2.c | 409 ++++++++++++++++++++++++++++++++++++
> >>>>  drivers/vfio/pci/Kconfig            |   4 +
> >>>>  6 files changed, 469 insertions(+), 2 deletions(-)
> >>>>  create mode 100644 drivers/vfio/pci/vfio_pci_nvlink2.c
> >>>>
> >>>> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
> >>>> index 76d8ec0..9662c06 100644
> >>>> --- a/drivers/vfio/pci/Makefile
> >>>> +++ b/drivers/vfio/pci/Makefile
> >>>> @@ -1,5 +1,6 @@
> >>>>  
> >>>>  vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
> >>>>  vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o
> >>>> +vfio-pci-$(CONFIG_VFIO_PCI_NVLINK2) += vfio_pci_nvlink2.o
> >>>>  
> >>>>  obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
> >>>> diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
> >>>> index 93c1738..7639241 100644
> >>>> --- a/drivers/vfio/pci/vfio_pci_private.h
> >>>> +++ b/drivers/vfio/pci/vfio_pci_private.h
> >>>> @@ -163,4 +163,6 @@ static inline int vfio_pci_igd_init(struct vfio_pci_device *vdev)
> >>>>  	return -ENODEV;
> >>>>  }
> >>>>  #endif
> >>>> +extern int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev);
> >>>> +extern int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev);
> >>>>  #endif /* VFIO_PCI_PRIVATE_H */
> >>>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> >>>> index f378b98..9e9a8d3 100644
> >>>> --- a/include/uapi/linux/vfio.h
> >>>> +++ b/include/uapi/linux/vfio.h
> >>>> @@ -303,6 +303,12 @@ struct vfio_region_info_cap_type {
> >>>>  #define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG	(2)
> >>>>  #define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG	(3)
> >>>>  
> >>>> +/* NVIDIA GPU NVlink2 RAM */
> >>>> +#define VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM	(1)
> >>>> +
> >>>> +/* IBM NPU NVlink2 ATSD */
> >>>> +#define VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD	(1)
> >>>> +    
> >>>
> >>> Please include some of the description in the commitlog here for
> >>> reference.  Also please be explicit that these are vendor defined
> >>> regions and note the numerical vendor ID associated with them.    
> >>
> >> These are PCI region subtypes which are not from any PCI spec and which
> >> we define as we like, are not they all "vendor"?  
> > 
> > No, it's not entirely obvious that they're vendor regions, I had to
> > look at the usage later in the patch.  See linux-next where Gerd has
> > added an EDID region which is not a vendor region.  You make use of
> > these by masking VFIO_REGION_TYPE_PCI_VENDOR_TYPE into the type, which
> > indicates they are a PCI vendor type region, which means they are
> > associated to a specific vendor.  That vendor should be explicitly,
> > numerically, indicated (some hardware vendors own multiple PCI vendor
> > IDs or may acquire more).  
> 
> Ah, I see now. ok.
> 
> 
> >>>>  /*
> >>>>   * The MSIX mappable capability informs that MSIX data of a BAR can be mmapped
> >>>>   * which allows direct access to non-MSIX registers which happened to be within
> >>>> @@ -313,6 +319,18 @@ struct vfio_region_info_cap_type {
> >>>>   */
> >>>>  #define VFIO_REGION_INFO_CAP_MSIX_MAPPABLE	3
> >>>>  
> >>>> +/*
> >>>> + * Capability with compressed real address (aka SSA - small system address)
> >>>> + * where GPU RAM is mapped on a system bus. Used by a GPU for DMA routing.
> >>>> + */
> >>>> +#define VFIO_REGION_INFO_CAP_NPU2		4
> >>>> +
> >>>> +struct vfio_region_info_cap_npu2 {
> >>>> +	struct vfio_info_cap_header header;
> >>>> +	__u64 tgt;
> >>>> +	/* size is defined in VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM */    
> >>>
> >>> But this is a capability for the IBM_NVLINK2_ATSD?  What is the
> >>> relevance of this comment?  Is this capability relevant to the RAM or
> >>> ATSD?    
> >>
> >> It is relevant to NPU (NVLink host bus adapter of POWER9) which maps the
> >> GPU RAM to the system bus and acts as a proxy to nestMMU (NVIDIA's unit
> >> in POWER9 CPU) for ATS/ATSD services so it is a property of NPU. But
> >> then one might ask "wait, here is the address, where is the size then",
> >>  hence the comment...  
> > 
> > So the tgt field within the npu2 capability of the ATSD region on the
> > NPU device describes the GPU internal address of the GPU RAM which is
> > described by the NVLINK2 RAM region on the GPU device... *twitch*  What
> > business does the NPU have exposing this piece of information and how
> > is it related to the ATSD region/register?   
> 
> NPU is the source of the information on the host. ATSD is just another
> feature of NPU.
> 
> > Is this tgt base used in
> > the process of doing address translation shootdowns?  
> 
> No, only routing.
> 
> 
> >>>> +};
> >>>> +
> >>>>  /**
> >>>>   * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
> >>>>   *				    struct vfio_irq_info)
> >>>> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> >>>> index 4a3b93e..e9afd43 100644
> >>>> --- a/drivers/vfio/pci/vfio_pci.c
> >>>> +++ b/drivers/vfio/pci/vfio_pci.c
> >>>> @@ -224,6 +224,16 @@ static bool vfio_pci_nointx(struct pci_dev *pdev)
> >>>>  	return false;
> >>>>  }
> >>>>  
> >>>> +int __weak vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev)
> >>>> +{
> >>>> +	return -ENODEV;
> >>>> +}
> >>>> +
> >>>> +int __weak vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev)
> >>>> +{
> >>>> +	return -ENODEV;
> >>>> +}
> >>>> +
> >>>>  static int vfio_pci_enable(struct vfio_pci_device *vdev)
> >>>>  {
> >>>>  	struct pci_dev *pdev = vdev->pdev;
> >>>> @@ -302,14 +312,37 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
> >>>>  		if (ret) {
> >>>>  			dev_warn(&vdev->pdev->dev,
> >>>>  				 "Failed to setup Intel IGD regions\n");
> >>>> -			vfio_pci_disable(vdev);
> >>>> -			return ret;
> >>>> +			goto disable_exit;
> >>>> +		}
> >>>> +	}
> >>>> +
> >>>> +	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
> >>>> +	    pdev->device == 0x1db1) {
> >>>> +		ret = vfio_pci_nvdia_v100_nvlink2_init(vdev);
> >>>> +		if (ret) {
> >>>> +			dev_warn(&vdev->pdev->dev,
> >>>> +				 "Failed to setup NVIDIA NV2 RAM region\n");
> >>>> +			goto disable_exit;
> >>>> +		}
> >>>> +	}    
> >>>
> >>> This device ID is not unique to POWER9 Witherspoon systems, I see your
> >>> comment in the commitlog, but this is clearly going to generate a
> >>> dev_warn and failure on an x86 system with the same hardware.  Perhaps
> >>> this could be masked off with IS_ENABLED(CONFIG_VFIO_PCI_NVLINK2) like
> >>> the IGD code above this chunk does?    
> >>
> >> Right, will fix.
> >>
> >>  
> >>>> +
> >>>> +	if (pdev->vendor == PCI_VENDOR_ID_IBM &&
> >>>> +			pdev->device == 0x04ea) {
> >>>> +		ret = vfio_pci_ibm_npu2_init(vdev);
> >>>> +		if (ret) {
> >>>> +			dev_warn(&vdev->pdev->dev,
> >>>> +					"Failed to setup NVIDIA NV2 ATSD region\n");
> >>>> +			goto disable_exit;
> >>>>  		}    
> >>>
> >>> So the NPU is also actually owned by vfio-pci and assigned to the VM?    
> >>
> >> Yes. On a running system it looks like:
> >>
> >> 0007:00:00.0 Bridge: IBM Device 04ea (rev 01)
> >> 0007:00:00.1 Bridge: IBM Device 04ea (rev 01)
> >> 0007:00:01.0 Bridge: IBM Device 04ea (rev 01)
> >> 0007:00:01.1 Bridge: IBM Device 04ea (rev 01)
> >> 0007:00:02.0 Bridge: IBM Device 04ea (rev 01)
> >> 0007:00:02.1 Bridge: IBM Device 04ea (rev 01)
> >> 0035:00:00.0 PCI bridge: IBM Device 04c1
> >> 0035:01:00.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> >> 0035:02:04.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> >> 0035:02:05.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> >> 0035:02:0d.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> >> 0035:03:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
> >> (rev a1
> >> 0035:04:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
> >> (rev a1)
> >> 0035:05:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
> >> (rev a1)
> >>
> >> One "IBM Device" bridge represents one NVLink2, i.e. a piece of NPU.
> >> They all and 3 GPUs go to the same IOMMU group and get passed through to
> >> a guest.
> >>
> >> The entire NPU does not have representation via sysfs as a whole though.  
> > 
> > So the NPU is a bridge, but it uses a normal header type so vfio-pci
> > will bind to it?   
> 
> An NPU is a NVLink bridge, it is not PCI in any sense. We (the host
> powerpc firmware known as "skiboot" or "opal") have chosen to emulate a
> virtual bridge per 1 NVLink on the firmware level. So for each physical
> NPU there are 6 virtual bridges. So the NVIDIA driver does not need to
> know much about NPUs.
> 
> > And the ATSD register that we need on it is not
> > accessible through these PCI representations of the sub-pieces of the
> > NPU?  Thanks,  
> 
> No, only via the device tree. The skiboot puts the ATSD register address
> to the PHB's DT property called 'ibm,mmio-atsd' of these virtual bridges.

Ok, so the NPU is essential a virtual device already, mostly just a
stub.  But it seems that each NPU is associated to a specific GPU, how
is that association done?  In the use case here it seems like it's just
a vehicle to provide this ibm,mmio-atsd property to guest DT and the tgt
routing information to the GPU.  So if both of those were attached to
the GPU, there'd be no purpose in assigning the NPU other than it's in
the same IOMMU group with a type 0 header, so something needs to be
done with it.  If it's a virtual device, perhaps it could have a type 1
header so vfio wouldn't care about it, then we would only assign the
GPU with these extra properties, which seems easier for management
tools and users.  If the guest driver needs a visible NPU device, QEMU
could possibly emulate one to make the GPU association work
automatically.  Maybe this isn't really a problem, but I wonder if
you've looked up the management stack to see what tools need to know to
assign these NPU devices and whether specific configurations are
required to make the NPU to GPU association work.  Thanks,

Alex

On 10/18/18 9:55 AM, Alex Williamson wrote:
> On Thu, 18 Oct 2018 11:31:33 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
>> On 18/10/2018 08:52, Alex Williamson wrote:
>>> On Wed, 17 Oct 2018 12:19:20 +1100
>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>    
>>>> On 17/10/2018 06:08, Alex Williamson wrote:
>>>>> On Mon, 15 Oct 2018 20:42:33 +1100
>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>      
>>>>>> POWER9 Witherspoon machines come with 4 or 6 V100 GPUs which are not
>>>>>> pluggable PCIe devices but implement PCIe links for config space and MMIO.
>>>>>> In addition to that the GPUs are interconnected to each other and also
>>>>>> have direct links to the P9 CPU. The links are NVLink2 and provide direct
>>>>>> access to the system RAM for GPUs via NPU (an NVLink2 "proxy" on P9 chip).
>>>>>> These systems also support ATS (address translation services) which is
>>>>>> a part of the NVLink2 prototol. Such GPUs also share on-board RAM
>>>>>> (16GB in tested config) to the system via the same NVLink2 so a CPU has
>>>>>> cache-coherent access to a GPU RAM.
>>>>>>
>>>>>> This exports GPU RAM to the userspace as a new PCI region. This
>>>>>> preregisters the new memory as device memory as it might be used for DMA.
>>>>>> This inserts pfns from the fault handler as the GPU memory is not onlined
>>>>>> until the NVIDIA driver is loaded and trained the links so doing this
>>>>>> earlier produces low level errors which we fence in the firmware so
>>>>>> it does not hurt the host system but still better to avoid.
>>>>>>
>>>>>> This exports ATSD (Address Translation Shootdown) register of NPU which
>>>>>> allows the guest to invalidate TLB. The register conviniently occupies
>>>>>> a single 64k page. Since NPU maps the GPU memory, it has a "tgt" property
>>>>>> (which is an abbreviated host system bus address). This exports the "tgt"
>>>>>> as a capability so the guest can program it into the GPU so the GPU can
>>>>>> know how to route DMA trafic.
>>>>>
>>>>> I'm not really following what "tgt" is and why it's needed.  Is the GPU
>>>>> memory here different than the GPU RAM region above?  Why does the user
>>>>> need the host system bus address of this "tgt" thing?  Are we not able
>>>>> to relocate it in guest physical address space, does this shootdown
>>>>> only work in the host physical address space and therefore we need this
>>>>> offset?  Please explain, I'm confused.
>>>>
>>>>
>>>> This "tgt" is made of:
>>>> - "memory select" (bits 45, 46)
>>>> - "group select" (bits 43, 44)
>>>> - "chip select" (bit 42)
>>>> - chip internal address (bits 0..41)
>>>>
>>>> These are internal to GPU and this is where GPU RAM is mapped into the
>>>> GPU's real space, this fits 46 bits.
>>>>
>>>> On POWER9 CPU the bits are different and higher so the same memory is
>>>> mapped higher on P9 CPU. Just because we can map it higher, I guess.
>>>>
>>>> So it is not exactly the address but this provides the exact physical
>>>> location of the memory.
>>>>
>>>> We have a group of 3 interconnected GPUs, they got their own
>>>> memory/group/chip numbers. The GPUs use ATS service to translate
>>>> userspace to physical (host or guest) addresses. Now a GPU needs to know
>>>> which specific link to use for a specific physical address, in other
>>>> words what this physical address belongs to - a CPU or one of GPUs. This
>>>> is when "tgt" is used by the GPU hardware.
>>>
>>> Clear as mud ;)
>>
>> /me is sad. I hope Piotr explained it better...
> 
> It's starting to be a bit more clear, and Piotr anticipated the
> security questions I was mulling over.

Great.

> 
>>> So tgt, provided by the npu2 capability of the ATSD
>>> region of the NPU tells the GPU (a completely separate device) how to
>>> route it its own RAM via its NVLink interface?  How can one tgt
>>> indicate the routing for multiple interfaces?
>>
>> This NVLink DMA is using direct host physical addresses (no IOMMU, no
>> filtering) which come from ATS. So unless we tell the GPU its own
>> address range on the host CPU, it will route trafic via CPU. And the
>> driver can also discover the NVLink topology and tell each GPU physical
>> addresses of peer GPUs.
> 
> I think this is a key point too, the tgt seems to only identify the
> routing for the GPU local RAM, but the guest driver is able to
> conglomerate this information so each GPU knows how to get directly to
> the other GPUs.

Yes.

> 
>>>> A GPU could run all the DMA trafic via the system bus indeed, just not
>>>> as fast.
>>>>
>>>> I am also struggling here and adding an Nvidia person in cc: (I should
>>>> have done that when I posted the patches, my bad) to correct when/if I
>>>> am wrong.
>>>>
>>>>
>>>>   
>>>>>       
>>>>>> For ATS to work, the nest MMU (an NVIDIA block in a P9 CPU) needs to
>>>>>> know LPID (a logical partition ID or a KVM guest hardware ID in other
>>>>>> words) and PID (a memory context ID of an userspace process, not to be
>>>>>> confused with a linux pid). This assigns a GPU to LPID in the NPU and
>>>>>> this is why this adds a listener for KVM on an IOMMU group. A PID comes
>>>>>> via NVLink from a GPU and NPU uses a PID wildcard to pass it through.
>>>>>>
>>>>>> This requires coherent memory and ATSD to be available on the host as
>>>>>> the GPU vendor only supports configurations with both features enabled
>>>>>> and other configurations are known not to work. Because of this and
>>>>>> because of the ways the features are advertised to the host system
>>>>>> (which is a device tree with very platform specific properties),
>>>>>> this requires enabled POWERNV platform.
>>>>>>
>>>>>> This hardcodes the NVLink2 support for specific vendor and device IDs
>>>>>> as there is no reliable way of knowing about coherent memory and ATS
>>>>>> support. The GPU has an unique vendor PCIe capability 0x23 but it was
>>>>>> confirmed that it does not provide required information (and it is still
>>>>>> undisclosed what it actually does).
>>>>>>
>>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>>>> ---
>>>>>>   drivers/vfio/pci/Makefile           |   1 +
>>>>>>   drivers/vfio/pci/vfio_pci_private.h |   2 +
>>>>>>   include/uapi/linux/vfio.h           |  18 ++
>>>>>>   drivers/vfio/pci/vfio_pci.c         |  37 +++-
>>>>>>   drivers/vfio/pci/vfio_pci_nvlink2.c | 409 ++++++++++++++++++++++++++++++++++++
>>>>>>   drivers/vfio/pci/Kconfig            |   4 +
>>>>>>   6 files changed, 469 insertions(+), 2 deletions(-)
>>>>>>   create mode 100644 drivers/vfio/pci/vfio_pci_nvlink2.c
>>>>>>
>>>>>> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
>>>>>> index 76d8ec0..9662c06 100644
>>>>>> --- a/drivers/vfio/pci/Makefile
>>>>>> +++ b/drivers/vfio/pci/Makefile
>>>>>> @@ -1,5 +1,6 @@
>>>>>>   
>>>>>>   vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
>>>>>>   vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o
>>>>>> +vfio-pci-$(CONFIG_VFIO_PCI_NVLINK2) += vfio_pci_nvlink2.o
>>>>>>   
>>>>>>   obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
>>>>>> diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
>>>>>> index 93c1738..7639241 100644
>>>>>> --- a/drivers/vfio/pci/vfio_pci_private.h
>>>>>> +++ b/drivers/vfio/pci/vfio_pci_private.h
>>>>>> @@ -163,4 +163,6 @@ static inline int vfio_pci_igd_init(struct vfio_pci_device *vdev)
>>>>>>   	return -ENODEV;
>>>>>>   }
>>>>>>   #endif
>>>>>> +extern int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev);
>>>>>> +extern int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev);
>>>>>>   #endif /* VFIO_PCI_PRIVATE_H */
>>>>>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>>>>>> index f378b98..9e9a8d3 100644
>>>>>> --- a/include/uapi/linux/vfio.h
>>>>>> +++ b/include/uapi/linux/vfio.h
>>>>>> @@ -303,6 +303,12 @@ struct vfio_region_info_cap_type {
>>>>>>   #define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG	(2)
>>>>>>   #define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG	(3)
>>>>>>   
>>>>>> +/* NVIDIA GPU NVlink2 RAM */
>>>>>> +#define VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM	(1)
>>>>>> +
>>>>>> +/* IBM NPU NVlink2 ATSD */
>>>>>> +#define VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD	(1)
>>>>>> +
>>>>>
>>>>> Please include some of the description in the commitlog here for
>>>>> reference.  Also please be explicit that these are vendor defined
>>>>> regions and note the numerical vendor ID associated with them.
>>>>
>>>> These are PCI region subtypes which are not from any PCI spec and which
>>>> we define as we like, are not they all "vendor"?
>>>
>>> No, it's not entirely obvious that they're vendor regions, I had to
>>> look at the usage later in the patch.  See linux-next where Gerd has
>>> added an EDID region which is not a vendor region.  You make use of
>>> these by masking VFIO_REGION_TYPE_PCI_VENDOR_TYPE into the type, which
>>> indicates they are a PCI vendor type region, which means they are
>>> associated to a specific vendor.  That vendor should be explicitly,
>>> numerically, indicated (some hardware vendors own multiple PCI vendor
>>> IDs or may acquire more).
>>
>> Ah, I see now. ok.
>>
>>
>>>>>>   /*
>>>>>>    * The MSIX mappable capability informs that MSIX data of a BAR can be mmapped
>>>>>>    * which allows direct access to non-MSIX registers which happened to be within
>>>>>> @@ -313,6 +319,18 @@ struct vfio_region_info_cap_type {
>>>>>>    */
>>>>>>   #define VFIO_REGION_INFO_CAP_MSIX_MAPPABLE	3
>>>>>>   
>>>>>> +/*
>>>>>> + * Capability with compressed real address (aka SSA - small system address)
>>>>>> + * where GPU RAM is mapped on a system bus. Used by a GPU for DMA routing.
>>>>>> + */
>>>>>> +#define VFIO_REGION_INFO_CAP_NPU2		4
>>>>>> +
>>>>>> +struct vfio_region_info_cap_npu2 {
>>>>>> +	struct vfio_info_cap_header header;
>>>>>> +	__u64 tgt;
>>>>>> +	/* size is defined in VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM */
>>>>>
>>>>> But this is a capability for the IBM_NVLINK2_ATSD?  What is the
>>>>> relevance of this comment?  Is this capability relevant to the RAM or
>>>>> ATSD?
>>>>
>>>> It is relevant to NPU (NVLink host bus adapter of POWER9) which maps the
>>>> GPU RAM to the system bus and acts as a proxy to nestMMU (NVIDIA's unit
>>>> in POWER9 CPU) for ATS/ATSD services so it is a property of NPU. But
>>>> then one might ask "wait, here is the address, where is the size then",
>>>>   hence the comment...
>>>
>>> So the tgt field within the npu2 capability of the ATSD region on the
>>> NPU device describes the GPU internal address of the GPU RAM which is
>>> described by the NVLINK2 RAM region on the GPU device... *twitch*  What
>>> business does the NPU have exposing this piece of information and how
>>> is it related to the ATSD region/register?
>>
>> NPU is the source of the information on the host. ATSD is just another
>> feature of NPU.
>>
>>> Is this tgt base used in
>>> the process of doing address translation shootdowns?
>>
>> No, only routing.
>>
>>
>>>>>> +};
>>>>>> +
>>>>>>   /**
>>>>>>    * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
>>>>>>    *				    struct vfio_irq_info)
>>>>>> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
>>>>>> index 4a3b93e..e9afd43 100644
>>>>>> --- a/drivers/vfio/pci/vfio_pci.c
>>>>>> +++ b/drivers/vfio/pci/vfio_pci.c
>>>>>> @@ -224,6 +224,16 @@ static bool vfio_pci_nointx(struct pci_dev *pdev)
>>>>>>   	return false;
>>>>>>   }
>>>>>>   
>>>>>> +int __weak vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev)
>>>>>> +{
>>>>>> +	return -ENODEV;
>>>>>> +}
>>>>>> +
>>>>>> +int __weak vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev)
>>>>>> +{
>>>>>> +	return -ENODEV;
>>>>>> +}
>>>>>> +
>>>>>>   static int vfio_pci_enable(struct vfio_pci_device *vdev)
>>>>>>   {
>>>>>>   	struct pci_dev *pdev = vdev->pdev;
>>>>>> @@ -302,14 +312,37 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
>>>>>>   		if (ret) {
>>>>>>   			dev_warn(&vdev->pdev->dev,
>>>>>>   				 "Failed to setup Intel IGD regions\n");
>>>>>> -			vfio_pci_disable(vdev);
>>>>>> -			return ret;
>>>>>> +			goto disable_exit;
>>>>>> +		}
>>>>>> +	}
>>>>>> +
>>>>>> +	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
>>>>>> +	    pdev->device == 0x1db1) {
>>>>>> +		ret = vfio_pci_nvdia_v100_nvlink2_init(vdev);
>>>>>> +		if (ret) {
>>>>>> +			dev_warn(&vdev->pdev->dev,
>>>>>> +				 "Failed to setup NVIDIA NV2 RAM region\n");
>>>>>> +			goto disable_exit;
>>>>>> +		}
>>>>>> +	}
>>>>>
>>>>> This device ID is not unique to POWER9 Witherspoon systems, I see your
>>>>> comment in the commitlog, but this is clearly going to generate a
>>>>> dev_warn and failure on an x86 system with the same hardware.  Perhaps
>>>>> this could be masked off with IS_ENABLED(CONFIG_VFIO_PCI_NVLINK2) like
>>>>> the IGD code above this chunk does?
>>>>
>>>> Right, will fix.
>>>>
>>>>   
>>>>>> +
>>>>>> +	if (pdev->vendor == PCI_VENDOR_ID_IBM &&
>>>>>> +			pdev->device == 0x04ea) {
>>>>>> +		ret = vfio_pci_ibm_npu2_init(vdev);
>>>>>> +		if (ret) {
>>>>>> +			dev_warn(&vdev->pdev->dev,
>>>>>> +					"Failed to setup NVIDIA NV2 ATSD region\n");
>>>>>> +			goto disable_exit;
>>>>>>   		}
>>>>>
>>>>> So the NPU is also actually owned by vfio-pci and assigned to the VM?
>>>>
>>>> Yes. On a running system it looks like:
>>>>
>>>> 0007:00:00.0 Bridge: IBM Device 04ea (rev 01)
>>>> 0007:00:00.1 Bridge: IBM Device 04ea (rev 01)
>>>> 0007:00:01.0 Bridge: IBM Device 04ea (rev 01)
>>>> 0007:00:01.1 Bridge: IBM Device 04ea (rev 01)
>>>> 0007:00:02.0 Bridge: IBM Device 04ea (rev 01)
>>>> 0007:00:02.1 Bridge: IBM Device 04ea (rev 01)
>>>> 0035:00:00.0 PCI bridge: IBM Device 04c1
>>>> 0035:01:00.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>> 0035:02:04.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>> 0035:02:05.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>> 0035:02:0d.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>> 0035:03:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>>>> (rev a1
>>>> 0035:04:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>>>> (rev a1)
>>>> 0035:05:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>>>> (rev a1)
>>>>
>>>> One "IBM Device" bridge represents one NVLink2, i.e. a piece of NPU.
>>>> They all and 3 GPUs go to the same IOMMU group and get passed through to
>>>> a guest.
>>>>
>>>> The entire NPU does not have representation via sysfs as a whole though.
>>>
>>> So the NPU is a bridge, but it uses a normal header type so vfio-pci
>>> will bind to it?
>>
>> An NPU is a NVLink bridge, it is not PCI in any sense. We (the host
>> powerpc firmware known as "skiboot" or "opal") have chosen to emulate a
>> virtual bridge per 1 NVLink on the firmware level. So for each physical
>> NPU there are 6 virtual bridges. So the NVIDIA driver does not need to
>> know much about NPUs.
>>
>>> And the ATSD register that we need on it is not
>>> accessible through these PCI representations of the sub-pieces of the
>>> NPU?  Thanks,
>>
>> No, only via the device tree. The skiboot puts the ATSD register address
>> to the PHB's DT property called 'ibm,mmio-atsd' of these virtual bridges.
> 
> Ok, so the NPU is essential a virtual device already, mostly just a
> stub.  But it seems that each NPU is associated to a specific GPU, how
> is that association done?  In the use case here it seems like it's just
> a vehicle to provide this ibm,mmio-atsd property to guest DT and the tgt
> routing information to the GPU.  So if both of those were attached to
> the GPU, there'd be no purpose in assigning the NPU other than it's in
> the same IOMMU group with a type 0 header, so something needs to be
> done with it.  If it's a virtual device, perhaps it could have a type 1
> header so vfio wouldn't care about it, then we would only assign the
> GPU with these extra properties, which seems easier for management
> tools and users.  If the guest driver needs a visible NPU device, QEMU
> could possibly emulate one to make the GPU association work
> automatically.  Maybe this isn't really a problem, but I wonder if
> you've looked up the management stack to see what tools need to know to
> assign these NPU devices and whether specific configurations are
> required to make the NPU to GPU association work.  Thanks,

I'm not that familiar with how this was originally set up, but note that 
Alexey is just making it work exactly like baremetal does. The baremetal 
GPU driver works as-is in the VM and expects the same properties in the 
device-tree. Obviously it doesn't have to be that way, but there is 
value in keeping it identical.

Another probably bigger point is that the NPU device also implements the 
nvlink HW interface and is required for actually training and 
maintaining the link up. The driver in the guest trains the links by 
programming both the GPU end and the NPU end of each link so the NPU 
device needs to be exposed to the guest.

Thanks,
Piotr

> 
> Alex
>

On Thu, 18 Oct 2018 10:37:46 -0700
Piotr Jaroszynski <pjaroszynski@nvidia.com> wrote:

> On 10/18/18 9:55 AM, Alex Williamson wrote:
> > On Thu, 18 Oct 2018 11:31:33 +1100
> > Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> >   
> >> On 18/10/2018 08:52, Alex Williamson wrote:  
> >>> On Wed, 17 Oct 2018 12:19:20 +1100
> >>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> >>>      
> >>>> On 17/10/2018 06:08, Alex Williamson wrote:  
> >>>>> On Mon, 15 Oct 2018 20:42:33 +1100
> >>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:    
> >>>>>> +
> >>>>>> +	if (pdev->vendor == PCI_VENDOR_ID_IBM &&
> >>>>>> +			pdev->device == 0x04ea) {
> >>>>>> +		ret = vfio_pci_ibm_npu2_init(vdev);
> >>>>>> +		if (ret) {
> >>>>>> +			dev_warn(&vdev->pdev->dev,
> >>>>>> +					"Failed to setup NVIDIA NV2 ATSD region\n");
> >>>>>> +			goto disable_exit;
> >>>>>>   		}  
> >>>>>
> >>>>> So the NPU is also actually owned by vfio-pci and assigned to the VM?  
> >>>>
> >>>> Yes. On a running system it looks like:
> >>>>
> >>>> 0007:00:00.0 Bridge: IBM Device 04ea (rev 01)
> >>>> 0007:00:00.1 Bridge: IBM Device 04ea (rev 01)
> >>>> 0007:00:01.0 Bridge: IBM Device 04ea (rev 01)
> >>>> 0007:00:01.1 Bridge: IBM Device 04ea (rev 01)
> >>>> 0007:00:02.0 Bridge: IBM Device 04ea (rev 01)
> >>>> 0007:00:02.1 Bridge: IBM Device 04ea (rev 01)
> >>>> 0035:00:00.0 PCI bridge: IBM Device 04c1
> >>>> 0035:01:00.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> >>>> 0035:02:04.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> >>>> 0035:02:05.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> >>>> 0035:02:0d.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> >>>> 0035:03:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
> >>>> (rev a1
> >>>> 0035:04:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
> >>>> (rev a1)
> >>>> 0035:05:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
> >>>> (rev a1)
> >>>>
> >>>> One "IBM Device" bridge represents one NVLink2, i.e. a piece of NPU.
> >>>> They all and 3 GPUs go to the same IOMMU group and get passed through to
> >>>> a guest.
> >>>>
> >>>> The entire NPU does not have representation via sysfs as a whole though.  
> >>>
> >>> So the NPU is a bridge, but it uses a normal header type so vfio-pci
> >>> will bind to it?  
> >>
> >> An NPU is a NVLink bridge, it is not PCI in any sense. We (the host
> >> powerpc firmware known as "skiboot" or "opal") have chosen to emulate a
> >> virtual bridge per 1 NVLink on the firmware level. So for each physical
> >> NPU there are 6 virtual bridges. So the NVIDIA driver does not need to
> >> know much about NPUs.
> >>  
> >>> And the ATSD register that we need on it is not
> >>> accessible through these PCI representations of the sub-pieces of the
> >>> NPU?  Thanks,  
> >>
> >> No, only via the device tree. The skiboot puts the ATSD register address
> >> to the PHB's DT property called 'ibm,mmio-atsd' of these virtual bridges.  
> > 
> > Ok, so the NPU is essential a virtual device already, mostly just a
> > stub.  But it seems that each NPU is associated to a specific GPU, how
> > is that association done?  In the use case here it seems like it's just
> > a vehicle to provide this ibm,mmio-atsd property to guest DT and the tgt
> > routing information to the GPU.  So if both of those were attached to
> > the GPU, there'd be no purpose in assigning the NPU other than it's in
> > the same IOMMU group with a type 0 header, so something needs to be
> > done with it.  If it's a virtual device, perhaps it could have a type 1
> > header so vfio wouldn't care about it, then we would only assign the
> > GPU with these extra properties, which seems easier for management
> > tools and users.  If the guest driver needs a visible NPU device, QEMU
> > could possibly emulate one to make the GPU association work
> > automatically.  Maybe this isn't really a problem, but I wonder if
> > you've looked up the management stack to see what tools need to know to
> > assign these NPU devices and whether specific configurations are
> > required to make the NPU to GPU association work.  Thanks,  
> 
> I'm not that familiar with how this was originally set up, but note that 
> Alexey is just making it work exactly like baremetal does. The baremetal 
> GPU driver works as-is in the VM and expects the same properties in the 
> device-tree. Obviously it doesn't have to be that way, but there is 
> value in keeping it identical.
> 
> Another probably bigger point is that the NPU device also implements the 
> nvlink HW interface and is required for actually training and 
> maintaining the link up. The driver in the guest trains the links by 
> programming both the GPU end and the NPU end of each link so the NPU 
> device needs to be exposed to the guest.

Ok, so there is functionality in assigning the NPU device itself, it's
not just an attachment point for meta data.  But it still seems there
must be some association of NPU to GPU, the tgt address seems to pair
the NPU with a specific GPU, they're not simply a fungible set of NPUs
and GPUs.  Is that association explicit anywhere or is it related to
the topology or device numbering that needs to match between the host
and guest?  Thanks,

Alex

On 10/18/18 11:05 AM, Alex Williamson wrote:
> On Thu, 18 Oct 2018 10:37:46 -0700
> Piotr Jaroszynski <pjaroszynski@nvidia.com> wrote:
> 
>> On 10/18/18 9:55 AM, Alex Williamson wrote:
>>> On Thu, 18 Oct 2018 11:31:33 +1100
>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>    
>>>> On 18/10/2018 08:52, Alex Williamson wrote:
>>>>> On Wed, 17 Oct 2018 12:19:20 +1100
>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>       
>>>>>> On 17/10/2018 06:08, Alex Williamson wrote:
>>>>>>> On Mon, 15 Oct 2018 20:42:33 +1100
>>>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>>>> +
>>>>>>>> +	if (pdev->vendor == PCI_VENDOR_ID_IBM &&
>>>>>>>> +			pdev->device == 0x04ea) {
>>>>>>>> +		ret = vfio_pci_ibm_npu2_init(vdev);
>>>>>>>> +		if (ret) {
>>>>>>>> +			dev_warn(&vdev->pdev->dev,
>>>>>>>> +					"Failed to setup NVIDIA NV2 ATSD region\n");
>>>>>>>> +			goto disable_exit;
>>>>>>>>    		}
>>>>>>>
>>>>>>> So the NPU is also actually owned by vfio-pci and assigned to the VM?
>>>>>>
>>>>>> Yes. On a running system it looks like:
>>>>>>
>>>>>> 0007:00:00.0 Bridge: IBM Device 04ea (rev 01)
>>>>>> 0007:00:00.1 Bridge: IBM Device 04ea (rev 01)
>>>>>> 0007:00:01.0 Bridge: IBM Device 04ea (rev 01)
>>>>>> 0007:00:01.1 Bridge: IBM Device 04ea (rev 01)
>>>>>> 0007:00:02.0 Bridge: IBM Device 04ea (rev 01)
>>>>>> 0007:00:02.1 Bridge: IBM Device 04ea (rev 01)
>>>>>> 0035:00:00.0 PCI bridge: IBM Device 04c1
>>>>>> 0035:01:00.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>>>> 0035:02:04.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>>>> 0035:02:05.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>>>> 0035:02:0d.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>>>> 0035:03:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>>>>>> (rev a1
>>>>>> 0035:04:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>>>>>> (rev a1)
>>>>>> 0035:05:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>>>>>> (rev a1)
>>>>>>
>>>>>> One "IBM Device" bridge represents one NVLink2, i.e. a piece of NPU.
>>>>>> They all and 3 GPUs go to the same IOMMU group and get passed through to
>>>>>> a guest.
>>>>>>
>>>>>> The entire NPU does not have representation via sysfs as a whole though.
>>>>>
>>>>> So the NPU is a bridge, but it uses a normal header type so vfio-pci
>>>>> will bind to it?
>>>>
>>>> An NPU is a NVLink bridge, it is not PCI in any sense. We (the host
>>>> powerpc firmware known as "skiboot" or "opal") have chosen to emulate a
>>>> virtual bridge per 1 NVLink on the firmware level. So for each physical
>>>> NPU there are 6 virtual bridges. So the NVIDIA driver does not need to
>>>> know much about NPUs.
>>>>   
>>>>> And the ATSD register that we need on it is not
>>>>> accessible through these PCI representations of the sub-pieces of the
>>>>> NPU?  Thanks,
>>>>
>>>> No, only via the device tree. The skiboot puts the ATSD register address
>>>> to the PHB's DT property called 'ibm,mmio-atsd' of these virtual bridges.
>>>
>>> Ok, so the NPU is essential a virtual device already, mostly just a
>>> stub.  But it seems that each NPU is associated to a specific GPU, how
>>> is that association done?  In the use case here it seems like it's just
>>> a vehicle to provide this ibm,mmio-atsd property to guest DT and the tgt
>>> routing information to the GPU.  So if both of those were attached to
>>> the GPU, there'd be no purpose in assigning the NPU other than it's in
>>> the same IOMMU group with a type 0 header, so something needs to be
>>> done with it.  If it's a virtual device, perhaps it could have a type 1
>>> header so vfio wouldn't care about it, then we would only assign the
>>> GPU with these extra properties, which seems easier for management
>>> tools and users.  If the guest driver needs a visible NPU device, QEMU
>>> could possibly emulate one to make the GPU association work
>>> automatically.  Maybe this isn't really a problem, but I wonder if
>>> you've looked up the management stack to see what tools need to know to
>>> assign these NPU devices and whether specific configurations are
>>> required to make the NPU to GPU association work.  Thanks,
>>
>> I'm not that familiar with how this was originally set up, but note that
>> Alexey is just making it work exactly like baremetal does. The baremetal
>> GPU driver works as-is in the VM and expects the same properties in the
>> device-tree. Obviously it doesn't have to be that way, but there is
>> value in keeping it identical.
>>
>> Another probably bigger point is that the NPU device also implements the
>> nvlink HW interface and is required for actually training and
>> maintaining the link up. The driver in the guest trains the links by
>> programming both the GPU end and the NPU end of each link so the NPU
>> device needs to be exposed to the guest.
> 
> Ok, so there is functionality in assigning the NPU device itself, it's
> not just an attachment point for meta data.  But it still seems there
> must be some association of NPU to GPU, the tgt address seems to pair
> the NPU with a specific GPU, they're not simply a fungible set of NPUs
> and GPUs.  Is that association explicit anywhere or is it related to
> the topology or device numbering that needs to match between the host
> and guest?  Thanks,

GPUs are linked to NPU devices through device tree properties, I think. 
Linux has a helper to look up linked NPU devices for a PCI device 
pnv_pci_get_npu_dev() here:
https://elixir.bootlin.com/linux/latest/source/arch/powerpc/platforms/powernv/npu-dma.c#L86
 From what Alexey said, it sounds like the NPU devices are actually 
virtualized by the firmware, but they do map to real hardware in the end 
and each GPU needs to be accompanied by the same NPU devices as on 
baremetal. Currently, there are two types of P9+GV100 systems, one with 
2 CPU<->GPU links per GPU and hence 2 linked NPU devices per GPU. And a 
second system with 3 CPU<->GPU links per GPU and hence 3 linked NPU 
devices per GPU.

Having the user assign all the NPUs and GPUs correctly does seem a bit 
cumbersome, but maybe higher level tools on top of qemu can simplify 
that somehow by pulling them in automatically? FWIW, whenever a GPU is 
assigned to a guest, its linked NPU devices are likely not very useful 
to anything else so grouping them together somehow would make sense.

> 
> Alex
>

On 19/10/2018 05:05, Alex Williamson wrote:
> On Thu, 18 Oct 2018 10:37:46 -0700
> Piotr Jaroszynski <pjaroszynski@nvidia.com> wrote:
> 
>> On 10/18/18 9:55 AM, Alex Williamson wrote:
>>> On Thu, 18 Oct 2018 11:31:33 +1100
>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>   
>>>> On 18/10/2018 08:52, Alex Williamson wrote:  
>>>>> On Wed, 17 Oct 2018 12:19:20 +1100
>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>      
>>>>>> On 17/10/2018 06:08, Alex Williamson wrote:  
>>>>>>> On Mon, 15 Oct 2018 20:42:33 +1100
>>>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:    
>>>>>>>> +
>>>>>>>> +	if (pdev->vendor == PCI_VENDOR_ID_IBM &&
>>>>>>>> +			pdev->device == 0x04ea) {
>>>>>>>> +		ret = vfio_pci_ibm_npu2_init(vdev);
>>>>>>>> +		if (ret) {
>>>>>>>> +			dev_warn(&vdev->pdev->dev,
>>>>>>>> +					"Failed to setup NVIDIA NV2 ATSD region\n");
>>>>>>>> +			goto disable_exit;
>>>>>>>>   		}  
>>>>>>>
>>>>>>> So the NPU is also actually owned by vfio-pci and assigned to the VM?  
>>>>>>
>>>>>> Yes. On a running system it looks like:
>>>>>>
>>>>>> 0007:00:00.0 Bridge: IBM Device 04ea (rev 01)
>>>>>> 0007:00:00.1 Bridge: IBM Device 04ea (rev 01)
>>>>>> 0007:00:01.0 Bridge: IBM Device 04ea (rev 01)
>>>>>> 0007:00:01.1 Bridge: IBM Device 04ea (rev 01)
>>>>>> 0007:00:02.0 Bridge: IBM Device 04ea (rev 01)
>>>>>> 0007:00:02.1 Bridge: IBM Device 04ea (rev 01)
>>>>>> 0035:00:00.0 PCI bridge: IBM Device 04c1
>>>>>> 0035:01:00.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>>>> 0035:02:04.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>>>> 0035:02:05.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>>>> 0035:02:0d.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>>>> 0035:03:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>>>>>> (rev a1
>>>>>> 0035:04:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>>>>>> (rev a1)
>>>>>> 0035:05:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>>>>>> (rev a1)
>>>>>>
>>>>>> One "IBM Device" bridge represents one NVLink2, i.e. a piece of NPU.
>>>>>> They all and 3 GPUs go to the same IOMMU group and get passed through to
>>>>>> a guest.
>>>>>>
>>>>>> The entire NPU does not have representation via sysfs as a whole though.  
>>>>>
>>>>> So the NPU is a bridge, but it uses a normal header type so vfio-pci
>>>>> will bind to it?  
>>>>
>>>> An NPU is a NVLink bridge, it is not PCI in any sense. We (the host
>>>> powerpc firmware known as "skiboot" or "opal") have chosen to emulate a
>>>> virtual bridge per 1 NVLink on the firmware level. So for each physical
>>>> NPU there are 6 virtual bridges. So the NVIDIA driver does not need to
>>>> know much about NPUs.
>>>>  
>>>>> And the ATSD register that we need on it is not
>>>>> accessible through these PCI representations of the sub-pieces of the
>>>>> NPU?  Thanks,  
>>>>
>>>> No, only via the device tree. The skiboot puts the ATSD register address
>>>> to the PHB's DT property called 'ibm,mmio-atsd' of these virtual bridges.  
>>>
>>> Ok, so the NPU is essential a virtual device already, mostly just a
>>> stub.  But it seems that each NPU is associated to a specific GPU, how
>>> is that association done?  In the use case here it seems like it's just
>>> a vehicle to provide this ibm,mmio-atsd property to guest DT and the tgt
>>> routing information to the GPU.  So if both of those were attached to
>>> the GPU, there'd be no purpose in assigning the NPU other than it's in
>>> the same IOMMU group with a type 0 header, so something needs to be
>>> done with it.  If it's a virtual device, perhaps it could have a type 1
>>> header so vfio wouldn't care about it, then we would only assign the
>>> GPU with these extra properties, which seems easier for management
>>> tools and users.  If the guest driver needs a visible NPU device, QEMU
>>> could possibly emulate one to make the GPU association work
>>> automatically.  Maybe this isn't really a problem, but I wonder if
>>> you've looked up the management stack to see what tools need to know to
>>> assign these NPU devices and whether specific configurations are
>>> required to make the NPU to GPU association work.  Thanks,  
>>
>> I'm not that familiar with how this was originally set up, but note that 
>> Alexey is just making it work exactly like baremetal does. The baremetal 
>> GPU driver works as-is in the VM and expects the same properties in the 
>> device-tree. Obviously it doesn't have to be that way, but there is 
>> value in keeping it identical.
>>
>> Another probably bigger point is that the NPU device also implements the 
>> nvlink HW interface and is required for actually training and 
>> maintaining the link up. The driver in the guest trains the links by 
>> programming both the GPU end and the NPU end of each link so the NPU 
>> device needs to be exposed to the guest.
> 
> Ok, so there is functionality in assigning the NPU device itself, it's
> not just an attachment point for meta data.  But it still seems there
> must be some association of NPU to GPU, the tgt address seems to pair
> the NPU with a specific GPU, they're not simply a fungible set of NPUs
> and GPUs.  Is that association explicit anywhere or is it related to
> the topology or device numbering that needs to match between the host
> and guest?  Thanks,

It is in the device tree (phandle is a node ID).

NPU:
xscom@623fc00000000/npu@5011000

NVLinks:
xscom@623fc00000000/npu@5011000/link@0
xscom@623fc00000000/npu@5011000/link@1
xscom@623fc00000000/npu@5011000/link@2
xscom@623fc00000000/npu@5011000/link@3
xscom@623fc00000000/npu@5011000/link@5
xscom@623fc00000000/npu@5011000/link@6

GPU RAM:
memory@240000000000
memory@242000000000
memory@244000000000

GPUs:
pciex@620c3c0500000/pci@0/pci@0/pci@4/3d-controller@0
	ibm,npu property - 2 phandles of associated virtual
		bridges as in my config a GPU has 2 NVLinks
		to the CPU (or NPU in particular)
pciex@620c3c0500000/pci@0/pci@0/pci@5/3d-controller@0
pciex@620c3c0500000/pci@0/pci@0/pci@d/3d-controller@0

Virtual bridges:
pciex@6230200000000/pci@0
	ibm,gpu property - a phandle of associated GPU
	memory-region property - a phandle of a GPU RAM block
	ibm,nvlink property - a phandle of an NVLink
	ibm,device-tgt-addr property - the short physical
		address of a GPU RAM (0x00000c00.00000000
		in this example)
pciex@6230200000000/pci@0,1
pciex@6230200000000/pci@1
pciex@6230200000000/pci@1,1
pciex@6230200000000/pci@2
pciex@6230200000000/pci@2,1

On Thu, Oct 18, 2018 at 11:40:19AM -0700, Piotr Jaroszynski wrote:
> GPUs are linked to NPU devices through device tree properties, I think.
> Linux has a helper to look up linked NPU devices for a PCI device
> pnv_pci_get_npu_dev() here:
> https://elixir.bootlin.com/linux/latest/source/arch/powerpc/platforms/powernv/npu-dma.c#L86

FYI, all this code has never been used in the three years it was in the
tree, so I'm going to remove it pretty soon.

On Fri, Oct 19, 2018 at 11:53:53AM +1100, Alexey Kardashevskiy wrote:
> 
> 
> On 19/10/2018 05:05, Alex Williamson wrote:
> > On Thu, 18 Oct 2018 10:37:46 -0700
> > Piotr Jaroszynski <pjaroszynski@nvidia.com> wrote:
> > 
> >> On 10/18/18 9:55 AM, Alex Williamson wrote:
> >>> On Thu, 18 Oct 2018 11:31:33 +1100
> >>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> >>>   
> >>>> On 18/10/2018 08:52, Alex Williamson wrote:  
> >>>>> On Wed, 17 Oct 2018 12:19:20 +1100
> >>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> >>>>>      
> >>>>>> On 17/10/2018 06:08, Alex Williamson wrote:  
> >>>>>>> On Mon, 15 Oct 2018 20:42:33 +1100
> >>>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:    
> >>>>>>>> +
> >>>>>>>> +	if (pdev->vendor == PCI_VENDOR_ID_IBM &&
> >>>>>>>> +			pdev->device == 0x04ea) {
> >>>>>>>> +		ret = vfio_pci_ibm_npu2_init(vdev);
> >>>>>>>> +		if (ret) {
> >>>>>>>> +			dev_warn(&vdev->pdev->dev,
> >>>>>>>> +					"Failed to setup NVIDIA NV2 ATSD region\n");
> >>>>>>>> +			goto disable_exit;
> >>>>>>>>   		}  
> >>>>>>>
> >>>>>>> So the NPU is also actually owned by vfio-pci and assigned to the VM?  
> >>>>>>
> >>>>>> Yes. On a running system it looks like:
> >>>>>>
> >>>>>> 0007:00:00.0 Bridge: IBM Device 04ea (rev 01)
> >>>>>> 0007:00:00.1 Bridge: IBM Device 04ea (rev 01)
> >>>>>> 0007:00:01.0 Bridge: IBM Device 04ea (rev 01)
> >>>>>> 0007:00:01.1 Bridge: IBM Device 04ea (rev 01)
> >>>>>> 0007:00:02.0 Bridge: IBM Device 04ea (rev 01)
> >>>>>> 0007:00:02.1 Bridge: IBM Device 04ea (rev 01)
> >>>>>> 0035:00:00.0 PCI bridge: IBM Device 04c1
> >>>>>> 0035:01:00.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> >>>>>> 0035:02:04.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> >>>>>> 0035:02:05.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> >>>>>> 0035:02:0d.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> >>>>>> 0035:03:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
> >>>>>> (rev a1
> >>>>>> 0035:04:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
> >>>>>> (rev a1)
> >>>>>> 0035:05:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
> >>>>>> (rev a1)
> >>>>>>
> >>>>>> One "IBM Device" bridge represents one NVLink2, i.e. a piece of NPU.
> >>>>>> They all and 3 GPUs go to the same IOMMU group and get passed through to
> >>>>>> a guest.
> >>>>>>
> >>>>>> The entire NPU does not have representation via sysfs as a whole though.  
> >>>>>
> >>>>> So the NPU is a bridge, but it uses a normal header type so vfio-pci
> >>>>> will bind to it?  
> >>>>
> >>>> An NPU is a NVLink bridge, it is not PCI in any sense. We (the host
> >>>> powerpc firmware known as "skiboot" or "opal") have chosen to emulate a
> >>>> virtual bridge per 1 NVLink on the firmware level. So for each physical
> >>>> NPU there are 6 virtual bridges. So the NVIDIA driver does not need to
> >>>> know much about NPUs.
> >>>>  
> >>>>> And the ATSD register that we need on it is not
> >>>>> accessible through these PCI representations of the sub-pieces of the
> >>>>> NPU?  Thanks,  
> >>>>
> >>>> No, only via the device tree. The skiboot puts the ATSD register address
> >>>> to the PHB's DT property called 'ibm,mmio-atsd' of these virtual bridges.  
> >>>
> >>> Ok, so the NPU is essential a virtual device already, mostly just a
> >>> stub.  But it seems that each NPU is associated to a specific GPU, how
> >>> is that association done?  In the use case here it seems like it's just
> >>> a vehicle to provide this ibm,mmio-atsd property to guest DT and the tgt
> >>> routing information to the GPU.  So if both of those were attached to
> >>> the GPU, there'd be no purpose in assigning the NPU other than it's in
> >>> the same IOMMU group with a type 0 header, so something needs to be
> >>> done with it.  If it's a virtual device, perhaps it could have a type 1
> >>> header so vfio wouldn't care about it, then we would only assign the
> >>> GPU with these extra properties, which seems easier for management
> >>> tools and users.  If the guest driver needs a visible NPU device, QEMU
> >>> could possibly emulate one to make the GPU association work
> >>> automatically.  Maybe this isn't really a problem, but I wonder if
> >>> you've looked up the management stack to see what tools need to know to
> >>> assign these NPU devices and whether specific configurations are
> >>> required to make the NPU to GPU association work.  Thanks,  
> >>
> >> I'm not that familiar with how this was originally set up, but note that 
> >> Alexey is just making it work exactly like baremetal does. The baremetal 
> >> GPU driver works as-is in the VM and expects the same properties in the 
> >> device-tree. Obviously it doesn't have to be that way, but there is 
> >> value in keeping it identical.
> >>
> >> Another probably bigger point is that the NPU device also implements the 
> >> nvlink HW interface and is required for actually training and 
> >> maintaining the link up. The driver in the guest trains the links by 
> >> programming both the GPU end and the NPU end of each link so the NPU 
> >> device needs to be exposed to the guest.
> > 
> > Ok, so there is functionality in assigning the NPU device itself, it's
> > not just an attachment point for meta data.  But it still seems there
> > must be some association of NPU to GPU, the tgt address seems to pair
> > the NPU with a specific GPU, they're not simply a fungible set of NPUs
> > and GPUs.  Is that association explicit anywhere or is it related to
> > the topology or device numbering that needs to match between the host
> > and guest?  Thanks,
> 
> It is in the device tree (phandle is a node ID).

Hrm.  But the device tree just publishes information about the
hardware.  What's the device tree value actually exposing here?

Is there an inherent hardware connection between one NPU and one GPU?
Or is there just an arbitrary assignment performed by the firmware
which it then exposed to the device tree?

On 12/11/2018 12:08, David Gibson wrote:
> On Fri, Oct 19, 2018 at 11:53:53AM +1100, Alexey Kardashevskiy wrote:
>>
>>
>> On 19/10/2018 05:05, Alex Williamson wrote:
>>> On Thu, 18 Oct 2018 10:37:46 -0700
>>> Piotr Jaroszynski <pjaroszynski@nvidia.com> wrote:
>>>
>>>> On 10/18/18 9:55 AM, Alex Williamson wrote:
>>>>> On Thu, 18 Oct 2018 11:31:33 +1100
>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>   
>>>>>> On 18/10/2018 08:52, Alex Williamson wrote:  
>>>>>>> On Wed, 17 Oct 2018 12:19:20 +1100
>>>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>>>      
>>>>>>>> On 17/10/2018 06:08, Alex Williamson wrote:  
>>>>>>>>> On Mon, 15 Oct 2018 20:42:33 +1100
>>>>>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:    
>>>>>>>>>> +
>>>>>>>>>> +	if (pdev->vendor == PCI_VENDOR_ID_IBM &&
>>>>>>>>>> +			pdev->device == 0x04ea) {
>>>>>>>>>> +		ret = vfio_pci_ibm_npu2_init(vdev);
>>>>>>>>>> +		if (ret) {
>>>>>>>>>> +			dev_warn(&vdev->pdev->dev,
>>>>>>>>>> +					"Failed to setup NVIDIA NV2 ATSD region\n");
>>>>>>>>>> +			goto disable_exit;
>>>>>>>>>>   		}  
>>>>>>>>>
>>>>>>>>> So the NPU is also actually owned by vfio-pci and assigned to the VM?  
>>>>>>>>
>>>>>>>> Yes. On a running system it looks like:
>>>>>>>>
>>>>>>>> 0007:00:00.0 Bridge: IBM Device 04ea (rev 01)
>>>>>>>> 0007:00:00.1 Bridge: IBM Device 04ea (rev 01)
>>>>>>>> 0007:00:01.0 Bridge: IBM Device 04ea (rev 01)
>>>>>>>> 0007:00:01.1 Bridge: IBM Device 04ea (rev 01)
>>>>>>>> 0007:00:02.0 Bridge: IBM Device 04ea (rev 01)
>>>>>>>> 0007:00:02.1 Bridge: IBM Device 04ea (rev 01)
>>>>>>>> 0035:00:00.0 PCI bridge: IBM Device 04c1
>>>>>>>> 0035:01:00.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>>>>>> 0035:02:04.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>>>>>> 0035:02:05.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>>>>>> 0035:02:0d.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>>>>>> 0035:03:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>>>>>>>> (rev a1
>>>>>>>> 0035:04:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>>>>>>>> (rev a1)
>>>>>>>> 0035:05:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>>>>>>>> (rev a1)
>>>>>>>>
>>>>>>>> One "IBM Device" bridge represents one NVLink2, i.e. a piece of NPU.
>>>>>>>> They all and 3 GPUs go to the same IOMMU group and get passed through to
>>>>>>>> a guest.
>>>>>>>>
>>>>>>>> The entire NPU does not have representation via sysfs as a whole though.  
>>>>>>>
>>>>>>> So the NPU is a bridge, but it uses a normal header type so vfio-pci
>>>>>>> will bind to it?  
>>>>>>
>>>>>> An NPU is a NVLink bridge, it is not PCI in any sense. We (the host
>>>>>> powerpc firmware known as "skiboot" or "opal") have chosen to emulate a
>>>>>> virtual bridge per 1 NVLink on the firmware level. So for each physical
>>>>>> NPU there are 6 virtual bridges. So the NVIDIA driver does not need to
>>>>>> know much about NPUs.
>>>>>>  
>>>>>>> And the ATSD register that we need on it is not
>>>>>>> accessible through these PCI representations of the sub-pieces of the
>>>>>>> NPU?  Thanks,  
>>>>>>
>>>>>> No, only via the device tree. The skiboot puts the ATSD register address
>>>>>> to the PHB's DT property called 'ibm,mmio-atsd' of these virtual bridges.  
>>>>>
>>>>> Ok, so the NPU is essential a virtual device already, mostly just a
>>>>> stub.  But it seems that each NPU is associated to a specific GPU, how
>>>>> is that association done?  In the use case here it seems like it's just
>>>>> a vehicle to provide this ibm,mmio-atsd property to guest DT and the tgt
>>>>> routing information to the GPU.  So if both of those were attached to
>>>>> the GPU, there'd be no purpose in assigning the NPU other than it's in
>>>>> the same IOMMU group with a type 0 header, so something needs to be
>>>>> done with it.  If it's a virtual device, perhaps it could have a type 1
>>>>> header so vfio wouldn't care about it, then we would only assign the
>>>>> GPU with these extra properties, which seems easier for management
>>>>> tools and users.  If the guest driver needs a visible NPU device, QEMU
>>>>> could possibly emulate one to make the GPU association work
>>>>> automatically.  Maybe this isn't really a problem, but I wonder if
>>>>> you've looked up the management stack to see what tools need to know to
>>>>> assign these NPU devices and whether specific configurations are
>>>>> required to make the NPU to GPU association work.  Thanks,  
>>>>
>>>> I'm not that familiar with how this was originally set up, but note that 
>>>> Alexey is just making it work exactly like baremetal does. The baremetal 
>>>> GPU driver works as-is in the VM and expects the same properties in the 
>>>> device-tree. Obviously it doesn't have to be that way, but there is 
>>>> value in keeping it identical.
>>>>
>>>> Another probably bigger point is that the NPU device also implements the 
>>>> nvlink HW interface and is required for actually training and 
>>>> maintaining the link up. The driver in the guest trains the links by 
>>>> programming both the GPU end and the NPU end of each link so the NPU 
>>>> device needs to be exposed to the guest.
>>>
>>> Ok, so there is functionality in assigning the NPU device itself, it's
>>> not just an attachment point for meta data.  But it still seems there
>>> must be some association of NPU to GPU, the tgt address seems to pair
>>> the NPU with a specific GPU, they're not simply a fungible set of NPUs
>>> and GPUs.  Is that association explicit anywhere or is it related to
>>> the topology or device numbering that needs to match between the host
>>> and guest?  Thanks,
>>
>> It is in the device tree (phandle is a node ID).
> 
> Hrm.  But the device tree just publishes information about the
> hardware.  What's the device tree value actually exposing here?
> 
> Is there an inherent hardware connection between one NPU and one GPU?
> Or is there just an arbitrary assignment performed by the firmware
> which it then exposed to the device tree?

I am not sure I understood the question...

The ibm,gpu and ibm,npu values  (which are phandles) of NPUs and GPUs
represent physical wiring.

The tgt property defines an address at which the GPU memory is mapped on
the system bus; these addresses are made of:
- System Select
- Memory Select
- Group Select
- Chip Select
- Chip internal address
and I have no idea if they can be changed :-/

On Mon, Nov 12, 2018 at 01:36:45PM +1100, Alexey Kardashevskiy wrote:
> 
> 
> On 12/11/2018 12:08, David Gibson wrote:
> > On Fri, Oct 19, 2018 at 11:53:53AM +1100, Alexey Kardashevskiy wrote:
> >>
> >>
> >> On 19/10/2018 05:05, Alex Williamson wrote:
> >>> On Thu, 18 Oct 2018 10:37:46 -0700
> >>> Piotr Jaroszynski <pjaroszynski@nvidia.com> wrote:
> >>>
> >>>> On 10/18/18 9:55 AM, Alex Williamson wrote:
> >>>>> On Thu, 18 Oct 2018 11:31:33 +1100
> >>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> >>>>>   
> >>>>>> On 18/10/2018 08:52, Alex Williamson wrote:  
> >>>>>>> On Wed, 17 Oct 2018 12:19:20 +1100
> >>>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> >>>>>>>      
> >>>>>>>> On 17/10/2018 06:08, Alex Williamson wrote:  
> >>>>>>>>> On Mon, 15 Oct 2018 20:42:33 +1100
> >>>>>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:    
> >>>>>>>>>> +
> >>>>>>>>>> +	if (pdev->vendor == PCI_VENDOR_ID_IBM &&
> >>>>>>>>>> +			pdev->device == 0x04ea) {
> >>>>>>>>>> +		ret = vfio_pci_ibm_npu2_init(vdev);
> >>>>>>>>>> +		if (ret) {
> >>>>>>>>>> +			dev_warn(&vdev->pdev->dev,
> >>>>>>>>>> +					"Failed to setup NVIDIA NV2 ATSD region\n");
> >>>>>>>>>> +			goto disable_exit;
> >>>>>>>>>>   		}  
> >>>>>>>>>
> >>>>>>>>> So the NPU is also actually owned by vfio-pci and assigned to the VM?  
> >>>>>>>>
> >>>>>>>> Yes. On a running system it looks like:
> >>>>>>>>
> >>>>>>>> 0007:00:00.0 Bridge: IBM Device 04ea (rev 01)
> >>>>>>>> 0007:00:00.1 Bridge: IBM Device 04ea (rev 01)
> >>>>>>>> 0007:00:01.0 Bridge: IBM Device 04ea (rev 01)
> >>>>>>>> 0007:00:01.1 Bridge: IBM Device 04ea (rev 01)
> >>>>>>>> 0007:00:02.0 Bridge: IBM Device 04ea (rev 01)
> >>>>>>>> 0007:00:02.1 Bridge: IBM Device 04ea (rev 01)
> >>>>>>>> 0035:00:00.0 PCI bridge: IBM Device 04c1
> >>>>>>>> 0035:01:00.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> >>>>>>>> 0035:02:04.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> >>>>>>>> 0035:02:05.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> >>>>>>>> 0035:02:0d.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
> >>>>>>>> 0035:03:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
> >>>>>>>> (rev a1
> >>>>>>>> 0035:04:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
> >>>>>>>> (rev a1)
> >>>>>>>> 0035:05:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
> >>>>>>>> (rev a1)
> >>>>>>>>
> >>>>>>>> One "IBM Device" bridge represents one NVLink2, i.e. a piece of NPU.
> >>>>>>>> They all and 3 GPUs go to the same IOMMU group and get passed through to
> >>>>>>>> a guest.
> >>>>>>>>
> >>>>>>>> The entire NPU does not have representation via sysfs as a whole though.  
> >>>>>>>
> >>>>>>> So the NPU is a bridge, but it uses a normal header type so vfio-pci
> >>>>>>> will bind to it?  
> >>>>>>
> >>>>>> An NPU is a NVLink bridge, it is not PCI in any sense. We (the host
> >>>>>> powerpc firmware known as "skiboot" or "opal") have chosen to emulate a
> >>>>>> virtual bridge per 1 NVLink on the firmware level. So for each physical
> >>>>>> NPU there are 6 virtual bridges. So the NVIDIA driver does not need to
> >>>>>> know much about NPUs.
> >>>>>>  
> >>>>>>> And the ATSD register that we need on it is not
> >>>>>>> accessible through these PCI representations of the sub-pieces of the
> >>>>>>> NPU?  Thanks,  
> >>>>>>
> >>>>>> No, only via the device tree. The skiboot puts the ATSD register address
> >>>>>> to the PHB's DT property called 'ibm,mmio-atsd' of these virtual bridges.  
> >>>>>
> >>>>> Ok, so the NPU is essential a virtual device already, mostly just a
> >>>>> stub.  But it seems that each NPU is associated to a specific GPU, how
> >>>>> is that association done?  In the use case here it seems like it's just
> >>>>> a vehicle to provide this ibm,mmio-atsd property to guest DT and the tgt
> >>>>> routing information to the GPU.  So if both of those were attached to
> >>>>> the GPU, there'd be no purpose in assigning the NPU other than it's in
> >>>>> the same IOMMU group with a type 0 header, so something needs to be
> >>>>> done with it.  If it's a virtual device, perhaps it could have a type 1
> >>>>> header so vfio wouldn't care about it, then we would only assign the
> >>>>> GPU with these extra properties, which seems easier for management
> >>>>> tools and users.  If the guest driver needs a visible NPU device, QEMU
> >>>>> could possibly emulate one to make the GPU association work
> >>>>> automatically.  Maybe this isn't really a problem, but I wonder if
> >>>>> you've looked up the management stack to see what tools need to know to
> >>>>> assign these NPU devices and whether specific configurations are
> >>>>> required to make the NPU to GPU association work.  Thanks,  
> >>>>
> >>>> I'm not that familiar with how this was originally set up, but note that 
> >>>> Alexey is just making it work exactly like baremetal does. The baremetal 
> >>>> GPU driver works as-is in the VM and expects the same properties in the 
> >>>> device-tree. Obviously it doesn't have to be that way, but there is 
> >>>> value in keeping it identical.
> >>>>
> >>>> Another probably bigger point is that the NPU device also implements the 
> >>>> nvlink HW interface and is required for actually training and 
> >>>> maintaining the link up. The driver in the guest trains the links by 
> >>>> programming both the GPU end and the NPU end of each link so the NPU 
> >>>> device needs to be exposed to the guest.
> >>>
> >>> Ok, so there is functionality in assigning the NPU device itself, it's
> >>> not just an attachment point for meta data.  But it still seems there
> >>> must be some association of NPU to GPU, the tgt address seems to pair
> >>> the NPU with a specific GPU, they're not simply a fungible set of NPUs
> >>> and GPUs.  Is that association explicit anywhere or is it related to
> >>> the topology or device numbering that needs to match between the host
> >>> and guest?  Thanks,
> >>
> >> It is in the device tree (phandle is a node ID).
> > 
> > Hrm.  But the device tree just publishes information about the
> > hardware.  What's the device tree value actually exposing here?
> > 
> > Is there an inherent hardware connection between one NPU and one GPU?
> > Or is there just an arbitrary assignment performed by the firmware
> > which it then exposed to the device tree?
> 
> I am not sure I understood the question...
> 
> The ibm,gpu and ibm,npu values  (which are phandles) of NPUs and GPUs
> represent physical wiring.

So you're saying there is specific physical wiring between one
particular NPU and one particular GPU?  And the device tree properties
describe that wiring?

I think what Alex and I are both trying to determine is if the binding
of NPUs to GPUs is as a result of physical wiring constraints, or just
a firmware imposed convention.

On 12/11/2018 15:23, David Gibson wrote:
> On Mon, Nov 12, 2018 at 01:36:45PM +1100, Alexey Kardashevskiy wrote:
>>
>>
>> On 12/11/2018 12:08, David Gibson wrote:
>>> On Fri, Oct 19, 2018 at 11:53:53AM +1100, Alexey Kardashevskiy wrote:
>>>>
>>>>
>>>> On 19/10/2018 05:05, Alex Williamson wrote:
>>>>> On Thu, 18 Oct 2018 10:37:46 -0700
>>>>> Piotr Jaroszynski <pjaroszynski@nvidia.com> wrote:
>>>>>
>>>>>> On 10/18/18 9:55 AM, Alex Williamson wrote:
>>>>>>> On Thu, 18 Oct 2018 11:31:33 +1100
>>>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>>>   
>>>>>>>> On 18/10/2018 08:52, Alex Williamson wrote:  
>>>>>>>>> On Wed, 17 Oct 2018 12:19:20 +1100
>>>>>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>>>>>      
>>>>>>>>>> On 17/10/2018 06:08, Alex Williamson wrote:  
>>>>>>>>>>> On Mon, 15 Oct 2018 20:42:33 +1100
>>>>>>>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:    
>>>>>>>>>>>> +
>>>>>>>>>>>> +	if (pdev->vendor == PCI_VENDOR_ID_IBM &&
>>>>>>>>>>>> +			pdev->device == 0x04ea) {
>>>>>>>>>>>> +		ret = vfio_pci_ibm_npu2_init(vdev);
>>>>>>>>>>>> +		if (ret) {
>>>>>>>>>>>> +			dev_warn(&vdev->pdev->dev,
>>>>>>>>>>>> +					"Failed to setup NVIDIA NV2 ATSD region\n");
>>>>>>>>>>>> +			goto disable_exit;
>>>>>>>>>>>>   		}  
>>>>>>>>>>>
>>>>>>>>>>> So the NPU is also actually owned by vfio-pci and assigned to the VM?  
>>>>>>>>>>
>>>>>>>>>> Yes. On a running system it looks like:
>>>>>>>>>>
>>>>>>>>>> 0007:00:00.0 Bridge: IBM Device 04ea (rev 01)
>>>>>>>>>> 0007:00:00.1 Bridge: IBM Device 04ea (rev 01)
>>>>>>>>>> 0007:00:01.0 Bridge: IBM Device 04ea (rev 01)
>>>>>>>>>> 0007:00:01.1 Bridge: IBM Device 04ea (rev 01)
>>>>>>>>>> 0007:00:02.0 Bridge: IBM Device 04ea (rev 01)
>>>>>>>>>> 0007:00:02.1 Bridge: IBM Device 04ea (rev 01)
>>>>>>>>>> 0035:00:00.0 PCI bridge: IBM Device 04c1
>>>>>>>>>> 0035:01:00.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>>>>>>>> 0035:02:04.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>>>>>>>> 0035:02:05.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>>>>>>>> 0035:02:0d.0 PCI bridge: PLX Technology, Inc. Device 8725 (rev ca)
>>>>>>>>>> 0035:03:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>>>>>>>>>> (rev a1
>>>>>>>>>> 0035:04:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>>>>>>>>>> (rev a1)
>>>>>>>>>> 0035:05:00.0 3D controller: NVIDIA Corporation GV100GL [Tesla V100 SXM2]
>>>>>>>>>> (rev a1)
>>>>>>>>>>
>>>>>>>>>> One "IBM Device" bridge represents one NVLink2, i.e. a piece of NPU.
>>>>>>>>>> They all and 3 GPUs go to the same IOMMU group and get passed through to
>>>>>>>>>> a guest.
>>>>>>>>>>
>>>>>>>>>> The entire NPU does not have representation via sysfs as a whole though.  
>>>>>>>>>
>>>>>>>>> So the NPU is a bridge, but it uses a normal header type so vfio-pci
>>>>>>>>> will bind to it?  
>>>>>>>>
>>>>>>>> An NPU is a NVLink bridge, it is not PCI in any sense. We (the host
>>>>>>>> powerpc firmware known as "skiboot" or "opal") have chosen to emulate a
>>>>>>>> virtual bridge per 1 NVLink on the firmware level. So for each physical
>>>>>>>> NPU there are 6 virtual bridges. So the NVIDIA driver does not need to
>>>>>>>> know much about NPUs.
>>>>>>>>  
>>>>>>>>> And the ATSD register that we need on it is not
>>>>>>>>> accessible through these PCI representations of the sub-pieces of the
>>>>>>>>> NPU?  Thanks,  
>>>>>>>>
>>>>>>>> No, only via the device tree. The skiboot puts the ATSD register address
>>>>>>>> to the PHB's DT property called 'ibm,mmio-atsd' of these virtual bridges.  
>>>>>>>
>>>>>>> Ok, so the NPU is essential a virtual device already, mostly just a
>>>>>>> stub.  But it seems that each NPU is associated to a specific GPU, how
>>>>>>> is that association done?  In the use case here it seems like it's just
>>>>>>> a vehicle to provide this ibm,mmio-atsd property to guest DT and the tgt
>>>>>>> routing information to the GPU.  So if both of those were attached to
>>>>>>> the GPU, there'd be no purpose in assigning the NPU other than it's in
>>>>>>> the same IOMMU group with a type 0 header, so something needs to be
>>>>>>> done with it.  If it's a virtual device, perhaps it could have a type 1
>>>>>>> header so vfio wouldn't care about it, then we would only assign the
>>>>>>> GPU with these extra properties, which seems easier for management
>>>>>>> tools and users.  If the guest driver needs a visible NPU device, QEMU
>>>>>>> could possibly emulate one to make the GPU association work
>>>>>>> automatically.  Maybe this isn't really a problem, but I wonder if
>>>>>>> you've looked up the management stack to see what tools need to know to
>>>>>>> assign these NPU devices and whether specific configurations are
>>>>>>> required to make the NPU to GPU association work.  Thanks,  
>>>>>>
>>>>>> I'm not that familiar with how this was originally set up, but note that 
>>>>>> Alexey is just making it work exactly like baremetal does. The baremetal 
>>>>>> GPU driver works as-is in the VM and expects the same properties in the 
>>>>>> device-tree. Obviously it doesn't have to be that way, but there is 
>>>>>> value in keeping it identical.
>>>>>>
>>>>>> Another probably bigger point is that the NPU device also implements the 
>>>>>> nvlink HW interface and is required for actually training and 
>>>>>> maintaining the link up. The driver in the guest trains the links by 
>>>>>> programming both the GPU end and the NPU end of each link so the NPU 
>>>>>> device needs to be exposed to the guest.
>>>>>
>>>>> Ok, so there is functionality in assigning the NPU device itself, it's
>>>>> not just an attachment point for meta data.  But it still seems there
>>>>> must be some association of NPU to GPU, the tgt address seems to pair
>>>>> the NPU with a specific GPU, they're not simply a fungible set of NPUs
>>>>> and GPUs.  Is that association explicit anywhere or is it related to
>>>>> the topology or device numbering that needs to match between the host
>>>>> and guest?  Thanks,
>>>>
>>>> It is in the device tree (phandle is a node ID).
>>>
>>> Hrm.  But the device tree just publishes information about the
>>> hardware.  What's the device tree value actually exposing here?
>>>
>>> Is there an inherent hardware connection between one NPU and one GPU?
>>> Or is there just an arbitrary assignment performed by the firmware
>>> which it then exposed to the device tree?
>>
>> I am not sure I understood the question...
>>
>> The ibm,gpu and ibm,npu values  (which are phandles) of NPUs and GPUs
>> represent physical wiring.
> 
> So you're saying there is specific physical wiring between one
> particular NPU and one particular GPU?  And the device tree properties
> describe that wiring?

Yes.

> I think what Alex and I are both trying to determine is if the binding
> of NPUs to GPUs is as a result of physical wiring constraints, or just
> a firmware imposed convention.


It is physical wiring which cannot change with a firmware update - there
are NVLink wires between CPU socket and GPU socket without bridges in
between.