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

Jan Beulich <[email protected]>
Newsgroups gmane.comp.emulators.xen.devel
Message-ID <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.