Question about VPID during MOV-TO-CR3

Commit Message

Razvan Cojocaru Sept. 23, 2016, 8:35 a.m. UTC

On 09/23/16 11:24, Jan Beulich wrote:
>>>> On 22.09.16 at 19:18, <[e-mail]> wrote:
>> So I verified that when CPU-based load exiting is enabled, the TLB
>> flush here is critical. Without it the guest kernel crashes at random
>> points during boot. OTOH why does Xen trap every guest CR3 update
>> unconditionally? While we have features such as the vm_event/monitor
>> that may choose to subscribe to that event, Xen traps it even when
>> that is not in use. Is that trapping necessary for something else?
> 
> Where do you see this being unconditional? construct_vmcs()
> clearly avoids setting these intercepts when using EPT. Are you
> perhaps suffering from
> 
>             /* Trap CR3 updates if CR3 memory events are enabled. */
>             if ( v->domain->arch.monitor.write_ctrlreg_enabled &
>                  monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
>                 v->arch.hvm_vmx.exec_control |= CPU_BASED_CR3_LOAD_EXITING;
> 
> in vmx_update_guest_cr()? That'll be rather something for you
> or Razvan to explain. Outside of nested VMX I don't see any
> other enabling of that intercept (didn't check AMD code on the
> assumption that you're working on Intel hardware).

I did touch that line, but that was mostly a mechanical change in commit
712bdd01:

             vmx_update_cpu_exec_control(v);
@@ -2010,7 +2012,7 @@ static int vmx_cr_access(unsigned long
exit_qualification)
         unsigned long old = curr->arch.hvm_vcpu.guest_cr[0];
         curr->arch.hvm_vcpu.guest_cr[0] &= ~X86_CR0_TS;
         vmx_update_guest_cr(curr, 0);
-        hvm_event_cr0(curr->arch.hvm_vcpu.guest_cr[0], old);
+        hvm_event_crX(CR0, curr->arch.hvm_vcpu.guest_cr[0], old);
         HVMTRACE_0D(CLTS);
         break;
     }

The basic logic has remained untouched. The logic has been added in
commit df402bb9, by Joe Epstein. It's of course open to debate.


Thanks,
Razvan

Patch

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 74f563f..af257db 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -57,6 +57,7 @@ 
 #include <asm/apic.h>
 #include <asm/hvm/nestedhvm.h>
 #include <asm/event.h>
+#include <asm/monitor.h>
 #include <public/arch-x86/cpuid.h>

 static bool_t __initdata opt_force_ept;
@@ -1262,7 +1263,8 @@  static void vmx_update_guest_cr(struct vcpu *v,
unsigned int cr)
                 v->arch.hvm_vmx.exec_control |= cr3_ctls;

             /* Trap CR3 updates if CR3 memory events are enabled. */
-            if ( v->domain->arch.monitor.mov_to_cr3_enabled )
+            if ( v->domain->arch.monitor.write_ctrlreg_enabled &
+                 monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
                 v->arch.hvm_vmx.exec_control |= CPU_BASED_CR3_LOAD_EXITING;