[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] KVM: Fix GSI number space limit
From: |
Cornelia Huck |
Subject: |
Re: [Qemu-devel] [PATCH] KVM: Fix GSI number space limit |
Date: |
Fri, 6 Jun 2014 15:41:36 +0200 |
On Fri, 06 Jun 2014 15:28:13 +0200
Alexander Graf <address@hidden> wrote:
>
> On 06.06.14 15:23, Cornelia Huck wrote:
> > On Fri, 06 Jun 2014 15:15:54 +0200
> > Alexander Graf <address@hidden> wrote:
> >
> >> On 06.06.14 15:12, Cornelia Huck wrote:
> >>> On Fri, 6 Jun 2014 14:46:05 +0200
> >>> Alexander Graf <address@hidden> wrote:
> >>>
> >>>> KVM tells us the number of GSIs it can handle inside the kernel. That
> >>>> value is
> >>>> basically KVM_MAX_IRQ_ROUTES. However when we try to set the GSI mapping
> >>>> table,
> >>>> it checks for
> >>>>
> >>>> r = -EINVAL;
> >>>> if (routing.nr >= KVM_MAX_IRQ_ROUTES)
> >>>> goto out;
> >>>>
> >>>> erroring out even when we're only using all of the GSIs. To make sure we
> >>>> never
> >>>> hit that limit, let's reduce the number of GSIs we get from KVM by one.
> >>>>
> >>>> Signed-off-by: Alexander Graf <address@hidden>
> >>>> ---
> >>>> kvm-all.c | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/kvm-all.c b/kvm-all.c
> >>>> index 4e19eff..56a251b 100644
> >>>> --- a/kvm-all.c
> >>>> +++ b/kvm-all.c
> >>>> @@ -938,7 +938,7 @@ void kvm_init_irq_routing(KVMState *s)
> >>>> {
> >>>> int gsi_count, i;
> >>>>
> >>>> - gsi_count = kvm_check_extension(s, KVM_CAP_IRQ_ROUTING);
> >>>> + gsi_count = kvm_check_extension(s, KVM_CAP_IRQ_ROUTING) - 1;
> >>>> if (gsi_count > 0) {
> >>>> unsigned int gsi_bits, i;
> >>>>
> >>> But gsi_count is already marked as used further down in this function,
> >>> isn't it? Confused.
> >> gsi_bits = ALIGN(gsi_count, 32);
> >> [...]
> >> for (i = gsi_count; i < gsi_bits; i++) {
> >> set_gsi(s, i);
> >> }
> >>
> >> So if you take gsi_count = 1024, what happens?
> >>
> >> gsi_count = 1024;
> >> gsi_bits = 1024;
> >> for (i = 1024; i < 1024; i++) {
> >> set_gsi(s, i);
> >> }
> >>
> >> At least in my world of C that loop never runs, no?
> >>
> > But then kvm_irqchip_get_virq() should never return 1024, shouldn't it?
>
> Right, because it returns the virq number which starts at 0. However, to
> describe all virqs from [0..1023] we need 1024 entries which the kernel
> errors out on.
Ah... that's kvm_irq_routing::nr and not kvm_irq_routing_entry::gsi, so
it's basically a kernel misfeature we need to work around.
>
> >
> > And:
> >
> > void kvm_irqchip_add_irq_route(KVMState *s, int irq, int irqchip, int pin)
> > {
> > [...]
> > assert(pin < s->gsi_count);
> >
> > would trigger too early with your change, wouldn't it?
>
> Not really - with my change we only support 1023 virqs. So the biggest
> virq number is 1022 which is < 1023 :).
>
>
> Sorry for describing this with actual numbers - I find it easier to
> grasp when I think in concrete numbers here - this stuff is just really
> spinning my head :).
And on top of that, it's Friday :)
But yes, makes sense now.
Acked-by: Cornelia Huck <address@hidden>
Re: [Qemu-devel] [PATCH] KVM: Fix GSI number space limit, Paolo Bonzini, 2014/06/06