On 27.08.2026 17:20, Oleksii Kurochko wrote: > asm/riscv_encoding.h already provides INSN_16BIT_MASK and INSN_LEN(), and > emulate.c uses them, so the tree carried two spellings of the same thing > which could drift apart. COMPRESSED_INSN_MASK never had a user. > > No functional change. > > Signed-off-by: Oleksii Kurochko <[email protected]>
As I'm happy to see the duplication go away: Acked-by: Jan Beulich <[email protected]> However, ... > --- a/xen/arch/riscv/traps.c > +++ b/xen/arch/riscv/traps.c > @@ -214,7 +214,7 @@ void do_trap(struct cpu_user_regs *cpu_regs) > die(); > } > > - cpu_regs->sepc += GET_INSN_LENGTH(*(uint16_t *)pc); > + cpu_regs->sepc += INSN_LEN(*(uint16_t *)pc); ... for one I'd prefer if we took the opportunity and add "const" to the pointer target here. And then #define INSN_16BIT_MASK 0x3 #define INSN_32BIT_MASK 0x1c are really named backwards, seeing e.g. their use in #define INSN_IS_16BIT(insn) \ (((insn) & INSN_16BIT_MASK) != INSN_16BIT_MASK) #define INSN_IS_32BIT(insn) \ (((insn) & INSN_16BIT_MASK) == INSN_16BIT_MASK && \ ((insn) & INSN_32BIT_MASK) != INSN_32BIT_MASK) Furthermore, #define INSN_LEN(insn) (INSN_IS_16BIT(insn) ? 2 : 4) fails to use INSN_32BIT_MASK / INSN_IS_32BIT() altogether. Jan
