Re: [PATCH v3 23/39] xen/riscv: add helpers for decoding a trapped load or store

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.11.2026 17:38, Oleksii Kurochko wrote:
> On 10/22/26 12:02 PM, Baptiste Le Duc wrote:
>>> + * It is needed to decode a trapped instruction: the encodings which exist 
>>> only
>>> + * for XLEN=64 must not be recognized for a 32-bit guest. Besides those 
>>> simply
>>> + * being reserved there, the compressed ones are ambiguous: C.LD and C.FLW
>>> + * share the encoding 0x6000 (mask 0xe003), and likewise C.SD/C.FSW,
>>> + * C.LDSP/C.FLWSP and C.SDSP/C.FSWSP.
>>> + *
>>> + * IS_ENABLED() can't be used here as HSTATUS_VSXL is defined for
>>> + * __riscv_xlen == 64 only, the field not existing on RV32 in the first 
>>> place.
>>> + */
>>> +static unsigned int guest_xlen(const struct cpu_user_regs *regs)
>>> +{
>>> +#ifdef CONFIG_RISCV_32
>>> +    return 32;
>>> +#else
>>> +    unsigned long xl = MASK_EXTR(regs->hstatus, HSTATUS_VSXL);
>> xl only ever holds a 2-bit field, so unsigned int would be better here.
> 
> MASK_EXTR() yields the type of its operands, unsigned long here (and 
> unsigned long long for the UXL extraction), as do the XLEN_FIELD_* 
> constants xl is compared against. Making xl unsigned int would add an 
> implicit narrowing conversion: harmless value-wise but I think it will 
> violate one of MISRA rules at least if build everything with 
> -Wconversion then it will tell: conversion from 'long unsigned int' to 
> 'unsigned int' may change value.

If we were to follow that Misra rule, we'd have violations all over the
place. Hence imo where unsigned int is sufficient for a value, that's
what should be used.

>>> [ ... 49 lines skipped ... ]
>>> +static unsigned long *guest_gpr(struct cpu_user_regs *regs,
>>> +                                const struct decoded_insn *di)
>>> +{
>>> +    /*
>>> +     * decode_ldst_insn() is what fills @di in, and it rejects an access
>>> +     * wider than a guest register.
>>> +     */
>>> +    ASSERT(di->len <= (guest_xlen(regs) / BITS_PER_BYTE));
>> Nit: the parentheses around the division aren't needed.
> 
> I can drop them but I think it improves readability here a little bit.

That's subjective. Personally I agree with Baptiste, yet project-wide
we don't do things consistently in this regard, and hence either form
is (supposedly) okay.

Jan