Re: [PATCH v4 2/9] x86/passthrough: Wrap pt_irq_create_bind() restart block in braces

[Teddy Astie <teddy.astie@xxxxxxxxxx>]

Le 27/04/2026 à 15:57, Julian Vetter a écrit :
> Enclose the restart/retry block in pt_irq_create_bind() in an explicit
> compound statement to prepare for its extraction into a helper function.
> No functional change.
> 
> Signed-off-by: Julian Vetter <julian.vetter@xxxxxxxxxx>
> ---
> Changes in v4:
> - New patch
> - Split out as a preparatory no-functional-change step to make the diff
>    in patch 5 (pt_irq_bind_msi() interface change) easier to review
> ---
>   xen/drivers/passthrough/x86/hvm.c | 80 ++++++++++++++++---------------
>   1 file changed, 41 insertions(+), 39 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/x86/hvm.c 
> b/xen/drivers/passthrough/x86/hvm.c
> index b73bb55055..691fa1b2c7 100644
> --- a/xen/drivers/passthrough/x86/hvm.c
> +++ b/xen/drivers/passthrough/x86/hvm.c
> @@ -229,52 +229,54 @@ int pt_irq_create_bind(
>           return -EINVAL;
>   
>    restart:
> -    write_lock(&d->event_lock);
> -
> -    hvm_irq_dpci = domain_get_irq_dpci(d);
> -    if ( !hvm_irq_dpci && !is_hardware_domain(d) )
>       {
> -        unsigned int i;
> +        write_lock(&d->event_lock);
>   
> -        /*
> -         * NB: the hardware domain doesn't use a hvm_irq_dpci struct because
> -         * it's only allowed to identity map GSIs, and so the data contained 
> in
> -         * that struct (used to map guest GSIs into machine GSIs and perform
> -         * interrupt routing) is completely useless to it.
> -         */
> -        hvm_irq_dpci = xzalloc(struct hvm_irq_dpci);
> -        if ( hvm_irq_dpci == NULL )
> +        hvm_irq_dpci = domain_get_irq_dpci(d);
> +        if ( !hvm_irq_dpci && !is_hardware_domain(d) )
> +        {
> +            unsigned int i;
> +
> +            /*
> +             * NB: the hardware domain doesn't use a hvm_irq_dpci struct 
> because
> +             * it's only allowed to identity map GSIs, and so the data 
> contained in
> +             * that struct (used to map guest GSIs into machine GSIs and 
> perform
> +             * interrupt routing) is completely useless to it.
> +             */
> +            hvm_irq_dpci = xzalloc(struct hvm_irq_dpci);
> +            if ( hvm_irq_dpci == NULL )
> +            {
> +                write_unlock(&d->event_lock);
> +                return -ENOMEM;
> +            }
> +            for ( i = 0; i < NR_HVM_DOMU_IRQS; i++ )
> +                INIT_LIST_HEAD(&hvm_irq_dpci->girq[i]);
> +
> +            hvm_domain_irq(d)->dpci = hvm_irq_dpci;
> +        }
> +
> +        info = pirq_get_info(d, pirq);
> +        if ( !info )
>           {
>               write_unlock(&d->event_lock);
>               return -ENOMEM;
>           }
> -        for ( i = 0; i < NR_HVM_DOMU_IRQS; i++ )
> -            INIT_LIST_HEAD(&hvm_irq_dpci->girq[i]);
> -
> -        hvm_domain_irq(d)->dpci = hvm_irq_dpci;
> -    }
> -
> -    info = pirq_get_info(d, pirq);
> -    if ( !info )
> -    {
> -        write_unlock(&d->event_lock);
> -        return -ENOMEM;
> -    }
> -    pirq_dpci = pirq_dpci(info);
> +        pirq_dpci = pirq_dpci(info);
>   
> -    /*
> -     * A crude 'while' loop with us dropping the spinlock and giving
> -     * the softirq_dpci a chance to run.
> -     * We MUST check for this condition as the softirq could be scheduled
> -     * and hasn't run yet. Note that this code replaced tasklet_kill which
> -     * would have spun forever and would do the same thing (wait to flush out
> -     * outstanding hvm_dirq_assist calls.
> -     */
> -    if ( pt_pirq_softirq_active(pirq_dpci) )
> -    {
> -        write_unlock(&d->event_lock);
> -        cpu_relax();
> -        goto restart;
> +        /*
> +         * A crude 'while' loop with us dropping the spinlock and giving
> +         * the softirq_dpci a chance to run.
> +         * We MUST check for this condition as the softirq could be scheduled
> +         * and hasn't run yet. Note that this code replaced tasklet_kill 
> which
> +         * would have spun forever and would do the same thing (wait to 
> flush out
> +         * outstanding hvm_dirq_assist calls.
> +         */
> +        if ( pt_pirq_softirq_active(pirq_dpci) )
> +        {
> +            write_unlock(&d->event_lock);
> +            cpu_relax();
> +            goto restart;
> +        }
>       }
>   
>       switch ( pt_irq_bind->irq_type )

shows up as whitespace only changes (aside brackets)

Reviewed-by: Teddy Astie <teddy.astie@xxxxxxxxxx>


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech