Re: [PATCH v2 38/39] xen/riscv: implement continue_new_vcpu()

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 9/22/26 12:20 PM, Jan Beulich wrote:
> On 22.09.2026 10:23, Oleksii Kurochko wrote:
> > On 9/21/26 2:12 PM, Jan Beulich wrote:
> > > On 27.08.2026 17:21, Oleksii Kurochko wrote:
> > > > continue_new_vcpu() is the arch hook invoked the first time a freshly
> > > > created vCPU is scheduled. Implement both cases it has to cover:
> > > >    - for the idle vCPU, switch to its own stack and jump to idle_loop();
> > > >    - for a guest vCPU, restore hstatus and enter the guest through the new
> > > >      return_to_new_vcpu() path in entry.S, which loads sepc, passes the
> > > >      hart id in a0 and the DTB address in a1 as expected by the RISC-V
> > > >      boot protocol, sets sstatus.SPP and executes sret.
> > > Is this a requirement for all CPUs, or just for the boot one? (I can't
> > > quite see why secondary processors would need passing a DTB address.)
> > It is requirement for boot one. For secondary processors it is HSM boot
> > data which is passed to sbi_hsm_hart_start() and then intercpeted by Xen.
> >
> > At the moment of writing of this commit message we have only boot CPU
> > and so only DTB could be passed.
> That you're talking about Xen. Despite Xen being UP only right now, guests
> still can have more than one vCPU, can't they?
They can.

> > I can update the commit message and the comment in return_to_new_vcpu()
> > to tell that it could be DTB address for boot cpu and/or for secondary
> > CPUs HSM boot data or it will be better to add info about HSM boot data
> > during and an introduction of secondary CPUs support?
> As per above you want to deal with multi-vCPU guests right now.
Then I will update the commit message and the comment in 
return_to_new_vcpu() to say that a1 holds the DTB address for the boot 
vCPU, and for secondary vCPUs the opaque value the guest passed to SBI 
HSM hart_start().
For commit message:

 - for a guest vCPU, restore hstatus and enter the guest through the new
    return_to_new_vcpu() path in entry.S, which loads sepc, passes the
    hart id in a0 and, in a1, either the DTB address (boot vCPU) or the
    opaque argument of SBI HSM hart_start() (secondary vCPUs), as 
required by the RISC-V boot protocol and the SBI specification, sets 
sstatus.SPP and executes sret.
In the code:

        /*
         * .a1 holds the DTB address for the boot vCPU, or the opaque value
         * passed to SBI HSM hart_start() for secondary vCPUs
         */

> > > > +        /* Set guest mode to supervisor */
> > > > +        li      t0, SSTATUS_SPP
> > > > +        csrs    CSR_SSTATUS, t0
> > > > +
> > > > +        /* Enter guest */
> > > > +        sret
> > > > +END(return_to_new_vcpu)
> > > Aiui SRET does not switch stacks. Shouldn't you therefore clear sp here?
> > > And perhaps also other GPRs, not the least ra? Exposing hypervisor
> > > register values to guests is, well, a bit of a problem.
> > Good point, sret leaves all GPRs as they are, so the guest would indeed
> > see Xen's sp, ra and friends. Only a0 and a1 are architecturally
> > meaningful for a booting hart, so I'll clear every other GPR right
> > before sret.
> >
> > I will add the following before sret:
> >
> >           /*
> >            * sret doesn't switch stacks and leaves the GPRs alone, so every
> >            * register which isn't meaningful to the vCPU being started
> > has to be
> >            * cleared here: otherwise the guest would see Xen's values, sp
> > (this
> >            * vCPU's Xen stack) and ra among them.
> >            */
> >           .irp reg, ra, sp, gp, tp, t0, t1, t2, s0, s1, a2, a3, a4, a5,
> > a6, a7, \
> >                     s2, s3, s4, s5, s6, s7, s8, s9, s10, s11, t3, t4, t5, t6
> >           mv      \reg, zero
> >           .endr
> At which point discussing the clobbering of t0 in the comment ahead of
> the function also isn't needed anymore.
Indeed, I'll drop it.

~ Oleksii