Re: [PATCH v2 16/39] xen/riscv: extend exception tables with type and data fields

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

On 9/9/26 2:22 PM, Jan Beulich wrote:
> On 09.09.2026 13:20, Oleksii Kurochko wrote:
>> On 9/8/26 3:44 PM, Jan Beulich wrote:
>>> On 27.08.2026 17:21, Oleksii Kurochko wrote:
>>>> @@ -59,7 +67,49 @@ static void ex_handler_fixup(const struct exception_table_entry *ex,
>>>>        regs->sepc = ex_fixup(ex);
>>>>    }
>>>>    
>>>> -bool fixup_exception(struct cpu_user_regs *regs)
>>>> +#define CHECK_GPR_INDEX(num, name)                      \
>>>> +    BUILD_BUG_ON(offsetof(struct cpu_user_regs, name)   \
>>>> +                 != (num) * sizeof(unsigned long));
>>>
>>> Nit: Placement of the !=. Also there should be no semicolon here; it wants
>>> to ...
>>>
>>>> +static unsigned long regs_get_gpr(const struct cpu_user_regs *regs,
>>>> +                                  unsigned int num)
>>>> +{
>>>> +    /*
>>>> +     * The GPR number -> struct index mapping below relies on x0..x31 being
>>>> +     * laid out at the start of struct cpu_user_regs in architectural order,
>>>> +     * matching the register numbers GPR_LIST() hands to the assembler.
>>>> +     */
>>>> +    GPR_LIST(CHECK_GPR_INDEX)
>>>
>>> ... appear here instead, for this to actually look like a statement.
>>
>> I will apply that.
>>
>>>
>>>> +    ASSERT(num < 32);
>>>> +
>>>> +    return ((const unsigned long *)regs)[num];
>>>
>>> What about release builds? You'd happily overrun the array there. Maybe
>>> (ab)use array_index_nospec() here?
>>
>> num is coming not from guest, not from calculation in runtume, it is
>> generated by assembler at the build time. So it should be always correct.
>>
>> So just having the following looks okay to me:
>>
>> static unsigned long regs_get_gpr(const struct cpu_user_regs *regs,
>>                                     unsigned int num)
>> {
>> #define CHECK_GPR_INDEX(num, name) \
>>       BUILD_BUG_ON(offsetof(struct cpu_user_regs, name) != \
>>                    (num) * sizeof(unsigned long))
>>
>> #define GPR_CASE(nr, name) case nr: return regs->name;
>>
>>       /*
>>        * The GPR number -> struct index mapping below relies on x0..x31 being
>>        * laid out at the start of struct cpu_user_regs in architectural
>> order,
>>        * matching the register numbers GPR_LIST() hands to the assembler.
>>        */
>>       GPR_LIST(CHECK_GPR_INDEX);
>>
>> #undef CHECK_GPR_INDEX
>>
>>       switch ( num )
>>       {
>>       GPR_LIST(GPR_CASE)
>>       }
>>
>> #undef GPR_CASE
>>
>>       ASSERT_UNREACHABLE();
>>
>>       return 0;
>> }
>>
>> Any thoughts on that regard?
> 
> Depends very much on how efficiently the compiler translates this (as opposed
> to the other variant).

It is worthier. I will follow then your original suggestion and use 
array_index_nospec().

~ Oleksii