From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53933) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bpdrS-0003Xz-Tr for qemu-devel@nongnu.org; Thu, 29 Sep 2016 12:07:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bpdrO-0007jk-KI for qemu-devel@nongnu.org; Thu, 29 Sep 2016 12:07:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47788) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bpdrO-0007it-93 for qemu-devel@nongnu.org; Thu, 29 Sep 2016 12:06:58 -0400 Date: Thu, 29 Sep 2016 18:06:54 +0200 From: Igor Mammedov Message-ID: <20160929180654.782b73c1@nial.brq.redhat.com> In-Reply-To: References: <20160929112329.2408-1-rkrcmar@redhat.com> <20160929112329.2408-7-rkrcmar@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 6/7] intel_iommu: reject broken EIM List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Radim =?UTF-8?B?S3LEjW3DocWZ?= , qemu-devel@nongnu.org, Peter Xu , Richard Henderson , Eduardo Habkost , "Michael S. Tsirkin" On Thu, 29 Sep 2016 15:18:36 +0200 Paolo Bonzini wrote: > On 29/09/2016 13:23, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: > > 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. > >=20 > > 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. > >=20 > > Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 > > --- > > 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 +++++++++++++++- > > target-i386/kvm-stub.c | 5 +++++ > > target-i386/kvm.c | 13 +++++++++++++ > > target-i386/kvm_i386.h | 1 + > > 4 files changed, 34 insertions(+), 1 deletion(-) > >=20 > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > > index 47141cea64f4..b1afe5b133e0 100644 > > --- a/hw/i386/intel_iommu.c > > +++ b/hw/i386/intel_iommu.c > > @@ -32,6 +32,7 @@ > > #include "hw/pci-host/q35.h" > > #include "sysemu/kvm.h" > > #include "hw/i386/apic_internal.h" > > +#include "kvm_i386.h" > > =20 > > /*#define DEBUG_INTEL_IOMMU*/ > > #ifdef DEBUG_INTEL_IOMMU > > @@ -2481,7 +2482,20 @@ static void vtd_realize(DeviceState *dev, Error = **errp) > > s->intr_eim =3D ON_OFF_AUTO_OFF; > > } > > if (s->intr_eim =3D=3D ON_OFF_AUTO_AUTO) { > > - s->intr_eim =3D ON_OFF_AUTO_ON; > > + s->intr_eim =3D kvm_irqchip_in_kernel() ? ON_OFF_AUTO_ON > > + : ON_OFF_AUTO_OFF; > > + } > > + if (s->intr_eim =3D=3D ON_OFF_AUTO_ON) { > > + if (kvm_irqchip_in_kernel() && !kvm_enable_x2apic()) { > > + error_report("intel-iommu,eim=3Don requires support on the= KVM side " > > + "(X2APIC_API, first shipped in v4.7)."); > > + exit(1); =20 >=20 > Please use error_setg and return instead (same in patches 4 and 5). Radim's version is consistent with error handling used throughout the file. If we are to use preferred error_setg() here that preceding cleanup patch is in order to convert error_reports to error_setg. >=20 > Paolo >=20 > > + } > > + if (!kvm_irqchip_in_kernel()) { > > + error_report("intel-iommu,eim=3Don requires " > > + "accel=3Dkvm,kernel-irqchip=3Dsplit."); this prints: -device intel-iommu,intremap=3Don,eim=3Don: intel-iommu,eim=3Don requires = accel=3Dkvm,kernel-irqchip=3Dsplit so 'intel-iommu,' not really needed, the same would happen with error_setg() > > + exit(1); > > + } > > } [...]