From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-dl1-f41.google.com (mail-dl1-f41.google.com [74.125.82.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0E73C3B38A2 for ; Mon, 11 May 2026 20:53:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.82.41 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778532799; cv=none; b=GWiPqTyLSK8Ixko4sVEyvnt9eEVJAjikknoZVe53bUeHJRxNIXsUHTSYG4uEKWBA64N2knKZrrevZP0VFqDHwmmuRQx+8q/YGoNIXbX4CRNiQzuKEcTqpKLuLi0hDAmxpoOtqG2x1G40nRYlVQejHDtdJ2RkGPZK/l1n76H0V00= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778532799; c=relaxed/simple; bh=Km++bgstJy22aPuoAl4fC3UyjcfdgbFAcUcT6xfJrcc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=YVZzU3stcZ+WEnKRVDgdqLpUAgMPFCbqOlMhcVH81fh5UzvUhR37pTMjlRNBVPAJ//dwpnYDvHiFxgt/ArrUmL/R0I1xNp0cKJW4OHfkmd+U7kZDTOw+lQdUak6KtPvL+9RlrlfajM2FF8XFeynnUNZ88QbKFeUIjpTWyVjBYgc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=SsNyo+qD; arc=none smtp.client-ip=74.125.82.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="SsNyo+qD" Received: by mail-dl1-f41.google.com with SMTP id a92af1059eb24-133362c30cfso123c88.0 for ; Mon, 11 May 2026 13:53:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1778532797; x=1779137597; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=10PxtWyhgedi2BJxlEdo5PBTQQ1UNrd1U0mb5g7f0RA=; b=SsNyo+qD6cU8mcfN5wwC8wMRe+4aG5EQKB6JU5aNkpy6FK9Au/IxzC7v9C/lL50p/D iVUuDBjzJPdqkQAIJaMqmh78P/Sid9XOOvSwDrHeBBwD/l4s9C9ce/iNOma2Ag3tG3oK TFtP4Skry3C8dz+OT9OEnKufTkheoLzoW+aYsZEeMclEN2s3gD0bBzZaFefCtRwL0UEX YdHw6sUqsMsFTVPW5BTUs99HHdThwDNKEgoLUPyN7j/RDc7fRcl8A8c6dd7Mmw10G0RL oAlD36zBfh186DykIvr8Q1c1zCmyKN2RypLteHIhMkSRCL9VmChXNZTuyqqgCHRU1HrC ET0w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1778532797; x=1779137597; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=10PxtWyhgedi2BJxlEdo5PBTQQ1UNrd1U0mb5g7f0RA=; b=ooB3bmZPNLKspkUGHYLCILJyOwto1tTEblphaRnZZO0lKkIpq+K8YS4wteNEMHpjxR t+9rZAU3ppgckWsjDPEkvpe1uqRZq3Lm+38EF9oA3eahlE+4OTzP/podNBGfvBDUiGZE n8oCSdmFp+d5zaR8fsuzx944ShPHAgIVCMjlViRZGhIBQ6wf0X8PCP86Uewsihrxfm8a qnLcNyk4F4qMyoVAjU9pLJ+CxguZyQq9NqFXW7CBw2kFE6rV9jhHnnWeoadunI+bodCT rQlRyJKn56RhE6dRI1dePmLp5k38qtnbI+NjeZBOgObbn7XdbsvXrd6LQbD8X09jh+SS ifwQ== X-Forwarded-Encrypted: i=1; AFNElJ8EFMt0BWYJwNxyw7m4V4JT3PBmXlVbZnjldE3mKkZcC7EHPuFJ/JiAIzqWK4Z0v0M/UrBI33WzuHYRRBo=@vger.kernel.org X-Gm-Message-State: AOJu0Yz9PxFxajPH/RF+4L07k7ZRBo3pMJiAs9/MrOd0h33/qlmYTjC7 RPCY/XBzWRICnqFDOD9NemCflx7ZrAihAtTWm47nlMaAe83kwyXJfolIThSdqET+ow== X-Gm-Gg: Acq92OFJ42c7R0ujWHs7syPNW5qVWG42PdM+oSuwVG1uHh+URda+SFUvy25NOBt3Jnx MAqAj92AgzQUcUilJvhRFrcq+ypiME3ut/wglA+qOQSvFYd1uRXlFtMo+mLcwutjelp6yUSWtcE mPYRIPi68hOott5eWi5C0N+bb9oHRESNfVXFRyZeNYJ2fujiej55Pj/yI/x86dphYJ1ufNYDFbQ QbLX9Sn+m0JuXoxtFEEINUdSyEX9VzvA9V2HW3K1bPXrHRVjIvUdFxg5MZWv0kYnulEfLckvchj n6bt/KTaXDHzARKlq6u+dPSFkqipwLldGk/34cAUPXy1Evg2oqsjdjz5SIEVivaIL+sdNU+hFXn eM0iH80egOR/g2iOqCTZ5SregsZfmH59Bba8D/NXl6mFB+PeWm14n7/LTMXOl/ORQaKKNyCpoX2 XvLwOFGfVBuWz9gpiDXzMdxtA0AM5oO3+i373+xBBuVpgIsY/ipQ9b8R4vgdOniT7cCHibTw== X-Received: by 2002:a05:7022:fd09:b0:12d:ff22:d624 with SMTP id a92af1059eb24-1333e4e4224mr100561c88.10.1778532796409; Mon, 11 May 2026 13:53:16 -0700 (PDT) Received: from google.com (153.46.83.34.bc.googleusercontent.com. [34.83.46.153]) by smtp.gmail.com with ESMTPSA id 5a478bee46e88-2f8860ce1bfsm15595665eec.9.2026.05.11.13.53.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 11 May 2026 13:53:16 -0700 (PDT) Date: Mon, 11 May 2026 20:53:12 +0000 From: Samiullah Khawaja To: David Matlack Cc: Aex Williamson , Shuah Khan , linux-kernel@vger.kernel.org, kvm@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: Re: [PATCH 2/2] vfio: selftests: Add iommufd multi iommu test Message-ID: References: <20260505221518.619123-1-skhawaja@google.com> <20260505221518.619123-3-skhawaja@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: On Fri, May 08, 2026 at 08:14:08PM +0000, David Matlack wrote: >On 2026-05-05 10:14 PM, Samiullah Khawaja wrote: >> Add a test for iommufd to verify creation of multiple iommus using an >> already setup iommufd and switch between them. >> >> Signed-off-by: Samiullah Khawaja >> --- >> tools/testing/selftests/vfio/Makefile | 1 + >> .../vfio/vfio_iommufd_multi_iommu_test.c | 203 ++++++++++++++++++ >> 2 files changed, 204 insertions(+) >> create mode 100644 tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c >> >> diff --git a/tools/testing/selftests/vfio/Makefile b/tools/testing/selftests/vfio/Makefile >> index 0684932d91bf..24bdeaad590f 100644 >> --- a/tools/testing/selftests/vfio/Makefile >> +++ b/tools/testing/selftests/vfio/Makefile >> @@ -8,6 +8,7 @@ else >> CFLAGS = $(KHDR_INCLUDES) >> TEST_GEN_PROGS += vfio_dma_mapping_test >> TEST_GEN_PROGS += vfio_dma_mapping_mmio_test >> +TEST_GEN_PROGS += vfio_iommufd_multi_iommu_test >> TEST_GEN_PROGS += vfio_iommufd_setup_test >> TEST_GEN_PROGS += vfio_pci_device_test >> TEST_GEN_PROGS += vfio_pci_device_init_perf_test >> diff --git a/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c b/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c >> new file mode 100644 >> index 000000000000..7199c662637c >> --- /dev/null >> +++ b/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c >> @@ -0,0 +1,203 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +#include >> +#include >> + >> +#include >> +#include >> + >> +#include >> + >> +#include "kselftest_harness.h" >> + >> +static const char *device_bdf; >> + >> +#define IOMMU_DEFAULT 0 >> +#define IOMMU_WITH_PT 1 >> +#define IOMMU_WITHOUT_PT 2 >> + >> +static void region_setup(struct iommu *iommu, >> + struct iova_allocator *iova_allocator, >> + struct dma_region *region, u64 size) >> +{ >> + const int flags = MAP_SHARED | MAP_ANONYMOUS; >> + const int prot = PROT_READ | PROT_WRITE; >> + void *vaddr; >> + >> + vaddr = mmap(NULL, size, prot, flags, -1, 0); >> + VFIO_ASSERT_NE(vaddr, MAP_FAILED); >> + >> + region->vaddr = vaddr; >> + region->iova = iova_allocator_alloc(iova_allocator, size); >> + region->size = size; >> + >> + iommu_map(iommu, region); >> +} >> + >> +static void region_teardown(struct iommu *iommu, struct dma_region *region) >> +{ >> + iommu_unmap(iommu, region); >> + VFIO_ASSERT_EQ(munmap(region->vaddr, region->size), 0); >> +} > >Can you create library helpers in the library to share with >vfio_pci_driver_test.c? > > dma_region_setup() > dma_region_teardown() I thought it is ok to duplicate this as each test (in future) might have different mapping requirements? we might have memfd, guest_memfd, dma_buf with various mapping strategies for different usecases? Or you are thinking of a basic helper that can be used by tests if they are not doing anything fancy? > >> + >> +FIXTURE(vfio_iommufd_multi_iommu_test) { >> + struct iommu *iommu; >> + struct vfio_pci_device *device; >> + struct iova_allocator *iova_allocator; >> + struct dma_region memcpy_region; >> + void *vaddr; >> + >> + u64 size; >> + void *src; >> + void *dst; >> + iova_t src_iova; >> + iova_t dst_iova; >> +}; >> + >> +FIXTURE_SETUP(vfio_iommufd_multi_iommu_test) >> +{ >> + struct vfio_pci_driver *driver; >> + >> + self->iommu = iommu_init("iommufd"); > >MODE_IOMMUFD Will update. > >> + self->device = vfio_pci_device_init(device_bdf, self->iommu); >> + self->iova_allocator = iova_allocator_init(self->iommu); >> + >> + driver = &self->device->driver; >> + >> + region_setup(self->iommu, self->iova_allocator, &self->memcpy_region, SZ_1G); >> + region_setup(self->iommu, self->iova_allocator, &driver->region, SZ_2M); >> + >> + if (driver->ops) >> + vfio_pci_driver_init(self->device); >> + >> + self->size = self->memcpy_region.size / 2; >> + self->src = self->memcpy_region.vaddr; >> + self->dst = self->src + self->size; >> + >> + self->src_iova = to_iova(self->device, self->src); >> + self->dst_iova = to_iova(self->device, self->dst); >> +} >> + [snip] >> +static void test_memcpy(struct vfio_pci_device *device, void *src, void *dst, >> + iova_t src_iova, iova_t dst_iova, u64 size, char val) >> +{ >> + if (!device->driver.ops) >> + return; >> + >> + memset(src, val, size); >> + memset(dst, 0, size); >> + >> + vfio_pci_driver_memcpy_start(device, src_iova, dst_iova, size, 100); > >Why do 100 copies? Will remove in next revision. > >> + VFIO_ASSERT_EQ(vfio_pci_driver_memcpy_wait(device), 0); >> + VFIO_ASSERT_EQ(memcmp(src, dst, size), 0); >> +} >> + >> +TEST_F(vfio_iommufd_multi_iommu_test, memcpy) >> +{ >> + struct dma_region memcpy_region1, driver_region1; >> + struct dma_region memcpy_region2, driver_region2; >> + struct iommu *iommu1; >> + struct iommu *iommu2; >> + >> + iommu1 = setup_variant_iommu(self->iommu, self->device->dev_id, >> + variant->start_iommu); >> + if (iommu1 != self->iommu) { >> + memcpy_region1 = self->memcpy_region; >> + driver_region1 = self->device->driver.region; >> + >> + iommu_map(iommu1, &memcpy_region1); > >Can you make a helper to copy dma_region into a new iommu? It should set >up the new struct dma_region based on the old one, re-initialize the >link field, and then call iommu_map(). Can you please clarify, I didn't get it completely. so basically something that can be called like following? setup_copy_dma_regions(self, iommu1, &memcpy_region1, &driver_region1); I think I get that you are concerned about link init. Maybe I can do? int clone_dma_region(struct dma_region *source_region, struct dma_region *dest_region); We can maybe add it to the library? > >> + iommu_map(iommu1, &driver_region1); >> + >> + vfio_pci_device_attach_iommu(self->device, iommu1); >> + } >> + >> + test_memcpy(self->device, self->src, self->dst, self->src_iova, >> + self->dst_iova, self->size, 'x'); > >nit: You can pass in self to avoid unpacking it in every call to >test_memcpy(). The type would be: > > struct _test_data_vfio_iommufd_multi_iommu_test *self Agreed. Will do. > >> + >> + iommu2 = setup_variant_iommu(self->iommu, self->device->dev_id, >> + variant->end_iommu); >> + if (iommu2 != self->iommu) { >> + memcpy_region2 = self->memcpy_region; >> + driver_region2 = self->device->driver.region; >> + >> + iommu_map(iommu2, &memcpy_region2); >> + iommu_map(iommu2, &driver_region2); >> + } >> + >> + vfio_pci_device_attach_iommu(self->device, iommu2); >> + test_memcpy(self->device, self->src, self->dst, self->src_iova, >> + self->dst_iova, self->size, 'a'); > >test_memcpy() zeroes self->dst every time so do you need to use a >different character for each call? I will update this. > >> + >> + vfio_pci_device_attach_iommu(self->device, iommu1); >> + test_memcpy(self->device, self->src, self->dst, self->src_iova, >> + self->dst_iova, self->size, '1'); >> + >> + if (iommu1 != self->iommu) { >> + /* attach back to default iommu for cleanup */ >> + vfio_pci_device_attach_iommu(self->device, self->iommu); >> + iommu_unmap(iommu1, &memcpy_region1); >> + iommu_unmap(iommu1, &driver_region1); >> + iommu_cleanup(iommu1); >> + } >> + >> + if (iommu2 != self->iommu) { >> + iommu_unmap(iommu2, &memcpy_region2); >> + iommu_unmap(iommu2, &driver_region2); > >nit: You don't need to unmap before cleanup. Is this just to exercise >that unmap works on the variant iommus? Not really. I thought it is needed. But I think I will keep it to exercise that. Will add a comment to clarify. > >> + iommu_cleanup(iommu2); >> + } >> +} >> + >> +int main(int argc, char *argv[]) >> +{ >> + device_bdf = vfio_selftests_get_bdf(&argc, argv); >> + >> + return test_harness_run(argc, argv); >> +} >> -- >> 2.54.0.545.g6539524ca2-goog >> Thanks, Sami