Hi Fengyuan,
On Sat, Mar 14, 2026 at 09:11:36AM +0800, Fengyuan Yu wrote:
> Signed-off-by: Fengyuan Yu <[email protected]>
The commit message needs a body paragraph describing what
this helper library provides and why it's needed.
Something like the cover letter's "Motivation" section would work.
The subject alone doesn't tell a reader what translation
modes are covered or that this uses the iommu-testdev
framework.
> ---
> MAINTAINERS | 6 +
> tests/qtest/libqos/meson.build | 3 +
> tests/qtest/libqos/qos-intel-iommu.c | 454 +++++++++++++++++++++++++++
> tests/qtest/libqos/qos-intel-iommu.h | 191 +++++++++++
> 4 files changed, 654 insertions(+)
> create mode 100644 tests/qtest/libqos/qos-intel-iommu.c
> create mode 100644 tests/qtest/libqos/qos-intel-iommu.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 247799c817..876e00ff77 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -151,6 +151,9 @@ F: target/i386/meson.build
> F: tools/i386/
> F: tests/functional/i386/
> F: tests/functional/x86_64/
> +F: tests/qtest/intel-iommu-test.c
> +F: tests/qtest/libqos/qos-intel-iommu*
> +F: tests/qtest/iommu-intel-test.c
>
This is wrong.
1) intel-iommu-test.c is already listed in the VT-d
section below. Adding it here creates a duplicate.
2) qos-intel-iommu* should go under the "QTest IOMMU
helpers" section (maintained by Tao Tang), following
the established pattern:
QTest IOMMU helpers
...
F: tests/qtest/libqos/qos-iommu*
F: tests/qtest/libqos/qos-smmuv3*
F: tests/qtest/libqos/qos-riscv-iommu*
+ F: tests/qtest/libqos/qos-intel-iommu*
3) iommu-intel-test.c is created in patch 2/2, so its
MAINTAINERS entry should be in that patch, not here.
The VT-d section additions below look correct for
iommu-intel-test.c, but drop the three lines here.
> X86 VM file descriptor change on reset test
> M: Ani Sinha <[email protected]>
> @@ -4026,6 +4029,9 @@ F: hw/i386/intel_iommu_accel.*
> F: include/hw/i386/intel_iommu.h
> F: tests/functional/x86_64/test_intel_iommu.py
> F: tests/qtest/intel-iommu-test.c
> +F: tests/qtest/libqos/qos-intel-iommu*
> +F: tests/qtest/iommu-intel-test.c
As noted above, only iommu-intel-test.c belongs in the
VT-d section (and should move to patch 2/2). The
qos-intel-iommu* glob belongs in "QTest IOMMU helpers".
[...]
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef QTEST_LIBQOS_INTEL_IOMMU_H
> +#define QTEST_LIBQOS_INTEL_IOMMU_H
> +
> +#include "hw/misc/iommu-testdev.h"
> +#include "hw/i386/intel_iommu_internal.h"
> +
> +/*
> + * Intel IOMMU MMIO register base. This is the standard Q35 IOMMU address.
> + */
> +#define Q35_IOMMU_BASE 0xfed90000ULL
> +
This duplicates Q35_HOST_BRIDGE_IOMMU_ADDR from
include/hw/i386/intel_iommu.h, which is already
pulled in transitively via intel_iommu_internal.h.
Should just use Q35_HOST_BRIDGE_IOMMU_ADDR directly
to avoid the value going stale.
[...]
> +/*
> + * Translation modes supported by Intel IOMMU
> + */
> +typedef enum QVTDTransMode {
> + QVTD_TM_LEGACY_PT, /* Legacy pass-through mode */
> + QVTD_TM_LEGACY_TRANS, /* Legacy translated mode (4-level paging) */
> + QVTD_TM_SCALABLE_PT, /* Scalable pass-through mode */
> + QVTD_TM_SCALABLE_SLT, /* Scalable Second Level Translation */
> + QVTD_TM_SCALABLE_FLT, /* Scalable First Level Translation */
> + QVTD_TM_SCALABLE_NESTED, /* Scalable Nested Translation */
> +} QVTDTransMode;
> +
QVTD_TM_SCALABLE_NESTED is declared but never used in this series,
and qvtd_is_scalable() does not cover it. Either drop it or
include it in qvtd_is_scalable() with a comment that the test
case is not yet implemented.
is not yet implemented.
> +static inline bool qvtd_is_scalable(QVTDTransMode mode)
> +{
> + return mode == QVTD_TM_SCALABLE_PT ||
> + mode == QVTD_TM_SCALABLE_SLT ||
> + mode == QVTD_TM_SCALABLE_FLT;
> +}
> +
> +typedef struct QVTDTestConfig {
> + QVTDTransMode trans_mode; /* Translation mode */
> + uint64_t dma_gpa; /* GPA for readback validation */
> + uint32_t dma_len; /* DMA length for testing */
> + uint32_t expected_result; /* Expected DMA result */
> +} QVTDTestConfig;
> +
> +typedef struct QVTDTestContext {
> + QTestState *qts; /* QTest state handle */
> + QPCIDevice *dev; /* PCI device handle */
> + QPCIBar bar; /* PCI BAR for MMIO access */
> + QVTDTestConfig config; /* Test configuration */
> + uint64_t iommu_base; /* Intel IOMMU base address */
> + uint32_t trans_status; /* Translation configuration status */
> + uint32_t dma_result; /* DMA operation result */
> + uint16_t sid; /* Source ID (bus:devfn) */
> +} QVTDTestContext;
> +
> +/*
> + * qvtd_setup_and_enable_translation - Complete translation setup and enable
> + *
> + * @ctx: Test context containing configuration and device handles
> + *
> + * Returns: Translation status (0 = success, non-zero = error)
> + *
> + * This function performs the complete translation setup sequence:
> + * 1. Builds VT-d structures (root/context entry, page tables)
> + * 2. Programs IOMMU registers and enables translation
> + * 3. Returns configuration status
> + */
> +uint32_t qvtd_setup_and_enable_translation(QVTDTestContext *ctx);
> +
> +/*
> + * qvtd_build_translation - Build Intel IOMMU translation structures
> + *
> + * @qts: QTest state handle
> + * @mode: Translation mode (pass-through or translated)
> + * @sid: Source ID (bus:devfn)
> + *
> + * Returns: Build status (0 = success, non-zero = error)
> + *
> + * Constructs all necessary VT-d translation structures in guest memory:
> + * - Root Entry for the device's bus
> + * - Context Entry for the device
> + * - Complete 4-level page table hierarchy (if translated mode)
> + */
> +uint32_t qvtd_build_translation(QTestState *qts, QVTDTransMode mode,
> + uint16_t sid);
> +
> +/*
> + * qvtd_program_regs - Program Intel IOMMU registers and enable translation
> + *
> + * @qts: QTest state handle
> + * @iommu_base: IOMMU base address
> + * @mode: Translation mode (scalable modes set RTADDR SMT bit)
> + *
> + * Programs IOMMU registers with the following sequence:
> + * 1. Set root table pointer (SRTP), with SMT bit for scalable mode
> + * 2. Setup invalidation queue (QIE)
> + * 3. Configure fault event MSI
> + * 4. Enable translation (TE)
> + *
> + * Each step verifies completion via GSTS register read-back.
> + */
> +void qvtd_program_regs(QTestState *qts, uint64_t iommu_base,
> + QVTDTransMode mode);
Line break and align with the first parameter.
There are similar issues elsewhere; please
double-check the entire codebase.
Thanks,
Chao
> +
> +/*
> + * qvtd_setup_translation_tables - Setup complete VT-d page table hierarchy
> + *
> + * @qts: QTest state handle
> + * @iova: Input Virtual Address to translate
> + * @mode: Translation mode
> + *
> + * This builds the 4-level page table structure for translating
> + * the given IOVA to PA through Intel VT-d. The structure is:
> + * - PML4 (Level 4): IOVA bits [47:39]
> + * - PDPT (Level 3): IOVA bits [38:30]
> + * - PD (Level 2): IOVA bits [29:21]
> + * - PT (Level 1): IOVA bits [20:12]
> + * - Page offset: IOVA bits [11:0]
> + *
> + * The function writes all necessary Page Table Entries (PTEs) to guest
> + * memory using qtest_writeq(), setting up the complete translation path
> + * that the VT-d hardware will traverse during DMA operations.
> + */
> +void qvtd_setup_translation_tables(QTestState *qts, uint64_t iova,
> + QVTDTransMode mode);
> +
> +/* Calculate expected DMA result */
> +uint32_t qvtd_expected_dma_result(QVTDTestContext *ctx);
> +
> +/* Build DMA attributes for Intel VT-d */
> +uint32_t qvtd_build_dma_attrs(void);
> +
> +/* High-level test execution helpers */
> +void qvtd_run_translation_case(QTestState *qts, QPCIDevice *dev,
> + QPCIBar bar, uint64_t iommu_base,
> + const QVTDTestConfig *cfg);
> +
> +#endif /* QTEST_LIBQOS_INTEL_IOMMU_H */
> --
> 2.39.5
>
