Re: [PATCH v4] xen/riscv: allow Xen to use SSTC while hiding it from guests

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.04.2026 11:33, Oleksii Kurochko wrote:
> On 4/21/26 11:10 AM, Jan Beulich wrote:
>> On 21.04.2026 11:01, Oleksii Kurochko wrote:
>>> On 4/20/26 9:56 AM, Jan Beulich wrote:
>>>> On 17.04.2026 09:24, Oleksii Kurochko wrote:
>>>>> @@ -279,8 +299,6 @@ static int cf_check sbi_set_timer_v01(uint64_t 
>>>>> stime_value)
>>>>>        return sbi_err_map_xen_errno(ret.error);
>>>>>    }
>>>>>    
>>>>> -int (* __ro_after_init sbi_set_timer)(uint64_t stime_value) = 
>>>>> sbi_set_timer_v01;
>>>>> -
>>>>>    int sbi_remote_sfence_vma(const cpumask_t *cpu_mask, vaddr_t start,
>>>>>                              size_t size)
>>>>>    {
>>>>> @@ -360,10 +378,9 @@ int __init sbi_init(void)
>>>>>            }
>>>>>    
>>>>>            if ( sbi_probe_extension(SBI_EXT_TIME) > 0 )
>>>>> -        {
>>>>> -            sbi_set_timer = sbi_set_timer_v02;
>>>>> -            dprintk(XENLOG_INFO, "SBI v0.2 TIME extension detected\n");
>>>>> -        }
>>>>> +            set_xen_timer = sbi_set_timer_v02;
>>>>> +        else
>>>>> +            set_xen_timer = sbi_set_timer_v01;
>>>>>        }
>>>>
>>>> Sadly this isn't quite equivalent to sbi_set_timer having had an 
>>>> initializer.
>>>> I would have wanted to suggest to use a constructor function, but we call
>>>> init_constructors() even later than do_initcalls() on both Arm and x86 (we
>>>> don't call the latter at all on RISC-V so far). Might it be necessary to
>>>> introduce sbi_early_init(), called very early during boot? Else how do you
>>>> guarantee no accidental use of the variable before it is first set?
>>>
>>> I thought about an introduction of sbi_early_init() but then decided
>>> that set_xen_timer() won't be used earlier than at lest timer_init() +
>>> local_irq_enable().
>>> Also, sbi_init() is executed pretty early.
>>
>> Many more additions to setup.c are to be expected. Are you sure hardly any 
>> will
>> go ahead of the call to sbi_init()?
> 
> Looking at the current state, I don't see something new what will added 
> before sbi_init() except percpu_init_areas().
> 
> I am okay to introduce sbi_early_init() if it will be really better:
> 
> --- a/xen/arch/riscv/sbi.c
> +++ b/xen/arch/riscv/sbi.c
> @@ -387,3 +387,8 @@ int __init sbi_init(void)
> 
>       return 0;
>   }
> +
> +void __init sbi_early_init(void)
> +{
> +    set_xen_timer = sbi_set_timer_v01;
> +}
> diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
> index 56a0907a855f..b187a84cd28d 100644
> --- a/xen/arch/riscv/setup.c
> +++ b/xen/arch/riscv/setup.c
> @@ -78,6 +78,8 @@ void __init noreturn start_xen(unsigned long bootcpu_id,
>       const char *cmdline;
>       size_t fdt_size;
> 
> +    sbi_early_init();
> 
> But it looks to me that is fine to have what we have now as even someone 
> will try to use set_xen_timer earlier a trap will occur and thereby it 
> will be need to put the code which start to use set_xen_timer after 
> sbi_init().

It's your call really.

Jan