2016-09-30 13:40+0800, Peter Xu: > On Thu, Sep 29, 2016 at 01:23:29PM +0200, Radim Krčmář wrote: > > [...] > >> @@ -2481,11 +2482,14 @@ static void vtd_realize(DeviceState *dev, Error >> **errp) >> if (s->intr_eim == ON_OFF_AUTO_AUTO && !x86_iommu->intr_supported) { >> s->intr_eim = ON_OFF_AUTO_OFF; >> } >> + if (s->intr_eim == ON_OFF_AUTO_AUTO && pcmc->buggy_intel_iommu_eim) { >> + s->intr_eim = ON_OFF_AUTO_ON; >> + } >> if (s->intr_eim == ON_OFF_AUTO_AUTO) { >> s->intr_eim = kvm_irqchip_in_kernel() ? ON_OFF_AUTO_ON >> : ON_OFF_AUTO_OFF; >> } >> - if (s->intr_eim == ON_OFF_AUTO_ON) { >> + if (s->intr_eim == ON_OFF_AUTO_ON && !pcmc->buggy_intel_iommu_eim) { >> 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)."); > > No matter how we would treat this patch, I see that we are stacking up > if clauses here. So IMHO maybe it's time to award EIM a new routine: > > int vtd_eim_detect(IntelIOMMUState *, Error **errp); > > And squash all these conditions in. Then in vtd_realize(): > > if (vtd_eim_detect(s, errp)) { > return; > }
Yeah, thanks.