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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

>> If the omission of respective constraints here
>> and below is intentional, I think a comment (covering all instances)
>> is
>> needed. Note that while supposedly cloned from Arm code, Arm variants
>> do
>> have such constraints in Linux.
>>
> It uses this constains only in arm32:
> #define __raw_writeb __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));
> }
> 
> But in case of arm64:
> 
> #define __raw_writeb __raw_writeb
> static __always_inline void __raw_writeb(u8 val, volatile void __iomem
> *addr)
> {
>       asm volatile("strb %w0, [%1]" : : "rZ" (val), "r" (addr));
> }
> 
> And again looking at the defintion they use different option of strb
> instruction, and in case of strb they use [%1] which tells compiler
> that %1 is addressed which should be dereferenced.

Same bug here then; I happened to look at Arm32 only. As mentioned in
the other patch using what's provided here, the problem becomes more
than latent only there. And I can't spot such use in Arm64 code, so it
is likely only a latent bug there.

Jan