On Tue, Nov 26, 2013 at 06:27:47PM +0200, Avi Kivity wrote: > 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). There still may be race between pci read and MSI registered in IRR. I do not believe such read can undo IRR changes. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
