Re: [PATCH v3 37/39] xen/riscv: implement continue_new_vcpu()

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 11/4/26 3:00 PM, Baptiste Le Duc 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, enter the guest through the new
> >     return_to_new_vcpu() path in entry.S, which restores hstatus and
> >     sepc, passes the hart id in a0 and, in a1, either the DTB address
> >     (boot vCPU, as expected by the RISC-V boot protocol) or the opaque
> >     value passed to SBI HSM hart_start() (secondary vCPUs), clears the
> >     remaining GPRs, sets sstatus.SPP and sstatus.SPIE and executes sret.
> Nit: the guest vCPU sentence is hard to follow in one go. Maybe split it:
>
>    - for a guest vCPU, enter the guest through the new
>      return_to_new_vcpu() path in entry.S. It restores hstatus and
>      sepc and passes the hart id in a0. a1 holds the DTB address for
>      the boot vCPU (as expected by the RISC-V boot protocol), or the
>      opaque value passed to SBI HSM hart_start() for secondary vCPUs.
>      It then clears the remaining GPRs, sets sstatus.SPP and
>      sstatus.SPIE, and executes sret.
Thanks for the suggestion. I'll use this wording.

> > [ ... 94 lines skipped ... ]
> > --- a/xen/arch/riscv/entry.S
> > +++ b/xen/arch/riscv/entry.S
> > @@ -147,3 +147,42 @@ FUNC(__context_switch)
> >   
> >           ret
> >   END(__context_switch)
> > +
> > +/*
> > + * Enter a vCPU for the first time. Must be called with interrupts disabled,
> > + * see continue_new_vcpu().
> > + */
> > +FUNC(return_to_new_vcpu)
> > +        REG_L   t0, CPU_USER_REGS_HSTATUS(sp)
> > +        csrw    CSR_HSTATUS, t0
> > +
> At this point regs->hstatus only has SPV | SPVP (arch_vcpu_create()), so
> VSXL is written as 0, which is a reserved encoding. That only gets fixed
> by "xen/riscv: set the guest's XLEN explicitly in hstatus.VSXL", which
> comes later in the series. Wouldn't it be better to move that one ahead
> of this patch?
Agreed. I'll move "xen/riscv: set the guest's XLEN explicitly in 
hstatus.VSXL" before this patch.
> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>
Thanks!

~ Oleksii