Re: [PATCH v2 5/8] accel/qaic: Add datapath

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/24/2023 8:25 AM, Stanislaw Gruszka wrote:
On Mon, Feb 06, 2023 at 08:41:42AM -0700, Jeffrey Hugo wrote:
+#define SEM_VAL_MASK	GENMASK_ULL(11, 0)
+#define SEM_INDEX_MASK	GENMASK_ULL(4, 0)
+#define BULK_XFER	BIT(3)
+#define GEN_COMPLETION	BIT(4)
+#define INBOUND_XFER	1
+#define OUTBOUND_XFER	2
+#define REQHP_OFF	0x0 /* we read this */
+#define REQTP_OFF	0x4 /* we write this */
+#define RSPHP_OFF	0x8 /* we write this */
+#define RSPTP_OFF	0xc /* we read this */
+
+#define ENCODE_SEM(val, index, sync, cmd, flags)			\
+			((val) |					\
+			(index) << 16 |					\
+			(sync) << 22 |					\
+			(cmd) << 24 |					\
+			((cmd) ? BIT(31) : 0) |				\
+			(((flags) & SEM_INSYNCFENCE) ? BIT(30) : 0) |	\
+			(((flags) & SEM_OUTSYNCFENCE) ? BIT(29) : 0))

This could be probably better coded using FIELD_PREP()
with integrated checks of passed values not exceed
field width.

Interesting idea.  Will have a look.


+struct dbc_req { /* everything must be little endian encoded */

This comment does not provide much value IMHO ...

With the LE types, this does seem to be redundant now.  Will remove.


+inline int get_dbc_req_elem_size(void)
+{
+	return sizeof(struct dbc_req);
+}
+
+inline int get_dbc_rsp_elem_size(void)
+{
+	return sizeof(struct dbc_rsp);
+}

.. as those those inliners, instead of using sizeof() directly.
Up to you to change.

Will review these.


+static int reserve_pages(unsigned long start_pfn, unsigned long nr_pages,
+			 bool reserve)
+{
+	unsigned long pfn;
+	unsigned long end_pfn = start_pfn + nr_pages;
+	struct page *page;
+
+	for (pfn = start_pfn; pfn < end_pfn; pfn++) {
+		if (!pfn_valid(pfn))
+			return -EINVAL;
+		page =  pfn_to_page(pfn);
+		if (reserve)
+			SetPageReserved(page);
+		else
+			ClearPageReserved(page);

It is needed? Looks like taken from some legacy code.

Required for remap_pfn_range().

+static int copy_sgt(struct qaic_device *qdev, struct sg_table **sgt_out,
+		    struct sg_table *sgt_in, u64 size, u64 offset)
+{
+	int total_len, len, nents, offf = 0, offl = 0;
+	struct scatterlist *sg, *sgn, *sgf, *sgl;
+	struct sg_table *sgt;
+	int ret, j;
+
+	/* find out number of relevant nents needed for this mem */
+	total_len = 0;
+	sgf = NULL;
+	sgl = NULL;
+	nents = 0;
+
+	size = size ? size : PAGE_SIZE;
+	for (sg = sgt_in->sgl; sg; sg = sg_next(sg)) {
+		len = sg_dma_len(sg);
+
+		if (!len)
+			continue;
+		if (offset >= total_len && offset < total_len + len) {
+			sgf = sg;
+			offf = offset - total_len;
+		}
+		if (sgf)
+			nents++;
+		if (offset + size >= total_len &&
+		    offset + size <= total_len + len) {
+			sgl = sg;
+			offl = offset + size - total_len;
+			break;
+		}
+		total_len += len;
+	}
+
+	if (!sgf || !sgl) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
+	if (!sgt) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ret = sg_alloc_table(sgt, nents, GFP_KERNEL);
+	if (ret)
+		goto free_sgt;
+
+	/* copy relevant sg node and fix page and length */
+	sgn = sgf;
+	for_each_sgtable_sg(sgt, sg, j) {
+		memcpy(sg, sgn, sizeof(*sg));
+		if (sgn == sgf) {
+			sg_dma_address(sg) += offf;
+			sg_dma_len(sg) -= offf;
+			sg_set_page(sg, sg_page(sgn),
+				    sg_dma_len(sg), offf);
+		} else {
+			offf = 0;
+		}
+		if (sgn == sgl) {
+			sg_dma_len(sg) = offl - offf;
+			sg_set_page(sg, sg_page(sgn),
+				    offl - offf, offf);
+			sg_mark_end(sg);
+			break;
+		}
+		sgn = sg_next(sgn);
+	}

Why not use sg_copy_table() ? Overall copy_sgt() seems to be something
that could be replaced by generic helper or at least simplify.

I don't see "sg_copy_table" defined in 6.2. Are you suggesting renaming this function? I guess I'm not quite understanding your comment here. Can you elaborate?



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux