From 2f114315dcf239bc513f18ae0b04b5df81cae059 Mon Sep 17 00:00:00 2001 From: Radim Krčmář Date: Mon, 10 Oct 2016 17:28:42 +0200 Subject: apic: add global apic_get_class() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every configuration has only up to one APIC class and we'll be extending the class with a function that can be called without an instanced object, so a direct access to the class is convenient. This patch will break compilation if some code uses apic_get_class() with CONFIG_USER_ONLY. Suggested-by: Eduardo Habkost Reviewed-by: Eduardo Habkost Reviewed-by: Peter Xu Signed-off-by: Radim Krčmář Signed-off-by: Eduardo Habkost --- include/hw/i386/apic_internal.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include') diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h index 06c4e9f6f9..286684857e 100644 --- a/include/hw/i386/apic_internal.h +++ b/include/hw/i386/apic_internal.h @@ -222,4 +222,6 @@ static inline int apic_get_bit(uint32_t *tab, int index) return !!(tab[i] & mask); } +APICCommonClass *apic_get_class(void); + #endif /* QEMU_APIC_INTERNAL_H */ -- cgit 1.4.1 From 267ee357153bb61870b60da69ba9f839ddb0e32e Mon Sep 17 00:00:00 2001 From: Radim Krčmář Date: Mon, 10 Oct 2016 17:28:43 +0200 Subject: apic: add send_msi() to APICCommonClass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MMIO based interface to APIC doesn't work well with MSIs that have upper address bits set (remapped x2APIC MSIs). A specialized interface is a quick and dirty way to avoid the shortcoming. Reviewed-by: Igor Mammedov Reviewed-by: Peter Xu Signed-off-by: Radim Krčmář Signed-off-by: Eduardo Habkost --- hw/i386/kvm/apic.c | 19 +++++++++++++------ hw/i386/xen/xen_apic.c | 6 ++++++ hw/intc/apic.c | 8 ++++++-- include/hw/i386/apic_internal.h | 4 ++++ 4 files changed, 29 insertions(+), 8 deletions(-) (limited to 'include') diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c index c016e63fc2..be55102c00 100644 --- a/hw/i386/kvm/apic.c +++ b/hw/i386/kvm/apic.c @@ -169,6 +169,17 @@ static void kvm_apic_external_nmi(APICCommonState *s) run_on_cpu(CPU(s->cpu), do_inject_external_nmi, s); } +static void kvm_send_msi(MSIMessage *msg) +{ + int ret; + + ret = kvm_irqchip_send_msi(kvm_state, *msg); + if (ret < 0) { + fprintf(stderr, "KVM: injection failed, MSI lost (%s)\n", + strerror(-ret)); + } +} + static uint64_t kvm_apic_mem_read(void *opaque, hwaddr addr, unsigned size) { @@ -179,13 +190,8 @@ static void kvm_apic_mem_write(void *opaque, hwaddr addr, uint64_t data, unsigned size) { MSIMessage msg = { .address = addr, .data = data }; - int ret; - ret = kvm_irqchip_send_msi(kvm_state, msg); - if (ret < 0) { - fprintf(stderr, "KVM: injection failed, MSI lost (%s)\n", - strerror(-ret)); - } + kvm_send_msi(&msg); } static const MemoryRegionOps kvm_apic_io_ops = { @@ -232,6 +238,7 @@ static void kvm_apic_class_init(ObjectClass *klass, void *data) k->enable_tpr_reporting = kvm_apic_enable_tpr_reporting; k->vapic_base_update = kvm_apic_vapic_base_update; k->external_nmi = kvm_apic_external_nmi; + k->send_msi = kvm_send_msi; } static const TypeInfo kvm_apic_info = { diff --git a/hw/i386/xen/xen_apic.c b/hw/i386/xen/xen_apic.c index 21d68ee04b..55769eba7e 100644 --- a/hw/i386/xen/xen_apic.c +++ b/hw/i386/xen/xen_apic.c @@ -68,6 +68,11 @@ static void xen_apic_external_nmi(APICCommonState *s) { } +static void xen_send_msi(MSIMessage *msi) +{ + xen_hvm_inject_msi(msi->address, msi->data); +} + static void xen_apic_class_init(ObjectClass *klass, void *data) { APICCommonClass *k = APIC_COMMON_CLASS(klass); @@ -78,6 +83,7 @@ static void xen_apic_class_init(ObjectClass *klass, void *data) k->get_tpr = xen_apic_get_tpr; k->vapic_base_update = xen_apic_vapic_base_update; k->external_nmi = xen_apic_external_nmi; + k->send_msi = xen_send_msi; } static const TypeInfo xen_apic_info = { diff --git a/hw/intc/apic.c b/hw/intc/apic.c index 7bd1d279c4..fe15fb6024 100644 --- a/hw/intc/apic.c +++ b/hw/intc/apic.c @@ -740,8 +740,10 @@ static uint32_t apic_mem_readl(void *opaque, hwaddr addr) return val; } -static void apic_send_msi(hwaddr addr, uint32_t data) +static void apic_send_msi(MSIMessage *msi) { + uint64_t addr = msi->address; + uint32_t data = msi->data; uint8_t dest = (addr & MSI_ADDR_DEST_ID_MASK) >> MSI_ADDR_DEST_ID_SHIFT; uint8_t vector = (data & MSI_DATA_VECTOR_MASK) >> MSI_DATA_VECTOR_SHIFT; uint8_t dest_mode = (addr >> MSI_ADDR_DEST_MODE_SHIFT) & 0x1; @@ -762,7 +764,8 @@ static void apic_mem_writel(void *opaque, hwaddr addr, uint32_t val) * APIC is connected directly to the CPU. * Mapping them on the global bus happens to work because * MSI registers are reserved in APIC MMIO and vice versa. */ - apic_send_msi(addr, val); + MSIMessage msi = { .address = addr, .data = val }; + apic_send_msi(&msi); return; } @@ -913,6 +916,7 @@ static void apic_class_init(ObjectClass *klass, void *data) k->external_nmi = apic_external_nmi; k->pre_save = apic_pre_save; k->post_load = apic_post_load; + k->send_msi = apic_send_msi; } static const TypeInfo apic_info = { diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h index 286684857e..cdd11fb093 100644 --- a/include/hw/i386/apic_internal.h +++ b/include/hw/i386/apic_internal.h @@ -146,6 +146,10 @@ typedef struct APICCommonClass void (*pre_save)(APICCommonState *s); void (*post_load)(APICCommonState *s); void (*reset)(APICCommonState *s); + /* send_msi emulates an APIC bus and its proper place would be in a new + * device, but it's convenient to have it here for now. + */ + void (*send_msi)(MSIMessage *msi); } APICCommonClass; struct APICCommonState { -- cgit 1.4.1 From e6b6af05607a8bc828c454f6830b5fc68e5a9ac1 Mon Sep 17 00:00:00 2001 From: Radim Krčmář Date: Mon, 10 Oct 2016 17:28:46 +0200 Subject: intel_iommu: add OnOffAuto intr_eim as "eim" property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The default (auto) emulates the current behavior. A user can now control EIM like -device intel-iommu,intremap=on,eim=off Reviewed-by: Igor Mammedov Reviewed-by: Peter Xu Signed-off-by: Radim Krčmář Signed-off-by: Eduardo Habkost --- hw/i386/intel_iommu.c | 24 +++++++++++++++++++++--- include/hw/i386/intel_iommu.h | 1 + 2 files changed, 22 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 29cd2ae51d..a70aa8494e 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -2013,6 +2013,8 @@ static const MemoryRegionOps vtd_mem_ops = { static Property vtd_properties[] = { DEFINE_PROP_UINT32("version", IntelIOMMUState, version, 0), + DEFINE_PROP_ON_OFF_AUTO("eim", IntelIOMMUState, intr_eim, + ON_OFF_AUTO_AUTO), DEFINE_PROP_END_OF_LIST(), }; @@ -2380,7 +2382,11 @@ static void vtd_init(IntelIOMMUState *s) s->ecap = VTD_ECAP_QI | VTD_ECAP_IRO; if (x86_iommu->intr_supported) { - s->ecap |= VTD_ECAP_IR | VTD_ECAP_EIM | VTD_ECAP_MHMV; + s->ecap |= VTD_ECAP_IR | VTD_ECAP_MHMV; + if (s->intr_eim == ON_OFF_AUTO_ON) { + s->ecap |= VTD_ECAP_EIM; + } + assert(s->intr_eim != ON_OFF_AUTO_AUTO); } vtd_reset_context_cache(s); @@ -2461,8 +2467,10 @@ static AddressSpace *vtd_host_dma_iommu(PCIBus *bus, void *opaque, int devfn) return &vtd_as->as; } -static bool vtd_check_config(X86IOMMUState *x86_iommu, Error **errp) +static bool vtd_decide_config(IntelIOMMUState *s, Error **errp) { + X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s); + /* Currently Intel IOMMU IR only support "kernel-irqchip={off|split}" */ if (x86_iommu->intr_supported && kvm_irqchip_in_kernel() && !kvm_irqchip_is_split()) { @@ -2470,6 +2478,16 @@ static bool vtd_check_config(X86IOMMUState *x86_iommu, Error **errp) "kernel-irqchip=on, please use 'split|off'."); return false; } + if (s->intr_eim == ON_OFF_AUTO_ON && !x86_iommu->intr_supported) { + error_setg(errp, "eim=on cannot be selected without intremap=on"); + return false; + } + + if (s->intr_eim == ON_OFF_AUTO_AUTO) { + s->intr_eim = x86_iommu->intr_supported ? + ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; + } + return true; } @@ -2483,7 +2501,7 @@ static void vtd_realize(DeviceState *dev, Error **errp) VTD_DPRINTF(GENERAL, ""); x86_iommu->type = TYPE_INTEL; - if (!vtd_check_config(x86_iommu, errp)) { + if (!vtd_decide_config(s, errp)) { return; } diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h index a42dbd745a..b5ac60927b 100644 --- a/include/hw/i386/intel_iommu.h +++ b/include/hw/i386/intel_iommu.h @@ -289,6 +289,7 @@ struct IntelIOMMUState { dma_addr_t intr_root; /* Interrupt remapping table pointer */ uint32_t intr_size; /* Number of IR table entries */ bool intr_eime; /* Extended interrupt mode enabled */ + OnOffAuto intr_eim; /* Toggle for EIM cabability */ }; /* Find the VTD Address space associated with the given bus pointer, -- cgit 1.4.1 From fb506e701e9bafa3e0685747c1c98962c52d1962 Mon Sep 17 00:00:00 2001 From: Radim Krčmář Date: Mon, 10 Oct 2016 17:28:47 +0200 Subject: intel_iommu: reject broken EIM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cluster x2APIC cannot work without KVM's x2apic API when the maximal APIC ID is greater than 8 and only KVM's LAPIC can support x2APIC, so we forbid other APICs and also the old KVM case with less than 9, to simplify the code. There is no point in enabling EIM in forbidden APICs, so we keep it enabled only for the KVM APIC; unconditionally, because making the option depend on KVM version would be a maintanance burden. Old QEMUs would enable eim whenever intremap was on, which would trick guests into thinking that they can enable cluster x2APIC even if any interrupt destination would get clamped to 8 bits. Depending on your configuration, QEMU could notice that the destination LAPIC is not present and report it with a very non-obvious: KVM: injection failed, MSI lost (Operation not permitted) Or the guest could say something about unexpected interrupts, because clamping leads to aliasing so interrupts were being delivered to incorrect VCPUs. KVM_X2APIC_API is the feature that allows us to enable EIM for KVM. QEMU 2.7 allowed EIM whenever interrupt remapping was enabled. In order to keep backward compatibility, we again allow guests to misbehave in non-obvious ways, and make it the default for old machine types. A user can enable the buggy mode it with "x-buggy-eim=on". Signed-off-by: Radim Krčmář Reviewed-by: Eduardo Habkost Reviewed-by: Peter Xu Signed-off-by: Eduardo Habkost --- hw/i386/intel_iommu.c | 16 +++++++++++++++- include/hw/compat.h | 4 ++++ include/hw/i386/intel_iommu.h | 1 + target-i386/kvm-stub.c | 5 +++++ target-i386/kvm.c | 13 +++++++++++++ target-i386/kvm_i386.h | 1 + 6 files changed, 39 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index a70aa8494e..1655a65bce 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -34,6 +34,7 @@ #include "hw/pci-host/q35.h" #include "sysemu/kvm.h" #include "hw/i386/apic_internal.h" +#include "kvm_i386.h" /*#define DEBUG_INTEL_IOMMU*/ #ifdef DEBUG_INTEL_IOMMU @@ -2015,6 +2016,7 @@ static Property vtd_properties[] = { DEFINE_PROP_UINT32("version", IntelIOMMUState, version, 0), DEFINE_PROP_ON_OFF_AUTO("eim", IntelIOMMUState, intr_eim, ON_OFF_AUTO_AUTO), + DEFINE_PROP_BOOL("x-buggy-eim", IntelIOMMUState, buggy_eim, false), DEFINE_PROP_END_OF_LIST(), }; @@ -2484,9 +2486,21 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp) } if (s->intr_eim == ON_OFF_AUTO_AUTO) { - s->intr_eim = x86_iommu->intr_supported ? + s->intr_eim = (kvm_irqchip_in_kernel() || s->buggy_eim) + && x86_iommu->intr_supported ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; } + if (s->intr_eim == ON_OFF_AUTO_ON && !s->buggy_eim) { + if (!kvm_irqchip_in_kernel()) { + error_setg(errp, "eim=on requires accel=kvm,kernel-irqchip=split"); + return false; + } + if (!kvm_enable_x2apic()) { + error_setg(errp, "eim=on requires support on the KVM side" + "(X2APIC_API, first shipped in v4.7)"); + return false; + } + } return true; } diff --git a/include/hw/compat.h b/include/hw/compat.h index ef3fae3e1b..0f06e113be 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -14,6 +14,10 @@ .driver = "ioapic",\ .property = "version",\ .value = "0x11",\ + },{\ + .driver = "intel-iommu",\ + .property = "x-buggy-eim",\ + .value = "true",\ }, #define HW_COMPAT_2_6 \ diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h index b5ac60927b..1989c1eec1 100644 --- a/include/hw/i386/intel_iommu.h +++ b/include/hw/i386/intel_iommu.h @@ -290,6 +290,7 @@ struct IntelIOMMUState { uint32_t intr_size; /* Number of IR table entries */ bool intr_eime; /* Extended interrupt mode enabled */ OnOffAuto intr_eim; /* Toggle for EIM cabability */ + bool buggy_eim; /* Force buggy EIM unless eim=off */ }; /* Find the VTD Address space associated with the given bus pointer, diff --git a/target-i386/kvm-stub.c b/target-i386/kvm-stub.c index cdf1506109..bda4dc2f0c 100644 --- a/target-i386/kvm-stub.c +++ b/target-i386/kvm-stub.c @@ -25,6 +25,11 @@ bool kvm_has_smm(void) return 1; } +bool kvm_enable_x2apic(void) +{ + return false; +} + /* This function is only called inside conditionals which we * rely on the compiler to optimize out when CONFIG_KVM is not * defined. diff --git a/target-i386/kvm.c b/target-i386/kvm.c index ee1f53e569..0fd6646486 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -122,6 +122,19 @@ bool kvm_allows_irq0_override(void) return !kvm_irqchip_in_kernel() || kvm_has_gsi_routing(); } +static bool kvm_x2apic_api_set_flags(uint64_t flags) +{ + KVMState *s = KVM_STATE(current_machine->accelerator); + + return !kvm_vm_enable_cap(s, KVM_CAP_X2APIC_API, 0, flags); +} + +bool kvm_enable_x2apic(void) +{ + return kvm_x2apic_api_set_flags(KVM_X2APIC_API_USE_32BIT_IDS | + KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK); +} + static int kvm_get_tsc(CPUState *cs) { X86CPU *cpu = X86_CPU(cs); diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h index 36407e0a5d..5c369b1c5b 100644 --- a/target-i386/kvm_i386.h +++ b/target-i386/kvm_i386.h @@ -43,4 +43,5 @@ int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id); void kvm_put_apicbase(X86CPU *cpu, uint64_t value); +bool kvm_enable_x2apic(void); #endif -- cgit 1.4.1