On Thu, 29 Sep 2016 15:18:36 +0200 Paolo Bonzini <pbonz...@redhat.com> 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ář <rkrc...@redhat.com> > > --- > > 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); > > + } > > } [...]