From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41138) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsAfb-0002B4-8z for qemu-devel@nongnu.org; Thu, 06 Oct 2016 11:33:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bsAfW-0006v2-9X for qemu-devel@nongnu.org; Thu, 06 Oct 2016 11:33:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48298) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsAfV-0006uo-WE for qemu-devel@nongnu.org; Thu, 06 Oct 2016 11:33:10 -0400 Date: Thu, 6 Oct 2016 18:33:07 +0300 From: "Michael S. Tsirkin" Message-ID: <20161006183130-mutt-send-email-mst@kernel.org> References: <20161005130657.3399-1-rkrcmar@redhat.com> <20161005130657.3399-8-rkrcmar@redhat.com> <20161006145142.GR3877@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20161006145142.GR3877@thinpad.lan.raisama.net> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 7/8] intel_iommu: keep buggy EIM enabled in 2.7 machine type List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Radim =?utf-8?B?S3LEjW3DocWZ?= , qemu-devel@nongnu.org, Peter Xu , Igor Mammedov , Paolo Bonzini , Richard Henderson On Thu, Oct 06, 2016 at 11:51:42AM -0300, Eduardo Habkost wrote: > On Wed, Oct 05, 2016 at 03:06:56PM +0200, Radim Kr=C4=8Dm=C3=A1=C5=99 w= rote: > > QEMU 2.7 allowed EIM even in configurations that were forbidden in th= e > > last patch because they were not working, like old KVM or userspace > > APIC. In order to keep backward compatibility, we again allow guests= to > > misbehave in non-obvious ways, and make it the default for old machin= e > > types. > >=20 > > A user can enable the buggy mode it with "buggy_eim=3Don", which is w= eird, > > but I don't know how to add a private property. >=20 > Properties et by compat_props are always user-visible. I believe > that's a feature (this way, it will be possible to let management > software and other tools know what exactly is behind a > machine-type). There's a rule that any name beginning with x- is deemed experimental. See docs/qmp-spec.txt. It is a good idea to always use this for compat properties as we want to be able to drop them when we drop the old machine type. > Additional comment below: >=20 > >=20 > > Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 > > --- > > v4: > > * use a device property [Igor] > > * clarify the last sentence of the commit message > > v3: shorten the code [Peter] > > --- > > hw/i386/intel_iommu.c | 7 ++++--- > > include/hw/compat.h | 4 ++++ > > include/hw/i386/intel_iommu.h | 1 + > > 3 files changed, 9 insertions(+), 3 deletions(-) > >=20 > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > > index efb018b85544..fe257e8357b4 100644 > > --- a/hw/i386/intel_iommu.c > > +++ b/hw/i386/intel_iommu.c > > @@ -2015,6 +2015,7 @@ static Property vtd_properties[] =3D { > > DEFINE_PROP_UINT32("version", IntelIOMMUState, version, 0), > > DEFINE_PROP_ON_OFF_AUTO("eim", IntelIOMMUState, intr_eim, > > ON_OFF_AUTO_AUTO), > > + DEFINE_PROP_BOOL("buggy_eim", IntelIOMMUState, buggy_eim, false)= , >=20 > I suggest "buggy-eim", to follow the usual style for QOM property > names. >=20 > Assuming the name gets changed: >=20 > Reviewed-by: Eduardo Habkost >=20 >=20 > > DEFINE_PROP_END_OF_LIST(), > > }; > > =20 > > @@ -2473,11 +2474,11 @@ static bool vtd_decide_config(IntelIOMMUState= *s, Error **errp) > > } > > =20 > > if (s->intr_eim =3D=3D ON_OFF_AUTO_AUTO) { > > - s->intr_eim =3D x86_iommu->intr_supported && kvm_irqchip_in_= kernel() ? > > + s->intr_eim =3D (kvm_irqchip_in_kernel() || s->buggy_eim) > > + && x86_iommu->intr_supported ? > > ON_OFF_AUTO_ON : ON_OF= F_AUTO_OFF; > > } > > - > > - if (s->intr_eim =3D=3D ON_OFF_AUTO_ON) { > > + if (s->intr_eim =3D=3D ON_OFF_AUTO_ON && !s->buggy_eim) { > > if (kvm_irqchip_in_kernel() && !kvm_enable_x2apic()) { > > error_setg(errp, "eim=3Don requires support on the KVM s= ide" > > "(X2APIC_API, first shipped in v4.7)"); > > diff --git a/include/hw/compat.h b/include/hw/compat.h > > index 46412b229a70..43b50065e082 100644 > > --- a/include/hw/compat.h > > +++ b/include/hw/compat.h > > @@ -10,6 +10,10 @@ > > .driver =3D "ioapic",\ > > .property =3D "version",\ > > .value =3D "0x11",\ > > + },{\ > > + .driver =3D "intel-iommu",\ > > + .property =3D "buggy_eim",\ > > + .value =3D "true",\ > > }, > > =20 > > #define HW_COMPAT_2_6 \ > > diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_io= mmu.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 enabl= ed */ > > OnOffAuto intr_eim; /* Toggle for EIM cabability */ > > + bool buggy_eim; /* Force buggy EIM unless eim=3D= off */ > > }; > > =20 > > /* Find the VTD Address space associated with the given bus pointer, > > --=20 > > 2.10.0 > >=20 >=20 > --=20 > Eduardo