Re: [Qemu-devel] [RFC] create a single workqueue for each vm to update vm irq routing table

[Avi Kivity <avi@cloudius-systems.com>]

On 11/26/2013 06:24 PM, Gleb Natapov wrote:
> On Tue, Nov 26, 2013 at 04:20:27PM +0100, Paolo Bonzini wrote:
>> Il 26/11/2013 16:03, Gleb Natapov ha scritto:
>>>>>>>>> I understood the proposal was also to eliminate the synchronize_rcu(),
>>>>>>>>> so while new interrupts would see the new routing table, interrupts
>>>>>>>>> already in flight could pick up the old one.
>>>>>>> Isn't that always the case with RCU?  (See my answer above: "the vcpus
>>>>>>> already see the new routing table after the rcu_assign_pointer that is
>>>>>>> in kvm_irq_routing_update").
>>>>> With synchronize_rcu(), you have the additional guarantee that any
>>>>> parallel accesses to the old routing table have completed.  Since we
>>>>> also trigger the irq from rcu context, you know that after
>>>>> synchronize_rcu() you won't get any interrupts to the old
>>>>> destination (see kvm_set_irq_inatomic()).
>>> We do not have this guaranty for other vcpus that do not call
>>> synchronize_rcu(). They may still use outdated routing table while a vcpu
>>> or iothread that performed table update sits in synchronize_rcu().
>> Avi's point is that, after the VCPU resumes execution, you know that no
>> interrupt will be sent to the old destination because
>> kvm_set_msi_inatomic (and ultimately kvm_irq_delivery_to_apic_fast) is
>> also called within the RCU read-side critical section.
>>
>> Without synchronize_rcu you could have
>>
>>      VCPU writes to routing table
>>                                         e = entry from IRQ routing table
>>      kvm_irq_routing_update(kvm, new);
>>      VCPU resumes execution
>>                                         kvm_set_msi_irq(e, &irq);
>>                                         kvm_irq_delivery_to_apic_fast();
>>
>> where the entry is stale but the VCPU has already resumed execution.
>>
> So how is it different from what we have now:
>
> disable_irq()
> VCPU writes to routing table
>                                   e = entry from IRQ routing table
>                                   kvm_set_msi_irq(e, &irq);
>                                   kvm_irq_delivery_to_apic_fast();
> kvm_irq_routing_update(kvm, new);
> synchronize_rcu()
> VCPU resumes execution
> enable_irq()
> receive stale irq
>
>        
>

Suppose the guest did not disable_irq() and enable_irq(), but instead 
had a pci read where you have the enable_irq().  After the read you 
cannot have a stale irq (assuming the read flushes the irq all the way 
to the APIC).