x86/kvm/lapic: always disable MMIO interface in x2APIC mode

When VMX is used with flexpriority disabled (because of no support or
if disabled with module parameter) MMIO interface to lAPIC is still
available in x2APIC mode while it shouldn't be (kvm-unit-tests):

PASS: apic_disable: Local apic enabled in x2APIC mode
PASS: apic_disable: CPUID.1H:EDX.APIC[bit 9] is set
FAIL: apic_disable: *0xfee00030: 50014

The issue appears because we basically do nothing while switching to
x2APIC mode when APIC access page is not used. apic_mmio_{read,write}
only check if lAPIC is disabled before proceeding to actual write.

When APIC access is virtualized we correctly manipulate with VMX controls
in vmx_set_virtual_apic_mode() and we don't get vmexits from memory writes
in x2APIC mode so there's no issue.

Disabling MMIO interface seems to be easy. The question is: what do we
do with these reads and writes? If we add apic_x2apic_mode() check to
apic_mmio_in_range() and return -EOPNOTSUPP these reads and writes will
go to userspace. When lAPIC is in kernel, Qemu uses this interface to
inject MSIs only (see kvm_apic_mem_write() in hw/i386/kvm/apic.c). This
somehow works with disabled lAPIC but when we're in xAPIC mode we will
get a real injected MSI from every write to lAPIC. Not good.

The simplest solution seems to be to just ignore writes to the region
and return ~0 for all reads when we're in x2APIC mode. This is what this
patch does. However, this approach is inconsistent with what currently
happens when flexpriority is enabled: we allocate APIC access page and
create KVM memory region so in x2APIC modes all reads and writes go to
this pre-allocated page which is, btw, the same for all vCPUs.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 86299efa804adbfc35d3338f9fe75083e6cdf5af..fd23d5778ea17f04e4c1b178fbcb2ffb3718dec0 100644 (file)
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -377,6 +377,7 @@ struct kvm_sync_regs {
 
 #define KVM_X86_QUIRK_LINT0_REENABLED  (1 << 0)
 #define KVM_X86_QUIRK_CD_NW_CLEARED    (1 << 1)
+#define KVM_X86_QUIRK_LAPIC_MMIO_HOLE  (1 << 2)
 
 #define KVM_STATE_NESTED_GUEST_MODE    0x00000001
 #define KVM_STATE_NESTED_RUN_PENDING   0x00000002

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 17c0472c5b344faaaac3153ff53c4600a3fcd81e..fbb0e6df121b2f3d8c3a4d89c43e8cac3ca694e8 100644 (file)
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1344,9 +1344,8 @@ EXPORT_SYMBOL_GPL(kvm_lapic_reg_read);
 
 static int apic_mmio_in_range(struct kvm_lapic *apic, gpa_t addr)
 {
-       return kvm_apic_hw_enabled(apic) &&
-           addr >= apic->base_address &&
-           addr < apic->base_address + LAPIC_MMIO_LENGTH;
+       return addr >= apic->base_address &&
+               addr < apic->base_address + LAPIC_MMIO_LENGTH;
 }
 
 static int apic_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
@@ -1358,6 +1357,15 @@ static int apic_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
        if (!apic_mmio_in_range(apic, address))
                return -EOPNOTSUPP;
 
+       if (!kvm_apic_hw_enabled(apic) || apic_x2apic_mode(apic)) {
+               if (!kvm_check_has_quirk(vcpu->kvm,
+                                        KVM_X86_QUIRK_LAPIC_MMIO_HOLE))
+                       return -EOPNOTSUPP;
+
+               memset(data, 0xff, len);
+               return 0;
+       }
+
        kvm_lapic_reg_read(apic, offset, len, data);
 
        return 0;
@@ -1917,6 +1925,14 @@ static int apic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
        if (!apic_mmio_in_range(apic, address))
                return -EOPNOTSUPP;
 
+       if (!kvm_apic_hw_enabled(apic) || apic_x2apic_mode(apic)) {
+               if (!kvm_check_has_quirk(vcpu->kvm,
+                                        KVM_X86_QUIRK_LAPIC_MMIO_HOLE))
+                       return -EOPNOTSUPP;
+
+               return 0;
+       }
+
        /*
         * APIC register must be aligned on 128-bits boundary.
         * 32/64/128 bits registers must be accessed thru 32 bits.