Re: [PATCH v6 15/20] vfio/mdev: idxd: ims domain setup for the vdcm

From: Jason Gunthorpe <jgg@nvidia.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Dave Jiang <dave.jiang@intel.com>,
	alex.williamson@redhat.com, kwankhede@nvidia.com,
	vkoul@kernel.org, megha.dey@intel.com, jacob.jun.pan@intel.com,
	ashok.raj@intel.com, yi.l.liu@intel.com, baolu.lu@intel.com,
	kevin.tian@intel.com, sanjay.k.kumar@intel.com,
	tony.luck@intel.com, dan.j.williams@intel.com,
	eric.auger@redhat.com, pbonzini@redhat.com,
	dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org
Subject: Re: [PATCH v6 15/20] vfio/mdev: idxd: ims domain setup for the vdcm
Date: Tue, 1 Jun 2021 09:16:12 -0300	[thread overview]
Message-ID: <20210601121612.GH1002214@nvidia.com> (raw)
In-Reply-To: <875yyy31cl.ffs@nanos.tec.linutronix.de>

On Tue, Jun 01, 2021 at 01:55:22AM +0200, Thomas Gleixner wrote:
> On Mon, May 31 2021 at 13:57, Jason Gunthorpe wrote:
> > On Mon, May 31, 2021 at 04:02:02PM +0200, Thomas Gleixner wrote:
> >> > I'm quite surprised that every mdev doesn't create its own ims_domain
> >> > in its probe function.
> >> 
> >> What for?
> >
> > IDXD wouldn't need it, but proper IMS HW with no bound of number of
> > vectors can't provide a ims_info.max_slots value here.
> 
> There is no need to do so:
> 
>      https://lore.kernel.org/r/20200826112335.202234502@linutronix.de
> 
> which has the IMS_MSI_QUEUE variant at which you looked at and said:
> 
>  "I haven't looked through everything in detail, but this does look like
>   it is good for the mlx5 devices."
> 
> ims_info.max_slots is a property of the IMS_MSI_ARRAY and does not make
> any restrictions on other storage.

Ok, it has been a while since then

> >> That depends on how IMS is implemented. The IDXD variant has a fixed
> >> sized message store which is shared between all subdevices, so yet
> >> another domain would not provide any value.
> >
> > Right, IDXD would have been perfectly happy to use the normal MSI-X
> > table from what I can see.
> 
> Again. No, it's a storage size problem and regular MSI-X does not
> support auxiliary data.

I mean the IDXD HW could have been designed with a normal format MSI-X
table and a side table with the PASID.

> Ergo, the proper thing to do is to fix this ID storage problem (PASID,
> VM_ID or whatever) at the proper place, i.e. store it in struct device
> (which is associated to that mdev) and let the individual drivers handle
> it as they require.

If the struct device defines all the details of how to place the IRQ
into the HW, including what HW table to use, then it seems like it
could work.

I don't clearly remember all the details anymore so lets look at how
non-IDXD devices might work when HW actually comes.

Jason