[RFC,v2,04/18] irq/dev-msi: Introduce APIs to allocate/free dev-msi interrupts

Commit Message

Dave Jiang July 21, 2020, 4:02 p.m. UTC

From: Megha Dey <[e-mail]>

The dev-msi interrupts are to be allocated/freed only for custom devices,
not standard PCI-MSIX devices.

These interrupts are device-defined and they are distinct from the already
existing msi interrupts:
pci-msi: Standard PCI MSI/MSI-X setup format
platform-msi: Platform custom, but device-driver opaque MSI setup/control
arch-msi: fallback for devices not assigned to the generic PCI domain
dev-msi: device defined IRQ domain for ancillary devices. For e.g. DSA
portal devices use device specific IMS(Interrupt message store) interrupts.

dev-msi interrupts are represented by their own device-type. That means
dev->msi_list is never contended for different interrupt types. It
will either be all PCI-MSI or all device-defined.

Reviewed-by: Dan Williams <[e-mail]>
Signed-off-by: Megha Dey <[e-mail]>
Signed-off-by: Dave Jiang <[e-mail]>
---
 drivers/base/dev-msi.c |   23 +++++++++++++++++++++++
 include/linux/msi.h    |    4 ++++
 2 files changed, 27 insertions(+)

Comments

Jason Gunthorpe July 21, 2020, 4:25 p.m. UTC | #1

On Tue, Jul 21, 2020 at 09:02:41AM -0700, Dave Jiang wrote:
> From: Megha Dey <[e-mail]>
> 
> The dev-msi interrupts are to be allocated/freed only for custom devices,
> not standard PCI-MSIX devices.
> 
> These interrupts are device-defined and they are distinct from the already
> existing msi interrupts:
> pci-msi: Standard PCI MSI/MSI-X setup format
> platform-msi: Platform custom, but device-driver opaque MSI setup/control
> arch-msi: fallback for devices not assigned to the generic PCI domain
> dev-msi: device defined IRQ domain for ancillary devices. For e.g. DSA
> portal devices use device specific IMS(Interrupt message store) interrupts.
> 
> dev-msi interrupts are represented by their own device-type. That means
> dev->msi_list is never contended for different interrupt types. It
> will either be all PCI-MSI or all device-defined.

Not sure I follow this, where is the enforcement that only dev-msi or
normal MSI is being used at one time on a single struct device?

Jason

Dey, Megha July 22, 2020, 5:05 p.m. UTC | #2

On 7/21/2020 9:25 AM, Jason Gunthorpe wrote:
> On Tue, Jul 21, 2020 at 09:02:41AM -0700, Dave Jiang wrote:
>> From: Megha Dey <[e-mail]>
>>
>> The dev-msi interrupts are to be allocated/freed only for custom devices,
>> not standard PCI-MSIX devices.
>>
>> These interrupts are device-defined and they are distinct from the already
>> existing msi interrupts:
>> pci-msi: Standard PCI MSI/MSI-X setup format
>> platform-msi: Platform custom, but device-driver opaque MSI setup/control
>> arch-msi: fallback for devices not assigned to the generic PCI domain
>> dev-msi: device defined IRQ domain for ancillary devices. For e.g. DSA
>> portal devices use device specific IMS(Interrupt message store) interrupts.
>>
>> dev-msi interrupts are represented by their own device-type. That means
>> dev->msi_list is never contended for different interrupt types. It
>> will either be all PCI-MSI or all device-defined.
> 
> Not sure I follow this, where is the enforcement that only dev-msi or
> normal MSI is being used at one time on a single struct device?
> 

So, in the dev_msi_alloc_irqs, I first check if the dev_is_pci..
If it is a pci device, it is forbidden to use dev-msi and must use the 
pci subsystem calls. dev-msi is to be used for all other custom devices, 
mdev or otherwise.

> Jason
>

Jason Gunthorpe July 22, 2020, 5:35 p.m. UTC | #3

On Wed, Jul 22, 2020 at 10:05:52AM -0700, Dey, Megha wrote:
> 
> 
> On 7/21/2020 9:25 AM, Jason Gunthorpe wrote:
> > On Tue, Jul 21, 2020 at 09:02:41AM -0700, Dave Jiang wrote:
> > > From: Megha Dey <[e-mail]>
> > > 
> > > The dev-msi interrupts are to be allocated/freed only for custom devices,
> > > not standard PCI-MSIX devices.
> > > 
> > > These interrupts are device-defined and they are distinct from the already
> > > existing msi interrupts:
> > > pci-msi: Standard PCI MSI/MSI-X setup format
> > > platform-msi: Platform custom, but device-driver opaque MSI setup/control
> > > arch-msi: fallback for devices not assigned to the generic PCI domain
> > > dev-msi: device defined IRQ domain for ancillary devices. For e.g. DSA
> > > portal devices use device specific IMS(Interrupt message store) interrupts.
> > > 
> > > dev-msi interrupts are represented by their own device-type. That means
> > > dev->msi_list is never contended for different interrupt types. It
> > > will either be all PCI-MSI or all device-defined.
> > 
> > Not sure I follow this, where is the enforcement that only dev-msi or
> > normal MSI is being used at one time on a single struct device?
> > 
> 
> So, in the dev_msi_alloc_irqs, I first check if the dev_is_pci..
> If it is a pci device, it is forbidden to use dev-msi and must use the pci
> subsystem calls. dev-msi is to be used for all other custom devices, mdev or
> otherwise.

What prevents creating a dev-msi directly on the struct pci_device ?

Jason

Dey, Megha Aug. 5, 2020, 8:19 p.m. UTC | #4

Hi Jason,

> -----Original Message-----
> From: Jason Gunthorpe <[e-mail]>
> Sent: Wednesday, July 22, 2020 10:35 AM
> To: Dey, Megha <[e-mail]>
> Cc: Jiang, Dave <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail]; Pan, Jacob
> jun <[e-mail]>; Raj, Ashok <[e-mail]>; Liu, Yi L
> <[e-mail]>; Lu, Baolu <[e-mail]>; Tian, Kevin
> <[e-mail]>; Kumar, Sanjay K <[e-mail]>; Luck,
> Tony <[e-mail]>; Lin, Jing <[e-mail]>; Williams, Dan J
> <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]; Hansen, Dave <[e-mail]>;
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; Ortiz, Samuel <[e-mail]>; Hossain, Mona
> <[e-mail]>; [e-mail]; linux-
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]
> Subject: Re: [PATCH RFC v2 04/18] irq/dev-msi: Introduce APIs to allocate/free
> dev-msi interrupts
> 
> On Wed, Jul 22, 2020 at 10:05:52AM -0700, Dey, Megha wrote:
> >
> >
> > On 7/21/2020 9:25 AM, Jason Gunthorpe wrote:
> > > On Tue, Jul 21, 2020 at 09:02:41AM -0700, Dave Jiang wrote:
> > > > From: Megha Dey <[e-mail]>
> > > >
> > > > The dev-msi interrupts are to be allocated/freed only for custom
> > > > devices, not standard PCI-MSIX devices.
> > > >
> > > > These interrupts are device-defined and they are distinct from the
> > > > already existing msi interrupts:
> > > > pci-msi: Standard PCI MSI/MSI-X setup format
> > > > platform-msi: Platform custom, but device-driver opaque MSI
> > > > setup/control
> > > > arch-msi: fallback for devices not assigned to the generic PCI
> > > > domain
> > > > dev-msi: device defined IRQ domain for ancillary devices. For e.g.
> > > > DSA portal devices use device specific IMS(Interrupt message store)
> interrupts.
> > > >
> > > > dev-msi interrupts are represented by their own device-type. That
> > > > means
> > > > dev->msi_list is never contended for different interrupt types. It
> > > > will either be all PCI-MSI or all device-defined.
> > >
> > > Not sure I follow this, where is the enforcement that only dev-msi
> > > or normal MSI is being used at one time on a single struct device?
> > >
> >
> > So, in the dev_msi_alloc_irqs, I first check if the dev_is_pci..
> > If it is a pci device, it is forbidden to use dev-msi and must use the
> > pci subsystem calls. dev-msi is to be used for all other custom
> > devices, mdev or otherwise.
> 
> What prevents creating a dev-msi directly on the struct pci_device ?

In the next patchset, I have explicitly added code which denies PCI devices from using the dev_msi alloc/free APIS
> 
> Jason

Patch

diff --git a/drivers/base/dev-msi.c b/drivers/base/dev-msi.c
index 43d6ed3ba10f..4cc75bfd62da 100644
--- a/drivers/base/dev-msi.c
+++ b/drivers/base/dev-msi.c
@@ -145,3 +145,26 @@  struct irq_domain *create_remap_dev_msi_irq_domain(struct irq_domain *parent,
 	return domain;
 }
 #endif
+
+int dev_msi_domain_alloc_irqs(struct device *dev, unsigned int nvec,
+			      const struct platform_msi_ops *platform_ops)
+{
+	if (!dev->msi_domain) {
+		dev->msi_domain = dev_msi_default_domain;
+	} else if (dev->msi_domain != dev_msi_default_domain) {
+		dev_WARN_ONCE(dev, 1, "already registered to another irq domain?\n");
+		return -ENXIO;
+	}
+
+	return platform_msi_domain_alloc_irqs(dev, nvec, platform_ops);
+}
+EXPORT_SYMBOL_GPL(dev_msi_domain_alloc_irqs);
+
+void dev_msi_domain_free_irqs(struct device *dev)
+{
+	if (dev->msi_domain != dev_msi_default_domain)
+		dev_WARN_ONCE(dev, 1, "registered to incorrect irq domain?\n");
+
+	platform_msi_domain_free_irqs(dev);
+}
+EXPORT_SYMBOL_GPL(dev_msi_domain_free_irqs);
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 7098ba566bcd..9dde8a43a0f7 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -381,6 +381,10 @@  void platform_msi_mask_irq(struct irq_data *data);
 
 int dev_msi_prepare(struct irq_domain *domain, struct device *dev,
                            int nvec, msi_alloc_info_t *arg);
+
+int dev_msi_domain_alloc_irqs(struct device *dev, unsigned int nvec,
+			      const struct platform_msi_ops *platform_ops);
+void dev_msi_domain_free_irqs(struct device *dev);
 #endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
 
 #ifdef CONFIG_PCI_MSI_IRQ_DOMAIN