On 27.08.2026 17:21, Oleksii Kurochko wrote:
> @@ -419,7 +416,41 @@ static __maybe_unused bool decode_ldst_insn(struct
> decoded_insn *di,
>
> static int emulate_load(const struct guest_fault *gf)
> {
> - return -EOPNOTSUPP;
> + struct cpu_user_regs *regs = gf->regs;
> + mmio_info_t info = { .is_write = false };
> + struct decoded_insn di;
> + unsigned int shift = 0;
> + int rc;
> +
> + /* A fault taken re-reading the instruction is redirected to the guest.
> */
> + if ( insn_fetch_faulted(gf, &di) )
> + return 0;
> +
> + if ( !decode_ldst_insn(&di, guest_xlen(regs)) || di.is_write )
> + return -EOPNOTSUPP;
> +
> + if ( !di.is_unsigned )
> + shift = BITS_PER_BYTE * (sizeof(unsigned long) - di.len);
This is one of the cases where sizeof(<type>) is not only unclear to
read, but actively risky: Which variable(s) of that type does this
refer to? What if those variable(s)' type(s) change? Aha, ...
> +#ifdef EMULATE_LOAD_DEBUG
> + gdprintk(XENLOG_DEBUG, "pc=%#lx, addr=%#"PRIpaddr", len=%u, shift=%u\n",
> + regs->sepc, gf->gpa, di.len, shift);
> +#endif
> +
> + rc = do_mmio(&info, gf->gpa, di.len);
> + if ( rc )
> + return rc;
> +
> + /*
> + * A load into x0 discards its result: writing regs->zero would break the
> + * invariant that it reads as zero when x0 is a source operand elsewhere.
> + */
> + if ( di.reg )
> + *guest_gpr(regs, di.reg) = (long)(info.data << shift) >> shift;
... you apparently mean sizeof(info.data) there.
The comment is (nit) also too long for my taste. Everything from the
colon onwards is imo redundant.
Jan
