Re: intel_iommu unit test is also failing

[Thomas Huth <thuth@redhat.com>]

On 05/05/2026 11.53, Clément MATHIEU--DRIF wrote:
> 
> 
> On Tue, 2026-05-05 at 11:45 +0200, Thomas Huth wrote:
>> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
>>
>>
>> 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?
> 
> Volatile is expected to make sure that the read side effect is visible.
> I don't know if the size of the access is in the scope of this constraint or not o.O

Maybe we should simply adjust the read/write functions in the pci-edu.h 
code, WDYT:

diff --git a/lib/pci-edu.h b/lib/pci-edu.h
--- a/lib/pci-edu.h
+++ b/lib/pci-edu.h
@@ -59,24 +59,24 @@ struct pci_edu_dev {

  static inline uint64_t edu_reg_readq(struct pci_edu_dev *dev, int reg)
  {
-       return __raw_readq(edu_reg(dev, reg));
+       return readq(edu_reg(dev, reg));
  }

  static inline uint32_t edu_reg_readl(struct pci_edu_dev *dev, int reg)
  {
-       return __raw_readl(edu_reg(dev, reg));
+       return readl(edu_reg(dev, reg));
  }

  static inline void edu_reg_writeq(struct pci_edu_dev *dev, int reg,
                                   uint64_t val)
  {
-       __raw_writeq(val, edu_reg(dev, reg));
+       writeq(val, edu_reg(dev, reg));
  }

  static inline void edu_reg_writel(struct pci_edu_dev *dev, int reg,
                                   uint32_t val)
  {
-       __raw_writel(val, edu_reg(dev, reg));
+       writel(val, edu_reg(dev, reg));
  }

  bool edu_init(struct pci_edu_dev *dev);

... it might be good to use the non-raw functions here for ordered access 
anyway...

  Thomas