Re: [PATCH v4 4/9] xen/arm/gic: Allow adding interrupt to running VMs

[Julien Grall <julien@xxxxxxx>]

Hi Henry,

On 23/05/2024 08:40, Henry Wang wrote:
> Currently, adding physical interrupts are only allowed at
> the domain creation time. For use cases such as dynamic device
> tree overlay addition, the adding of physical IRQ to
> running domains should be allowed.
>
> Drop the above-mentioned domain creation check. Since this
> will introduce interrupt state unsync issues for cases when the
> interrupt is active or pending in the guest, therefore for these
> cases we simply reject the operation. Do it for both new and old
> vGIC implementations.
>
> Signed-off-by: Henry Wang <xin.wang2@xxxxxxx>
With one remark below:

Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>

> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
> index b9463a5f27..048e12c562 100644
> --- a/xen/arch/arm/vgic/vgic.c
> +++ b/xen/arch/arm/vgic/vgic.c
> @@ -876,8 +876,11 @@ int vgic_connect_hw_irq(struct domain *d, struct vcpu 
> *vcpu,
>   
>       if ( connect )                      /* assign a mapped IRQ */
>       {
> -        /* The VIRQ should not be already enabled by the guest */
> -        if ( !irq->hw && !irq->enabled )
> +        /*
> +         * The VIRQ should not be already enabled by the guest nor
> +         * active/pending in the guest
Typo: Missing full stop.

It can be fixed on commit.

> +         */
> +        if ( !irq->hw && !irq->enabled && !irq->active && !irq->pending_latch )
>           {
>               irq->hw = true;
>               irq->hwintid = desc->irq;
Cheers,

--
Julien Grall