Re: intel_iommu unit test is also failing

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, May 05, 2026 at 11:45:17AM +0200, Thomas Huth wrote:
> On 05/05/2026 11.27, Clément MATHIEU--DRIF wrote:
> > I had a bit more time to hook into qemu to check the root cause.
> > 
> > It seems that testb issues a single byte read (out of the valid size range), as we can see on the following breakpoint:
> > 
> > ```
> > Thread 6 "CPU 0/TCG" hit Breakpoint 2, memory_region_dispatch_read (mr=0x55d72883cb30, addr=152, pval=0x7f62d25f4590, op=MO_BSWAP, attrs=...) at ../system/memory.c:1473
> > 1473       unsigned size = memop_size(op);
> > (gdb) n
> > 1474       MemTxResult r;
> > (gdb) p size
> > $1 = 1
> > (gdb)
> > ```
> 
> Ouch! That's an excellent finding, Clément ... so GCC 16 is "smart" enough
> to see that we only want to test the lowest bit here, so it optimizes the
> code to access only one byte of memory instead of 4 bytes... which would be
> ok for normal memory, but not for an MMIO register :-/
> 
> Ugly work-around, to force GCC to read 32 bits:
> 
> diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
> --- a/lib/asm-generic/io.h
> +++ b/lib/asm-generic/io.h
> @@ -38,7 +38,9 @@ static inline u16 __raw_readw(const volatile void *addr)
>  #ifndef __raw_readl
>  static inline u32 __raw_readl(const volatile void *addr)
>  {
> -       return *(const volatile u32 *)addr;
> +       u32 val = *(const volatile u32 *)addr;
> +       asm volatile ("\n" : : "r"(addr));
> +       return val;
>  }
>  #endif
> 
> ... but I wonder whether this should rather be treated as a bug in GCC
> instead, since it should IMHO really not change the access size for a
> volatile memory access?
> 
>  Thomas

Wouldn't this break linux generally?

#ifndef __READ_ONCE
#define __READ_ONCE(x)  (*(const volatile __unqual_scalar_typeof(x) *)&(x))
#endif




> 
> > cmd
> > 
> > On Tue, 2026-05-05 at 09:36 +0200, Clement Mathieu--Drif wrote:
> > > Back with some answers:
> > > 
> > > This is the incriminated hunk:
> > > 
> > > ```diff
> > > --- <unnamed>
> > > +++ <unnamed>
> > > @@ -1,17 +1,16 @@
> > > -  404395:       8b 80 98 00 00 00       mov    0x98(%eax),%eax
> > > +  40441d:       8b 43 38                mov    0x38(%ebx),%eax
> > >           edu_reg_writeq(dev, EDU_REG_DMA_DST, to);
> > >           edu_reg_writeq(dev, EDU_REG_DMA_COUNT, size);
> > >           edu_reg_writel(dev, EDU_REG_DMA_CMD, cmd);
> > > 
> > >           /* Wait until DMA finished */
> > >           while (edu_reg_readl(dev, EDU_REG_DMA_CMD) & EDU_CMD_DMA_START)
> > > -  40439b:       a8 01                   test   $0x1,%al
> > > -  40439d:       74 10                   je     4043af <edu_dma+0x121>
> > > -  40439f:       f3 90                   pause
> > > -  4043a1:       48                      dec    %eax
> > > -  4043a2:       8b 43 38                mov    0x38(%ebx),%eax
> > > -  4043a5:       8b 80 98 00 00 00       mov    0x98(%eax),%eax
> > > -  4043ab:       a8 01                   test   $0x1,%al
> > > -  4043ad:       75 f0                   jne    40439f <edu_dma+0x111>
> > > +  404420:       f6 80 98 00 00 00 01    testb  $0x1,0x98(%eax)
> > > +  404427:       74 0f                   je     404438 <edu_dma+0x11f>
> > > +  404429:       f3 90                   pause
> > > +  40442b:       48                      dec    %eax
> > > +  40442c:       8b 43 38                mov    0x38(%ebx),%eax
> > > +  40442f:       f6 80 98 00 00 00 01    testb  $0x1,0x98(%eax)
> > > +  404436:       75 f1                   jne    404429 <edu_dma+0x110>
> > >                   cpu_relax();
> > >   }
> > > 
> > > + is gcc 16
> > > - is gcc 15
> > > 
> > > The instructions generated by gcc 16 always skip the following condition:
> > > 
> > > ```
> > >        /* Wait until DMA finished */
> > >        while (edu_reg_readl(dev, EDU_REG_DMA_CMD) & EDU_CMD_DMA_START)
> > >                cpu_relax();
> > > ```
> > > 
> > > As a consequence, the test performs the second dma operation too early and reads a wrong value.
> > > 
> > > Regards,
> > > cmd