Re: [PATCH v1 16/17] xen/riscv: add guest load emulation for trapped MMIO accesses

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.08.2026 09:15, Jan Beulich wrote:
> On 29.07.2026 15:40, Oleksii Kurochko wrote:
>>  static int emulate_load(unsigned long fault_addr, unsigned long htinst)
>>  {
>> -    return -EOPNOTSUPP;
>> +    struct cpu_user_regs *regs = vcpu_guest_cpu_user_regs(current);
>> +    mmio_info_t info = { .is_write = false };
>> +    unsigned long insn;
>> +    unsigned int shift = 0, len, insn_len;
>> +    bool is_unsigned = false;
>> +    int rc;
>> +
>> +    if ( decode_trapped_insn(htinst, &insn, &insn_len) )
>> +        return 0;
>> +
>> +    /* Decode length of MMIO and whether it is a sign- or zero-extending 
>> load */
>> +    if ( (insn & INSN_MASK_LB) == INSN_MATCH_LB )
>> +        len = 1;
>> +    else if ( (insn & INSN_MASK_LBU) == INSN_MATCH_LBU )
>> +    {
>> +        len = 1;
>> +        is_unsigned = true;
>> +    }
>> +    else if ( (insn & INSN_MASK_LH) == INSN_MATCH_LH )
>> +        len = 2;
>> +    else if ( (insn & INSN_MASK_LHU) == INSN_MATCH_LHU )
>> +    {
>> +        len = 2;
>> +        is_unsigned = true;
>> +    }
>> +    else if ( (insn & INSN_MASK_LW) == INSN_MATCH_LW )
>> +        len = 4;
> 
> Already up to here this demonstrates a weakness of the INSN_MASK_*
> set of #define-s (which I similarly observe in binutils, and I expect it
> all has the same questionable origin). All INSN_MASK_L* and INSN_MASK_FL*
> (also INSN_MASK_S* and INSN_MASK_FS*) are identical, allowing for a nice
> switch() to be used here in principle. That said, with access width
> nicely encoded in FUNCT3, it's not even clear whether a switch() would
> end up being needed / efficient.
> 
> Otoh none of these masks cover the pseudoinsns that htinst may supply.
> 
> Further, what about A-extension insns? Some (if not all) of them can
> plausibly be used on MMIO, I think.

Because of the further additions that are going to be needed, may I also
suggest to consider putting emulation code in its own file (emulate.c
perhaps), rather than directly in traps.c?

Jan