Re: [PATCH 4/4] x86/IRQ: ACKTYPE_NONE cannot make it into irq_guest_eoi_timer_fn()

["Roger Pau Monné" <roger.pau@citrix.com>]

On Wed, May 08, 2019 at 06:48:16AM -0600, Jan Beulich wrote:
> action->ack_type is set once before the timer even gets initialized, and
> is never changed later. The timer gets activated only for EOI and UNMASK
> types. Hence there's no need to have a respective if() in there. Replace
> it by an ASSERT().
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Just one comment below which I'm not overly fussed about.

> 
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -1103,7 +1103,7 @@ static void set_eoi_ready(void *data);
>  static void irq_guest_eoi_timer_fn(void *data)
>  {
>      struct irq_desc *desc = data;
> -    unsigned int irq = desc - irq_desc;
> +    unsigned int i, irq = desc - irq_desc;
>      irq_guest_action_t *action;
>      cpumask_t cpu_eoi_map;
>  
> @@ -1114,19 +1114,18 @@ static void irq_guest_eoi_timer_fn(void
>  
>      action = (irq_guest_action_t *)desc->action;
>  
> +    ASSERT(action->ack_type != ACKTYPE_NONE);
> +
>      if ( !action->in_flight || timer_is_active(&action->eoi_timer) )
>          goto out;
>  
> -    if ( action->ack_type != ACKTYPE_NONE )
> +    for ( i = 0; i < action->nr_guests; i++ )
>      {
> -        unsigned int i;
> -        for ( i = 0; i < action->nr_guests; i++ )
> -        {
> -            struct domain *d = action->guest[i];

I think you could constify d here.

Thanks.

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