Re: [RFC v3 1/3] hw/misc: introduce iommu-testdev for bare-metal IOMMU testing

["Alex Bennée" <alex.bennee@linaro.org>]

Tao Tang <tangtao1634@phytium.com.cn> writes:

> Hi Clement,
>
> On 2025/11/13 15:02, CLEMENT MATHIEU--DRIF wrote:
>> Hi Tao,
>>
>> On Thu, 2025-11-13 at 00:21 +0800, Tao Tang wrote:
>>> Add a minimal PCI test device designed to exercise IOMMU translation
>>> (such as ARM SMMUv3) without requiring guest firmware or OS. The device
>>> provides MMIO registers to configure and trigger DMA operations with
>>> controllable attributes (security state, address space), enabling
>>> deterministic IOMMU testing.
>>>
>>> Key features:
>>> - Bare-metal IOMMU testing via simple MMIO interface
>>> - Configurable DMA attributes for security states and address spaces
>>> - Write-then-read verification pattern with automatic result checking
>>>
>>> The device performs a deterministic DMA test pattern: write a known
>>> value(0x88888888) to a configured IOVA, read it back, and verify data
>>> integrity. Results are reported through a dedicated result register,
>>> eliminating the need for complex interrupt handling or driver
>>> infrastructure in tests.
>>>
>>> This is purely a test device and not intended for production use or
>>> machine realism. It complements existing test infrastructure like
>>> pci-testdev but focuses specifically on IOMMU translation path
>>> validation.
>>>
>>> Signed-off-by: Tao Tang <[tangtao1634@phytium.com.cn](mailto:tangtao1634@phytium.com.cn)>
>>> ---
>>>   docs/specs/index.rst            |   1 +
>>>   docs/specs/iommu-testdev.rst    |  96 +++++++++++
>>>   hw/misc/Kconfig                 |   5 +
>>>   hw/misc/iommu-testdev.c         | 292 ++++++++++++++++++++++++++++++++
>>>   hw/misc/meson.build             |   1 +
>>>   hw/misc/trace-events            |  10 ++
>>>   include/hw/misc/iommu-testdev.h |  78 +++++++++
>>>   7 files changed, 483 insertions(+)
>>>   create mode 100644 docs/specs/iommu-testdev.rst
>>>   create mode 100644 hw/misc/iommu-testdev.c
>>>   create mode 100644 include/hw/misc/iommu-testdev.h
>>>
>>> ------------------------------<snip>------------------------------
>>>
>>>
>>>
>>> ------------------------------<snip>------------------------------
>>> +
>>> +static void iommu_testdev_maybe_run_dma(IOMMUTestDevState *s)
>>> +{
>>> +    int i, j, remaining_bytes;
>> I think i and j could be declared in their respective loop
>
>
> Thanks a lot for taking the time to review the patch. All your
> suggestions are excellent and make a lot of sense.
>
>
> Move the i and j loop variable declarations into their respective for
> loops sounds a good idea. But I wasn't entirely sure if QEMU had a
> strict code style requirement for this, such as mandating all
> variables be defined at the start of the function before any
> executable code (C89 style), so I appreciate the clarification.
>
<snip>

We have a special exemption, from style.rst:

  Mixed declarations (interleaving statements and declarations within
  blocks) are generally not allowed; declarations should be at the beginning
  of blocks. To avoid accidental re-use it is permissible to declare
  loop variables inside for loops:

  .. code-block:: c

      for (int i = 0; i < ARRAY_SIZE(thing); i++) {
          /* do something loopy */
      }

  Every now and then, an exception is made for declarations inside a
  #ifdef or #ifndef block: if the code looks nicer, such declarations can
  be placed at the top of the block even if there are statements above.
  On the other hand, however, it's often best to move that #ifdef/#ifndef
  block to a separate function altogether.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro