Re: [patch 21/33] genirq/msi: Provide msi_domain_alloc_irq_at()

From: Jason Gunthorpe <jgg@nvidia.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>,
	x86@kernel.org, Joerg Roedel <joro@8bytes.org>,
	Will Deacon <will@kernel.org>,
	linux-pci@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Marc Zyngier <maz@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Dave Jiang <dave.jiang@intel.com>,
	Alex Williamson <alex.williamson@redhat.com>,
	Kevin Tian <kevin.tian@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Logan Gunthorpe <logang@deltatee.com>,
	Ashok Raj <ashok.raj@intel.com>, Jon Mason <jdmason@kudzu.us>,
	Allen Hubbe <allenbh@gmail.com>,
	"Ahmed S. Darwish" <darwi@linutronix.de>,
	Reinette Chatre <reinette.chatre@intel.com>
Subject: Re: [patch 21/33] genirq/msi: Provide msi_domain_alloc_irq_at()
Date: Wed, 16 Nov 2022 15:36:01 -0400	[thread overview]
Message-ID: <Y3U7oeW7jfEDv0Qu@nvidia.com> (raw)
In-Reply-To: <20221111135206.463650635@linutronix.de>

On Fri, Nov 11, 2022 at 02:58:44PM +0100, Thomas Gleixner wrote:
> For supporting post MSI-X enable allocations and for the upcoming PCI/IMS
> support a seperate interface is required which allows not only the
> allocation of a specific index, but also the allocation of any, i.e. the
> next free index. The latter is especially required for IMS because IMS
> completely does away with index to functionality mappings which are
> often found in MSI/MSI-X implementation.
> 
> But even with MSI-X there are devices where only the first few indices have
> a fixed functionality and the rest is freely assignable by software,
> e.g. to queues.
> 
> msi_domain_alloc_irq_at() is also different from the range based interfaces
> as it always enforces that the MSI descriptor is allocated by the core code
> and not preallocated by the caller like the PCI/MSI[-X] enable code path
> does.
> 
> msi_domain_alloc_irq_at() can be invoked with the index argument set to
> MSI_ANY_INDEX which makes the core code pick the next free index. The irq
> domain can provide a prepare_desc() operation callback in its
> msi_domain_ops to do domain specific post allocation initialization before
> the actual Linux interrupt and the associated interrupt descriptor and
> hierarchy alloccations are conducted.
> 
> The function also takes an optional @cookie argument which is of type union
> msi_dev_cookie. This cookie is not used by the core code and is stored in
> the allocated msi_desc::data::cookie. The meaning of the cookie is
> completely implementation defined. In case of IMS this might be a PASID or
> a pointer to a device queue, but for the MSI core it's opaque and not used
> in any way.

To my mind it makes more sense to pass a 'void *' through from
msi_domain_alloc_irq_at() to the prepare_desc() op with the idea that
the driver calling msi_domain_alloc_irq_at() knows it is calling it
against the domain that it allocated. The prepare_desc can then use
the void * to properly initialize anything about the desc under the
right lock.

Before calling this the driver should have setup whatever thing is
going to originate the interrupt, eg allocated the HW object that
sources the interrupt and part of what the void * would convey is the
detailed information on how to program the HW object. eg IDXD is using
an iobase and an offset along with the enforcing PASID, but something
like mlx5 would probably want an object id, type, and SF ID.

This is again where I don't much like the use of an ID to refer to the
domain.

Having the driver allocate the device domain, retain a pointer to it,
and use that domain pointer with all these new APIs seems much clearer
than converting the pointer to an ID.

Jason