qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 6/7] intel_iommu: reject broken EIM


From: Igor Mammedov
Subject: Re: [Qemu-devel] [PATCH v2 6/7] intel_iommu: reject broken EIM
Date: Thu, 29 Sep 2016 18:06:54 +0200

On Thu, 29 Sep 2016 15:18:36 +0200
Paolo Bonzini <address@hidden> wrote:

> On 29/09/2016 13:23, Radim Krčmář 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.
> > 
> > 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.
> > 
> > Signed-off-by: Radim Krčmář <address@hidden>
> > ---
> > 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(-)
> > 
> > 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"
> >  
> >  /*#define DEBUG_INTEL_IOMMU*/
> >  #ifdef DEBUG_INTEL_IOMMU
> > @@ -2481,7 +2482,20 @@ static void vtd_realize(DeviceState *dev, Error 
> > **errp)
> >          s->intr_eim = ON_OFF_AUTO_OFF;
> >      }
> >      if (s->intr_eim == ON_OFF_AUTO_AUTO) {
> > -        s->intr_eim = ON_OFF_AUTO_ON;
> > +        s->intr_eim = kvm_irqchip_in_kernel() ? ON_OFF_AUTO_ON
> > +                                              : ON_OFF_AUTO_OFF;
> > +    }
> > +    if (s->intr_eim == ON_OFF_AUTO_ON) {
> > +        if (kvm_irqchip_in_kernel() && !kvm_enable_x2apic()) {
> > +            error_report("intel-iommu,eim=on requires support on the KVM 
> > side "
> > +                         "(X2APIC_API, first shipped in v4.7).");
> > +            exit(1);  
> 
> 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.

> 
> Paolo
> 
> > +        }
> > +        if (!kvm_irqchip_in_kernel()) {
> > +            error_report("intel-iommu,eim=on requires "
> > +                         "accel=kvm,kernel-irqchip=split.");
this prints:
 -device intel-iommu,intremap=on,eim=on: intel-iommu,eim=on requires 
accel=kvm,kernel-irqchip=split
so 'intel-iommu,' not really needed, the same would happen with error_setg()

> > +            exit(1);
> > +        }
> >      }
[...]




reply via email to

[Prev in Thread] Current Thread [Next in Thread]