Re: [PATCH 10/10] Change irq routing table to use gsi indexed array.

[Gleb Natapov <gleb@redhat.com>]

On Wed, Jul 15, 2009 at 10:17:22PM +0300, Michael S. Tsirkin wrote:
> On Tue, Jul 14, 2009 at 05:30:45PM +0300, Gleb Natapov wrote:
> > @@ -147,14 +149,13 @@ int kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level)
> >  	 * writes to the unused one.
> >  	 */
> >  	rcu_read_lock();
> > -	for (e = rcu_dereference(kvm->irq_routing); e && e->set; e++) {
> > -		if (e->gsi == irq) {
> > -			int r = e->set(e, kvm, sig_level);
> > -			if (r < 0)
> > -				continue;
> > +	irq_rt = rcu_dereference(kvm->irq_routing);
> > +	hlist_for_each_entry(e, n, &irq_rt->map[irq], link) {
> 
> Don't you need to range-check irq? E.g. with irqfd, gsi is
> controlled by guest.
> 
Yes, I need to add range checking. Good point.

> > +		int r = e->set(e, kvm, sig_level);
> > +		if (r < 0)
> > +			continue;
> >  
> > -			ret = r + ((ret < 0) ? 0 : ret);
> > -		}
> > +		ret = r + ((ret < 0) ? 0 : ret);
> >  	}
> >  	rcu_read_unlock();
> >  	return ret;
> > @@ -162,21 +163,16 @@ int kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level)
> >  
> >  void kvm_notify_acked_irq(struct kvm *kvm, unsigned irqchip, unsigned pin)
> >  {
> > -	struct kvm_kernel_irq_routing_entry *e;
> >  	struct kvm_irq_ack_notifier *kian;
> >  	struct hlist_node *n;
> > -	unsigned gsi = pin;
> > +	unsigned gsi;
> >  
> >  	trace_kvm_ack_irq(irqchip, pin);
> >  
> >  	rcu_read_lock();
> > -	for (e = rcu_dereference(kvm->irq_routing); e && e->set; e++) {
> > -		if (e->irqchip.irqchip == irqchip &&
> > -		    e->irqchip.pin == pin) {
> > -			gsi = e->gsi;
> > -			break;
> > -		}
> > -	}
> > +	gsi = rcu_dereference(kvm->irq_routing)->chip[irqchip][pin];
> 
> And possibly here as well. Can guest control pin?
> 
> > +	if (gsi == -1)
> > +		gsi = pin;
> >  
> >  	hlist_for_each_entry_rcu(kian, n, &kvm->irq_ack_notifier_list, link)
> >  		if (kian->gsi == gsi)
> > @@ -277,7 +273,8 @@ void kvm_free_irq_routing(struct kvm *kvm)
> >  	kfree(kvm->irq_routing);
> >  }
> >  
> > -static int setup_routing_entry(struct kvm_kernel_irq_routing_entry *e,
> > +static int setup_routing_entry(struct kvm_irq_routing_table *rt,
> > +			       struct kvm_kernel_irq_routing_entry *e,
> >  			       const struct kvm_irq_routing_entry *ue)
> >  {
> >  	int r = -EINVAL;
> > @@ -303,6 +300,7 @@ static int setup_routing_entry(struct kvm_kernel_irq_routing_entry *e,
> >  		}
> >  		e->irqchip.irqchip = ue->u.irqchip.irqchip;
> >  		e->irqchip.pin = ue->u.irqchip.pin + delta;
> > +		rt->chip[ue->u.irqchip.irqchip][e->irqchip.pin] = ue->gsi;
> >  		break;
> >  	case KVM_IRQ_ROUTING_MSI:
> >  		e->set = kvm_set_msi;
> > @@ -313,6 +311,8 @@ static int setup_routing_entry(struct kvm_kernel_irq_routing_entry *e,
> >  	default:
> >  		goto out;
> >  	}
> > +
> > +	hlist_add_head(&e->link, &rt->map[e->gsi]);
> >  	r = 0;
> >  out:
> >  	return r;
> > @@ -324,23 +324,37 @@ int kvm_set_irq_routing(struct kvm *kvm,
> >  			unsigned nr,
> >  			unsigned flags)
> >  {
> > -	struct kvm_kernel_irq_routing_entry *new, *old;
> > -	unsigned i;
> > +	struct kvm_irq_routing_table *new, *old;
> > +	u32 i, j, max_gsi = 0;
> >  	int r;
> >  
> > -	/* last elemet is left zeored and indicates the end of the array */
> > -	new = kzalloc(sizeof(*new) * (nr + 1), GFP_KERNEL);
> > +	for (i = 0; i < nr; ++i) {
> > +		if (ue[i].gsi >= KVM_MAX_IRQ_ROUTES)
> > +			return -EINVAL;
> > +		max_gsi = max(max_gsi, ue[i].gsi);
> > +	}
> > +
> > +	max_gsi += 1;
> > +
> > +	new = kzalloc(sizeof(*new) + (max_gsi * sizeof(struct hlist_head)) +
> > +		      (nr * sizeof(struct kvm_kernel_irq_routing_entry)),
> > +		      GFP_KERNEL);
> >  
> >  	if (!new)
> >  		return -ENOMEM;
> >  
> > +	new->rt_entries = (void *)&new->map[max_gsi];
> > +
> > +	new->max_gsi = max_gsi;
> > +	for (i = 0; i < 3; i++)
> > +		for (j = 0; j < KVM_IOAPIC_NUM_PINS; j++)
> > +			new->chip[i][j] = -1;
> > +
> >  	for (i = 0; i < nr; ++i) {
> >  		r = -EINVAL;
> > -		if (ue->gsi >= KVM_MAX_IRQ_ROUTES)
> > -			goto out;
> >  		if (ue->flags)
> >  			goto out;
> > -		r = setup_routing_entry(new + i, ue);
> > +		r = setup_routing_entry(new, &new->rt_entries[i], ue);
> >  		if (r)
> >  			goto out;
> >  		++ue;
> > -- 
> > 1.6.2.1
> > 
> > --
> > 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.