On 9/14/26 1:48 PM, Jan Beulich wrote:
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.
Good point. I will try to follow such approach in future and use the
variable name instead of a type.
The comment is (nit) also too long for my taste. Everything from the
colon onwards is imo redundant.
Probably you are right. It is a little bit obvious just from the
defintion of x0 that it should be always zero. I will drop that part of
the comment after the colon.
THanks.
~ Oleksii
