Re: [PATCH v2 1/4] vfio: selftests: add iova range query helpers

[Alex Mastro <amastro@fb.com>]

On Tue, Nov 11, 2025 at 10:52:02AM -0700, Alex Williamson wrote:
> On Tue, 11 Nov 2025 09:35:31 -0800
> Alex Mastro <amastro@fb.com> wrote:
> 
> > On Tue, Nov 11, 2025 at 10:09:48AM -0700, Alex Williamson wrote:
> > > On Tue, 11 Nov 2025 06:52:02 -0800
> > > Alex Mastro <amastro@fb.com> wrote:  
> > > > diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> > > > index a381fd253aa7..7a523e3f2dce 100644
> > > > --- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> > > > +++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> > > > @@ -29,6 +29,173 @@
> > > >  	VFIO_ASSERT_EQ(__ret, 0, "ioctl(%s, %s, %s) returned %d\n", #_fd, #_op, #_arg, __ret); \
> > > >  } while (0)
> > > >  
> > > > +static struct vfio_info_cap_header *next_cap_hdr(void *buf, size_t bufsz,
> > > > +						 size_t *cap_offset)
> > > > +{
> > > > +	struct vfio_info_cap_header *hdr;
> > > > +
> > > > +	if (!*cap_offset)
> > > > +		return NULL;
> > > > +
> > > > +	VFIO_ASSERT_LT(*cap_offset, bufsz);
> > > > +	VFIO_ASSERT_GE(bufsz - *cap_offset, sizeof(*hdr));
> > > > +
> > > > +	hdr = (struct vfio_info_cap_header *)((u8 *)buf + *cap_offset);
> > > > +
> > > > +	if (hdr->next)
> > > > +		VFIO_ASSERT_GT(hdr->next, *cap_offset);  
> > > 
> > > This might be implementation, but I don't think it's a requirement.
> > > The vfio capability chains are based on PCI capabilities, which have no
> > > ordering requirement.  Thanks,  
> > 
> > My main interest was to enforce that the chain doesn't contain a cycle, and
> > checking for monotonically increasing cap offset was the simplest way I could
> > think of to guarantee such.
> > 
> > If there isn't such a check, and kernel vends a malformed cycle-containing
> > chain, chain traversal would infinite loop.
> > 
> > Given the location of this test code coupled to the kernel tree, do you think
> > such assumptions about implementation still reach too far? If yes, I can either
> > remove this check, or try to make cycle detection more relaxed about offsets
> > potentially going backwards.
> 
> I've seen cycle detection in PCI config space implemented as just a
> depth/ttl counter.  Max cycles is roughly (buffer-size/header-size).  I
> think that would be sufficient if we want to include that sanity
> testing.  Thanks,

Thanks, that's a good suggestion -- will take this in v3.

> 
> Alex