diff mbox

[v4,7/8] intel_iommu: keep buggy EIM enabled in 2.7 machine type

Message ID 20161005130657.3399-8-rkrcmar@redhat.com
State New
Headers show

Commit Message

Radim Krčmář Oct. 5, 2016, 1:06 p.m. UTC
QEMU 2.7 allowed EIM even in configurations that were forbidden in the
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 machine
types.

A user can enable the buggy mode it with "buggy_eim=on", which is weird,
but I don't know how to add a private property.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
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(-)

Comments

Eduardo Habkost Oct. 6, 2016, 2:51 p.m. UTC | #1
On Wed, Oct 05, 2016 at 03:06:56PM +0200, Radim Krčmář wrote:
> QEMU 2.7 allowed EIM even in configurations that were forbidden in the
> 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 machine
> types.
> 
> A user can enable the buggy mode it with "buggy_eim=on", which is weird,
> but I don't know how to add a private property.

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).

Additional comment below:

> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
> 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(-)
> 
> 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[] = {
>      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),

I suggest "buggy-eim", to follow the usual style for QOM property
names.

Assuming the name gets changed:

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>


>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -2473,11 +2474,11 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
>      }
>  
>      if (s->intr_eim == ON_OFF_AUTO_AUTO) {
> -        s->intr_eim = x86_iommu->intr_supported && kvm_irqchip_in_kernel() ?
> +        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) {
> +    if (s->intr_eim == ON_OFF_AUTO_ON && !s->buggy_eim) {
>          if (kvm_irqchip_in_kernel() && !kvm_enable_x2apic()) {
>              error_setg(errp, "eim=on requires support on the KVM side"
>                               "(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   = "ioapic",\
>          .property = "version",\
>          .value    = "0x11",\
> +    },{\
> +        .driver   = "intel-iommu",\
> +        .property = "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,
> -- 
> 2.10.0
>
Michael S. Tsirkin Oct. 6, 2016, 3:33 p.m. UTC | #2
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čmář wrote:
> > QEMU 2.7 allowed EIM even in configurations that were forbidden in the
> > 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 machine
> > types.
> > 
> > A user can enable the buggy mode it with "buggy_eim=on", which is weird,
> > but I don't know how to add a private property.
> 
> 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:
> 
> > 
> > Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> > ---
> > 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(-)
> > 
> > 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[] = {
> >      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),
> 
> I suggest "buggy-eim", to follow the usual style for QOM property
> names.
> 
> Assuming the name gets changed:
> 
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> 
> >      DEFINE_PROP_END_OF_LIST(),
> >  };
> >  
> > @@ -2473,11 +2474,11 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
> >      }
> >  
> >      if (s->intr_eim == ON_OFF_AUTO_AUTO) {
> > -        s->intr_eim = x86_iommu->intr_supported && kvm_irqchip_in_kernel() ?
> > +        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) {
> > +    if (s->intr_eim == ON_OFF_AUTO_ON && !s->buggy_eim) {
> >          if (kvm_irqchip_in_kernel() && !kvm_enable_x2apic()) {
> >              error_setg(errp, "eim=on requires support on the KVM side"
> >                               "(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   = "ioapic",\
> >          .property = "version",\
> >          .value    = "0x11",\
> > +    },{\
> > +        .driver   = "intel-iommu",\
> > +        .property = "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,
> > -- 
> > 2.10.0
> > 
> 
> -- 
> Eduardo
Radim Krčmář Oct. 6, 2016, 3:55 p.m. UTC | #3
2016-10-06 18:33+0300, Michael S. Tsirkin:
> 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čmář wrote:
>> > QEMU 2.7 allowed EIM even in configurations that were forbidden in the
>> > 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 machine
>> > types.
>> > 
>> > A user can enable the buggy mode it with "buggy_eim=on", which is weird,
>> > but I don't know how to add a private property.
>> 
>> 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.

"x-buggy-eim" should deter most users, thanks.

pc-0.10 seems to be the first machine type ever (2009), is there already
a plan to deprecate it?
Radim Krčmář Oct. 6, 2016, 4 p.m. UTC | #4
2016-10-06 11:51-0300, Eduardo Habkost:
> On Wed, Oct 05, 2016 at 03:06:56PM +0200, Radim Krčmář wrote:
>> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
>> ---
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> @@ -2015,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("buggy_eim", IntelIOMMUState, buggy_eim, false),
> 
> I suggest "buggy-eim", to follow the usual style for QOM property
> names.
> 
> Assuming the name gets changed:
> 
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

I'll change the name to "x-buggy-eim" and also squash the patch with
[6/8] as the property doesn't seem to be hated too much.

It's going to be a different patch, so I'll drop the r-b by default,
sorry.
Michael S. Tsirkin Oct. 9, 2016, 11:33 p.m. UTC | #5
On Thu, Oct 06, 2016 at 06:00:52PM +0200, Radim Krčmář wrote:
> 2016-10-06 11:51-0300, Eduardo Habkost:
> > On Wed, Oct 05, 2016 at 03:06:56PM +0200, Radim Krčmář wrote:
> >> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> >> ---
> >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> >> @@ -2015,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("buggy_eim", IntelIOMMUState, buggy_eim, false),
> > 
> > I suggest "buggy-eim", to follow the usual style for QOM property
> > names.
> > 
> > Assuming the name gets changed:
> > 
> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> I'll change the name to "x-buggy-eim" and also squash the patch with
> [6/8] as the property doesn't seem to be hated too much.
> 
> It's going to be a different patch, so I'll drop the r-b by default,
> sorry.

You don't have to drop the r-b if the change is mechanical or
minor. Anyway, waiting for the next version.
Eduardo Habkost Oct. 10, 2016, 5:46 p.m. UTC | #6
On Thu, Oct 06, 2016 at 05:55:25PM +0200, Radim Krčmář wrote:
[...]
> pc-0.10 seems to be the first machine type ever (2009), is there already
> a plan to deprecate it?

I don't think we have a plan, but I would support deprecating and
removing very old machine-types. The question is: how old is too
old?

For reference, the commits and dates when each machine-type was
added are below:

machine   commit    commit date  release  release date
pc-0.10   e8b2a1c6  Jul 8 2009   v0.11.0  Sep 22 2009
pc-0.13   95747581  Jul 22 2009  v0.12.0  Dec 19 2009
pc-0.12   2cae6f5e  Jan 8 2010   v0.13.0  Oct 14 2010
pc-0.13   d76fa62d  Feb 15 2010  v0.13.0  Oct 14 2010
pc-0.14   b903a0f7  Nov 11 2010  v0.14.0  Feb 16 2011
pc-0.15   ce01a508  Dec 18 2011  v1.1.0   Jun 1 2012
pc-1.0    19857e62  Nov 7 2011   v1.0     Dec 1 2011
pc-1.1    382b3a68  Feb 21 2012  v1.1.0   Jun 1 2012
pc-1.2    f1dacf1c  Jun 11 2012  v1.2.0   Sep 5 2012
pc-1.3    f4306941  Sep 13 2012  v1.3.0   Dec 3 2012
pc-*-1.4  f1ae2e38  Dec 4 2012   v1.4.0   Feb 15 2013
pc-*-1.5  bf3caa3d  Feb 8 2013   v1.5.0   May 20 2013
pc-*-1.6  45053fde  May 27 2013  v1.6.0   Aug 15 2013
pc-*-1.7  e9845f09  Aug 2 2013   v1.7.0   Nov 27 2013
pc-*-2.0  aeca6e8d  Dec 2 2013   v2.0.0   Apr 17 2014
pc-*-2.1  3458b2b0  Apr 24 2014  v2.1.0   Aug 1 2014
pc-*-2.2  f9f21873  Jul 30 2014  v2.2.0   Dec 9 2014
pc-*-2.3  64bbd372  Dec 5 2014   v2.3.0   Apr 24 2015
pc-*-2.4  5cb50e0a  Apr 23 2015  v2.4.0   Aug 11 2015
pc-*-2.5  87e896ab  Sep 11 2015  v2.5.0   Dec 16 2015
pc-*-2.6  240240d5  Nov 30 2015  v2.6.0   May 11 2016
pc-*-2.7  d86c1451  May 17 2016  v2.7.0   Sep 2 2016
pc-*-2.8  a4d3c834  Sep 7 2016   -        -
Paolo Bonzini Oct. 11, 2016, 7:36 a.m. UTC | #7
On 10/10/2016 19:46, Eduardo Habkost wrote:
> I don't think we have a plan, but I would support deprecating and
> removing very old machine-types. The question is: how old is too
> old?
> 
> For reference, the commits and dates when each machine-type was
> added are below:
> 
> machine   commit    commit date  release  release date
> pc-0.10   e8b2a1c6  Jul 8 2009   v0.11.0  Sep 22 2009
> pc-0.13   95747581  Jul 22 2009  v0.12.0  Dec 19 2009
> pc-0.12   2cae6f5e  Jan 8 2010   v0.13.0  Oct 14 2010
> pc-0.13   d76fa62d  Feb 15 2010  v0.13.0  Oct 14 2010
> pc-0.14   b903a0f7  Nov 11 2010  v0.14.0  Feb 16 2011
> pc-0.15   ce01a508  Dec 18 2011  v1.1.0   Jun 1 2012
> pc-1.0    19857e62  Nov 7 2011   v1.0     Dec 1 2011
> pc-1.1    382b3a68  Feb 21 2012  v1.1.0   Jun 1 2012
> pc-1.2    f1dacf1c  Jun 11 2012  v1.2.0   Sep 5 2012
> pc-1.3    f4306941  Sep 13 2012  v1.3.0   Dec 3 2012

Anything before pc-1.3 has issues with migration due to the introduction
of the memory API.  Basically, 0xf0000-0xfffff is not migrated
correctly, and the result is that rebooting after migration causes the
guest to crash.  So that could be a reasonable place to draw the line at.

Paolo
Daniel P. Berrangé Oct. 11, 2016, 8:23 a.m. UTC | #8
On Tue, Oct 11, 2016 at 09:36:29AM +0200, Paolo Bonzini wrote:
> 
> 
> On 10/10/2016 19:46, Eduardo Habkost wrote:
> > I don't think we have a plan, but I would support deprecating and
> > removing very old machine-types. The question is: how old is too
> > old?
> > 
> > For reference, the commits and dates when each machine-type was
> > added are below:
> > 
> > machine   commit    commit date  release  release date
> > pc-0.10   e8b2a1c6  Jul 8 2009   v0.11.0  Sep 22 2009
> > pc-0.13   95747581  Jul 22 2009  v0.12.0  Dec 19 2009
> > pc-0.12   2cae6f5e  Jan 8 2010   v0.13.0  Oct 14 2010
> > pc-0.13   d76fa62d  Feb 15 2010  v0.13.0  Oct 14 2010
> > pc-0.14   b903a0f7  Nov 11 2010  v0.14.0  Feb 16 2011
> > pc-0.15   ce01a508  Dec 18 2011  v1.1.0   Jun 1 2012
> > pc-1.0    19857e62  Nov 7 2011   v1.0     Dec 1 2011
> > pc-1.1    382b3a68  Feb 21 2012  v1.1.0   Jun 1 2012
> > pc-1.2    f1dacf1c  Jun 11 2012  v1.2.0   Sep 5 2012
> > pc-1.3    f4306941  Sep 13 2012  v1.3.0   Dec 3 2012
> 
> Anything before pc-1.3 has issues with migration due to the introduction
> of the memory API.  Basically, 0xf0000-0xfffff is not migrated
> correctly, and the result is that rebooting after migration causes the
> guest to crash.  So that could be a reasonable place to draw the line at.

That is a one-off special case - I think it would be desirable to come up
with a general rule we can follow indefinitely, which we can apply at the
start of each release cycle to purge old stuff.

If we wanted to pick pc-1.3 as the starting point and generalize it, we
choose declare we'll support machine types for 4 years. Or we could do
it in terms of number of releases - eg we'll support the last N releases
(for 3/releases per year cadence, that'd be N == 12)

Regards,
Daniel
Paolo Bonzini Oct. 11, 2016, 8:47 a.m. UTC | #9
On 11/10/2016 10:23, Daniel P. Berrange wrote:
> On Tue, Oct 11, 2016 at 09:36:29AM +0200, Paolo Bonzini wrote:
>>
>>
>> On 10/10/2016 19:46, Eduardo Habkost wrote:
>>> I don't think we have a plan, but I would support deprecating and
>>> removing very old machine-types. The question is: how old is too
>>> old?
>>>
>>> For reference, the commits and dates when each machine-type was
>>> added are below:
>>>
>>> machine   commit    commit date  release  release date
>>> pc-0.10   e8b2a1c6  Jul 8 2009   v0.11.0  Sep 22 2009
>>> pc-0.13   95747581  Jul 22 2009  v0.12.0  Dec 19 2009
>>> pc-0.12   2cae6f5e  Jan 8 2010   v0.13.0  Oct 14 2010
>>> pc-0.13   d76fa62d  Feb 15 2010  v0.13.0  Oct 14 2010
>>> pc-0.14   b903a0f7  Nov 11 2010  v0.14.0  Feb 16 2011
>>> pc-0.15   ce01a508  Dec 18 2011  v1.1.0   Jun 1 2012
>>> pc-1.0    19857e62  Nov 7 2011   v1.0     Dec 1 2011
>>> pc-1.1    382b3a68  Feb 21 2012  v1.1.0   Jun 1 2012
>>> pc-1.2    f1dacf1c  Jun 11 2012  v1.2.0   Sep 5 2012
>>> pc-1.3    f4306941  Sep 13 2012  v1.3.0   Dec 3 2012
>>
>> Anything before pc-1.3 has issues with migration due to the introduction
>> of the memory API.  Basically, 0xf0000-0xfffff is not migrated
>> correctly, and the result is that rebooting after migration causes the
>> guest to crash.  So that could be a reasonable place to draw the line at.
> 
> That is a one-off special case - I think it would be desirable to come up
> with a general rule we can follow indefinitely, which we can apply at the
> start of each release cycle to purge old stuff.
> 
> If we wanted to pick pc-1.3 as the starting point and generalize it, we
> choose declare we'll support machine types for 4 years. Or we could do
> it in terms of number of releases - eg we'll support the last N releases
> (for 3/releases per year cadence, that'd be N == 12)

I don't know, it's already boring to create a new machine every time...
I would hate to have to remove one or more machine types every three
months.  Consider that adding new machine types will hardly introduce
bugs; what causes bugs is removing them.

Paolo
Eduardo Habkost Oct. 14, 2016, 2:50 p.m. UTC | #10
On Tue, Oct 11, 2016 at 10:47:43AM +0200, Paolo Bonzini wrote:
> 
> 
> On 11/10/2016 10:23, Daniel P. Berrange wrote:
> > On Tue, Oct 11, 2016 at 09:36:29AM +0200, Paolo Bonzini wrote:
> >>
> >>
> >> On 10/10/2016 19:46, Eduardo Habkost wrote:
> >>> I don't think we have a plan, but I would support deprecating and
> >>> removing very old machine-types. The question is: how old is too
> >>> old?
> >>>
> >>> For reference, the commits and dates when each machine-type was
> >>> added are below:
> >>>
> >>> machine   commit    commit date  release  release date
> >>> pc-0.10   e8b2a1c6  Jul 8 2009   v0.11.0  Sep 22 2009
> >>> pc-0.13   95747581  Jul 22 2009  v0.12.0  Dec 19 2009
> >>> pc-0.12   2cae6f5e  Jan 8 2010   v0.13.0  Oct 14 2010
> >>> pc-0.13   d76fa62d  Feb 15 2010  v0.13.0  Oct 14 2010
> >>> pc-0.14   b903a0f7  Nov 11 2010  v0.14.0  Feb 16 2011
> >>> pc-0.15   ce01a508  Dec 18 2011  v1.1.0   Jun 1 2012
> >>> pc-1.0    19857e62  Nov 7 2011   v1.0     Dec 1 2011
> >>> pc-1.1    382b3a68  Feb 21 2012  v1.1.0   Jun 1 2012
> >>> pc-1.2    f1dacf1c  Jun 11 2012  v1.2.0   Sep 5 2012
> >>> pc-1.3    f4306941  Sep 13 2012  v1.3.0   Dec 3 2012
> >>
> >> Anything before pc-1.3 has issues with migration due to the introduction
> >> of the memory API.  Basically, 0xf0000-0xfffff is not migrated
> >> correctly, and the result is that rebooting after migration causes the
> >> guest to crash.  So that could be a reasonable place to draw the line at.
> > 
> > That is a one-off special case - I think it would be desirable to come up
> > with a general rule we can follow indefinitely, which we can apply at the
> > start of each release cycle to purge old stuff.
> > 
> > If we wanted to pick pc-1.3 as the starting point and generalize it, we
> > choose declare we'll support machine types for 4 years. Or we could do
> > it in terms of number of releases - eg we'll support the last N releases
> > (for 3/releases per year cadence, that'd be N == 12)
> 
> I don't know, it's already boring to create a new machine every time...
> I would hate to have to remove one or more machine types every three
> months.  Consider that adding new machine types will hardly introduce
> bugs; what causes bugs is removing them.

I wouldn't like to _have_ to remove them, but I would love to
have a clear policy that would set user expectations and allow us
to remove some of them once in a while.
diff mbox

Patch

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[] = {
     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),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2473,11 +2474,11 @@  static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
     }
 
     if (s->intr_eim == ON_OFF_AUTO_AUTO) {
-        s->intr_eim = x86_iommu->intr_supported && kvm_irqchip_in_kernel() ?
+        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) {
+    if (s->intr_eim == ON_OFF_AUTO_ON && !s->buggy_eim) {
         if (kvm_irqchip_in_kernel() && !kvm_enable_x2apic()) {
             error_setg(errp, "eim=on requires support on the KVM side"
                              "(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   = "ioapic",\
         .property = "version",\
         .value    = "0x11",\
+    },{\
+        .driver   = "intel-iommu",\
+        .property = "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,