From patchwork Mon Oct 10 15:28:47 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?UmFkaW0gS3LEjW3DocWZ?= X-Patchwork-Id: 9369681 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 842A06048F for ; Mon, 10 Oct 2016 15:58:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 71B21293CB for ; Mon, 10 Oct 2016 15:58:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6688F29435; Mon, 10 Oct 2016 15:58:27 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id C0806293CB for ; Mon, 10 Oct 2016 15:58:26 +0000 (UTC) Received: from localhost ([::1]:50983 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btcy9-0006uM-Pc for patchwork-qemu-devel@patchwork.kernel.org; Mon, 10 Oct 2016 11:58:25 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60993) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btcVz-0000NS-SL for qemu-devel@nongnu.org; Mon, 10 Oct 2016 11:29:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1btcVy-00005T-2I for qemu-devel@nongnu.org; Mon, 10 Oct 2016 11:29:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53922) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btcVx-00005M-Pd for qemu-devel@nongnu.org; Mon, 10 Oct 2016 11:29:18 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5AC294DD6F; Mon, 10 Oct 2016 15:29:17 +0000 (UTC) Received: from potion (dhcp-1-125.brq.redhat.com [10.34.1.125]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with SMTP id u9AFTEOA012983; Mon, 10 Oct 2016 11:29:14 -0400 Received: by potion (sSMTP sendmail emulation); Mon, 10 Oct 2016 17:29:14 +0200 From: =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= To: qemu-devel@nongnu.org Date: Mon, 10 Oct 2016 17:28:47 +0200 Message-Id: <20161010152848.17902-7-rkrcmar@redhat.com> In-Reply-To: <20161010152848.17902-1-rkrcmar@redhat.com> References: <20161010152848.17902-1-rkrcmar@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Mon, 10 Oct 2016 15:29:17 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v5 6/7] intel_iommu: reject broken EIM X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Eduardo Habkost , "Michael S. Tsirkin" , Peter Xu , Paolo Bonzini , Igor Mammedov , Richard Henderson Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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 --- v5: * squash with the patch that added compat property: v5: rename property to x-buggy-eim [Eduardo and Michael] v4: * use a device property [Igor] * clarify the last sentence of the commit message v3: shorten the code [Peter] * no one gave r-b to both patches, so all were dropped * beautify a nested condition in vtd_decide_config() [Peter] v4: be more specific in the comment [Igor] v3: * use error_setg [Paolo] * shorten the code [Peter] v2: * adapt to new intr_eim parameter * provide first linux version that has x2apic api * disable QEMU's LAPIC --- 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(-) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 17892b8c336b..c8dcdf965025 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -33,6 +33,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 @@ -2014,6 +2015,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(), }; @@ -2472,9 +2474,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 46412b229a70..f9aa85d7ac42 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -10,6 +10,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 b5ac60927b1f..1989c1eec10a 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 cdf15061091d..bda4dc2f0c57 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 ee1f53e56953..0fd664648665 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 36407e0a5dc7..5c369b1c5b40 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