Re: [RFC RESEND v5 3/4] tests/qtest/libqos: Add SMMUv3 helper library

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 12/5/25 7:03 AM, Tao Tang wrote:
> Hi Pierrick,
> 
> On 2025/12/5 07:53, Pierrick Bouvier wrote:
>> On 11/26/25 7:45 AM, Tao Tang wrote:
>>> Introduce qos-smmuv3, a reusable library for SMMUv3-related qtest
>>> operations. This module encapsulates common tasks like:
>>>
>>> - SMMUv3 initialization (enabling, configuring command/event queues)
>>> - Stream Table Entry (STE) and Context Descriptor (CD) setup
>>> - Multi-level page table construction (L0-L3 for 4KB granules)
>>> - Support for Stage 1, Stage 2, and nested translation modes
>>> - Could be easily extended to support multi-space testing infrastructure
>>>       (Non-Secure, Secure, Root, Realm)
>>>
>>> The library provides high-level abstractions that allow test code to
>>> focus on IOMMU behavior validation rather than low-level register
>>> manipulation and page table encoding. Key features include:
>>>
>>> - Automatic memory allocation for translation structures with proper
>>>       alignment
>>> - Helper functions to build valid STEs/CDs for different translation
>>>       scenarios
>>> - Page table walkers that handle address offset calculations per
>>>       security space
>>> - Command queue management for SMMU configuration commands
>>>
>>> This infrastructure is designed to be used by iommu-testdev-based tests
>>> and future SMMUv3 test suites, reducing code duplication and improving
>>> test maintainability.
>>>
>>> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
>>> ---
>>>    tests/qtest/libqos/meson.build  |   3 +
>>>    tests/qtest/libqos/qos-smmuv3.c | 731 ++++++++++++++++++++++++++++++++
>>>    tests/qtest/libqos/qos-smmuv3.h | 267 ++++++++++++
>>>    3 files changed, 1001 insertions(+)
>>>    create mode 100644 tests/qtest/libqos/qos-smmuv3.c
>>>    create mode 100644 tests/qtest/libqos/qos-smmuv3.h
>>>
>>
>> ...
>>
>>> +
>>> +void qsmmu_single_translation(QSMMUTestContext *ctx)
>>> +{
>>> +    uint32_t config_result;
>>> +    uint32_t dma_result;
>>> +    bool test_passed;
>>> +
>>> +    /* Configure SMMU translation */
>>> +    config_result = qsmmu_setup_and_enable_translation(ctx);
>>> +    if (config_result != 0) {
>>> +        g_test_message("Configuration failed: mode=%u status=0x%x",
>>> +                       ctx->config.trans_mode, config_result);
>>> +        return;
>>
>> Is that expected to silently return if we can't configure translation?
> 
> 
> No, it is not intended to silently return on a failed configuration.
> Maybe an assertion is a better choice:
> 
> 
> config_result = qsmmu_setup_and_enable_translation(ctx);
> 
> g_assert_cmpuint(config_result, ==, 0);
> 

Looks good. We should rely on exit code first, and then on verbose log 
to find what is the problem.

>>
>>> +    }
>>> +
>>> +    /* Trigger DMA operation */
>>> +    dma_result = qsmmu_trigger_dma(ctx);
>>> +    if (dma_result != 0) {
>>> +        g_test_message("DMA failed: mode=%u result=0x%x",
>>> +                       ctx->config.trans_mode, dma_result);
>>> +    } else {
>>> +        g_test_message("-> DMA succeeded: mode=%u",
>>> ctx->config.trans_mode);
>>> +    }
>>> +
>>> +    /* Validate test result */
>>> +    test_passed = qsmmu_validate_test_result(ctx);
>>> +    g_assert_true(test_passed);
>>> +
>>> +    /* Clean up translation state to prepare for the next test */
>>> +    qsmmu_cleanup_translation(ctx);
>>> +}
>>> +
>>> +void qsmmu_translation_batch(const QSMMUTestConfig *configs, size_t
>>> count,
>>> +                             QTestState *qts, QPCIDevice *dev,
>>> +                             QPCIBar bar, uint64_t smmu_base)
>>> +{
>>> +    for (int i = 0; i < count; i++) {
>>> +        /* Initialize test memory */
>>> +        qtest_memset(qts, configs[i].dma_iova, 0x00,
>>> configs[i].dma_len);
>>> +        /* Execute each test configuration */
>>> +        QSMMUTestContext ctx = {
>>> +            .qts = qts,
>>> +            .dev = dev,
>>> +            .bar = bar,
>>> +            .smmu_base = smmu_base,
>>> +            .config = configs[i],
>>> +            .trans_status = 0,
>>> +            .dma_result = 0,
>>> +            .sid = dev->devfn,
>>> +            .tx_space = qsmmu_sec_sid_to_space(configs[i].sec_sid),
>>> +        };
>>> +
>>> +        qsmmu_single_translation(&ctx);
>>> +        g_test_message("--> Test %d completed: mode=%u sec_sid=%u "
>>> +                       "status=0x%x result=0x%x", i,
>>> configs[i].trans_mode,
>>> +                       configs[i].sec_sid, ctx.trans_status,
>>> ctx.dma_result);
>>> +    }
>>> +}
>>
>> What is the reason for batching operations?
>> We are not in a performance critical scenario for running this test,
>> so it's probably better to have distinct calls to single_translation.
> 
> 
> As described in the previous thread [1] , I plan to split the tests so
> that each translation mode is exercised by its own qtest. With that
> split in place, there is no real need for a qsmmu_translation_batch()
> helper anymore, so I refactor it into a qsmmu_run_translation_case
> function and drop the inside for-loop.
>

All good, indeed removes the need for translation_batch.

> 
> [1]
> https://lore.kernel.org/qemu-devel/7370070a-c569-4b77-bd1e-6fc749ba9c90@phytium.com.cn/
> 
>>
>> ...
>>
>> For the rest of the patch, which is quite consequent, congrats. It's
>> hard to review all the setup phase here, but knowing it works with the
>> current smmuv3 implementation, that's a good proof that it's working
>> as expected.
> 
> 
> Yes, setting up all this infrastructure did take some time, especially
> getting the nested mode page tables right (and Secure state-related
> configuration which is still in my local repo).
>

Feel free to start with the current version, and then you'll add secure 
state related changes as part of your other series.

> I really appreciate that you ran the tests yourself and even checked
> with a coverage-enabled build to confirm that it exercises the smmuv3
> implementation. Thanks again for the thorough review.
>

In case someone else wants to reproduce:
$ export CFLAGS="--coverage"
$ ./configure --target-list=aarch64-softmmu
$ ninja -C build
$ QTEST_QEMU_BINARY=./build/qemu-system-aarch64 \
   ./build/tests/qtest/iommu-smmuv3-test
$ rm -rf build/coverage_html
$ mkdir build/coverage_html
$ gcovr \
       --gcov-ignore-parse-errors suspicious_hits.warn \
       --gcov-ignore-parse-errors negative_hits.warn \
       --merge-mode-functions=separate \
       --html-details build/coverage_html/index.html \
       --filter 'hw/arm/smmu*'
$ echo file://$(pwd)/build/coverage_html/index.html
# open this in browser by clicking on your terminal

If useful for you, you can attach those instructions in your next cover 
letter, so people can easily reproduce.

> 
> Best regards,
> 
> Tao
>