From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36948) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bpedN-0001z0-WD for qemu-devel@nongnu.org; Thu, 29 Sep 2016 12:56:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bpedK-0004h2-RS for qemu-devel@nongnu.org; Thu, 29 Sep 2016 12:56:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44866) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bpedK-0004gq-Hw for qemu-devel@nongnu.org; Thu, 29 Sep 2016 12:56:30 -0400 Date: Thu, 29 Sep 2016 18:56:26 +0200 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Message-ID: <20160929165626.GC12788@potion> References: <20160929112329.2408-1-rkrcmar@redhat.com> <20160929112329.2408-7-rkrcmar@redhat.com> <20160929180654.782b73c1@nial.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160929180654.782b73c1@nial.brq.redhat.com> 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: Igor Mammedov Cc: Paolo Bonzini , qemu-devel@nongnu.org, Peter Xu , Richard Henderson , Eduardo Habkost , "Michael S. Tsirkin" 2016-09-29 18:06+0200, Igor Mammedov: > 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, s= o 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 >> > --- >> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >> > @@ -2481,7 +2482,20 @@ static void vtd_realize(DeviceState *dev, Err= or **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 f= ile. > If we are to use preferred error_setg() here that preceding cleanup > patch is in order to convert error_reports to error_setg. There is one more error_report() in the file and it doesn't have "Error **" -- I'll leave it be and change the rest. It amounts to one extra patch that before [4/7] (could be squashed too). >> > + } >> > + 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 requi= res accel=3Dkvm,kernel-irqchip=3Dsplit > so 'intel-iommu,' not really needed, the same would happen with error_s= etg() Yeah, really unreadable, thanks.