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

[Oleksii <oleksii.kurochko@gmail.com>]

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. 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?

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@hurleysoftware.com/

To me it looks like constraints should be always "+Qo".

~ Oleksii