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

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2120 bytes --]


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

[-- Attachment #2: Type: text/html, Size: 3234 bytes --]