Re: [patch 19/33] genirq/msi: Provide msi_desc::msi_data

[Jason Gunthorpe]

On Mon, Nov 21, 2022 at 08:40:05PM +0100, Thomas Gleixner wrote:
> On Mon, Nov 21 2022 at 13:20, Jason Gunthorpe wrote:
> > On Fri, Nov 18, 2022 at 11:08:55PM +0100, Thomas Gleixner wrote:
> >> Sure I could make both cookies plain u64, but I hate these forced type
> >> casts and the above is simple to handle and understand.
> >
> > I guess, they aren't what I think of as cookies, so I wouldn't make
> > them u64 in the first place.
> >
> > The argument to msi_domain_alloc_irq_at() ideally wants to be a
> > per-domain-type struct so we can folow it around more cleanly. This is
> > C so we have to type erase it as a void * through the core code, but
> > OK.
> 
> When looking at the wire to MSI abomination and also PASID there is no
> real per domain struct. It's plain integer information and I hate to
> store it in a pointer. Especially as the pointer width on 32bit is not
> necessarily sufficient.
> 
> Allocating 8 bytes and tracking them to be freed would be an horrible
> idea.

No, not allocation, just wrap in a stack variable:

  struct foo_bar_domain_data arg = {.pasid = XX};

  msi_domain_alloc_irq_at(..., &arg);

Then there is a great big clue right in the code who is supposed to be
consuming that opaque argument. grep the code for foo_bar_domain_data
and you can find the receiving side

> At least from the two examples I have (IDXD and wire2MSI) the per
> instance union works perfectly fine and I can't see a reason why
> e.g. for your usecase
> 
>      cookie = { .ptr = myqueue };
> 
> would not work. 

I'm not saying not work, I'm asking about the style choice

Regards,
Jason