Re: [PATCH v5 03/16] x86/mtrr: get MTRR number and support TOP_MEM2

2018-09-04 Thread Pu Wen 

On 2018/9/4 16:02, Borislav Petkov wrote:

shows only old mails so I'm going to assume this got fixed, finally! And
you probably have received a *fixed* BIOS even, allegedly.

So what's up?


I tested the function on Hygon Dhyana platforms with the latest BIOS,
found that this problem is indeed fixed. So the workaround is not
needed for Hygon Dhyana platforms any more, and the vendor checking
for Hygon will be removed in next version of patch.

Thanks,
Pu Wen

Re: [PATCH v5 03/16] x86/mtrr: get MTRR number and support TOP_MEM2

2018-09-04 Thread Borislav Petkov 
On Tue, Sep 04, 2018 at 11:02:41AM +0800, Pu Wen wrote:
> > > - if (!((boot_cpu_data.x86_vendor == X86_VENDOR_AMD) &&
> > > -   (boot_cpu_data.x86 >= 0x0f)))
> > > + if (!((boot_cpu_data.x86_vendor == X86_VENDOR_AMD &&
> > > +boot_cpu_data.x86 >= 0x0f) ||
> > > +boot_cpu_data.x86_vendor == X86_VENDOR_HYGON))
> > 
> > Why are you even touching this statement? The function returns early on
> > !X86_VENDOR_AMD.
> 
> The statement is briefly equal to !(X86_VENDOR_AMD || X86_VENDOR_HYGON).
> So the function will not return early on !X86_VENDOR_AMD. :-)

I meant the *original* version - not yours!

My question is why are you even touching a K8-old BIOS workaround?! Your
commit message says:

The MtrrFixDramModEn bit on Hygon platform should also be set to 1
during BIOS initialization of the fixed MTRRs, then cleared to 0 for
operation.

Is your BIOS already broken?

Searching the net for

pr_err(FW_WARN "MTRR: CPU %u: SYSCFG[MtrrFixDramModEn]"
   " not cleared by BIOS, clearing this bit\n",

shows only old mails so I'm going to assume this got fixed, finally! And
you probably have received a *fixed* BIOS even, allegedly.

So what's up?

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v5 03/16] x86/mtrr: get MTRR number and support TOP_MEM2

2018-09-03 Thread Pu Wen 

On 2018/9/4 3:04, Borislav Petkov wrote:

It was "Hygon Dhyana" before now "Hygon" only. Can we agree on the
naming nomenclature and stick with it.


OK, agree on it.

...


-   if (!((boot_cpu_data.x86_vendor == X86_VENDOR_AMD) &&
- (boot_cpu_data.x86 >= 0x0f)))
+   if (!((boot_cpu_data.x86_vendor == X86_VENDOR_AMD &&
+  boot_cpu_data.x86 >= 0x0f) ||
+  boot_cpu_data.x86_vendor == X86_VENDOR_HYGON))


Why are you even touching this statement? The function returns early on
!X86_VENDOR_AMD.


The statement is briefly equal to !(X86_VENDOR_AMD || X86_VENDOR_HYGON).
So the function will not return early on !X86_VENDOR_AMD. :-)

Also the statement can be changed to:
+   if (!(boot_cpu_data.x86_vendor == X86_VENDOR_AMD &&
+ boot_cpu_data.x86 >= 0x0f) &&
+   !(boot_cpu_data.x86_vendor == X86_VENDOR_HYGON))
or:
+   if ((boot_cpu_data.x86_vendor != X86_VENDOR_AMD ||
+boot_cpu_data.x86 < 0x0f) &&
+   (boot_cpu_data.x86_vendor != X86_VENDOR_HYGON))

Which statement is better?

Thanks,
Pu Wen

Re: [PATCH v5 03/16] x86/mtrr: get MTRR number and support TOP_MEM2

2018-09-03 Thread Borislav Petkov 
On Wed, Aug 29, 2018 at 08:43:23PM +0800, Pu Wen wrote:
> Hygon CPU have a special magic MSR way to force WB for memory >4GB,

It was "Hygon Dhyana" before now "Hygon" only. Can we agree on the
naming nomenclature and stick with it.

Also, it is "The ... CPU has a special..."

> and also support TOP_MEM2. Therefore, it is necessary to add Hygon
> support in amd_special_default_mtrr().
> 
> The MtrrFixDramModEn bit on Hygon platform should also be set to 1
> during BIOS initialization of the fixed MTRRs, then cleared to 0 for
> operation.
> 
> The number of variable MTRRs for Hygon is 2 as AMD's.
> 
> Signed-off-by: Pu Wen 
> ---
>  arch/x86/kernel/cpu/mtrr/cleanup.c | 3 ++-
>  arch/x86/kernel/cpu/mtrr/generic.c | 5 +++--
>  arch/x86/kernel/cpu/mtrr/mtrr.c| 2 +-
>  3 files changed, 6 insertions(+), 4 deletions(-)

...

> diff --git a/arch/x86/kernel/cpu/mtrr/generic.c 
> b/arch/x86/kernel/cpu/mtrr/generic.c
> index e12ee86..77c3eaa 100644
> --- a/arch/x86/kernel/cpu/mtrr/generic.c
> +++ b/arch/x86/kernel/cpu/mtrr/generic.c
> @@ -49,8 +49,9 @@ static inline void k8_check_syscfg_dram_mod_en(void)
>  {
>   u32 lo, hi;
>  
> - if (!((boot_cpu_data.x86_vendor == X86_VENDOR_AMD) &&
> -   (boot_cpu_data.x86 >= 0x0f)))
> + if (!((boot_cpu_data.x86_vendor == X86_VENDOR_AMD &&
> +boot_cpu_data.x86 >= 0x0f) ||
> +boot_cpu_data.x86_vendor == X86_VENDOR_HYGON))

Why are you even touching this statement? The function returns early on
!X86_VENDOR_AMD.

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[PATCH v5 03/16] x86/mtrr: get MTRR number and support TOP_MEM2

2018-08-29 Thread Pu Wen 
Hygon CPU have a special magic MSR way to force WB for memory >4GB,
and also support TOP_MEM2. Therefore, it is necessary to add Hygon
support in amd_special_default_mtrr().

The MtrrFixDramModEn bit on Hygon platform should also be set to 1
during BIOS initialization of the fixed MTRRs, then cleared to 0 for
operation.

The number of variable MTRRs for Hygon is 2 as AMD's.

Signed-off-by: Pu Wen 
---
 arch/x86/kernel/cpu/mtrr/cleanup.c | 3 ++-
 arch/x86/kernel/cpu/mtrr/generic.c | 5 +++--
 arch/x86/kernel/cpu/mtrr/mtrr.c| 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/mtrr/cleanup.c 
b/arch/x86/kernel/cpu/mtrr/cleanup.c
index 765afd5..3668c5d 100644
--- a/arch/x86/kernel/cpu/mtrr/cleanup.c
+++ b/arch/x86/kernel/cpu/mtrr/cleanup.c
@@ -831,7 +831,8 @@ int __init amd_special_default_mtrr(void)
 {
u32 l, h;
 
-   if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
+   if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD &&
+   boot_cpu_data.x86_vendor != X86_VENDOR_HYGON)
return 0;
if (boot_cpu_data.x86 < 0xf)
return 0;
diff --git a/arch/x86/kernel/cpu/mtrr/generic.c 
b/arch/x86/kernel/cpu/mtrr/generic.c
index e12ee86..77c3eaa 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -49,8 +49,9 @@ static inline void k8_check_syscfg_dram_mod_en(void)
 {
u32 lo, hi;
 
-   if (!((boot_cpu_data.x86_vendor == X86_VENDOR_AMD) &&
- (boot_cpu_data.x86 >= 0x0f)))
+   if (!((boot_cpu_data.x86_vendor == X86_VENDOR_AMD &&
+  boot_cpu_data.x86 >= 0x0f) ||
+  boot_cpu_data.x86_vendor == X86_VENDOR_HYGON))
return;
 
rdmsr(MSR_K8_SYSCFG, lo, hi);
diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.c b/arch/x86/kernel/cpu/mtrr/mtrr.c
index 9a19c80..507039c 100644
--- a/arch/x86/kernel/cpu/mtrr/mtrr.c
+++ b/arch/x86/kernel/cpu/mtrr/mtrr.c
@@ -127,7 +127,7 @@ static void __init set_num_var_ranges(void)
 
if (use_intel())
rdmsr(MSR_MTRRcap, config, dummy);
-   else if (is_cpu(AMD))
+   else if (is_cpu(AMD) || is_cpu(HYGON))
config = 2;
else if (is_cpu(CYRIX) || is_cpu(CENTAUR))
config = 8;
-- 
2.7.4