2016-09-29 18:06+0200, Igor Mammedov: > 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 >> > --- >> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >> > @@ -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.
There is one more error_report() in the file and it doesn't have "Error **" -- I'll leave it be and change the rest. It amounts to one extra patch that before [4/7] (could be squashed too). >> > + } >> > + 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() Yeah, really unreadable, thanks.