Re: [PATCH v4 3/9] x86/passthrough: Extract pt_irq_dpci_setup() from pt_irq_create_bind()

Jan Beulich <[email protected]>
Newsgroups gmane.comp.emulators.xen.devel
Message-ID <[email protected]>
On 27.04.2026 15:54, Julian Vetter wrote:
> @@ -238,10 +242,11 @@ int pt_irq_create_bind(
>              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.
> +             * 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 )

This wants to be part of the patch increasing indentation.

> @@ -269,15 +274,36 @@ int pt_irq_create_bind(
>           * 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.
> +         * outstanding hvm_dirq_assist calls).
>           */

This would also better be part of the earlier patch. And this block needs re-
flowing there as well.

>          if ( pt_pirq_softirq_active(pirq_dpci) )
>          {
>              write_unlock(&d->event_lock);
>              cpu_relax();
> -            goto restart;
> +            continue;
>          }
> -    }
> +
> +        *hvm_irq_dpci_out = hvm_irq_dpci;
> +        *pirq_dpci_out = pirq_dpci;
> +        *info_out = info;
> +        return 0;
> +    } while ( true );

do { } while ( false ) (as Teddy suggests) wouldn't be much better. Why not
simply for ( ; ; ), as we have it in quite a few places elsewhere? Yet then
I'm not overly happy to see this secondary change (to complicated code) be
folded into a change of entirely different purpose. Please consider
(further) splitting.

Jan
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.