[PATCH 0/7] Powerpc MSI Implementation

[PATCH 0/7] Powerpc MSI Implementation

[Michael Ellerman]

This series implements infrastructure for PCI MSI on Powerpc.

It also includes a "backend" for MSI via HT on the MPIC, and supporting
patches. Other backends are in the works.

I think this is ready to merge for 2.6.21, let me know if you disagree ..
unless your name is Segher :D

Greg, assuming you agree this is OK to merge, is this something you'd
want to take in your tree, or are you happy ack it and have it go into
Paul's tree?

cheers

[PATCH 1/7] Rip out the existing powerpc msi stubs

[Michael Ellerman]

Rip out the existing powerpc msi stubs. These were the start of an
implementation based on ppc_md calls, but were never used in mainline.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---

 arch/powerpc/kernel/irq.c     |   28 ----------------------------
 include/asm-powerpc/machdep.h |    5 -----
 2 files changed, 33 deletions(-)

Index: msi/arch/powerpc/kernel/irq.c
===================================================================
--- msi.orig/arch/powerpc/kernel/irq.c
+++ msi/arch/powerpc/kernel/irq.c
@@ -945,34 +945,6 @@ arch_initcall(irq_late_init);
 
 #endif /* CONFIG_PPC_MERGE */
 
-#ifdef CONFIG_PCI_MSI
-int pci_enable_msi(struct pci_dev * pdev)
-{
-	if (ppc_md.enable_msi)
-		return ppc_md.enable_msi(pdev);
-	else
-		return -1;
-}
-EXPORT_SYMBOL(pci_enable_msi);
-
-void pci_disable_msi(struct pci_dev * pdev)
-{
-	if (ppc_md.disable_msi)
-		ppc_md.disable_msi(pdev);
-}
-EXPORT_SYMBOL(pci_disable_msi);
-
-void pci_scan_msi_device(struct pci_dev *dev) {}
-int pci_enable_msix(struct pci_dev* dev, struct msix_entry *entries, int nvec) {return -1;}
-void pci_disable_msix(struct pci_dev *dev) {}
-void msi_remove_pci_irq_vectors(struct pci_dev *dev) {}
-void disable_msi_mode(struct pci_dev *dev, int pos, int type) {}
-void pci_no_msi(void) {}
-EXPORT_SYMBOL(pci_enable_msix);
-EXPORT_SYMBOL(pci_disable_msix);
-
-#endif
-
 #ifdef CONFIG_PPC64
 static int __init setup_noirqdistrib(char *str)
 {
Index: msi/include/asm-powerpc/machdep.h
===================================================================
--- msi.orig/include/asm-powerpc/machdep.h
+++ msi/include/asm-powerpc/machdep.h
@@ -243,11 +243,6 @@ struct machdep_calls {
 	 */
 	void (*machine_kexec)(struct kimage *image);
 #endif /* CONFIG_KEXEC */
-
-#ifdef CONFIG_PCI_MSI
-	int (*enable_msi)(struct pci_dev *pdev);
-	void (*disable_msi)(struct pci_dev *pdev);
-#endif /* CONFIG_PCI_MSI */
 };
 
 extern void power4_idle(void);

Re: [PATCH 1/7] Rip out the existing powerpc msi stubs

[Christoph Hellwig]

On Thu, Jan 11, 2007 at 10:25:16PM +1100, Michael Ellerman wrote:
> Rip out the existing powerpc msi stubs. These were the start of an
> implementation based on ppc_md calls, but were never used in mainline.

Looks good.

[PATCH 2/7] Powerpc MSI implementation

[Michael Ellerman]

Powerpc MSI implementation, based on a collection of "ops" callbacks.
We have to take the ops approach to accomodate RTAS, where firmware
handles almost all details of MSI setup/teardown. Bare-metal MSI
can be accomodated also.

See the comments in include/asm-powerpc/msi.h for more info.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---

 arch/powerpc/kernel/msi.c     |  320 ++++++++++++++++++++++++++++++++++++++++++
 include/asm-powerpc/machdep.h |    6 
 include/asm-powerpc/msi.h     |  173 ++++++++++++++++++++++
 include/linux/pci.h           |    7 
 4 files changed, 506 insertions(+)

Index: msi/arch/powerpc/kernel/msi.c
===================================================================
--- /dev/null
+++ msi/arch/powerpc/kernel/msi.c
@@ -0,0 +1,320 @@
+/*
+ * Copyright 2006-2007, Michael Ellerman, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <asm/msi.h>
+#include <asm/machdep.h>
+
+static struct ppc_msi_ops *get_msi_ops(struct pci_dev *pdev)
+{
+	if (ppc_md.get_msi_ops)
+		return ppc_md.get_msi_ops(pdev);
+
+	return NULL;
+}
+
+/* Activated by pci=nomsi on the command line. */
+static int no_msi;
+
+void pci_no_msi(void)
+{
+	printk(KERN_DEBUG "PCI MSI disabled on command line.\n");
+	no_msi = 1;
+}
+
+
+/* msi_info helpers */
+
+static int alloc_msi_info(struct pci_dev *pdev, int num,
+			  struct msix_entry *entries, int type)
+{
+	struct msi_info *info;
+	unsigned int entries_size;
+
+	entries_size = sizeof(struct msix_entry) * num;
+
+	info = kzalloc(sizeof(struct msi_info) + entries_size, GFP_KERNEL);
+	if (!info) {
+		msi_debug("kzalloc failed for %s\n", pci_name(pdev));
+		return -ENOMEM;
+	}
+
+	info->type = type;
+	info->num = num;
+	info->entries = (struct msix_entry *)(info + 1);
+	memcpy(info->entries, entries, entries_size);
+
+	BUG_ON(pdev->msi_info); /* don't leak info structs */
+	pdev->msi_info = info;
+
+	return 0;
+}
+
+static struct msi_info *get_msi_info(struct pci_dev *pdev)
+{
+	return pdev->msi_info;
+}
+
+static void free_msi_info(struct pci_dev *pdev)
+{
+	kfree(pdev->msi_info);
+	pdev->msi_info = NULL;
+}
+
+
+/* Generic helpers */
+
+static int generic_msi_enable(struct pci_dev *pdev, int nvec,
+				struct msix_entry *entries, int type)
+{
+	struct ppc_msi_ops *ops;
+	int i, rc;
+
+	if (no_msi || !pdev || !entries || !nvec || get_msi_info(pdev)) {
+		msi_debug("precondition failed for %p\n", pdev);
+		return -EINVAL;
+	}
+
+	ops = get_msi_ops(pdev);
+	if (!ops) {
+		msi_debug("no ops for %s\n", pci_name(pdev));
+		return -EINVAL;
+	}
+
+	for (i = 0; i < nvec; i++)
+		entries[i].vector = NO_IRQ;
+
+	rc = ops->check(pdev, nvec, entries, type);
+	if (rc) {
+		msi_debug("check failed (%d) for %s\n", rc, pci_name(pdev));
+		return rc;
+	}
+
+	rc = alloc_msi_info(pdev, nvec, entries, type);
+	if (rc)
+		return rc;
+
+	rc = ops->alloc(pdev, nvec, entries, type);
+	if (rc) {
+		msi_debug("alloc failed (%d) for %s\n", rc, pci_name(pdev));
+		goto out_free_info;
+	}
+
+	if (ops->enable) {
+		rc = ops->enable(pdev, nvec, entries, type);
+		if (rc) {
+			msi_debug("enable failed (%d) for %s\n", rc,
+				pci_name(pdev));
+			goto out_ops_free;
+		}
+	}
+
+	pci_intx(pdev, 0);
+
+	return 0;
+
+ out_ops_free:
+	ops->free(pdev, nvec, entries, type);
+ out_free_info:
+	free_msi_info(pdev);
+
+	return rc;
+}
+
+static int generic_msi_disable(struct pci_dev *pdev, int type)
+{
+	struct ppc_msi_ops *ops;
+	struct msi_info *info;
+
+	if (no_msi || !pdev) {
+		msi_debug("precondition failed for %p\n", pdev);
+		return -1;
+	}
+
+	info = get_msi_info(pdev);
+	if (!info) {
+		msi_debug("No info for %s\n", pci_name(pdev));
+		return -1;
+	}
+
+	ops = get_msi_ops(pdev);
+	if (!ops) {
+		msi_debug("no ops for %s\n", pci_name(pdev));
+		return -1;
+	}
+
+	if (ops->disable)
+		ops->disable(pdev, info->num, info->entries, type);
+
+	ops->free(pdev, info->num, info->entries, type);
+
+	pci_intx(pdev, 1);
+
+	return 0;
+}
+
+
+/* MSI */
+
+int pci_enable_msi(struct pci_dev *pdev)
+{
+	struct msix_entry entry;
+	int rc;
+
+	entry.entry = 0;
+
+	rc = generic_msi_enable(pdev, 1, &entry, PCI_CAP_ID_MSI);
+	if (rc)
+		return rc;
+
+	get_msi_info(pdev)->saved_irq = pdev->irq;
+	pdev->irq = entry.vector;
+	pdev->msi_enabled = 1;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pci_enable_msi);
+
+void pci_disable_msi(struct pci_dev *pdev)
+{
+	if (generic_msi_disable(pdev, PCI_CAP_ID_MSI) != 0)
+		return;
+
+	pdev->irq = get_msi_info(pdev)->saved_irq;
+	free_msi_info(pdev);
+	pdev->msi_enabled = 0;
+}
+EXPORT_SYMBOL_GPL(pci_disable_msi);
+
+
+/* MSI-X */
+
+int pci_enable_msix(struct pci_dev *pdev, struct msix_entry *entries, int nvec)
+{
+	int rc;
+
+	rc = generic_msi_enable(pdev, nvec, entries, PCI_CAP_ID_MSIX);
+	if (rc)
+		return rc;
+
+	pdev->msix_enabled = 1;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pci_enable_msix);
+
+void pci_disable_msix(struct pci_dev *pdev)
+{
+	if (generic_msi_disable(pdev, PCI_CAP_ID_MSIX) != 0)
+		return;
+
+	free_msi_info(pdev);
+	pdev->msix_enabled = 0;
+}
+EXPORT_SYMBOL_GPL(pci_disable_msix);
+
+
+/* Stubs for now */
+
+void disable_msi_mode(struct pci_dev *dev, int pos, int type)
+{
+	return;
+}
+
+void pci_scan_msi_device(struct pci_dev *dev)
+{
+	return;
+}
+
+void msi_remove_pci_irq_vectors(struct pci_dev* dev)
+{
+	return;
+}
+
+
+/* Bare metal enable/disable */
+
+int msi_raw_enable(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type)
+{
+	struct ppc_msi_ops *ops;
+	struct msi_msg msg;
+	int pos;
+	u16 control;
+
+	pos = pci_find_capability(pdev, type);
+	if (!pos) {
+		msi_debug("cap (%d) not found for %s\n", type, pci_name(pdev));
+		return -1;
+	}
+
+	ops = get_msi_ops(pdev);
+	BUG_ON(!ops);
+
+	pci_read_config_word(pdev, pos + PCI_MSI_FLAGS, &control);
+
+	switch (type) {
+	case PCI_CAP_ID_MSI:
+		BUG_ON(!ops->setup_msi_msg);
+
+		ops->setup_msi_msg(pdev, &entries[0], &msg, type);
+
+		pci_write_config_dword(pdev, pos + PCI_MSI_ADDRESS_LO,
+			msg.address_lo);
+
+		if (control & PCI_MSI_FLAGS_64BIT) {
+			pci_write_config_dword(pdev, pos + PCI_MSI_ADDRESS_HI,
+						msg.address_hi);
+			pci_write_config_dword(pdev, pos + PCI_MSI_DATA_64,
+						msg.data);
+		} else {
+			pci_write_config_dword(pdev, pos + PCI_MSI_DATA_32,
+						msg.data);
+		}
+
+		control |= PCI_MSI_FLAGS_ENABLE;
+		break;
+	case PCI_CAP_ID_MSIX:
+		WARN_ON(1); /* XXX implement me */
+		return -1;
+	default:
+		BUG();
+	}
+
+	pci_write_config_word(pdev, pos + PCI_MSI_FLAGS, control);
+
+	return 0;
+}
+
+void msi_raw_disable(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type)
+{
+	int pos;
+	u16 control;
+
+	pos = pci_find_capability(pdev, type);
+	BUG_ON(!pos);
+
+	pci_read_config_word(pdev, pos + PCI_MSI_FLAGS, &control);
+
+	switch (type) {
+	case PCI_CAP_ID_MSI:
+		control &= ~PCI_MSI_FLAGS_ENABLE;
+		break;
+	case PCI_CAP_ID_MSIX:
+		control &= ~PCI_MSIX_FLAGS_ENABLE;
+		break;
+	default:
+		BUG();
+	}
+
+	pci_write_config_word(pdev, pos + PCI_MSI_FLAGS, control);
+
+	return;
+}
Index: msi/include/asm-powerpc/machdep.h
===================================================================
--- msi.orig/include/asm-powerpc/machdep.h
+++ msi/include/asm-powerpc/machdep.h
@@ -30,6 +30,9 @@ struct pci_controller;
 #ifdef CONFIG_KEXEC
 struct kimage;
 #endif
+#ifdef CONFIG_PCI_MSI
+struct ppc_msi_ops;
+#endif
 
 #ifdef CONFIG_SMP
 struct smp_ops_t {
@@ -111,6 +114,9 @@ struct machdep_calls {
 	void		(*pcibios_fixup)(void);
 	int		(*pci_probe_mode)(struct pci_bus *);
 	void		(*pci_irq_fixup)(struct pci_dev *dev);
+#ifdef CONFIG_PCI_MSI
+	struct ppc_msi_ops*	(*get_msi_ops)(struct pci_dev *pdev);
+#endif
 
 	/* To setup PHBs when using automatic OF platform driver for PCI */
 	int		(*pci_setup_phb)(struct pci_controller *host);
Index: msi/include/asm-powerpc/msi.h
===================================================================
--- /dev/null
+++ msi/include/asm-powerpc/msi.h
@@ -0,0 +1,173 @@
+/*
+ * Copyright 2006-2007, Michael Ellerman, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#ifndef _ASM_POWERPC_MSI_H
+#define _ASM_POWERPC_MSI_H
+
+#ifdef __KERNEL__
+#ifndef __ASSEMBLY__
+
+#include <linux/pci.h>
+#include <linux/msi.h>
+
+/*
+ * MSI and MSI-X although different in some details, are also similar in
+ * many respects, and ultimately achieve the same end. Given that, this code
+ * tries as far as possible to implement both MSI and MSI-X with a minimum
+ * of code duplication. We will use "MSI" to refer to both MSI and MSI-X,
+ * except where it is important to differentiate between the two.
+ *
+ * Enabling MSI for a device can be broken down into:
+ *  1) Checking the device can support the type/number of MSIs requested.
+ *  2) Allocating irqs for the MSIs and setting up the irq_descs.
+ *  3) Writing the appropriate configuration to the device and enabling MSIs.
+ *
+ * To implement that we have the following callbacks:
+ *  1) check(pdev, num, msix_entries, type)
+ *  2) alloc(pdev, num, msix_entries, type)
+ *  3) enable(pdev, num, msix_entries, type)
+ *	a) setup_msi_msg(pdev, msix_entry, msi_msg, type)
+ *
+ * We give platforms full control over the enable step. However many
+ * platforms will simply want to program the device using standard PCI
+ * accessors. These platforms can use a generic enable callback and define
+ * a setup_msi_msg() callback which simply fills in the "magic" address and
+ * data values. Other platforms may leave setup_msi_msg() empty.
+ *
+ * Disabling MSI requires:
+ *  1) Disabling MSI on the device.
+ *  2) Freeing the irqs and any associated accounting information.
+ *
+ * Which maps directly to the two callbacks:
+ *  1) disable(pdev, num, msix_entries, type)
+ *  2) free(pdev, num, msix_entries, type)
+ */
+
+struct ppc_msi_ops
+{
+	/* check - Check that the requested MSI allocation is OK.
+	 *
+	 * @pdev:	PCI device structure.
+	 * @num:	The number of MSIs being requested.
+	 * @entries:	An array of @num msix_entry structures.
+	 * @type:	The type, MSI or MSI-X.
+	 *
+	 * This routine is responsible for checking that the given PCI device
+	 * can be allocated the requested type and number of MSIs.
+	 *
+	 * It is up to this routine to determine if the requested number of
+	 * MSIs is valid for the device in question. If the number of MSIs,
+	 * or the particular MSI entries, can not be supported for any
+	 * reason this routine must return non-zero.
+	 *
+	 * If the check is succesful this routine must return 0.
+	 */
+	int (*check) (struct pci_dev *pdev, int num,
+				struct msix_entry *entries, int type);
+
+	/* alloc - Allocate MSIs for the given device.
+	 *
+	 * @pdev:	PCI device structure.
+	 * @num:	The number of MSIs being requested.
+	 * @entries:	An array of @num msix_entry structures.
+	 * @type:	The type, MSI or MSI-X.
+	 *
+	 * This routine is responsible for allocating the number of
+	 * MSIs to the given PCI device.
+	 *
+	 * Upon completion there must be @num MSIs assigned to this device,
+	 * the "vector" member of each struct msix_entry must be filled in
+	 * with the Linux irq number allocated to it. The corresponding
+	 * irq_descs must also be setup with an appropriate handler if
+	 * required.
+	 *
+	 * If the allocation completes succesfully this routine must return 0.
+	 */
+	int (*alloc) (struct pci_dev *pdev, int num,
+				struct msix_entry *entries, int type);
+
+	/* enable - Enable the MSIs on the given device.
+	 *
+	 * @pdev:	PCI device structure.
+	 * @num:	The number of MSIs being requested.
+	 * @entries:	An array of @num msix_entry structures.
+	 * @type:	The type, MSI or MSI-X.
+	 *
+	 * This routine enables the MSIs on the given PCI device.
+	 *
+	 * If the enable completes succesfully this routine must return 0.
+	 *
+	 * This callback is optional.
+	 */
+	int (*enable) (struct pci_dev *pdev, int num,
+				struct msix_entry *entries, int type);
+
+	/* setup_msi_msg - Setup an MSI message for the given device.
+	 *
+	 * @pdev:	PCI device structure.
+	 * @entry:	The MSI entry to create a msi_msg for.
+	 * @msg:	Written with the magic address and data.
+	 * @type:	The type, MSI or MSI-X.
+	 *
+	 * Returns the "magic address and data" used to trigger the msi.
+	 * If the setup is succesful this routine must return 0.
+	 *
+	 * This callback is optional.
+	 */
+	int (*setup_msi_msg) (struct pci_dev *pdev, struct msix_entry *entry,
+				struct msi_msg *msg, int type);
+
+	/* disable - disable the MSI for the given device.
+	 *
+	 * @pdev:	PCI device structure.
+	 * @num:	The number of MSIs to disable.
+	 * @entries:	An array of @num msix_entry structures.
+	 * @type:	The type, MSI or MSI-X.
+	 *
+         * This routine should perform the inverse of enable.
+	 */
+	void (*disable) (struct pci_dev *pdev, int num,
+				struct msix_entry *entries, int type);
+
+	/* free - free the MSIs assigned to the device.
+	 *
+	 * @pdev:	PCI device structure.
+	 * @num:	The number of MSIs.
+	 * @entries:	An array of @num msix_entry structures.
+	 * @type:	The type, MSI or MSI-X.
+	 *
+	 * Free all MSIs and associated resources for the device. If any
+	 * MSIs have been enabled they will have been disabled already by
+	 * the generic code.
+	 */
+	void (*free) (struct pci_dev *pdev, int num,
+				struct msix_entry *entries, int type);
+};
+
+
+/* Used by the MSI code to track MSI info for a pci_dev */
+struct msi_info {
+	int type;
+	unsigned int saved_irq;
+	unsigned int num;
+	struct msix_entry *entries;
+	void __iomem *msix_base;
+};
+
+extern int msi_raw_enable(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type);
+extern void msi_raw_disable(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type);
+
+#define msi_debug(fmt, args...)	\
+	pr_debug("MSI:%s:%d: " fmt, __FUNCTION__, __LINE__, ## args)
+
+#endif /* __ASSEMBLY__ */
+#endif /* __KERNEL__ */
+#endif /* _ASM_POWERPC_MSI_H */
Index: msi/include/linux/pci.h
===================================================================
--- msi.orig/include/linux/pci.h
+++ msi/include/linux/pci.h
@@ -107,6 +107,10 @@ struct pci_cap_saved_state {
 	u32 data[0];
 };
 
+#if defined(CONFIG_PCI_MSI) && defined(CONFIG_PPC_MERGE)
+struct msi_info;
+#endif
+
 /*
  * The pci_dev structure is used to describe PCI devices.
  */
@@ -174,6 +178,9 @@ struct pci_dev {
 	struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
 	int rom_attr_enabled;		/* has display of the rom attribute been enabled? */
 	struct bin_attribute *res_attr[DEVICE_COUNT_RESOURCE]; /* sysfs file for resources */
+#if defined(CONFIG_PCI_MSI) && defined(CONFIG_PPC_MERGE)
+	struct	msi_info *msi_info;
+#endif
 };
 
 #define pci_dev_g(n) list_entry(n, struct pci_dev, global_list)

Re: [PATCH 2/7] Powerpc MSI implementation

[Greg KH]

On Thu, Jan 11, 2007 at 10:25:19PM +1100, Michael Ellerman wrote:
> Powerpc MSI implementation, based on a collection of "ops" callbacks.
> We have to take the ops approach to accomodate RTAS, where firmware
> handles almost all details of MSI setup/teardown. Bare-metal MSI
> can be accomodated also.
> 
> See the comments in include/asm-powerpc/msi.h for more info.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
> 
>  arch/powerpc/kernel/msi.c     |  320 ++++++++++++++++++++++++++++++++++++++++++
>  include/asm-powerpc/machdep.h |    6 
>  include/asm-powerpc/msi.h     |  173 ++++++++++++++++++++++
>  include/linux/pci.h           |    7 
>  4 files changed, 506 insertions(+)
> 
> Index: msi/arch/powerpc/kernel/msi.c

Why isn't all of this in the drivers/pci/msi.c file instead?  You are
creating a new API here that is only availble to the PPC platform, which
is not acceptable.

Please work with the current MSI core code to work properly for PPC,
instead of reinventing the whole thing.

> +#if defined(CONFIG_PCI_MSI) && defined(CONFIG_PPC_MERGE)
> +struct msi_info;
> +#endif

CONFIG_PPC_MERGE?  What is that option for?

So, no, I don't agree with this implementation and don't want to see it
go through anyone's tree into mainline just yet.

thanks,

greg k-h

Re: [PATCH 2/7] Powerpc MSI implementation

[Benjamin Herrenschmidt]

> So, no, I don't agree with this implementation and don't want to see it
> go through anyone's tree into mainline just yet.

So why, when we posted it earlier, you said the exact opposite ? That
you actually liked it and wanted to see it as a replacement of the
current cruft ?

That's been the logic from day one. We implement a generic MSI support
that supports multiple backends, we do it as a powerpc version at first
since that's what we can actively test and provide backends for, then as
a second step, we work with the intel folks to port their stuff over to
our implementation and replace the current crappy generic one.

Ben.

Re: [PATCH 2/7] Powerpc MSI implementation

[Benjamin Herrenschmidt]

On Fri, 2007-01-12 at 08:20 +1100, Benjamin Herrenschmidt wrote:
> > So, no, I don't agree with this implementation and don't want to see it
> > go through anyone's tree into mainline just yet.
> 
> So why, when we posted it earlier, you said the exact opposite ? That
> you actually liked it and wanted to see it as a replacement of the
> current cruft ?
> 
> That's been the logic from day one. We implement a generic MSI support
> that supports multiple backends, we do it as a powerpc version at first
> since that's what we can actively test and provide backends for, then as
> a second step, we work with the intel folks to port their stuff over to
> our implementation and replace the current crappy generic one.

/me calms down a bit ... :-)

Ok, so what about we do:

 - Move that new implementation to drivers/pci
 - Have a CONFIG_NEW_MSI or something like that to select between
the old and the new implementation
 - Have the change to pci_dev be based on that config option

Note: There should be no impact on drivers nor anything out of
drivers/pci and associated arch support code, the API exposed to drivers
should be identical.

That way, it will be easier for Intel/Altix folks to port over to the
new code as it will be accessible to all archs via a config option. Once
Intel & Altix have ported over, the old code can be removed and
CONFIG_NEW_MSI too.

I know that other archs have actually been waiting for that new
infrastructure (well, according to some feedback we got when Michael
first posted it) so that's probably a better approach than having it in
arch/powerpc indeed.

Cheers,
Ben.

Re: [PATCH 2/7] Powerpc MSI implementation

[Greg KH]

On Fri, Jan 12, 2007 at 08:54:57AM +1100, Benjamin Herrenschmidt wrote:
> On Fri, 2007-01-12 at 08:20 +1100, Benjamin Herrenschmidt wrote:
> > > So, no, I don't agree with this implementation and don't want to see it
> > > go through anyone's tree into mainline just yet.
> > 
> > So why, when we posted it earlier, you said the exact opposite ? That
> > you actually liked it and wanted to see it as a replacement of the
> > current cruft ?
> > 
> > That's been the logic from day one. We implement a generic MSI support
> > that supports multiple backends, we do it as a powerpc version at first
> > since that's what we can actively test and provide backends for, then as
> > a second step, we work with the intel folks to port their stuff over to
> > our implementation and replace the current crappy generic one.
> 
> /me calms down a bit ... :-)
> 
> Ok, so what about we do:
> 
>  - Move that new implementation to drivers/pci
>  - Have a CONFIG_NEW_MSI or something like that to select between
> the old and the new implementation
>  - Have the change to pci_dev be based on that config option

No, what's wrong with just fixing everything up to work properly all at
once?  The Altix code should have been a step forward in making this
"abstracted" into something that other arches could use.  If that's not
true, then please work everything so that it all works nicely together.

In short, I don't want to see two different implementations in the tree
at the same time, that's not acceptable, sorry.

thanks,

greg k-h

Re: [PATCH 2/7] Powerpc MSI implementation

[Benjamin Herrenschmidt]

> No, what's wrong with just fixing everything up to work properly all at
> once?  The Altix code should have been a step forward in making this
> "abstracted" into something that other arches could use.  If that's not
> true, then please work everything so that it all works nicely together.
>
> In short, I don't want to see two different implementations in the tree
> at the same time, that's not acceptable, sorry.

So you are saying that despite the current stuff being beyond repair, we
can't provide an alternate working implementation that fits our needs
unless we also port over Altic and Intel, which we don't know and don't
have testing gear, not even within our arch code ?

So you are trying to make sure PowerPC doesn't get MSI support until
next year or what ?

Ben.

Re: [PATCH 2/7] Powerpc MSI implementation

[Greg KH]

On Sat, Jan 13, 2007 at 04:57:43PM +1100, Benjamin Herrenschmidt wrote:
> 
> > No, what's wrong with just fixing everything up to work properly all at
> > once?  The Altix code should have been a step forward in making this
> > "abstracted" into something that other arches could use.  If that's not
> > true, then please work everything so that it all works nicely together.
> >
> > In short, I don't want to see two different implementations in the tree
> > at the same time, that's not acceptable, sorry.
> 
> So you are saying that despite the current stuff being beyond repair, we
> can't provide an alternate working implementation that fits our needs
> unless we also port over Altic and Intel, which we don't know and don't
> have testing gear, not even within our arch code ?

I'm saying I don't want to see 2 different MSI implementations in the
kernel.  I'm sure you can understand this reasoning.

I misunderstood your original patches in that I thought you were
cleaning up the generic versions for everyone, not creating a separate
set of APIs.  Sorry for the misunderstanding.

thanks,

greg k-h

Re: [PATCH 2/7] Powerpc MSI implementation

[Benjamin Herrenschmidt]

> I'm saying I don't want to see 2 different MSI implementations in the
> kernel.  I'm sure you can understand this reasoning.
> 
> I misunderstood your original patches in that I thought you were
> cleaning up the generic versions for everyone, not creating a separate
> set of APIs.  Sorry for the misunderstanding.

It's not a separate set of APIs. The APIs are the same (as far as
drivers are concerned). We are providing a new "core" that indeed aims
are replacing the current one (and provides the same APIs to drivers).

We have two backends implemented for it for now, soon 3 as I need to
write one for Cell blades.

So at this point we have three possible approaches:

 1- We can put it entirely in arch/powerpc. You NAKed the few generic
changes to pci.h but we can hide that in sysdata or some other ppc
specific stuff hanging off pci_dev. That is maybe the easiest approach
for us to at least have MSI support in 2.6.21. (We really do need MSI
support in urgently).

 2- We can put it as I suggested and you just NAKed in
drivers/pci/msi-new.c along with our new backends. Thus, Kconfig defines
wether the old core is used (Intel,Altix) or the new core. I still feel
that's the best option as it makes it easier for other archs (including
Intel & Altix) to port MSI support to the new infrastructure as it will
be there. Maybe the best approach is to compromise here and do that, but
keep it in -mm until Intel and Altix are ported over in which case it
can be merged in linus tree.

 3- We can keep it out of tree and try to work with others to have Intel
and Altix stuff ported over out of tree, until we have a new patch which
completely replaces the existing stuff.

I still think it's better to have the new core in -mm before everything
is ported to it though.

Ben.

Re: [PATCH 2/7] Powerpc MSI implementation

[Greg KH]

On Fri, Jan 12, 2007 at 08:20:02AM +1100, Benjamin Herrenschmidt wrote:
> 
> > So, no, I don't agree with this implementation and don't want to see it
> > go through anyone's tree into mainline just yet.
> 
> So why, when we posted it earlier, you said the exact opposite ? That
> you actually liked it and wanted to see it as a replacement of the
> current cruft ?

Well, I didn't realize at that time that this was ppc only code.  I
missed the location of those function calls, assuming that no one would
put generic PCI calls down in a arch-specific tree only.

thanks,

greg k-h

Re: [PATCH 2/7] Powerpc MSI implementation

[Christoph Hellwig]

On Thu, Jan 11, 2007 at 10:25:19PM +1100, Michael Ellerman wrote:
> Powerpc MSI implementation, based on a collection of "ops" callbacks.
> We have to take the ops approach to accomodate RTAS, where firmware
> handles almost all details of MSI setup/teardown. Bare-metal MSI
> can be accomodated also.
> 
> See the comments in include/asm-powerpc/msi.h for more info.

Most of this shouldn't live in arch/powerpc as it's generic code except
for tiny little bits.

> +static struct ppc_msi_ops *get_msi_ops(struct pci_dev *pdev)
> +{
> +	if (ppc_md.get_msi_ops)
> +		return ppc_md.get_msi_ops(pdev);
> +
> +	return NULL;
> +}

struct ppc_msi_ops should become msi_ops, and this function should
be an arch hook in asm/msi.h

> +static struct msi_info *get_msi_info(struct pci_dev *pdev)
> +{
> +	return pdev->msi_info;
> +}

This wrapper looks rather useless :)

> +#if defined(CONFIG_PCI_MSI) && defined(CONFIG_PPC_MERGE)
> +struct msi_info;
> +#endif

Unconditional, please.

>  /*
>   * The pci_dev structure is used to describe PCI devices.
>   */
> @@ -174,6 +178,9 @@ struct pci_dev {
>  	struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
>  	int rom_attr_enabled;		/* has display of the rom attribute been enabled? */
>  	struct bin_attribute *res_attr[DEVICE_COUNT_RESOURCE]; /* sysfs file for resources */
> +#if defined(CONFIG_PCI_MSI) && defined(CONFIG_PPC_MERGE)
> +	struct	msi_info *msi_info;
> +#endif

and only #ifdef CONFIG_PCI_MSI here please.

Re: [PATCH 2/7] Powerpc MSI implementation

[Benjamin Herrenschmidt]

> Most of this shouldn't live in arch/powerpc as it's generic code except
> for tiny little bits.

Generally agreed.

> > +static struct ppc_msi_ops *get_msi_ops(struct pci_dev *pdev)
> > +{
> > +	if (ppc_md.get_msi_ops)
> > +		return ppc_md.get_msi_ops(pdev);
> > +
> > +	return NULL;
> > +}
>
> struct ppc_msi_ops should become msi_ops, and this function should
> be an arch hook in asm/msi.h

Yes. We need an asm hook as some archs will return one single global ops
instance for any PCI devices, while others (like powerpc) need to be
able to return different ops depending on what bus a device hangs off.

> > +static struct msi_info *get_msi_info(struct pci_dev *pdev)
> > +{
> > +	return pdev->msi_info;
> > +}
> 
> This wrapper looks rather useless :)

It dates from when we were unsure where to put the msi_info (initially
in some ppc-specific pci_dn data structure iirc, until we had Greg's ok
about having it in pci_dev... Easier to just update the wrapper than fix
everybody using it :-)

> > +#if defined(CONFIG_PCI_MSI) && defined(CONFIG_PPC_MERGE)
> > +struct msi_info;
> > +#endif
> 
> Unconditional, please.
>
> >  /*
> >   * The pci_dev structure is used to describe PCI devices.
> >   */
> > @@ -174,6 +178,9 @@ struct pci_dev {
> >  	struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
> >  	int rom_attr_enabled;		/* has display of the rom attribute been enabled? */
> >  	struct bin_attribute *res_attr[DEVICE_COUNT_RESOURCE]; /* sysfs file for resources */
> > +#if defined(CONFIG_PCI_MSI) && defined(CONFIG_PPC_MERGE)
> > +	struct	msi_info *msi_info;
> > +#endif
> 
> and only #ifdef CONFIG_PCI_MSI here please.

Yeah well, CONFIG_PCI_MSI_NEW for now and CONFIG_PCI_MSI once
everything's been ported over the new infrastructure.

Ben.

[PATCH 3/7] Enable MSI on Powerpc

[Michael Ellerman]

Allow PCI_MSI to build on Powerpc. Until we merge and enable some
backends, pci_enable_msi() etc. will always return an error.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---

 arch/powerpc/kernel/Makefile |    4 ++++
 drivers/pci/Kconfig          |    2 +-
 drivers/pci/Makefile         |    4 +++-
 3 files changed, 8 insertions(+), 2 deletions(-)

Index: msi/arch/powerpc/kernel/Makefile
===================================================================
--- msi.orig/arch/powerpc/kernel/Makefile
+++ msi/arch/powerpc/kernel/Makefile
@@ -65,6 +65,10 @@ obj-$(CONFIG_MODULES)		+= $(module-y)
 pci64-$(CONFIG_PPC64)		+= pci_64.o pci_dn.o
 pci32-$(CONFIG_PPC32)		:= pci_32.o
 obj-$(CONFIG_PCI)		+= $(pci64-y) $(pci32-y)
+
+msiobj-y			:= msi.o
+obj-$(CONFIG_PCI_MSI)		+= $(msiobj-y)
+
 kexec-$(CONFIG_PPC64)		:= machine_kexec_64.o
 kexec-$(CONFIG_PPC32)		:= machine_kexec_32.o
 obj-$(CONFIG_KEXEC)		+= machine_kexec.o crash.o $(kexec-y)
Index: msi/drivers/pci/Kconfig
===================================================================
--- msi.orig/drivers/pci/Kconfig
+++ msi/drivers/pci/Kconfig
@@ -4,7 +4,7 @@
 config PCI_MSI
 	bool "Message Signaled Interrupts (MSI and MSI-X)"
 	depends on PCI
-	depends on (X86_LOCAL_APIC && X86_IO_APIC) || IA64
+	depends on (X86_LOCAL_APIC && X86_IO_APIC) || IA64 || PPC_MERGE
 	help
 	   This allows device drivers to enable MSI (Message Signaled
 	   Interrupts).  Message Signaled Interrupts enable a device to
Index: msi/drivers/pci/Makefile
===================================================================
--- msi.orig/drivers/pci/Makefile
+++ msi/drivers/pci/Makefile
@@ -14,8 +14,10 @@ obj-$(CONFIG_HOTPLUG) += hotplug.o
 # Build the PCI Hotplug drivers if we were asked to
 obj-$(CONFIG_HOTPLUG_PCI) += hotplug/
 
-# Build the PCI MSI interrupt support
+# Build the PCI MSI interrupt support, but not for arch/powerpc
+ifndef CONFIG_PPC_MERGE
 obj-$(CONFIG_PCI_MSI) += msi.o
+endif
 
 # Build the Hypertransport interrupt support
 obj-$(CONFIG_HT_IRQ) += htirq.o

Re: [PATCH 3/7] Enable MSI on Powerpc

[Christoph Hellwig]

> -# Build the PCI MSI interrupt support
> +# Build the PCI MSI interrupt support, but not for arch/powerpc
> +ifndef CONFIG_PPC_MERGE
>  obj-$(CONFIG_PCI_MSI) += msi.o
> +endif

This is not very nice.  I'd rather prefer this code beeing funnel in
through your higher level code.

Re: [PATCH 3/7] Enable MSI on Powerpc

[Benjamin Herrenschmidt]

On Thu, 2007-01-11 at 22:37 +0100, Christoph Hellwig wrote:
> > -# Build the PCI MSI interrupt support
> > +# Build the PCI MSI interrupt support, but not for arch/powerpc
> > +ifndef CONFIG_PPC_MERGE
> >  obj-$(CONFIG_PCI_MSI) += msi.o
> > +endif
> 
> This is not very nice.  I'd rather prefer this code beeing funnel in
> through your higher level code.

There are some issues in hooking the existing intel/altix stuff on
Michael's infrastructure that I'd rather not even go near for 2.6.21 so
I'd prefer if we could just build the new stuff or the old stuff based
on config options as a first step...

>From there, we can start working on doing proper new back-ends for the
new stuff for intel and altix.

Among others, there have been some nasty calls to the MSI stuff by the
pci power management code that I'm not sure the new stuff is dealing
with just yet (we don't need them yet for the few powerpc machines for
which we have backends for now, though we'll have to fix it).

I think it's important that we get this code in 2.6.21 and not wait for
having all the old stuff ported to it, as it's really getting fairly
urgent to get MSI support in, and I'm quite confident having both old
and new infrastructures in until intel/altix are ported over shouldn't
be a problem.

Ben.

[PATCH 5/7] MPIC MSI backend

[Michael Ellerman]

MPIC MSI backend. Based on code from Segher, heavily hacked by me.
Renamed to mpic_htmsi, as it only deals with MSI over Hypertransport.

We properly discover the HT magic address by reading the config space.
Now we have an irq allocator we can support > 1 MSI, and we don't reuse
the LSI.

Tested, succesfully getting MSIs from the tg3 via HT/PCI-X on a JS21
running SLOF. Successive insmod/rmmods working too.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---

 arch/powerpc/sysdev/Makefile     |    2 
 arch/powerpc/sysdev/mpic.h       |    7 +
 arch/powerpc/sysdev/mpic_htmsi.c |  191 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 199 insertions(+), 1 deletion(-)

Index: msi/arch/powerpc/sysdev/Makefile
===================================================================
--- msi.orig/arch/powerpc/sysdev/Makefile
+++ msi/arch/powerpc/sysdev/Makefile
@@ -3,7 +3,7 @@ EXTRA_CFLAGS			+= -mno-minimal-toc
 endif
 
 mpic-obj-y			:= mpic.o
-mpic-obj-$(CONFIG_PCI_MSI)	+= mpic_msi.o
+mpic-obj-$(CONFIG_PCI_MSI)	+= mpic_msi.o mpic_htmsi.o
 obj-$(CONFIG_MPIC)		+= $(mpic-obj-y)
 
 obj-$(CONFIG_PPC_INDIRECT_PCI)	+= indirect_pci.o
Index: msi/arch/powerpc/sysdev/mpic.h
===================================================================
--- msi.orig/arch/powerpc/sysdev/mpic.h
+++ msi/arch/powerpc/sysdev/mpic.h
@@ -15,11 +15,18 @@
 #include <asm/msi.h>
 
 #ifdef CONFIG_PCI_MSI
+extern int mpic_htmsi_init(struct mpic *mpic);
+
 extern int mpic_msi_init_allocator(struct mpic *mpic);
 extern void mpic_msi_reserve_hwirq(struct mpic *mpic, irq_hw_number_t hwirq);
 extern irq_hw_number_t mpic_msi_alloc_hwirqs(struct mpic *mpic, int num);
 extern void mpic_msi_free_hwirqs(struct mpic *mpic, int offset, int num);
 #else
+static inline int mpic_htmsi_init(struct mpic *mpic)
+{
+	return -1;
+}
+
 static inline void mpic_msi_reserve_hwirq(struct mpic *mpic,
 					  irq_hw_number_t hwirq)
 {
Index: msi/arch/powerpc/sysdev/mpic_htmsi.c
===================================================================
--- /dev/null
+++ msi/arch/powerpc/sysdev/mpic_htmsi.c
@@ -0,0 +1,191 @@
+/*
+ * Copyright 2006, Segher Boessenkool, IBM Corporation.
+ * Copyright 2006-2007, Michael Ellerman, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2 of the
+ * License.
+ *
+ */
+
+#include <linux/irq.h>
+#include <linux/bootmem.h>
+#include <asm/msi.h>
+#include <asm/mpic.h>
+#include <asm/prom.h>
+#include <asm/hw_irq.h>
+#include <asm/ppc-pci.h>
+
+#include "mpic.h"
+
+/* XXX Do we ever need > 1 of these? void * msi_ops.data perhaps ? */
+static struct mpic *msi_mpic;
+
+static unsigned int find_ht_msi_capability(struct pci_dev *pdev)
+{
+	unsigned int pos = pci_find_capability(pdev, PCI_CAP_ID_HT);
+	u8 subcap, ttl = 48;
+
+	while (pos && ttl--) {
+		pci_read_config_byte(pdev, pos + 3, &subcap);
+		if ((subcap & 0xF8) == HT_CAPTYPE_MSI_MAPPING)
+			return pos;
+		pos = pci_find_next_capability(pdev, pos, PCI_CAP_ID_HT);
+	}
+
+	return 0;
+}
+
+static u64 read_ht_magic_addr(struct pci_dev *pdev, unsigned int pos)
+{
+	u8 flags;
+	u32 tmp;
+	u64 addr;
+
+	pci_read_config_byte(pdev, pos + HT_MSI_FLAGS, &flags);
+
+	if (flags & HT_MSI_FLAGS_FIXED)
+		return HT_MSI_FIXED_ADDR;
+
+	pci_read_config_dword(pdev, pos + HT_MSI_ADDR_LO, &tmp);
+	addr = tmp & HT_MSI_ADDR_LO_MASK;
+	pci_read_config_dword(pdev, pos + HT_MSI_ADDR_HI, &tmp);
+	addr = addr | ((u64)tmp << 32);
+
+	return addr;
+}
+
+static u64 find_ht_magic_addr(struct pci_dev *pdev)
+{
+	struct pci_bus *bus;
+	unsigned int pos;
+
+	for (bus = pdev->bus; bus; bus = bus->parent) {
+		pos = find_ht_msi_capability(bus->self);
+		if (pos)
+			return read_ht_magic_addr(bus->self, pos);
+	}
+
+	return 0;
+}
+
+static int htmsi_check(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type)
+{
+	if (type == PCI_CAP_ID_MSIX) {
+		msi_debug("MSI-X unsupported for %s\n", pci_name(pdev));
+		return 1;
+	}
+
+	/* If we can't find a magic address then MSI ain't gonna work */
+	if (find_ht_magic_addr(pdev) == 0) {
+		msi_debug("no magic address found for %s\n", pci_name(pdev));
+		return 1;
+	}
+
+	return 0;
+}
+
+static void htmsi_free(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type)
+{
+	irq_hw_number_t hwirq;
+	int i;
+
+	hwirq = irq_map[entries[0].vector].hwirq;
+
+	for (i = 0; i < num; i++) {
+		irq_dispose_mapping(entries[i].vector);
+		entries[i].vector = NO_IRQ;
+	}
+
+	msi_debug("freeing %d hwirqs for msi at offset 0x%lx\n", num, hwirq);
+	mpic_msi_free_hwirqs(msi_mpic, hwirq, num);
+
+	return;
+}
+
+static int htmsi_alloc(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type)
+{
+	int i;
+	irq_hw_number_t hwirq;
+	unsigned int virq;
+
+	hwirq = mpic_msi_alloc_hwirqs(msi_mpic, num);
+	if (hwirq < 0) {
+		msi_debug("failed allocating %d hwirqs for %s\n", num,
+			  pci_name(pdev));
+		return -1;
+	}
+
+	for (i = 0; i < num; i++) {
+		/* FIXME should we save the existing type */
+		set_irq_type(hwirq, IRQ_TYPE_EDGE_RISING);
+
+		virq = irq_create_mapping(msi_mpic->irqhost, hwirq);
+		if (virq == NO_IRQ) {
+			msi_debug("failed mapping hwirq 0x%lx for %s\n", hwirq,
+				  pci_name(pdev));
+			goto out_free;
+		}
+
+		entries[i].vector = virq;
+		hwirq++;
+	}
+
+	return 0;
+
+ out_free:
+	htmsi_free(pdev, num, entries, type);
+	return -1;
+}
+
+static int htmsi_setup_msi_msg(struct pci_dev *pdev,
+		struct msix_entry *entry, struct msi_msg *msg, int type)
+{
+	u64 addr;
+
+	addr = find_ht_magic_addr(pdev);
+	msg->address_lo = addr & 0xFFFFFFFF;
+	msg->address_hi = addr >> 32;
+	msg->data = irq_map[entry->vector].hwirq;
+
+	msi_debug("allocated irq %d at 0x%lx for %s\n", entry->vector,
+			addr, pci_name(pdev));
+
+	return 0;
+}
+
+static struct ppc_msi_ops mpic_htmsi_ops = {
+	.check = htmsi_check,
+	.alloc = htmsi_alloc,
+	.free = htmsi_free,
+	.enable = msi_raw_enable,
+	.disable = msi_raw_disable,
+	.setup_msi_msg = htmsi_setup_msi_msg,
+};
+
+static struct ppc_msi_ops *htmsi_get_msi_ops(struct pci_dev *pdev)
+{
+	return &mpic_htmsi_ops;
+}
+
+int mpic_htmsi_init(struct mpic *mpic)
+{
+	int rc;
+
+	rc = mpic_msi_init_allocator(mpic);
+	if (rc) {
+		pr_debug("mpic_htmsi_init: Error allocating bitmap!\n");
+		return rc;
+	}
+
+	msi_mpic = mpic;
+
+	pr_debug("mpic_htmsi_init: Registering MPIC MSI ops.\n");
+	ppc_md.get_msi_ops = htmsi_get_msi_ops;
+
+	return 0;
+}

[PATCH 4/7] MPIC MSI allocator

[Michael Ellerman]

To support MSI on MPIC we need a way to reserve and allocate hardware irq
numbers, this patch implements an allocator for that.

Updated to only do dogy-U3-fallback-hacks on U3, all other platforms must
define a "msi-ranges" property on their MPIC node for MSI to work.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---

 arch/powerpc/sysdev/Makefile   |    5 -
 arch/powerpc/sysdev/mpic.c     |    4 
 arch/powerpc/sysdev/mpic.h     |   30 +++++++
 arch/powerpc/sysdev/mpic_msi.c |  172 +++++++++++++++++++++++++++++++++++++++++
 include/asm-powerpc/mpic.h     |    5 +
 5 files changed, 215 insertions(+), 1 deletion(-)

Index: msi/arch/powerpc/sysdev/Makefile
===================================================================
--- msi.orig/arch/powerpc/sysdev/Makefile
+++ msi/arch/powerpc/sysdev/Makefile
@@ -2,7 +2,10 @@ ifeq ($(CONFIG_PPC64),y)
 EXTRA_CFLAGS			+= -mno-minimal-toc
 endif
 
-obj-$(CONFIG_MPIC)		+= mpic.o
+mpic-obj-y			:= mpic.o
+mpic-obj-$(CONFIG_PCI_MSI)	+= mpic_msi.o
+obj-$(CONFIG_MPIC)		+= $(mpic-obj-y)
+
 obj-$(CONFIG_PPC_INDIRECT_PCI)	+= indirect_pci.o
 obj-$(CONFIG_PPC_MPC106)	+= grackle.o
 obj-$(CONFIG_PPC_DCR)		+= dcr.o
Index: msi/arch/powerpc/sysdev/mpic.c
===================================================================
--- msi.orig/arch/powerpc/sysdev/mpic.c
+++ msi/arch/powerpc/sysdev/mpic.c
@@ -36,6 +36,8 @@
 #include <asm/mpic.h>
 #include <asm/smp.h>
 
+#include "mpic.h"
+
 #ifdef DEBUG
 #define DBG(fmt...) printk(fmt)
 #else
@@ -825,6 +827,8 @@ static int mpic_host_map(struct irq_host
 	if (hw >= mpic->irq_count)
 		return -EINVAL;
 
+	mpic_msi_reserve_hwirq(mpic, hw);
+
 	/* Default chip */
 	chip = &mpic->hc_irq;
 
Index: msi/arch/powerpc/sysdev/mpic.h
===================================================================
--- /dev/null
+++ msi/arch/powerpc/sysdev/mpic.h
@@ -0,0 +1,30 @@
+#ifndef _POWERPC_SYSDEV_MPIC_H
+#define _POWERPC_SYSDEV_MPIC_H
+
+/*
+ * Copyright 2006-2007, Michael Ellerman, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2 of the
+ * License.
+ *
+ */
+
+#include <linux/bitmap.h>
+#include <asm/msi.h>
+
+#ifdef CONFIG_PCI_MSI
+extern int mpic_msi_init_allocator(struct mpic *mpic);
+extern void mpic_msi_reserve_hwirq(struct mpic *mpic, irq_hw_number_t hwirq);
+extern irq_hw_number_t mpic_msi_alloc_hwirqs(struct mpic *mpic, int num);
+extern void mpic_msi_free_hwirqs(struct mpic *mpic, int offset, int num);
+#else
+static inline void mpic_msi_reserve_hwirq(struct mpic *mpic,
+					  irq_hw_number_t hwirq)
+{
+	return;
+}
+#endif
+
+#endif /* _POWERPC_SYSDEV_MPIC_H */
Index: msi/arch/powerpc/sysdev/mpic_msi.c
===================================================================
--- /dev/null
+++ msi/arch/powerpc/sysdev/mpic_msi.c
@@ -0,0 +1,172 @@
+/*
+ * Copyright 2006-2007, Michael Ellerman, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2 of the
+ * License.
+ *
+ */
+
+#include <linux/irq.h>
+#include <linux/bootmem.h>
+#include <asm/msi.h>
+#include <asm/mpic.h>
+#include <asm/prom.h>
+#include <asm/hw_irq.h>
+#include <asm/ppc-pci.h>
+
+
+static void __mpic_msi_reserve_hwirq(struct mpic *mpic, irq_hw_number_t hwirq)
+{
+	msi_debug("reserving hwirq 0x%lx\n", hwirq);
+	bitmap_allocate_region(mpic->hwirq_bitmap, hwirq, 0);
+}
+
+void mpic_msi_reserve_hwirq(struct mpic *mpic, irq_hw_number_t hwirq)
+{
+	unsigned long flags;
+
+	/* The mpic calls this even when there is no allocator setup */
+	if (!mpic->hwirq_bitmap)
+		return;
+
+	spin_lock_irqsave(&mpic->bitmap_lock, flags);
+	__mpic_msi_reserve_hwirq(mpic, hwirq);
+	spin_unlock_irqrestore(&mpic->bitmap_lock, flags);
+}
+
+irq_hw_number_t mpic_msi_alloc_hwirqs(struct mpic *mpic, int num)
+{
+	unsigned long flags;
+	int offset, order = fls(num);
+
+	spin_lock_irqsave(&mpic->bitmap_lock, flags);
+	/*
+	 * This is fast, but stricter than we need. We might want to add
+	 * a fallback routine which does a linear search with no alignment.
+	 */
+	offset = bitmap_find_free_region(mpic->hwirq_bitmap, mpic->irq_count,
+					 order);
+	spin_unlock_irqrestore(&mpic->bitmap_lock, flags);
+
+	msi_debug("allocated %d (2^%d) at offset %d\n", num, order, offset);
+
+	return offset;
+}
+
+void mpic_msi_free_hwirqs(struct mpic *mpic, int offset, int num)
+{
+	unsigned long flags;
+	int order = fls(num);
+
+	msi_debug("freeing %d (2^%d) at offset %d\n", num, order, offset);
+
+	spin_lock_irqsave(&mpic->bitmap_lock, flags);
+	bitmap_release_region(mpic->hwirq_bitmap, offset, order);
+	spin_unlock_irqrestore(&mpic->bitmap_lock, flags);
+}
+
+#ifdef CONFIG_MPIC_BROKEN_U3
+static int mpic_msi_reserve_u3_hwirqs(struct mpic *mpic)
+{
+	irq_hw_number_t hwirq;
+	struct irq_host_ops *ops = mpic->irqhost->ops;
+	struct device_node *np;
+	int flags, index, i;
+	struct of_irq oirq;
+
+	msi_debug("found U3, guessing msi allocator setup\n");
+
+	/* Reserve source numbers we know are reserved in the HW */
+	for (i = 0;   i < 8;   i++) __mpic_msi_reserve_hwirq(mpic, i);
+	for (i = 42;  i < 46;  i++) __mpic_msi_reserve_hwirq(mpic, i);
+	for (i = 100; i < 105; i++) __mpic_msi_reserve_hwirq(mpic, i);
+
+	np = NULL;
+	while ((np = of_find_all_nodes(np))) {
+		msi_debug("mapping hwirqs for %s\n", np->full_name);
+
+		index = 0;
+		while (of_irq_map_one(np, index++, &oirq) == 0) {
+			ops->xlate(mpic->irqhost, NULL, oirq.specifier,
+						oirq.size, &hwirq, &flags);
+			__mpic_msi_reserve_hwirq(mpic, hwirq);
+		}
+	}
+
+	return 0;
+}
+#else
+static int mpic_msi_reserve_u3_hwirqs(struct mpic *mpic) { return -1; }
+#endif
+
+static int mpic_msi_reserve_dt_hwirqs(struct mpic *mpic)
+{
+	int i, len;
+	const u32 *p;
+
+	p = get_property(mpic->of_node, "msi-ranges", &len);
+	if (!p) {
+		msi_debug("no msi-ranges property found on %s\n",
+			  mpic->of_node->full_name);
+		return -ENODEV;
+	}
+
+	if (len % 8 != 0) {
+		printk(KERN_WARNING "Malformed msi-ranges property on %s\n",
+		       mpic->of_node->full_name);
+		return -EINVAL;
+	}
+
+	/* msi-ranges defines _available_ ranges */
+	bitmap_allocate_region(mpic->hwirq_bitmap, 0, fls(mpic->irq_count));
+
+	/* Format is: (<u32 start> <u32 count>)+ */
+	len /= sizeof(u32);
+	for (i = 0; i < len / 2; i++, p += 2)
+		mpic_msi_free_hwirqs(mpic, *p, *(p + 1));
+
+	return 0;
+}
+
+int mpic_msi_init_allocator(struct mpic *mpic)
+{
+	int rc, size;
+
+	BUG_ON(mpic->hwirq_bitmap);
+	spin_lock_init(&mpic->bitmap_lock);
+
+	size = mpic->irq_count / 8;
+	msi_debug("allocator bitmap size is 0x%x bytes\n", size);
+
+	if (mem_init_done)
+		mpic->hwirq_bitmap = kmalloc(size, GFP_KERNEL);
+	else
+		mpic->hwirq_bitmap = alloc_bootmem(size);
+
+	if (!mpic->hwirq_bitmap) {
+		msi_debug("no mem allocating allocator bitmap!\n");
+		return -ENOMEM;
+	}
+
+	memset(mpic->hwirq_bitmap, 0, size);
+
+	rc = mpic_msi_reserve_dt_hwirqs(mpic);
+	if (rc) {
+		if (mpic->flags & MPIC_BROKEN_U3)
+			rc = mpic_msi_reserve_u3_hwirqs(mpic);
+
+		if (rc)
+			goto out_free;
+	}
+
+	return 0;
+
+ out_free:
+	if (mem_init_done)
+		kfree(mpic->hwirq_bitmap);
+
+	mpic->hwirq_bitmap = NULL;
+	return rc;
+}
Index: msi/include/asm-powerpc/mpic.h
===================================================================
--- msi.orig/include/asm-powerpc/mpic.h
+++ msi/include/asm-powerpc/mpic.h
@@ -300,6 +300,11 @@ struct mpic
 	u32			*hw_set;
 #endif
 
+#ifdef CONFIG_PCI_MSI
+	spinlock_t		bitmap_lock;
+	unsigned long		*hwirq_bitmap;
+#endif
+
 	/* link */
 	struct mpic		*next;
 };

Re: [PATCH 4/7] MPIC MSI allocator

[Segher Boessenkool]

> +	msi_debug("found U3, guessing msi allocator setup\n");
> +
> +	/* Reserve source numbers we know are reserved in the HW */
> +	for (i = 0;   i < 8;   i++) __mpic_msi_reserve_hwirq(mpic, i);
> +	for (i = 42;  i < 46;  i++) __mpic_msi_reserve_hwirq(mpic, i);
> +	for (i = 100; i < 105; i++) __mpic_msi_reserve_hwirq(mpic, i);

Coding style.  The ranges you reserve are a bit too wide,
but that is certainly safe.

> +static int mpic_msi_reserve_u3_hwirqs(struct mpic *mpic) { return -1; 
> }

Coding style.

Looks good to me otherwise.


Segher

Re: [PATCH 4/7] MPIC MSI allocator

[Will Schmidt]

On Thu, 2007-11-01 at 16:14 +0100, Segher Boessenkool wrote:
> > +	msi_debug("found U3, guessing msi allocator setup\n");
> > +
> > +	/* Reserve source numbers we know are reserved in the HW */
> > +	for (i = 0;   i < 8;   i++) __mpic_msi_reserve_hwirq(mpic, i);
> > +	for (i = 42;  i < 46;  i++) __mpic_msi_reserve_hwirq(mpic, i);
> > +	for (i = 100; i < 105; i++) __mpic_msi_reserve_hwirq(mpic, i);
> 
> Coding style.  The ranges you reserve are a bit too wide,
> but that is certainly safe.

Can you clarify that comment a bit for me?    Why are they too wide, and
if they are too wide, how are they still safe?   

-Will

> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 4/7] MPIC MSI allocator

[Segher Boessenkool]

>>> +	/* Reserve source numbers we know are reserved in the HW */
>>> +	for (i = 0;   i < 8;   i++) __mpic_msi_reserve_hwirq(mpic, i);
>>> +	for (i = 42;  i < 46;  i++) __mpic_msi_reserve_hwirq(mpic, i);
>>> +	for (i = 100; i < 105; i++) __mpic_msi_reserve_hwirq(mpic, i);
>>
>> Coding style.  The ranges you reserve are a bit too wide,
>> but that is certainly safe.
>
> Can you clarify that comment a bit for me?    Why are they too wide, 
> and
> if they are too wide, how are they still safe?

A few vectors that can be used freely are marked as
not being so.  That is fine, there are plenty left.


Segher

Re: [PATCH 4/7] MPIC MSI allocator

[Olof Johansson]

On Thu, Jan 11, 2007 at 10:25:21PM +1100, Michael Ellerman wrote:
> To support MSI on MPIC we need a way to reserve and allocate hardware irq
> numbers, this patch implements an allocator for that.
> 
> Updated to only do dogy-U3-fallback-hacks on U3, all other platforms must
> define a "msi-ranges" property on their MPIC node for MSI to work.

The msi-ranges spec isn't documented anywhere besides the code where it's
used. Please add it to Documentation/powerpc/booting-without-of.txt too.


-Olof

Re: [PATCH 4/7] MPIC MSI allocator

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1110 bytes --]

On Thu, 2007-01-11 at 18:27 -0600, Olof Johansson wrote:
> On Thu, Jan 11, 2007 at 10:25:21PM +1100, Michael Ellerman wrote:
> > To support MSI on MPIC we need a way to reserve and allocate hardware irq
> > numbers, this patch implements an allocator for that.
> > 
> > Updated to only do dogy-U3-fallback-hacks on U3, all other platforms must
> > define a "msi-ranges" property on their MPIC node for MSI to work.
> 
> The msi-ranges spec isn't documented anywhere besides the code where it's
> used. Please add it to Documentation/powerpc/booting-without-of.txt too.

OK, will do.

I'm going to rename it to "msi-available-ranges" while I'm at it, PAPR
defines an "ibm,msi-ranges" property which has a very different meaning,
so I think the longer name is less confusing - even if there's never
going to be any machine with both.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[PATCH 6/7] Enable MSI mappings for MPIC

[Michael Ellerman]

On some Apple machines the HT MSI mappings are not enabled by firmware, so
we need to do it by hand.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---

 arch/powerpc/sysdev/mpic.c |   49 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)

Index: msi/arch/powerpc/sysdev/mpic.c
===================================================================
--- msi.orig/arch/powerpc/sysdev/mpic.c
+++ msi/arch/powerpc/sysdev/mpic.c
@@ -379,7 +379,51 @@ static void mpic_shutdown_ht_interrupt(s
 	spin_unlock_irqrestore(&mpic->fixup_lock, flags);
 }
 
-static void __init mpic_scan_ht_pic(struct mpic *mpic, u8 __iomem *devbase,
+#ifdef CONFIG_PCI_MSI
+static void __init mpic_setup_ht_msi(struct mpic *mpic, u8 __iomem *devbase,
+				    unsigned int devfn)
+{
+	u8 __iomem *base;
+	u8 pos, flags;
+	u64 addr = 0;
+
+	for (pos = readb(devbase + PCI_CAPABILITY_LIST); pos != 0;
+	     pos = readb(devbase + pos + PCI_CAP_LIST_NEXT)) {
+		u8 id = readb(devbase + pos + PCI_CAP_LIST_ID);
+		if (id == PCI_CAP_ID_HT) {
+			id = readb(devbase + pos + 3);
+			if ((id & HT_5BIT_CAP_MASK) == HT_CAPTYPE_MSI_MAPPING)
+				break;
+		}
+	}
+
+	if (pos == 0)
+		return;
+
+	base = devbase + pos;
+
+	flags = readb(base + HT_MSI_FLAGS);
+	if (!(flags & HT_MSI_FLAGS_FIXED)) {
+		addr = readl(base + HT_MSI_ADDR_LO) & HT_MSI_ADDR_LO_MASK;
+		addr = addr | ((u64)readl(base + HT_MSI_ADDR_HI) << 32);
+	}
+
+	printk(KERN_INFO "mpic:   - HT:%02x.%x %s MSI mapping found @ 0x%lx\n",
+		PCI_SLOT(devfn), PCI_FUNC(devfn),
+		flags & HT_MSI_FLAGS_ENABLE ? "enabled" : "disabled", addr);
+
+	if (!(flags & HT_MSI_FLAGS_ENABLE))
+		writeb(flags | HT_MSI_FLAGS_ENABLE, base + HT_MSI_FLAGS);
+}
+#else
+static void __init mpic_setup_ht_msi(struct mpic *mpic, u8 __iomem *devbase,
+				    unsigned int devfn)
+{
+	return;
+}
+#endif
+
+static void __init mpic_setup_ht_pic(struct mpic *mpic, u8 __iomem *devbase,
 				    unsigned int devfn, u32 vdid)
 {
 	int i, irq, n;
@@ -469,7 +513,8 @@ static void __init mpic_scan_ht_pics(str
 		if (!(s & PCI_STATUS_CAP_LIST))
 			goto next;
 
-		mpic_scan_ht_pic(mpic, devbase, devfn, l);
+		mpic_setup_ht_pic(mpic, devbase, devfn, l);
+		mpic_setup_ht_msi(mpic, devbase, devfn);
 
 	next:
 		/* next device, if function 0 */

Re: [PATCH 6/7] Enable MSI mappings for MPIC

[Segher Boessenkool]

> +	flags = readb(base + HT_MSI_FLAGS);
> +	if (!(flags & HT_MSI_FLAGS_FIXED)) {
> +		addr = readl(base + HT_MSI_ADDR_LO) & HT_MSI_ADDR_LO_MASK;
> +		addr = addr | ((u64)readl(base + HT_MSI_ADDR_HI) << 32);
> +	}
> +
> +	printk(KERN_INFO "mpic:   - HT:%02x.%x %s MSI mapping found @ 
> 0x%lx\n",
> +		PCI_SLOT(devfn), PCI_FUNC(devfn),
> +		flags & HT_MSI_FLAGS_ENABLE ? "enabled" : "disabled", addr);

It's not safe in general to just take what the hardware
is currently set to.

Perhaps it is best to just always program 0xfee00000 (it's
a per-bus thing so you can use the same address everywhere),
since that address *has* to not conflict with other mappings
already, it being the standard fixed address and everything.


Segher

[PATCH 7/7] Activate MSI for the MPIC backend on U3

[Michael Ellerman]

If we have a U3 and it's the primary, enable MSIs via HT.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---

 arch/powerpc/sysdev/mpic.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Index: msi/arch/powerpc/sysdev/mpic.c
===================================================================
--- msi.orig/arch/powerpc/sysdev/mpic.c
+++ msi/arch/powerpc/sysdev/mpic.c
@@ -1171,8 +1171,10 @@ void __init mpic_init(struct mpic *mpic)
 
 	/* Do the HT PIC fixups on U3 broken mpic */
 	DBG("MPIC flags: %x\n", mpic->flags);
-	if ((mpic->flags & MPIC_BROKEN_U3) && (mpic->flags & MPIC_PRIMARY))
- 		mpic_scan_ht_pics(mpic);
+	if ((mpic->flags & MPIC_BROKEN_U3) && (mpic->flags & MPIC_PRIMARY)) {
+		mpic_scan_ht_pics(mpic);
+		mpic_htmsi_init(mpic);
+	}
 
 	for (i = 0; i < mpic->num_sources; i++) {
 		/* start with vector = source number, and masked */

Re: [PATCH 0/7] Powerpc MSI Implementation

[Segher Boessenkool]

> I think this is ready to merge for 2.6.21, let me know if you disagree 
> ..
> unless your name is Segher :D

Oh I agree it is ready, just a few nits :-)

Thanks for doing all this stuff!


Segher