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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.04.2026 17:49, Oleksii Kurochko wrote:
> 
> 
> On 4/16/26 5:45 PM, Jan Beulich wrote:
>> On 16.04.2026 17:37, Oleksii Kurochko wrote:
>>>
>>>
>>> On 4/16/26 9:50 AM, Jan Beulich wrote:
>>>> On 10.04.2026 19:41, Andrew Cooper wrote:
>>>>> On 10/04/2026 4:45 pm, Oleksii Kurochko wrote:
>>>>>> diff --git a/xen/arch/riscv/time.c b/xen/arch/riscv/time.c
>>>>>> index 7efa76fdbcb1..80f0e9ddae6a 100644
>>>>>> --- a/xen/arch/riscv/time.c
>>>>>> +++ b/xen/arch/riscv/time.c
>>>>>> @@ -91,4 +90,23 @@ void __init preinit_xen_time(void)
>>>>>>            panic("%s: ACPI isn't supported\n", __func__);
>>>>>>    
>>>>>>        boot_clock_cycles = get_cycles();
>>>>>> +
>>>>>> +    /* set_xen_timer must have been set by sbi_init() already */
>>>>>> +    ASSERT(set_xen_timer);
>>>>>> +
>>>>>> +    if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_sstc) )
>>>>>> +    {
>>>>>> +        set_xen_timer = sstc_set_xen_timer;
>>>>>> +
>>>>>> +        /*
>>>>>> +         * A VS-timer interrupt becomes pending whenever the value of
>>>>>> +         * (time + htimedelta) is greater than or equal to vstimecmp 
>>>>>> CSR.
>>>>>> +         * Thereby to avoid spurious VS-timer irqs set vstimecmp CSR to
>>>>>> +         * ULONG_MAX.
>>>>>> +         */
>>>>>> +        csr_write(CSR_VSTIMECMP, ULONG_MAX);
>>>>>> +#ifdef CONFIG_RISCV_32
>>>>>> +        csr_write(CSR_VSTIMECMPH, ULONG_MAX);
>>>>>> +#endif
>>>>>
>>>>> You've got this pattern twice in this patch alone, and these aren't the
>>>>> only CSRs which are formed of pairs to get a 64bit value in 32bit mode.
>>>>>
>>>>> Sadly, the numbering isn't consistent for the high constant, but we can
>>>>> let the compiler do most of the hard work for us.
>>>>>
>>>>> #ifdef CONFIG_RISCV_32
>>>>> # define __csr_write32h(csr, val) csr_write(csr ## H, (val) >> 32)
>>>>> #else
>>>>> # define __csr_write32h(csr, val) (void)(csr, val)
>>>>
>>>> Just to mention: Since the comma is an operator here (not a lexical element
>>>> separating function arguments), more parenthesization may be needed for
>>>> Misra's sake.
>>>
>>> I will define it in the next way:
>>>
>>> # define __csr_write32h(csr, val) (void)((csr), (val))
>>
>> Actually, yet more parentheses are needed:
>>
>> # define __csr_write32h(csr, val) ((void)((csr), (val)))
> 
> It seems like (void) should be for (csr) and (val), so it should be:
> 
> # define __csr_write32h(csr, val) ((void)(csr), (void)(val))
> 
> Otherwise the following compilation error occurs.
> 
> ./arch/riscv/include/asm/csr.h:38:48: error: left-hand operand of comma 
> expression has no effect [-Werror=unused-value]
>     38 | # define __csr_write32h(csr, val) ((void)((csr), (val)))

I was indeed wondering, but I did assume Andrew had checked his suggestion.
Then again maybe it was only meant as an outline anyway.

Jan