* tip-bot2 for Paolo Bonzini <[email protected]> wrote:
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -1031,6 +1031,8 @@ static void zenbleed_check(struct cpuinfo_x86 *c)
>
> static void init_amd(struct cpuinfo_x86 *c)
> {
> + u64 vm_cr;
> +
> early_init_amd(c);
>
> /*
> @@ -1082,6 +1084,14 @@ static void init_amd(struct cpuinfo_x86 *c)
>
> init_amd_cacheinfo(c);
>
> + if (cpu_has(c, X86_FEATURE_SVM)) {
> + rdmsrl(MSR_VM_CR, vm_cr);
> + if (vm_cr & SVM_VM_CR_SVM_DIS_MASK) {
> + pr_notice_once("SVM disabled (by BIOS) in MSR_VM_CR\n");
> + clear_cpu_cap(c, X86_FEATURE_SVM);
> + }
> + }
> +
> if (!cpu_has(c, X86_FEATURE_LFENCE_RDTSC) && cpu_has(c, X86_FEATURE_XMM2)) {
> /*
> * Use LFENCE for execution serialization. On families which
> diff --git a/arch/x86/kernel/cpu/hygon.c b/arch/x86/kernel/cpu/hygon.c
> index defdc59..16f3463 100644
> --- a/arch/x86/kernel/cpu/hygon.c
> +++ b/arch/x86/kernel/cpu/hygon.c
> @@ -290,6 +290,8 @@ static void early_init_hygon(struct cpuinfo_x86 *c)
>
> static void init_hygon(struct cpuinfo_x86 *c)
> {
> + u64 vm_cr;
> +
> early_init_hygon(c);
>
> /*
> @@ -320,6 +322,14 @@ static void init_hygon(struct cpuinfo_x86 *c)
>
> init_hygon_cacheinfo(c);
>
> + if (cpu_has(c, X86_FEATURE_SVM)) {
> + rdmsrl(MSR_VM_CR, vm_cr);
> + if (vm_cr & SVM_VM_CR_SVM_DIS_MASK) {
> + pr_notice_once("SVM disabled (by BIOS) in MSR_VM_CR\n");
> + clear_cpu_cap(c, X86_FEATURE_SVM);
> + }
> + }
> +
1)
It's a bit sad that we are duplicating identical code.
2)
We are doing it in other cases as well: for example nearby_node() is
duplicated between arch/x86/kernel/cpu/amd.c and
arch/x86/kernel/cpu/hygon.c too.
3)
BTW., while look at this code I noticed that the 'Author' copyright
tag in arch/x86/kernel/cpu/hygon.c seems to be inaccurate:
// SPDX-License-Identifier: GPL-2.0+
/*
* Hygon Processor Support for Linux
*
* Copyright (C) 2018 Chengdu Haiguang IC Design Co., Ltd.
*
* Author: Pu Wen <[email protected]>
*/
... as for example the nearby_node() was clearly copied & derived from
arch/x86/kernel/cpu/amd.c, which does not appear to be accurately reflected
in this copyright notice?
Thanks,
Ingo
On Fri, Sep 22, 2023 at 12:26 PM Ingo Molnar <[email protected]> wrote:
> It's a bit sad that we are duplicating identical code.
> We are doing it in other cases as well: for example nearby_node() is
> duplicated between arch/x86/kernel/cpu/amd.c and
> arch/x86/kernel/cpu/hygon.c too.
It is sad, and yeah I looked at nearby_node() to see if that was nevertheless
expected.
AMD and Hygon pretend that they are different, and use different families
for what is effectively the same processor, and that's silly. In fact back in
2018 (https://www.mail-archive.com/[email protected]/msg1741155.html)
I complained that if AMD and Hygon are organizing the family numbers so
that there's never going to be a conflict, it makes no sense to have a
separate vendor at all.
Yes it's not a lot of code but sooner or later some change will be applied
only to amd.c, because honestly who even thinks of Hygon...
Paolo
On Fri, Sep 22, 2023 at 12:26:09PM +0200, Ingo Molnar wrote:
> It's a bit sad that we are duplicating identical code.
>
> 2)
>
> We are doing it in other cases as well: for example nearby_node() is
> duplicated between arch/x86/kernel/cpu/amd.c and
> arch/x86/kernel/cpu/hygon.c too.
We could do a unification pass at some point. At the moment is not worth
the effort, IMO, for only a handful of small functions.
> BTW., while look at this code I noticed that the 'Author' copyright
> tag in arch/x86/kernel/cpu/hygon.c seems to be inaccurate:
>
> // SPDX-License-Identifier: GPL-2.0+
> /*
> * Hygon Processor Support for Linux
> *
> * Copyright (C) 2018 Chengdu Haiguang IC Design Co., Ltd.
> *
> * Author: Pu Wen <[email protected]>
> */
>
> ... as for example the nearby_node() was clearly copied & derived from
> arch/x86/kernel/cpu/amd.c, which does not appear to be accurately reflected
> in this copyright notice?
Perhaps it should say "copied from amd.c and adjusted" or so. That whole
file has pretty-much copied parts of amd.c AFAICT.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Fri, Sep 22, 2023 at 01:18:09PM +0200, Paolo Bonzini wrote:
> AMD and Hygon pretend that they are different, and use different families
> for what is effectively the same processor, and that's silly.
Not entirely true. Off and on they come with enablement which is close
but then a bit different and it is either ifdeffery or do their own
functions. For simplicity, we've done "do their own functions" as we
cannot test the common code on Hygon.
> ... because honestly who even thinks of Hygon...
There's that too.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette