Re: [Xen-devel] [PATCH v4 1/2] arm: support fewer LR registers than virtual irqs

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 1 Mar 2012, Ian Campbell wrote:
> On Wed, 2012-02-29 at 18:34 +0000, Stefano Stabellini wrote:
> 
> > > > +    {
> > > > +        i = find_first_zero_bit(&gic.lr_mask, sizeof(uint64_t));
> > > > +        if (i < sizeof(uint64_t)) {
> > > 
> > > What does find_first_zero_bit(0xffff.fff, 64) return?
> > 
> > 0
> 
> So the if is wrong?
> 
> What does it return for 0x0? I'd have expected it to return 0 too (the
> zeroth bit). I guess the response must be 1-based? In which case don't
> you need to subtract 1 somewhere so that bit 1 being clear leads you to
> use LR0?

Ops, sorry, I misread your previous comment. It would return
sizeof(uint64_t), hence the if should work as expected.


> Also, it occurs to me that you need to check i against the number of LRs
> too -- otherwise if you have 4 LRs all in use then mask is 0xF but
> find_first_zero_bit(0xF, sizeof(...) will return 5 (or is it six?) and
> you will try and deploy the non-existent 5th LR.

Yes, I realized that yesterday while I was reading through the code again.
We need to pass nr_lrs instead of sizeof(uint64_t) and check against it.


> I'm a bit concerned that the #irqs > #LRs code paths probably haven't
> been run, even though you tested on a system, with only 4 LRs. Perhaps
> you could artificially inject a bunch of unused/spurious interrupts at
> the same time e.g. from a keyhandler?

Actually after the few fixes discussed in this email thread I limited
the number of LRs to 1, and I can see IRQs being added/removed from
pending_irq the way they are supposed to.


> Also there is an option in the model (at build time for sure, perhaps at
> runtime too via -C parameters) to reduce the number of LRs, perhaps
> setting it to 1 or 2 would help exercise these code paths a bit more
> than 4? We are unlikely to be hitting 5 concurrent pending interrupts
> with our current uses.

indeed

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel