Re: [RFC PATCH 0/6] selftests/vfio: Add sPAPR TCE v2 coverage

[Narayana Murty N <nnmlinux@linux.ibm.com>]

Hi David,
Thank you for your detailed review comments on this series.

On 07/07/26 4:25 AM, David Matlack wrote:
> On 2026-07-02 11:28 PM, Narayana Murty N wrote:
>> This RFC adds PowerPC sPAPR TCE v2 support to VFIO selftests.
> nit: Please use "vfio: selftests: ..." for commit shortlogs.
Agree, will fix all commit shortlogs in v2.
>
>> The series adds a selftest IOMMU mode for the legacy VFIO container
>> backend, adds helpers for default and dynamic DMA windows, and exercises
>> the sPAPR DDW path through the DMA mapping tests.
> Thanks for sending. Is there anyway I could run these tests on PowerPC,
> e.g. with QEMU, to try things out? I don't have access to any hardware.
I had tested this patch series on real power h/w (pseries and powernv).
However I am not sure if the required firmware support is implemented in 
QEMU.
I will check and get back on this.
>> sPAPR TCE v2 remains a legacy VFIO container backend. This series does
>> not add iommufd or VFIO cdev support for sPAPR.
>>
>> The main points are
>> 1. add VFIO_SELFTESTS_IOMMU_MODE for selecting a backend
>> 2. add vfio_spapr_tce_v2_iommu as the PowerPC VFIO backend
>> 3. preserve the platform default DMA window
>> 4. create and tear down only selftest-created DDWs
>> 5. prepare the DMA window before IOVA allocation
>> 6. exercise DDW creation for hugepage DMA mappings
>> 7. accept sPAPR-specific errno for invalid DMA ranges
>>
>> Example:
>>
>> VFIO_SELFTESTS_IOMMU_MODE=vfio_spapr_tce_v2_iommu
>> ./vfio_dma_mapping_test <BDF>
>>
>> Observed coverage includes anonymous mappings through the default window
>> and 2MB hugetlb mappings through a DDW. 1GB hugetlb mappings may skip when
>> the platform rejects the requested DDW characteristics.
>>
>> Feedback is requested from the VFIO and PowerPC communities on:
>> 1. whether the sPAPR TCE v2 helpers should remain in the common VFIO
>> selftest library or move into a sPAPR-specific test helper?
> Responded in the patch. Let's move to their own file.
Will do. v2 will have:

   lib/iommu_spapr.c         — all sPAPR logic, compiled only on powerpc
   lib/iommu_spapr_stub.c    — empty stubs returning -EOPNOTSUPP,
                               compiled on all other architectures
   lib/include/libvfio/iommu_spapr.h — declarations

iommu.c will call the sPAPR helpers unconditionally with no #ifdefs in
function bodies.
>> 2. whether DDW setup through hugepage mappings is the right initial
>> coverage?
> One thing I'm not sure is how much value you get out of exercising the
> DDW stuff through that test specifically. Would it make more sense to
> have a DDW-specific test so you can test all the edge cases you care
> about directly instead? Then vfio_dma_mapping_test can stay generic.
Agreed. For v2 I plan to introduce a dedicated vfio_spapr_ddw_test that
covers DDW creation, window selection, page-size negotiation, and
removal directly, and keep vfio_dma_mapping_test generic. The hugepage
variants in vfio_dma_mapping_test will then just exercise the mapping
path itself without being the primary DDW coverage vehicle.
>
> BTW, is that the only test that passes right now? What is the status of
> the other VFIO selftests on PowerPC after this series?
   vfio_dma_mapping_test    — anonymous and 2 MB hugetlb variants PASS;
   vfio_dma_mapping_mmio_test  — PASS (overflow test passes with -ENXIO 
fix).
   vfio_pci_device_test     — PASS.
Below tests are not supported:
   vfio_pci_driver_test
   vfio_iommufd_setup_test
currently it fails unsupported tests when triggred via scripts/run.sh

I will start including test logs in cover letter starting with RFC V2.
>
>> 3. whether the sPAPR-specific overflow errno expectation should be
>> accepted, skipped, or hidden behind a backend helper?
> Responded in the patch. Let's keep the check but base it off IOMMU mode
> not __powerpc__. Also it'd be good to avoid duplicating the code with a
> helper function or macro but that's a small thing.
Agreed. Will replace the #ifdef __powerpc__ guard with a runtime check:

   int expected = iommu_is_spapr(self->iommu) ? -ENXIO : -EOVERFLOW;
   ASSERT_EQ(rc, expected);
I will add a helper to avoid code duplication.
>> 4. whether PowerPC should generate only the sPAPR TCE v2 fixture variants,
>> or keep the generic IOMMU-mode matrix and skip unsupported modes.
> For (4), I think skipping unsupported modes would be a good improvement.
> Even on x86 and ARM we would want that feature. e.g. It's possible for
> someone to run VFIO selftests on a kernel that has VFIO Type1 IOMMUs
> enabled but not IOMMUFD. Today that will cause all the tests to fail.
> But ideally it would wouldn't. Tests that just want the default IOMMU
> mode should use Type1, and tests that want to replicate across all IOMMU
> modes should skip the IOMMUFD variants. Same thing applies to the SPAPR
> IOMMU mode.
This is a great point and it also addresses the concern raised in the
Patch 1/6 review about forcing the user to set VFIO_SELFTESTS_IOMMU_MODE.

Once again thanks for the thorough review.
I shall address all the review comments on the series before sending v2.

Regards,
Narayana
>> TODO: vfio_spapr_tce_v1_iommu yet to be covered.
>>
>> Narayana Murty N (6):
>>    selftests/vfio: allow selecting IOMMU backend from environment
>>    selftests/vfio: add sPAPR TCE v2 IOMMU mode
>>    selftests/vfio: add sPAPR TCE v2 DMA window helpers
>>    selftests/vfio: Exercise sPAPR DDW path for hugepage DMA mappings
>>    selftests/vfio: Accept sPAPR errno for DMA range overflow
>>    selftests/vfio: Enable VFIO selftests on ppc64 and ppc64le
>>
>>   .../selftests/vfio/lib/include/libvfio.h      |   9 +
>>   .../vfio/lib/include/libvfio/iommu.h          |  26 ++
>>   tools/testing/selftests/vfio/lib/iommu.c      | 280 +++++++++++++++++-
>>   tools/testing/selftests/vfio/lib/libvfio.c    |  37 +++
>>   .../selftests/vfio/vfio_dma_mapping_test.c    |  30 ++
>>   .../selftests/vfio/vfio_pci_device_test.c     |   4 +-
>>   tools/testing/selftests/vfio/Makefile         |   2 +-
>>   7 files changed, 382 insertions(+), 6 deletions(-)
>>
>> -- 
>> 2.51.1
>>