[v10,06/16] dma/idxd: add datapath structures

Message ID 20211019141041.1890983-7-kevin.laatz@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series add dmadev driver for idxd devices |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Kevin Laatz Oct. 19, 2021, 2:10 p.m. UTC
  Add data structures required for the data path for IDXD devices.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
Reviewed-by: Conor Walsh <conor.walsh@intel.com>
---
 drivers/dma/idxd/idxd_bus.c      |  1 +
 drivers/dma/idxd/idxd_common.c   | 33 +++++++++++++++++++++++++
 drivers/dma/idxd/idxd_hw_defs.h  | 41 ++++++++++++++++++++++++++++++++
 drivers/dma/idxd/idxd_internal.h |  4 ++++
 drivers/dma/idxd/idxd_pci.c      |  2 ++
 5 files changed, 81 insertions(+)
  

Comments

Chengwen Feng Oct. 20, 2021, 7:44 a.m. UTC | #1
On 2021/10/19 22:10, Kevin Laatz wrote:
> Add data structures required for the data path for IDXD devices.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> Reviewed-by: Conor Walsh <conor.walsh@intel.com>
> ---
>  drivers/dma/idxd/idxd_bus.c      |  1 +
>  drivers/dma/idxd/idxd_common.c   | 33 +++++++++++++++++++++++++
>  drivers/dma/idxd/idxd_hw_defs.h  | 41 ++++++++++++++++++++++++++++++++
>  drivers/dma/idxd/idxd_internal.h |  4 ++++
>  drivers/dma/idxd/idxd_pci.c      |  2 ++
>  5 files changed, 81 insertions(+)

[snip]

> +/**
> + * Hardware descriptor used by DSA hardware, for both bursts and
> + * for individual operations.
> + */
> +struct idxd_hw_desc {
> +	uint32_t pasid;
> +	uint32_t op_flags;
> +	rte_iova_t completion;
> +
> +	RTE_STD_C11
> +	union {
> +		rte_iova_t src;      /* source address for copy ops etc. */
> +		rte_iova_t desc_addr; /* descriptor pointer for batch */
> +	};
> +	rte_iova_t dst;
> +
> +	uint32_t size;    /* length of data for op, or batch size */
> +
> +	uint16_t intr_handle; /* completion interrupt handle */
> +
> +	/* remaining 26 bytes are reserved */
> +	uint16_t __reserved[13];

The non-reserved take about 30+B, and the struct align 64, so the __reserved[13] could delete.

It's a minor problem, so:
Reviewed-by: Chengwen Feng <fengchengwen@huawei.com>

> +} __rte_aligned(64);
> +
>  #define IDXD_COMP_STATUS_INCOMPLETE        0
>  #define IDXD_COMP_STATUS_SUCCESS           1
>  #define IDXD_COMP_STATUS_INVALID_OPCODE 0x10
> diff --git a/drivers/dma/idxd/idxd_internal.h b/drivers/dma/idxd/idxd_internal.h
> index 8473bf939f..5e253fdfbc 100644
> --- a/drivers/dma/idxd/idxd_internal.h
> +++ b/drivers/dma/idxd/idxd_internal.h
> @@ -40,6 +40,8 @@ struct idxd_pci_common {
>  };

[snip]
  
Bruce Richardson Oct. 20, 2021, 8:20 a.m. UTC | #2
On Wed, Oct 20, 2021 at 03:44:28PM +0800, fengchengwen wrote:
> On 2021/10/19 22:10, Kevin Laatz wrote:
> > Add data structures required for the data path for IDXD devices.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> > Reviewed-by: Conor Walsh <conor.walsh@intel.com>
> > ---
> >  drivers/dma/idxd/idxd_bus.c      |  1 +
> >  drivers/dma/idxd/idxd_common.c   | 33 +++++++++++++++++++++++++
> >  drivers/dma/idxd/idxd_hw_defs.h  | 41 ++++++++++++++++++++++++++++++++
> >  drivers/dma/idxd/idxd_internal.h |  4 ++++
> >  drivers/dma/idxd/idxd_pci.c      |  2 ++
> >  5 files changed, 81 insertions(+)
> 
> [snip]
> 
> > +/**
> > + * Hardware descriptor used by DSA hardware, for both bursts and
> > + * for individual operations.
> > + */
> > +struct idxd_hw_desc {
> > +	uint32_t pasid;
> > +	uint32_t op_flags;
> > +	rte_iova_t completion;
> > +
> > +	RTE_STD_C11
> > +	union {
> > +		rte_iova_t src;      /* source address for copy ops etc. */
> > +		rte_iova_t desc_addr; /* descriptor pointer for batch */
> > +	};
> > +	rte_iova_t dst;
> > +
> > +	uint32_t size;    /* length of data for op, or batch size */
> > +
> > +	uint16_t intr_handle; /* completion interrupt handle */
> > +
> > +	/* remaining 26 bytes are reserved */
> > +	uint16_t __reserved[13];
> 
> The non-reserved take about 30+B, and the struct align 64, so the __reserved[13] could delete.
> 
> It's a minor problem, so:
> Reviewed-by: Chengwen Feng <fengchengwen@huawei.com>
> 

There are actually cases where that reserved field makes a difference. If
we go to initialize a descriptor as a local variable the compiler is required
to initialize all unspecified fields to 0, which means that if we don't
explicitly put in place those reserved fields those bytes will be
uninitialized. Since the hardware requires all unused fields to be zero, we
need to keep this field in place to simplify the code and save us having to
do extra memsets to zero the unused space.
  

Patch

diff --git a/drivers/dma/idxd/idxd_bus.c b/drivers/dma/idxd/idxd_bus.c
index b48fa954ed..3c0837ec52 100644
--- a/drivers/dma/idxd/idxd_bus.c
+++ b/drivers/dma/idxd/idxd_bus.c
@@ -95,6 +95,7 @@  idxd_dev_close(struct rte_dma_dev *dev)
 
 static const struct rte_dma_dev_ops idxd_bus_ops = {
 		.dev_close = idxd_dev_close,
+		.dev_dump = idxd_dump,
 };
 
 static void *
diff --git a/drivers/dma/idxd/idxd_common.c b/drivers/dma/idxd/idxd_common.c
index 5abff34292..f972260a56 100644
--- a/drivers/dma/idxd/idxd_common.c
+++ b/drivers/dma/idxd/idxd_common.c
@@ -10,6 +10,35 @@ 
 
 #define IDXD_PMD_NAME_STR "dmadev_idxd"
 
+int
+idxd_dump(const struct rte_dma_dev *dev, FILE *f)
+{
+	struct idxd_dmadev *idxd = dev->fp_obj->dev_private;
+	unsigned int i;
+
+	fprintf(f, "== IDXD Private Data ==\n");
+	fprintf(f, "  Portal: %p\n", idxd->portal);
+	fprintf(f, "  Config: { ring_size: %u }\n",
+			idxd->qcfg.nb_desc);
+	fprintf(f, "  Batch ring (sz = %u, max_batches = %u):\n\t",
+			idxd->max_batches + 1, idxd->max_batches);
+	for (i = 0; i <= idxd->max_batches; i++) {
+		fprintf(f, " %u ", idxd->batch_idx_ring[i]);
+		if (i == idxd->batch_idx_read && i == idxd->batch_idx_write)
+			fprintf(f, "[rd ptr, wr ptr] ");
+		else if (i == idxd->batch_idx_read)
+			fprintf(f, "[rd ptr] ");
+		else if (i == idxd->batch_idx_write)
+			fprintf(f, "[wr ptr] ");
+		if (i == idxd->max_batches)
+			fprintf(f, "\n");
+	}
+
+	fprintf(f, "  Curr batch: start = %u, size = %u\n", idxd->batch_start, idxd->batch_size);
+	fprintf(f, "  IDS: avail = %u, returned: %u\n", idxd->ids_avail, idxd->ids_returned);
+	return 0;
+}
+
 int
 idxd_dmadev_create(const char *name, struct rte_device *dev,
 		   const struct idxd_dmadev *base_idxd,
@@ -19,6 +48,10 @@  idxd_dmadev_create(const char *name, struct rte_device *dev,
 	struct rte_dma_dev *dmadev = NULL;
 	int ret = 0;
 
+	RTE_BUILD_BUG_ON(sizeof(struct idxd_hw_desc) != 64);
+	RTE_BUILD_BUG_ON(offsetof(struct idxd_hw_desc, size) != 32);
+	RTE_BUILD_BUG_ON(sizeof(struct idxd_completion) != 32);
+
 	if (!name) {
 		IDXD_PMD_ERR("Invalid name of the device!");
 		ret = -EINVAL;
diff --git a/drivers/dma/idxd/idxd_hw_defs.h b/drivers/dma/idxd/idxd_hw_defs.h
index 86f7f3526b..55ca9f7f52 100644
--- a/drivers/dma/idxd/idxd_hw_defs.h
+++ b/drivers/dma/idxd/idxd_hw_defs.h
@@ -5,6 +5,47 @@ 
 #ifndef _IDXD_HW_DEFS_H_
 #define _IDXD_HW_DEFS_H_
 
+/*
+ * Defines used in the data path for interacting with IDXD hardware.
+ */
+#define IDXD_CMD_OP_SHIFT 24
+enum rte_idxd_ops {
+	idxd_op_nop = 0,
+	idxd_op_batch,
+	idxd_op_drain,
+	idxd_op_memmove,
+	idxd_op_fill
+};
+
+#define IDXD_FLAG_FENCE                 (1 << 0)
+#define IDXD_FLAG_COMPLETION_ADDR_VALID (1 << 2)
+#define IDXD_FLAG_REQUEST_COMPLETION    (1 << 3)
+#define IDXD_FLAG_CACHE_CONTROL         (1 << 8)
+
+/**
+ * Hardware descriptor used by DSA hardware, for both bursts and
+ * for individual operations.
+ */
+struct idxd_hw_desc {
+	uint32_t pasid;
+	uint32_t op_flags;
+	rte_iova_t completion;
+
+	RTE_STD_C11
+	union {
+		rte_iova_t src;      /* source address for copy ops etc. */
+		rte_iova_t desc_addr; /* descriptor pointer for batch */
+	};
+	rte_iova_t dst;
+
+	uint32_t size;    /* length of data for op, or batch size */
+
+	uint16_t intr_handle; /* completion interrupt handle */
+
+	/* remaining 26 bytes are reserved */
+	uint16_t __reserved[13];
+} __rte_aligned(64);
+
 #define IDXD_COMP_STATUS_INCOMPLETE        0
 #define IDXD_COMP_STATUS_SUCCESS           1
 #define IDXD_COMP_STATUS_INVALID_OPCODE 0x10
diff --git a/drivers/dma/idxd/idxd_internal.h b/drivers/dma/idxd/idxd_internal.h
index 8473bf939f..5e253fdfbc 100644
--- a/drivers/dma/idxd/idxd_internal.h
+++ b/drivers/dma/idxd/idxd_internal.h
@@ -40,6 +40,8 @@  struct idxd_pci_common {
 };
 
 struct idxd_dmadev {
+	struct idxd_hw_desc *desc_ring;
+
 	/* counters to track the batches */
 	unsigned short max_batches;
 	unsigned short batch_idx_read;
@@ -63,6 +65,7 @@  struct idxd_dmadev {
 	unsigned short max_batch_size;
 
 	struct rte_dma_dev *dmadev;
+	struct rte_dma_vchan_conf qcfg;
 	uint8_t sva_support;
 	uint8_t qid;
 
@@ -77,5 +80,6 @@  struct idxd_dmadev {
 
 int idxd_dmadev_create(const char *name, struct rte_device *dev,
 		const struct idxd_dmadev *base_idxd, const struct rte_dma_dev_ops *ops);
+int idxd_dump(const struct rte_dma_dev *dev, FILE *f);
 
 #endif /* _IDXD_INTERNAL_H_ */
diff --git a/drivers/dma/idxd/idxd_pci.c b/drivers/dma/idxd/idxd_pci.c
index 7127483b10..96c8c65cc0 100644
--- a/drivers/dma/idxd/idxd_pci.c
+++ b/drivers/dma/idxd/idxd_pci.c
@@ -76,12 +76,14 @@  idxd_pci_dev_close(struct rte_dma_dev *dev)
 	/* free device memory */
 	IDXD_PMD_DEBUG("Freeing device driver memory");
 	rte_free(idxd->batch_idx_ring);
+	rte_free(idxd->desc_ring);
 
 	return 0;
 }
 
 static const struct rte_dma_dev_ops idxd_pci_ops = {
 	.dev_close = idxd_pci_dev_close,
+	.dev_dump = idxd_dump,
 };
 
 /* each portal uses 4 x 4k pages */