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).
Jan
