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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan