Skip to content

add gm algorithm support.#297

Open
chench246 wants to merge 1 commit into
tpm2-software:masterfrom
chench246:master
Open

add gm algorithm support.#297
chench246 wants to merge 1 commit into
tpm2-software:masterfrom
chench246:master

Conversation

@chench246
Copy link
Copy Markdown

@chench246 chench246 commented Dec 17, 2025 •
edited
Loading

add gm algorithm support.

kalvdans
kalvdans reviewed Dec 18, 2025
View reviewed changes
Copy link
Copy Markdown

@kalvdans kalvdans left a comment

Choose a reason for hiding this comment

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

Some comments, I'm not very familiar with the source code yet, feel free to ignore them!

Comment thread src/tpm2-tss-engine-common.h Outdated
Comment thread src/tpm2-tss-engine-common.h Outdated
Comment thread src/tpm2-tss-engine-ecc.c Outdated
Comment thread src/tpm2tss-genkey.c
@chench246 chench246 closed this Dec 19, 2025
@chench246 chench246 reopened this Dec 19, 2025
kalvdans
kalvdans reviewed Dec 20, 2025
View reviewed changes
Comment thread src/tpm2-tss-engine-ecc.c Outdated
kalvdans
kalvdans reviewed Dec 20, 2025
View reviewed changes
Comment thread src/tpm2-tss-engine-ecc.c
@chench246 chench246 force-pushed the master branch 3 times, most recently from a39240c to 5795ccf Compare December 24, 2025 08:16
kalvdans
kalvdans reviewed Dec 25, 2025
View reviewed changes
Comment thread src/tpm2-tss-engine-ecc.c Outdated
Comment thread src/tpm2-tss-engine-ecc.c Outdated
Comment thread src/tpm2-tss-engine-ecc.c Outdated
Comment thread src/tpm2-tss-engine-ecc.c Outdated
Comment thread src/tpm2-tss-engine-common.h Outdated
Comment thread src/tpm2-tss-engine-ecc.c Outdated
Comment thread src/tpm2-tss-engine-ecc.c Outdated
@chench246 chench246 force-pushed the master branch 2 times, most recently from d79fe82 to 5bbecf9 Compare January 29, 2026 07:03
kalvdans
kalvdans approved these changes Jan 29, 2026
View reviewed changes
Copy link
Copy Markdown

@kalvdans kalvdans left a comment

Choose a reason for hiding this comment

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

The patch looks good to me now! I don't have merge rights though. Thanks for your patience!

@chench246
Copy link
Copy Markdown
Author

chench246 commented Jan 29, 2026

@AndreasFuchsTPM @williamcroberts Would you mind taking a look at this PR and sharing your feedback? Thanks!

williamcroberts
williamcroberts requested changes Jan 29, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@williamcroberts williamcroberts left a comment

Choose a reason for hiding this comment

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

Overall looks really good, some minor style nits. Major item to work on is a test, we need a test in the test suite.

Comment thread src/tpm2-tss-engine-common.c
Comment thread src/tpm2-tss-engine-common.c Outdated
Comment thread src/tpm2-tss-engine-ecc.c Outdated
Comment thread src/tpm2-tss-engine-ecc.c Outdated
Comment thread src/tpm2-tss-engine-ecc.c Outdated
Comment thread src/tpm2-tss-engine-ecc.c
Comment thread src/tpm2-tss-engine-ecc.c
Comment thread src/tpm2-tss-engine-ecc.c Outdated
Comment thread src/tpm2-tss-engine-ecc.c Outdated
@chench246
Copy link
Copy Markdown
Author

chench246 commented Jan 30, 2026

Overall looks really good, some minor style nits. Major item to work on is a test, we need a test in the test suite.

Thanks for the feedback. I’ve fixed the style issues in the latest revision — please take a look. I’ll also add the corresponding test cases to the test suite shortly.

@williamcroberts
Copy link
Copy Markdown
Member

williamcroberts commented Jan 30, 2026

LGTM, just needs a test now. Thanks, nice code.

@chench246 chench246 force-pushed the master branch 2 times, most recently from 2ef63d2 to d8a876c Compare March 2, 2026 09:27
@chench246
add gm algorithm support.
d8a876c 
Signed-off-by: chench246 <chench246@hotmail.com>
@chench246
Copy link
Copy Markdown
Author

chench246 commented Mar 3, 2026

LGTM, just needs a test now. Thanks, nice code.

Sorry for the late reply. I’ve added some test cases in the latest revision. Could you please take another look? Thanks.

@williamcroberts
Copy link
Copy Markdown
Member

williamcroberts commented Mar 3, 2026

Looks good, we'll see how the CI turns out

@chench246
Copy link
Copy Markdown
Author

chench246 commented Mar 11, 2026 •
edited
Loading

Sorry to bother you. I just wanted to check whether there has been any progress on the review of this PR. Also, from what I can see, the CI failures do not appear to be related to my changes.

@williamcroberts
Copy link
Copy Markdown
Member

williamcroberts commented Mar 23, 2026

Sorry to bother you. I just wanted to check whether there has been any progress on the review of this PR. Also, from what I can see, the CI failures do not appear to be related to my changes.

Sorry, sick. Yeah, your PR seems fine. We need to invest some time into sorting the CI out and then taking this change. It's just a matter of time at this point.

@chench246
Copy link
Copy Markdown
Author

chench246 commented Mar 24, 2026

Sorry to bother you. I just wanted to check whether there has been any progress on the review of this PR. Also, from what I can see, the CI failures do not appear to be related to my changes.

Sorry, sick. Yeah, your PR seems fine. We need to invest some time into sorting the CI out and then taking this change. It's just a matter of time at this point.

Sorry to hear that, hope you feel better soon. Thanks for the update.

williamcroberts
williamcroberts approved these changes Apr 29, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@williamcroberts williamcroberts left a comment

Choose a reason for hiding this comment

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

LGTM, @AndreasFuchsTPM what do you think?

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

Reviewers

@williamcroberts williamcroberts williamcroberts approved these changes

+1 more reviewer

@kalvdans kalvdans kalvdans approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@chench246 @williamcroberts @kalvdans