On Fri, 06 Jun 2014 15:28:13 +0200
Alexander Graf <agraf@xxxxxxx> wrote:
>
> On 06.06.14 15:23, Cornelia Huck wrote:
> > On Fri, 06 Jun 2014 15:15:54 +0200
> > Alexander Graf <agraf@xxxxxxx> wrote:
> >
> >> On 06.06.14 15:12, Cornelia Huck wrote:
> >>> On Fri, 6 Jun 2014 14:46:05 +0200
> >>> Alexander Graf <agraf@xxxxxxx> 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 <agraf@xxxxxxx>
> >>>> ---
> >>>> 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 <cornelia.huck@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
