2016-09-30 13:13+0800, Peter Xu: > On Thu, Sep 29, 2016 at 01:23:27PM +0200, Radim Krčmář wrote: >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >> @@ -2466,6 +2472,18 @@ static void vtd_realize(DeviceState *dev, Error >> **errp) >> exit(1); >> } >> >> + if (s->intr_eim == ON_OFF_AUTO_ON && !x86_iommu->intr_supported) { >> + error_report("intel-iommu,eim=on cannot be selected without " >> + "intremap=on."); >> + exit(1); >> + } >> + 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) { >> + s->intr_eim = ON_OFF_AUTO_ON; >> + } > > A single if() instead of above two might be nicer: > > if (s->intr_eim == ON_OFF_AUTO_AUTO) { > e->intr_eim = x86_iommu->intr_supported ? > ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; > }
Hm, it'll end up looking like if (s->intr_eim == ON_OFF_AUTO_AUTO) { e->intr_eim = (x86_iommu->intr_supported && kvm_irqchip_in_kernel()) || pcmc->buggy_intel_iommu_eim ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; } It's harder to read, but less LOC ... I'll use it, thanks.