Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xtest: add SM2 perf test #739

Merged
merged 1 commit into from Apr 24, 2024
Merged

xtest: add SM2 perf test #739

merged 1 commit into from Apr 24, 2024

Conversation

yuzexiyzx
Copy link
Contributor

add perf test for SM2 algorithm

Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

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

@yuzexiyzx, usage() needs to be updated for these new cmdline options.

@@ -904,6 +904,25 @@ int asym_perf_runner_cmd_parser(int argc, char *argv[])
main_algo = ALGO_X25519;
width_bits = 256;
mode = MODE_GENKEYPAIR;
} else if (!strcasecmp(argv[i], "SM2_GENKEYPAIR")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't you need to provide the main_algo info to generate an SM2 keypair?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

solved

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know much about SM2 but it seems the key type differs if used for ciphering and authentication (TEE_TYPE_SM2_DSA_KEYPAIR vs TEE_TYPE_SM2_PKE_KEYPAIR). Here we don't knw what the key will be used for, is that ok?
(resolved: answered by #739 (comment))

@@ -957,7 +976,7 @@ int asym_perf_runner_cmd_parser(int argc, char *argv[])
}
}

if (mode == MODE_GENKEYPAIR)
if (mode == MODE_GENKEYPAIR || main_algo >= ALGO_SM2_PKE)
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer explicit matching main_algo values:

	if (mode == MODE_GENKEYPAIR || main_algo >= ALGO_SM2_PKE ||
	    main_algo >= ALGO_SM2_DSA_SM3)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

solved

@yuzexiyzx yuzexiyzx force-pushed the master branch 5 times, most recently from 46e5fbb to 057db93 Compare April 16, 2024 06:02
@yuzexiyzx
Copy link
Contributor Author

@etienne-lms All comments have been solved.

Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

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

LGTM.
Acked-by: Etienne Carriere <etienne.carriere@foss.st.com> if this new comment is out of scope.

This change modifies asym performance test TA API, but I think it's ok we don't enfore backward compat on OP-TEE Test TAs ABI.

@@ -904,6 +904,25 @@ int asym_perf_runner_cmd_parser(int argc, char *argv[])
main_algo = ALGO_X25519;
width_bits = 256;
mode = MODE_GENKEYPAIR;
} else if (!strcasecmp(argv[i], "SM2_GENKEYPAIR")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know much about SM2 but it seems the key type differs if used for ciphering and authentication (TEE_TYPE_SM2_DSA_KEYPAIR vs TEE_TYPE_SM2_PKE_KEYPAIR). Here we don't knw what the key will be used for, is that ok?
(resolved: answered by #739 (comment))

@yuzexiyzx
Copy link
Contributor Author

@etienne-lms This is used for both TEE_TYPE_SM2_DSA_KEYPAIR and TEE_TYPE_SM2_PKE_KEYPAIR
image

add perf test for SM2 algorithm

Signed-off-by: Zexi Yu <yuzexi@hisilicon.com>
Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>
@yuzexiyzx
Copy link
Contributor Author

@jforissier All comments have been solved.

@etienne-lms
Copy link
Contributor

@jforissier, LGTM. Note the test TA API change. I think it's ok we don't enfore backward compat on OP-TEE Test TAs ABI. Ok with you?

@jforissier
Copy link
Contributor

@jforissier, LGTM. Note the test TA API change. I think it's ok we don't enfore backward compat on OP-TEE Test TAs ABI. Ok with you?

Yes it's fine.

@jforissier jforissier merged commit 50e1cbd into OP-TEE:master Apr 24, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants