Re: [Intel-gfx] [PATCH v3 12/12] vfio/pci: Report dev_id in VFIO_DEVICE_GET_PCI_HOT_RESET_INFO

From: Jason Gunthorpe <jgg@nvidia.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: "mjrosato@linux.ibm.com" <mjrosato@linux.ibm.com>,
	"jasowang@redhat.com" <jasowang@redhat.com>,
	"Hao, Xudong" <xudong.hao@intel.com>,
	"Duan, Zhenzhong" <zhenzhong.duan@intel.com>,
	"peterx@redhat.com" <peterx@redhat.com>,
	"Xu, Terrence" <terrence.xu@intel.com>,
	"chao.p.peng@linux.intel.com" <chao.p.peng@linux.intel.com>,
	"linux-s390@vger.kernel.org" <linux-s390@vger.kernel.org>,
	"Liu, Yi L" <yi.l.liu@intel.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"lulu@redhat.com" <lulu@redhat.com>,
	"Jiang, Yanting" <yanting.jiang@intel.com>,
	"joro@8bytes.org" <joro@8bytes.org>,
	"nicolinc@nvidia.com" <nicolinc@nvidia.com>,
	"Zhao, Yan Y" <yan.y.zhao@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"eric.auger@redhat.com" <eric.auger@redhat.com>,
	"intel-gvt-dev@lists.freedesktop.org"
	<intel-gvt-dev@lists.freedesktop.org>,
	"yi.y.sun@linux.intel.com" <yi.y.sun@linux.intel.com>,
	"cohuck@redhat.com" <cohuck@redhat.com>,
	"shameerali.kolothum.thodi@huawei.com"
	<shameerali.kolothum.thodi@huawei.com>,
	"suravee.suthikulpanit@amd.com" <suravee.suthikulpanit@amd.com>,
	"robin.murphy@arm.com" <robin.murphy@arm.com>
Subject: Re: [Intel-gfx] [PATCH v3 12/12] vfio/pci: Report dev_id in VFIO_DEVICE_GET_PCI_HOT_RESET_INFO
Date: Mon, 17 Apr 2023 11:05:19 -0300	[thread overview]
Message-ID: <ZD1SH8/bpcytlp+E@nvidia.com> (raw)
In-Reply-To: <20230413120712.3b9bf42d.alex.williamson@redhat.com>

On Thu, Apr 13, 2023 at 12:07:12PM -0600, Alex Williamson wrote:

> IIUC, the semantics we're proposing is that an INFO2 ioctl would return
> success or failure indicating whether the user has sufficient ownership
> of the affected devices, 

Or a flag, but yes

> and in the success case returns an array of
> affected dev-ids within the user's iommufd_ctx.  Unopened, affected
> devices, are not reported via INFO2, and unopened, affected devices
> outside the user's scope of ownership (ie. outside the owned IOMMU
> group) will generate a failure condition.

Yes

> As for the INFO ioctl, it's described as unchanged, which does raise
> the question of what is reported for IOMMU groups and how does the
> value there coherently relate to anything else in the cdev-exclusive
> vfio API...

For cdev mode the value of the group_id has no functional
purpose. INFO has no functional purpose beyond debugging. The cdev
enabled userspace should print the BDFs from the INFO in a debug
message and ignore the group_id.

Kernel will still fill the group_id using the iommu_get_group() stuff,
and set -1 for no-iommu.

> We had already iterated a proposal where the group-id is replaced with
> a dev-id in the existing ioctl and a flag indicates when the return
> value is a dev-id vs group-id.  This had a gap that userspace cannot
> determine if a reset is available given this information since un-owned
> devices report an invalid dev-id and userspace can't know if it has
> implicit ownership.

IIRC, yes.

> It seems cleaner to me though that we would could still re-use INFO in
> a similar way, simply defining a new flag bit which is valid only in
> the case of returning dev-ids and indicates if the reset is
> available.

Yes, it could be done like this as well. INFO2 is more a discussion
object, how we encode it in the uAPI matters a lot less. The point is
that INFO2, as an idea, returns information that no other existing API
returns: the "ownership passed flag" and "dev_id list"

Then as I said in the other mail we roll no-iommu into an iommufd_ctx
object and just follow the design that userspace must have a single
iommufd_ctx containing all the devices to use the hot reset feature.

Jason