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

[Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>]

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.