Skip to content

Commit 13a50c9

Browse files
fyin1lijinxia
authored andcommitted
hv: Explicitly trap VMXE and PCIDE bit for CR4 write
Now, we let guest own most CR4 bit. Which means guest handles whether the CR4 writting is invalid or not and GP injection if it's invalid writing. Two bits are exception here: we filter VMX and PCID feature to guest (which means they are supported on native). So we can't depends on guest to inject GP for these bits. Instead, we should explicitly trap these CR4 bits update and inject GP to guest from HV. Signed-off-by: Yin Fengwei <fengwei.yin@intel.com> Acked-by: Anthony Xu <anthony.xu@intel.com>
1 parent f0ef41c commit 13a50c9

File tree

2 files changed

+25
-16
lines changed

2 files changed

+25
-16
lines changed

hypervisor/arch/x86/vmx.c

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,23 @@ int vmx_write_cr3(struct vcpu *vcpu, uint64_t cr3)
505505
return 0;
506506
}
507507

508+
static bool is_cr4_write_valid(struct vcpu *vcpu, uint64_t cr4)
509+
{
510+
/* Check if guest try to set fixed to 0 bits or reserved bits */
511+
if ((cr4 & cr4_always_off_mask) != 0U)
512+
return false;
513+
514+
/* Do NOT support nested guest */
515+
if ((cr4 & CR4_VMXE) != 0UL)
516+
return false;
517+
518+
/* Do NOT support PCID in guest */
519+
if ((cr4 & CR4_PCIDE) != 0UL)
520+
return false;
521+
522+
return true;
523+
}
524+
508525
/*
509526
* Handling of CR4:
510527
* Assume "unrestricted guest" feature is supported by vmx.
@@ -529,10 +546,13 @@ int vmx_write_cr3(struct vcpu *vcpu, uint64_t cr3)
529546
* - PCE (8) Flexible to guest
530547
* - OSFXSR (9) Flexible to guest
531548
* - OSXMMEXCPT (10) Flexible to guest
532-
* - VMXE (13) must always be 1 => must lead to a VM exit
549+
* - VMXE (13) Trapped to hide from guest
533550
* - SMXE (14) must always be 0 => must lead to a VM exit
534-
* - PCIDE (17) Flexible to guest
551+
* - PCIDE (17) Trapped to hide from guest
535552
* - OSXSAVE (18) Flexible to guest
553+
* - XSAVE (19) Flexible to guest
554+
* We always keep align with physical cpu. So it's flexible to
555+
* guest
536556
* - SMEP (20) Flexible to guest
537557
* - SMAP (21) Flexible to guest
538558
* - PKE (22) Flexible to guest
@@ -543,19 +563,8 @@ int vmx_write_cr4(struct vcpu *vcpu, uint64_t cr4)
543563
&vcpu->arch_vcpu.contexts[vcpu->arch_vcpu.cur_context];
544564
uint64_t cr4_vmx;
545565

546-
/* TODO: Check all invalid guest statuses according to the change of
547-
* CR4, and inject a #GP to guest */
548-
549-
/* Check if guest try to set fixed to 0 bits or reserved bits */
550-
if((cr4 & cr4_always_off_mask) != 0U) {
551-
pr_err("Not allow to set reserved/always off bits for CR4");
552-
vcpu_inject_gp(vcpu, 0U);
553-
return 0;
554-
}
555-
556-
/* Do NOT support nested guest */
557-
if ((cr4 & CR4_VMXE) != 0UL) {
558-
pr_err("Nested guest not supported");
566+
if (!is_cr4_write_valid(vcpu, cr4)) {
567+
pr_dbg("Invalid cr4 write operation from guest");
559568
vcpu_inject_gp(vcpu, 0U);
560569
return 0;
561570
}

hypervisor/include/arch/x86/vmx.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@
404404
CR0_NE | CR0_ET | CR0_TS | CR0_EM | CR0_MP | CR0_PE)
405405

406406
/* CR4 bits hv want to trap to track status change */
407-
#define CR4_TRAP_MASK (CR4_PSE | CR4_PAE)
407+
#define CR4_TRAP_MASK (CR4_PSE | CR4_PAE | CR4_VMXE | CR4_PCIDE)
408408
#define CR4_RESERVED_MASK ~(CR4_VME | CR4_PVI | CR4_TSD | CR4_DE | CR4_PSE | \
409409
CR4_PAE | CR4_MCE | CR4_PGE | CR4_PCE | \
410410
CR4_OSFXSR | CR4_PCIDE | CR4_OSXSAVE | \

0 commit comments

Comments
 (0)