Re: [PATCH v2 22/39] xen/riscv: add guest memory read helper

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 9/11/26 3:47 PM, Jan Beulich wrote:
> On 11.09.2026 15:41, Oleksii Kurochko wrote:
> > On 9/11/26 3:06 PM, Oleksii Kurochko wrote:
> > > On 9/9/26 2:04 PM, Baptiste Le Duc wrote:
> > > > > Introduce riscv_read_guest() to allow Xen to safely read guest memory
> > > > > using HLV/HLVX instructions while reliably capturing trap context.
> > > > > This is required for instruction fetch emulation and MMIO decoding,
> > > > > where
> > > > > Xen must inspect guest memory that may not be directly accessible and
> > > > > may
> > > > > fault.
> > > > >
> > > > > The implementation is based on kvm_riscv_vcpu_unpriv_read() from Linux,
> > > > > with one deviation: the hlv/hlvx instructions translate the guest
> > > > > address
> > > > > through the live vsatp/hgatp CSRs, i.e. through the address space of the
> > > > > currently running vCPU, so the function can only be called safely for
> > > > > current. Instead of taking a struct vcpu argument, it always operates on
> > > > > current directly.
> > > > >
> > > > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > > > >
> > > > > diff --git a/xen/arch/riscv/guestcopy.c b/xen/arch/riscv/guestcopy.c
> > > > > index 8a89212e0b..b2327822ac 100644
> > > > > --- a/xen/arch/riscv/guestcopy.c
> > > > > +++ b/xen/arch/riscv/guestcopy.c
> > > > > @@ -6,6 +6,7 @@
> > > > >    #include <xen/string.h>
> > > > >    #include <asm/guest_access.h>
> > > > > +#include <asm/traps.h>
> > > > >    #define COPY_from_guest     0U
> > > > >    #define COPY_to_guest       BIT(0, U)
> > > > > @@ -114,3 +115,89 @@ unsigned long copy_to_guest_phys(struct domain
> > > > > *d, paddr_t gpa, void *buf,
> > > > >        return copy_guest(buf, gpa, len, GPA_INFO(d),
> > > > >                          COPY_to_guest | COPY_gpa);
> > > > >    }
> > > > > +
> > > > > +/*
> > > > > + * Read machine word from guest memory
> > > > > + *
> > > > > + * @guest_addr: Guest address to read
> > > > > + * @read_insn: Flag representing whether we are reading instruction
> > > > > + * @trap: Output pointer to trap details if something went wrong
> > > > > during read
> > > > > + *
> > > > > + * The hlv/hlvx instructions translate guest_addr through the live
> > > > > + * vsatp/hgatp CSRs, so the read is only meaningful for the address
> > > > > + * space of the currently running vCPU.
> > > > > + *
> > > > > + * At most two halfwords are fetched when @read_insn is true, i.e.
> > > > > encodings
> > > > > + * wider than 32 bits are not supported. Such an encoding cannot be
> > > > > completed
> > > > > + * by calling this function again at @guest_addr + 4: the length
> > > > > check is
> > > > > + * applied to the first halfword read, which would then be a
> > > > > continuation of
> > > > > + * the instruction rather than its opcode. It is up to the caller to
> > > > > reject
> > > > > + * anything that is neither a 16- nor a 32-bit encoding.
> > > > > + */
> > > > > +unsigned long riscv_read_guest(unsigned long guest_addr, bool
> > > > > read_insn,
> > > > > +                               struct trap_info *trap)
> > > > Nit: every other function in this file / declared in this header
> > > > (raw_copy_from_guest, copy_to_guest_phys, ...) has no riscv_ prefix. Why
> > > > does this one get it?
> > > Agree not to much sense. I will drop it.
> > Probably we still want to have riscv_ prefix to show that this
> > read_guest() is specific to RISC-V but others (raw_copy_from_guest,
> > copy_to_guest_phys, ...) could be used in Xen common code too.
> >
> > So I think we could keep riscv_ prefix.
> Please may I suggest to avoid unnecessary prefixes?
Sure then I will drop it as originally suggested.

~ Oleksii