[PATCH] x86/cpu: Support a new cpu vendor, which is Shanghai. Shanghai cpu defines two msr registers to enable Random Number Generator and Advanced Cryprography Engine.The cpu supports iommu, which is designed according to Intel's specification.

[PATCH] x86/cpu: Support a new cpu vendor, which is Shanghai. Shanghai cpu defines two msr registers to enable Random Number Generator and Advanced Cryprography Engine.The cpu supports iommu, which is designed according to Intel's specification.

[Fionali]

From: FionaLi <FionaLi@zhaoxin.com>

Signed-off-by: Fiona Li<fionali@zhaoxin.com>
---
 xen/arch/x86/cpu/Makefile         |  1 +
 xen/arch/x86/cpu/common.c         |  1 +
 xen/arch/x86/cpu/shanghai.c       | 61 +++++++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/iommu.h       |  2 ++
 xen/include/asm-x86/msr-index.h   |  4 +++
 xen/include/asm-x86/setup.h       |  1 +
 xen/include/asm-x86/x86-vendors.h |  3 +-
 7 files changed, 72 insertions(+), 1 deletion(-)
 create mode 100644 xen/arch/x86/cpu/shanghai.c

diff --git a/xen/arch/x86/cpu/Makefile b/xen/arch/x86/cpu/Makefile
index 74f23ae..8fcffdd 100644
--- a/xen/arch/x86/cpu/Makefile
+++ b/xen/arch/x86/cpu/Makefile
@@ -5,6 +5,7 @@ obj-y += amd.o
 obj-y += centaur.o
 obj-y += common.o
 obj-y += intel.o
+obj-y += shanghai.o
 obj-y += intel_cacheinfo.o
 obj-y += mwait-idle.o
 obj-y += vpmu.o vpmu_amd.o vpmu_intel.o
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 0a452ae..02863c9 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -709,6 +709,7 @@ void __init early_cpu_init(void)
 	intel_cpu_init();
 	amd_init_cpu();
 	centaur_init_cpu();
+	shanghai_init_cpu();
 	early_cpu_detect();
 }
 
diff --git a/xen/arch/x86/cpu/shanghai.c b/xen/arch/x86/cpu/shanghai.c
new file mode 100644
index 0000000..7910f03
--- /dev/null
+++ b/xen/arch/x86/cpu/shanghai.c
@@ -0,0 +1,61 @@
+#include <xen/lib.h>
+#include <xen/init.h>
+#include <xen/bitops.h>
+#include <asm/processor.h>
+#include <asm/msr.h>
+#include <asm/e820.h>
+#include "cpu.h"
+
+#define ACE_PRESENT(x)  ((x)&(1U<<6))
+#define ACE_ENABLED(x)  ((x)&(1U<<7))
+#define ACE_FCR		(1U << 28)	/* MSR_ZX_ACE Advanced Cryprography Engine */
+
+#define RNG_PRESENT(x)  ((x)&(1U<<6))
+#define RNG_ENABLED(x)  ((x)&(1U<<7))
+#define RNG_ENABLE	(1U << 6)	/* MSR_ZX_RNG Random Number Generator */
+
+
+
+static void init_shanghai(struct cpuinfo_x86 *c)
+{
+	uint64_t msr_ace,msr_rng;
+	/* Test for Shanghai Extended CPUID information */
+	if (cpuid_eax(0xC0000000) >= 0xC0000001) {
+		/*Get Shanghai Extended function number */
+		u32 extented_feature_flags = cpuid_edx(0xC0000001);
+
+		/* enable ACE,if support ACE unit */
+		if(ACE_PRESENT(extented_feature_flags) && !ACE_ENABLED(extented_feature_flags))	{
+			rdmsrl(MSR_ZX_ACE, msr_ace);
+			/* enable ACE  */
+			wrmsrl(MSR_ZX_ACE, (msr_ace | ACE_FCR));
+			printk(KERN_INFO "CPU: Enabled ACE h/w crypto\n");
+		}
+		/* enable RNG,if support RNG unit */
+		if (RNG_PRESENT(extented_feature_flags) && !RNG_ENABLED(extented_feature_flags)) {
+			rdmsrl(MSR_ZX_RNG, msr_rng);
+			/* enable RNG  */
+			wrmsrl(MSR_ZX_RNG, msr_rng | RNG_ENABLE);
+			printk(KERN_INFO "CPU: Enabled h/w RNG\n");
+		}
+	}
+
+	if (c->x86 == 0x6 && c->x86_model >= 0xf) {
+		c->x86_cache_alignment = c->x86_clflush_size * 2;
+		__set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
+	}
+	get_model_name(c);
+	display_cacheinfo(c);
+}
+
+static const struct cpu_dev shanghai_cpu_dev = {
+	.c_vendor	= "Shanghai",
+	.c_ident	= { "  Shanghai  " },
+	.c_init		= init_shanghai,
+};
+
+int __init shanghai_init_cpu(void)
+{
+	cpu_devs[X86_VENDOR_SHANGHAI] = &shanghai_cpu_dev;
+	return 0;
+}
diff --git a/xen/include/asm-x86/iommu.h b/xen/include/asm-x86/iommu.h
index 14ad048..c125da6 100644
--- a/xen/include/asm-x86/iommu.h
+++ b/xen/include/asm-x86/iommu.h
@@ -53,6 +53,7 @@ static inline const struct iommu_ops *iommu_get_ops(void)
 {
     switch ( boot_cpu_data.x86_vendor )
     {
+    case X86_VENDOR_SHANGHAI:
     case X86_VENDOR_INTEL:
         return &intel_iommu_ops;
     case X86_VENDOR_AMD:
@@ -68,6 +69,7 @@ static inline int iommu_hardware_setup(void)
 {
     switch ( boot_cpu_data.x86_vendor )
     {
+    case X86_VENDOR_SHANGHAI:
     case X86_VENDOR_INTEL:
         return intel_vtd_setup();
     case X86_VENDOR_AMD:
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index 23ad743..f2ce71a 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -293,6 +293,10 @@
 #define MSR_TMTA_LRTI_READOUT		0x80868018
 #define MSR_TMTA_LRTI_VOLT_MHZ		0x8086801a
 
+/* Shanghai ZhaoXin defined MSRs*/
+#define MSR_ZX_ACE			0x00001107
+#define MSR_ZX_RNG			0x0000110b
+
 /* Intel defined MSRs. */
 #define MSR_IA32_P5_MC_ADDR		0x00000000
 #define MSR_IA32_P5_MC_TYPE		0x00000001
diff --git a/xen/include/asm-x86/setup.h b/xen/include/asm-x86/setup.h
index 19232af..827daf8 100644
--- a/xen/include/asm-x86/setup.h
+++ b/xen/include/asm-x86/setup.h
@@ -22,6 +22,7 @@ int amd_init_cpu(void);
 int cyrix_init_cpu(void);
 int nsc_init_cpu(void);
 int centaur_init_cpu(void);
+int shanghai_init_cpu(void);
 int transmeta_init_cpu(void);
 
 void set_nr_cpu_ids(unsigned int max_cpus);
diff --git a/xen/include/asm-x86/x86-vendors.h b/xen/include/asm-x86/x86-vendors.h
index cae5507..c53d0b9 100644
--- a/xen/include/asm-x86/x86-vendors.h
+++ b/xen/include/asm-x86/x86-vendors.h
@@ -7,7 +7,8 @@
 #define X86_VENDOR_INTEL 0
 #define X86_VENDOR_AMD 1
 #define X86_VENDOR_CENTAUR 2
-#define X86_VENDOR_NUM 3
+#define X86_VENDOR_SHANGHAI 3
+#define X86_VENDOR_NUM 4
 #define X86_VENDOR_UNKNOWN 0xff
 
 #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

Re: [PATCH] x86/cpu: Support a new cpu vendor, which is Shanghai. Shanghai cpu defines two msr registers to enable Random Number Generator and Advanced Cryprography Engine.The cpu supports iommu, which is designed according to Intel's specification.

[Wei Liu]

On Fri, Mar 23, 2018 at 07:28:56PM +0800, Fionali wrote:
> From: FionaLi <FionaLi@zhaoxin.com>
> 
> Signed-off-by: Fiona Li<fionali@zhaoxin.com>
> ---
>  xen/arch/x86/cpu/Makefile         |  1 +
>  xen/arch/x86/cpu/common.c         |  1 +
>  xen/arch/x86/cpu/shanghai.c       | 61 +++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-x86/iommu.h       |  2 ++
>  xen/include/asm-x86/msr-index.h   |  4 +++
>  xen/include/asm-x86/setup.h       |  1 +
>  xen/include/asm-x86/x86-vendors.h |  3 +-
>  7 files changed, 72 insertions(+), 1 deletion(-)
>  create mode 100644 xen/arch/x86/cpu/shanghai.c
> 
> diff --git a/xen/arch/x86/cpu/Makefile b/xen/arch/x86/cpu/Makefile
> index 74f23ae..8fcffdd 100644
> --- a/xen/arch/x86/cpu/Makefile
> +++ b/xen/arch/x86/cpu/Makefile
> @@ -5,6 +5,7 @@ obj-y += amd.o
>  obj-y += centaur.o
>  obj-y += common.o
>  obj-y += intel.o
> +obj-y += shanghai.o

I'm confused. Shouldn't you use zhaoxin instead?

Wei.

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

Re: [PATCH] x86/cpu: Support a new cpu vendor, which is Shanghai. Shanghai cpu defines two msr registers to enable Random Number Generator and Advanced Cryprography Engine.The cpu supports iommu, which is designed according to Intel's specification.

[Wei Liu]

On Fri, Mar 23, 2018 at 07:28:56PM +0800, Fionali wrote:
> From: FionaLi <FionaLi@zhaoxin.com>
> 
> Signed-off-by: Fiona Li<fionali@zhaoxin.com>
> ---
>  xen/arch/x86/cpu/Makefile         |  1 +
>  xen/arch/x86/cpu/common.c         |  1 +
>  xen/arch/x86/cpu/shanghai.c       | 61 +++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-x86/iommu.h       |  2 ++
>  xen/include/asm-x86/msr-index.h   |  4 +++
>  xen/include/asm-x86/setup.h       |  1 +
>  xen/include/asm-x86/x86-vendors.h |  3 +-
>  7 files changed, 72 insertions(+), 1 deletion(-)
>  create mode 100644 xen/arch/x86/cpu/shanghai.c
> 
> diff --git a/xen/arch/x86/cpu/Makefile b/xen/arch/x86/cpu/Makefile
> index 74f23ae..8fcffdd 100644
> --- a/xen/arch/x86/cpu/Makefile
> +++ b/xen/arch/x86/cpu/Makefile
> @@ -5,6 +5,7 @@ obj-y += amd.o
>  obj-y += centaur.o
>  obj-y += common.o
>  obj-y += intel.o
> +obj-y += shanghai.o
>  obj-y += intel_cacheinfo.o
>  obj-y += mwait-idle.o
>  obj-y += vpmu.o vpmu_amd.o vpmu_intel.o
> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
> index 0a452ae..02863c9 100644
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -709,6 +709,7 @@ void __init early_cpu_init(void)
>  	intel_cpu_init();
>  	amd_init_cpu();
>  	centaur_init_cpu();
> +	shanghai_init_cpu();
>  	early_cpu_detect();
>  }
>  
> diff --git a/xen/arch/x86/cpu/shanghai.c b/xen/arch/x86/cpu/shanghai.c
> new file mode 100644
> index 0000000..7910f03
> --- /dev/null
> +++ b/xen/arch/x86/cpu/shanghai.c
> @@ -0,0 +1,61 @@
> +#include <xen/lib.h>
> +#include <xen/init.h>
> +#include <xen/bitops.h>
> +#include <asm/processor.h>
> +#include <asm/msr.h>
> +#include <asm/e820.h>

Use the following order please:

 #include <xen/bitops.h>
 #include <xen/init.h>
 #include <xen/lib.h>

 #include <asm/e820.h>
 #include <asm/msr.h>
 #include <asm/processor.h>

> +#include "cpu.h"
> +
> +#define ACE_PRESENT(x)  ((x)&(1U<<6))

Please add spaces around "&" and "<<".

> +#define ACE_ENABLED(x)  ((x)&(1U<<7))
> +#define ACE_FCR		(1U << 28)	/* MSR_ZX_ACE Advanced Cryprography Engine */
> +
> +#define RNG_PRESENT(x)  ((x)&(1U<<6))
> +#define RNG_ENABLED(x)  ((x)&(1U<<7))
> +#define RNG_ENABLE	(1U << 6)	/* MSR_ZX_RNG Random Number Generator */
> +
> +
> +
> +static void init_shanghai(struct cpuinfo_x86 *c)
> +{
> +	uint64_t msr_ace,msr_rng;

Add a blank line here.

> +	/* Test for Shanghai Extended CPUID information */
> +	if (cpuid_eax(0xC0000000) >= 0xC0000001) {

Coding style. Should be

        if ( XXXX ) 
	{

Please fix all instances.


> +		/*Get Shanghai Extended function number */
> +		u32 extented_feature_flags = cpuid_edx(0xC0000001);
> +
> +		/* enable ACE,if support ACE unit */
> +		if(ACE_PRESENT(extented_feature_flags) && !ACE_ENABLED(extented_feature_flags))	{
> +			rdmsrl(MSR_ZX_ACE, msr_ace);
> +			/* enable ACE  */
> +			wrmsrl(MSR_ZX_ACE, (msr_ace | ACE_FCR));
> +			printk(KERN_INFO "CPU: Enabled ACE h/w crypto\n");

Drop KERN_INFO please.

> +		}

Blank line here please.

> +		/* enable RNG,if support RNG unit */
> +		if (RNG_PRESENT(extented_feature_flags) && !RNG_ENABLED(extented_feature_flags)) {
> +			rdmsrl(MSR_ZX_RNG, msr_rng);
> +			/* enable RNG  */
> +			wrmsrl(MSR_ZX_RNG, msr_rng | RNG_ENABLE);
> +			printk(KERN_INFO "CPU: Enabled h/w RNG\n");
> +		}
> +	}
> +
> +	if (c->x86 == 0x6 && c->x86_model >= 0xf) {
> +		c->x86_cache_alignment = c->x86_clflush_size * 2;
> +		__set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
> +	}

Blank line.

> +	get_model_name(c);
> +	display_cacheinfo(c);
> +}


Wei.

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

Re: [PATCH] x86/cpu: Support a new cpu vendor, which is Shanghai. Shanghai cpu defines two msr registers to enable Random Number Generator and Advanced Cryprography Engine.The cpu supports iommu, which is designed according to Intel's specification.

[Jan Beulich]

>>> On 23.03.18 at 13:41, <wei.liu2@citrix.com> wrote:
> On Fri, Mar 23, 2018 at 07:28:56PM +0800, Fionali wrote:
>> +	/* Test for Shanghai Extended CPUID information */
>> +	if (cpuid_eax(0xC0000000) >= 0xC0000001) {
> 
> Coding style. Should be
> 
>         if ( XXXX ) 
> 	{

FAOD with the tab replaced by 4 spaces.

Jan


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

Re: [PATCH] x86/cpu: Support a new cpu vendor, which is Shanghai

[Jan Beulich]

>>> On 23.03.18 at 12:28, <lifang110@126.com> wrote:
> From: FionaLi <FionaLi@zhaoxin.com>
> 
> Signed-off-by: Fiona Li<fionali@zhaoxin.com>

First of all, please shorten the subject and put a fair part of what
you had there in the description.

Then you talk about a VT-d compatible IOMMU, but not about VMX
or some other CPU side hardware virtualization. Is that really not
available?

Further it would help if the mail address you send from was in
sync (or at least allow some matching with) the one in the From
and S-o-b.

> --- a/xen/arch/x86/cpu/Makefile
> +++ b/xen/arch/x86/cpu/Makefile
> @@ -5,6 +5,7 @@ obj-y += amd.o
>  obj-y += centaur.o
>  obj-y += common.o
>  obj-y += intel.o
> +obj-y += shanghai.o

Please put where it belongs alphabetically.

> --- /dev/null
> +++ b/xen/arch/x86/cpu/shanghai.c
> @@ -0,0 +1,61 @@
> +#include <xen/lib.h>
> +#include <xen/init.h>
> +#include <xen/bitops.h>
> +#include <asm/processor.h>
> +#include <asm/msr.h>
> +#include <asm/e820.h>
> +#include "cpu.h"
> +
> +#define ACE_PRESENT(x)  ((x)&(1U<<6))
> +#define ACE_ENABLED(x)  ((x)&(1U<<7))
> +#define ACE_FCR		(1U << 28)	/* MSR_ZX_ACE Advanced Cryprography Engine */
> +
> +#define RNG_PRESENT(x)  ((x)&(1U<<6))
> +#define RNG_ENABLED(x)  ((x)&(1U<<7))
> +#define RNG_ENABLE	(1U << 6)	/* MSR_ZX_RNG Random Number Generator */

Style: Blanks around binary operators please.

> +
> +
> +

Please don't put multiple consecutive blank lines anywhere.

> +static void init_shanghai(struct cpuinfo_x86 *c)
> +{
> +	uint64_t msr_ace,msr_rng;
> +	/* Test for Shanghai Extended CPUID information */
> +	if (cpuid_eax(0xC0000000) >= 0xC0000001) {
> +		/*Get Shanghai Extended function number */
> +		u32 extented_feature_flags = cpuid_edx(0xC0000001);
> +
> +		/* enable ACE,if support ACE unit */
> +		if(ACE_PRESENT(extented_feature_flags) && !ACE_ENABLED(extented_feature_flags))	{
> +			rdmsrl(MSR_ZX_ACE, msr_ace);
> +			/* enable ACE  */
> +			wrmsrl(MSR_ZX_ACE, (msr_ace | ACE_FCR));
> +			printk(KERN_INFO "CPU: Enabled ACE h/w crypto\n");
> +		}
> +		/* enable RNG,if support RNG unit */
> +		if (RNG_PRESENT(extented_feature_flags) && !RNG_ENABLED(extented_feature_flags)) {
> +			rdmsrl(MSR_ZX_RNG, msr_rng);
> +			/* enable RNG  */
> +			wrmsrl(MSR_ZX_RNG, msr_rng | RNG_ENABLE);
> +			printk(KERN_INFO "CPU: Enabled h/w RNG\n");
> +		}
> +	}
> +
> +	if (c->x86 == 0x6 && c->x86_model >= 0xf) {
> +		c->x86_cache_alignment = c->x86_clflush_size * 2;
> +		__set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
> +	}

Is there a specification available anywhere for all of the above?

What about guests? How would they know these extensions are
available for their use?

> --- a/xen/include/asm-x86/iommu.h
> +++ b/xen/include/asm-x86/iommu.h
> @@ -53,6 +53,7 @@ static inline const struct iommu_ops *iommu_get_ops(void)
>  {
>      switch ( boot_cpu_data.x86_vendor )
>      {
> +    case X86_VENDOR_SHANGHAI:
>      case X86_VENDOR_INTEL:
>          return &intel_iommu_ops;
>      case X86_VENDOR_AMD:
> @@ -68,6 +69,7 @@ static inline int iommu_hardware_setup(void)
>  {
>      switch ( boot_cpu_data.x86_vendor )
>      {
> +    case X86_VENDOR_SHANGHAI:
>      case X86_VENDOR_INTEL:
>          return intel_vtd_setup();
>      case X86_VENDOR_AMD:

Please don't put new entries first.

> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -293,6 +293,10 @@
>  #define MSR_TMTA_LRTI_READOUT		0x80868018
>  #define MSR_TMTA_LRTI_VOLT_MHZ		0x8086801a
>  
> +/* Shanghai ZhaoXin defined MSRs*/
> +#define MSR_ZX_ACE			0x00001107
> +#define MSR_ZX_RNG			0x0000110b

As Wei has already indicated, we'd prefer consistent names.
Either ZX / ZhaoXin everywhere, or Shanghai. If one of them is
just a code name, the permanent one would obviously better.
This extends to ...

> --- a/xen/include/asm-x86/setup.h
> +++ b/xen/include/asm-x86/setup.h
> @@ -22,6 +22,7 @@ int amd_init_cpu(void);
>  int cyrix_init_cpu(void);
>  int nsc_init_cpu(void);
>  int centaur_init_cpu(void);
> +int shanghai_init_cpu(void);

... this and ...

> --- a/xen/include/asm-x86/x86-vendors.h
> +++ b/xen/include/asm-x86/x86-vendors.h
> @@ -7,7 +7,8 @@
>  #define X86_VENDOR_INTEL 0
>  #define X86_VENDOR_AMD 1
>  #define X86_VENDOR_CENTAUR 2
> -#define X86_VENDOR_NUM 3
> +#define X86_VENDOR_SHANGHAI 3

... this (alongside the file name chosen).

Jan

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

Re: [PATCH] x86/cpu: Support a new cpu vendor, which is Shanghai. Shanghai cpu defines two msr registers to enable Random Number Generator and Advanced Cryprography Engine.The cpu supports iommu, which is designed according to Intel's speci...

[Fiona Li(BJ-RD)]

Wei,
    Thanks for your reply. I will revise that in the next version of the patch.
    To solve the problem that the mail address I send from was not in sync the one in the From and S-o-b. I will use another email address(davidwang@zhaoxin.com) to resubmit my patch. If necessary, please abandon current patch and subject. I am sorry for the trouble.

-----Original Message-----
From: Wei Liu [mailto:wei.liu2@citrix.com]
Sent: Friday, March 23, 2018 8:42 PM
To: Fionali <lifang110@126.com>
Cc: xen-devel@lists.xenproject.org; Fiona Li(BJ-RD) <FionaLi@zhaoxin.com>; jbeulich@suse.com; Wei Liu <wei.liu2@citrix.com>
Subject: Re: [Xen-devel] [PATCH] x86/cpu: Support a new cpu vendor, which is Shanghai. Shanghai cpu defines two msr registers to enable Random Number Generator and Advanced Cryprography Engine.The cpu supports iommu, which is designed according to Intel's speci...

On Fri, Mar 23, 2018 at 07:28:56PM +0800, Fionali wrote:
> From: FionaLi <FionaLi@zhaoxin.com>
>
> Signed-off-by: Fiona Li<fionali@zhaoxin.com>
> ---
>  xen/arch/x86/cpu/Makefile         |  1 +
>  xen/arch/x86/cpu/common.c         |  1 +
>  xen/arch/x86/cpu/shanghai.c       | 61 +++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-x86/iommu.h       |  2 ++
>  xen/include/asm-x86/msr-index.h   |  4 +++
>  xen/include/asm-x86/setup.h       |  1 +
>  xen/include/asm-x86/x86-vendors.h |  3 +-
>  7 files changed, 72 insertions(+), 1 deletion(-)  create mode 100644
> xen/arch/x86/cpu/shanghai.c
>
> diff --git a/xen/arch/x86/cpu/Makefile b/xen/arch/x86/cpu/Makefile
> index 74f23ae..8fcffdd 100644
> --- a/xen/arch/x86/cpu/Makefile
> +++ b/xen/arch/x86/cpu/Makefile
> @@ -5,6 +5,7 @@ obj-y += amd.o
>  obj-y += centaur.o
>  obj-y += common.o
>  obj-y += intel.o
> +obj-y += shanghai.o
>  obj-y += intel_cacheinfo.o
>  obj-y += mwait-idle.o
>  obj-y += vpmu.o vpmu_amd.o vpmu_intel.o diff --git
> a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c index
> 0a452ae..02863c9 100644
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -709,6 +709,7 @@ void __init early_cpu_init(void)
>  intel_cpu_init();
>  amd_init_cpu();
>  centaur_init_cpu();
> +shanghai_init_cpu();
>  early_cpu_detect();
>  }
>
> diff --git a/xen/arch/x86/cpu/shanghai.c b/xen/arch/x86/cpu/shanghai.c
> new file mode 100644 index 0000000..7910f03
> --- /dev/null
> +++ b/xen/arch/x86/cpu/shanghai.c
> @@ -0,0 +1,61 @@
> +#include <xen/lib.h>
> +#include <xen/init.h>
> +#include <xen/bitops.h>
> +#include <asm/processor.h>
> +#include <asm/msr.h>
> +#include <asm/e820.h>

Use the following order please:

 #include <xen/bitops.h>
 #include <xen/init.h>
 #include <xen/lib.h>

 #include <asm/e820.h>
 #include <asm/msr.h>
 #include <asm/processor.h>

> +#include "cpu.h"
> +
> +#define ACE_PRESENT(x)  ((x)&(1U<<6))

Please add spaces around "&" and "<<".

> +#define ACE_ENABLED(x)  ((x)&(1U<<7))
> +#define ACE_FCR(1U << 28)/* MSR_ZX_ACE Advanced Cryprography Engine */
> +
> +#define RNG_PRESENT(x)  ((x)&(1U<<6)) #define RNG_ENABLED(x)
> +((x)&(1U<<7))
> +#define RNG_ENABLE(1U << 6)/* MSR_ZX_RNG Random Number Generator */
> +
> +
> +
> +static void init_shanghai(struct cpuinfo_x86 *c) {
> +uint64_t msr_ace,msr_rng;

Add a blank line here.

> +/* Test for Shanghai Extended CPUID information */
> +if (cpuid_eax(0xC0000000) >= 0xC0000001) {

Coding style. Should be

        if ( XXXX )
{

Please fix all instances.


> +/*Get Shanghai Extended function number */
> +u32 extented_feature_flags = cpuid_edx(0xC0000001);
> +
> +/* enable ACE,if support ACE unit */
> +if(ACE_PRESENT(extented_feature_flags) && !ACE_ENABLED(extented_feature_flags)){
> +rdmsrl(MSR_ZX_ACE, msr_ace);
> +/* enable ACE  */
> +wrmsrl(MSR_ZX_ACE, (msr_ace | ACE_FCR));
> +printk(KERN_INFO "CPU: Enabled ACE h/w crypto\n");

Drop KERN_INFO please.

> +}

Blank line here please.

> +/* enable RNG,if support RNG unit */
> +if (RNG_PRESENT(extented_feature_flags) && !RNG_ENABLED(extented_feature_flags)) {
> +rdmsrl(MSR_ZX_RNG, msr_rng);
> +/* enable RNG  */
> +wrmsrl(MSR_ZX_RNG, msr_rng | RNG_ENABLE);
> +printk(KERN_INFO "CPU: Enabled h/w RNG\n");
> +}
> +}
> +
> +if (c->x86 == 0x6 && c->x86_model >= 0xf) {
> +c->x86_cache_alignment = c->x86_clflush_size * 2;
> +__set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
> +}

Blank line.

> +get_model_name(c);
> +display_cacheinfo(c);
> +}


Wei.


保密声明：
本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
CONFIDENTIAL NOTE:
This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86/cpu: Support a new cpu vendor, which is Shanghai

[Fiona Li(BJ-RD)]

Hi Jan,
Thanks for your reply. I want to give some explanation as follows.

-----Original Message-----
From: Jan Beulich [mailto:JBeulich@suse.com]
Sent: Friday, March 23, 2018 11:32 PM
To: Fionali <lifang110@126.com>
Cc: xen-devel@lists.xenproject.org; Fiona Li(BJ-RD) <FionaLi@zhaoxin.com>
Subject: Re: [PATCH] x86/cpu: Support a new cpu vendor,which is Shanghai

>>> On 23.03.18 at 12:28, <lifang110@126.com> wrote:
> From: FionaLi <FionaLi@zhaoxin.com>
>
> Signed-off-by: Fiona Li<fionali@zhaoxin.com>

First of all, please shorten the subject and put a fair part of what you had there in the description.
[FionaLi]:Thanks, I will revise that in next version of the patch.

Then you talk about a VT-d compatible IOMMU, but not about VMX or some other CPU side hardware virtualization. Is that really not available?
[FionaLi]: My description about the IOMMU in subject is inexact. ZhaoXin is a x86 IC designer. Its SOC products support both CPU virtualization and I/O virtualization, which are compatible with Intel VMX and VT-d respectively. Whose cpu vendor id is 'Shanghai'.

Further it would help if the mail address you send from was in sync (or at least allow some matching with) the one in the From and S-o-b.
[FionaLi]: To solve this problem, I will use another email address(davidwang@zhaoxin.com) to resubmit my patch. If necessary, please abandon current patch and subject. I am sorry for the trouble.

> --- a/xen/arch/x86/cpu/Makefile
> +++ b/xen/arch/x86/cpu/Makefile
> @@ -5,6 +5,7 @@ obj-y += amd.o
>  obj-y += centaur.o
>  obj-y += common.o
>  obj-y += intel.o
> +obj-y += shanghai.o

Please put where it belongs alphabetically.

> --- /dev/null
> +++ b/xen/arch/x86/cpu/shanghai.c
> @@ -0,0 +1,61 @@
> +#include <xen/lib.h>
> +#include <xen/init.h>
> +#include <xen/bitops.h>
> +#include <asm/processor.h>
> +#include <asm/msr.h>
> +#include <asm/e820.h>
> +#include "cpu.h"
> +
> +#define ACE_PRESENT(x)  ((x)&(1U<<6)) #define ACE_ENABLED(x)
> +((x)&(1U<<7))
> +#define ACE_FCR(1U << 28)/* MSR_ZX_ACE Advanced Cryprography Engine */
> +
> +#define RNG_PRESENT(x)  ((x)&(1U<<6)) #define RNG_ENABLED(x)
> +((x)&(1U<<7))
> +#define RNG_ENABLE(1U << 6)/* MSR_ZX_RNG Random Number Generator */

Style: Blanks around binary operators please.

> +
> +
> +

Please don't put multiple consecutive blank lines anywhere.

> +static void init_shanghai(struct cpuinfo_x86 *c) {
> +uint64_t msr_ace,msr_rng;
> +/* Test for Shanghai Extended CPUID information */
> +if (cpuid_eax(0xC0000000) >= 0xC0000001) {
> +/*Get Shanghai Extended function number */
> +u32 extented_feature_flags = cpuid_edx(0xC0000001);
> +
> +/* enable ACE,if support ACE unit */
> +if(ACE_PRESENT(extented_feature_flags) && !ACE_ENABLED(extented_feature_flags)){
> +rdmsrl(MSR_ZX_ACE, msr_ace);
> +/* enable ACE  */
> +wrmsrl(MSR_ZX_ACE, (msr_ace | ACE_FCR));
> +printk(KERN_INFO "CPU: Enabled ACE h/w crypto\n");
> +}
> +/* enable RNG,if support RNG unit */
> +if (RNG_PRESENT(extented_feature_flags) && !RNG_ENABLED(extented_feature_flags)) {
> +rdmsrl(MSR_ZX_RNG, msr_rng);
> +/* enable RNG  */
> +wrmsrl(MSR_ZX_RNG, msr_rng | RNG_ENABLE);
> +printk(KERN_INFO "CPU: Enabled h/w RNG\n");
> +}
> +}
> +
> +if (c->x86 == 0x6 && c->x86_model >= 0xf) {
> +c->x86_cache_alignment = c->x86_clflush_size * 2;
> +__set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
> +}

Is there a specification available anywhere for all of the above?
[FionaLi]: Main usage of Zhaoxin platforms in recent years is for limited and embedded instead of distributed markets. So there is no such document for public access at present. But, Zhaoxin's x86 CPU is compatible with Intel x86 architecture. Its instruction sets are compatible with Intel. Furthermore, Zhaoxin's CPU virtualization tech and I/O virtualization tech are compatible with Intel VMX and VT-d respectively. So maybe we can refer to Intel manual at present.

What about guests? How would they know these extensions are available for their use?
[FionaLi]: My colleagues is committing code to Linux kernel and windows. Its extensions will be available to guests.

> --- a/xen/include/asm-x86/iommu.h
> +++ b/xen/include/asm-x86/iommu.h
> @@ -53,6 +53,7 @@ static inline const struct iommu_ops
> *iommu_get_ops(void)  {
>      switch ( boot_cpu_data.x86_vendor )
>      {
> +    case X86_VENDOR_SHANGHAI:
>      case X86_VENDOR_INTEL:
>          return &intel_iommu_ops;
>      case X86_VENDOR_AMD:
> @@ -68,6 +69,7 @@ static inline int iommu_hardware_setup(void)  {
>      switch ( boot_cpu_data.x86_vendor )
>      {
> +    case X86_VENDOR_SHANGHAI:
>      case X86_VENDOR_INTEL:
>          return intel_vtd_setup();
>      case X86_VENDOR_AMD:

Please don't put new entries first.

> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -293,6 +293,10 @@
>  #define MSR_TMTA_LRTI_READOUT0x80868018
>  #define MSR_TMTA_LRTI_VOLT_MHZ0x8086801a
>
> +/* Shanghai ZhaoXin defined MSRs*/
> +#define MSR_ZX_ACE0x00001107
> +#define MSR_ZX_RNG0x0000110b

As Wei has already indicated, we'd prefer consistent names.
Either ZX / ZhaoXin everywhere, or Shanghai. If one of them is just a code name, the permanent one would obviously better.
This extends to ...

> --- a/xen/include/asm-x86/setup.h
> +++ b/xen/include/asm-x86/setup.h
> @@ -22,6 +22,7 @@ int amd_init_cpu(void);  int cyrix_init_cpu(void);
> int nsc_init_cpu(void);  int centaur_init_cpu(void);
> +int shanghai_init_cpu(void);

... this and ...

> --- a/xen/include/asm-x86/x86-vendors.h
> +++ b/xen/include/asm-x86/x86-vendors.h
> @@ -7,7 +7,8 @@
>  #define X86_VENDOR_INTEL 0
>  #define X86_VENDOR_AMD 1
>  #define X86_VENDOR_CENTAUR 2
> -#define X86_VENDOR_NUM 3
> +#define X86_VENDOR_SHANGHAI 3

... this (alongside the file name chosen).

Jan


保密声明：
本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
CONFIDENTIAL NOTE:
This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86/cpu: Support a new cpu vendor, which is Shanghai

[Jan Beulich]

>>> "Fiona Li(BJ-RD)" <FionaLi@zhaoxin.com> 04/10/18 3:08 AM >>>
>> +static void init_shanghai(struct cpuinfo_x86 *c) {
>> +uint64_t msr_ace,msr_rng;
>> +/* Test for Shanghai Extended CPUID information */
>> +if (cpuid_eax(0xC0000000) >= 0xC0000001) {
>> +/*Get Shanghai Extended function number */
>> +u32 extented_feature_flags = cpuid_edx(0xC0000001);
>> +
>> +/* enable ACE,if support ACE unit */
>> +if(ACE_PRESENT(extented_feature_flags) && !ACE_ENABLED(extented_feature_flags)){
>> +rdmsrl(MSR_ZX_ACE, msr_ace);
>> +/* enable ACE  */
>> +wrmsrl(MSR_ZX_ACE, (msr_ace | ACE_FCR));
>>> +printk(KERN_INFO "CPU: Enabled ACE h/w crypto\n");
>> +}
>> +/* enable RNG,if support RNG unit */
>> +if (RNG_PRESENT(extented_feature_flags) && !RNG_ENABLED(extented_feature_flags)) {
>> +rdmsrl(MSR_ZX_RNG, msr_rng);
>> +/* enable RNG  */
>> +wrmsrl(MSR_ZX_RNG, msr_rng | RNG_ENABLE);
>> +printk(KERN_INFO "CPU: Enabled h/w RNG\n");
>> +}
>> +}
>> +
>> +if (c->x86 == 0x6 && c->x86_model >= 0xf) {
>> +c->x86_cache_alignment = c->x86_clflush_size * 2;
>> +__set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
>> +}
>
>Is there a specification available anywhere for all of the above?
>[FionaLi]: Main usage of Zhaoxin platforms in recent years is for limited and embedded
>instead of distributed markets. So there is no such document for public access at
>present. But, Zhaoxin's x86 CPU is compatible with Intel x86 architecture. Its instruction
>sets are compatible with Intel. Furthermore, Zhaoxin's CPU virtualization tech and I/O
>virtualization tech are compatible with Intel VMX and VT-d respectively. So maybe we
>can refer to Intel manual at present.

Notw how I had said "for all of the above": The extensions are clearly not in the Intel
docs, and the cache alignment and TSC properties aren't general x86 attributes either.

>What about guests? How would they know these extensions are available for their use?
>[FionaLi]: My colleagues is committing code to Linux kernel and windows. Its extensions
>will be available to guests.

That wasn't the point of the question: Even with aware guest OSes you first of all need
to make sure guests can actually obtain the respective CPUID leaves. Afaict the
C000xxxx range will come out as all zeros for them without you doing something about
it. It is also questionable whether blanket enabling of the features for all guests is a
good step - I think this should be left to guest OSes (requiring you to properly emulate
their MSR accesses within Xen, unless the MSRs can be made directly accessible to
guests without security risks).

Jan


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

Re: [PATCH] x86/cpu: Support a new cpu vendor, which is Shanghai

[Fiona Li(BJ-RD)]

[-- Attachment #1: Type: text/plain, Size: 4251 bytes --]

Jan
Thanks for your reply.  Answer the following.
Best wish!
FionaLi

-----Original Message-----
From: Jan Beulich [mailto:jbeulich@suse.com]
Sent: Tuesday, April 10, 2018 2:35 PM
To: lifang110@126.com; Fiona Li(BJ-RD) <FionaLi@zhaoxin.com>
Cc: xen-devel@lists.xenproject.org
Subject: Re: RE: [PATCH] x86/cpu: Support a new cpu vendor,which is Shanghai

>>> "Fiona Li(BJ-RD)" <FionaLi@zhaoxin.com> 04/10/18 3:08 AM >>>
>> +static void init_shanghai(struct cpuinfo_x86 *c) { uint64_t
>> +msr_ace,msr_rng;
>> +/* Test for Shanghai Extended CPUID information */ if
>> +(cpuid_eax(0xC0000000) >= 0xC0000001) { /*Get Shanghai Extended
>> +function number */
>> +u32 extented_feature_flags = cpuid_edx(0xC0000001);
>> +
>> +/* enable ACE,if support ACE unit */
>> +if(ACE_PRESENT(extented_feature_flags) &&
>> +!ACE_ENABLED(extented_feature_flags)){
>> +rdmsrl(MSR_ZX_ACE, msr_ace);
>> +/* enable ACE  */
>> +wrmsrl(MSR_ZX_ACE, (msr_ace | ACE_FCR));
>>> +printk(KERN_INFO "CPU: Enabled ACE h/w crypto\n");ai
>> +}
>> +/* enable RNG,if support RNG unit */ if
>> +(RNG_PRESENT(extented_feature_flags) &&
>> +!RNG_ENABLED(extented_feature_flags)) { rdmsrl(MSR_ZX_RNG, msr_rng);
>> +/* enable RNG  */
>> +wrmsrl(MSR_ZX_RNG, msr_rng | RNG_ENABLE); printk(KERN_INFO "CPU:
>> +Enabled h/w RNG\n"); } }
>> +
>> +if (c->x86 == 0x6 && c->x86_model >= 0xf) {
>> +c->x86_cache_alignment = c->x86_clflush_size * 2;
>> +__set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability); }
>
>Is there a specification available anywhere for all of the above?
>[FionaLi]: Main usage of Zhaoxin platforms in recent years is for
>limited and embedded instead of distributed markets. So there is no
>such document for public access at present. But, Zhaoxin's x86 CPU is
>compatible with Intel x86 architecture. Its instruction sets are
>compatible with Intel. Furthermore, Zhaoxin's CPU virtualization tech
>and I/O virtualization tech are compatible with Intel VMX and VT-d respectively. So maybe we can refer to Intel manual at present.

Notw how I had said "for all of the above": The extensions are clearly not in the Intel docs, and the cache alignment and TSC properties aren't general x86 attributes either.
[FionaLi]:
For cache alignment, it is equal to the size of cache line on Zhaoxin platform. Since it has been initialized in early_cpu_detect(), we should remove this modification in next version patch.
For X86_FEATURE_CONSTANT_TSC, It  is an additional feature of TSC timer. For Shanghai CPU, it should be set according to X86_FEATURE_ITSC feature as Intel in next version patch.
Simple CPUID document in attached file is for your reference, the features supported by Shanghai CPU can be got from this doc.

>What about guests? How would they know these extensions are available for their use?
>[FionaLi]: My colleagues is committing code to Linux kernel and
>windows. Its extensions will be available to guests.

That wasn't the point of the question: Even with aware guest OSes you first of all need to make sure guests can actually obtain the respective CPUID leaves. Afaict the C000xxxx range will come out as all zeros for them without you doing something about it. It is also questionable whether blanket enabling of the features for all guests is a good step - I think this should be left to guest OSes (requiring you to properly emulate their MSR accesses within Xen, unless the MSRs can be made directly accessible to guests without security risks).
[FionaLi] : I am sorry. I understood wrongly. The C000xxxx range are extensions, which provide some additional feature different from Intel.  As you suggested, we will enable those features in guest OSes and remove these code from patch. Can we support the CPUID leaves and emulate the MSR  with another submit?



保密声明：
本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
CONFIDENTIAL NOTE:
This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited.

[-- Attachment #2: zhaoxin_cpuid_leaf_description.xlsx --]
[-- Type: application/vnd.openxmlformats-officedocument.spreadsheetml.sheet, Size: 40090 bytes --]

[-- Attachment #3: Type: text/plain, Size: 157 bytes --]

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

Re: [PATCH] x86/cpu: Support a new cpu vendor, which is Shanghai

[Jan Beulich]

>>> On 18.04.18 at 12:25, <FionaLi@zhaoxin.com> wrote:
> [FionaLi] : I am sorry. I understood wrongly. The C000xxxx range are 
> extensions, which provide some additional feature different from Intel.  As 
> you suggested, we will enable those features in guest OSes and remove these 
> code from patch. Can we support the CPUID leaves and emulate the MSR  with 
> another submit?

With another patch you mean? Yes please - if at all possible put separate
sets of changes into separate patches, combined into a series if there are
dependencies between them.

Jan



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

Re: [PATCH] x86/cpu: Support a new cpu vendor, which is Shanghai

[Fiona Li(BJ-RD)]

Hi Jan，
Yes, I mean with another patch. And Thank you again.

-----Original Message-----
From: Jan Beulich [mailto:JBeulich@suse.com]
Sent: Wednesday, April 18, 2018 6:52 PM
To: Fiona Li(BJ-RD) <FionaLi@zhaoxin.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>
Subject: RE: RE: [PATCH] x86/cpu: Support a new cpu vendor,which is Shanghai

>>> On 18.04.18 at 12:25, <FionaLi@zhaoxin.com> wrote:
> [FionaLi] : I am sorry. I understood wrongly. The C000xxxx range are
> extensions, which provide some additional feature different from
> Intel.  As you suggested, we will enable those features in guest OSes
> and remove these code from patch. Can we support the CPUID leaves and
> emulate the MSR  with another submit?

With another patch you mean? Yes please - if at all possible put separate sets of changes into separate patches, combined into a series if there are dependencies between them.

Jan




保密声明：
本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
CONFIDENTIAL NOTE:
This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel