Re: [PATCH RFC] kvm: fast-path msi injection with irqfd

[Michael S. Tsirkin]

On Thu, Nov 18, 2010 at 03:35:01PM +0200, Gleb Natapov wrote:
> On Thu, Nov 18, 2010 at 03:20:27PM +0200, Michael S. Tsirkin wrote:
> > On Thu, Nov 18, 2010 at 03:14:53PM +0200, Gleb Natapov wrote:
> > > On Thu, Nov 18, 2010 at 03:03:37PM +0200, Michael S. Tsirkin wrote:
> > > > > >+static inline void kvm_irq_routing_update(struct kvm *kvm,
> > > > > >+					  struct kvm_irq_routing_table *irq_rt)
> > > > > >+{
> > > > > >+	rcu_assign_pointer(kvm->irq_routing, irq_rt);
> > > > > >+}
> > > > > >+
> > > > > >  static inline int kvm_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
> > > > > >  {
> > > > > >  	return -ENOSYS;
> > > > > 
> > > > > Apart from these minor issues, looks good.
> > > > 
> > > > 
> > > > Something we should consider improving is the loop over all VCPUs that
> > > > kvm_irq_delivery_to_apic invokes.  I think that (for non-broadcast
> > > > interrupts) it should be possible to precompute an store the CPU
> > > > in question as part of the routing entry.
> > > > 
> > > > Something for a separate patch ... comments?
> > > > 
> > > I do not think this info should be part of routing entry. Routing entry
> > > is more about describing wires on the board.
> > 
> > Not for msi. kvm_kernel_irq_routing_entry seems to just keep an
> > address/data pair in that case. So
> > 
> Yeah. Using routing_entry for MSI was miss design. We discussed that too :)
> 
> > 	union {
> > 		struct {
> > 			unsigned irqchip;
> > 			unsigned pin;
> > 		} irqchip;
> > 		struct msi_msg msi;
> > 	};
> > 
> > would become
> > 
> > 	union {
> > 		struct {
> > 			unsigned irqchip;
> > 			unsigned pin;
> > 		} irqchip;
> > 		struct {
> > 			struct msi_msg msi;
> > 			struct kvm_vpcu *dest;
> > 		} msi;
> > 	};
> > 
> > or something like this.
> Ah so you want to do it only for MSI? For MSI it makes sense. Remember
> though that sometimes destination depend on message itself (specifically
> on delivery mode).

Of course. We'll take message/data and precompute destination.
Set to NULL for e.g. broadcast and recompute at injection time
in that case.  BTW SELF doesn't work for MSI at the moment, not sure
whether it's relevant or when is it used.

> > 
> > > Other then that
> > > this is a good idea that, IIRC, we already discussed once.
> > > 
> > > --
> > > 			Gleb.
> 
> --
> 			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/