Re: [PATCH v5 12/23] xen/riscv: introduce io.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.03.2024 17:21, Oleksii wrote:
> On Thu, 2024-03-07 at 16:32 +0100, Jan Beulich wrote:
>> On 07.03.2024 14:44, Oleksii wrote:
>>> On Thu, 2024-03-07 at 14:24 +0100, Jan Beulich wrote:
>>>> On 07.03.2024 14:01, Oleksii wrote:
>>>>> On Wed, 2024-03-06 at 15:13 +0100, Jan Beulich wrote:
>>>>>>> +/* Generic IO read/write.  These perform native-endian
>>>>>>> accesses.
>>>>>>> */
>>>>>>> +static inline void __raw_writeb(uint8_t val, volatile void
>>>>>>> __iomem
>>>>>>> *addr)
>>>>>>> +{
>>>>>>> +    asm volatile ( "sb %0, 0(%1)" : : "r" (val), "r"
>>>>>>> (addr) );
>>>>>>> +}
>>>>>>
>>>>>> I realize this is like Linux has it, but how is the compiler
>>>>>> to
>>>>>> know
>>>>>> that
>>>>>> *addr is being access here? 
>>>>> Assembler syntax told compiler that. 0(%1) - means that the
>>>>> memory
>>>>> location pointed to by the address in register %1.
>>>>
>>>> No, the compiler doesn't decompose the string to figure how
>>>> operands
>>>> are used. That's what the constraints are for. The only two
>>>> things
>>>> the
>>>> compiler does with the string is replace % operators and count
>>>> line
>>>> separators.
>>> It looks like I am missing something.
>>>
>>> addr -> a some register ( because of "r" contraint ).
>>> val -> is also register ( because of "r" contraint ).
>>>
>>> So the compiler will update instert an instruction:
>>>  sb reg1, 0(reg2)
>>>
>>> what means *(uint_8 *)reg2 = (uint8_t)reg1.
>>>
>>> What am I missing?
>>
>> The fact that the compiler will not know that *(uint_8 *)reg2
>> actually
>> changes across this asm(). It may therefore continue to hold a cached
>> value in a register, without knowing that its contents went stale.
> Then it makes sense to me. Thanks.

FTAOD similar considerations apply to memory reads. The compiler may need
to know that values held in registers first need writing back to memory
before an asm() can be invoked.

> It explains why it is needed +Q, but
> I don't understand why constraint 'o' isn't used for __raw_writew, but
> was used for __raw_writeb:
> 
>    static inline void __raw_writeb(u8 val, volatile void __iomem *addr)
>    {
>            asm volatile("strb %1, %0"
>                         : "+Qo" (*(volatile u8 __force *)addr)
>                         : "r" (val));
>    }
>    
>    static inline void __raw_writew(u16 val, volatile void __iomem *addr)
>    {
>            asm volatile("strh %1, %0"
>                         : "+Q" (*(volatile u16 __force *)addr)
>                         : "r" (val));
>    } 
>    
> If I understand correctly 'o' that the address is offsettable, so why
> addr can not be offsettable for everyone case?

I don't know; there may be peculiarities in RISC-V specific constraints.

> And one more thing, in Xen constraint "+" is used, but in Linux it was
> dropped:
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/1426958753-26903-1-git-send-email-peter@xxxxxxxxxxxxxxxxxx/
> 
> To me it looks like constraints should be always "+Qo".

For plain writes it should at least be "=Qo" then, yes. To me making those
input operands on Arm can't have been quite right.

Jan