Re: [PATCH v2 30/35] x86/hvm: add helpers for raising guest IRQs

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

On Thu, Dec 05, 2024 at 08:42:00PM -0800, Denis Mukhin via B4 Relay wrote:
> From: Denis Mukhin <dmukhin@xxxxxxxx>
> 
> Added convenience wrappers for asserting/de-asserting interrupts in the
> hardware emulation code.
> 
> That will be used for PCI-based NS8250 emulator.

Strictly speaking the ns8250 uart should only generate ISA interrupts
as I understand it, as it only uses IRQs 3 and 4?  IOW from that code
you should only need to use hvm_isa_irq_assert().

> 
> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
> ---
>  xen/arch/x86/hvm/irq.c             | 24 ++++++++++++++++++++++++
>  xen/arch/x86/include/asm/hvm/irq.h |  3 +++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c
> index 
> 1eab44defca4c82ec35769617c66c380cc07d1b6..9e3a50d21dcf281c1015116094e47795c51ed5d0
>  100644
> --- a/xen/arch/x86/hvm/irq.c
> +++ b/xen/arch/x86/hvm/irq.c
> @@ -242,6 +242,30 @@ void hvm_isa_irq_deassert(
>      spin_unlock(&d->arch.hvm.irq_lock);
>  }
>  
> +void hvm_irq_raise(struct domain *d, unsigned int irq)
> +{
> +    if ( irq < NR_ISAIRQS )
> +    {
> +        hvm_isa_irq_assert(d, irq, NULL);
> +    }
> +    else
> +    {
> +        hvm_gsi_assert(d, irq);
> +    }
> +}
> +
> +void hvm_irq_lower(struct domain *d, unsigned int irq)

It would be better to use the assert/deassert nomenclature, like it's
used for the functions that are called.

> +{
> +    if ( irq < NR_ISAIRQS )
> +    {
> +        hvm_isa_irq_deassert(d, irq);
> +    }
> +    else
> +    {
> +        hvm_gsi_deassert(d, irq);
> +    }
> +}

The parameter to thins function is kind of fuzzy, as I understand it,
if the parameter is < NR_ISAIRQS it's an ISA IRQ, while if it's >=
NR_ISAIRQS it's a GSI?

It would also be helpul to mention that hvm_isa_irq_deassert() will
already do the ISA IRQ -> GSI conversion in case there are any source
overrides.

Thanks, Roger.