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

[6.6][bugfix]Hygon: Rename Hygon SM{3,4} feature macro to X86_FEATURE_HYGON_SM{3,4} #643

Conversation

wojiaohanliyang
Copy link

@wojiaohanliyang wojiaohanliyang commented Mar 5, 2025

The upstream commit a0423af92cb3 ("x86: KVM: Advertise CPUIDs for new instructions in Clearwater Forest") has introduced the macros X86_FEATURE_SM3 and X86_FEATURE_SM4, which conflict with the non-upstreamed commit 4a0be8d ("x86/cpufeatures: Add CPUID_8C86_0000_EDX CPUID leaf").

Summary by Sourcery

Enhancements:

  • Rename X86_FEATURE_SM3 and X86_FEATURE_SM4 to X86_FEATURE_HYGON_SM3 and X86_FEATURE_HYGON_SM4 respectively.

hygon inclusion
category: bugfix
CVE: NA

---------------------------

The upstream commit a0423af92cb3 ("x86: KVM: Advertise CPUIDs for new
instructions in Clearwater Forest") has introduced the macros
X86_FEATURE_SM3 and X86_FEATURE_SM4, which conflict with the
non-upstreamed commit 4a0be8d ("x86/cpufeatures: Add
CPUID_8C86_0000_EDX CPUID leaf"). To address this issue, we rename
X86_FEATURE_SM{3,4} to X86_FEATURE_HYGON_SM{3,4}.

Fixes: 4a0be8d ("x86/cpufeatures: Add CPUID_8C86_0000_EDX CPUID leaf")
Signed-off-by: hanliyang <[email protected]>
Copy link

sourcery-ai bot commented Mar 5, 2025

Reviewer's Guide by Sourcery

This pull request renames the Hygon SM3 and SM4 feature macros in arch/x86/include/asm/cpufeatures.h to X86_FEATURE_HYGON_SM3 and X86_FEATURE_HYGON_SM4 respectively. This change resolves a naming conflict with upstream code, specifically the X86_FEATURE_SM3 and X86_FEATURE_SM4 macros introduced in the upstream commit a0423af92cb3.

No diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change Details Files
Renames the Hygon SM3 and SM4 feature macros to avoid naming conflicts with upstream code.
  • Renamed X86_FEATURE_SM3 to X86_FEATURE_HYGON_SM3.
  • Renamed X86_FEATURE_SM4 to X86_FEATURE_HYGON_SM4.
arch/x86/include/asm/cpufeatures.h

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@deepin-ci-robot
Copy link

Hi @wojiaohanliyang. Thanks for your PR.

I'm waiting for a deepin-community member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @wojiaohanliyang - I've reviewed your changes - here's some feedback:

Overall Comments:

  • The added comments should use the same style as the surrounding comments, without the quotes around 'sm3' and 'sm4'.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@opsiff
Copy link
Member

opsiff commented Mar 5, 2025

/lgtm
/approve

@Avenger-285714
Copy link
Collaborator

/ok-to-test

@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. 代码注释:在修改后的代码中,新定义的宏X86_FEATURE_HYGON_SM3X86_FEATURE_HYGON_SM4缺少对应的注释说明。建议添加注释,解释这些宏的用途和含义。

  2. 命名约定:修改后的宏名称X86_FEATURE_HYGON_SM3X86_FEATURE_HYGON_SM4增加了前缀HYGON_,这有助于区分不同厂商定义的CPU特性。但是,建议在所有相关的宏定义中保持一致的命名风格,以避免混淆。

  3. 代码风格:在修改后的代码中,宏定义的注释使用了双引号"sm3""sm4",这可能是一个排版错误。通常,宏定义的注释应该使用单引号'sm3''sm4'。如果这是为了强调这些指令的名称,建议使用单引号。

  4. 代码质量:代码修改本身没有明显的逻辑错误,但是建议进行全面的回归测试,以确保这些更改不会影响现有的功能。

  5. 代码性能:这个更改不会直接影响性能,因为它只是宏定义的修改。但是,建议在未来的开发中,如果添加新的CPU特性,应该考虑其对性能的影响,并进行相应的性能测试。

  6. 代码安全:这个更改不涉及安全相关的代码,因此没有安全性的考虑。但是,建议在添加新的CPU特性时,确保这些特性不会引入安全漏洞,并且遵循最佳的安全编码实践。

综上所述,代码修改本身是合理的,但是建议在注释、命名风格和代码风格方面进行一些改进,以确保代码的可读性和一致性。同时,进行全面的回归测试和性能测试,以确保代码的质量和性能。

Copy link
Contributor

@MingcongBai MingcongBai left a comment

Choose a reason for hiding this comment

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

LGTM.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MingcongBai, opsiff

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Avenger-285714 Avenger-285714 merged commit a552a37 into deepin-community:linux-6.6.y Mar 5, 2025
4 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants