Re: Programming PASID in IMS entries

Re: Programming PASID in IMS entries

[Thomas Gleixner]

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.

> 2. Create an API which device drivers can call, to program the PASID 
> (PASID provided by the driver) on a per-irq basis. This API is to be 
> called after msi_domain_alloc_irqs and will write to the corresponding 
> msi_desc->pasid entry. (Assumption: For now, all devices will have the 
> same IMS format). for e.g:
>
> msi_desc_set_pasid (irq, pasid) {
>
> struct msi_desc *desc = irq_get_msi_desc(irq);
>
> desc->pasid = pasid;
>
> }

That interface should be opaque probably with an u64 argument so it can
be reused for Jason's VM-id. Jason?

> 3. In request_irq, add a irq_chip callback (in __setup_irq maybe??) to 
> automatically write the pasid into the corresponding IMS entry:

Why? There is no need for yet another callback. The PASID or whatever ID
is required can be written as part of e.g. irq_unmask().

> Is this the correct approach?

No.

> Also, from a previous discussion [1], we want to make IMS more dynamic:
>
> Given the QEMU behavior it doesn't ask for all IRQs upfront. It only 
> allocates 1, and when it unmasks the 2nd, it wants to dynamically add a 
> second. This will allow adding a second IRQ without having to free all 
> the old irqs and reacquire the new number (as it is done today).
>
> This dynamic behavior is only for MSIx/IMS backed entries. For legacy
> MSI, QEMU will allocate everything upfront. Since it has a
> "num_vectors" enabled, nothing can be dynamically done for MSI. Kevin
> is looking to have this fixed for legacy to stop the dynamic part for
> MSI. We are pursuing this change just for IMS first, and once it
> works, we can replicate the same for MSIx too.

No. Fix the existing stuff first and then IMS just works.

> In order to make IMS dynamic, we were thinking of the following 
> enhancements to the IMS core:
>
> 1. Device Driver specifies maximum number of interrupts the sub device 
> is allowed to request, while creating the dev-msi domain. E.g. in the 
> case of DSA, Driver can specify that each mdev created can have upto X

Why would this be mdev specific? IIRC the sub devices can be used on
bare metal as well.

> number of IMS interrupts. If device asks for more than this number,it 
> will behave like how current IRQ allocation works i.e. give what is 
> available.
>
> 2. Driver can ask for more interrupts after probe as well as long as the 
> request has not exceeded the maximum permitted for it and the physical 
> device has the requested number available.

Ok.

> We are still working on the virtualization flows: when guest updates 
> PASID, and how it flows to host IMS update. But we will come to that 
> once the above pieces are agreed upon.

Hypercall?

Thanks,

        tglx

Re: Programming PASID in IMS entries

[Jason Gunthorpe]

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.

This seems like a pretty good idea, though it requires that the
device's IRQ code cast the struct device to some driver specific
subtype, like mdev in this case.

> > 2. Create an API which device drivers can call, to program the PASID 
> > (PASID provided by the driver) on a per-irq basis. This API is to be 
> > called after msi_domain_alloc_irqs and will write to the corresponding 
> > msi_desc->pasid entry. (Assumption: For now, all devices will have the 
> > same IMS format). for e.g:
> >
> > msi_desc_set_pasid (irq, pasid) {
> >
> > struct msi_desc *desc = irq_get_msi_desc(irq);
> >
> > desc->pasid = pasid;
> >
> > }
> 
> That interface should be opaque probably with an u64 argument so it can
> be reused for Jason's VM-id. Jason?

Well, I certainly wouldn't put any IDXD specific words like PASID in
the general API. The comingling of PASID with the rest of the IRQ
registers is entirely a device specific choice.

Most likely something like mlx5 is going to want to associate a
pointer with the irq, and I believe it could use a struct device just
fine.

Jason

Re: Programming PASID in IMS entries

[Raj, Ashok]

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.

> 
> > 2. Create an API which device drivers can call, to program the PASID 
> > (PASID provided by the driver) on a per-irq basis. This API is to be 
> > called after msi_domain_alloc_irqs and will write to the corresponding 
> > msi_desc->pasid entry. (Assumption: For now, all devices will have the 
> > same IMS format). for e.g:
> >
> > msi_desc_set_pasid (irq, pasid) {
> >
> > struct msi_desc *desc = irq_get_msi_desc(irq);
> >
> > desc->pasid = pasid;
> >
> > }
> 
> That interface should be opaque probably with an u64 argument so it can
> be reused for Jason's VM-id. Jason?
> 
> > 3. In request_irq, add a irq_chip callback (in __setup_irq maybe??) to 
> > automatically write the pasid into the corresponding IMS entry:
> 
> Why? There is no need for yet another callback. The PASID or whatever ID
> is required can be written as part of e.g. irq_unmask().

Yes, that should work for the host side doing the irq_unmask() to program
the PASID in the ims entry.

> 
> > Is this the correct approach?
> 
> No.
> 
> > Also, from a previous discussion [1], we want to make IMS more dynamic:
> >
> > Given the QEMU behavior it doesn't ask for all IRQs upfront. It only 
> > allocates 1, and when it unmasks the 2nd, it wants to dynamically add a 
> > second. This will allow adding a second IRQ without having to free all 
> > the old irqs and reacquire the new number (as it is done today).
> >
> > This dynamic behavior is only for MSIx/IMS backed entries. For legacy
> > MSI, QEMU will allocate everything upfront. Since it has a
> > "num_vectors" enabled, nothing can be dynamically done for MSI. Kevin
> > is looking to have this fixed for legacy to stop the dynamic part for
> > MSI. We are pursuing this change just for IMS first, and once it
> > works, we can replicate the same for MSIx too.
> 
> No. Fix the existing stuff first and then IMS just works.
> 
> > In order to make IMS dynamic, we were thinking of the following 
> > enhancements to the IMS core:
> >
> > 1. Device Driver specifies maximum number of interrupts the sub device 
> > is allowed to request, while creating the dev-msi domain. E.g. in the 
> > case of DSA, Driver can specify that each mdev created can have upto X
> 
> Why would this be mdev specific? IIRC the sub devices can be used on
> bare metal as well.

I guess so. I thought for bare metal we don't need to play these games
since native abstraction is provided with things like uaccel for e.g. For
things like SRIOV its much different.

What the above limit accomplishes is telling the guest, you can request
upto the limit, but you aren't guaranteed to get them. This avoids the
static partitioning and becomes best effort by the host driver.

Ideally we want to tell the guest it can have upto say 1k interrupts, but
unlike MSIx where resources are commited in HW, we want to allow guest
allocations to fail. 

Cheers,
Ashok

RE: Programming PASID in IMS entries

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Wednesday, July 7, 2021 8:16 PM
> 
> 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.
> 
> This seems like a pretty good idea, though it requires that the
> device's IRQ code cast the struct device to some driver specific
> subtype, like mdev in this case.

it's not necessary, if in the end we still need a helper to set PASID to
struct msi_desc. It doesn't matter whether this PASID is default or for
vSVA. Just let device driver to pick whatever ID is necessary to mark
the entry.

> 
> > > 2. Create an API which device drivers can call, to program the PASID
> > > (PASID provided by the driver) on a per-irq basis. This API is to be
> > > called after msi_domain_alloc_irqs and will write to the corresponding
> > > msi_desc->pasid entry. (Assumption: For now, all devices will have the
> > > same IMS format). for e.g:
> > >
> > > msi_desc_set_pasid (irq, pasid) {
> > >
> > > struct msi_desc *desc = irq_get_msi_desc(irq);
> > >
> > > desc->pasid = pasid;
> > >
> > > }
> >
> > That interface should be opaque probably with an u64 argument so it can
> > be reused for Jason's VM-id. Jason?
> 
> Well, I certainly wouldn't put any IDXD specific words like PASID in
> the general API. The comingling of PASID with the rest of the IRQ
> registers is entirely a device specific choice.
> 
> Most likely something like mlx5 is going to want to associate a
> pointer with the irq, and I believe it could use a struct device just
> fine.
> 

Agree. Better call it source_id or cookie?

Thanks
Kevin

RE: Programming PASID in IMS entries

[Tian, Kevin]

> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Wednesday, July 7, 2021 4:51 PM
> 
> > Also, from a previous discussion [1], we want to make IMS more dynamic:
> >
> > Given the QEMU behavior it doesn't ask for all IRQs upfront. It only
> > allocates 1, and when it unmasks the 2nd, it wants to dynamically add a
> > second. This will allow adding a second IRQ without having to free all
> > the old irqs and reacquire the new number (as it is done today).
> >
> > This dynamic behavior is only for MSIx/IMS backed entries. For legacy
> > MSI, QEMU will allocate everything upfront. Since it has a
> > "num_vectors" enabled, nothing can be dynamically done for MSI. Kevin
> > is looking to have this fixed for legacy to stop the dynamic part for
> > MSI. We are pursuing this change just for IMS first, and once it
> > works, we can replicate the same for MSIx too.
> 
> No. Fix the existing stuff first and then IMS just works.
> 

Does below sound a plan?

1.  Fix Qemu to allocate all possible irqs when guest unmasks MSI/MSI-X,
    instead of freeing and re-allocating in the fly. This is an improvement
    for existing kernels which don't support dynamic resize.

2.  Extend MSI-X core to support dynamic resize. Via VFIO_IRQ_INFO_
    NORESIZE Qemu can enable dynamic resize if the flag is false. and
    from your comment dev-msi resize will be covered too  with this change.

3.  Add hypercall (or a pv irqchip) to provide feedback into guest in case 
    of irq shortage. This is also necessary to enable guest ims.

Thanks
Kevin

Re: Programming PASID in IMS entries

[Jason Gunthorpe]

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?

Jason

Re: Programming PASID in IMS entries

[Raj, Ashok]

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?

It seems we have conceptually 3 types of IMS already brewing up.

1. IDXD has legacy MSIx + MSix permission table to hold PASID.
2. IMS in IDXD has PASID in the IMS entry itself.
3. Jason's vm_id instead of the PASID use case.

So we can keep IMS core about these types, another way is to allow device
specific implementations provide the write handlers with the index, and let
the driver handle the location of the write. IMS core will provide the
format and data.

Cheers,
Ashok

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

Re: Programming PASID in IMS entries

[Thomas Gleixner]

Ashok,

On Wed, Jul 07 2021 at 15:12, Ashok Raj wrote:
> On Wed, Jul 07, 2021 at 10:50:52AM +0200, Thomas Gleixner wrote:
>> 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.

I'm well aware of that, but can we solve step 1 before step 2 please?

>> > In order to make IMS dynamic, we were thinking of the following 
>> > enhancements to the IMS core:
>> >
>> > 1. Device Driver specifies maximum number of interrupts the sub device 
>> > is allowed to request, while creating the dev-msi domain. E.g. in the 
>> > case of DSA, Driver can specify that each mdev created can have upto X
>> 
>> Why would this be mdev specific? IIRC the sub devices can be used on
>> bare metal as well.
>
> I guess so. I thought for bare metal we don't need to play these games
> since native abstraction is provided with things like uaccel for e.g. For
> things like SRIOV its much different.
>
> What the above limit accomplishes is telling the guest, you can request
> upto the limit, but you aren't guaranteed to get them. This avoids the
> static partitioning and becomes best effort by the host driver.

That's fine.

> Ideally we want to tell the guest it can have upto say 1k interrupts, but
> unlike MSIx where resources are commited in HW, we want to allow guest
> allocations to fail. 

Which as discussed requires a hypercall because silent fail is not an
option.

Thanks,

        tglx

Re: Programming PASID in IMS entries

[Raj, Ashok]

Hi Jason

On Thu, Jul 08, 2021 at 09:08:46AM -0300, Jason Gunthorpe wrote:
> 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.

Well, this is how it started way back last year. 

https://lore.kernel.org/lkml/158751209583.36773.15917761221672315662.stgit@djiang5-desk3.ch.intel.com/

Where the driver functions for mask/unmask/write_msg etc. So the core needs

So the format or layout is device specific, but core can dictate the exact
message that needs to be written.

> 
> 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.

I think this must be gauranteed at a min? changing things underneath when
the interrupts are unmasked would be bad usage.

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

Seems like it.. 

Cheers,
Ashok

Re: Programming PASID in IMS entries

[Thomas Gleixner]

Ashok,

On Thu, Jul 08 2021 at 07:36, Ashok Raj wrote:
> On Thu, Jul 08, 2021 at 09:08:46AM -0300, Jason Gunthorpe wrote:
>> 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:
>> > > > 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.
>
> Well, this is how it started way back last year. 
>
> https://lore.kernel.org/lkml/158751209583.36773.15917761221672315662.stgit@djiang5-desk3.ch.intel.com/

Which is wrong on so many levels as we all know.

> Where the driver functions for mask/unmask/write_msg etc. So the core
> needs

Needs what?

> So the format or layout is device specific, but core can dictate the exact
> message that needs to be written.

Sorry, I don't grok what you want to say here.

>> 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.
>
> I think this must be gauranteed at a min? changing things underneath when
> the interrupts are unmasked would be bad usage.

That's one way to look at it. OTOH, _if_ the association of some
arbitrary information to interrupts becomes a common scheme, then we are
surely better off to have some enforcement at the irq core level.

Thanks,

        tglx

Re: Programming PASID in IMS entries

[Raj, Ashok]

Hi Thomas,

On Thu, Jul 08, 2021 at 08:45:48PM +0200, Thomas Gleixner wrote:
> Ashok,
> 
> >> > 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.
> >
> > Well, this is how it started way back last year. 
> >
> > https://lore.kernel.org/lkml/158751209583.36773.15917761221672315662.stgit@djiang5-desk3.ch.intel.com/
> 
> Which is wrong on so many levels as we all know.

Sorry, I was just trying to point to Jason, that its how things started.
Since he was suggesting to have them as device specific. 


> 
> > Where the driver functions for mask/unmask/write_msg etc. So the core
> > needs
> 
> Needs what?

Fat fingered that reply.. I completed it partially but moved to a different
sentence formation :-(
> 
> > So the format or layout is device specific, but core can dictate the exact
> > message that needs to be written.
> 
> Sorry, I don't grok what you want to say here.

Sorry it was unclear.. I meant things like compose_msg() 

Cheers,
Ashok