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

["Fiona Li(BJ-RD)" <FionaLi@xxxxxxxxxxx>]

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

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

>>> On 23.03.18 at 12:28, <lifang110@xxxxxxx> wrote:
> From: FionaLi <FionaLi@xxxxxxxxxxx>
>
> Signed-off-by: Fiona Li<fionali@xxxxxxxxxxx>

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@xxxxxxxxxxx) 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel