Re: [PATCH v1 13/17] xen/riscv: add unprivileged guest memory read helper

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.08.2026 12:27, Oleksii Kurochko wrote:
> On 8/18/26 10:17 AM, Jan Beulich wrote:
>> On 17.08.2026 17:36, Oleksii Kurochko wrote:
>>> On 8/12/26 5:30 PM, Jan Beulich wrote:
>>>> On 20.07.2026 18:02, Oleksii Kurochko wrote:
>>>>> +    if ( read_insn )
>>>>> +    {
>>>>> +        asm volatile ( "\n"
>>>>> +            "1:\n"
>>>>> +            "   hlvx.hu %[val], (%[addr])\n"
>>>>> +            ASM_EXTABLE_TRAP_INFO(1b, 3f, %[ti])
>>>>> +            "   andi %[tmp], %[val], 3\n"
>>>>> +            "   addi %[tmp], %[tmp], -3\n"
>>>>> +            "   bne %[tmp], zero, 3f\n"
>>>>> +            "   addi %[addr], %[addr], 2\n"
>>>>> +            "\n"
>>>>> +            "2:\n"
>>>>> +            "   hlvx.hu %[tmp], (%[addr])\n"
>>>>> +            ASM_EXTABLE_TRAP_INFO(2b, 3f, %[ti])
>>>>> +            "   sll %[tmp], %[tmp], 16\n"
>>>>> +            "   add %[val], %[val], %[tmp]\n"
>>>>> +            "3:\n"
>>>>> +        : [val] "=&r" (val), [tmp] "=&r" (tmp), [addr] "+&r" (guest_addr)
>>>>> +        : [ti] "r" (trap) : "memory" );
>>>>
>>>> You want to tell the compiler that *trap is written. Instead I don't see
>>>> why a memory clobber would be needed: You access a different address space,
>>>> i.e. nothing the compiler can make any assumptions about.
>>>
>>> memory clobber tells the compiler that the assembly code performs memory
>>> reads or writes to items other than those listed in the input and output
>>> operands and so I don't tell here that *trap will be changed.
>>>
>>> Why this understanding is wrong?
>>
>> You can (ab)use "memory" for that purpose, but why would you when you can
>> properly express the operand? All that achieves is the compiler possibly
>> having to emit less efficient code.
> 
> Then I will use the option mentioned ...
> 
>>
>>> Alternative, I think, could be:
>>>           : [val] "+r" (val), "+m" (*trap)
>>>           : [addr] "r" (guest_addr), [ti] "r" (trap) );
>>> And then memory clobber could be dropped.
> 
> ... here.
> 
> Probably I have to return '[addr] "r" (guest_addr)' to output and use 
> +&r constraint.
> 
>>>> You also need to take precautions for not returning an uninitialized "val".
>>>> I think the variable wants initializing (perhaps to ~0) and "+r" wants
>>>> using as constraint. (Afaik & isn't necessary to use together with +.)
>>>
>>> I agree with '+' if we will initialize val with some value.
>>>
>>> Regarding, '&' my understanding is that I have to use it always when
>>
>> When what exactly? If an operand is both input and output, how could the
>> compiler re-use the (generally) register for any further purpose? '&'
>> indicates to the compiler that it may not use the register used for an
>> output to hold some input's value, as that value may be lost by the time
>> the input is actually consumed.
> 
> But what is written in the gcc doc:
> 
> & - Means (in a particular alternative) that this operand is an 
> earlyclobber operand, which is written before the instruction is 
> finished using the input operands.
> 
> What sounds like if an operand (val) in our case is written before the 
> instruction which using the input operands (and after the write 
> instuction which writes val there are instructions which are using input 
> operands) it is needed to have &.

And that's indeed relevant, just not here. My crucial earlier question was:
"If an operand is both input and output, how could the compiler re-use the
(generally) register for any further purpose?" There is a case where the
answer to this is not "it can't". In your case all inputs are distinct; in
e.g. (using x86 assembly, sorry):

int test(int i, int j) {
        asm("nop %0; nop %1" : "+r" (i) : "r" (i));
        asm("cmc; nop %0; nop %1" : "+&r" (j) : "r" (j));

        return i + j;
}

using "+&r" indeed makes a difference.

> t1:  hlvx.hu %[val], (%[addr])   W:val R:addr + can trap -> read register ti
> 
> t2:  andi    %[tmp], %[val], 3   W:tmp   R:val
> t3:  addi    %[tmp], %[tmp], -3
> t4:  bnez    %[tmp], 3f
> t5:  addi    %[addr], %[addr], 2 W:addr  R:addr
> t6:  hlvx.hu %[tmp], (%[addr])   W:tmp   R:addr + can trap -> read 
> register ti
> 
> t7:  slli    %[tmp], %[tmp], 16
> t8:  or      %[val], %[val], %[tmp] W:val
> 
> So val is written on t1 before t6 where addr and ti still alive.
> 
> The similar is for [addr] "+&r" (guest_addr):
> 
> addr is written on t5 and input ti is alive till t6. So if allocator 
> will allocate the same register for addr and ti then addi %[addr], 
> %[addr], 2 will break a pointer and handler will get something wrong.
> 
> Am I missing something?
> 
> If I am still wrong then in both cases should be just "+r"?

As per above, if you want to play absolutely by the rules, use "+&r",
even if that's unnecessary here.

Jan