On 2016/5/17 18:36, Roman Kagan wrote:
During reviewing this patch, I think there is an secure issue in
upstream KVM since the 5c919412fe61c:
1. The enable_apicv is setting to 1 if the underling hardware has APICv
capability.
2. set the x2apic_read/write msr to disable the interception for most
x2apic read and TPR/EOI/SELF-IPI write.
3. vmx_set_virtual_x2apic_mode is called when guest enables the
x2apic,but kvm_vcpu_apicv_active() check may fail if synIc is
activated.This means the virtualize_x2apic_mode will not set even guest
enabling the x2apic.
4. In the following vmx_set_msr_bitmap, it will use the x2apic msr
bitmap without check whether virtualize_x2apic_mode is enabled and apicv
is enabled.
5. Then the access to the msr that doesn't intercept by hypervisor will
access the hardware directly.
The function to update APICv on/off state (in particular, to deactivate
it when enabling Hyper-V SynIC), used to be incomplete: it didn't adjust
APICv-related fields among secondary processor-based VM-execution
controls.
As a result, Windows 2012 guests would get stuck when SynIC-based
auto-EOI interrupt intersected with e.g. an IPI in the guest.
In addition, the MSR intercept bitmap wasn't updated to correspond to
whether "virtualize x2APIC mode" was enabled. [This path used to be
untriggerable, since Windows didn't use x2APIC but rather their own
synthetic APIC access MSRs.]
The patch fixes those omissions.
Signed-off-by: Roman Kagan <rkagan@xxxxxxxxxxxxx>
Cc: Steve Rutherford <srutherford@xxxxxxxxxx>
---
v1 -> v2:
- only update relevant bits in the secondary exec control
- update msr intercept bitmap (also make x2apic msr bitmap always
correspond to APICv)
arch/x86/kvm/vmx.c | 47 +++++++++++++++++++++++++++++------------------
1 file changed, 29 insertions(+), 18 deletions(-)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ee1c8a9..5f9701e 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2418,7 +2418,8 @@ static void vmx_set_msr_bitmap(struct kvm_vcpu *vcpu)
if (is_guest_mode(vcpu))
msr_bitmap = vmx_msr_bitmap_nested;
- else if (vcpu->arch.apic_base & X2APIC_ENABLE) {
+ else if (vcpu->arch.apic_base & X2APIC_ENABLE &&
+ kvm_vcpu_apicv_active(vcpu)) {
Need to check the virtualize_x2apic_mode is enabled or not.
if (is_long_mode(vcpu))
msr_bitmap = vmx_msr_bitmap_longmode_x2apic;
else
@@ -4783,6 +4784,19 @@ static void vmx_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu)
struct vcpu_vmx *vmx = to_vmx(vcpu);
vmcs_write32(PIN_BASED_VM_EXEC_CONTROL, vmx_pin_based_exec_ctrl(vmx));
+ if (cpu_has_secondary_exec_ctrls()) {
+ if (kvm_vcpu_apicv_active(vcpu))
+ vmcs_set_bits(SECONDARY_VM_EXEC_CONTROL,
+ SECONDARY_EXEC_APIC_REGISTER_VIRT |
+ SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY);
+ else
+ vmcs_clear_bits(SECONDARY_VM_EXEC_CONTROL,
+ SECONDARY_EXEC_APIC_REGISTER_VIRT |
+ SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY);
+ }
+
+ if (cpu_has_vmx_msr_bitmap())
+ vmx_set_msr_bitmap(vcpu);
}
static u32 vmx_exec_control(struct vcpu_vmx *vmx)
@@ -6329,23 +6343,20 @@ static __init int hardware_setup(void)
set_bit(0, vmx_vpid_bitmap); /* 0 is reserved for host */
- if (enable_apicv) {
- for (msr = 0x800; msr <= 0x8ff; msr++)
- vmx_disable_intercept_msr_read_x2apic(msr);
-
- /* According SDM, in x2apic mode, the whole id reg is used.
- * But in KVM, it only use the highest eight bits. Need to
- * intercept it */
- vmx_enable_intercept_msr_read_x2apic(0x802);
- /* TMCCT */
- vmx_enable_intercept_msr_read_x2apic(0x839);
- /* TPR */
- vmx_disable_intercept_msr_write_x2apic(0x808);
- /* EOI */
- vmx_disable_intercept_msr_write_x2apic(0x80b);
- /* SELF-IPI */
- vmx_disable_intercept_msr_write_x2apic(0x83f);
- }
+ for (msr = 0x800; msr <= 0x8ff; msr++)
+ vmx_disable_intercept_msr_read_x2apic(msr);
+
+ /* According SDM, in x2apic mode, the whole id reg is used. But in
+ * KVM, it only use the highest eight bits. Need to intercept it */
+ vmx_enable_intercept_msr_read_x2apic(0x802);
+ /* TMCCT */
+ vmx_enable_intercept_msr_read_x2apic(0x839);
+ /* TPR */
+ vmx_disable_intercept_msr_write_x2apic(0x808);
+ /* EOI */
+ vmx_disable_intercept_msr_write_x2apic(0x80b);
+ /* SELF-IPI */
+ vmx_disable_intercept_msr_write_x2apic(0x83f);
if (enable_ept) {
kvm_mmu_set_mask_ptes(0ull,
--
best regards
yang
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
