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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.08.2026 15:38, Oleksii Kurochko wrote:
> 
> 
> On 8/18/26 12:43 PM, Jan Beulich wrote:
>> 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.
> 
> I think it is clear when operands are equal. but what about the case 
> when they are different in  first  case:
> ...
>    asm("nop %0; nop %1" : "+r" (i) : "r" (j));
> ...
> 
> What guarantees that i and j will be in different registers?

They can both be in the same register only if the compiler sees respective
call sites, and can determine that the same value is passed for i and j.
This ...

> We have the similar situation in RISC-V code of riscv_vcpu_unpriv_read():
> 
>          : [val] "+r" (val), [tmp] "=&r" (tmp), [addr] "+r" (guest_addr),
>            "+m" (*trap)
>          : [ti] "r" (trap) );
> 
> With having & for val and guest_addr & guarantees that the same 
> registers won't be re-used for ti (and so ti won't be corrupted) but 
> without it?

... pretty clearly is impossible in your case: val and guest_addr can't
possibly have the same origin as trap.

Yet to repeat what I said before, and to not needlessly continue this
discussion, just go and use "+&r", despite that being unnecessary here.

Jan