>>> On 11.04.18 at 10:38, <andrew.coop...@citrix.com> wrote: > On 11/04/2018 03:15, Tian, Kevin wrote: >>> From: Jan Beulich [mailto:jbeul...@suse.com] >>> Sent: Tuesday, April 10, 2018 4:44 PM >>> >>>>>> On 09.04.18 at 19:56, <andrew.coop...@citrix.com> 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.t...@intel.com> > > I tried MASK_EXTR/INSR to begin with, but the code was practically > illegible. The * and / trick is only easy to follow if you know your > bit hackary off by heart, and will go silently wrong if the constants > happen to change or the operands happen to be the wrong way around. > > I'm not a massive fan of the literal 1 either, but it was the most > obvious way I could find of expressing the transformation. > > Therefore, I'd prefer to keep the patch in this form unless there are > serious objections.
As said - with Kevin being fine with the patch as is, I clearly won't object to it going in without further changes. Jan _______________________________________________ Xen-devel mailing list Xenemail@example.com https://lists.xenproject.org/mailman/listinfo/xen-devel