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

MATOM/MCORE2 lines in lite patch #105

Closed
heftig opened this issue Sep 30, 2024 · 6 comments
Closed

MATOM/MCORE2 lines in lite patch #105

heftig opened this issue Sep 30, 2024 · 6 comments

Comments

@heftig
Copy link

heftig commented Sep 30, 2024

These lines should probably be removed:

+        cflags-$(CONFIG_MATOM) 	+= -march=bonnell
+        cflags-$(CONFIG_MCORE2) 	+= -march=core2
@heftig
Copy link
Author

heftig commented Sep 30, 2024

Also, the full patch should remove the original lines so that it doesn't produce two march arguments.

PS: And ditto for the rustflags.

@heftig
Copy link
Author

heftig commented Oct 1, 2024

Also, config SUPPORT_MARCH_CODEVERS seems to be unused.

@heftig
Copy link
Author

heftig commented Oct 1, 2024

Also, the MODULE_PROC_FAMILY modifications seem to be incomplete. E.g., ZEN5 is missing.

heftig added a commit to zen-kernel/zen-kernel that referenced this issue Oct 1, 2024
@graysky2
Copy link
Owner

graysky2 commented Oct 1, 2024

Hallo Jan. Let me take your suggestions one-by-one for follow-up.

These lines should probably be removed:
+ cflags-$(CONFIG_MATOM) += -march=bonnell
+ cflags-$(CONFIG_MCORE2) += -march=core2

Since these are valid uarches, why do you suggest removing them?

Also, the full patch should remove the original lines so that it doesn't produce two march arguments. PS: And ditto for the rustflags.

Which original lines are you referencing?

Also, config SUPPORT_MARCH_CODEVERS seems to be unused.

It is used when the user compiles with the Generic-x86-64 Processor family selected. A value of 1 is the upstream default, but values of 2 or 3 switch to -march=x86-64-v2 or -march=x86-64-v3 respectively.

Also, the MODULE_PROC_FAMILY modifications seem to be incomplete. E.g., ZEN5 is missing.

My understanding about the entries in arch/x86/include/asm/vermagic.h is that they are used for 32-bit builds. I did not bother adding ZEN3+ here thinking that no one in their right mind with these CPUs would be building 32-bit kernels.

@heftig
Copy link
Author

heftig commented Oct 1, 2024

Since these are valid uarches, why do you suggest removing them?

Because the patch leaves the original lines in place, creating two -march arguments:

         cflags-$(CONFIG_MCORE2)		+= -march=core2
         cflags-$(CONFIG_MATOM)		+= -march=atom
[…]
+        cflags-$(CONFIG_MATOM) 	+= -march=bonnell
+        cflags-$(CONFIG_MCORE2) 	+= -march=core2

And the lite patch, unlike the full one, does not have the MINOR NOTES RELATING TO INTEL ATOM PROCESSORS, so the added lines should be removed, not the original lines.

Which original lines are you referencing?

The full patch has the same problem:

         cflags-$(CONFIG_MCORE2)		+= -march=core2
         cflags-$(CONFIG_MATOM)		+= -march=atom
[…]
+        cflags-$(CONFIG_MATOM) 	+= -march=bonnell
+        cflags-$(CONFIG_MCORE2) 	+= -march=core2
[…]
         rustflags-$(CONFIG_MCORE2)	+= -Ctarget-cpu=core2
         rustflags-$(CONFIG_MATOM)	+= -Ctarget-cpu=atom
[…]
+        rustflags-$(CONFIG_MATOM) 	+= -Ctarget-cpu=bonnell
+        rustflags-$(CONFIG_MCORE2) 	+= -Ctarget-cpu=core2

Here, the original lines should be removed.

It is used when the user compiles with the Generic-x86-64 Processor family selected. A value of 1 is the upstream default, but values of 2 or 3 switch to -march=x86-64-v2 or -march=x86-64-v3 respectively.

What do you mean? SUPPORT_MARCH_CODEVERS is not used anywhere and you're talking about config X86_64_VERSION.

My understanding about the entries in arch/x86/include/asm/vermagic.h is that they are used for 32-bit builds. I did not bother adding ZEN3+ here thinking that no one in their right mind with these CPUs would be building 32-bit kernels.

Fair enough.

@graysky2
Copy link
Owner

graysky2 commented Oct 1, 2024

I see now, thank you for the suggestions. Implemented in latest commit.

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

No branches or pull requests

2 participants