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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

> > +static void noreturn idle_loop(void)
> > +{
> > +    BUG_ON("unimplemented");
> > +}
> > diff --git a/xen/arch/riscv/entry.S b/xen/arch/riscv/entry.S
> > index 331446a238..bf1843dcea 100644
> > --- a/xen/arch/riscv/entry.S
> > +++ b/xen/arch/riscv/entry.S
> > @@ -143,3 +143,26 @@ FUNC(__context_switch)
> >   
> >           ret
> >   END(__context_switch)
> > +
> > +/* t0 is used as a temporary reg and is clobbered to oblivion */
> > +FUNC(return_to_new_vcpu)
> > +        /* Swap tp with sscratch */
> > +        csrrw   tp, CSR_SSCRATCH, tp
> Why do we need a swap? After it, tp will be equal to zero, but we won't
> use it. Couldn't we do a basic csrw instead?
SSCRATCH needs to hold pcpu_info while the guest runs and be 0 while Xen 
runs. The trap entry will then do csrrw tp, sscratch, tp, and the 
resulting tp says where the trap came from:
- Non-zero: the trap came from the guest. tp now points to pcpu_info and 
the guest's tp is parked in SSCRATCH.
- Zero: the trap came from Xen. Recover tp with csrr tp, sscratch and 
write SSCRATCH back to 0.
On trap entry the swap really is needed. No register is free there, so 
one instruction has to save the guest's tp and load pcpu_info at once.
But considering just the line you commented I think csrw could be 
enough. I will double-check during an introduction of this line in more 
appropriate patch.
Considering that it can't be clear for now why it is needed I will move 
this part to the patch (which updates handle_trap() with guest trap 
support handling).
Thanks.

~ Oleksii