[PATCH v2 00/14] Add support for Hygon Dhyana Family 18h processor

[PATCH v2 00/14] Add support for Hygon Dhyana Family 18h processor

[Pu Wen]

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.


v1->v2:
  - Rebased on 4.20.0-rc3 and tested against it.
  - Move opt_cpuid_mask_l7s0_(eax/ebx) to common.c.
  - Insert Hygon cases after AMD ones instead of above.
  - Remove (rd/wr)msr_hygon_safe and use (rd/wr)msr_safe instead.
  - Remove wrmsr_hygon and use wrmsrl instead.
  - Remove the unnecessary change to xstate.
  - Refine some codes and comments.
  - Add Acked-by from Jan Beulich for x86/traps.
  - Add Acked-by from Wei Liu for tools/libxc.


Pu Wen (14):
  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 and AMD Zen 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/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    |   8 +-
 xen/arch/x86/acpi/cpufreq/powernow.c   |   3 +-
 xen/arch/x86/apic.c                    |   5 +
 xen/arch/x86/cpu/Makefile              |   1 +
 xen/arch/x86/cpu/amd.c                 |   5 -
 xen/arch/x86/cpu/common.c              |   9 +-
 xen/arch/x86/cpu/cpu.h                 |   2 +
 xen/arch/x86/cpu/hygon.c               | 248 +++++++++++++++++++++++++++++++++
 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      |   5 +-
 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         |  19 ++-
 xen/arch/x86/spec_ctrl.c               |   6 +-
 xen/arch/x86/traps.c                   |   3 +
 xen/include/asm-x86/iommu.h            |   1 +
 xen/include/asm-x86/x86-vendors.h      |   3 +-
 27 files changed, 354 insertions(+), 43 deletions(-)
 create mode 100644 xen/arch/x86/cpu/hygon.c

-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 01/14] x86/cpu: Create Hygon Dhyana architecture support file

[Pu Wen]

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.

As opt_cpuid_mask_l7s0_eax and opt_cpuid_mask_l7s0_ebx are used by both
AMD and Hygon, so move them to common.c.

Signed-off-by: Pu Wen <puwen@hygon.cn>
---
 xen/arch/x86/cpu/Makefile         |   1 +
 xen/arch/x86/cpu/amd.c            |   5 -
 xen/arch/x86/cpu/common.c         |   6 +
 xen/arch/x86/cpu/cpu.h            |   2 +
 xen/arch/x86/cpu/hygon.c          | 248 ++++++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/x86-vendors.h |   3 +-
 6 files changed, 259 insertions(+), 6 deletions(-)
 create mode 100644 xen/arch/x86/cpu/hygon.c

diff --git a/xen/arch/x86/cpu/Makefile b/xen/arch/x86/cpu/Makefile
index 34a01ca..1db7d88 100644
--- a/xen/arch/x86/cpu/Makefile
+++ b/xen/arch/x86/cpu/Makefile
@@ -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
diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index c790416..4c595cf 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -32,11 +32,6 @@
 static char __initdata opt_famrev[14];
 string_param("cpuid_mask_cpu", opt_famrev);
 
-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);
-
 static unsigned int __initdata opt_cpuid_mask_thermal_ecx = ~0u;
 integer_param("cpuid_mask_thermal_ecx", opt_cpuid_mask_thermal_ecx);
 
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index de6c5c9..5bab845 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -36,6 +36,11 @@ integer_param("cpuid_mask_ext_ecx", opt_cpuid_mask_ext_ecx);
 unsigned int opt_cpuid_mask_ext_edx = ~0u;
 integer_param("cpuid_mask_ext_edx", opt_cpuid_mask_ext_edx);
 
+unsigned int  opt_cpuid_mask_l7s0_eax = ~0u;
+integer_param("cpuid_mask_l7s0_eax", opt_cpuid_mask_l7s0_eax);
+unsigned int  opt_cpuid_mask_l7s0_ebx = ~0u;
+integer_param("cpuid_mask_l7s0_ebx", opt_cpuid_mask_l7s0_ebx);
+
 unsigned int __initdata expected_levelling_cap;
 unsigned int __read_mostly levelling_caps;
 
@@ -704,6 +709,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();
diff --git a/xen/arch/x86/cpu/cpu.h b/xen/arch/x86/cpu/cpu.h
index 2fcb931..9ea53e5 100644
--- a/xen/arch/x86/cpu/cpu.h
+++ b/xen/arch/x86/cpu/cpu.h
@@ -13,11 +13,13 @@ extern bool_t opt_arat;
 extern unsigned int opt_cpuid_mask_ecx, opt_cpuid_mask_edx;
 extern unsigned int opt_cpuid_mask_xsave_eax;
 extern unsigned int opt_cpuid_mask_ext_ecx, opt_cpuid_mask_ext_edx;
+extern unsigned int opt_cpuid_mask_l7s0_eax, opt_cpuid_mask_l7s0_ebx;
 
 extern int get_model_name(struct cpuinfo_x86 *c);
 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);
diff --git a/xen/arch/x86/cpu/hygon.c b/xen/arch/x86/cpu/hygon.c
new file mode 100644
index 0000000..3e79441
--- /dev/null
+++ b/xen/arch/x86/cpu/hygon.c
@@ -0,0 +1,248 @@
+#include <xen/init.h>
+#include <asm/processor.h>
+#include <asm/hvm/support.h>
+#include <asm/spec_ctrl.h>
+
+#include "cpu.h"
+
+/*
+ * 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)
+{
+	uint64_t value;
+
+	expected_levelling_cap |= caps;
+
+	if ((rdmsr_safe(msr, value) == 0) && (wrmsr_safe(msr, value) == 0))
+		levelling_caps |= caps;
+
+	return value;
+}
+
+/* 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)) {
+			wrmsrl(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)) {							\
+			wrmsrl(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);
+
+	/* 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);
+	}
+}
+
+static const struct cpu_dev hygon_cpu_dev = {
+	.c_vendor	= "Hygon",
+	.c_ident 	= { "HygonGenuine" },
+	.c_early_init	= early_init_hygon,
+	.c_init		= init_hygon,
+};
+
+int __init hygon_init_cpu(void)
+{
+	cpu_devs[X86_VENDOR_HYGON] = &hygon_cpu_dev;
+	return 0;
+}
diff --git a/xen/include/asm-x86/x86-vendors.h b/xen/include/asm-x86/x86-vendors.h
index 38a81c3..fa1cbb4 100644
--- a/xen/include/asm-x86/x86-vendors.h
+++ b/xen/include/asm-x86/x86-vendors.h
@@ -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__ */
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 02/14] x86/cpu/mtrr: Add Hygon Dhyana support to get TOP_MEM2

[Pu Wen]

The Hygon Dhyana CPU supports the MSR way to get TOP_MEM2. So add Hygon
Dhyana support to print the value of TOP_MEM2.

Signed-off-by: Pu Wen <puwen@hygon.cn>
---
 xen/arch/x86/cpu/mtrr/generic.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/cpu/mtrr/generic.c b/xen/arch/x86/cpu/mtrr/generic.c
index 8f9cf1b..94ee7d6 100644
--- a/xen/arch/x86/cpu/mtrr/generic.c
+++ b/xen/arch/x86/cpu/mtrr/generic.c
@@ -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);
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 03/14] x86/cpu/vpmu: Add Hygon Dhyana and AMD Zen support for vPMU

[Pu Wen]

The PMU architecture for the Hygon Dhyana CPU is similar to the AMD
family 17h one. So add Hygon Dhyana support in vpmu_arch_initialise
and vpmu_init by using the code path of AMD.

Since current Xen vPMU still not support Zen(0x17), so add both 0x17
and 0x18 support by using the 0x15's case in amd_vpmu_init, for 0x17
and 0x18 have the same performance event select and counter MSRs as
0x15 has.

Signed-off-by: Pu Wen <puwen@hygon.cn>
---
 xen/arch/x86/cpu/vpmu.c     | 2 ++
 xen/arch/x86/cpu/vpmu_amd.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 13da7d0..f679e79 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -459,6 +459,7 @@ static int vpmu_arch_initialise(struct vcpu *v)
     switch ( vendor )
     {
     case X86_VENDOR_AMD:
+    case X86_VENDOR_HYGON:
         ret = svm_vpmu_initialise(v);
         break;
 
@@ -876,6 +877,7 @@ static int __init vpmu_init(void)
     switch ( vendor )
     {
     case X86_VENDOR_AMD:
+    case X86_VENDOR_HYGON:
         if ( amd_vpmu_init() )
            vpmu_mode = XENPMU_MODE_OFF;
         break;
diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c
index 5efc39b..c9abe6e 100644
--- a/xen/arch/x86/cpu/vpmu_amd.c
+++ b/xen/arch/x86/cpu/vpmu_amd.c
@@ -545,6 +545,8 @@ int __init amd_vpmu_init(void)
     switch ( current_cpu_data.x86 )
     {
     case 0x15:
+    case 0x17:
+    case 0x18:
         num_counters = F15H_NUM_COUNTERS;
         counters = AMD_F15H_COUNTERS;
         ctrls = AMD_F15H_CTRLS;
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 04/14] x86/cpu/mce: Add Hygon Dhyana support to the MCA infrastructure

[Pu Wen]

The 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.

Signed-off-by: Pu Wen <puwen@hygon.cn>
---
 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      | 5 ++++-
 xen/arch/x86/cpu/mcheck/non-fatal.c    | 3 ++-
 xen/arch/x86/cpu/mcheck/vmce.c         | 2 ++
 6 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 5bab845..214bdb5 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -350,7 +350,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);
diff --git a/xen/arch/x86/cpu/mcheck/amd_nonfatal.c b/xen/arch/x86/cpu/mcheck/amd_nonfatal.c
index 222f539..589dac5 100644
--- a/xen/arch/x86/cpu/mcheck/amd_nonfatal.c
+++ b/xen/arch/x86/cpu/mcheck/amd_nonfatal.c
@@ -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. */
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 30cdb06..0798dea 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -778,6 +778,7 @@ void mcheck_init(struct cpuinfo_x86 *c, bool bsp)
     switch ( c->x86_vendor )
     {
     case X86_VENDOR_AMD:
+    case X86_VENDOR_HYGON:
         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:
diff --git a/xen/arch/x86/cpu/mcheck/mce_amd.c b/xen/arch/x86/cpu/mcheck/mce_amd.c
index d125bc1..040e449 100644
--- a/xen/arch/x86/cpu/mcheck/mce_amd.c
+++ b/xen/arch/x86/cpu/mcheck/mce_amd.c
@@ -274,7 +274,10 @@ enum mcheck_type
 amd_mcheck_init(struct cpuinfo_x86 *ci)
 {
     uint32_t i;
-    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);
 
     /* Assume that machine check support is available.
      * The minimum provided support is at least the K8. */
diff --git a/xen/arch/x86/cpu/mcheck/non-fatal.c b/xen/arch/x86/cpu/mcheck/non-fatal.c
index d12e8f2..77be418 100644
--- a/xen/arch/x86/cpu/mcheck/non-fatal.c
+++ b/xen/arch/x86/cpu/mcheck/non-fatal.c
@@ -101,7 +101,8 @@ static int __init init_nonfatal_mce_checker(void)
 	 */
 	switch (c->x86_vendor) {
 	case X86_VENDOR_AMD:
-		/* Assume we are on K8 or newer AMD CPU here */
+	case X86_VENDOR_HYGON:
+		/* Assume we are on K8 or newer AMD or Hygon CPU here */
 		amd_nonfatal_mcheck_init(c);
 		break;
 
diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index f15835e..4f5de07 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -154,6 +154,7 @@ static int bank_mce_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
             break;
 
         case X86_VENDOR_AMD:
+        case X86_VENDOR_HYGON:
             ret = vmce_amd_rdmsr(v, msr, val);
             break;
 
@@ -284,6 +285,7 @@ static int bank_mce_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
             break;
 
         case X86_VENDOR_AMD:
+        case X86_VENDOR_HYGON:
             ret = vmce_amd_wrmsr(v, msr, val);
             break;
 
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 05/14] x86/spec_ctrl: Add Hygon Dhyana to the respective mitigation machinery

[Pu Wen]

The Hygon Dhyana CPU has the same speculative execution as AMD family
17h, so share AMD Retpoline and PTI mitigation code with Hygon Dhyana.

Signed-off-by: Pu Wen <puwen@hygon.cn>
---
 xen/arch/x86/spec_ctrl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index ad72ecd..38a6ed2 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -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 )
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 06/14] x86/apic: Add Hygon Dhyana support

[Pu Wen]

Add Hygon Dhyana support to use modern APIC.

Signed-off-by: Pu Wen <puwen@hygon.cn>
---
 xen/arch/x86/apic.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 2a24326..004d685 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -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;
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 07/14] x86/acpi: Add Hygon Dhyana support

[Pu Wen]

Add Hygon Dhyana support to the acpi cpufreq and cpuidle subsystems by
using the code path of AMD.

Signed-off-by: Pu Wen <puwen@hygon.cn>
---
 xen/arch/x86/acpi/cpu_idle.c         | 3 ++-
 xen/arch/x86/acpi/cpufreq/cpufreq.c  | 8 +++++---
 xen/arch/x86/acpi/cpufreq/powernow.c | 3 ++-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
index 14b0278..f57825c 100644
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -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 */
diff --git a/xen/arch/x86/acpi/cpufreq/cpufreq.c b/xen/arch/x86/acpi/cpufreq/cpufreq.c
index 844ab85..14c18bd 100644
--- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
+++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
@@ -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,9 +661,10 @@ int cpufreq_cpu_init(unsigned int cpuid)
 {
     int ret;
 
-    /* Currently we only handle Intel and AMD processor */
+    /* Currently we only handle Intel, AMD and Hygon processor */
     if ( (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL ) ||
-         (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 = cpufreq_add_cpu(cpuid);
     else
         ret = -EFAULT;
diff --git a/xen/arch/x86/acpi/cpufreq/powernow.c b/xen/arch/x86/acpi/cpufreq/powernow.c
index 025b37d..f245908 100644
--- a/xen/arch/x86/acpi/cpufreq/powernow.c
+++ b/xen/arch/x86/acpi/cpufreq/powernow.c
@@ -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
         {
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 08/14] x86/iommu: Add Hygon Dhyana support

[Pu Wen]

The 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.

Signed-off-by: Pu Wen <puwen@hygon.cn>
---
 xen/include/asm-x86/iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/include/asm-x86/iommu.h b/xen/include/asm-x86/iommu.h
index 8dc3924..699a8f7 100644
--- a/xen/include/asm-x86/iommu.h
+++ b/xen/include/asm-x86/iommu.h
@@ -74,6 +74,7 @@ static inline int iommu_hardware_setup(void)
     case X86_VENDOR_INTEL:
         return intel_vtd_setup();
     case X86_VENDOR_AMD:
+    case X86_VENDOR_HYGON:
         return amd_iov_detect();
     }
 
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 09/14] x86/pv: Add Hygon Dhyana support to emulate MSRs access

[Pu Wen]

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.

Signed-off-by: Pu Wen <puwen@hygon.cn>
---
 xen/arch/x86/pv/emul-priv-op.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 942ece2..1521041 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -911,7 +911,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;
@@ -993,7 +995,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 )
@@ -1014,8 +1017,9 @@ static int write_msr(unsigned int reg, uint64_t val,
         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;
@@ -1054,7 +1058,8 @@ 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_AMD) )
+             (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 )
@@ -1087,7 +1092,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) )
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 10/14] x86/domain: Add Hygon Dhyana support

[Pu Wen]

Add 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.

Signed-off-by: Pu Wen <puwen@hygon.cn>
---
 xen/arch/x86/dom0_build.c | 3 ++-
 xen/arch/x86/domain.c     | 9 +++++----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 2b4d9e9..c0657ee 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -532,7 +532,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));
 
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 32dc4253..319eea1 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1300,13 +1300,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 )
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 11/14] x86/domctl: Add Hygon Dhyana support

[Pu Wen]

Add Hygon Dhyana support to update cpuid info for creating PV guest.

Signed-off-by: Pu Wen <puwen@hygon.cn>
---
 xen/arch/x86/domctl.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 9bf2d08..3a9e290 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -176,6 +176,7 @@ static int update_domain_cpuid_info(struct domain *d,
                 break;
 
             case X86_VENDOR_AMD:
+            case X86_VENDOR_HYGON:
                 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 )
@@ -292,6 +298,7 @@ static int update_domain_cpuid_info(struct domain *d,
                 break;
 
             case X86_VENDOR_AMD:
+            case X86_VENDOR_HYGON:
                 mask &= ((uint64_t)ecx << 32) | edx;
 
                 /*
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 12/14] x86/traps: Add Hygon Dhyana support

[Pu Wen]

The 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.

Signed-off-by: Pu Wen <puwen@hygon.cn>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/traps.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 05ddc39..97bf9e2 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1973,6 +1973,9 @@ static unsigned int calc_ler_msr(void)
             return MSR_IA32_LASTINTFROMIP;
         }
         break;
+
+    case X86_VENDOR_HYGON:
+        return MSR_IA32_LASTINTFROMIP;
     }
 
     return 0;
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 13/14] x86/cpuid: Add Hygon Dhyana support

[Pu Wen]

The 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.

Signed-off-by: Pu Wen <puwen@hygon.cn>
---
 xen/arch/x86/cpuid.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index ab0aab6..f760594 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -240,6 +240,7 @@ static void recalculate_misc(struct cpuid_policy *p)
         break;
 
     case X86_VENDOR_AMD:
+    case X86_VENDOR_HYGON:
         zero_leaves(p->basic.raw, 0x2, 0x3);
         memset(p->cache.raw, 0, sizeof(p->cache.raw));
         zero_leaves(p->basic.raw, 0x9, 0xa);
@@ -390,7 +391,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);
 
@@ -465,7 +467,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);
 
@@ -507,7 +510,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);
     }
 
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 14/14] tools/libxc: Add Hygon Dhyana support

[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 <puwen@hygon.cn>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxc/xc_cpuid_x86.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 098affe..d0cb9ae 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -234,6 +234,7 @@ struct cpuid_domain_info
         VENDOR_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;
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 00/14] Add support for Hygon Dhyana Family 18h processor

[Wei Liu]

I think the version should have been v5?

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 00/14] Add support for Hygon Dhyana Family 18h processor

[Pu Wen]

On 2019/2/22 0:38, Wei Liu wrote:
> I think the version should have been v5?

Aha. This is the second revision of the patch series. So why should it
have been v5?

-- 
Regards,
Pu Wen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 00/14] Add support for Hygon Dhyana Family 18h processor

[Wei Liu]

On Fri, Feb 22, 2019 at 10:30:17AM +0800, Pu Wen wrote:
> On 2019/2/22 0:38, Wei Liu wrote:
> > I think the version should have been v5?
> 
> Aha. This is the second revision of the patch series. So why should it
> have been v5?

I have seen several previous postings from last year. But I just
realised they were for Linux, not Xen.

Sorry for the noise.

Wei.

> 
> -- 
> Regards,
> Pu Wen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 01/14] x86/cpu: Create Hygon Dhyana architecture support file

[Jan Beulich]

>>> On 21.02.19 at 10:48, <puwen@hygon.cn> wrote:
> 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.
> 
> As opt_cpuid_mask_l7s0_eax and opt_cpuid_mask_l7s0_ebx are used by both
> AMD and Hygon, so move them to common.c.

I'm not convinced of this - it'll get in the way of introducing something
like Linux'es CONFIG_CPU_SUP_*. Our command line parsing logic
handles multiple instances of an option quite fine I think, so I'd prefer
if these were kept static to both files. Or did Andrew ask you to do
this?

> --- a/xen/arch/x86/cpu/Makefile
> +++ b/xen/arch/x86/cpu/Makefile
> @@ -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

Please insert at the alphabetically correct place.

> --- a/xen/arch/x86/cpu/cpu.h
> +++ b/xen/arch/x86/cpu/cpu.h
> @@ -13,11 +13,13 @@ extern bool_t opt_arat;
>  extern unsigned int opt_cpuid_mask_ecx, opt_cpuid_mask_edx;
>  extern unsigned int opt_cpuid_mask_xsave_eax;
>  extern unsigned int opt_cpuid_mask_ext_ecx, opt_cpuid_mask_ext_edx;
> +extern unsigned int opt_cpuid_mask_l7s0_eax, opt_cpuid_mask_l7s0_ebx;
>  
>  extern int get_model_name(struct cpuinfo_x86 *c);
>  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);

Whereas here you should really put the new declaration at the end.

> --- /dev/null
> +++ b/xen/arch/x86/cpu/hygon.c
> @@ -0,0 +1,248 @@
> +#include <xen/init.h>
> +#include <asm/processor.h>
> +#include <asm/hvm/support.h>
> +#include <asm/spec_ctrl.h>
> +
> +#include "cpu.h"
> +
> +/*
> + * 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)
> +{
> +	uint64_t value;
> +
> +	expected_levelling_cap |= caps;
> +
> +	if ((rdmsr_safe(msr, value) == 0) && (wrmsr_safe(msr, value) == 0))
> +		levelling_caps |= caps;
> +
> +	return value;
> +}
> +
> +/* Probe for the existance of the expected masking MSRs. */

Please don't drop the other part of the original comment.

> +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);

There's more relevant code here in the original function.

> +}
> +
> +/*
> + * 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)) {
> +			wrmsrl(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)) {							\
> +			wrmsrl(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;
> +}

This is a lot of duplicated code with only minor differences. I think
you would be better off calling into the AMD original functions.

> +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);

Didn't you cut off too much from the original code (which again
may better be shared, by splitting out into a function)?

> +	/*
> +	 * 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);
> +
> +	/* Hygon processors have APIC timer running in deep C states. */
> +	if ( opt_arat )

Please don't copy the bad spaces inside the parentheses here.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 01/14] x86/cpu: Create Hygon Dhyana architecture support file

[Pu Wen]

On 2019/3/15 1:11, Jan Beulich wrote:
>>>> On 21.02.19 at 10:48, <puwen@hygon.cn> wrote:
>> As opt_cpuid_mask_l7s0_eax and opt_cpuid_mask_l7s0_ebx are used by both
>> AMD and Hygon, so move them to common.c.
> 
> I'm not convinced of this - it'll get in the way of introducing something
> like Linux'es CONFIG_CPU_SUP_*. Our command line parsing logic
> handles multiple instances of an option quite fine I think, so I'd prefer
> if these were kept static to both files. Or did Andrew ask you to do
> this?

Yes, Andrew asked that these should be moved from the AMD specific code into
the common cpu code (alongside the other masks) rather than duplicated here.

>> --- a/xen/arch/x86/cpu/Makefile
>> +++ b/xen/arch/x86/cpu/Makefile
>> @@ -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
> 
> Please insert at the alphabetically correct place.

OK.

...
>>   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);
> 
> Whereas here you should really put the new declaration at the end.

OK.

...
>> +/* Probe for the existance of the expected masking MSRs. */
> 
> Please don't drop the other part of the original comment.

OK. Will revert to the original one.

>> +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);
> 
> There's more relevant code here in the original function.

The code is used for family 15h. Hygon CPU do not need it.

...
>> +	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;
>> +}
> 
> This is a lot of duplicated code with only minor differences. I think
> you would be better off calling into the AMD original functions.

These functions and AMD original ones are static. So Hygon cannot directly
call into them. Or we can put them into the common cpu code, but I think
it's not good for future maintenance. There are many codes to support AMD's
different families, the family/model/stepping checking are deeply embedded
in codes. If we add Hygon family/model/stepping checking in future version,
it will make code interleaved together and hard to maintain.

In fact in Linux we keep hygon.c separated from AMD even though there are
some duplicated codes at the moment. Andrew also agreed the logic that
following suit with Linux.

>> +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);
> 
> Didn't you cut off too much from the original code (which again
> may better be shared, by splitting out into a function)?

The cut codes are used for other AMD families except family 17h. Hygon
Dhyana is derived from AMD Zen, so we only need the family 17h parts.
This also make the init flow minimal.

>> +	/* Hygon processors have APIC timer running in deep C states. */
>> +	if ( opt_arat )
> 
> Please don't copy the bad spaces inside the parentheses here.

OK. Thanks for the reminding.

-- 
Regards,
Pu Wen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 01/14] x86/cpu: Create Hygon Dhyana architecture support file

[Jan Beulich]

>>> On 15.03.19 at 11:17, <puwen@hygon.cn> wrote:
> On 2019/3/15 1:11, Jan Beulich wrote:
>>>>> On 21.02.19 at 10:48, <puwen@hygon.cn> wrote:
>>> +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);
>> 
>> There's more relevant code here in the original function.
> 
> The code is used for family 15h. Hygon CPU do not need it.

There's a single Fam15 conditional in the middle of the function.
Everything beyond that is again family-independent.

>>> +	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;
>>> +}
>> 
>> This is a lot of duplicated code with only minor differences. I think
>> you would be better off calling into the AMD original functions.
> 
> These functions and AMD original ones are static. So Hygon cannot directly
> call into them. Or we can put them into the common cpu code, but I think
> it's not good for future maintenance.

Just make non-static what needs to be, add an amd_ prefix, and
call it from your code.

> There are many codes to support AMD's
> different families, the family/model/stepping checking are deeply embedded
> in codes. If we add Hygon family/model/stepping checking in future version,
> it will make code interleaved together and hard to maintain.

We can think about adding the duplication when it becomes
unwieldy to maintain the shared variant.

>>> +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);
>> 
>> Didn't you cut off too much from the original code (which again
>> may better be shared, by splitting out into a function)?
> 
> The cut codes are used for other AMD families except family 17h. Hygon
> Dhyana is derived from AMD Zen, so we only need the family 17h parts.
> This also make the init flow minimal.

How is

		else if (wrmsr_safe(MSR_AMD64_DE_CFG,
				    value | AMD64_DE_CFG_LFENCE_SERIALISE) ||
			 rdmsr_safe(MSR_AMD64_DE_CFG, value) ||
			 !(value & AMD64_DE_CFG_LFENCE_SERIALISE))
			/* Attempt to set failed.  Assume the safer default. */
			__clear_bit(X86_FEATURE_LFENCE_DISPATCH,
				    c->x86_capability);
		else
			/* Successfully enabled! */
			__set_bit(X86_FEATURE_LFENCE_DISPATCH,
				  c->x86_capability);

family dependent in any way?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 02/14] x86/cpu/mtrr: Add Hygon Dhyana support to get TOP_MEM2

[Jan Beulich]

>>> On 21.02.19 at 10:48, <puwen@hygon.cn> wrote:
> The Hygon Dhyana CPU supports the MSR way to get TOP_MEM2. So add Hygon
> Dhyana support to print the value of TOP_MEM2.
> 
> Signed-off-by: Pu Wen <puwen@hygon.cn>

Acked-by: Jan Beulich <jbeulich@suse.com>

I'm afraid I won't be able to offer Reviewed-by tags for any of
this series without you pointing us at the documentation for the
processor (a link should really be in the cover letter).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 03/14] x86/cpu/vpmu: Add Hygon Dhyana and AMD Zen support for vPMU

[Jan Beulich]

>>> On 21.02.19 at 10:50, <puwen@hygon.cn> wrote:
> --- a/xen/arch/x86/cpu/vpmu_amd.c
> +++ b/xen/arch/x86/cpu/vpmu_amd.c
> @@ -545,6 +545,8 @@ int __init amd_vpmu_init(void)
>      switch ( current_cpu_data.x86 )
>      {
>      case 0x15:
> +    case 0x17:
> +    case 0x18:
>          num_counters = F15H_NUM_COUNTERS;
>          counters = AMD_F15H_COUNTERS;
>          ctrls = AMD_F15H_CTRLS;

Unless you know what AMD Fam18 will look like, you can't do it
like this. Fam18 really needs to be further qualified by a vendor
check at this point in time.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 04/14] x86/cpu/mce: Add Hygon Dhyana support to the MCA infrastructure

[Jan Beulich]

>>> On 21.02.19 at 10:51, <puwen@hygon.cn> wrote:
> The 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.
> 
> Signed-off-by: Pu Wen <puwen@hygon.cn>

Acked-by: Jan Beulich <jbeulich@suse.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 05/14] x86/spec_ctrl: Add Hygon Dhyana to the respective mitigation machinery

[Jan Beulich]

>>> On 21.02.19 at 10:51, <puwen@hygon.cn> wrote:
> The Hygon Dhyana CPU has the same speculative execution as AMD family
> 17h, so share AMD Retpoline and PTI mitigation code with Hygon Dhyana.
> 
> Signed-off-by: Pu Wen <puwen@hygon.cn>

Acked-by: Jan Beulich <jbeulich@suse.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 06/14] x86/apic: Add Hygon Dhyana support

[Jan Beulich]

>>> On 21.02.19 at 10:51, <puwen@hygon.cn> wrote:
> Add Hygon Dhyana support to use modern APIC.
> 
> Signed-off-by: Pu Wen <puwen@hygon.cn>

Acked-by: Jan Beulich <jbeulich@suse.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 07/14] x86/acpi: Add Hygon Dhyana support

[Jan Beulich]

>>> On 21.02.19 at 10:52, <puwen@hygon.cn> wrote:
> Add Hygon Dhyana support to the acpi cpufreq and cpuidle subsystems by
> using the code path of AMD.
> 
> Signed-off-by: Pu Wen <puwen@hygon.cn>

Acked-by: Jan Beulich <jbeulich@suse.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 08/14] x86/iommu: Add Hygon Dhyana support

[Jan Beulich]

>>> On 21.02.19 at 10:52, <puwen@hygon.cn> wrote:
> The 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.
> 
> Signed-off-by: Pu Wen <puwen@hygon.cn>

Acked-by: Jan Beulich <jbeulich@suse.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 09/14] x86/pv: Add Hygon Dhyana support to emulate MSRs access

[Jan Beulich]

>>> On 21.02.19 at 10:52, <puwen@hygon.cn> 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.
> 
> Signed-off-by: Pu Wen <puwen@hygon.cn>

Acked-by: Jan Beulich <jbeulich@suse.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 10/14] x86/domain: Add Hygon Dhyana support

[Jan Beulich]

>>> On 21.02.19 at 10:53, <puwen@hygon.cn> wrote:
> Add 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.
> 
> Signed-off-by: Pu Wen <puwen@hygon.cn>

Acked-by: Jan Beulich <jbeulich@suse.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 11/14] x86/domctl: Add Hygon Dhyana support

[Jan Beulich]

>>> On 21.02.19 at 10:53, <puwen@hygon.cn> wrote:
> Add Hygon Dhyana support to update cpuid info for creating PV guest.
> 
> Signed-off-by: Pu Wen <puwen@hygon.cn>

Acked-by: Jan Beulich <jbeulich@suse.com>
with one remark:

> @@ -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.

I think this wants to be "or" instead of "and".

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 13/14] x86/cpuid: Add Hygon Dhyana support

[Jan Beulich]

>>> On 21.02.19 at 10:54, <puwen@hygon.cn> wrote:
> The 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.
> 
> Signed-off-by: Pu Wen <puwen@hygon.cn>

Acked-by: Jan Beulich <jbeulich@suse.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 00/14] Add support for Hygon Dhyana Family 18h processor

[Jan Beulich]

>>> On 21.02.19 at 10:48, <puwen@hygon.cn> wrote:
> Pu Wen (14):
>   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 and AMD Zen 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/cpuid: Add Hygon Dhyana support
>   tools/libxc: Add Hygon Dhyana support

In addition to these changes, what about pci_cfg_ok()? I don't think
I've seen any of the patches touch it.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 01/14] x86/cpu: Create Hygon Dhyana architecture support file

[Pu Wen]

On 2019/3/15 19:18, Jan Beulich wrote:
>>>> On 15.03.19 at 11:17, <puwen@hygon.cn> wrote:
>> On 2019/3/15 1:11, Jan Beulich wrote:
>>>>>> On 21.02.19 at 10:48, <puwen@hygon.cn> wrote:
>>>> +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);
>>>
>>> There's more relevant code here in the original function.
>>
>> The code is used for family 15h. Hygon CPU do not need it.
> 
> There's a single Fam15 conditional in the middle of the function.
> Everything beyond that is again family-independent.
> 
>>>> +	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;
>>>> +}
>>>
>>> This is a lot of duplicated code with only minor differences. I think
>>> you would be better off calling into the AMD original functions.
>>
>> These functions and AMD original ones are static. So Hygon cannot directly
>> call into them. Or we can put them into the common cpu code, but I think
>> it's not good for future maintenance.
> 
> Just make non-static what needs to be, add an amd_ prefix, and
> call it from your code.

That's OK. With this method only init_levelling in amd.c should be added
an amd_ prefix and called by hygon.c.

But I'm afraid Hygon should have its own init functions and not call the
AMD ones. The current Hygon init functions have been heavily stripped
from the original AMD's.

>> There are many codes to support AMD's
>> different families, the family/model/stepping checking are deeply embedded
>> in codes. If we add Hygon family/model/stepping checking in future version,
>> it will make code interleaved together and hard to maintain.
> 
> We can think about adding the duplication when it becomes
> unwieldy to maintain the shared variant.
> 
>>>> +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);
>>>
>>> Didn't you cut off too much from the original code (which again
>>> may better be shared, by splitting out into a function)?
>>
>> The cut codes are used for other AMD families except family 17h. Hygon
>> Dhyana is derived from AMD Zen, so we only need the family 17h parts.
>> This also make the init flow minimal.
> 
> How is
> 
> 		else if (wrmsr_safe(MSR_AMD64_DE_CFG,
> 				    value | AMD64_DE_CFG_LFENCE_SERIALISE) ||
> 			 rdmsr_safe(MSR_AMD64_DE_CFG, value) ||
> 			 !(value & AMD64_DE_CFG_LFENCE_SERIALISE))
> 			/* Attempt to set failed.  Assume the safer default. */
> 			__clear_bit(X86_FEATURE_LFENCE_DISPATCH,
> 				    c->x86_capability);
> 		else
> 			/* Successfully enabled! */
> 			__set_bit(X86_FEATURE_LFENCE_DISPATCH,
> 				  c->x86_capability);
> 
> family dependent in any way?

Independent here.

-- 
Regards,
Pu Wen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 02/14] x86/cpu/mtrr: Add Hygon Dhyana support to get TOP_MEM2

[Pu Wen]

On 2019/3/15 20:40, Jan Beulich wrote:
>>>> On 21.02.19 at 10:48, <puwen@hygon.cn> wrote:
>> The Hygon Dhyana CPU supports the MSR way to get TOP_MEM2. So add Hygon
>> Dhyana support to print the value of TOP_MEM2.
>>
>> Signed-off-by: Pu Wen <puwen@hygon.cn>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> I'm afraid I won't be able to offer Reviewed-by tags for any of
> this series without you pointing us at the documentation for the
> processor (a link should really be in the cover letter).

Do you mean the specification for Hygon processor? We don't have any
public spec now, but you can reference to the AMD Zen one[1].

Reference:
[1] https://www.amd.com/system/files/TechDocs/54945_PPR_Family_17h_Models_00h-0Fh.pdf 

-- 
Regards,
Pu Wen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 03/14] x86/cpu/vpmu: Add Hygon Dhyana and AMD Zen support for vPMU

[Pu Wen]

On 2019/3/15 20:41, Jan Beulich wrote:
>>>> On 21.02.19 at 10:50, <puwen@hygon.cn> wrote:
>> --- a/xen/arch/x86/cpu/vpmu_amd.c
>> +++ b/xen/arch/x86/cpu/vpmu_amd.c
>> @@ -545,6 +545,8 @@ int __init amd_vpmu_init(void)
>>       switch ( current_cpu_data.x86 )
>>       {
>>       case 0x15:
>> +    case 0x17:
>> +    case 0x18:
>>           num_counters = F15H_NUM_COUNTERS;
>>           counters = AMD_F15H_COUNTERS;
>>           ctrls = AMD_F15H_CTRLS;
> 
> Unless you know what AMD Fam18 will look like, you can't do it
> like this. Fam18 really needs to be further qualified by a vendor
> check at this point in time.

Hygon will negotiate with AMD to make sure that only Hygon should use
Fam18h.

-- 
Regards,
Pu Wen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 11/14] x86/domctl: Add Hygon Dhyana support

[Pu Wen]

On 2019/3/15 21:29, Jan Beulich wrote:
>>>> On 21.02.19 at 10:53, <puwen@hygon.cn> wrote:
>> Add Hygon Dhyana support to update cpuid info for creating PV guest.
>>
>> Signed-off-by: Pu Wen <puwen@hygon.cn>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> with one remark:
> 
>> @@ -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.
> 
> I think this wants to be "or" instead of "and".

Yes, will change to "or".

-- 
Regards,
Pu Wen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 00/14] Add support for Hygon Dhyana Family 18h processor

[Pu Wen]

On 2019/3/15 21:41, Jan Beulich wrote:
>>>> On 21.02.19 at 10:48, <puwen@hygon.cn> wrote:
>> Pu Wen (14):
>>    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 and AMD Zen 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/cpuid: Add Hygon Dhyana support
>>    tools/libxc: Add Hygon Dhyana support
> 
> In addition to these changes, what about pci_cfg_ok()? I don't think
> I've seen any of the patches touch it.

Because Hygon does not access the extended NB configuration space right
now, so there is no change to pci_cfg_ok().

-- 
Regards,
Pu Wen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 01/14] x86/cpu: Create Hygon Dhyana architecture support file

[Jan Beulich]

>>> On 16.03.19 at 10:57, <puwen@hygon.cn> wrote:
> On 2019/3/15 19:18, Jan Beulich wrote:
>>>>> On 15.03.19 at 11:17, <puwen@hygon.cn> wrote:
>>> On 2019/3/15 1:11, Jan Beulich wrote:
>>>>>>> On 21.02.19 at 10:48, <puwen@hygon.cn> wrote:
>>>>> +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);
>>>>
>>>> There's more relevant code here in the original function.
>>>
>>> The code is used for family 15h. Hygon CPU do not need it.
>> 
>> There's a single Fam15 conditional in the middle of the function.
>> Everything beyond that is again family-independent.
>> 
>>>>> +	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;
>>>>> +}
>>>>
>>>> This is a lot of duplicated code with only minor differences. I think
>>>> you would be better off calling into the AMD original functions.
>>>
>>> These functions and AMD original ones are static. So Hygon cannot directly
>>> call into them. Or we can put them into the common cpu code, but I think
>>> it's not good for future maintenance.
>> 
>> Just make non-static what needs to be, add an amd_ prefix, and
>> call it from your code.
> 
> That's OK. With this method only init_levelling in amd.c should be added
> an amd_ prefix and called by hygon.c.
> 
> But I'm afraid Hygon should have its own init functions and not call the
> AMD ones. The current Hygon init functions have been heavily stripped
> from the original AMD's.

Let me give you this rule of thumb (subject to discussion): If you can
safely re-use any non-trivial current AMD function with at most minor
adjustments (and irrespective of certain code there being unreachable
on Hygon), then I think it would be better to re-use it than to duplicate
it.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 02/14] x86/cpu/mtrr: Add Hygon Dhyana support to get TOP_MEM2

[Jan Beulich]

>>> On 16.03.19 at 11:06, <puwen@hygon.cn> wrote:
> On 2019/3/15 20:40, Jan Beulich wrote:
>>>>> On 21.02.19 at 10:48, <puwen@hygon.cn> wrote:
>>> The Hygon Dhyana CPU supports the MSR way to get TOP_MEM2. So add Hygon
>>> Dhyana support to print the value of TOP_MEM2.
>>>
>>> Signed-off-by: Pu Wen <puwen@hygon.cn>
>> 
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>> 
>> I'm afraid I won't be able to offer Reviewed-by tags for any of
>> this series without you pointing us at the documentation for the
>> processor (a link should really be in the cover letter).
> 
> Do you mean the specification for Hygon processor? We don't have any
> public spec now, but you can reference to the AMD Zen one[1].

Well, no, not really. There are undoubtedly differences (see your
reply to my pci_cfg_ok() question as an example). I can ack
changes on the basis that you know how your processors behave.
I can give R-b only if I have Hygon-specific doc to check against.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 03/14] x86/cpu/vpmu: Add Hygon Dhyana and AMD Zen support for vPMU

[Jan Beulich]

>>> On 16.03.19 at 11:11, <puwen@hygon.cn> wrote:
> On 2019/3/15 20:41, Jan Beulich wrote:
>>>>> On 21.02.19 at 10:50, <puwen@hygon.cn> wrote:
>>> --- a/xen/arch/x86/cpu/vpmu_amd.c
>>> +++ b/xen/arch/x86/cpu/vpmu_amd.c
>>> @@ -545,6 +545,8 @@ int __init amd_vpmu_init(void)
>>>       switch ( current_cpu_data.x86 )
>>>       {
>>>       case 0x15:
>>> +    case 0x17:
>>> +    case 0x18:
>>>           num_counters = F15H_NUM_COUNTERS;
>>>           counters = AMD_F15H_COUNTERS;
>>>           ctrls = AMD_F15H_CTRLS;
>> 
>> Unless you know what AMD Fam18 will look like, you can't do it
>> like this. Fam18 really needs to be further qualified by a vendor
>> check at this point in time.
> 
> Hygon will negotiate with AMD to make sure that only Hygon should use
> Fam18h.

In the success case of which please state this in the description.
Until those negotiations have succeeded I'm afraid I'm going to
insist to see the extra check added.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 02/14] x86/cpu/mtrr: Add Hygon Dhyana support to get TOP_MEM2

[Pu Wen]

On 2019/3/18 16:57, Jan Beulich wrote:
>>>> On 16.03.19 at 11:06, <puwen@hygon.cn> wrote:
>> On 2019/3/15 20:40, Jan Beulich wrote:
>>>>>> On 21.02.19 at 10:48, <puwen@hygon.cn> wrote:
>>>> The Hygon Dhyana CPU supports the MSR way to get TOP_MEM2. So add Hygon
>>>> Dhyana support to print the value of TOP_MEM2.
>>>>
>>>> Signed-off-by: Pu Wen <puwen@hygon.cn>
>>>
>>> Acked-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> I'm afraid I won't be able to offer Reviewed-by tags for any of
>>> this series without you pointing us at the documentation for the
>>> processor (a link should really be in the cover letter).
>>
>> Do you mean the specification for Hygon processor? We don't have any
>> public spec now, but you can reference to the AMD Zen one[1].
> 
> Well, no, not really. There are undoubtedly differences (see your
> reply to my pci_cfg_ok() question as an example). I can ack
> changes on the basis that you know how your processors behave.
> I can give R-b only if I have Hygon-specific doc to check against.

In fact there is no definition of MSR C001_001F(MSR_AMD64_NB_CFG) in the
AMD family 17h models 00h-0Fh PPR, I don't know why there is 0x17 support
in pci_cfg_ok().

Besides, the other MSRs and CPUIDs which are used by Hygon in this patch
series are all defined in this PPR, and the meanings are fully the same.
For example in current patch, bit 21 of MSR C001_0010(MSR_K8_TOP_MEM2)
in print_mtrr_state().

-- 
Regards,
Pu Wen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 02/14] x86/cpu/mtrr: Add Hygon Dhyana support to get TOP_MEM2

[Jan Beulich]

>>> On 18.03.19 at 16:51, <puwen@hygon.cn> wrote:
> On 2019/3/18 16:57, Jan Beulich wrote:
>>>>> On 16.03.19 at 11:06, <puwen@hygon.cn> wrote:
>>> On 2019/3/15 20:40, Jan Beulich wrote:
>>>>>>> On 21.02.19 at 10:48, <puwen@hygon.cn> wrote:
>>>>> The Hygon Dhyana CPU supports the MSR way to get TOP_MEM2. So add Hygon
>>>>> Dhyana support to print the value of TOP_MEM2.
>>>>>
>>>>> Signed-off-by: Pu Wen <puwen@hygon.cn>
>>>>
>>>> Acked-by: Jan Beulich <jbeulich@suse.com>
>>>>
>>>> I'm afraid I won't be able to offer Reviewed-by tags for any of
>>>> this series without you pointing us at the documentation for the
>>>> processor (a link should really be in the cover letter).
>>>
>>> Do you mean the specification for Hygon processor? We don't have any
>>> public spec now, but you can reference to the AMD Zen one[1].
>> 
>> Well, no, not really. There are undoubtedly differences (see your
>> reply to my pci_cfg_ok() question as an example). I can ack
>> changes on the basis that you know how your processors behave.
>> I can give R-b only if I have Hygon-specific doc to check against.
> 
> In fact there is no definition of MSR C001_001F(MSR_AMD64_NB_CFG) in the
> AMD family 17h models 00h-0Fh PPR, I don't know why there is 0x17 support
> in pci_cfg_ok().

Hmm, interesting observation. The extended access mechanism appears
to still exist, but is controlled by a different PCI config space bit now,
and there doesn't seem to be any mirroring MSR anymore. Brian, could
you confirm this? If there's indeed no NB_CFG MSR anymore, we may
need to adjust some of our code elsewhere as well; the control logic of
extended accesses looks to need adjustment in any event.

The use of 0x17 looks to date back to a 2010 commit (3157bb4e13),
where decimal 17 was converted to 0x17, without there being any
description to clarify whether this wasn't actually a mistake.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 03/14] x86/cpu/vpmu: Add Hygon Dhyana and AMD Zen support for vPMU

[Pu Wen]

On 2019/3/18 16:59, Jan Beulich wrote:
>>>> On 16.03.19 at 11:11, <puwen@hygon.cn> wrote:
>> On 2019/3/15 20:41, Jan Beulich wrote:
>>>>>> On 21.02.19 at 10:50, <puwen@hygon.cn> wrote:
>>>> --- a/xen/arch/x86/cpu/vpmu_amd.c
>>>> +++ b/xen/arch/x86/cpu/vpmu_amd.c
>>>> @@ -545,6 +545,8 @@ int __init amd_vpmu_init(void)
>>>>        switch ( current_cpu_data.x86 )
>>>>        {
>>>>        case 0x15:
>>>> +    case 0x17:
>>>> +    case 0x18:
>>>>            num_counters = F15H_NUM_COUNTERS;
>>>>            counters = AMD_F15H_COUNTERS;
>>>>            ctrls = AMD_F15H_CTRLS;
>>>
>>> Unless you know what AMD Fam18 will look like, you can't do it
>>> like this. Fam18 really needs to be further qualified by a vendor
>>> check at this point in time.
>>
>> Hygon will negotiate with AMD to make sure that only Hygon should use
>> Fam18h.
> 
> In the success case of which please state this in the description.
> Until those negotiations have succeeded I'm afraid I'm going to
> insist to see the extra check added.

How to check vendor? Maybe like this:
     case 0x15:
     case 0x17:
     case 0x18:
         if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD &&
             boot_cpu_data.x86 == 0x18)
             return -EINVAL;

         num_counters = F15H_NUM_COUNTERS;
         counters = AMD_F15H_COUNTERS;
         ctrls = AMD_F15H_CTRLS;

or just add Hygon support at beginning of amd_vpmu_init():
     if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) {
         num_counters = F15H_NUM_COUNTERS;
         counters = AMD_F15H_COUNTERS;
         ctrls = AMD_F15H_CTRLS;
         k7_counters_mirrored = 1;
     }

-- 
Regards,
Pu Wen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 01/14] x86/cpu: Create Hygon Dhyana architecture support file

[Pu Wen]

On 2019/3/18 16:55, Jan Beulich wrote:
> On 16.03.19 at 10:57, <puwen@hygon.cn> wrote:
>> On 2019/3/15 19:18, Jan Beulich wrote:
>>> On 15.03.19 at 11:17, <puwen@hygon.cn> wrote:
>>>> On 2019/3/15 1:11, Jan Beulich wrote:
>>>>> This is a lot of duplicated code with only minor differences. I think
>>>>> you would be better off calling into the AMD original functions.
>>>> These functions and AMD original ones are static. So Hygon cannot directly
>>>> call into them. Or we can put them into the common cpu code, but I think
>>>> it's not good for future maintenance.
>>> Just make non-static what needs to be, add an amd_ prefix, and
>>> call it from your code.
>> That's OK. With this method only init_levelling in amd.c should be added
>> an amd_ prefix and called by hygon.c.
>> But I'm afraid Hygon should have its own init functions and not call the
>> AMD ones. The current Hygon init functions have been heavily stripped
>> from the original AMD's.
> Let me give you this rule of thumb (subject to discussion): If you can
> safely re-use any non-trivial current AMD function with at most minor
> adjustments (and irrespective of certain code there being unreachable
> on Hygon), then I think it would be better to re-use it than to duplicate
> it.

I tried, but it will add 0x18 to init_amd(). It's strange because AMD
does not have family 18h now. So it becomes unwieldy to maintain
init_amd() just at this time. The same situation for amd_get_topology().

So I think it's better to carve out init_hygon() and hygon_get_topology()
(which also need Hygon's own adjustment for computing phys_proc_id).

I think it's time to develop a new patch series for your review.

-- 
Regards,
Pu Wen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 03/14] x86/cpu/vpmu: Add Hygon Dhyana and AMD Zen support for vPMU

[Jan Beulich]

>>> On 19.03.19 at 12:32, <puwen@hygon.cn> wrote:
> On 2019/3/18 16:59, Jan Beulich wrote:
>>>>> On 16.03.19 at 11:11, <puwen@hygon.cn> wrote:
>>> On 2019/3/15 20:41, Jan Beulich wrote:
>>>>>>> On 21.02.19 at 10:50, <puwen@hygon.cn> wrote:
>>>>> --- a/xen/arch/x86/cpu/vpmu_amd.c
>>>>> +++ b/xen/arch/x86/cpu/vpmu_amd.c
>>>>> @@ -545,6 +545,8 @@ int __init amd_vpmu_init(void)
>>>>>        switch ( current_cpu_data.x86 )
>>>>>        {
>>>>>        case 0x15:
>>>>> +    case 0x17:
>>>>> +    case 0x18:
>>>>>            num_counters = F15H_NUM_COUNTERS;
>>>>>            counters = AMD_F15H_COUNTERS;
>>>>>            ctrls = AMD_F15H_CTRLS;
>>>>
>>>> Unless you know what AMD Fam18 will look like, you can't do it
>>>> like this. Fam18 really needs to be further qualified by a vendor
>>>> check at this point in time.
>>>
>>> Hygon will negotiate with AMD to make sure that only Hygon should use
>>> Fam18h.
>> 
>> In the success case of which please state this in the description.
>> Until those negotiations have succeeded I'm afraid I'm going to
>> insist to see the extra check added.
> 
> How to check vendor? Maybe like this:
>      case 0x15:
>      case 0x17:
>      case 0x18:
>          if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD &&
>              boot_cpu_data.x86 == 0x18)
>              return -EINVAL;
> 
>          num_counters = F15H_NUM_COUNTERS;
>          counters = AMD_F15H_COUNTERS;
>          ctrls = AMD_F15H_CTRLS;
> 
> or just add Hygon support at beginning of amd_vpmu_init():
>      if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) {
>          num_counters = F15H_NUM_COUNTERS;
>          counters = AMD_F15H_COUNTERS;
>          ctrls = AMD_F15H_CTRLS;
>          k7_counters_mirrored = 1;
>      }

A suitable variant of the latter or

int __init amd_vpmu_init(void)
{
    unsigned int i, fam = current_cpu_data.x86

    /* <suitable comment> */
    if ( current_cpu_data.x86_vendor == X86_VENDOR_HYGON && fam == 0x18 )
       fam = 0x17;

    switch ( fam )
    ...

or perhaps even better would be two separate switch()-es, one for
AMD and one for Hygon. Possibly even a separate hygon_vpmu_init().

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 01/14] x86/cpu: Create Hygon Dhyana architecture support file

[Jan Beulich]

>>> On 19.03.19 at 13:33, <puwen@hygon.cn> wrote:
> On 2019/3/18 16:55, Jan Beulich wrote:
>> On 16.03.19 at 10:57, <puwen@hygon.cn> wrote:
>>> On 2019/3/15 19:18, Jan Beulich wrote:
>>>> On 15.03.19 at 11:17, <puwen@hygon.cn> wrote:
>>>>> On 2019/3/15 1:11, Jan Beulich wrote:
>>>>>> This is a lot of duplicated code with only minor differences. I think
>>>>>> you would be better off calling into the AMD original functions.
>>>>> These functions and AMD original ones are static. So Hygon cannot directly
>>>>> call into them. Or we can put them into the common cpu code, but I think
>>>>> it's not good for future maintenance.
>>>> Just make non-static what needs to be, add an amd_ prefix, and
>>>> call it from your code.
>>> That's OK. With this method only init_levelling in amd.c should be added
>>> an amd_ prefix and called by hygon.c.
>>> But I'm afraid Hygon should have its own init functions and not call the
>>> AMD ones. The current Hygon init functions have been heavily stripped
>>> from the original AMD's.
>> Let me give you this rule of thumb (subject to discussion): If you can
>> safely re-use any non-trivial current AMD function with at most minor
>> adjustments (and irrespective of certain code there being unreachable
>> on Hygon), then I think it would be better to re-use it than to duplicate
>> it.
> 
> I tried, but it will add 0x18 to init_amd(). It's strange because AMD
> does not have family 18h now. So it becomes unwieldy to maintain
> init_amd() just at this time. The same situation for amd_get_topology().
> 
> So I think it's better to carve out init_hygon() and hygon_get_topology()
> (which also need Hygon's own adjustment for computing phys_proc_id).

Just to be clear: I've never objected to a separate init_hygon().
For amd_get_topology() it's less clear: It looks to me as if you
wouldn't need to add any Hygon conditionals to the function, as
long as cpu_has(c, X86_FEATURE_TOPOEXT) is true in your case.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 01/14] x86/cpu: Create Hygon Dhyana architecture support file

[Pu Wen]

On 2019/3/19 21:02, Jan Beulich wrote:
> On 19.03.19 at 13:33, <puwen@hygon.cn> wrote:
>> On 2019/3/18 16:55, Jan Beulich wrote:
>>> On 16.03.19 at 10:57, <puwen@hygon.cn> wrote:
>>>> On 2019/3/15 19:18, Jan Beulich wrote:
>>>>> On 15.03.19 at 11:17, <puwen@hygon.cn> wrote:
>>>>>> On 2019/3/15 1:11, Jan Beulich wrote:
>>>>>>> This is a lot of duplicated code with only minor differences. I think
>>>>>>> you would be better off calling into the AMD original functions.
>>>>>> These functions and AMD original ones are static. So Hygon cannot directly
>>>>>> call into them. Or we can put them into the common cpu code, but I think
>>>>>> it's not good for future maintenance.
>>>>> Just make non-static what needs to be, add an amd_ prefix, and
>>>>> call it from your code.
>>>> That's OK. With this method only init_levelling in amd.c should be added
>>>> an amd_ prefix and called by hygon.c.
>>>> But I'm afraid Hygon should have its own init functions and not call the
>>>> AMD ones. The current Hygon init functions have been heavily stripped
>>>> from the original AMD's.
>>> Let me give you this rule of thumb (subject to discussion): If you can
>>> safely re-use any non-trivial current AMD function with at most minor
>>> adjustments (and irrespective of certain code there being unreachable
>>> on Hygon), then I think it would be better to re-use it than to duplicate
>>> it.
>>
>> I tried, but it will add 0x18 to init_amd(). It's strange because AMD
>> does not have family 18h now. So it becomes unwieldy to maintain
>> init_amd() just at this time. The same situation for amd_get_topology().
>>
>> So I think it's better to carve out init_hygon() and hygon_get_topology()
>> (which also need Hygon's own adjustment for computing phys_proc_id).
> 
> Just to be clear: I've never objected to a separate init_hygon().
> For amd_get_topology() it's less clear: It looks to me as if you
> wouldn't need to add any Hygon conditionals to the function, as
> long as cpu_has(c, X86_FEATURE_TOPOEXT) is true in your case.
 
Yes, it's no need to add Hygon conditional to amd_get_topology() as >=0x17
conditionals are default supported.

-- 
Regards,
Pu Wen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 03/14] x86/cpu/vpmu: Add Hygon Dhyana and AMD Zen support for vPMU

[Pu Wen]

On 2019/3/19 20:58, Jan Beulich wrote:
> On 19.03.19 at 12:32, <puwen@hygon.cn> wrote:
>> On 2019/3/18 16:59, Jan Beulich wrote:
>>> On 16.03.19 at 11:11, <puwen@hygon.cn> wrote:
>>>> On 2019/3/15 20:41, Jan Beulich wrote:
>>>>> On 21.02.19 at 10:50, <puwen@hygon.cn> wrote:
>>>>>> --- a/xen/arch/x86/cpu/vpmu_amd.c
>>>>>> +++ b/xen/arch/x86/cpu/vpmu_amd.c
>>>>>> @@ -545,6 +545,8 @@ int __init amd_vpmu_init(void)
>>>>>>         switch ( current_cpu_data.x86 )
>>>>>>         {
>>>>>>         case 0x15:
>>>>>> +    case 0x17:
>>>>>> +    case 0x18:
>>>>>>             num_counters = F15H_NUM_COUNTERS;
>>>>>>             counters = AMD_F15H_COUNTERS;
>>>>>>             ctrls = AMD_F15H_CTRLS;
>>>>>
>>>>> Unless you know what AMD Fam18 will look like, you can't do it
>>>>> like this. Fam18 really needs to be further qualified by a vendor
>>>>> check at this point in time.
>>>>
>>>> Hygon will negotiate with AMD to make sure that only Hygon should use
>>>> Fam18h.
>>>
>>> In the success case of which please state this in the description.
>>> Until those negotiations have succeeded I'm afraid I'm going to
>>> insist to see the extra check added.
>>
>> How to check vendor? Maybe like this:
>>       case 0x15:
>>       case 0x17:
>>       case 0x18:
>>           if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD &&
>>               boot_cpu_data.x86 == 0x18)
>>               return -EINVAL;
>>
>>           num_counters = F15H_NUM_COUNTERS;
>>           counters = AMD_F15H_COUNTERS;
>>           ctrls = AMD_F15H_CTRLS;
>>
>> or just add Hygon support at beginning of amd_vpmu_init():
>>       if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) {
>>           num_counters = F15H_NUM_COUNTERS;
>>           counters = AMD_F15H_COUNTERS;
>>           ctrls = AMD_F15H_CTRLS;
>>           k7_counters_mirrored = 1;
>>       }
> 
> A suitable variant of the latter or
> 
> int __init amd_vpmu_init(void)
> {
>      unsigned int i, fam = current_cpu_data.x86
> 
>      /* <suitable comment> */
>      if ( current_cpu_data.x86_vendor == X86_VENDOR_HYGON && fam == 0x18 )
>         fam = 0x17;

This is the minimum change, I think it's better.

> 
>      switch ( fam )
>      ...
> 
> or perhaps even better would be two separate switch()-es, one for
> AMD and one for Hygon. Possibly even a separate hygon_vpmu_init().

A separate hygon_vpmu_init() is also fine except that the last part of
the function can be shared.

-- 
Regards,
Pu Wen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 03/14] x86/cpu/vpmu: Add Hygon Dhyana and AMD Zen support for vPMU

[Jan Beulich]

>>> On 19.03.19 at 14:47, <puwen@hygon.cn> wrote:
> On 2019/3/19 20:58, Jan Beulich wrote:
>> On 19.03.19 at 12:32, <puwen@hygon.cn> wrote:
>>> On 2019/3/18 16:59, Jan Beulich wrote:
>>>> On 16.03.19 at 11:11, <puwen@hygon.cn> wrote:
>>>>> On 2019/3/15 20:41, Jan Beulich wrote:
>>>>>> On 21.02.19 at 10:50, <puwen@hygon.cn> wrote:
>>>>>>> --- a/xen/arch/x86/cpu/vpmu_amd.c
>>>>>>> +++ b/xen/arch/x86/cpu/vpmu_amd.c
>>>>>>> @@ -545,6 +545,8 @@ int __init amd_vpmu_init(void)
>>>>>>>         switch ( current_cpu_data.x86 )
>>>>>>>         {
>>>>>>>         case 0x15:
>>>>>>> +    case 0x17:
>>>>>>> +    case 0x18:
>>>>>>>             num_counters = F15H_NUM_COUNTERS;
>>>>>>>             counters = AMD_F15H_COUNTERS;
>>>>>>>             ctrls = AMD_F15H_CTRLS;
>>>>>>
>>>>>> Unless you know what AMD Fam18 will look like, you can't do it
>>>>>> like this. Fam18 really needs to be further qualified by a vendor
>>>>>> check at this point in time.
>>>>>
>>>>> Hygon will negotiate with AMD to make sure that only Hygon should use
>>>>> Fam18h.
>>>>
>>>> In the success case of which please state this in the description.
>>>> Until those negotiations have succeeded I'm afraid I'm going to
>>>> insist to see the extra check added.
>>>
>>> How to check vendor? Maybe like this:
>>>       case 0x15:
>>>       case 0x17:
>>>       case 0x18:
>>>           if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD &&
>>>               boot_cpu_data.x86 == 0x18)
>>>               return -EINVAL;
>>>
>>>           num_counters = F15H_NUM_COUNTERS;
>>>           counters = AMD_F15H_COUNTERS;
>>>           ctrls = AMD_F15H_CTRLS;
>>>
>>> or just add Hygon support at beginning of amd_vpmu_init():
>>>       if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) {
>>>           num_counters = F15H_NUM_COUNTERS;
>>>           counters = AMD_F15H_COUNTERS;
>>>           ctrls = AMD_F15H_CTRLS;
>>>           k7_counters_mirrored = 1;
>>>       }
>> 
>> A suitable variant of the latter or
>> 
>> int __init amd_vpmu_init(void)
>> {
>>      unsigned int i, fam = current_cpu_data.x86
>> 
>>      /* <suitable comment> */
>>      if ( current_cpu_data.x86_vendor == X86_VENDOR_HYGON && fam == 0x18 )
>>         fam = 0x17;
> 
> This is the minimum change, I think it's better.
> 
>> 
>>      switch ( fam )
>>      ...
>> 
>> or perhaps even better would be two separate switch()-es, one for
>> AMD and one for Hygon. Possibly even a separate hygon_vpmu_init().
> 
> A separate hygon_vpmu_init() is also fine except that the last part of
> the function can be shared.

So perhaps split that part out into a static _vpmu_init() or common_init()?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 03/14] x86/cpu/vpmu: Add Hygon Dhyana and AMD Zen support for vPMU

[Pu Wen]

On 2019/3/19 21:58, Jan Beulich wrote:
> On 19.03.19 at 14:47, <puwen@hygon.cn> wrote:
>> On 2019/3/19 20:58, Jan Beulich wrote:
>>> On 19.03.19 at 12:32, <puwen@hygon.cn> wrote:
>>>> On 2019/3/18 16:59, Jan Beulich wrote:
>>>>> On 16.03.19 at 11:11, <puwen@hygon.cn> wrote:
>>>>>> On 2019/3/15 20:41, Jan Beulich wrote:
>>>>>>> On 21.02.19 at 10:50, <puwen@hygon.cn> wrote:
>>>>>>>> --- a/xen/arch/x86/cpu/vpmu_amd.c
>>>>>>>> +++ b/xen/arch/x86/cpu/vpmu_amd.c
>>>>>>>> @@ -545,6 +545,8 @@ int __init amd_vpmu_init(void)
>>>>>>>>          switch ( current_cpu_data.x86 )
>>>>>>>>          {
>>>>>>>>          case 0x15:
>>>>>>>> +    case 0x17:
>>>>>>>> +    case 0x18:
>>>>>>>>              num_counters = F15H_NUM_COUNTERS;
>>>>>>>>              counters = AMD_F15H_COUNTERS;
>>>>>>>>              ctrls = AMD_F15H_CTRLS;
>>>>>>>
>>>>>>> Unless you know what AMD Fam18 will look like, you can't do it
>>>>>>> like this. Fam18 really needs to be further qualified by a vendor
>>>>>>> check at this point in time.
>>>>>>
>>>>>> Hygon will negotiate with AMD to make sure that only Hygon should use
>>>>>> Fam18h.
>>>>>
>>>>> In the success case of which please state this in the description.
>>>>> Until those negotiations have succeeded I'm afraid I'm going to
>>>>> insist to see the extra check added.
>>>> or just add Hygon support at beginning of amd_vpmu_init():
>>>>        if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) {
>>>>            num_counters = F15H_NUM_COUNTERS;
>>>>            counters = AMD_F15H_COUNTERS;
>>>>            ctrls = AMD_F15H_CTRLS;
>>>>            k7_counters_mirrored = 1;
>>>>        }
>>> or perhaps even better would be two separate switch()-es, one for
>>> AMD and one for Hygon. Possibly even a separate hygon_vpmu_init().
>>
>> A separate hygon_vpmu_init() is also fine except that the last part of
>> the function can be shared.
> 
> So perhaps split that part out into a static _vpmu_init() or common_init()?

Okay, I'll try to split that part.

-- 
Regards,
Pu Wen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel