Re: Programming PASID in IMS entries

[Jason Gunthorpe]

On Wed, Jul 07, 2021 at 05:33:35PM -0700, Raj, Ashok wrote:
> On Wed, Jul 07, 2021 at 08:58:22PM -0300, Jason Gunthorpe wrote:
> > On Wed, Jul 07, 2021 at 03:12:16PM -0700, Raj, Ashok wrote:
> > > Hi Thomas
> > > 
> > > On Wed, Jul 07, 2021 at 10:50:52AM +0200, Thomas Gleixner wrote:
> > > > Megha,
> > > > 
> > > > On Wed, Jul 07 2021 at 09:49, Megha Dey wrote:
> > > > > Per your suggestions during the last meeting, we wanted to confirm the 
> > > > > sequence to program the PASID into the IMS entries:
> > > > >
> > > > > 1. Add a PASID member to struct msi_desc (Add as part of a union. Other 
> > > > > source-id's such as Jason's vm-id can be added to it)
> > > > 
> > > > Yes. Though we also discussed storing the default PASID in struct device
> > > > to begin with which is then copied to the msi_desc entries during
> > > > allocation.
> > > 
> > > Using default PASID in struct device will work for sub-devices until the
> > > guest needs to enable ENQCMD support. Since the guest kernel can ask for an
> > > interrupt by specifying something in the descriptor submitted via ENQCMD.
> > > Using the PASID in struct device won't be sufficient.
> > 
> > Could you could store a pasid table in the struct device and index it
> > by vector?
> 
> Possibly... what ever Thomas things is clean. The device specific driver
> would have this already. So providing some call to get this filled in vs
> storing that in struct device. Someone close at heart to the driver model
> is best to comment :-)
> 
> IMS core owns the format of the entries right now vs device specific driver. 
> I suppose your use case requiring a vm_id might have a different format. 
> So this is yet another one the core needs to learn and adapt?

All entry format stuff is device specific, it shouldn't be in "core"
code.

It is is the same reason that the IRQ chip driver for IDXD should have
IDXD in the name, it is not a generic "IMS core" thing.

The question mark is probably the locking model, but if IDXD
guarentees the pasid table doesn't change while the irq is active then
maybe it works out well enough.

Associating a void * with the irq is also possibly reasonable, I'm not
sure which path makes the most sense.

Jason