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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.11.2026 13:39, Oleksii Kurochko wrote:
> 
> 
> 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.

Yet that's not expressed by volatile, but by a memory clobber.

Jan