Kernel KVM virtualization development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Sean Christopherson" <seanjc@google.com>
Cc: "Marc Zyngier" <maz@kernel.org>,
	kvm@vger.kernel.org, kvmarm@lists.linux.dev,
	"Oliver Upton" <oupton@kernel.org>
Subject: Re: [PATCH v7 09/20] KVM: selftests: Add VFIO device support to eventfd IRQ test
Date: Sat, 13 Jun 2026 00:35:01 +0000	[thread overview]
Message-ID: <20260613003501.DD5951F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260613002031.745413-10-seanjc@google.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] probe_iommu_type() incorrectly probes the /dev/vfio directory instead of the /dev/vfio/vfio legacy container character device, bypassing the framework's intended skip behavior for unprivileged users.
- [Medium] vfio_setup_msi() hardcodes the device's DMA region IOVA to 0, which can cause IOMMU mapping failures on platforms that reserve or restrict IOVA 0.
--

commit 2bdd8160743775c8d18e4004e72e7ff0898fbd70
Author: David Matlack <dmatlack@google.com>

KVM: selftests: Add VFIO device support to eventfd IRQ test

This commit extends the eventfd IRQ test to support triggering interrupts
using a VFIO device instead of manually signaling the eventfd. It also
probes for IOMMUFD vs legacy VFIO availability.

Link: https://lore.kernel.org/all/20260602222941.3133236-1-jrhilke%40google.com [1]

> diff --git a/tools/testing/selftests/kvm/irq_test.c b/tools/testing/selftests/kvm/irq_test.c
> index 9f8895b898214..6888be54ee4a9 100644
> --- a/tools/testing/selftests/kvm/irq_test.c
> +++ b/tools/testing/selftests/kvm/irq_test.c
[ ... ]
> @@ -55,6 +58,36 @@ static void *vcpu_thread_main(void *arg)
>  	return NULL;
>  }
>  
> +static int vfio_setup_msi(struct vfio_pci_device *device)
> +{
> +	const int flags = MAP_SHARED | MAP_ANONYMOUS;
> +	const int prot = PROT_READ | PROT_WRITE;
> +	struct dma_region *region;
> +
> +	/* A driver is required to generate an MSI. */
> +	TEST_REQUIRE(device->driver.ops);
> +
> +	/* Set up a DMA-able region for the driver to use. */
> +	region = &device->driver.region;
> +	region->iova = 0;

[Severity: Medium]
Does hardcoding the DMA region's IOVA to 0 cause IOMMU mapping failures on
platforms that reserve or restrict IOVA 0?

If IOVA 0 is unsupported, it seems like iommu_map() will fail and trigger a
test assertion crash instead of running.

> +	region->size = SZ_2M;
> +	region->vaddr = kvm_mmap(region->size, prot, flags, -1);
> +	TEST_ASSERT(region->vaddr != MAP_FAILED, "mmap() failed\n");
> +	iommu_map(device->iommu, region);
> +
> +	vfio_pci_driver_init(device);
> +	return device->driver.msi;
> +}
[ ... ]
> @@ -74,11 +107,29 @@ static void kvm_route_msi(struct kvm_vm *vm, u32 gsi, struct kvm_vcpu *vcpu,
>  	vm_ioctl(vm, KVM_SET_GSI_ROUTING, &routing.header);
>  }
>  
> +static const char *probe_iommu_type(void)
> +{
> +	int io_fd;
> +
> +	io_fd = open("/dev/iommu", O_RDONLY);
> +	if (io_fd >= 0) {
> +		close(io_fd);
> +		return MODE_IOMMUFD;
> +	}
> +
> +	io_fd = __open_path_or_exit("/dev/vfio", O_RDONLY,

[Severity: Medium]
Because /dev/vfio is a directory, will this open() call succeed for any user
with read and execute permissions to the directory, bypassing the intended skip
behavior?

Should this probe the /dev/vfio/vfio legacy container character device instead
so the test can skip gracefully for unprivileged users rather than crashing
later in iommu_init() due to lack of device permissions?

> +				    "Is VFIO (or IOMMUFD) loaded and enabled?");
> +	close(io_fd);
> +	return MODE_VFIO_TYPE1_IOMMU;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260613002031.745413-1-seanjc@google.com?part=9

  reply	other threads:[~2026-06-13  0:35 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-13  0:20 [PATCH v7 00/20] KVM: selftests: Add eventfd+VFIO IRQ test Sean Christopherson
2026-06-13  0:20 ` [PATCH v7 01/20] KVM: selftests: Build and link selftests/vfio/lib into KVM selftests Sean Christopherson
2026-06-13  0:20 ` [PATCH v7 02/20] KVM: selftests: Add macros to read/write+sync to/from guest memory Sean Christopherson
2026-06-13  0:20 ` [PATCH v7 03/20] KVM: selftests: Rename guest_rng to kvm_rng Sean Christopherson
2026-06-13  0:20 ` [PATCH v7 04/20] KVM: selftests: Initialize the default/global pRNG during kvm_selftest_init() Sean Christopherson
2026-06-13  0:20 ` [PATCH v7 05/20] KVM: selftests: Seed libc's RNG before using it to generate a seed for KVM's pRNG Sean Christopherson
2026-06-13  0:30   ` sashiko-bot
2026-06-13  0:20 ` [PATCH v7 06/20] KVM: selftests: Add helper to generate random u64 in range [min,max] Sean Christopherson
2026-06-13  0:31   ` sashiko-bot
2026-06-13  0:20 ` [PATCH v7 07/20] KVM: selftests: Add an irqfd send+receive (and later IRQ bypass) test Sean Christopherson
2026-06-13  0:20 ` [PATCH v7 08/20] KVM: selftests: Add helper to get host IRQ from device MSI-X for IRQ bypass test Sean Christopherson
2026-06-13  0:28   ` sashiko-bot
2026-06-13  0:20 ` [PATCH v7 09/20] KVM: selftests: Add VFIO device support to eventfd IRQ test Sean Christopherson
2026-06-13  0:35   ` sashiko-bot [this message]
2026-06-13  0:20 ` [PATCH v7 10/20] KVM: selftests: Add a helper to set proc IRQ affinity for " Sean Christopherson
2026-06-13  0:20 ` [PATCH v7 11/20] KVM: selftests: Verify interrupts are received when IRQ affinity changes in " Sean Christopherson
2026-06-13  0:29   ` sashiko-bot
2026-06-13  0:20 ` [PATCH v7 12/20] KVM: selftests: Add option to set empty routing between IRQs in eventfd " Sean Christopherson
2026-06-13  0:20 ` [PATCH v7 13/20] KVM: selftests: Make number of IRQs configurable in " Sean Christopherson
2026-06-13  0:20 ` [PATCH v7 14/20] KVM: selftests: Verify non-postable IRQ remapping " Sean Christopherson
2026-06-13  0:20 ` [PATCH v7 15/20] KVM: selftests: Add kvm_gettid() wrapper and convert users Sean Christopherson
2026-06-13  0:20 ` [PATCH v7 16/20] KVM: selftests: Add kvm_sched_getaffinity() " Sean Christopherson
2026-06-13  0:20 ` [PATCH v7 17/20] KVM: selftests: Add a utility to pin a task to a random CPU, given a CPU set Sean Christopherson
2026-06-13  0:20 ` [PATCH v7 18/20] KVM: selftests: Verify vCPU migration during IRQ delivery in IRQ test Sean Christopherson
2026-06-13  0:20 ` [PATCH v7 19/20] KVM: selftests: Make number of vCPUs configurable " Sean Christopherson
2026-06-13  0:20 ` [PATCH v7 20/20] KVM: selftests: Add xAPIC support in eventfd " Sean Christopherson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260613003501.DD5951F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=maz@kernel.org \
    --cc=oupton@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=seanjc@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox