This patch looks good.
I don't believe you need to explicitly check virtualize x2apic mode,
given that `vcpu->arch.apic_base & X2APIC_ENABLE &&
kvm_vcpu_apicv_active(vcpu)` implies that virtualize x2apic mode is
enabled (because KVM currently never re-enables apicv after disabling
it with the SyncIC), but being explicit is probably easier to
maintain.
On Wed, May 18, 2016 at 7:48 AM, Roman Kagan <rkagan@xxxxxxxxxxxxx> wrote:
> 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 not to be
> triggered, since Windows didn't use x2APIC but rather their own
> synthetic APIC access MSRs; however it represented a security risk
> because the guest running in a SynIC-enabled VM could switch to x2APIC
> and thus obtain direct access to host APIC MSRs (thanks to Yang Zhang
> <yang.zhang.wz@xxxxxxxxx> for spotting this).
>
> The patch fixes those omissions.
>
> Signed-off-by: Roman Kagan <rkagan@xxxxxxxxxxxxx>
> Cc: Steve Rutherford <srutherford@xxxxxxxxxx>
> Cc: Yang Zhang <yang.zhang.wz@xxxxxxxxx>
> ---
> v2 -> v3:
> - only switch to x2apic msr bitmap if virtualize x2apic mode is on in vmcs
>
> 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 | 48 ++++++++++++++++++++++++++++++------------------
> 1 file changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index ee1c8a9..cef741a 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2418,7 +2418,9 @@ 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 (cpu_has_secondary_exec_ctrls() &&
> + (vmcs_read32(SECONDARY_VM_EXEC_CONTROL) &
> + SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE)) {
> if (is_long_mode(vcpu))
> msr_bitmap = vmx_msr_bitmap_longmode_x2apic;
> else
> @@ -4783,6 +4785,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 +6344,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,
> --
> 2.5.5
>
--
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
