[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);
> > + }
> > }
[...]