Re: [PATCH v3 05/18] dmaengine: idxd: add IMS support in base driver

[Raj, Ashok]

Hi Jason

On Wed, Sep 30, 2020 at 03:51:03PM -0300, Jason Gunthorpe wrote:
> On Wed, Sep 30, 2020 at 08:47:00PM +0200, Thomas Gleixner wrote:
> 
> > > +	pci_read_config_dword(pdev, SIOVCAP(dvsec), &val32);
> > > +	if ((val32 & 0x1) && idxd->hw.gen_cap.max_ims_mult) {
> > > +		idxd->ims_size = idxd->hw.gen_cap.max_ims_mult * 256ULL;
> > > +		dev_dbg(dev, "IMS size: %u\n", idxd->ims_size);
> > > +		set_bit(IDXD_FLAG_SIOV_SUPPORTED, &idxd->flags);
> > > +		dev_dbg(&pdev->dev, "IMS supported for device\n");
> > > +		return;
> > > +	}
> > > +
> > > +	dev_dbg(&pdev->dev, "SIOV unsupported for device\n");
> > 
> > It's really hard to find the code inside all of this dev_dbg()
> > noise. But why is this capability check done in this driver? Is this
> > capability stuff really IDXD specific or is the next device which
> > supports this going to copy and pasta the above?
> 
> It is the weirdest thing, IMHO. Intel defined a dvsec cap in their
> SIOV cookbook, but as far as I can see it serves no purpose at
> all.
> 
> Last time I asked I got some unclear mumbling about "OEMs".
> 
One of the parameters it has is the "supported system page-sizes" which is
usually there in the SRIOV properties. So it needed a place holder for
that. 

IMS is a device specific capability, and I almost forgot why we needed
until I had to checking internally. Remember when a device is given to a
guest, MSIX routing via Interrupt Remapping is automatic via the VFIO/IRQFD
and such.

When we provision an entire PCI device that is IMS capable. The guest
driver does know it can update the IMS entries directly without going to
the host. But in order to do remapping we need something like how we manage
PASID allocation from guest, so an IRTE entry can be allocated and the host
driver can write the proper values for IMS.

Hope this helps clear things up.

Cheers,
Ashok