Re: [PATCH v4 01/18] xen/riscv: detect and initialize G-stage mode

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 9/25/25 3:46 PM, Jan Beulich wrote:

> On 24.09.2025 17:00, Oleksii Kurochko wrote:
> > On 9/24/25 1:31 PM, Oleksii Kurochko wrote:
> > > > > --- a/xen/arch/riscv/setup.c
> > > > > +++ b/xen/arch/riscv/setup.c
> > > > > @@ -22,6 +22,7 @@
> > > > >   #include <asm/early_printk.h>
> > > > >   #include <asm/fixmap.h>
> > > > >   #include <asm/intc.h>
> > > > > +#include <asm/p2m.h>
> > > > >   #include <asm/sbi.h>
> > > > >   #include <asm/setup.h>
> > > > >   #include <asm/traps.h>
> > > > > @@ -148,6 +149,8 @@ void __init noreturn start_xen(unsigned long bootcpu_id,
> > > > >   
> > > > >       console_init_postirq();
> > > > >   
> > > > > +    gstage_mode_detect();
> > > > >             
> > > > I find it odd for something as fine grained as this to be called from top-
> > > > level start_xen(). Imo this wants to be a sub-function of whatever does
> > > > global paging and/or p2m preparations (or even more generally guest ones).
> > > >           
> > > It makes sense. I will move the call to gstage_mode_detect() into p2m_init()
> > > when the latter is introduced.
> > > Probably, I will move the current patch after p2m_init() is introduced to make
> > > gstage_mode_detect() static function.
> > >         
> > Maybe putting gstage_mode_detect() into p2m_init() is not a good idea, since it
> > is called during domain creation. I am not sure there is any point in calling
> > gstage_mode_detect() each time.
> >
> > It seems that gstage_mode_detect() should be called once during physical CPU
> > initialization.
> >       
> Indeed.
>
> > A sub-function (riscv_hart_mm_init()? probably, riscv should be dropped from
> > the name) could be added in setup.c and then called in start_xen(), but
> > is it really needed a separate sub-function for something that will be called
> > once per initialization of pCPU?
> >       
> Counter question: Is this going to remain the only piece of global init that's
> needed for P2M machinery? Right in the next patch you already add vmid_init()
> as another top-level call.
>     
No, it isn’t the only piece — at least gstage_mode_detect() and vmid_init() are
also needed for the P2M machinery.
Okay, then it would be better to introduce a sub-function now and re-use it later
for other pCPUs as well.

~ Oleksii