Add support for Hygon Dhyana Family 18h processor
cover Add support for Hygon Dhyana Family 18h processor
As a new x86 CPU Vendor, Chengdu Haiguang IC Design Co., Ltd (Hygon) is a Joint Venture between AMD and Haiguang Information Technology Co., Ltd., and aims at providing high performance x86 processor for China server market.
The first generation Hygon's processor(Dhyana) originates from AMD technology and shares most of the architecture with AMD's family 17h, but with different CPU vendor ID("HygonGenuine") and family series
number: 18h (Hygon will negotiate with AMD to make sure that only Hygon will use family 18h).
To enable support of Xen to Hygon Dhyana CPU, we add a new vendor type (X86_VENDOR_HYGON, with value of 5), and share most of the codes with AMD family 17h.
This patch series have been applied and tested successfully on Hygon Dhyana processor. Also tested on AMD EPYC (family 17h) processor, it works fine and makes no harm to the existing codes.
Pu Wen (15):
x86/cpu: Create Hygon Dhyana architecture support file x86/cpu/mtrr: Add Hygon Dhyana support to get TOP_MEM2 x86/cpu/vpmu: Add Hygon Dhyana support for vPMU x86/cpu/mce: Add Hygon Dhyana support to the MCA infrastructure x86/spec_ctrl: Add Hygon Dhyana to the respective mitigation machinery x86/apic: Add Hygon Dhyana support
x86/acpi: Add Hygon Dhyana support
x86/iommu: Add Hygon Dhyana support
x86/pv: Add Hygon Dhyana support to emulate MSRs access x86/domain: Add Hygon Dhyana support
x86/domctl: Add Hygon Dhyana support
x86/traps: Add Hygon Dhyana support
x86/xstate: Add Hygon Dhyana support
x86/cpuid: Add Hygon Dhyana support
tools/libxc: Add Hygon Dhyana support
tools/libxc/xc_cpuid_x86.c | 16 +- xen/arch/x86/acpi/cpu_idle.c | 3 +- xen/arch/x86/acpi/cpufreq/cpufreq.c | 6 +- xen/arch/x86/acpi/cpufreq/powernow.c | 3 +- xen/arch/x86/apic.c | 5 + xen/arch/x86/cpu/Makefile | 1 + xen/arch/x86/cpu/common.c | 4 +- xen/arch/x86/cpu/cpu.h | 1 + xen/arch/x86/cpu/hygon.c | 296 +++++++++++++++++++++++++++++++++ xen/arch/x86/cpu/mcheck/amd_nonfatal.c | 5 +- xen/arch/x86/cpu/mcheck/mce.c | 6 +- xen/arch/x86/cpu/mcheck/mce_amd.c | 3 +- xen/arch/x86/cpu/mcheck/non-fatal.c | 3 +- xen/arch/x86/cpu/mcheck/vmce.c | 2 + xen/arch/x86/cpu/mtrr/generic.c | 5 +- xen/arch/x86/cpu/vpmu.c | 2 + xen/arch/x86/cpu/vpmu_amd.c | 2 + xen/arch/x86/cpuid.c | 10 +- xen/arch/x86/dom0_build.c | 3 +- xen/arch/x86/domain.c | 9 +- xen/arch/x86/domctl.c | 13 +- xen/arch/x86/pv/emul-priv-op.c | 29 +++- xen/arch/x86/spec_ctrl.c | 6 +- xen/arch/x86/traps.c | 2 + xen/arch/x86/xstate.c | 5 +- xen/include/asm-x86/iommu.h | 1 + xen/include/asm-x86/x86-vendors.h | 3 +- 27 files changed, 406 insertions(+), 38 deletions(-) create mode 100644 xen/arch/x86/cpu/hygon.c
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH 1/15 x86/cpu: Create Hygon Dhyana architecture support file
Add x86 architecture support for a new processor: Hygon Dhyana Family 18h. Carve out initialization codes from amd.c needed by Dhyana into a separate file hygon.c by removing unnecessary codes and make Hygon initialization codes more clear.
To identify Hygon Dhyana CPU, add a new vendor type X86_VENDOR_HYGON for system recognition.
--- xen/arch/x86/cpu/Makefile | 1 + xen/arch/x86/cpu/common.c | 1 + xen/arch/x86/cpu/cpu.h | 1 + xen/arch/x86/cpu/hygon.c | 296 ++++++++++++++++++++++++++++++++++++++ xen/include/asm-x86/x86-vendors.h | 3 +- 5 files changed, 301 insertions(+), 1 deletion(-) create mode 100644 xen/arch/x86/cpu/hygon.c
xen/arch/x86/cpu/Makefile +1 -0
@@ -8,4 +8,5 @@ obj-y += intel.o obj-y += intel_cacheinfo.o obj-y += mwait-idle.o obj-y += shanghai.o +obj-y += hygon.o obj-y += vpmu.o vpmu_amd.o vpmu_intel.o
xen/arch/x86/cpu/common.c +1 -0
@@ -704,6 +704,7 @@ void __init early_cpu_init(void) {intel_cpu_init ();amd_init_cpu (); +hygon_init_cpu ();centaur_init_cpu ();shanghai_init_cpu ();early_cpu_detect ();
xen/arch/x86/cpu/cpu.h +1 -0
@@ -19,5 +19,6 @@ extern void display_cacheinfo(struct cpuinfo_x86 *c);int intel_cpu_init (void );int amd_init_cpu (void ); +int hygon_init_cpu (void );int centaur_init_cpu (void );int shanghai_init_cpu (void );
xen/arch/x86/cpu/hygon.c new file +296 -0
296 lines not on this page · raw message
xen/include/asm-x86/x86-vendors.h +2 -1
@@ -9,6 +9,7 @@#define X86_VENDOR_AMD 2#define X86_VENDOR_CENTAUR 3#define X86_VENDOR_SHANGHAI 4 -#define X86_VENDOR_NUM 5 +#define X86_VENDOR_HYGON 5 +#define X86_VENDOR_NUM 6#endif /* __XEN_X86_VENDORS_H__ */
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 01/15] x86/cpu: Create Hygon Dhyana architecture support file
Quoted 7 lines · Pu Wen
xen/arch/x86/cpu/hygon.c new file +1
-0@@ -0,0 +1,296 @@ +#include <xen/init.h>
xen/arch/x86/cpu/hygon.c line 11
Quoted 10 lines
xen/arch/x86/cpu/hygon.c new file +10
-0+#include <asm/processor.h> +#include <asm/hvm/support.h> +#include <asm/spec_ctrl.h> + +#include "cpu.h" + +static unsigned int __initdata opt_cpuid_mask_l7s0_eax = ~0u ; +integer_param ("cpuid_mask_l7s0_eax" , opt_cpuid_mask_l7s0_eax); +static unsigned int __initdata opt_cpuid_mask_l7s0_ebx = ~0u ; +integer_param ("cpuid_mask_l7s0_ebx" , opt_cpuid_mask_l7s0_ebx);
These should be moved from the AMD specific code into the common cpu code (alongside the other masks) rather than duplicated here.
Quoted 4 lines
xen/arch/x86/cpu/hygon.c new file +4
-0+ +static inline int rdmsr_hygon_safe (unsigned int msr,unsigned int *lo, +unsigned int *hi) +{
xen/arch/x86/cpu/hygon.c line 25
Quoted 10 lines
xen/arch/x86/cpu/hygon.c new file +10
-0+int err; + + asmvolatile ("1: rdmsr\n2:\n" +".section .fixup,\"ax\"\n" +"3: movl %6,%2\n" +" jmp 2b\n" +".previous\n" + _ASM_EXTABLE(1b, 3b) + : "=a" (*lo), "=d" (*hi), "=r" (err) + : "c" (msr), "D" (0x9c5a203a), "2" (0), "i" (-EFAULT));
These rdmsr/wrmsr helpers with a password in %edi are only used in the K8 processors. Since Hygon is a Zen derivative, you shouldn't need any of these.
Quoted 247 lines
xen/arch/x86/cpu/hygon.c new file +247
-0+ +return err; +} + +static inline int wrmsr_hygon_safe (unsigned int msr,unsigned int lo, +unsigned int hi) +{ +int err; + + asmvolatile ("1: wrmsr\n2:\n" +".section .fixup,\"ax\"\n" +"3: movl %6,%0\n" +" jmp 2b\n" +".previous\n" + _ASM_EXTABLE(1b, 3b) + : "=r" (err) + : "c" (msr), "a" (lo), "d" (hi), "D" (0x9c5a203a), + "0" (0), "i" (-EFAULT)); + +return err; +} + +static void wrmsr_hygon (unsigned int msr,uint64_t val) +{ + asmvolatile ("wrmsr" :: +"c" (msr),"a" ((uint32_t )val), +"d" (val >>32 ),"D" (0x9c5a203a )); +} + +/* +* Sets caps in expected_levelling_cap, probes for the specified mask MSR, and +* set caps in levelling_caps if it is found. Returns the default value. +*/ +static uint64_t __init _probe_mask_msr (unsigned int msr,uint64_t caps) +{ +unsigned int hi, lo; + + expected_levelling_cap |= caps; + +if ((rdmsr_hygon_safe (msr, &lo, &hi) ==0 ) && + (wrmsr_hygon_safe (msr, lo, hi) ==0 )) + levelling_caps |= caps; + +return ((uint64_t )hi <<32 ) | lo; +} + +/* Probe for the existance of the expected masking MSRs. */ +static void __init noinline probe_masking_msrs(void) +{ +const struct cpuinfo_x86 *c = &boot_cpu_data; + +/* Work out which masking MSRs we should have. */ + cpuidmask_defaults._1cd = +_probe_mask_msr (MSR_K8_FEATURE_MASK , LCAP_1cd); + cpuidmask_defaults.e1cd = +_probe_mask_msr (MSR_K8_EXT_FEATURE_MASK , LCAP_e1cd); +if (c->cpuid_level >=7 ) + cpuidmask_defaults._7ab0 = +_probe_mask_msr (MSR_AMD_L7S0_FEATURE_MASK , LCAP_7ab0); +} + +/* +* Context switch CPUID masking state to the next domain. Only called if +* CPUID Faulting isn't available, but masking MSRs have been detected. A +* parameter of NULL is used to context switch to the default host state (by +* the cpu bringup-code, crash path, etc). +*/ +static void hygon_ctxt_switch_masking (const struct vcpu *next) +{ +struct cpuidmasks *these_masks = &this_cpu (cpuidmasks); +const struct domain *nextd = next ? next->domain :NULL ; +const struct cpuidmasks *masks = + (nextd &&is_pv_domain (nextd) && nextd->arch.pv.cpuidmasks) + ? nextd->arch.pv.cpuidmasks : &cpuidmask_defaults; + +if ((levelling_caps & LCAP_1cd) == LCAP_1cd) { +uint64_t val = masks->_1cd; + +/* +* OSXSAVE defaults to 1, which causes fast-forwarding of +* Xen's real setting. Clobber it if disabled by the guest +* kernel. +*/ +if (next &&is_pv_vcpu (next) && !is_idle_vcpu (next) && + !(next->arch.pv.ctrlreg[4 ] &X86_CR4_OSXSAVE )) + val &= ~((uint64_t )cpufeat_mask (X86_FEATURE_OSXSAVE ) <<32 ); + +if (unlikely (these_masks->_1cd != val)) { +wrmsr_hygon (MSR_K8_FEATURE_MASK , val); + these_masks->_1cd = val; + } + } + +#define LAZY (cap, msr, field) \ + ({ \ + if (unlikely(these_masks->field != masks->field) && \ + ((levelling_caps & cap) == cap)) { \ + wrmsr_hygon(msr, masks->field); \ + these_masks->field = masks->field; \ + } \ + }) + +LAZY (LCAP_e1cd,MSR_K8_EXT_FEATURE_MASK , e1cd); +LAZY (LCAP_7ab0,MSR_AMD_L7S0_FEATURE_MASK , _7ab0); +#undef LAZY +} + +/* +* Mask the features and extended features returned by CPUID. Parameters are +* set from the boot line via user-defined masks. +*/ +static void __init noinline hygon_init_levelling(void) +{ +probe_masking_msrs (); + +if ((levelling_caps & LCAP_1cd) == LCAP_1cd) { +uint32_t ecx, edx, tmp; + +cpuid (0x00000001 , &tmp, &tmp, &ecx, &edx); + +if (~(opt_cpuid_mask_ecx & opt_cpuid_mask_edx)) { + ecx &= opt_cpuid_mask_ecx; + edx &= opt_cpuid_mask_edx; + } + +/* Fast-forward bits - Must be set. */ +if (ecx &cpufeat_mask (X86_FEATURE_XSAVE )) + ecx |=cpufeat_mask (X86_FEATURE_OSXSAVE ); + edx |=cpufeat_mask (X86_FEATURE_APIC ); + +/* Allow the HYPERVISOR bit to be set via guest policy. */ + ecx |=cpufeat_mask (X86_FEATURE_HYPERVISOR ); + + cpuidmask_defaults._1cd = ((uint64_t )ecx <<32 ) | edx; + } + +if ((levelling_caps & LCAP_e1cd) == LCAP_e1cd) { +uint32_t ecx, edx, tmp; + +cpuid (0x80000001 , &tmp, &tmp, &ecx, &edx); + +if (~(opt_cpuid_mask_ext_ecx & opt_cpuid_mask_ext_edx)) { + ecx &= opt_cpuid_mask_ext_ecx; + edx &= opt_cpuid_mask_ext_edx; + } + +/* Fast-forward bits - Must be set. */ + edx |=cpufeat_mask (X86_FEATURE_APIC ); + + cpuidmask_defaults.e1cd = ((uint64_t )ecx <<32 ) | edx; + } + +if ((levelling_caps & LCAP_7ab0) == LCAP_7ab0) { +uint32_t eax, ebx, tmp; + +cpuid (0x00000007 , &eax, &ebx, &tmp, &tmp); + +if (~(opt_cpuid_mask_l7s0_eax & opt_cpuid_mask_l7s0_ebx)) { + eax &= opt_cpuid_mask_l7s0_eax; + ebx &= opt_cpuid_mask_l7s0_ebx; + } + + cpuidmask_defaults._7ab0 &= ((uint64_t )eax <<32 ) | ebx; + } + +if (opt_cpu_info) { +printk (XENLOG_INFO "Levelling caps: %#x\n" , levelling_caps); +printk (XENLOG_INFO +"MSR defaults: 1d 0x%08x, 1c 0x%08x, e1d 0x%08x, " +"e1c 0x%08x, 7a0 0x%08x, 7b0 0x%08x\n" , + (uint32_t )cpuidmask_defaults._1cd, + (uint32_t )(cpuidmask_defaults._1cd >>32 ), + (uint32_t )cpuidmask_defaults.e1cd, + (uint32_t )(cpuidmask_defaults.e1cd >>32 ), + (uint32_t )(cpuidmask_defaults._7ab0 >>32 ), + (uint32_t )cpuidmask_defaults._7ab0); + } + +if (levelling_caps) + ctxt_switch_masking = hygon_ctxt_switch_masking; +} + +static void hygon_get_topology (struct cpuinfo_x86 *c) +{ +u32 ebx; + +if (c->x86_max_cores <=1 ) +return ; + +/* Convert local APIC ID into the socket ID */ + c->phys_proc_id >>= (cpuid_ecx (0x80000008 ) >>12 ) &0xf ; + + ebx =cpuid_ebx (0x8000001e ); + c->x86_num_siblings = ((ebx >>8 ) &0x3 ) +1 ; + c->x86_max_cores /= c->x86_num_siblings; + c->cpu_core_id = ebx &0xff ; + +if (opt_cpu_info) +printk ("CPU %d(%d) -> Processor %d, Core %d\n" , +smp_processor_id (), c->x86_max_cores, + c->phys_proc_id, c->cpu_core_id); +} + +static void early_init_hygon (struct cpuinfo_x86 *c) +{ +if (c == &boot_cpu_data) +hygon_init_levelling (); + +ctxt_switch_levelling (NULL ); +} + +static void init_hygon (struct cpuinfo_x86 *c) +{ +u32 l, h; +unsigned long long value; + +/* Attempt to set lfence to be Dispatch Serialising. */ +if (rdmsr_safe (MSR_AMD64_DE_CFG , value)) +/* Unable to read. Assume the safer default. */ +__clear_bit (X86_FEATURE_LFENCE_DISPATCH , c->x86_capability); +else if (value &AMD64_DE_CFG_LFENCE_SERIALISE ) +/* Already dispatch serialising. */ +__set_bit (X86_FEATURE_LFENCE_DISPATCH , c->x86_capability); + +/* +* If the user has explicitly chosen to disable Memory Disambiguation +* to mitigiate Speculative Store Bypass, poke the appropriate MSR. +*/ +if (opt_ssbd && !rdmsr_safe (MSR_AMD64_LS_CFG , value)) { + value |=1ull <<10 ; +wrmsr_safe (MSR_AMD64_LS_CFG , value); + } + +display_cacheinfo (c); + +if (cpu_has (c,X86_FEATURE_ITSC )) { +__set_bit (X86_FEATURE_CONSTANT_TSC , c->x86_capability); +__set_bit (X86_FEATURE_NONSTOP_TSC , c->x86_capability); +__set_bit (X86_FEATURE_TSC_RELIABLE , c->x86_capability); + } + + c->x86_max_cores = (cpuid_ecx (0x80000008 ) &0xff ) +1 ; + +hygon_get_topology (c); + +/* Hygon CPUs do not support SYSENTER outside of legacy mode. */ +__clear_bit (X86_FEATURE_SEP , c->x86_capability);
xen/arch/x86/cpu/hygon.c line 282
Quoted 10 lines
xen/arch/x86/cpu/hygon.c new file +10
-0+ +/* Hygon processors have APIC timer running in deep C states. */ +if ( opt_arat ) +__set_bit (X86_FEATURE_ARAT , c->x86_capability); + +if (cpu_has (c,X86_FEATURE_EFRO )) { +rdmsr (MSR_K7_HWCR , l, h); + l |= (1 <<27 );/* Enable read-only APERF/MPERF bit */ +wrmsr (MSR_K7_HWCR , l, h); + }
Is there anything which is actually unique to Hygon here? I ask, because this looks like a lot of duplicate code, considering that the processor base is the same.
~Andrew
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 01/15] x86/cpu: Create Hygon Dhyana architecture support file
On 2018/12/21 18:20, Andrew Cooper wrote:
....
Quoted 7 lines
xen/arch/x86/cpu/hygon.c line 11
Quoted 4 lines
xen/arch/x86/cpu/hygon.c new file +4
-0+static unsigned int __initdata opt_cpuid_mask_l7s0_eax = ~0u ; +integer_param ("cpuid_mask_l7s0_eax" , opt_cpuid_mask_l7s0_eax); +static unsigned int __initdata opt_cpuid_mask_l7s0_ebx = ~0u ; +integer_param ("cpuid_mask_l7s0_ebx" , opt_cpuid_mask_l7s0_ebx);These should be moved from the AMD specific code into the common cpu code (alongside the other masks) rather than duplicated here.
Thanks for the suggestion, will move them into the common CPU code in next version patch.
...
Quoted 4 lines
Quoted 4 lines
xen/arch/x86/cpu/hygon.c new file +4
-0+ asmvolatile ("1: rdmsr\n2:\n" +".section .fixup,\"ax\"\n" +"3: movl %6,%2\n" + " jmp 2b\n"
Quoted 8 lines
xen/arch/x86/cpu/hygon.c line 25
Quoted 4 lines
xen/arch/x86/cpu/hygon.c new file +4
-0+".previous\n" + _ASM_EXTABLE(1b ,3b ) + : "=a" (*lo), "=d" (*hi), "=r" (err) + : "c" (msr), "D" (0x9c5a203a), "2" (0), "i" (-EFAULT));These rdmsr/wrmsr helpers with a password in %edi are only used in the K8 processors. Since Hygon is a Zen derivative, you shouldn't need any of these.
Thanks for the correction. Will use the common functions rdmsr_safe and wrmsr_safe instead, and remove the private helpers rdmsr/wrmsr_hygon_safe.
...
Quoted 8 lines
Quoted 8 lines
xen/arch/x86/cpu/hygon.c new file +8
-0+/* Hygon CPUs do not support SYSENTER outside of legacy mode. */ +__clear_bit (X86_FEATURE_SEP , c->x86_capability); + +/* Hygon processors have APIC timer running in deep C states. */ +if ( opt_arat ) +__set_bit (X86_FEATURE_ARAT , c->x86_capability); + +if (cpu_has (c,X86_FEATURE_EFRO )) {
Quoted 8 lines
xen/arch/x86/cpu/hygon.c line 282
Quoted 4 lines
xen/arch/x86/cpu/hygon.c new file +4
-0+rdmsr (MSR_K7_HWCR , l, h); + l |= (1 <<27 );/* Enable read-only APERF/MPERF bit */ +wrmsr (MSR_K7_HWCR , l, h); + }Is there anything which is actually unique to Hygon here? I ask, because this looks like a lot of duplicate code, considering that the processor base is the same.
Right now these codes are necessary for Hygon Dhyana processor even though they are duplicated. As Hygon Dhyana support many CPU features such as ITSC and EFRO, so I think the "if cpu_has" determine should be removed to make the code clear and essential.
Keeping the codes into a separate compilation unit(hygon.c) at least has two advantages:
1) Make the code flow more clear. Hygon is a new joint venture which has no historical old architectures, so I'm afraid that there are sufficient motivations to keep a clear new processor init flow.
2) Beneficial for the future maintaining. AMD and Hygon may maintain their respective architecture related codes with no interaction with each other.
For these reasons, we choose to keep the architecture initialization codes in hygon.c.
Thx.
Signature · 8 lines
Regards, Pu Wen _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 01/15] x86/cpu: Create Hygon Dhyana architecture support file
Quoted 18 lines · Pu Wen
Quoted 8 lines · Andrew Cooper
Quoted 8 lines
xen/arch/x86/cpu/hygon.c new file +8
-0+/* Hygon CPUs do not support SYSENTER outside of legacy mode. */ +__clear_bit (X86_FEATURE_SEP , c->x86_capability); + +/* Hygon processors have APIC timer running in deep C states. */ +if ( opt_arat ) +__set_bit (X86_FEATURE_ARAT , c->x86_capability); + +if (cpu_has (c,X86_FEATURE_EFRO )) {Quoted 8 lines
xen/arch/x86/cpu/hygon.c line 282
Quoted 4 lines
xen/arch/x86/cpu/hygon.c new file +4
-0+rdmsr (MSR_K7_HWCR , l, h); + l |= (1 <<27 );/* Enable read-only APERF/MPERF bit */ +wrmsr (MSR_K7_HWCR , l, h); + }Is there anything which is actually unique to Hygon here? I ask, because this looks like a lot of duplicate code, considering that the processor base is the same.
Quoted 4 lines
Right now these codes are necessary for Hygon Dhyana processor even though they are duplicated. As Hygon Dhyana support many CPU features such as ITSC and EFRO, so I think the "if
cpu_has" determine should be removed to make the code clear and essential.
What if you later add new CPUs that have a different set of features? You will have to change this code to use cpu_has again in order to cope with different models having different features.
Using cpu_has is IMO the correct approach, even tough if you are targeting a single CPU model ATM.
Quoted 5 lines
Keeping the codes into a separate compilation unit(hygon.c) at least has two advantages:
1) Make the code flow more clear. Hygon is a new joint venture which has no historical old architectures, so I'm afraid that there are sufficient motivations to keep a clear new processor init flow.
AFAICT the code that you add is mostly a reduced copy of AMD's code, so from a code PoV it doesn't matter much whether Hygon is a new company or not, if the code is the same it should be shared.
Adding a new compilation unit is fine, but you should make the AMD functions that you need to use global and call them from your hygon.c file.
Quoted 3 lines
2) Beneficial for the future maintaining. AMD and Hygon may maintain their respective architecture related codes with no interaction with each other.
But likely bugs in code or erratum that affect AMD's family 17h are going to affect Hygon CPU, in which case you want to share the code so that fixes done by AMD or Hygon will be shared.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 01/15] x86/cpu: Create Hygon Dhyana architecture support file
In reply to Pu WenQuoted 31 lines · Pu Wen
On 2018/12/21 18:20, Andrew Cooper wrote:
....Quoted 7 lines
Quoted 7 lines
xen/arch/x86/cpu/hygon.c new file +7
-0+/* Hygon CPUs do not support SYSENTER outside of legacy mode. */ +__clear_bit (X86_FEATURE_SEP , c->x86_capability); + +/* Hygon processors have APIC timer running in deep C states. */ +if ( opt_arat ) +__set_bit (X86_FEATURE_ARAT , c->x86_capability); +Quoted 8 lines
xen/arch/x86/cpu/hygon.c line 282
Quoted 5 lines
xen/arch/x86/cpu/hygon.c new file +5
-0+if (cpu_has (c,X86_FEATURE_EFRO )) { +rdmsr (MSR_K7_HWCR , l, h); + l |= (1 <<27 );/* Enable read-only APERF/MPERF bit */ +wrmsr (MSR_K7_HWCR , l, h); + }Is there anything which is actually unique to Hygon here? I ask, because this looks like a lot of duplicate code, considering that the processor base is the same.
Right now these codes are necessary for Hygon Dhyana processor even though they are duplicated. As Hygon Dhyana support many CPU features such as ITSC and EFRO, so I think the "if
cpu_has" determine should be removed to make the code clear and essential.Keeping the codes into a separate compilation unit(hygon.c) at least has two advantages:
1) Make the code flow more clear. Hygon is a new joint venture which has no historical old architectures, so I'm afraid that there are sufficient motivations to keep a clear new processor init flow.
2) Beneficial for the future maintaining. AMD and Hygon may maintain their respective architecture related codes with no interaction with each other.
Quoted 2 lines
For these reasons, we choose to keep the architecture initialization codes in
hygon.c.
The most important question here is how likely is it to diverge in the future?
Where possible, duplicate code should be kept to a minimum, because of the risk of it being modified in only one of the places.
If Hygon is expected to diverge substantially in the future, then perhaps the duplication is fine. If Hygon is unlikely to diverge far from Zen (particularly if you intend to use newer Zen cores as new Hygon bases), then perhaps it would be worth making a common amd_base.c file, and restrict amd.c and hygon.c to unique features.
~Andrew
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 01/15] x86/cpu: Create Hygon Dhyana architecture support file
Quoted 19 lines · Andrew Cooper
Quoted 18 lines · Pu Wen
Quoted 3 lines · Andrew Cooper
Is there anything which is actually unique to Hygon here? I ask, because this looks like a lot of duplicate code, considering that the processor base is the same.
Right now these codes are necessary for Hygon Dhyana processor even though they are duplicated. As Hygon Dhyana support many CPU features such as ITSC and EFRO, so I think the "if
cpu_has" determine should be removed to make the code clear and essential.Keeping the codes into a separate compilation unit(hygon.c) at least has two advantages:
1) Make the code flow more clear. Hygon is a new joint venture which has no historical old architectures, so I'm afraid that there are sufficient motivations to keep a clear new processor init flow.
2) Beneficial for the future maintaining. AMD and Hygon may maintain their respective architecture related codes with no interaction with each other.
Quoted 6 lines
Quoted 2 lines
For these reasons, we choose to keep the architecture initialization codes in
hygon.c.The most important question here is how likely is it to diverge in the future?> > Where possible, duplicate code should be kept to a minimum, because of the risk of it being modified in only one of the places.
Yes, we are trying our best to make the duplicated code minimum but essential for Hygon Dhyana processor.
Quoted 5 lines
If Hygon is expected to diverge substantially in the future, then perhaps the duplication is fine. If Hygon is unlikely to diverge far from Zen (particularly if you intend to use newer Zen cores as new Hygon bases), then perhaps it would be worth making a common
amd_base.cfile, and restrictamd.candhygon.cto unique features.
Your point is correct, for future version, Hygon CPU will diverge from AMD Zen and do its own modification. So we lay the ground for the future code maintenance.
Actually we have discussed the same topic with the linux community[1], and finally reached some agreement that to keep hygon.c separated from AMD even though there are some duplicated codes at the moment[2].
We understand the difficulty to find a balance point between sharing codes and maintenance effort.
Any suggestion is welcome.
Reference:
[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/
Signature · 7 lines
Regards, Pu Wen _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 01/15] x86/cpu: Create Hygon Dhyana architecture support file
Quoted 37 lines · Pu Wen
Quoted 19 lines · Andrew Cooper
Quoted 18 lines · Pu Wen
Quoted 3 lines · Andrew Cooper
Is there anything which is actually unique to Hygon here? I ask, because this looks like a lot of duplicate code, considering that the processor base is the same.
Right now these codes are necessary for Hygon Dhyana processor even though they are duplicated. As Hygon Dhyana support many CPU features such as ITSC and EFRO, so I think the "if
cpu_has" determine should be removed to make the code clear and essential.Keeping the codes into a separate compilation unit(hygon.c) at least has two advantages:
1) Make the code flow more clear. Hygon is a new joint venture which has no historical old architectures, so I'm afraid that there are sufficient motivations to keep a clear new processor init flow.
2) Beneficial for the future maintaining. AMD and Hygon may maintain their respective architecture related codes with no interaction with each other.Quoted 5 lines
Quoted 2 lines
For these reasons, we choose to keep the architecture initialization codes in
hygon.c.The most important question here is how likely is it to diverge in the future?> > Where possible, duplicate code should be kept to a minimum, because of the risk of it being modified in only one of the places.
Yes, we are trying our best to make the duplicated code minimum but essential for Hygon Dhyana processor.
Quoted 5 lines
If Hygon is expected to diverge substantially in the future, then perhaps the duplication is fine. If Hygon is unlikely to diverge far from Zen (particularly if you intend to use newer Zen cores as new Hygon bases), then perhaps it would be worth making a common
amd_base.cfile, and restrictamd.candhygon.cto unique features.Your point is correct, for future version, Hygon CPU will diverge from AMD Zen and do its own modification. So we lay the ground for the future code maintenance.
Quoted 7 lines
Actually we have discussed the same topic with the linux community[1], and finally reached some agreement that to keep
hygon.cseparated from AMD even though there are some duplicated codes at the moment[2].We understand the difficulty to find a balance point between sharing codes and maintenance effort.
Any suggestion is welcome.
If we think this is going to be the extent of the duplication, then lets follow suit with Linux.
That said, I may want to rethink the "xen/amd: Support for guest MSR_VIRT_SPEC_CTRL support" series as Hygon needs this logic as well.
~Andrew
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH 2/15 x86/cpu/mtrr: Add Hygon Dhyana support to get TOP_MEM2
In reply to Pu WenThe Hygon Dhyana CPU supports the MSR way to get TOP_MEM2. So add Hygon Dhyana support to print the value of TOP_MEM2.
--- xen/arch/x86/cpu/mtrr/generic.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
xen/arch/x86/cpu/mtrr/generic.c +3 -2
@@ -217,8 +217,9 @@ static void __init print_mtrr_state(const char *level)printk ("%s %u disabled\n" , level, i); } -if (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD - && boot_cpu_data.x86 >=0xf ) { +if ((boot_cpu_data.x86_vendor ==X86_VENDOR_AMD && + boot_cpu_data.x86 >=0xf ) || + boot_cpu_data.x86_vendor ==X86_VENDOR_HYGON ) {uint64_t syscfg, tom2;rdmsrl (MSR_K8_SYSCFG , syscfg);
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH 3/15 x86/cpu/vpmu: Add Hygon Dhyana support for vPMU
In reply to Pu WenThe PMU architecture for the Hygon Dhyana CPU is similar to the AMD family 17h one. To support it, add Hygon Dhyana support in the similar way as AMD does.
--- xen/arch/x86/cpu/vpmu.c | 2 ++ xen/arch/x86/cpu/vpmu_amd.c | 2 ++ 2 files changed, 4 insertions(+)
xen/arch/x86/cpu/vpmu.c +2 -0
@@ -473,6 +473,7 @@ static int vpmu_arch_initialise(struct vcpu *v)switch ( vendor ) { +case X86_VENDOR_HYGON :case X86_VENDOR_AMD : ret =svm_vpmu_initialise (v);break ;
@@ -890,6 +891,7 @@ static int __init vpmu_init(void)switch ( vendor ) { +case X86_VENDOR_HYGON :case X86_VENDOR_AMD :if (amd_vpmu_init () ) vpmu_mode =XENPMU_MODE_OFF ;
xen/arch/x86/cpu/vpmu_amd.c +2 -0
@@ -554,6 +554,8 @@ int __init amd_vpmu_init(void)case 0x12 :case 0x14 :case 0x16 : +case 0x17 : +case 0x18 : num_counters =F10H_NUM_COUNTERS ; counters =AMD_F10H_COUNTERS ; ctrls =AMD_F10H_CTRLS ;
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 03/15] x86/cpu/vpmu: Add Hygon Dhyana support for vPMU
Quoted 35 lines · Pu Wen
The PMU architecture for the Hygon Dhyana CPU is similar to the AMD family 17h one. To support it, add Hygon Dhyana support in the similar way as AMD does.
Signed-off-by Pu Wen <[email protected]>--- xen/arch/x86/cpu/vpmu.c | 2 ++ xen/arch/x86/cpu/vpmu_amd.c | 2 ++ 2 files changed, 4 insertions(+)xen/arch/x86/cpu/vpmu.c +2
-0@@ -473,6 +473,7 @@ static int vpmu_arch_initialise(struct vcpu *v)switch ( vendor ) { +case X86_VENDOR_HYGON :case X86_VENDOR_AMD : ret =svm_vpmu_initialise (v);break ;@@ -890,6 +891,7 @@ static int __init vpmu_init(void)switch ( vendor ) { +case X86_VENDOR_HYGON :case X86_VENDOR_AMD :if (amd_vpmu_init () ) vpmu_mode =XENPMU_MODE_OFF ;xen/arch/x86/cpu/vpmu_amd.c +0
-0
xen/arch/x86/cpu/vpmu_amd.c int __init amd_vpmu_init(void) · line 558
Quoted 5 lines
xen/arch/x86/cpu/vpmu_amd.c +2
-0@@ -554,6 +554,8 @@ int __init amd_vpmu_init(void)case 0x12 :case 0x14 :case 0x16 : +case 0x17 : +case 0x18 :
This also enables VPMU support for Zen which goes beyond what the commit message claims to do.
Also, why are you choosing to use legacy MSRs (and you did the same in Linux)? Doesn't Zen (which you are saying is similar to Hygon) support c001_020X bank?
-boris
xen/arch/x86/cpu/vpmu_amd.c int __init amd_vpmu_init(void) · line 561
Quoted 3 lines
xen/arch/x86/cpu/vpmu_amd.c +0
-0num_counters =F10H_NUM_COUNTERS ; counters =AMD_F10H_COUNTERS ; ctrls =AMD_F10H_CTRLS ;
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 03/15] x86/cpu/vpmu: Add Hygon Dhyana support for vPMU
On 2018/12/20 22:25, Boris Ostrovsky wrote:
...
Quoted 6 lines
Quoted 6 lines
xen/arch/x86/cpu/vpmu_amd.c +0
-0@@ -554,6 +554,8 @@ int __init amd_vpmu_init(void)case 0x12 :
Quoted 8 lines
xen/arch/x86/cpu/vpmu_amd.c int __init amd_vpmu_init(void) · line 558
Quoted 4 lines
xen/arch/x86/cpu/vpmu_amd.c +2
-0case 0x14 :case 0x16 : +case 0x17 : +case 0x18 :This also enables VPMU support for Zen which goes beyond what the commit message claims to do.
Sorry for the not clear commit message. Will add modification description in the commit message and make the changes complete.
On the other hand, since current Xen vPMU still not support Zen. so in this patch we enable 0x17 support. If this modification is not preferred, will remove AMD Xen 0x17 support in next version.
Quoted 4 lines
Also, why are you choosing to use legacy MSRs (and you did the same in Linux)? Doesn't Zen (which you are saying is similar to Hygon) support
c001_020Xbank?
In Linux, the Xen PMU driver use the default branch cases, which also use the legacy MSRs way. So we choose to follow legacy MSRs here in Dhyana cases.
Since both of Zen and Dhyana support C001_020X MSRs. If use the C001_020X is preferred, we will try to modify the related codes and create a patch. Also the Linux Xen PMU driver may need to be updated to use these MSRs.
Signature · 7 lines
Regards, Pu Wen _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 03/15] x86/cpu/vpmu: Add Hygon Dhyana support for vPMU
Quoted 15 lines · Pu Wen
On 2018/12/20 22:25, Boris Ostrovsky wrote:
...Quoted 5 lines
Quoted 5 lines
xen/arch/x86/cpu/vpmu_amd.c +0
-0@@ -554,6 +554,8 @@ int __init amd_vpmu_init(void)Quoted 8 lines
xen/arch/x86/cpu/vpmu_amd.c int __init amd_vpmu_init(void) · line 558
Quoted 5 lines
xen/arch/x86/cpu/vpmu_amd.c +2
-0case 0x12 :case 0x14 :case 0x16 : +case 0x17 : +case 0x18 :This also enables VPMU support for Zen which goes beyond what the commit message claims to do.
Quoted 6 lines
Sorry for the not clear commit message. Will add modification description in the commit message and make the changes complete.
On the other hand, since current Xen vPMU still not support Zen. so in this patch we enable
0x17support. If this modification is not preferred, will remove AMD Xen0x17support in next version.
Enabling 0x17 should be fine, I just thought commit message should be explicit about that.
Quoted 9 lines
Quoted 3 lines
Also, why are you choosing to use legacy MSRs (and you did the same in Linux)? Doesn't Zen (which you are saying is similar to Hygon) support
c001_020Xbank?In Linux, the Xen PMU driver use the default branch cases, which also use the legacy MSRs way. So we choose to follow legacy MSRs here in Dhyana cases.
Since both of Zen and Dhyana support
C001_020XMSRs. If use theC001_020Xis preferred, we will try to modify the related codes and create a patch.
I don't have a Zen box available right now but from what I can see 0x17 counters are compatible with 0x15 so I think switching to C001_020X should work. And looks like you are using those in Linux (non-Xen part) too.
Quoted 1 line
Also the Linux Xen PMU driver may need to be updated to use these MSRs.
Yes, although Linux part is used only by PV guests.
-boris
-boris
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 03/15] x86/cpu/vpmu: Add Hygon Dhyana support for vPMU
Quoted 19 lines · Boris Ostrovsky
Quoted 18 lines · Pu Wen
On 2018/12/20 22:25, Boris Ostrovsky wrote:
...Quoted 5 lines
Quoted 5 lines
xen/arch/x86/cpu/vpmu_amd.c +0
-0@@ -554,6 +554,8 @@ int __init amd_vpmu_init(void)Quoted 8 lines
xen/arch/x86/cpu/vpmu_amd.c int __init amd_vpmu_init(void) · line 558
Quoted 5 lines
xen/arch/x86/cpu/vpmu_amd.c +2
-0case 0x12 :case 0x14 :case 0x16 : +case 0x17 : +case 0x18 :This also enables VPMU support for Zen which goes beyond what the commit message claims to do.
Sorry for the not clear commit message. Will add modification description in the commit message and make the changes complete.
Quoted 6 lines
Quoted 3 lines
On the other hand, since current Xen vPMU still not support Zen. so in this patch we enable
0x17support. If this modification is not preferred, will remove AMD Xen0x17support in next version.Enabling
0x17should be fine, I just thought commit message should be explicit about that.
OK, will explicit describe the enabling of 0x17 in the commit message in next version patch set. Thanks for the suggestion.
Quoted 11 lines
Quoted 9 lines
Quoted 3 lines
Also, why are you choosing to use legacy MSRs (and you did the same in Linux)? Doesn't Zen (which you are saying is similar to Hygon) support
c001_020Xbank?In Linux, the Xen PMU driver use the default branch cases, which also use the legacy MSRs way. So we choose to follow legacy MSRs here in Dhyana cases.
Since both of Zen and Dhyana support
C001_020XMSRs. If use theC001_020Xis preferred, we will try to modify the related codes and create a patch.
Quoted 6 lines
I don't have a Zen box available right now but from what I can see
0x17counters are compatible with0x15so I think switching toC001_020Xshould work. And looks like you are using those in Linux (non-Xen part) too.Quoted 1 line
Also the Linux Xen PMU driver may need to be updated to use these MSRs.
Yes, although Linux part is used only by PV guests.
Yes, I have tested the MSRs of the 0x15 ones by booting a Dom0 PV guest. It works even when the Linux Xen PMU driver use the legacy MSRs. I'll test the PV guest by using C001_020X in the Linux Xen part tomorrow.
Thx.
Signature · 7 lines
Regards, Pu Wen _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 03/15] x86/cpu/vpmu: Add Hygon Dhyana support for vPMU
In reply to Pu WenQuoted 12 lines
Quoted 1 line
Quoted 1 line · On 20.12.18 at 14:12, <[email protected]> wrote:
xen/arch/x86/cpu/vpmu.c +1
-0@@ -473,6 +473,7 @@ static int vpmu_arch_initialise(struct vcpu *v)switch ( vendor ) { +case X86_VENDOR_HYGON :case X86_VENDOR_AMD : ret =svm_vpmu_initialise (v);break ;
xen/arch/x86/cpu/vpmu.c static int __init vpmu_init(void) · line 897
Quoted 7 lines
xen/arch/x86/cpu/vpmu.c +1
-0@@ -890,6 +891,7 @@ static int __init vpmu_init(void)switch ( vendor ) { +case X86_VENDOR_HYGON :case X86_VENDOR_AMD :if (amd_vpmu_init () ) vpmu_mode =XENPMU_MODE_OFF ;
Here and everywhere else, may I ask that you do your insertions below the respective AMD ones instead of above?
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 03/15] x86/cpu/vpmu: Add Hygon Dhyana support for vPMU
Quoted 13 lines · Jan Beulich
Quoted 13 lines
Quoted 1 line
Quoted 1 line · On 20.12.18 at 14:12, <[email protected]> wrote:
xen/arch/x86/cpu/vpmu.c +1
-0@@ -473,6 +473,7 @@ static int vpmu_arch_initialise(struct vcpu *v)switch ( vendor ) { +case X86_VENDOR_HYGON :case X86_VENDOR_AMD : ret =svm_vpmu_initialise (v);break ;@@ -890,6 +891,7 @@ static int __init vpmu_init(void)
Quoted 9 lines
xen/arch/x86/cpu/vpmu.c static int __init vpmu_init(void) · line 897
Quoted 6 lines
xen/arch/x86/cpu/vpmu.c +1
-0switch ( vendor ) { +case X86_VENDOR_HYGON :case X86_VENDOR_AMD :if (amd_vpmu_init () ) vpmu_mode =XENPMU_MODE_OFF ;Here and everywhere else, may I ask that you do your insertions below the respective AMD ones instead of above?
In all the switch cases, if Hygon directly followed AMD's cases, the insertions are placed above the respective AMD ones.
If Hygon used its own cases, such as in calc_ler_msr, the new cases are placed below the AMD ones.
Signature · 7 lines
Regards, Pu Wen _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 03/15] x86/cpu/vpmu: Add Hygon Dhyana support for vPMU
Quoted 25 lines
Quoted 1 line
Quoted 1 line · On 15.01.19 at 13:49, <[email protected]> wrote:
Quoted 13 lines · Jan Beulich
Quoted 13 lines
Quoted 1 line
Quoted 1 line · On 20.12.18 at 14:12, <[email protected]> wrote:
xen/arch/x86/cpu/vpmu.c +1
-0@@ -473,6 +473,7 @@ static int vpmu_arch_initialise(struct vcpu *v)switch ( vendor ) { +case X86_VENDOR_HYGON :case X86_VENDOR_AMD : ret =svm_vpmu_initialise (v);break ;@@ -890,6 +891,7 @@ static int __init vpmu_init(void)Quoted 9 lines
xen/arch/x86/cpu/vpmu.c static int __init vpmu_init(void) · line 897
Quoted 6 lines
xen/arch/x86/cpu/vpmu.c +1
-0switch ( vendor ) { +case X86_VENDOR_HYGON :case X86_VENDOR_AMD :if (amd_vpmu_init () ) vpmu_mode =XENPMU_MODE_OFF ;Here and everywhere else, may I ask that you do your insertions below the respective AMD ones instead of above?
Quoted 2 lines
In all the switch cases, if Hygon directly followed AMD's cases, the insertions are placed above the respective AMD ones.
As said - please don't, insert the new ones after AMD's.
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 03/15] x86/cpu/vpmu: Add Hygon Dhyana support for vPMU
Quoted 25 lines · Jan Beulich
Quoted 25 lines
Quoted 1 line
Quoted 1 line · On 15.01.19 at 13:49, <[email protected]> wrote:
Quoted 22 lines · Jan Beulich
Quoted 12 lines
Quoted 1 line
Quoted 1 line · On 20.12.18 at 14:12, <[email protected]> wrote:
xen/arch/x86/cpu/vpmu.c +1
-0@@ -473,6 +473,7 @@ static int vpmu_arch_initialise(struct vcpu *v)switch ( vendor ) { +case X86_VENDOR_HYGON :case X86_VENDOR_AMD : ret =svm_vpmu_initialise (v);break ;xen/arch/x86/cpu/vpmu.c static int __init vpmu_init(void) · line 897
Quoted 7 lines
xen/arch/x86/cpu/vpmu.c +1
-0@@ -890,6 +891,7 @@ static int __init vpmu_init(void)switch ( vendor ) { +case X86_VENDOR_HYGON :case X86_VENDOR_AMD :if (amd_vpmu_init () ) vpmu_mode =XENPMU_MODE_OFF ;Here and everywhere else, may I ask that you do your insertions below the respective AMD ones instead of above?
Quoted 4 lines
Quoted 2 lines
In all the switch cases, if Hygon directly followed AMD's cases, the insertions are placed above the respective AMD ones.
As said - please don't, insert the new ones after AMD's.
OK. Will insert the new ones after AMD's in next version patch set.
Signature · 7 lines
Regards, Pu Wen _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH 4/15 x86/cpu/mce: Add Hygon Dhyana support to the MCA infrastructure
In reply to Pu WenThe machine check architecture for Hygon Dhyana CPU is similar to the AMD family 17h one. Add vendor checking for Hygon Dhyana to share the code path of AMD family 17h.
--- xen/arch/x86/cpu/common.c | 3 ++- xen/arch/x86/cpu/mcheck/amd_nonfatal.c | 5 +++-- xen/arch/x86/cpu/mcheck/mce.c | 6 ++++-- xen/arch/x86/cpu/mcheck/mce_amd.c | 3 ++- xen/arch/x86/cpu/mcheck/non-fatal.c | 3 ++- xen/arch/x86/cpu/mcheck/vmce.c | 2 ++ 6 files changed, 15 insertions(+), 7 deletions(-)
xen/arch/x86/cpu/common.c +2 -1
@@ -345,7 +345,8 @@ static void __init early_cpu_detect(void) hap_paddr_bits =PADDR_BITS ; } -if (c->x86_vendor !=X86_VENDOR_AMD ) +if (c->x86_vendor !=X86_VENDOR_AMD && + c->x86_vendor !=X86_VENDOR_HYGON ) park_offline_cpus = opt_mce;initialize_cpu_data (0 );
xen/arch/x86/cpu/mcheck/amd_nonfatal.c +3 -2
@@ -203,10 +203,11 @@ static void mce_amd_work_fn(void *data)void __init amd_nonfatal_mcheck_init (struct cpuinfo_x86 *c) { -if (c->x86_vendor !=X86_VENDOR_AMD ) +if (c->x86_vendor !=X86_VENDOR_AMD && + c->x86_vendor !=X86_VENDOR_HYGON )return ; -/* Assume we are on K8 or newer AMD CPU here */ +/* Assume we are on K8 or newer AMD or Hygon CPU here */ /* The threshold bitfields in MSR_IA32_MC4_MISC has * been introduced along with the SVME feature bit. */
xen/arch/x86/cpu/mcheck/mce.c +4 -2
@@ -777,6 +777,7 @@ void mcheck_init(struct cpuinfo_x86 *c, bool bsp)switch ( c->x86_vendor ) { +case X86_VENDOR_HYGON :case X86_VENDOR_AMD : inited =amd_mcheck_init (c);break ;
@@ -1172,10 +1173,11 @@ static bool x86_mc_msrinject_verify(struct xen_mc_msrinject *mci)/* MSRs that the HV will take care of */ case MSR_K8_HWCR : -if ( c->x86_vendor ==X86_VENDOR_AMD ) +if ( c->x86_vendor ==X86_VENDOR_AMD || + c->x86_vendor ==X86_VENDOR_HYGON ) reason ="HV will operate HWCR" ;else - reason ="only supported on AMD" ; + reason ="only supported on AMD or Hygon" ;break ;default :
xen/arch/x86/cpu/mcheck/mce_amd.c +2 -1
@@ -162,7 +162,8 @@ mcequirk_lookup_amd_quirkdata(struct cpuinfo_x86 *c) {int i; -BUG_ON (c->x86_vendor !=X86_VENDOR_AMD ); +if (c->x86_vendor !=X86_VENDOR_AMD ) +return 0 ;for ( i =0 ; i <ARRAY_SIZE (mce_amd_quirks); i++ ) {
xen/arch/x86/cpu/mcheck/non-fatal.c +2 -1
@@ -100,8 +100,9 @@ static int __init init_nonfatal_mce_checker(void)* Check for non-fatal errors every MCE_RATE s */ switch (c->x86_vendor) { +case X86_VENDOR_HYGON :case X86_VENDOR_AMD : -/* Assume we are on K8 or newer AMD CPU here */ +/* Assume we are on K8 or newer AMD or Hygon CPU here */ amd_nonfatal_mcheck_init (c);break ;
xen/arch/x86/cpu/mcheck/vmce.c +2 -0
@@ -153,6 +153,7 @@ static int bank_mce_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val) ret =vmce_intel_rdmsr (v, msr, val);break ; +case X86_VENDOR_HYGON :case X86_VENDOR_AMD : ret =vmce_amd_rdmsr (v, msr, val);break ;
@@ -283,6 +284,7 @@ static int bank_mce_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val) ret =vmce_intel_wrmsr (v, msr, val);break ; +case X86_VENDOR_HYGON :case X86_VENDOR_AMD : ret =vmce_amd_wrmsr (v, msr, val);break ;
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 04/15] x86/cpu/mce: Add Hygon Dhyana support to the MCA infrastructure
xen/arch/x86/cpu/mcheck/mce_amd.c mcequirk_lookup_amd_quirkdata(struct cpuinfo_x86 *c) · line 166
Quoted 10 lines
Quoted 1 line
Quoted 1 line · On 20.12.18 at 14:12, <[email protected]> wrote:
xen/arch/x86/cpu/mcheck/mce_amd.c +2
-1@@ -162,7 +162,8 @@ mcequirk_lookup_amd_quirkdata(struct cpuinfo_x86 *c) {int i; -BUG_ON (c->x86_vendor !=X86_VENDOR_AMD ); +if (c->x86_vendor !=X86_VENDOR_AMD ) +return 0 ;
Please can you leave this untouched and change the single caller instead? If need be down the road (but of course you'll never introduce quirky behavior), we'd then add mcequirk_lookup_hygon_quirkdata().
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 04/15] x86/cpu/mce: Add Hygon Dhyana support to the MCA infrastructure
Quoted 7 lines · Jan Beulich
Quoted 7 lines
Quoted 1 line
Quoted 1 line · On 20.12.18 at 14:12, <[email protected]> wrote:
xen/arch/x86/cpu/mcheck/mce_amd.c +0
-0@@ -162,7 +162,8 @@ mcequirk_lookup_amd_quirkdata(struct cpuinfo_x86 *c) {int i;
Quoted 8 lines
xen/arch/x86/cpu/mcheck/mce_amd.c mcequirk_lookup_amd_quirkdata(struct cpuinfo_x86 *c) · line 166
Quoted 3 lines
xen/arch/x86/cpu/mcheck/mce_amd.c +2
-1-BUG_ON (c->x86_vendor !=X86_VENDOR_AMD ); +if (c->x86_vendor !=X86_VENDOR_AMD ) +return 0 ;Please can you leave this untouched and change the single caller instead? If need be down the road (but of course you'll never introduce quirky behavior), we'd then add
mcequirk_lookup_hygon_quirkdata().
Yes, I can leave this function untouched. And change the single caller amd_mcheck_init():
- enum mcequirk_amd_flags quirkflag = mcequirk_lookup_amd_quirkdata(ci);
+ enum mcequirk_amd_flags quirkflag = 0;
+
+ if (ci->x86_vendor != X86_VENDOR_HYGON)
+ quirkflag = mcequirk_lookup_amd_quirkdata(ci);
Is the modification OK?
Also add mcequirk_lookup_hygon_quirkdata() is another solution, even though it will do nothing at the moment.
Signature · 7 lines
Regards, Pu Wen _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 04/15] x86/cpu/mce: Add Hygon Dhyana support to the MCA infrastructure
Quoted 26 lines
Quoted 1 line
Quoted 1 line · On 15.01.19 at 13:52, <[email protected]> wrote:
Quoted 7 lines · Jan Beulich
Quoted 7 lines
Quoted 1 line
Quoted 1 line · On 20.12.18 at 14:12, <[email protected]> wrote:
xen/arch/x86/cpu/mcheck/mce_amd.c +0
-0@@ -162,7 +162,8 @@ mcequirk_lookup_amd_quirkdata(struct cpuinfo_x86 *c) {int i;Quoted 8 lines
xen/arch/x86/cpu/mcheck/mce_amd.c mcequirk_lookup_amd_quirkdata(struct cpuinfo_x86 *c) · line 166
Quoted 3 lines
xen/arch/x86/cpu/mcheck/mce_amd.c +2
-1-BUG_ON (c->x86_vendor !=X86_VENDOR_AMD ); +if (c->x86_vendor !=X86_VENDOR_AMD ) +return 0 ;Please can you leave this untouched and change the single caller instead? If need be down the road (but of course you'll never introduce quirky behavior), we'd then add
mcequirk_lookup_hygon_quirkdata().Yes, I can leave this function untouched. And change the single caller
amd_mcheck_init():(quoted diff) +4
-1- enum mcequirk_amd_flags quirkflag = mcequirk_lookup_amd_quirkdata(ci); + enum mcequirk_amd_flags quirkflag = 0; + + if (ci->x86_vendor != X86_VENDOR_HYGON) + quirkflag = mcequirk_lookup_amd_quirkdata(ci);
Quoted 1 line
Is the modification OK?
Yes.
Quoted 2 lines
Also add
mcequirk_lookup_hygon_quirkdata()is another solution, even though it will do nothing at the moment.
No need to introduce it until it would actually do anything.
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH 5/15 x86/spec_ctrl: Add Hygon Dhyana to the respective mitigation machinery
In reply to Pu WenThe Hygon Dhyana CPU has the same speculative execution as AMD family 17h, so share AMD Retpoline and PTI mitigation code with Hygon Dhyana.
--- xen/arch/x86/spec_ctrl.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
xen/arch/x86/spec_ctrl.c +4 -2
@@ -306,7 +306,8 @@ static bool __init retpoline_safe(uint64_t caps) {unsigned int ucode_rev =this_cpu (ucode_cpu_info).cpu_sig.rev; -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 true; if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
@@ -612,7 +613,8 @@ int8_t __read_mostly opt_xpti_domu = -1;static __init void xpti_init_default (uint64_t caps) { -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 ) caps =ARCH_CAPABILITIES_RDCL_NO ;if ( caps &ARCH_CAPABILITIES_RDCL_NO )
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH 6/15 x86/apic: Add Hygon Dhyana support
In reply to Pu WenAdd Hygon Dhyana support to use modern APIC.
--- xen/arch/x86/apic.c | 5 +++++ 1 file changed, 5 insertions(+)
xen/arch/x86/apic.c +5 -0
@@ -92,6 +92,11 @@ static int modern_apic(void)if (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD && boot_cpu_data.x86 >=0xf )return 1 ; + +/* Hygon systems use modern APIC */ +if (boot_cpu_data.x86_vendor ==X86_VENDOR_HYGON ) +return 1 ; + lvr =apic_read (APIC_LVR ); version =GET_APIC_VERSION (lvr);return version >=0x14 ;
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH 7/15 x86/acpi: Add Hygon Dhyana support
In reply to Pu WenAdd Hygon Dhyana support to the acpi cpufreq subsystem by using the code path of AMD.
--- xen/arch/x86/acpi/cpu_idle.c | 3 ++- xen/arch/x86/acpi/cpufreq/cpufreq.c | 6 ++++-- xen/arch/x86/acpi/cpufreq/powernow.c | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-)
xen/arch/x86/acpi/cpu_idle.c +2 -1
@@ -795,7 +795,8 @@ void acpi_dead_idle(void)__mwait (cx->address,0 ); } } -else if ( current_cpu_data.x86_vendor ==X86_VENDOR_AMD && +else if ( (current_cpu_data.x86_vendor ==X86_VENDOR_AMD || + current_cpu_data.x86_vendor ==X86_VENDOR_HYGON ) && cx->entry_method ==ACPI_CSTATE_EM_SYSIO ) {/* Intel prefers not to use SYSIO */
xen/arch/x86/acpi/cpufreq/cpufreq.c +4 -2
@@ -649,7 +649,8 @@ static int __init cpufreq_driver_init(void) (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)) ret =cpufreq_register_driver (&acpi_cpufreq_driver); else if ((cpufreq_controller == FREQCTL_xen) && - (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)) + (boot_cpu_data.x86_vendor == X86_VENDOR_AMD || + boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)) ret =powernow_register_driver ();return ret;
@@ -660,8 +661,9 @@ int cpufreq_cpu_init(unsigned int cpuid) {int ret; -/* Currently we only handle Intel and AMD processor */ +/* Currently we only handle Intel and AMD and Hygon processor */ if ( (boot_cpu_data.x86_vendor ==X86_VENDOR_INTEL ) || + (boot_cpu_data.x86_vendor ==X86_VENDOR_HYGON ) || (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD ) ) ret =cpufreq_add_cpu (cpuid);else
xen/arch/x86/acpi/cpufreq/powernow.c +2 -1
@@ -360,7 +360,8 @@ unsigned int __init powernow_register_driver()for_each_online_cpu (i) {struct cpuinfo_x86 *c = &cpu_data[i]; -if (c->x86_vendor !=X86_VENDOR_AMD ) +if (c->x86_vendor !=X86_VENDOR_AMD && + c->x86_vendor !=X86_VENDOR_HYGON ) ret = -ENODEV ;else {
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 07/15] x86/acpi: Add Hygon Dhyana support
Quoted 3 lines
Quoted 1 line
Quoted 1 line · On 20.12.18 at 14:14, <[email protected]> wrote:
Add Hygon Dhyana support to the acpi cpufreq subsystem by using the code path of AMD.
... cpufreq and cpuidle subsystems ...
xen/arch/x86/acpi/cpufreq/cpufreq.c int cpufreq_cpu_init(unsigned int cpuid) · line 664
Quoted 6 lines
xen/arch/x86/acpi/cpufreq/cpufreq.c +1
-1@@ -660,8 +661,9 @@ int cpufreq_cpu_init(unsigned int cpuid) {int ret; -/* Currently we only handle Intel and AMD processor */ +/* Currently we only handle Intel and AMD and Hygon processor */
Please replace the first "and" by a comma.
xen/arch/x86/acpi/cpufreq/cpufreq.c int cpufreq_cpu_init(unsigned int cpuid) · line 669
Quoted 5 lines
xen/arch/x86/acpi/cpufreq/cpufreq.c +1
-0if ( (boot_cpu_data.x86_vendor ==X86_VENDOR_INTEL ) || + (boot_cpu_data.x86_vendor ==X86_VENDOR_HYGON ) || (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD ) ) ret =cpufreq_add_cpu (cpuid);else
At the very least here I think you want to change to switch().
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 07/15] x86/acpi: Add Hygon Dhyana support
Quoted 5 lines · Jan Beulich
Quoted 3 lines
Quoted 1 line
Quoted 1 line · On 20.12.18 at 14:14, <[email protected]> wrote:
Add Hygon Dhyana support to the acpi cpufreq subsystem by using the code path of AMD.
... cpufreq and cpuidle subsystems ...
Thanks for the correction.
Quoted 8 lines
xen/arch/x86/acpi/cpufreq/cpufreq.c int cpufreq_cpu_init(unsigned int cpuid) · line 664
Quoted 6 lines
xen/arch/x86/acpi/cpufreq/cpufreq.c +1
-1@@ -660,8 +661,9 @@ int cpufreq_cpu_init(unsigned int cpuid) {int ret; -/* Currently we only handle Intel and AMD processor */ +/* Currently we only handle Intel and AMD and Hygon processor */ Please replace the first "and" by a comma.
OK.
Quoted 7 lines
xen/arch/x86/acpi/cpufreq/cpufreq.c int cpufreq_cpu_init(unsigned int cpuid) · line 669
Quoted 5 lines
xen/arch/x86/acpi/cpufreq/cpufreq.c +1
-0if ( (boot_cpu_data.x86_vendor ==X86_VENDOR_INTEL ) || + (boot_cpu_data.x86_vendor ==X86_VENDOR_HYGON ) || (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD ) ) ret =cpufreq_add_cpu (cpuid);else At the very least here I think you want to change to
switch().
I think there is no need to change if() to switch(), because if() looks concise enough here. Also the change will generate a bigger patch.
Signature · 7 lines
Regards, Pu Wen _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 07/15] x86/acpi: Add Hygon Dhyana support
Quoted 9 lines
Quoted 1 line
Quoted 1 line · On 15.01.19 at 13:52, <[email protected]> wrote:
Quoted 7 lines · Jan Beulich
Quoted 6 lines
Quoted 1 line
Quoted 1 line · On 20.12.18 at 14:14, <[email protected]> wrote:
if ( (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL ) || + (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON ) || (boot_cpu_data.x86_vendor == X86_VENDOR_AMD ) ) ret = cpufreq_add_cpu(cpuid); else
Quoted 4 lines
Quoted 1 line
At the very least here I think you want to change to
switch().I think there is no need to change
if()toswitch(), becauseif()looks concise enough here. Also the change will generate a bigger patch.
There may not be a strict need, but such recurring comparisons generally read better when coded as switch().
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH 8/15 x86/iommu: Add Hygon Dhyana support
In reply to Pu WenThe IOMMU architecture for the Hygon Dhyana CPU is similar to the AMD family 17h one. So add Hygon Dhyana support to it by sharing the code path of AMD.
--- xen/include/asm-x86/iommu.h | 1 + 1 file changed, 1 insertion(+)
xen/include/asm-x86/iommu.h +1 -0
@@ -73,6 +73,7 @@ static inline int iommu_hardware_setup(void) {case X86_VENDOR_INTEL :return intel_vtd_setup (); +case X86_VENDOR_HYGON :case X86_VENDOR_AMD :return amd_iov_detect (); }
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH 9/15 x86/pv: Add Hygon Dhyana support to emulate MSRs access
In reply to Pu WenThe Hygon Dhyana CPU supports lots of MSRs(such as perf event select and counter MSRs, hardware configuration MSR, MMIO configuration base address MSR, MPERF/APERF MSRs) as AMD CPU does, so add Hygon Dhyana support to the PV emulation infrastructure by using the code path of AMD.
As hygon.c needs to write the load-store configuration(LS_CFG) MSR, so add new case in write_msr to handle this situation.
--- xen/arch/x86/pv/emul-priv-op.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-)
xen/arch/x86/pv/emul-priv-op.c +24 -5
@@ -927,7 +927,9 @@ static int read_msr(unsigned int reg, uint64_t *val,/* fall through */ case MSR_AMD_FAM15H_EVNTSEL0 ... MSR_AMD_FAM15H_PERFCTR5: case MSR_K7_EVNTSEL0 ... MSR_K7_PERFCTR3: -if ( vpmu_msr || (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD ) ) +if ( vpmu_msr || + (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD ) || + (boot_cpu_data.x86_vendor ==X86_VENDOR_HYGON ) ) {if (vpmu_do_rdmsr (reg, val) )break ;
@@ -1009,7 +1011,8 @@ static int write_msr(unsigned int reg, uint64_t val,case MSR_K8_PSTATE6 :case MSR_K8_PSTATE7 :case MSR_K8_HWCR : -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 )break ;if (likely (!is_cpufreq_controller (currd)) ||wrmsr_safe (reg, val) ==0 )
@@ -1029,9 +1032,22 @@ static int write_msr(unsigned int reg, uint64_t val,return X86EMUL_OKAY ;break ; +case MSR_AMD64_LS_CFG : +if ( boot_cpu_data.x86_vendor !=X86_VENDOR_AMD && + boot_cpu_data.x86_vendor !=X86_VENDOR_HYGON ) +break ; +if ( !is_hardware_domain (currd) || !is_pinned_vcpu (curr) ) +return X86EMUL_OKAY ; +if (is_pv_32bit_domain (currd) || !is_canonical_address (val) ) +break ; +if (wrmsr_safe (reg, val) ==0 ) +return X86EMUL_OKAY ; +break ; +case MSR_FAM10H_MMIO_CONF_BASE : -if ( boot_cpu_data.x86_vendor !=X86_VENDOR_AMD || - boot_cpu_data.x86 <0x10 || boot_cpu_data.x86 >0x17 ) +if ( (boot_cpu_data.x86_vendor !=X86_VENDOR_AMD || + boot_cpu_data.x86 <0x10 || boot_cpu_data.x86 >0x17 ) && + boot_cpu_data.x86_vendor !=X86_VENDOR_HYGON )break ;if ( !is_hardware_domain (currd) || !is_pinned_vcpu (curr) )return X86EMUL_OKAY ;
@@ -1070,6 +1086,7 @@ static int write_msr(unsigned int reg, uint64_t val,case MSR_IA32_MPERF :case MSR_IA32_APERF :if ( (boot_cpu_data.x86_vendor !=X86_VENDOR_INTEL ) && + (boot_cpu_data.x86_vendor !=X86_VENDOR_HYGON ) && (boot_cpu_data.x86_vendor !=X86_VENDOR_AMD ) )break ; if ( likely(!is_cpufreq_controller(currd)) ||
@@ -1103,7 +1120,9 @@ static int write_msr(unsigned int reg, uint64_t val, vpmu_msr = true; case MSR_AMD_FAM15H_EVNTSEL0 ... MSR_AMD_FAM15H_PERFCTR5: case MSR_K7_EVNTSEL0 ... MSR_K7_PERFCTR3: -if ( vpmu_msr || (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD ) ) +if ( vpmu_msr || + (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD ) || + (boot_cpu_data.x86_vendor ==X86_VENDOR_HYGON ) ) {if ( (vpmu_mode &XENPMU_MODE_ALL ) && !is_hardware_domain (currd) )
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 09/15] x86/pv: Add Hygon Dhyana support to emulate MSRs access
Quoted 8 lines
Quoted 1 line
Quoted 1 line · On 20.12.18 at 14:14, <[email protected]> wrote:
The Hygon Dhyana CPU supports lots of MSRs(such as perf event select and counter MSRs, hardware configuration MSR, MMIO configuration base address MSR, MPERF/APERF MSRs) as AMD CPU does, so add Hygon Dhyana support to the PV emulation infrastructure by using the code path of AMD.
As
hygon.cneeds to write the load-storeconfiguration(LS_CFG)MSR, so add new case inwrite_msrto handle this situation.
Which hygon.c do you mean here? This addition, if valid at all, clearly needs its own patch and justification, the more that you permit access (even if only for Dom0) on AMD as well.
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 09/15] x86/pv: Add Hygon Dhyana support to emulate MSRs access
Quoted 10 lines · Jan Beulich
Quoted 8 lines
Quoted 1 line
Quoted 1 line · On 20.12.18 at 14:14, <[email protected]> wrote:
The Hygon Dhyana CPU supports lots of MSRs(such as perf event select and counter MSRs, hardware configuration MSR, MMIO configuration base address MSR, MPERF/APERF MSRs) as AMD CPU does, so add Hygon Dhyana support to the PV emulation infrastructure by using the code path of AMD.
As
hygon.cneeds to write the load-storeconfiguration(LS_CFG)MSR, so add new case inwrite_msrto handle this situation.Which
hygon.cdo you mean here? This addition, if valid at all, clearly
I mean hygon.c from the linux kernel. But in fact it's some other kernel part will write LS_CFG MSR. So the description will be refined.
Quoted 1 line
needs its own patch and justification, the more that you permit access
This addition is needed, otherwise there will be warnings like:
"(XEN) emul-priv-op.c:1165:d0v12 Domain attempted WRMSR c0011020 from 0x0206800000000000 to 0x0206800000000400"
Your suggestion is fine, will develop its own patch and justification in next version patch set.
Quoted 1 line
(even if only for Dom0) on AMD as well.
As there will be the same warnings on AMD platforms, so I permit access for AMD as well. I don't know if is this OK? If not, I'll permit access for Hygon only.
Signature · 7 lines
Regards, Pu Wen _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 09/15] x86/pv: Add Hygon Dhyana support to emulate MSRs access
Quoted 13 lines
Quoted 1 line
Quoted 1 line · On 30.01.19 at 10:52, <[email protected]> wrote:
Quoted 10 lines · Jan Beulich
Quoted 8 lines
Quoted 1 line
Quoted 1 line · On 20.12.18 at 14:14, <[email protected]> wrote:
The Hygon Dhyana CPU supports lots of MSRs(such as perf event select and counter MSRs, hardware configuration MSR, MMIO configuration base address MSR, MPERF/APERF MSRs) as AMD CPU does, so add Hygon Dhyana support to the PV emulation infrastructure by using the code path of AMD.
As
hygon.cneeds to write the load-storeconfiguration(LS_CFG)MSR, so add new case inwrite_msrto handle this situation.Which
hygon.cdo you mean here? This addition, if valid at all, clearly
Quoted 8 lines
I mean
hygon.cfrom the linux kernel. But in fact it's some other kernel part will writeLS_CFGMSR. So the description will be refined.Quoted 1 line
needs its own patch and justification, the more that you permit access
This addition is needed, otherwise there will be warnings like:
"(XEN) emul-priv-op.c:1165:d0v12 Domain attempted WRMSRc0011020from0x0206800000000000to0x0206800000000400"
But that's an issue to be taken care of in kernel code, not by making Xen permit the write. There are other MSRs where the kernel similarly tries accesses which it shouldn't try when run in PV mode.
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH 10/15 x86/domain: Add Hygon Dhyana support
In reply to Pu WenAdd Hygon Dhyana support to handle HyperTransport range.
Also loading a nul selector does not clear bases and limits on Hygon CPUs, so add Hygon Dhyana support to the function preload_segment.
--- xen/arch/x86/dom0_build.c | 3 ++- xen/arch/x86/domain.c | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-)
xen/arch/x86/dom0_build.c +2 -1
@@ -533,7 +533,8 @@ int __init dom0_setup_permissions(struct domain *d)paddr_to_pfn (MSI_ADDR_BASE_LO + MSI_ADDR_DEST_ID_MASK));/* HyperTransport range. */ -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 ) rc |=iomem_deny_access (d,paddr_to_pfn (0xfdULL <<32 ),paddr_to_pfn ((1ULL <<40 ) -1 ));
xen/arch/x86/domain.c +5 -4
@@ -1293,13 +1293,14 @@ arch_do_vcpu_op( }/* -* Loading a nul selector does not clear bases and limits on AMD CPUs. Be on -* the safe side and re-initialize both to flat segment values before loading -* a nul selector. +* Loading a nul selector does not clear bases and limits on AMD or Hygon +* CPUs. Be on the safe side and re-initialize both to flat segment values +* before loading a nul selector. */ #define preload_segment (seg, value) do { \ if ( !((value) & ~3) && \ - boot_cpu_data.x86_vendor == X86_VENDOR_AMD ) \ + (boot_cpu_data.x86_vendor == X86_VENDOR_AMD || \ + boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) ) \ asm volatile ( "movl %k0, %%" #seg \ :: "r" (FLAT_USER_DS32) ); \ } while ( false )
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH 11/15 x86/domctl: Add Hygon Dhyana support
In reply to Pu WenAdd Hygon Dhyana support to update cpuid info for creating PV guest.
--- xen/arch/x86/domctl.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
xen/arch/x86/domctl.c +10 -3
@@ -175,6 +175,7 @@ static int update_domain_cpuid_info(struct domain *d, mask |= ((uint64_t )edx <<32 ) | ecx;break ; +case X86_VENDOR_HYGON :case X86_VENDOR_AMD : mask &= ((uint64_t )ecx <<32 ) | edx;
@@ -220,7 +221,8 @@ static int update_domain_cpuid_info(struct domain *d,uint32_t eax = ctl->eax;uint32_t ebx = p->feat._7b0; -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 ) mask &= ((uint64_t )eax <<32 ) | ebx; d->arch.pv.cpuidmasks->_7ab0 = mask;
@@ -281,8 +283,12 @@ static int update_domain_cpuid_info(struct domain *d,if ( cpu_has_cmp_legacy ) ecx |=cpufeat_mask (X86_FEATURE_CMP_LEGACY ); -/* If not emulating AMD, clear the duplicated features in e1d. */ -if ( p->x86_vendor !=X86_VENDOR_AMD ) +/* +* If not emulating AMD and Hygon, clear the duplicated features +* in e1d. +*/ +if ( p->x86_vendor !=X86_VENDOR_AMD && + p->x86_vendor !=X86_VENDOR_HYGON ) edx &= ~CPUID_COMMON_1D_FEATURES ; switch ( boot_cpu_data.x86_vendor )
@@ -291,6 +297,7 @@ static int update_domain_cpuid_info(struct domain *d, mask &= ((uint64_t )edx <<32 ) | ecx;break ; +case X86_VENDOR_HYGON :case X86_VENDOR_AMD : mask &= ((uint64_t )ecx <<32 ) | edx;
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH 12/15 x86/traps: Add Hygon Dhyana support
In reply to Pu WenThe Hygon Dhyana processor has the methold to get the last exception source IP from MSR0000_01DD. So add support for it if the boot param ler is true.
--- xen/arch/x86/traps.c | 2 ++ 1 file changed, 2 insertions(+)
xen/arch/x86/traps.c +2 -0
@@ -1973,6 +1973,8 @@ static unsigned int calc_ler_msr(void)return MSR_IA32_LASTINTFROMIP ; }break ; +case X86_VENDOR_HYGON : +return MSR_IA32_LASTINTFROMIP ; }return 0 ;
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 12/15] x86/traps: Add Hygon Dhyana support
xen/arch/x86/traps.c static unsigned int calc_ler_msr(void) · line 1977
Quoted 9 lines
Quoted 1 line
Quoted 1 line
Pu Wen <[email protected]> 12/20/18 2:16 PM >>>
xen/arch/x86/traps.c +2
-0@@ -1973,6 +1973,8 @@ static unsigned int calc_ler_msr(void)return MSR_IA32_LASTINTFROMIP ; }break ; +case X86_VENDOR_HYGON : +return MSR_IA32_LASTINTFROMIP ;
With a blank line added above your addition
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 12/15] x86/traps: Add Hygon Dhyana support
Quoted 11 lines · Jan Beulich
xen/arch/x86/traps.c static unsigned int calc_ler_msr(void) · line 1977
Quoted 9 lines
Quoted 1 line
Quoted 1 line
Pu Wen <[email protected]> 12/20/18 2:16 PM >>>
xen/arch/x86/traps.c +2
-0@@ -1973,6 +1973,8 @@ static unsigned int calc_ler_msr(void)return MSR_IA32_LASTINTFROMIP ; }break ; +case X86_VENDOR_HYGON : +return MSR_IA32_LASTINTFROMIP ;With a blank line added above your addition
OK. Thanks for the suggestion.
Quoted 1 line
Acked-by Jan Beulich <[email protected]>
Signature · 7 lines
Regards, Pu Wen _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH 13/15 x86/xstate: Add Hygon Dhyana support
In reply to Pu WenThe Hygon Dhyana CPU don't save/restore FDP/FIP/FOP unless an exception is pending. So add support for it in the function xrstor.
--- xen/arch/x86/xstate.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
xen/arch/x86/xstate.c +3 -2
@@ -369,7 +369,7 @@ void xrstor(struct vcpu *v, uint64_t mask)unsigned int faults, prev_faults;/* -* AMD CPUs don't save/restore FDP/FIP/FOP unless an exception +* AMD or Hygon CPUs don't save/restore FDP/FIP/FOP unless an exception * is pending. Clear the x87 state here by setting it to fixed * values. The hypervisor data segment can be sometimes 0 and * sometimes new user value. Both should be ok. Use the FPU saved
@@ -377,7 +377,8 @@ void xrstor(struct vcpu *v, uint64_t mask)*/ if ( (mask & ptr->xsave_hdr.xstate_bv &X86_XCR0_FP ) && !(ptr->fpu_sse.fsw & ~ptr->fpu_sse.fcw &0x003f ) && - boot_cpu_data.x86_vendor ==X86_VENDOR_AMD ) + (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD || + boot_cpu_data.x86_vendor ==X86_VENDOR_HYGON ) ) asmvolatile ("fnclex\n\t" /* clear exceptions */ "ffree %%st(7)\n\t" /* clear stack tag */ "fildl %0" /* load to clear state */
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 13/15] x86/xstate: Add Hygon Dhyana support
Quoted 2 lines · Pu Wen
The Hygon Dhyana CPU don't save/restore FDP/FIP/FOP unless an exception is pending. So add support for it in the function xrstor.
Really?
Zen was the first AMD processor to fix this (mis)feature, and the Xen code doesn't appear to have caught up yet.
I'm putting together a series trying to fix this, and several other similar issues.
~Andrew
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 13/15] x86/xstate: Add Hygon Dhyana support
Quoted 11 lines · Andrew Cooper
Quoted 2 lines · Pu Wen
The Hygon Dhyana CPU don't save/restore FDP/FIP/FOP unless an exception is pending. So add support for it in the function xrstor.
Really?
Zen was the first AMD processor to fix this (mis)feature, and the Xen code doesn't appear to have caught up yet.
I'm putting together a series trying to fix this, and several other similar issues.
There is indeed a feature bit in the CPU spec to indicate that this (mis)feature no longer exists. Also I read the value of the feature bit on Hygon Dhyana platform and found out that it's 1.
So this patch is no needed and will be removed in next version.
Thx.
Signature · 7 lines
Regards, Pu Wen _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 13/15] x86/xstate: Add Hygon Dhyana support
In reply to Pu WenQuoted 19 lines
Quoted 1 line
Quoted 1 line
Pu Wen <[email protected]> 12/20/18 2:16 PM >>>
xen/arch/x86/xstate.c +1
-1@@ -369,7 +369,7 @@ void xrstor(struct vcpu *v, uint64_t mask)unsigned int faults, prev_faults;/* -* AMD CPUs don't save/restore FDP/FIP/FOP unless an exception +* AMD or Hygon CPUs don't save/restore FDP/FIP/FOP unless an exception * is pending. Clear the x87 state here by setting it to fixed * values. The hypervisor data segment can be sometimes 0 and * sometimes new user value. Both should be ok. Use the FPU saved @@ -377,7 +377,8 @@ void xrstor(struct vcpu *v, uint64_t mask)*/ if ( (mask &
ptr->xsave_hdr.xstate_bv&X86_XCR0_FP) && !(ptr->fpu_sse.fsw & ~ptr->fpu_sse.fcw &0x003f) &&xen/arch/x86/xstate.c +2
-1- boot_cpu_data.x86_vendor == X86_VENDOR_AMD ) + (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD || + boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) )
Quoted 3 lines
asm volatile ( "fnclex\n\t" /* clear exceptions */
"ffree %%st(7)\n\t" /* clear stack tag */
"fildl %0" /* load to clear state */
A similar change then is needed to fpu_fxrstor() as well, in case people disable use of XSAVE via command line option. And then there was also a recent change to this area by Andrew, which may affect you as well. (Sorry, I don't have a pointer at hand.)
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 13/15] x86/xstate: Add Hygon Dhyana support
Quoted 17 lines · Jan Beulich
Quoted 17 lines
Quoted 1 line
Quoted 1 line
Pu Wen <[email protected]> 12/20/18 2:16 PM >>>
xen/arch/x86/xstate.c +1
-1@@ -369,7 +369,7 @@ void xrstor(struct vcpu *v, uint64_t mask)unsigned int faults, prev_faults;/* -* AMD CPUs don't save/restore FDP/FIP/FOP unless an exception +* AMD or Hygon CPUs don't save/restore FDP/FIP/FOP unless an exception * is pending. Clear the x87 state here by setting it to fixed * values. The hypervisor data segment can be sometimes 0 and * sometimes new user value. Both should be ok. Use the FPU saved @@ -377,7 +377,8 @@ void xrstor(struct vcpu *v, uint64_t mask)*/ if ( (mask &
ptr->xsave_hdr.xstate_bv&X86_XCR0_FP) && !(ptr->fpu_sse.fsw & ~ptr->fpu_sse.fcw &0x003f) &&
- boot_cpu_data.x86_vendor ==X86_VENDOR_AMD)
Quoted 8 lines
Quoted 5 lines
xen/arch/x86/xstate.c +2
-0+ (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD || + boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) )asm volatile ( "fnclex\n\t" /* clear exceptions */
"ffree %%st(7)\n\t" /* clear stack tag */
"fildl %0" /* load to clear state */A similar change then is needed to
fpu_fxrstor()as well, in case people disable use of XSAVE via command line option. And then there was also
The same change to fpu_fxrstor() is fine.
Quoted 2 lines
a recent change to this area by Andrew, which may affect you as well. (Sorry, I don't have a pointer at hand.)
If Andrew's change is accepted upstream, then these changes for Hygon are no needed any more.
Right now I wonder how to develop the following version Hygon patch set. Following the current change logic or waiting for Andrew's changes settled down?
Andrew,
How about your patch series "x86: Improvements to handling of various CPU bugs"?
Signature · 7 lines
Regards, Pu Wen _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH 14/15 x86/cpuid: Add Hygon Dhyana support
In reply to Pu WenThe Hygon Dhyana family 18h processor shares the same cpuid leaves as the AMD family 17h one. So add Hygon Dhyana support to caculate the cpuid policies as the AMD CPU does.
--- xen/arch/x86/cpuid.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
xen/arch/x86/cpuid.c +7 -3
@@ -238,6 +238,7 @@ static void recalculate_misc(struct cpuid_policy *p) p->extd.raw[0x8 ].c =0 ;break ; +case X86_VENDOR_HYGON :case X86_VENDOR_AMD :zero_leaves (p->basic.raw,0x2 ,0x3 );memset (p->cache.raw,0 ,sizeof (p->cache.raw));
@@ -389,7 +390,8 @@ static void __init calculate_hvm_max_policy(void)* long mode (and init_amd() has cleared it out of host capabilities), but * HVM guests are able if running in protected mode. */ -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 ) && raw_cpuid_policy.basic.sep )__set_bit (X86_FEATURE_SEP , hvm_featureset);
@@ -464,7 +466,8 @@ void recalculate_cpuid_policy(struct domain *d) p->basic.max_leaf =min (p->basic.max_leaf, max->basic.max_leaf); p->feat.max_subleaf =min (p->feat.max_subleaf, max->feat.max_subleaf); p->extd.max_leaf =0x80000000 |min (p->extd.max_leaf &0xffff , - (p->x86_vendor ==X86_VENDOR_AMD + ((p->x86_vendor ==X86_VENDOR_AMD || + p->x86_vendor ==X86_VENDOR_HYGON ) ?CPUID_GUEST_NR_EXTD_AMD :CPUID_GUEST_NR_EXTD_INTEL ) -1 );
@@ -506,7 +509,8 @@ void recalculate_cpuid_policy(struct domain *d)if (is_pv_32bit_domain (d) ) {__clear_bit (X86_FEATURE_LM , max_fs); -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 )__clear_bit (X86_FEATURE_SYSCALL , max_fs); }
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH 15/15 tools/libxc: Add Hygon Dhyana support
In reply to Pu WenAdd Hygon Dhyana support to caculate the cpuid policies for creating PV or HVM guest by using the code path of AMD.
--- tools/libxc/xc_cpuid_x86.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
tools/libxc/xc_cpuid_x86.c +12 -4
@@ -234,6 +234,7 @@ struct cpuid_domain_infoVENDOR_UNKNOWN ,VENDOR_INTEL ,VENDOR_AMD , +VENDOR_HYGON , } vendor;bool hvm;
@@ -304,6 +305,10 @@ static int get_cpuid_domain_info(xc_interface *xch, uint32_t domid, regs[2 ] ==0x444d4163U && regs[3] == 0x69746e65U ) info->vendor = VENDOR_AMD; + else if ( regs[1] == 0x6f677948U && /* "HygonGenuine" */ + regs[2] == 0x656e6975U && + regs[3] == 0x6e65476eU ) + info->vendor =VENDOR_HYGON ; else info->vendor = VENDOR_UNKNOWN;
@@ -568,7 +573,8 @@ static void xc_cpuid_hvm_policy(const struct cpuid_domain_info *info,break ; } -if ( info->vendor ==VENDOR_AMD ) +if ( info->vendor ==VENDOR_AMD || + info->vendor ==VENDOR_HYGON )amd_xc_cpuid_policy (info, input, regs);else intel_xc_cpuid_policy (info, input, regs);
@@ -630,7 +636,8 @@ static void xc_cpuid_pv_policy(const struct cpuid_domain_info *info,case 0x80000000 : { -unsigned int max = info->vendor ==VENDOR_AMD +unsigned int max = (info->vendor ==VENDOR_AMD || + info->vendor ==VENDOR_HYGON ) ?DEF_MAX_AMDEXT :DEF_MAX_INTELEXT ;if ( regs[0 ] > max )
@@ -736,7 +743,8 @@ static void sanitise_featureset(struct cpuid_domain_info *info)if ( !info->pv64 ) {clear_bit (X86_FEATURE_LM , info->featureset); -if ( info->vendor !=VENDOR_AMD ) +if ( info->vendor !=VENDOR_AMD && + info->vendor !=VENDOR_HYGON )clear_bit (X86_FEATURE_SYSCALL , info->featureset); }
@@ -787,7 +795,7 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, input[0 ] =0x80000000 ;cpuid (input, regs); -if ( info.vendor ==VENDOR_AMD ) +if ( info.vendor ==VENDOR_AMD || info.vendor ==VENDOR_HYGON ) ext_max = (regs[0 ] <=DEF_MAX_AMDEXT ) ? regs[0 ] :DEF_MAX_AMDEXT ;else ext_max = (regs[0 ] <=DEF_MAX_INTELEXT ) ? regs[0 ] :DEF_MAX_INTELEXT ;
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH 15/15] tools/libxc: Add Hygon Dhyana support
Quoted 4 lines · Pu Wen
Add Hygon Dhyana support to caculate the cpuid policies for creating PV or HVM guest by using the code path of AMD.
Signed-off-by Pu Wen <[email protected]>
Piggy-backing on AMD code is fine at this stage:
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel