Re: [Xen-devel] [PATCH v2 1/2] xc_cpuid_x86.c: Simplify masking conditions and remove redundant work

[z <alfred.z.song@xxxxxxxxx>]

On Tue, Sep 9, 2014 at 8:21 PM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:

On Tue, 2014-09-09 at 11:45 +0100, Jan Beulich wrote:
> >>> On 09.09.14 at 06:31, <alfred.z.song@xxxxxxxxx> wrote:
> > @@ -195,16 +186,14 @@ static void intel_xc_cpuid_policy(
> >Â Â Â Â Â break;
> >
> >Â Â Â case 0x80000001: {
> > -Â Â Â Â int is_64bit = hypervisor_is_64bit(xch) && is_pae;
> > -
> >Â Â Â Â Â /* Only a few features are advertised in Intel's 0x80000001. */
> > -Â Â Â Â 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) |
> > -Â Â Â Â Â Â Â Â Â Â (is_64bit ? bitmaskof(X86_FEATURE_LM) : 0) |
> > -Â Â Â Â Â Â Â Â Â Â (is_64bit ? bitmaskof(X86_FEATURE_SYSCALL) : 0) |
> > -Â Â Â Â Â Â Â Â Â Â (is_64bit ? bitmaskof(X86_FEATURE_RDTSCP) : 0));
> > +Â Â Â Â regs[2] &= (bitmaskof(X86_FEATURE_LAHF_LM) |
> > +Â Â Â Â Â Â Â Â Â Â bitmaskof(X86_FEATURE_3DNOWPREFETCH) |
> > +Â Â Â Â Â Â Â Â Â Â bitmaskof(X86_FEATURE_ABM);
> > +Â Â Â Â regs[3] &= (bitmaskof(X86_FEATURE_NX) |
> > +Â Â Â Â Â Â Â Â Â Â bitmaskof(X86_FEATURE_LM) |
> > +Â Â Â Â Â Â Â Â Â Â (is_pae ? bitmaskof(X86_FEATURE_SYSCALL) : 0) |
> > +Â Â Â Â Â Â Â Â Â Â (is_pae ? bitmaskof(X86_FEATURE_RDTSCP) : 0));
>
> As said before, tying these two features to is_pae seems a
> little strange, but if the tools maintainers can live with that, I
> guess I can too (short of having a better suggestion other
> than to drop the conditionals altogether).

Patch #2 here seems to remove it from the RDTSCP, surely that should be
folded in.

I also don't understand the link between PAE and the presence of
SYSCALL.

Ian.

Hi, Ian & Jan

It seems that SYSCALL is valid only in 64-bit mode in Intel's implementation, but for AMD, it is available in any mode (64/32).

So it should be vendor-dependent. That's why I keep Is_pae here for Intel.

Zhuo

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