| Message ID | 1249993895-11119-3-git-send-email-gleb@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/11/2009 03:31 PM, Gleb Natapov wrote: > Maintain back mapping from irqchip/pin to gsi to speedup > interrupt acknowledgment notifications. > > Signed-off-by: Gleb Natapov<gleb@redhat.com> > --- > include/linux/kvm_host.h | 1 + > virt/kvm/irq_comm.c | 24 +++++++++++------------- > 2 files changed, 12 insertions(+), 13 deletions(-) > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 09df31d..d2b8eb3 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -133,6 +133,7 @@ struct kvm_kernel_irq_routing_entry { > }; > > struct kvm_irq_routing_table { > + int chip[3][KVM_IOAPIC_NUM_PINS]; > KVM_NR_IRQCHIPS An alternative implementation is to embed a lookup table in each irqchip. I don't think it's really necessary. > --- a/virt/kvm/irq_comm.c > +++ b/virt/kvm/irq_comm.c > @@ -168,21 +168,13 @@ void kvm_notify_acked_irq(struct kvm *kvm, unsigned irqchip, unsigned pin) > { > struct kvm_irq_ack_notifier *kian; > struct hlist_node *n; > - unsigned gsi = pin; > - int i; > + unsigned gsi; > > trace_kvm_ack_irq(irqchip, pin); > > - for (i = 0; i< kvm->irq_routing->nr_rt_entries; i++) { > - struct kvm_kernel_irq_routing_entry *e; > - e =&kvm->irq_routing->rt_entries[i]; > - if (e->type == KVM_IRQ_ROUTING_IRQCHIP&& > - e->irqchip.irqchip == irqchip&& > - e->irqchip.pin == pin) { > - gsi = e->gsi; > - break; > - } > - } > + gsi = kvm->irq_routing->chip[irqchip][pin]; > + if (gsi == -1) > + gsi = pin; > What's this?
On Wed, Aug 12, 2009 at 11:09:58AM +0300, Avi Kivity wrote: > On 08/11/2009 03:31 PM, Gleb Natapov wrote: >> Maintain back mapping from irqchip/pin to gsi to speedup >> interrupt acknowledgment notifications. >> >> Signed-off-by: Gleb Natapov<gleb@redhat.com> >> --- >> include/linux/kvm_host.h | 1 + >> virt/kvm/irq_comm.c | 24 +++++++++++------------- >> 2 files changed, 12 insertions(+), 13 deletions(-) >> >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >> index 09df31d..d2b8eb3 100644 >> --- a/include/linux/kvm_host.h >> +++ b/include/linux/kvm_host.h >> @@ -133,6 +133,7 @@ struct kvm_kernel_irq_routing_entry { >> }; >> >> struct kvm_irq_routing_table { >> + int chip[3][KVM_IOAPIC_NUM_PINS]; >> > > KVM_NR_IRQCHIPS > > An alternative implementation is to embed a lookup table in each > irqchip. I don't think it's really necessary. > >> --- a/virt/kvm/irq_comm.c >> +++ b/virt/kvm/irq_comm.c >> @@ -168,21 +168,13 @@ void kvm_notify_acked_irq(struct kvm *kvm, unsigned irqchip, unsigned pin) >> { >> struct kvm_irq_ack_notifier *kian; >> struct hlist_node *n; >> - unsigned gsi = pin; >> - int i; >> + unsigned gsi; >> >> trace_kvm_ack_irq(irqchip, pin); >> >> - for (i = 0; i< kvm->irq_routing->nr_rt_entries; i++) { >> - struct kvm_kernel_irq_routing_entry *e; >> - e =&kvm->irq_routing->rt_entries[i]; >> - if (e->type == KVM_IRQ_ROUTING_IRQCHIP&& >> - e->irqchip.irqchip == irqchip&& >> - e->irqchip.pin == pin) { >> - gsi = e->gsi; >> - break; >> - } >> - } >> + gsi = kvm->irq_routing->chip[irqchip][pin]; >> + if (gsi == -1) >> + gsi = pin; >> > > What's this? > If there is not GSI mapping use pin as gsi. This what current code does. We can BUG() here I think. -- Gleb. -- 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
On 08/12/2009 11:42 AM, Gleb Natapov wrote: > > >> What's this? >> >> > If there is not GSI mapping use pin as gsi. This what current code does. > We can BUG() here I think. > > We can't BUG() userspace can change the mapping before the ack is received.
On Wed, Aug 12, 2009 at 12:06:59PM +0300, Avi Kivity wrote: > On 08/12/2009 11:42 AM, Gleb Natapov wrote: >> >> >>> What's this? >>> >>> >> If there is not GSI mapping use pin as gsi. This what current code does. >> We can BUG() here I think. >> >> > > We can't BUG() userspace can change the mapping before the ack is received. > Yes. Malicious userspace can remove GSI mapping for GSI with ack notifier registered, so we can't BUG(). But making GSI=pin doesn't look correct too. Should we just skip calling ack notifier? -- Gleb. -- 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
On 08/12/2009 01:17 PM, Gleb Natapov wrote: > Yes. Malicious userspace can remove GSI mapping for GSI with ack > notifier registered, so we can't BUG(). But making GSI=pin doesn't look > correct too. Should we just skip calling ack notifier? > I think so.
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 09df31d..d2b8eb3 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -133,6 +133,7 @@ struct kvm_kernel_irq_routing_entry { }; struct kvm_irq_routing_table { + int chip[3][KVM_IOAPIC_NUM_PINS]; struct kvm_kernel_irq_routing_entry *rt_entries; u32 nr_rt_entries; /* diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c index a9d2262..42994c4 100644 --- a/virt/kvm/irq_comm.c +++ b/virt/kvm/irq_comm.c @@ -168,21 +168,13 @@ void kvm_notify_acked_irq(struct kvm *kvm, unsigned irqchip, unsigned pin) { struct kvm_irq_ack_notifier *kian; struct hlist_node *n; - unsigned gsi = pin; - int i; + unsigned gsi; trace_kvm_ack_irq(irqchip, pin); - for (i = 0; i < kvm->irq_routing->nr_rt_entries; i++) { - struct kvm_kernel_irq_routing_entry *e; - e = &kvm->irq_routing->rt_entries[i]; - if (e->type == KVM_IRQ_ROUTING_IRQCHIP && - e->irqchip.irqchip == irqchip && - e->irqchip.pin == pin) { - gsi = e->gsi; - break; - } - } + gsi = kvm->irq_routing->chip[irqchip][pin]; + if (gsi == -1) + gsi = pin; hlist_for_each_entry(kian, n, &kvm->arch.irq_ack_notifier_list, link) if (kian->gsi == gsi) @@ -308,6 +300,9 @@ static int setup_routing_entry(struct kvm_irq_routing_table *rt, } e->irqchip.irqchip = ue->u.irqchip.irqchip; e->irqchip.pin = ue->u.irqchip.pin + delta; + if (e->irqchip.pin > KVM_IOAPIC_NUM_PINS) + goto out; + rt->chip[ue->u.irqchip.irqchip][e->irqchip.pin] = ue->gsi; break; case KVM_IRQ_ROUTING_MSI: e->set = kvm_set_msi; @@ -332,7 +327,7 @@ int kvm_set_irq_routing(struct kvm *kvm, unsigned flags) { struct kvm_irq_routing_table *new, *old; - u32 i, nr_rt_entries = 0; + u32 i, j, nr_rt_entries = 0; int r; for (i = 0; i < nr; ++i) { @@ -353,6 +348,9 @@ int kvm_set_irq_routing(struct kvm *kvm, new->rt_entries = (void *)&new->map[nr_rt_entries]; new->nr_rt_entries = nr_rt_entries; + 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;
Maintain back mapping from irqchip/pin to gsi to speedup interrupt acknowledgment notifications. Signed-off-by: Gleb Natapov <gleb@redhat.com> --- include/linux/kvm_host.h | 1 + virt/kvm/irq_comm.c | 24 +++++++++++------------- 2 files changed, 12 insertions(+), 13 deletions(-)