Re: [RFC 6/7] iommufd/selftest: Add test coverage for SIOV virtual device

[Yi Liu]

On 2023/10/10 16:30, Tian, Kevin wrote:
> > From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> > Sent: Monday, October 9, 2023 4:51 PM
> >
> > @@ -2071,6 +2083,43 @@ TEST_F(iommufd_device_pasid, pasid_attach)
> >
> > IOMMU_HWPT_ALLOC_DATA_SELFTEST,
> >   					   &data, sizeof(data));
> >
> > +		if (variant->pasid) {
> > +			uint32_t new_hwpt_id = 0;
> > +
> > +			ASSERT_EQ(0,
> > +				  test_cmd_pasid_check_domain(self->fd,
> > +							      self->stdev_id,
> > +							      variant->pasid,
> > +							      self->hwpt_id,
> > +							      &result));
> > +			EXPECT_EQ(1, result);
> > +			test_cmd_hwpt_alloc(self->device_id, self->ioas_id,
> > +					    0, &new_hwpt_id);
> > +			test_cmd_mock_domain_replace(self->stdev_id,
> > +						     new_hwpt_id);
> > +			ASSERT_EQ(0,
> > +				  test_cmd_pasid_check_domain(self->fd,
> > +							      self->stdev_id,
> > +							      variant->pasid,
> > +							      new_hwpt_id,
> > +							      &result));
> > +			EXPECT_EQ(1, result);
> > +
> > +			/*
> > +			 * Detach hwpt from variant->pasid, and check if the
> > +			 * variant->pasid has null domain
> > +			 */
> > +			test_cmd_pasid_detach(variant->pasid);
> > +			ASSERT_EQ(0,
> > +				  test_cmd_pasid_check_domain(self->fd,
> > +							      self->stdev_id,
> > +							      variant->pasid,
> > +							      0, &result));
> > +			EXPECT_EQ(1, result);
> > +
> > +			test_ioctl_destroy(new_hwpt_id);
> > +		}
> > +
>
> I wonder whether above better reuses the device attach/replace cases
> given default_pasid is hidden inside iommufd_device. this pasid_attach
> case is more for testing user pasids on a iommufd_device which hasn't
> yet been supported by SIOV device?

perhaps the way how the above code checks the attached domain misled you.
Actually, this is still testing the siov default_pasid. In the variant
setup, the default_pasid is passed to the testing driver when creating
the stdev. That's why the replace test does not require a pasid.

maybe I can let have a new selftest op to check attached domain for a given 
stdev instead of reusing test_cmd_pasid_check_domain().

--
Regards,
Yi Liu