Re: intel_iommu unit test is also failing

[Thomas Huth <thuth@redhat.com>]

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


> 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