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