Skip to content

drivers: crypto: hisilicon: implement QM driver - #6212

Closed
xiaoxuZeng wants to merge 2 commits into
OP-TEE:masterfrom
xiaoxuZeng:qm_br
Closed

drivers: crypto: hisilicon: implement QM driver#6212
xiaoxuZeng wants to merge 2 commits into
OP-TEE:masterfrom
xiaoxuZeng:qm_br

Conversation

@xiaoxuZeng

Copy link
Copy Markdown
Contributor

The Hisilicon QM is a Queue Management module.
In order to unify the interface between accelerator and software, a unified queue management module QM is used to interact with software. Each accelerator module integrates a QM. Software issues tasks to the SQ (Submmision Queue),and the QM obtains the address of the SQE (Submmision Queue Element). The BD (Buffer Description, same as SQE) information is sent to the accelerator. After the task processing is complete, the accelerator applies for a write-back address from the QM to write back the SQ.

@xiaoxuZeng

xiaoxuZeng commented Aug 2, 2023

Copy link
Copy Markdown
Contributor Author

The experts from community have provided many constructive and helpful suggestions, which we have incorporated through multiple rounds of revisions. However, the original PR #5894 contained too many modules, making it difficult to review. Therefore, we have split out a completed module into a separate PR for easier review. We look forward to receiving valuable feedback from the experts.

@jenswikl

jenswikl commented Aug 2, 2023

Copy link
Copy Markdown
Contributor

Makes sense, it's not the first time this approach has been needed.

@ZijjWang

ZijjWang commented Aug 2, 2023

Copy link
Copy Markdown

Acked-by: wangzijian <wangzijian22@huawei.com>

@jenswikl

jenswikl commented Aug 2, 2023

Copy link
Copy Markdown
Contributor

How about something like:

--- a/core/drivers/crypto/hisilicon/hisi_qm.c
+++ b/core/drivers/crypto/hisilicon/hisi_qm.c
@@ -93,11 +93,16 @@ enum qm_mailbox_cmd_v3 {
 };
 
 struct qm_mailbox {
-	uint16_t w0;
-	uint16_t queue;
-	uint32_t base_l;
-	uint32_t base_h;
-	uint32_t token;
+	union {
+		struct {
+			uint16_t w0;
+			uint16_t queue;
+			uint32_t base_l;
+			uint32_t base_h;
+			uint32_t token;
+		};
+		uint64_t x[2];
+	};
 };
 
 struct qm_dfx_registers {
@@ -148,18 +153,12 @@ static enum hisi_drv_status qm_wait_mb_ready(struct hisi_qm *qm)
 					  POLL_TIMEOUT);
 }
 
-static void qm_mb_write(struct hisi_qm *qm, void *src)
+static void qm_mb_write(struct hisi_qm *qm, struct qm_mailbox *mb)
 {
 	vaddr_t dst = qm->io_base + QM_MAILBOX_BASE;
-	unsigned long tmp0 = 0;
-	unsigned long tmp1 = 0;
 
 	/* 128bits should be written to hardware at one time */
-	asm volatile ("ldp %0, %1, %3\n"
-		      "stp %0, %1, %2\n"
-		      : "=&r"(tmp0), "=&r"(tmp1), "+Q"(*((char *)dst))
-		      : "Q"(*((char *)src))
-		      : "memory");
+	write_64bit_pair(dst, mb->x[0], mb->x[1]);
 	dsb();
 }

to avoid mixing inline assembly directly and also fix the potential alignment problem with struct qm_mailbox.

write_64bit_pair() should obviously be introduced in a separate patch.

HISI_QM_DRVCRYPT_HW_EACCESS,
};

#define readl_relaxed_poll_timeout(addr, val, cond, delay_us, timeout_us) \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think "relax_" prefix should be removed. This function does not relax CPU: udelay() is spinning.

Comment thread core/drivers/crypto/hisilicon/include/hisi_qm.h
Comment thread core/drivers/crypto/hisilicon/hisi_qm.c Outdated
for (i = QM_SQC_VFT; i <= QM_CQC_VFT; i++) {
ret = qm_set_vft_common(qm, i, function, base, num);
if (ret) {
EMSG("QM set type%d fail", i);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpicking: "QM set type%"PRId32" fail"

Comment thread core/drivers/crypto/hisilicon/hisi_qm.c Outdated
}
}

#define HISI_QM_RECV_DONE 0xAF

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe move to source file top?

Implement write_64bit_pair that write two 64 bits data together.

Signed-off-by: Xiaoxu Zeng <zengxiaoxu@huawei.com>
@xiaoxuZeng

Copy link
Copy Markdown
Contributor Author

to avoid mixing inline assembly directly and also fix the potential alignment problem with struct qm_mailbox.

write_64bit_pair() should obviously be introduced in a separate patch.

has been implement in a new pr #6228
https://github.com/OP-TEE/optee_os/pull/6228/files

The Hisilicon QM is a Queue Management module.
In order to unify the interface between accelerator and software,
a unified queue management module QM is used to interact with software.
Each accelerator module integrates a QM. Software issues tasks to the
SQ (Submmision Queue),and the QM obtains the address of the SQE (Submmision
Queue Element). The BD (Buffer Description, same as SQE) information is
sent to the accelerator. After the task processing is complete, the
accelerator applies for a write-back address from the QM to write back
the SQ.

Signed-off-by: Xiaoxu Zeng <zengxiaoxu@huawei.com>
@github-actions

github-actions Bot commented Sep 8, 2023

Copy link
Copy Markdown

This pull request has been marked as a stale pull request because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this pull request will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants