[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 14/22] x86/boot: choose AP stack based on APIC ID



On Thu, Jan 22, 2026 at 04:52:36PM +0100, Jan Beulich wrote:
> > +        /* Not x2APIC, read from MMIO */
> > +        and     $APIC_BASE_ADDR_MASK, %eax
> > +        mov     APIC_ID(%eax), %esp
> > +        shr     $24, %esp
>
> I have to admit that I'm rather hesitant towards seeing %esp used like this.

I'll switch to %ebp to pass APIC ID.

> > --- a/xen/arch/x86/boot/x86_64.S
> > +++ b/xen/arch/x86/boot/x86_64.S
> > @@ -15,7 +15,33 @@ ENTRY(__high_start)
> >          mov     $XEN_MINIMAL_CR4,%rcx
> >          mov     %rcx,%cr4
> >
> > -        mov     stack_start(%rip),%rsp
> > +        test    %ebx,%ebx
> > +        cmovz   stack_start(%rip), %rsp
> > +        jz      .L_stack_set
> > +
> > +        /* APs only: get stack base from APIC ID saved in %esp. */
> > +        mov     $-1, %rax
>
> Here and below 32-bit insn would do fine. However, ...

Are all addresses guaraneed to be below 4 GiB?

> > +        lea     x86_cpu_to_apicid(%rip), %rcx
> > +1:
> > +        add     $1, %rax
> > +        cmp     $NR_CPUS, %eax
> > +        jb      2f
> > +        hlt
> > +2:
> > +        cmp     %esp, (%rcx, %rax, 4)
> > +        jne     1b
>
> ... can't all of this be a simple REPNE SCASL?

It can, but then can't have an upper bound, right?

> As to the upper bound of NR_CPUS, do we really need to look this far?

Are you suggesting to use `max_cpus` instead?

> > +        /* %eax is now Xen CPU index. */
> > +        lea     stack_base(%rip), %rcx
> > +        mov     (%rcx, %rax, 8), %rsp
> > +
> > +        test    %rsp,%rsp
>
> Nit: Blank after comma please.

Sure.

Regards,
Sergii



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.