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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.09.14 at 10:48, <alfred.z.song@xxxxxxxxx> wrote:

Things look fine from a general pov, but

> @@ -278,12 +274,14 @@ static void xc_cpuid_hvm_policy(
>      DECLARE_DOMCTL;
>      char brand[13];
>      uint64_t val;
> -    int is_pae, is_nestedhvm;
> +    int is_64bit, is_pae, is_nestedhvm;
>      uint64_t xfeature_mask;
> 
>      xc_hvm_param_get(xch, domid, HVM_PARAM_PAE_ENABLED, &val);
>      is_pae = !!val;
> -
> +
> +    is_64bit = hypervisor_is_64bit(xch) && is_pae;

... with this using hypervisor_is_64bit() and there not being a 32-bit
hypervisor anymore, there's clearly room for more cleanup (and in
particular no need to pass around an "is_64bit" variable that's always
going to be set to true).

> @@ -391,10 +389,18 @@ static void xc_cpuid_hvm_policy(
>          break;
> 
>      case 0x80000001:
> -        if ( !is_pae ) {
> +        if ( !is_64bit ) {
> +            clear_bit(X86_FEATURE_LAHF_LM, regs[2]);
> +            clear_bit(X86_FEATURE_LM, regs[3]);
> +            clear_bit(X86_FEATURE_NX, regs[3]);
> +            clear_bit(X86_FEATURE_PSE36, regs[3]);
> +        } else if ( !is_pae ) {
>              clear_bit(X86_FEATURE_NX, regs[3]);
>              clear_bit(X86_FEATURE_PSE36, regs[3]);
> +        } else {
> +            /* Do nothing for 32-bit guest */
>          }

The ordering of the if/else-if above seems wrong to me, but this
would become moot anyway if "is_64bit" got dropped.

Jan


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