Re: [Qemu-devel] [PATCH v2 7/7] intel-iommu: keep buggy EIM enabled in 2.7 machine type

[Radim Krčmář]

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.