[dpdk-dev] [PATCH 02/10] vdpa/sfc: add support for device initialization

[Maxime Coquelin]

On 7/6/21 6:44 PM, Vijay Srivastava wrote:
> From: Vijay Kumar Srivastava <vsrivast at xilinx.com>
> 
> Add HW initialization and vDPA device registration support.
> 
> Signed-off-by: Vijay Kumar Srivastava <vsrivast at xilinx.com>
> ---
>  doc/guides/vdpadevs/sfc.rst       |   6 +
>  drivers/vdpa/sfc/meson.build      |   3 +
>  drivers/vdpa/sfc/sfc_vdpa.c       |  23 +++
>  drivers/vdpa/sfc/sfc_vdpa.h       |  49 +++++-
>  drivers/vdpa/sfc/sfc_vdpa_debug.h |  21 +++
>  drivers/vdpa/sfc/sfc_vdpa_hw.c    | 322 ++++++++++++++++++++++++++++++++++++++
>  drivers/vdpa/sfc/sfc_vdpa_log.h   |   3 +
>  drivers/vdpa/sfc/sfc_vdpa_mcdi.c  |  74 +++++++++
>  drivers/vdpa/sfc/sfc_vdpa_ops.c   | 129 +++++++++++++++
>  drivers/vdpa/sfc/sfc_vdpa_ops.h   |  36 +++++
>  10 files changed, 665 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/vdpa/sfc/sfc_vdpa_debug.h
>  create mode 100644 drivers/vdpa/sfc/sfc_vdpa_hw.c
>  create mode 100644 drivers/vdpa/sfc/sfc_vdpa_mcdi.c
>  create mode 100644 drivers/vdpa/sfc/sfc_vdpa_ops.c
>  create mode 100644 drivers/vdpa/sfc/sfc_vdpa_ops.h
> 

...

> diff --git a/drivers/vdpa/sfc/sfc_vdpa_hw.c b/drivers/vdpa/sfc/sfc_vdpa_hw.c
> new file mode 100644
> index 0000000..83f3696
> --- /dev/null
> +++ b/drivers/vdpa/sfc/sfc_vdpa_hw.c
> @@ -0,0 +1,322 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + *
> + * Copyright(c) 2020-2021 Xilinx, Inc.
> + */
> +
> +#include <unistd.h>
> +
> +#include <rte_common.h>
> +#include <rte_errno.h>
> +#include <rte_vfio.h>
> +
> +#include "efx.h"
> +#include "sfc_vdpa.h"
> +#include "sfc_vdpa_ops.h"
> +
> +extern uint32_t sfc_logtype_driver;
> +
> +#ifndef PAGE_SIZE
> +#define PAGE_SIZE   (sysconf(_SC_PAGESIZE))
> +#endif
> +
> +int
> +sfc_vdpa_dma_alloc(struct sfc_vdpa_adapter *sva, const char *name,
> +		   size_t len, efsys_mem_t *esmp)
> +{
> +	void *mcdi_buf;
> +	uint64_t mcdi_iova;
> +	size_t mcdi_buff_size;
> +	int ret;
> +
> +	mcdi_buff_size = RTE_ALIGN_CEIL(len, PAGE_SIZE);
> +
> +	sfc_vdpa_log_init(sva, "name=%s, len=%zu", name, len);
> +
> +	mcdi_buf = rte_zmalloc(name, mcdi_buff_size, PAGE_SIZE);

You might want to allocate on the same NUMA node the device is on?

> +	if (mcdi_buf == NULL) {
> +		sfc_vdpa_err(sva, "cannot reserve memory for %s: len=%#x: %s",
> +			     name, (unsigned int)len, rte_strerror(rte_errno));
> +		return -ENOMEM;
> +	}
> +
> +	/* IOVA address for MCDI would be re-calculated if mapping
> +	 * using default IOVA would fail.
> +	 * TODO: Earlier there was no way to get valid IOVA range.
> +	 * Recently a patch has been submitted to get the IOVA range
> +	 * using ioctl. VFIO_IOMMU_GET_INFO. This patch is available
> +	 * in the kernel version >= 5.4. Support to get the default
> +	 * IOVA address for MCDI buffer using available IOVA range
> +	 * would be added later. Meanwhile default IOVA for MCDI buffer
> +	 * is kept at high mem at 2TB. In case of overlap new available
> +	 * addresses would be searched and same would be used.
> +	 */
> +	mcdi_iova = SFC_VDPA_DEFAULT_MCDI_IOVA;
> +
> +	do {
> +		ret = rte_vfio_container_dma_map(sva->vfio_container_fd,
> +						 (uint64_t)mcdi_buf, mcdi_iova,
> +						 mcdi_buff_size);
> +		if (ret == 0)
> +			break;
> +
> +		mcdi_iova = mcdi_iova >> 1;
> +		if (mcdi_iova < mcdi_buff_size)	{
> +			sfc_vdpa_err(sva,
> +				     "DMA mapping failed for MCDI : %s",
> +				     rte_strerror(rte_errno));
> +			return ret;

You leak mcdi_buf here if DMA map fails.

> +		}
> +
> +	} while (ret < 0);
> +
> +	esmp->esm_addr = mcdi_iova;
> +	esmp->esm_base = mcdi_buf;
> +	sva->mcdi_buff_size = mcdi_buff_size;
> +
> +	sfc_vdpa_info(sva,
> +		      "DMA name=%s len=%zu => virt=%p iova=%" PRIx64,
> +		      name, len, esmp->esm_base, esmp->esm_addr);
> +
> +	return 0;
> +}
> +

Thanks,
Maxime