Re: [Xen-devel] [PATCH v3 2/6] x86/msr: add VMX MSRs into struct msr_domain_policy

[Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>]

On Mon, 2017-10-16 at 15:01 +0100, Andrew Cooper wrote:
> On 16/10/17 08:42, Sergey Dyasli wrote:
> > +
> > +    secondary_available =
> > +        dp->vmx_procbased_ctls.u.allowed_1.activate_secondary_controls;
> > +
> > +    switch (msr)
> > +    {
> > +    case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMCS_ENUM:
> > +        return true;
> > +
> > +    case MSR_IA32_VMX_PROCBASED_CTLS2:
> > +        return secondary_available;
> > +
> > +    case MSR_IA32_VMX_EPT_VPID_CAP:
> > +        return ( secondary_available &&
> > +                 (dp->vmx_procbased_ctls2.u.allowed_1.enable_ept ||
> > +                  dp->vmx_procbased_ctls2.u.allowed_1.enable_vpid) );
> 
> This check can be made more efficient in two ways.  First, use a bitwise
> rather than logical or, which allows both _ept and _vpid to be tested
> with a single instruction, rather than a conditional branch.

But it's compiler's job to optimize conditions like that.
I'm getting the following asm:

            if ( dp->vmx_procbased_ctls2.allowed_1.enable_ept ||
ffff82d08027bc3d:       48 c1 e8 20             shr    $0x20,%rax
ffff82d08027bc41:       a8 22                   test   $0x22,%al
ffff82d08027bc43:       74 0d                   je     ffff82d08027bc52 
<recalculate_domain_vmx_msr_policy+0x196>

And "test   $0x22" is exactly the test for "enable_ept || enable_vpid"
with a single instruction.

-- 
Thanks,
Sergey
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel