Re: [PATCH 2/7] x86/wait: prevent duplicated assembly labels

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 14/03/2025 10:12 am, Roger Pau Monné wrote:
> On Fri, Mar 14, 2025 at 10:13:07AM +0100, Jan Beulich wrote:
>> But isn't it then going to be enough to latch &wqv->esp into a local variable,
>> and use that in the asm() and in the subsequent if()?
> I have the following diff which seems to prevent the duplication,
> would you both be OK with this approach?
>
> Thanks, Roger.
> ---
> diff --git a/xen/common/wait.c b/xen/common/wait.c
> index cb6f5ff3c20a..60ebd58a0abd 100644
> --- a/xen/common/wait.c
> +++ b/xen/common/wait.c
> @@ -124,6 +124,7 @@ static void __prepare_to_wait(struct waitqueue_vcpu *wqv)
>      struct cpu_info *cpu_info = get_cpu_info();
>      struct vcpu *curr = current;
>      unsigned long dummy;
> +    void *esp = NULL;
>  
>      ASSERT(wqv->esp == NULL);
>  
> @@ -166,12 +167,12 @@ static void __prepare_to_wait(struct waitqueue_vcpu *wqv)
>          ".L_skip:"
>          "pop %%r15; pop %%r14; pop %%r13;"
>          "pop %%r12; pop %%rbp; pop %%rbx"
> -        : "=&S" (wqv->esp), "=&c" (dummy), "=&D" (dummy)
> +        : "=&S" (esp), "=&c" (dummy), "=&D" (dummy)
>          : "0" (0), "1" (cpu_info), "2" (wqv->stack),
>            [sz] "i" (PAGE_SIZE)
>          : "memory", "rax", "rdx", "r8", "r9", "r10", "r11" );
>  
> -    if ( unlikely(wqv->esp == NULL) )
> +    if ( unlikely(esp == NULL) )
>      {
>          gdprintk(XENLOG_ERR, "Stack too large in %s\n", __func__);
>          domain_crash(curr->domain);
> @@ -179,6 +180,7 @@ static void __prepare_to_wait(struct waitqueue_vcpu *wqv)
>          for ( ; ; )
>              do_softirq();
>      }
> +    wqv->esp = esp;
>  }
>  
>  static void __finish_wait(struct waitqueue_vcpu *wqv)
>

If that works, then fine.  It's certainly less invasive.

The moment I actually get around to (or persuade someone else to) switch
the introspection mappings over to acquire_resource, then wait.c is
going to be deleted.

~Andrew