Re: [PATCH v3 21/39] xen/riscv: add guest memory read helper

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 10/21/26 5:11 PM, Baptiste Le Duc wrote:
> Sorry I forgot one thing.> +unsigned long read_guest(unsigned long 
> guest_addr, bool read_insn,
> > +                         struct trap_info *trap)
> > +{
> > +    /*
> > +     * Poison the result: if the very first access faults, the fixup 
> > skips
> > +     * over the loads without writing it. Callers must check trap- 
> > >scause.
> > +     */
> > +    unsigned long val = ~0UL, tmp;
> > +
> > +    /*
> > +     * hlv/hlvx use hstatus.SPVP for the privilege of the access, and 
> > the
> > +     * live vsatp/hgatp for the translation. Xen never installs a 
> > value of
> > +     * its own in hstatus (it is only saved on trap entry and restored
> > +     * before sret) and it doesn't reschedule before returning to the
> > +     * guest, so all three still belong to the vCPU which trapped.
> > +     */
> > +    if ( read_insn )
> > +    {
> > +        asm volatile ( "\n"
> I think this would need asm_inline instead...> +            "1: hlvx.hu 
> %[val], (%[addr])\n"
> > +            ASM_EXTABLE_TRAP_INFO(1b, 3f, %[ti])
> > +            "   andi %[tmp], %[val], 3\n"
> > +            "   addi %[tmp], %[tmp], -3\n"
> > +            "   bnez %[tmp], 3f\n"
> > +            "   addi %[addr], %[addr], 2\n"
> > +            "\n"
> > +            "2: hlvx.hu %[tmp], (%[addr])\n"
> > +            ASM_EXTABLE_TRAP_INFO(2b, 3f, %[ti])
> > +            "   slli %[tmp], %[tmp], 16\n"
> > +            "   or %[val], %[val], %[tmp]\n"
> > +            "3:\n"
> > +        : [val] "+&r" (val), [tmp] "=&r" (tmp), [addr] "+&r" 
> > (guest_addr),
> > +          "+m" (*trap)
> > +        : [ti] "r" (trap) );
> > +
> > +        /*
> > +         * Although HLVX instructions' explicit memory accesses 
> > require execute
> > +         * permissions, they still raise the same exceptions as other 
> > load
> > +         * instructions, rather than raising fetch exceptions instead.
> > +         */
> > +        if ( trap->scause == CAUSE_LOAD_PAGE_FAULT )
> > +            trap->scause = CAUSE_FETCH_PAGE_FAULT;
> > +    }
> > +    else
> > +    {
> > +        asm volatile ( "\n"
> ... same for here.
read_guest() is an out-of-line function with its only caller in another 
translation unit, so asm_inline wouldn't change any inlining decision here.
volatile is still needed, as the asm may trap and reads memory which can 
change behind the compiler's back.
~ Oleksii