Re: [Xen-devel] [PATCH v3 13/15] x86/IRQ: tighten vector checks

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, May 20, 2019 at 09:26:37AM -0600, Jan Beulich wrote:
> >>> On 20.05.19 at 16:04, <roger.pau@xxxxxxxxxx> wrote:
> > On Fri, May 17, 2019 at 04:52:32AM -0600, Jan Beulich wrote:
> >> Use valid_irq_vector() rather than "> 0".
> >> 
> >> Also replace an open-coded use of IRQ_VECTOR_UNASSIGNED.
> >> 
> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > 
> > Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Thanks.
> 
> > The question I have below is not directly related to the usage of
> > valid_irq_vector, but rather with the existing code.
> > 
> >> @@ -452,15 +452,18 @@ static vmask_t *irq_get_used_vector_mask
> >>              int vector;
> >>              
> >>              vector = irq_to_vector(irq);
> >> -            if ( vector > 0 )
> >> +            if ( valid_irq_vector(vector) )
> >>              {
> >> -                printk(XENLOG_INFO "IRQ %d already assigned vector %d\n",
> >> +                printk(XENLOG_INFO "IRQ%d already assigned vector %02x\n",
> >>                         irq, vector);
> >>                  
> >>                  ASSERT(!test_bit(vector, ret));
> >>  
> >>                  set_bit(vector, ret);
> >>              }
> >> +            else if ( vector != IRQ_VECTOR_UNASSIGNED )
> >> +                printk(XENLOG_WARNING "IRQ%d mapped to bogus vector 
> >> %02x\n",
> >> +                       irq, vector);
> > 
> > Maybe add an assert_unreachable here? It seems really bogus to call
> > irq_get_used_vector_mask with an unassigned vector.
> 
> How that? This would e.g. get called the very first time a vector
> is to be assigned. But I'm afraid I'm a little confused anyway by
> the wording you use - after all this is the code path dealing with
> an IRQ _not_ being marked as having no vector assigned, but
> also not having a valid vector.

Thanks for the clarification, by the name of the function I assumed it
must be called with an irq that has a vector assigned, if that's not
the case then I think it's fine.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel