2016-10-08 15:21+0800, Peter Xu:
> On Wed, Oct 05, 2016 at 03:06:55PM +0200, Radim Krčmář wrote:
>
> [...]
>
>> @@ -2472,10 +2473,22 @@ static bool vtd_decide_config(IntelIOMMUState *s,
>> Error **errp)
>> }
>>
>> if (s->intr_eim == ON_OFF_AUTO_AUTO) {
>> - s->intr_eim = x86_iommu->intr_supported ?
>> + s->intr_eim = x86_iommu->intr_supported && 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_setg(errp, "eim=on requires support on the KVM side"
>> + "(X2APIC_API, first shipped in v4.7)");
>> + return false;
>> + }
>> + if (!kvm_irqchip_in_kernel()) {
>> + error_setg(errp, "eim=on requires
>> accel=kvm,kernel-irqchip=split");
>> + return false;
>> + }
>
> I would prefer:
>
> if (kvm_irqchip_in_kernel()) {
> if (!kvm_enable_x2apic()) {
> error("enable x2apic failed");
> return false;
> }
> } else {
> error("need split irqchip");
> return false;
> }
Yeah, it looks better, thanks.
> But that's really a matter of taste. So:
I'll currently go for an implicit else: (because 4 levels of indentation
are getting helper-function worthy and it has less curly braces)
if (!kvm_irqchip_in_kernel()) {
error("need split irqchip");
return false;
}
if (!kvm_enable_x2apic()) {
error("enable x2apic failed");
return false;
}
> Reviewed-by: Peter Xu <[email protected]>
I squashed [7/8] into this patch in v5 and the second one didn't have
your r-b, so I made the change as I'd have to drop the r-b anyway.
