On Thu, Jul 15, 2021 at 12:40:01PM +0530, Jerin Jacob wrote:

)
 a

On Tue, Jul 13, 2021 at 6:01 PM Chengwen Feng [off-list ref] wrote:

quoted

This patch introduce 'dmadevice' which is a generic type of DMA
device.

The APIs of dmadev library exposes some generic operations which can
enable configuration and I/O with the DMA devices.

Signed-off-by: Chengwen Feng <redacted>

Thanks for v3. Seems like all major items as covered. Some more
comments below inline.

I would suggest v4 to split the patch like (so that we can review and
ack each patch)
1) Only public header file with Doxygen inclusion, (There is a lot of
Doxygen syntax issue in the patch)
2) 1 or more patches for implementation.

One additional follow-up comment on flags below.

/Bruce

quoted

diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h
new file mode 100644
index 0000000..f6cc4e5

<snip>

quoted

+       enum rte_dmadev_port_type port_type;

missing doxgen comment for this.

quoted

+       union {
+               /** For PCIE port
+                *
+                * The following model show SoC's PCIE module connects to
+                * multiple PCIE hosts and multiple endpoints. The PCIE module
+                * has an integrate DMA controller.
+                * If the DMA wants to access the memory of host A, it can be
+                * initiated by PF1 in core0, or by VF0 of PF0 in core0.
+                *

<snip>

+                       /** The pasid filed in TLP packet */

quoted

+                       uint64_t pasid : 20;
+                       /** The attributes filed in TLP packet */
+                       uint64_t attr : 3;
+                       /** The processing hint filed in TLP packet */
+                       uint64_t ph : 2;
+                       /** The steering tag filed in TLP packet */
+                       uint64_t st : 16;

We don't support a few attributes like passid, ph, st. Do we need
the capability of this? or ignore this. In either case, please update the doc.

We also support additional flags for allocating LLC flag.
This is a hint to DMA engine that the cache blocks should be allocated
in the LLC (if they were not already).
When the MEM pointer is a destination in DMA operation, the referenced
cache blocks are allocated into the cache as part of completing the
DMA (when not already present in the LLC)
this is helpful if software has to access the data right after dma is completed.

Could you add bit or flag for the same?

I wonder if this is the best location for such a flag for LLC vs memory
writes. It would also apply to memory-to-memory transactions, not just for
those done to PCI devices. As well as that, I think any flag should default
to "on" rather than "off" since writing to cache rather than DRAM is
generally the desired behaviour, I would think. Should it be a
per-operation flag, rather than per context?

<snip>