Re: [Xen-devel] [PATCH] xc_cpuid_x86.c: No need to mask NX twice

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.09.14 at 18:35, <alfred.z.song@xxxxxxxxx> wrote:
> Got it. Thanks, Jan.
> If so, I think we could remove the condition for masking NX in both vendor
> specific functions, since the architectural logic has help cover it and the
> judgement is unnecessary.    For example:
> 
> diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
> index 61af3e6..6bd89b0 100644
> --- a/tools/libxc/xc_cpuid_x86.c
> +++ b/tools/libxc/xc_cpuid_x86.c
> @@ -116,7 +116,7 @@ static void amd_xc_cpuid_policy(
>                      bitmaskof(X86_FEATURE_TBM) |
>                      bitmaskof(X86_FEATURE_DBEXT));
>          regs[3] &= (0x0183f3ff | /* features shared with 0x00000001:EDX */
> -                    (is_pae ? bitmaskof(X86_FEATURE_NX) : 0) |
> +                    bitmaskof(X86_FEATURE_NX) |
>                      (is_64bit ? bitmaskof(X86_FEATURE_LM) : 0) |
>                      bitmaskof(X86_FEATURE_SYSCALL) |
>                      bitmaskof(X86_FEATURE_MP) |
> @@ -201,7 +201,7 @@ static void intel_xc_cpuid_policy(
>          regs[2] &= (is_64bit ? bitmaskof(X86_FEATURE_LAHF_LM) : 0) |
>                                 bitmaskof(X86_FEATURE_3DNOWPREFETCH) |
>                                 bitmaskof(X86_FEATURE_ABM);
> -        regs[3] &= ((is_pae ? bitmaskof(X86_FEATURE_NX) : 0) |
> +        regs[3] &= (bitmaskof(X86_FEATURE_NX) |
>                      (is_64bit ? bitmaskof(X86_FEATURE_LM) : 0) |
>                      (is_64bit ? bitmaskof(X86_FEATURE_SYSCALL) : 0) |
>                      (is_64bit ? bitmaskof(X86_FEATURE_RDTSCP) : 0));

Right, and I think you could go further and move at least the LM
check into vendor-independent code too.

Looking at the context above I also wonder whether tying RDTSCP
to 64-bit guests is really correct in (at least) the Intel case.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel