Re: [Xen-devel] [PATCH for-4.11] x86/VT-x: Fix determination of EFER.LMA in vmcs_dump_vcpu()

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Tuesday, April 10, 2018 4:44 PM
> 
> >>> On 09.04.18 at 19:56, <andrew.cooper3@xxxxxxxxxx> wrote:
> > --- a/xen/arch/x86/hvm/vmx/vmcs.c
> > +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> > @@ -1788,7 +1788,10 @@ void vmcs_dump_vcpu(struct vcpu *v)
> >      vmentry_ctl = vmr32(VM_ENTRY_CONTROLS),
> >      vmexit_ctl = vmr32(VM_EXIT_CONTROLS);
> >      cr4 = vmr(GUEST_CR4);
> > -    efer = vmr(GUEST_EFER);
> > +
> > +    /* EFER.LMA is read as zero, and is loaded from vmentry_ctl on entry.
> */
> > +    BUILD_BUG_ON(VM_ENTRY_IA32E_MODE << 1 != EFER_LMA);
> > +    efer = vmr(GUEST_EFER) | ((vmentry_ctl & VM_ENTRY_IA32E_MODE)
> << 1);
> 
> I have to admit that - despite the BUILD_BUG_ON() - I dislike the
> literal 1 here, which would better be
> (_EFER_LMA - _VM_ENTRY_IA32E_MODE), albeit the latter doesn't
> exist, so perhaps
> 
>     efer = vmr(GUEST_EFER) | ((vmentry_ctl & VM_ENTRY_IA32E_MODE) *
> (EFER_LMA / VM_ENTRY_IA32E_MODE));
> 
> or the same expressed through MASK_EXTR() / MASK_INSR()? But
> it's the VMX maintainers to judge anyway.
> 

using 1 is fine to me, with intention well explained with BUILD_BUG_ON.
as long as BUILD_BUG_ON is still a valid usage, I'm OK with current one:

Acked-by: Kevin Tian <kevin.tian@xxxxxxxxx>

Thanks
kevin

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel