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
