Re: [PATCH for-4.14 5/8] x86/hvm: only translate ISA interrupts to GSIs in virtual timers

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.06.2020 17:56, Roger Pau Monne wrote:
> Only call hvm_isa_irq_to_gsi for ISA interrupts, interrupts
> originating from an IO APIC pin already use a GSI and don't need to be
> translated.
> 
> I haven't observed any issues from this, but I think it's better to
> use it correctly.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

However, ...

> --- a/xen/arch/x86/hvm/vpt.c
> +++ b/xen/arch/x86/hvm/vpt.c
> @@ -86,7 +86,7 @@ static int pt_irq_vector(struct periodic_time *pt, enum 
> hvm_intsrc src)
>          return pt->irq;
>  
>      isa_irq = pt->irq;
> -    gsi = hvm_isa_irq_to_gsi(isa_irq);
> +    gsi = pt->source == PTSRC_isa ? hvm_isa_irq_to_gsi(isa_irq) : pt->irq;

... would you mind taking the opportunity and moving this ...

>      if ( src == hvm_intsrc_pic )
>          return (v->domain->arch.hvm.vpic[isa_irq >> 3].irq_base

... below here, perhaps even past the ASSERT() that follows?

(I have to admit that I find the two kinds of "source" indicators
- the "src" function parameter and "pt->source" confusing. Aren't
they supposed to match up?)

Jan