>>> On 26.01.18 at 11:49, <rcojoc...@bitdefender.com> wrote:
> On 01/26/2018 12:27 PM, Jan Beulich wrote:
>>>>> On 26.01.18 at 10:39, <rcojoc...@bitdefender.com> wrote:
>>> --- a/xen/arch/x86/hvm/monitor.c
>>> +++ b/xen/arch/x86/hvm/monitor.c
>>> @@ -36,6 +36,12 @@ bool hvm_monitor_cr(unsigned int index, unsigned long
>>> value, unsigned long old)
>>> struct arch_domain *ad = &curr->domain->arch;
>>> unsigned int ctrlreg_bitmask = monitor_ctrlreg_bitmask(index);
>>>
>>> + if ( index == 3 && hvm_pcid_enabled(curr) ) /* Clear the noflush bit.
>>> */
>>> + {
>>> + value &= ((1ull << 63) - 1);
>>> + old &= ((1ull << 63) - 1);
>>
>> Why would "old" need clearing the bit? It being set during write
>> doesn't actually get stored into the register, aiui.
>
> v->arch.hvm_vcpu.guest_cr[3] (which is used for "old") does get set in a
> number of places:
>
> $ grep -IRn "hvm_vcpu.guest_cr\[3\] ="
> arch/x86/hvm/hvm.c:2343: v->arch.hvm_vcpu.guest_cr[3] = value;
> arch/x86/hvm/hvm.c:3914: v->arch.hvm_vcpu.guest_cr[3] = 0;
> arch/x86/hvm/vmx/vmx.c:790: v->arch.hvm_vcpu.guest_cr[3] = cr3;
> arch/x86/hvm/vmx/vmx.c:3522: v->arch.hvm_vcpu.guest_cr[3] =
> v->arch.hvm_vcpu.hw_cr[3];
> arch/x86/hvm/domain.c:207: v->arch.hvm_vcpu.guest_cr[3] = regs->cr3;
> arch/x86/hvm/domain.c:258: v->arch.hvm_vcpu.guest_cr[3] = regs->cr3;
> arch/x86/hvm/svm/svm.c:316: v->arch.hvm_vcpu.guest_cr[3] = c->cr3;
> arch/x86/hvm/svm/svm.c:2476: v->arch.hvm_vcpu.guest_cr[3] =
> v->arch.hvm_vcpu.hw_cr[3] =
>
> and I thought it's better to be safe than sorry.
As can be implied from the bit not being stored into the actual
register, we should either consistently store the bit and mask
it where needed or - following your patch - mask the bit before
storing a value. This latter approach also has the advantage
of properly causing failure when e.g. the incoming migration
stream has the bit set.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
