Re: [PATCH stub] kvm: caching API for interrupts

[Gleb Natapov <gleb@redhat.com>]

On Sun, Jul 29, 2012 at 11:00:58PM +0300, Michael S. Tsirkin wrote:
> I've been looking at adding caching for IRQs so that we don't need to
> scan all VCPUs on each interrupt.  One issue I had a problem with, is
> how the cache structure can be used from both a thread (to fill out the
> cache) and interrupt (to actually send if cache is valid).
I do not get this. Cache should be fill out at the time of use.

> 
> For now just added a lock field in the cache so we don't need to worry
> about this, and with such a lock in place we don't have to worry about
> RCU as cache can be invalidated simply under this lock.
Cache should be invalidated at the time of use if irq routing changed.

> 
> For now this just declares the structure and updates the APIs
> so it's not intended to be applied, but just to give you
> the idea.
> 
> Comments, flames wellcome.
> 
There is no enough meat to discuss here I am afraid.

> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> --
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 96c158a..3384b39 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -214,11 +214,28 @@ static inline unsigned long kvm_dirty_bitmap_bytes(struct kvm_memory_slot *memsl
>  	return ALIGN(memslot->npages, BITS_PER_LONG) / 8;
>  }
>  
> +struct kvm_irq_cache {
> +	spinlock_t lock;
> +	bool valid;
> +	struct kvm_vcpu *dest;
> +	/* For now we only cache lapic irqs */
> +	struct kvm_lapic_irq irq;
> +	/* Protected by kvm->irq_cache_lock */
> +	struct list_head list;
Why are you storing them in the list? This should be embedded structure.

> +};
> +
> +int kvm_set_irq_and_cache(struct kvm *kvm, int irq_source_id, u32 irq, int level,
> +			  struct kvm_irq_cache *cache);
> +int kvm_set_irq_cached(struct kvm *kvm, struct kvm_irq_cache *cache);
> +void kvm_irq_cache_init(struct kvm *kvm, struct kvm_irq_cache *cache);
> +void kvm_irq_cache_cleanup(struct kvm_irq_cache *cache);
> +
>  struct kvm_kernel_irq_routing_entry {
>  	u32 gsi;
>  	u32 type;
>  	int (*set)(struct kvm_kernel_irq_routing_entry *e,
> -		   struct kvm *kvm, int irq_source_id, int level);
> +		   struct kvm *kvm, int irq_source_id, int level,
> +		   struct kvm_irq_cache *cache);
>  	union {
>  		struct {
>  			unsigned irqchip;
> @@ -304,6 +321,8 @@ struct kvm {
>  	struct kvm_irq_routing_table __rcu *irq_routing;
>  	struct hlist_head mask_notifier_list;
>  	struct hlist_head irq_ack_notifier_list;
> +	struct mutex irq_cache_lock;
> +	struct list_head irq_cache_list;
>  #endif
>  
>  #ifdef KVM_ARCH_WANT_MMU_NOTIFIER
> @@ -617,7 +636,8 @@ void kvm_get_intr_delivery_bitmask(struct kvm_ioapic *ioapic,
>  #endif
>  int kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 irq, int level);
>  int kvm_set_msi(struct kvm_kernel_irq_routing_entry *irq_entry, struct kvm *kvm,
> -		int irq_source_id, int level);
> +		int irq_source_id, int level,
> +		struct kvm_irq_cache *cache);
>  void kvm_notify_acked_irq(struct kvm *kvm, unsigned irqchip, unsigned pin);
>  void kvm_register_irq_ack_notifier(struct kvm *kvm,
>  				   struct kvm_irq_ack_notifier *kian);
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index 7d7e2aa..9622076 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -138,7 +138,7 @@ irqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key)
>  		irq = rcu_dereference(irqfd->irq_entry);
>  		/* An event has been signaled, inject an interrupt */
>  		if (irq)
> -			kvm_set_msi(irq, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1);
> +			kvm_set_msi(irq, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1, NULL);
>  		else
>  			schedule_work(&irqfd->inject);
>  		rcu_read_unlock();
> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
> index 5afb431..d92f3d3 100644
> --- a/virt/kvm/irq_comm.c
> +++ b/virt/kvm/irq_comm.c
> @@ -46,7 +46,8 @@ static inline int kvm_irq_line_state(unsigned long *irq_state,
>  }
>  
>  static int kvm_set_pic_irq(struct kvm_kernel_irq_routing_entry *e,
> -			   struct kvm *kvm, int irq_source_id, int level)
> +			   struct kvm *kvm, int irq_source_id, int level,
> +			   struct kvm_irq_cache *cache)
>  {
>  #ifdef CONFIG_X86
>  	struct kvm_pic *pic = pic_irqchip(kvm);
> @@ -59,7 +60,8 @@ static int kvm_set_pic_irq(struct kvm_kernel_irq_routing_entry *e,
>  }
>  
>  static int kvm_set_ioapic_irq(struct kvm_kernel_irq_routing_entry *e,
> -			      struct kvm *kvm, int irq_source_id, int level)
> +			      struct kvm *kvm, int irq_source_id, int level,
> +			      struct kvm_irq_cache *cache)
>  {
>  	struct kvm_ioapic *ioapic = kvm->arch.vioapic;
>  	level = kvm_irq_line_state(&ioapic->irq_states[e->irqchip.pin],
> @@ -115,7 +117,8 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>  }
>  
>  int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
> -		struct kvm *kvm, int irq_source_id, int level)
> +		struct kvm *kvm, int irq_source_id, int level,
> +		struct kvm_irq_cache *cache)
>  {
>  	struct kvm_lapic_irq irq;
>  
> @@ -149,7 +152,7 @@ int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi)
>  	route.msi.address_hi = msi->address_hi;
>  	route.msi.data = msi->data;
>  
> -	return kvm_set_msi(&route, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1);
> +	return kvm_set_msi(&route, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1, NULL);
>  }
>  
>  /*
> @@ -180,7 +183,7 @@ int kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 irq, int level)
>  
>  	while(i--) {
>  		int r;
> -		r = irq_set[i].set(&irq_set[i], kvm, irq_source_id, level);
> +		r = irq_set[i].set(&irq_set[i], kvm, irq_source_id, level, NULL);
>  		if (r < 0)
>  			continue;
>  
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
			Gleb.