Re: [Xen-devel] [PATCH v2 2/2] x86/hvm: add support for pcommit instruction

[Haozhong Zhang <haozhong.zhang@xxxxxxxxx>]

On 01/07/16 06:53, Jan Beulich wrote:
> >>> On 30.12.15 at 12:48, <haozhong.zhang@xxxxxxxxx> wrote:
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -4605,6 +4605,9 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, 
> > unsigned int *ebx,
> >  
> >              if ( !cpu_has_clwb )
> >                  *ebx &= ~cpufeat_mask(X86_FEATURE_CLWB);
> > +
> > +            if ( !cpu_has_pcommit )
> > +                *ebx &= ~cpufeat_mask(X86_FEATURE_PCOMMIT);
> 
> Other than for patch 1, this not only need to stay, but needs to be
> extended along the lines of X86_FEATURE_MPX handling.
>

In section "PCOMMIT - Virtualization Support" of Intel Architecture
Instruction Set Extensions Programming Reference, it says

    IA32_VMX_PROCBASED_CTLS2[53] (which enumerates support for the
    1-setting of âPCOMMIT exitingâ) is always the same as
    CPUID.07H:EBX.PCOMMIT[bit 22].

so checking cpu_has_pcommit is enough here.

> > @@ -1075,6 +1076,9 @@ static int construct_vmcs(struct vcpu *v)
> >          __vmwrite(PLE_WINDOW, ple_window);
> >      }
> >  
> > +    if ( cpu_has_vmx_pcommit )
> > +        v->arch.hvm_vmx.secondary_exec_control &= ~SECONDARY_EXEC_PCOMMIT;
> 
> Why is this conditional? Instead of the if() there should be a comment
> imo.
>
The condition is not necessary. I'll remove it and add a comment in
the next version.

Haozhong

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