[dpdk-dev] [PATCH v5 04/16] dma/idxd: create dmadev instances on bus probe

[Kevin Laatz]

On 22/09/2021 03:04, fengchengwen wrote:
> On 2021/9/17 23:24, Kevin Laatz wrote:
>> When a suitable device is found during the bus scan/probe, create a dmadev
>> instance for each HW queue. Internal structures required for device
>> creation are also added.
>>
>> Signed-off-by: Bruce Richardson <bruce.richardson at intel.com>
>> Signed-off-by: Kevin Laatz <kevin.laatz at intel.com>
>> Reviewed-by: Conor Walsh <conor.walsh at intel.com>
>>
>> ---
>> v4:
>>    - fix 'vdev' naming, changed to 'bus'
>>    - rebase changes
>> ---
>>   drivers/dma/idxd/idxd_bus.c      | 19 ++++++++
>>   drivers/dma/idxd/idxd_common.c   | 76 ++++++++++++++++++++++++++++++++
>>   drivers/dma/idxd/idxd_internal.h | 40 +++++++++++++++++
>>   drivers/dma/idxd/meson.build     |  1 +
>>   4 files changed, 136 insertions(+)
>>   create mode 100644 drivers/dma/idxd/idxd_common.c
>>
>> diff --git a/drivers/dma/idxd/idxd_bus.c b/drivers/dma/idxd/idxd_bus.c
>> index ef589af30e..b48fa954ed 100644
>> --- a/drivers/dma/idxd/idxd_bus.c
>> +++ b/drivers/dma/idxd/idxd_bus.c
>> @@ -85,6 +85,18 @@ dsa_get_sysfs_path(void)
>>   	return path ? path : DSA_SYSFS_PATH;
>>   }
>>   
>> +static int
>> +idxd_dev_close(struct rte_dma_dev *dev)
>> +{
>> +	struct idxd_dmadev *idxd = dev->data->dev_private;
>> +	munmap(idxd->portal, 0x1000);
>> +	return 0;
>> +}
>> +
>> +static const struct rte_dma_dev_ops idxd_bus_ops = {
>> +		.dev_close = idxd_dev_close,
>> +};
>> +
>>   static void *
>>   idxd_bus_mmap_wq(struct rte_dsa_device *dev)
>>   {
>> @@ -206,6 +218,7 @@ idxd_probe_dsa(struct rte_dsa_device *dev)
>>   		return -1;
>>   	idxd.max_batch_size = ret;
>>   	idxd.qid = dev->addr.wq_id;
>> +	idxd.u.bus.dsa_id = dev->addr.device_id;
>>   	idxd.sva_support = 1;
>>   
>>   	idxd.portal = idxd_bus_mmap_wq(dev);
>> @@ -214,6 +227,12 @@ idxd_probe_dsa(struct rte_dsa_device *dev)
>>   		return -ENOENT;
>>   	}
>>   
>> +	ret = idxd_dmadev_create(dev->wq_name, &dev->device, &idxd, &idxd_bus_ops);
>> +	if (ret) {
>> +		IDXD_PMD_ERR("Failed to create rawdev %s", dev->wq_name);
>> +		return ret;
>> +	}
>> +
>>   	return 0;
>>   }
>>   
>> diff --git a/drivers/dma/idxd/idxd_common.c b/drivers/dma/idxd/idxd_common.c
>> new file mode 100644
>> index 0000000000..8afad637fc
>> --- /dev/null
>> +++ b/drivers/dma/idxd/idxd_common.c
>> @@ -0,0 +1,76 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause
>> + * Copyright 2021 Intel Corporation
>> + */
>> +
>> +#include <rte_dmadev_pmd.h>
>> +#include <rte_malloc.h>
>> +#include <rte_common.h>
>> +
>> +#include "idxd_internal.h"
>> +
>> +#define IDXD_PMD_NAME_STR "dmadev_idxd"
>> +
>> +int
>> +idxd_dmadev_create(const char *name, struct rte_device *dev,
>> +		   const struct idxd_dmadev *base_idxd,
>> +		   const struct rte_dma_dev_ops *ops)
>> +{
>> +	struct idxd_dmadev *idxd;
>> +	struct rte_dma_dev *dmadev = NULL;
>> +	int ret = 0;
>> +
>> +	if (!name) {
>> +		IDXD_PMD_ERR("Invalid name of the device!");
>> +		ret = -EINVAL;
>> +		goto cleanup;
>> +	}
>> +
>> +	/* Allocate device structure */
>> +	dmadev = rte_dma_pmd_allocate(name, dev->numa_node,
>> +			sizeof(dmadev->dev_private));
>> +	if (dmadev == NULL) {
>> +		IDXD_PMD_ERR("Unable to allocate raw device");
>> +		ret = -ENOMEM;
>> +		goto cleanup;
>> +	}
>> +	dmadev->dev_ops = ops;
>> +	dmadev->device = dev;
>> +
>> +	idxd = rte_malloc_socket(NULL, sizeof(struct idxd_dmadev), 0, dev->numa_node);
>> +	if (idxd == NULL) {
>> +		IDXD_PMD_ERR("Unable to allocate memory for device");
>> +		ret = -ENOMEM;
>> +		goto cleanup;
>> +	}
>> +	dmadev->data->dev_private = idxd;
>> +	dmadev->dev_private = idxd;
> The dmadev->dev_private and dmadev->data->dev_private already inited by rte_dma_pmd_allocate,
> and the driver only needs to pass in the correct parameters.
>
> Recommended:
>    dmadev = rte_dma_pmd_allocate(name, dev->name, sizeof(struct idxd_dmadev));
>
>
>> +	*idxd = *base_idxd; /* copy over the main fields already passed in */
>> +	idxd->dmadev = dmadev;
>> +
>> +	/* allocate batch index ring and completion ring.
>> +	 * The +1 is because we can never fully use
>> +	 * the ring, otherwise read == write means both full and empty.
>> +	 */
>> +	idxd->batch_comp_ring = rte_zmalloc(NULL, (sizeof(idxd->batch_idx_ring[0]) +
>> +			sizeof(idxd->batch_comp_ring[0]))	* (idxd->max_batches + 1),
>> +			sizeof(idxd->batch_comp_ring[0]));
>> +	if (idxd->batch_comp_ring == NULL) {
>> +		IDXD_PMD_ERR("Unable to reserve memory for batch data\n");
>> +		ret = -ENOMEM;
>> +		goto cleanup;
>> +	}
>> +	idxd->batch_idx_ring = (void *)&idxd->batch_comp_ring[idxd->max_batches+1];
>> +	idxd->batch_iova = rte_mem_virt2iova(idxd->batch_comp_ring);
>> +
> Once init one dmadev successful, driver need changes it's state to READY, like:
>    dmadev->state = RTE_DMA_DEV_READY;
> This was useful when call rte_dma_pmd_release: if the state is ready, lib will call
> rte_dma_close() to release the dmadev, else it only clean the struct which lib holds.

Will make these changes in v6, thanks!