[RFC/PATCH 0/7] Powerpc MSI stuff

[RFC/PATCH 0/7] Powerpc MSI stuff

[Michael Ellerman]

Here's what we've got so far towards an MSI layer for Powerpc. Perhaps
it's interesting for other people too.

Neither of the backends have had any real testing, waiting on hardware, but
I think there's enough to make me think the design makes (some) sense.

cheers

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

[Michael Ellerman]

Rip out the existing powerpc msi stubs.

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: to-merge/arch/powerpc/kernel/irq.c
===================================================================
--- to-merge.orig/arch/powerpc/kernel/irq.c
+++ to-merge/arch/powerpc/kernel/irq.c
@@ -820,34 +820,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: to-merge/include/asm-powerpc/machdep.h
===================================================================
--- to-merge.orig/include/asm-powerpc/machdep.h
+++ to-merge/include/asm-powerpc/machdep.h
@@ -239,11 +239,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);

[RFC/PATCH 2/7] Make some MSIX defines generic

[Michael Ellerman]

Make some MSIX defines generic

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

 drivers/pci/msi.h        |    8 --------
 include/linux/pci_regs.h |    8 ++++++++
 2 files changed, 8 insertions(+), 8 deletions(-)

Index: to-merge/drivers/pci/msi.h
===================================================================
--- to-merge.orig/drivers/pci/msi.h
+++ to-merge/drivers/pci/msi.h
@@ -8,14 +8,6 @@
 
 #include <asm/msi.h>
 
-/*
- * MSI-X Address Register
- */
-#define PCI_MSIX_FLAGS_QSIZE		0x7FF
-#define PCI_MSIX_FLAGS_ENABLE		(1 << 15)
-#define PCI_MSIX_FLAGS_BIRMASK		(7 << 0)
-#define PCI_MSIX_FLAGS_BITMASK		(1 << 0)
-
 #define PCI_MSIX_ENTRY_SIZE			16
 #define  PCI_MSIX_ENTRY_LOWER_ADDR_OFFSET	0
 #define  PCI_MSIX_ENTRY_UPPER_ADDR_OFFSET	4
Index: to-merge/include/linux/pci_regs.h
===================================================================
--- to-merge.orig/include/linux/pci_regs.h
+++ to-merge/include/linux/pci_regs.h
@@ -287,6 +287,14 @@
 #define PCI_MSI_DATA_64		12	/* 16 bits of data for 64-bit devices */
 #define PCI_MSI_MASK_BIT	16	/* Mask bits register */
 
+/* MSI-X registers */
+#define PCI_MSIX_FLAGS		2	/* Various flags */
+#define  PCI_MSIX_FLAGS_ENABLE	0x8000	/* MSI-X feature enabled */
+#define  PCI_MSIX_FLAGS_FNMASK	0x4000	/* Mask entire function */
+#define  PCI_MSIX_FLAGS_QSIZE	0x07FF	/* Number of MSI-X available */
+#define  PCI_MSIX_FLAGS_BIRMASK	0x0007	/* BIR */
+#define  PCI_MSIX_FLAGS_BITMASK	0x0001	/* ??? */
+
 /* CompactPCI Hotswap Register */
 
 #define PCI_CHSWP_CSR		2	/* Control and Status Register */

[RFC/PATCH 3/7] Powerpc MSI ops layer

[Michael Ellerman]

Powerpc MSI ops layer.

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

 arch/powerpc/kernel/msi.c        |  347 +++++++++++++++++++++++++++++++++++++++
 include/asm-powerpc/machdep.h    |    6 
 include/asm-powerpc/msi.h        |  175 +++++++++++++++++++
 include/asm-powerpc/pci-bridge.h |    4 
 4 files changed, 532 insertions(+)

Index: to-merge/arch/powerpc/kernel/msi.c
===================================================================
--- /dev/null
+++ to-merge/arch/powerpc/kernel/msi.c
@@ -0,0 +1,347 @@
+/*
+ * Copyright 2006 (C), 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.
+ */
+
+#undef DEBUG
+
+#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)
+{
+	no_msi = 1;
+}
+
+
+/* msi_info helpers */
+
+static struct pci_dn *get_pdn(struct pci_dev *pdev)
+{
+	struct device_node *dn;
+	struct pci_dn *pdn;
+
+	dn = pci_device_to_OF_node(pdev);
+	if (!dn) {
+		pr_debug("get_pdn: no dn found for %s\n", pci_name(pdev));
+		return NULL;
+	}
+
+	pdn = PCI_DN(dn);
+	if (!pdn) {
+		pr_debug("get_pdn: no pci_dn found for %s\n", pci_name(pdev));
+		return NULL;
+	}
+
+	return pdn;
+}
+
+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;
+	struct pci_dn *pdn;
+
+	entries_size = sizeof(struct msix_entry) * num;
+
+	info = kzalloc(sizeof(struct msi_info) + entries_size, GFP_KERNEL);
+	if (!info) {
+		pr_debug("alloc_msi_info: kzalloc failed for %s\n",
+				pci_name(pdev));
+		return -ENOMEM;
+	}
+
+	info->type = type;
+	info->num = num;
+	memcpy(info->entries, entries, entries_size);
+
+	pdn = get_pdn(pdev);
+	if (!pdn || pdn->msi_info)	/* don't leak info structs */
+		BUG();
+
+	pdn->msi_info = info;
+
+	return 0;
+}
+
+static struct msi_info *get_msi_info(struct pci_dev *pdev)
+{
+	struct pci_dn *pdn;
+
+	pdn = get_pdn(pdev);
+	if (!pdn)
+		return NULL;
+
+	return pdn->msi_info;
+}
+
+static void free_msi_info(struct pci_dev *pdev)
+{
+	struct pci_dn *pdn;
+
+	pdn = get_pdn(pdev);
+	if (!pdn) {
+		pr_debug("free_msi_info: No pdn for %s\n", pci_name(pdev));
+		return;
+	}
+
+	kfree(pdn->msi_info);
+	pdn->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_pdn(pdev)
+			|| get_msi_info(pdev))
+		return -EINVAL;
+
+	ops = get_msi_ops(pdev);
+	if (!ops)
+		return -EINVAL;
+
+	for (i = 0; i < nvec; i++)
+		entries[i].vector = NO_IRQ;
+
+	rc = ops->check(pdev, nvec, entries, type);
+	if (rc) {
+		pr_debug("generic_msi_enable: check failed (%d) for %s\n",
+			rc, pci_name(pdev));
+		return rc;
+	}
+
+	rc = ops->alloc(pdev, nvec, entries, type);
+	if (rc) {
+		pr_debug("generic_msi_enable: alloc failed (%d) for %s\n",
+			rc, pci_name(pdev));
+		return rc;
+	}
+
+	if (ops->enable) {
+		rc = ops->enable(pdev, nvec, entries, type);
+		if (rc) {
+			pr_debug("generic_msi_enable: enable failed (%d) "
+				"for %s\n", rc, pci_name(pdev));
+			goto out_free;
+		}
+	}
+
+	rc = alloc_msi_info(pdev, nvec, entries, type);
+	if (rc)
+		goto out_free;
+
+	return 0;
+
+ out_free:
+	ops->free(pdev, nvec, entries, type);
+
+	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 || !get_pdn(pdev))
+		return -1;
+
+	info = get_msi_info(pdev);
+	if (!info) {
+		pr_debug("generic_msi_disable: No info for %s\n",
+			pci_name(pdev));
+		return -1;
+	}
+
+	ops = get_msi_ops(pdev);
+	if (!ops)
+		return -1;
+
+	if (ops->disable)
+		ops->disable(pdev, info->num, info->entries, type);
+
+	ops->free(pdev, info->num, info->entries, type);
+
+	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)
+		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:
+		/* XXX implement me */
+		BUG();
+		break;
+	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: to-merge/include/asm-powerpc/machdep.h
===================================================================
--- to-merge.orig/include/asm-powerpc/machdep.h
+++ to-merge/include/asm-powerpc/machdep.h
@@ -29,6 +29,9 @@ struct file;
 #ifdef CONFIG_KEXEC
 struct kimage;
 #endif
+#ifdef CONFIG_PCI_MSI
+struct ppc_msi_ops;
+#endif
 
 #ifdef CONFIG_SMP
 struct smp_ops_t {
@@ -106,6 +109,9 @@ struct machdep_calls {
 	/* Called after scanning the bus, before allocating resources */
 	void		(*pcibios_fixup)(void);
 	int		(*pci_probe_mode)(struct pci_bus *);
+#ifdef CONFIG_PCI_MSI
+	struct ppc_msi_ops*	(*get_msi_ops)(struct pci_dev *pdev);
+#endif
 
 	void		(*restart)(char *cmd);
 	void		(*power_off)(void);
Index: to-merge/include/asm-powerpc/msi.h
===================================================================
--- /dev/null
+++ to-merge/include/asm-powerpc/msi.h
@@ -0,0 +1,175 @@
+/*
+ * Copyright (C) 2006 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>
+
+struct msi_msg {
+	u32	address_lo;	/* low 32 bits of msi message address */
+	u32	address_hi;	/* high 32 bits of msi message address */
+	u32	data;		/* 32 bits of msi message data */
+};
+
+/*
+ * 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 *priv;
+};
+
+extern int msi_raw_enable(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type);
+extern int msi_raw_disable(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type);
+
+#endif /* __ASSEMBLY__ */
+#endif /* __KERNEL__ */
+#endif /* _ASM_POWERPC_MSI_H */
Index: to-merge/include/asm-powerpc/pci-bridge.h
===================================================================
--- to-merge.orig/include/asm-powerpc/pci-bridge.h
+++ to-merge/include/asm-powerpc/pci-bridge.h
@@ -9,6 +9,7 @@
 #include <linux/config.h>
 #include <linux/pci.h>
 #include <linux/list.h>
+#include <asm/msi.h>
 
 /*
  * This program is free software; you can redistribute it and/or
@@ -81,6 +82,9 @@ struct pci_dn {
 	struct	pci_dev *pcidev;	/* back-pointer to the pci device */
 	struct	device_node *node;	/* back-pointer to the device_node */
 	u32	config_space[16];	/* saved PCI config space */
+#ifdef CONFIG_PCI_MSI
+	struct	msi_info *msi_info;
+#endif
 };
 
 /* Get the pointer to a device_node's pci_dn */

[RFC/PATCH 4/7] Allow for non-Intel MSI implementations

[Michael Ellerman]

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

 drivers/pci/Makefile |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: to-merge/drivers/pci/Makefile
===================================================================
--- to-merge.orig/drivers/pci/Makefile
+++ to-merge/drivers/pci/Makefile
@@ -27,7 +27,8 @@ obj-$(CONFIG_PPC64) += setup-bus.o
 obj-$(CONFIG_MIPS) += setup-bus.o setup-irq.o
 obj-$(CONFIG_X86_VISWS) += setup-irq.o
 
-msiobj-y := msi.o msi-apic.o
+msiobj-$(CONFIG_X86) := msi.o msi-apic.o
+msiobj-$(CONFIG_IA64) := msi.o msi-apic.o
 msiobj-$(CONFIG_IA64_GENERIC) += msi-altix.o
 msiobj-$(CONFIG_IA64_SGI_SN2) += msi-altix.o
 obj-$(CONFIG_PCI_MSI) += $(msiobj-y)

[RFC/PATCH 5/7] Enable MSI on Powerpc

[Michael Ellerman]

Allow PCI_MSI to build on Powerpc.

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

 arch/powerpc/kernel/Makefile |    4 ++++
 drivers/pci/Kconfig          |    2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

Index: to-merge/arch/powerpc/kernel/Makefile
===================================================================
--- to-merge.orig/arch/powerpc/kernel/Makefile
+++ to-merge/arch/powerpc/kernel/Makefile
@@ -67,6 +67,10 @@ pci64-$(CONFIG_PPC64)		+= pci_64.o pci_d
 				   pci_direct_iommu.o iomap.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: to-merge/drivers/pci/Kconfig
===================================================================
--- to-merge.orig/drivers/pci/Kconfig
+++ to-merge/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
 	help
 	   This allows device drivers to enable MSI (Message Signaled
 	   Interrupts).  Message Signaled Interrupts enable a device to

[RFC/PATCH 6/7] RTAS MSI implementation

[Michael Ellerman]

Powerpc MSI support via RTAS. Based on Jake's code.

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

 arch/powerpc/kernel/Makefile   |    1 
 arch/powerpc/kernel/msi-rtas.c |  246 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 247 insertions(+)

Index: to-merge/arch/powerpc/kernel/Makefile
===================================================================
--- to-merge.orig/arch/powerpc/kernel/Makefile
+++ to-merge/arch/powerpc/kernel/Makefile
@@ -69,6 +69,7 @@ pci32-$(CONFIG_PPC32)		:= pci_32.o
 obj-$(CONFIG_PCI)		+= $(pci64-y) $(pci32-y)
 
 msiobj-y			:= msi.o
+msiobj-$(CONFIG_PPC_PSERIES)	+= msi-rtas.o
 obj-$(CONFIG_PCI_MSI)		+= $(msiobj-y)
 
 kexec-$(CONFIG_PPC64)		:= machine_kexec_64.o
Index: to-merge/arch/powerpc/kernel/msi-rtas.c
===================================================================
--- /dev/null
+++ to-merge/arch/powerpc/kernel/msi-rtas.c
@@ -0,0 +1,246 @@
+/*
+ * Copyright (C) 2006 Jake Moilanen <moilanen@austin.ibm.com>, IBM Corp.
+ * Copyright (C) 2006 Michael Ellerman, IBM Corp.
+ *
+ * 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.
+ *
+ */
+
+#define DEBUG 1
+
+#include <linux/irq.h>
+#include <asm/msi.h>
+#include <asm/rtas.h>
+#include <asm/hw_irq.h>
+#include <asm/ppc-pci.h>
+
+static int query_token, change_token;
+
+#define RTAS_QUERY_MSI_FN	0
+#define RTAS_CHANGE_MSI_FN	1
+#define RTAS_RESET_MSI_FN	2
+
+
+/* RTAS Helpers */
+
+static int rtas_change_msi(struct pci_dn *pdn, u32 function, u32 num_irqs)
+{
+	u32 addr, seq_num, rtas_ret[2];
+	unsigned long buid;
+	int rc;
+
+	addr = rtas_config_addr(pdn->busno, pdn->devfn, 0);
+	buid = pdn->phb->buid;
+
+	seq_num = 1;
+	do {
+		rc = rtas_call(change_token, 6, 3, rtas_ret, addr,
+				BUID_HI(buid), BUID_LO(buid),
+				function, num_irqs, seq_num);
+
+		seq_num = rtas_ret[1];
+	} while (rtas_busy_delay(rc));
+
+	if (rc) {
+		printk(KERN_WARNING "Error[%d]: getting the number of"
+			" MSI interrupts for %s\n", rc, pci_name(pdn->pcidev));
+		return rc;
+	}
+
+	return rtas_ret[0];
+}
+
+static int rtas_query_irq_number(struct pci_dn *pdn, int offset)
+{
+	u32 addr, rtas_ret[2];
+	unsigned long buid;
+	int rc;
+
+	addr = rtas_config_addr(pdn->busno, pdn->devfn, 0);
+	buid = pdn->phb->buid;
+
+	do {
+		rc = rtas_call(query_token, 4, 3, rtas_ret, addr,
+			       BUID_HI(buid), BUID_LO(buid), offset);
+	} while (rtas_busy_delay(rc));
+
+	if (rc) {
+		printk(KERN_WARNING "Error[%d]: Querying irq source number "
+				"for %s\n", rc, pci_name(pdn->pcidev));
+		return rc;
+	}
+
+	return rtas_ret[0];
+}
+
+/*
+ * The spec gives firmware the option to enable either MSI or MSI-X,
+ * this doesn't wash with the Linux API. For the time beinging, we
+ * kludge around that by checking ourselves the right type is enabled.
+ */
+static int check_msi_type(struct pci_dev *pdev, int type)
+{
+	int pos, msi_enabled, msix_enabled;
+	u16 reg;
+
+	pos = pci_find_capability(pdev, PCI_CAP_ID_MSI);
+	if (!pos)
+		return -1;
+
+	pci_read_config_word(pdev, pos + PCI_MSI_FLAGS, &reg);
+
+	msi_enabled = msix_enabled = 0;
+
+	if (reg & PCI_MSI_FLAGS_ENABLE)
+		msi_enabled = 1;
+
+	if (reg & PCI_MSIX_FLAGS_ENABLE)
+		msix_enabled = 1;
+
+	if (type == PCI_CAP_ID_MSI && (msix_enabled || !msi_enabled)) {
+		pr_debug("check_msi_type: Expected MSI but got %s.\n",
+			msix_enabled ? "MSI-X" : "none");
+		return -1;
+	}
+
+	if (type == PCI_CAP_ID_MSIX && (msi_enabled || !msix_enabled)) {
+		pr_debug("check_msi_type: Expected MSI-X but got %s.\n",
+			msi_enabled ? "MSI" : "none");
+		return -1;
+	}
+
+	return 0;
+}
+
+static void msi_rtas_free(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type)
+{
+	struct device_node *dn;
+	struct pci_dn *pdn;
+	int i;
+
+	dn = pci_device_to_OF_node(pdev);
+	if (!dn) {
+		pr_debug("msi_rtas_free: No OF device node for %s\n",
+				pci_name(pdev));
+		return;
+	}
+
+	pdn = PCI_DN(dn);
+	if (!pdn) {
+		pr_debug("msi_rtas_free: No PCI DN for %s\n",
+				pci_name(pdev));
+		return;
+	}
+
+	for (i = 0; i < num; i++) {
+		irq_dispose_mapping(entries[i].vector);
+	}
+
+	/* XXX can we do anything sane if this fails? */
+	rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, 0);
+}
+
+static int msi_rtas_check(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type)
+{
+	struct device_node *dn;
+	int i;
+
+	dn = pci_device_to_OF_node(pdev);
+
+	if (!of_find_property(dn, "ibm,req#msi", NULL)) {
+		pr_debug("msi_rtas_check: No ibm,req#msi for %s\n",
+				pci_name(pdev));
+		return -1;
+	}
+
+	/*
+	 * Firmware gives us no control over which entries are allocated
+	 * for MSI-X, it seems to assume we want 0 - n. For now just insist
+	 * that the entries array entry members are 0 - n.
+	 */
+	for (i = 0; i < num; i++) {
+		if (entries[i].entry != i) {
+			pr_debug("msi_rtas_check: entries[i].entry != i\n");
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
+static int msi_rtas_alloc(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type)
+{
+	struct pci_dn *pdn;
+	int hwirq, virq, i;
+
+	pdn = PCI_DN(pci_device_to_OF_node(pdev));
+
+	/*
+	 * In the case of an error it's not clear whether the device is left
+	 * with MSI enabled or not, I think we should explicitly disable.
+	 */
+	if (rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, num) != num)
+		goto out_free;
+
+	if (check_msi_type(pdev, type))
+		goto out_free;
+
+	for (i = 0; i < num; i++) {
+		hwirq = rtas_query_irq_number(pdn, i);
+		if (hwirq < 0)
+			goto out_free;
+
+		virq = irq_create_mapping(NULL, hwirq);
+
+		if (virq == NO_IRQ) {
+			pr_debug("msi_rtas_alloc: Failed mapping hwirq %d\n",
+				hwirq);
+			goto out_free;
+		}
+
+		entries[i].vector = virq;
+	}
+
+	return 0;
+
+ out_free:
+	msi_rtas_free(pdev, num, entries, type);
+	return -1;
+}
+
+static struct ppc_msi_ops rtas_msi_ops = {
+	.check = msi_rtas_check,
+	.alloc = msi_rtas_alloc,
+	.free  = msi_rtas_free
+};
+
+static struct ppc_msi_ops *rtas_get_msi_ops(struct pci_dev *pdev)
+{
+	return &rtas_msi_ops;
+}
+
+static int msi_rtas_init(void)
+{
+	query_token  = rtas_token("ibm,query-interrupt-source-number");
+	change_token = rtas_token("ibm,change-msi");
+
+	if ((query_token == RTAS_UNKNOWN_SERVICE) ||
+			(change_token == RTAS_UNKNOWN_SERVICE)) {
+		pr_debug("rtas_msi_init: Couldn't find RTAS tokens, no "
+				"MSI support available.\n");
+		return 0;
+	}
+
+	pr_debug("rtas_msi_init: Registering RTAS MSI ops.\n");
+
+	ppc_md.get_msi_ops = rtas_get_msi_ops;
+
+	return 0;
+}
+__initcall(msi_rtas_init);

[RFC/PATCH 7/7] Preliminary MPIC MSI backend

[Michael Ellerman]

A pretty hackish MPIC backend, just enough to flesh out the design.
Based on code from Segher.

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

 arch/powerpc/kernel/Makefile   |    1 
 arch/powerpc/kernel/msi-mpic.c |   90 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+)

Index: to-merge/arch/powerpc/kernel/Makefile
===================================================================
--- to-merge.orig/arch/powerpc/kernel/Makefile
+++ to-merge/arch/powerpc/kernel/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_PCI)		+= $(pci64-y) $(pci32
 
 msiobj-y			:= msi.o
 msiobj-$(CONFIG_PPC_PSERIES)	+= msi-rtas.o
+msiobj-$(CONFIG_PPC_PMAC)	+= msi-mpic.o
 obj-$(CONFIG_PCI_MSI)		+= $(msiobj-y)
 
 kexec-$(CONFIG_PPC64)		:= machine_kexec_64.o
Index: to-merge/arch/powerpc/kernel/msi-mpic.c
===================================================================
--- /dev/null
+++ to-merge/arch/powerpc/kernel/msi-mpic.c
@@ -0,0 +1,90 @@
+/*
+ * Copyright (C) 2006 Segher Boessenkool, IBM Corp.
+ * Copyright (C) 2006 Michael Ellerman, IBM Corp.
+ *
+ * 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.
+ *
+ */
+
+#define DEBUG 1
+
+#include <linux/irq.h>
+#include <asm/msi.h>
+#include <asm/hw_irq.h>
+#include <asm/ppc-pci.h>
+
+static int msi_mpic_check(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type)
+{
+	/* The irq allocator needs more work to support MSI-X/multi-MSI */
+	if (type == PCI_CAP_ID_MSIX || num != 1)
+		return 1;
+
+	return 0;
+}
+
+static void msi_mpic_free(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type)
+{
+	for (i = 0; i < num; i++)
+		irq_dispose_mapping(entries[i].vector);
+}
+
+static int msi_mpic_alloc(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type)
+{
+	irq_hw_number_t hwirq;
+	unsigned int virq;
+
+	/* We need a smarter allocator for MSI-X/multi-MSI */
+	hwirq = irq_map[pdev->irq].hwirq;
+	hwirq += 100;
+
+	virq = irq_create_mapping(NULL, hwirq);
+	if (virq == NO_IRQ) {
+		pr_debug("msi_mpic_alloc: Failed mapping hwirq %d\n", hwirq);
+		return -1;
+	}
+
+	set_irq_type(virq, IRQ_TYPE_EDGE_RISING);
+	entries[i].vector = virq;
+
+	return 0;
+}
+
+static int msi_mpic_setup_msi_msg(struct pci_dev *pdev,
+		struct msix_entry *entry, struct msi_msg *msg, int type)
+{
+	msg->address_lo = 0xfee00000;	/* XXX What is this value? */
+	msg->address_hi = 0;
+	msg->data = pdev->irq | 0x8000;
+
+	return 0;
+}
+
+static struct ppc_msi_ops mpic_msi_ops = {
+	.check = msi_mpic_check,
+	.alloc = msi_mpic_alloc,
+	.free = msi_mpic_free
+	.enable = msi_raw_enable,
+	.disable = msi_raw_disable,
+	.setup_msi_msg = msi_mpic_setup_msi_msg,
+};
+
+static struct ppc_msi_ops *mpic_get_msi_ops(struct pci_dev *pdev)
+{
+	return &mpic_msi_ops;
+}
+
+static int msi_mpic_init(void)
+{
+	/* XXX Do this in mpic_init ? */
+	pr_debug("mpic_msi_init: Registering MPIC MSI ops.\n");
+	ppc_md.get_msi_ops = mpic_get_msi_ops;
+
+	return 0;
+}
+__initcall(msi_mpic_init);

Re: [RFC/PATCH 7/7] Preliminary MPIC MSI backend

[Michael Ellerman]

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

On Fri, 2006-09-29 at 07:53 +1000, Michael Ellerman wrote:
> A pretty hackish MPIC backend, just enough to flesh out the design.
> Based on code from Segher.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

Missing a quilt ref, new one coming RSN.

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: 191 bytes --]

[PATCH] Preliminary MPIC MSI backend

[Michael Ellerman]

A pretty hackish MPIC backend, just enough to flesh out the design.
Based on code from Segher.

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

 arch/powerpc/kernel/Makefile   |    1 
 arch/powerpc/kernel/msi-mpic.c |   92 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)

Index: to-merge/arch/powerpc/kernel/Makefile
===================================================================
--- to-merge.orig/arch/powerpc/kernel/Makefile
+++ to-merge/arch/powerpc/kernel/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_PCI)		+= $(pci64-y) $(pci32
 
 msiobj-y			:= msi.o
 msiobj-$(CONFIG_PPC_PSERIES)	+= msi-rtas.o
+msiobj-$(CONFIG_PPC_PMAC)	+= msi-mpic.o
 obj-$(CONFIG_PCI_MSI)		+= $(msiobj-y)
 
 kexec-$(CONFIG_PPC64)		:= machine_kexec_64.o
Index: to-merge/arch/powerpc/kernel/msi-mpic.c
===================================================================
--- /dev/null
+++ to-merge/arch/powerpc/kernel/msi-mpic.c
@@ -0,0 +1,92 @@
+/*
+ * Copyright (C) 2006 Segher Boessenkool, IBM Corp.
+ * Copyright (C) 2006 Michael Ellerman, IBM Corp.
+ *
+ * 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.
+ *
+ */
+
+#define DEBUG 1
+
+#include <linux/irq.h>
+#include <asm/msi.h>
+#include <asm/hw_irq.h>
+#include <asm/ppc-pci.h>
+
+static int msi_mpic_check(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type)
+{
+	/* The irq allocator needs more work to support MSI-X/multi-MSI */
+	if (type == PCI_CAP_ID_MSIX || num != 1)
+		return 1;
+
+	return 0;
+}
+
+static void msi_mpic_free(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type)
+{
+	int i;
+
+	for (i = 0; i < num; i++)
+		irq_dispose_mapping(entries[i].vector);
+}
+
+static int msi_mpic_alloc(struct pci_dev *pdev, int num,
+			struct msix_entry *entries, int type)
+{
+	irq_hw_number_t hwirq;
+	unsigned int virq;
+
+	/* We need a smarter allocator for MSI-X/multi-MSI */
+	hwirq = irq_map[pdev->irq].hwirq;
+	hwirq += 100;
+
+	virq = irq_create_mapping(NULL, hwirq);
+	if (virq == NO_IRQ) {
+		pr_debug("msi_mpic_alloc: Failed mapping hwirq %lu\n", hwirq);
+		return -1;
+	}
+
+	set_irq_type(virq, IRQ_TYPE_EDGE_RISING);
+	entries[0].vector = virq;
+
+	return 0;
+}
+
+static int msi_mpic_setup_msi_msg(struct pci_dev *pdev,
+		struct msix_entry *entry, struct msi_msg *msg, int type)
+{
+	msg->address_lo = 0xfee00000;	/* XXX What is this value? */
+	msg->address_hi = 0;
+	msg->data = pdev->irq | 0x8000;
+
+	return 0;
+}
+
+static struct ppc_msi_ops mpic_msi_ops = {
+	.check = msi_mpic_check,
+	.alloc = msi_mpic_alloc,
+	.free = msi_mpic_free,
+	.enable = msi_raw_enable,
+	.disable = msi_raw_disable,
+	.setup_msi_msg = msi_mpic_setup_msi_msg,
+};
+
+static struct ppc_msi_ops *mpic_get_msi_ops(struct pci_dev *pdev)
+{
+	return &mpic_msi_ops;
+}
+
+static int msi_mpic_init(void)
+{
+	/* XXX Do this in mpic_init ? */
+	pr_debug("mpic_msi_init: Registering MPIC MSI ops.\n");
+	ppc_md.get_msi_ops = mpic_get_msi_ops;
+
+	return 0;
+}
+__initcall(msi_mpic_init);

Re: [PATCH] Preliminary MPIC MSI backend

[Segher Boessenkool]

> A pretty hackish MPIC backend, just enough to flesh out the design.
> Based on code from Segher.

It's pretty alright, and very hackish ;-)  I'll sign off on it,
but some comments...

Signed-off-by: Segher Boessenkool <segher@kernel.crashing.org>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

> +static int msi_mpic_check(struct pci_dev *pdev, int num,
> +			struct msix_entry *entries, int type)
> +{
> +	/* The irq allocator needs more work to support MSI-X/multi-MSI */
> +	if (type == PCI_CAP_ID_MSIX || num != 1)
> +		return 1;

I never tested any MSI-X, so maybe keep MSI-X disabled completely
for now?

> +static int msi_mpic_alloc(struct pci_dev *pdev, int num,
> +			struct msix_entry *entries, int type)
> +{
> +	irq_hw_number_t hwirq;
> +	unsigned int virq;
> +
> +	/* We need a smarter allocator for MSI-X/multi-MSI */
> +	hwirq = irq_map[pdev->irq].hwirq;
> +	hwirq += 100;

Yep, that's the main problem with this code.  A sanity check to
make sure the number isn't >= 120 would be good, too.

> +	set_irq_type(virq, IRQ_TYPE_EDGE_RISING);

I also had some code to show MSI IRQs as "MSI" instead of "EDGE"
in /proc/interrupts, maybe you want to add a generic version of
that?  Or maybe you have, and I judt didn't see it.

> +static int msi_mpic_setup_msi_msg(struct pci_dev *pdev,
> +		struct msix_entry *entry, struct msi_msg *msg, int type)
> +{
> +	msg->address_lo = 0xfee00000;	/* XXX What is this value? */
> +	msg->address_hi = 0;
> +	msg->data = pdev->irq | 0x8000;

Lose the | 0x8000 part, that was an old experiment to work around
U3/U4 MPIC brokenness (and it didn't work).

> +static int msi_mpic_init(void)
> +{
> +	/* XXX Do this in mpic_init ? */
> +	pr_debug("mpic_msi_init: Registering MPIC MSI ops.\n");
> +	ppc_md.get_msi_ops = mpic_get_msi_ops;

It's best to do this in the platform code I think.


Segher

Re: [RFC/PATCH 6/7] RTAS MSI implementation

[Jake Moilanen]

On Fri, 2006-09-29 at 07:53 +1000, Michael Ellerman wrote:
> Powerpc MSI support via RTAS. Based on Jake's code.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
> 
>  arch/powerpc/kernel/Makefile   |    1 
>  arch/powerpc/kernel/msi-rtas.c |  246 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 247 insertions(+)
> 
> Index: to-merge/arch/powerpc/kernel/Makefile
> ===================================================================
> --- to-merge.orig/arch/powerpc/kernel/Makefile
> +++ to-merge/arch/powerpc/kernel/Makefile
> @@ -69,6 +69,7 @@ pci32-$(CONFIG_PPC32)		:= pci_32.o
>  obj-$(CONFIG_PCI)		+= $(pci64-y) $(pci32-y)
>  
>  msiobj-y			:= msi.o
> +msiobj-$(CONFIG_PPC_PSERIES)	+= msi-rtas.o
>  obj-$(CONFIG_PCI_MSI)		+= $(msiobj-y)
>  
>  kexec-$(CONFIG_PPC64)		:= machine_kexec_64.o
> Index: to-merge/arch/powerpc/kernel/msi-rtas.c
> ===================================================================
> --- /dev/null
> +++ to-merge/arch/powerpc/kernel/msi-rtas.c
> @@ -0,0 +1,246 @@
> +/*
> + * Copyright (C) 2006 Jake Moilanen <moilanen@austin.ibm.com>, IBM Corp.
> + * Copyright (C) 2006 Michael Ellerman, IBM Corp.
> + *
> + * 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.
> + *
> + */
> +
> +#define DEBUG 1
> +
> +#include <linux/irq.h>
> +#include <asm/msi.h>
> +#include <asm/rtas.h>
> +#include <asm/hw_irq.h>
> +#include <asm/ppc-pci.h>
> +
> +static int query_token, change_token;
> +
> +#define RTAS_QUERY_MSI_FN	0
> +#define RTAS_CHANGE_MSI_FN	1
> +#define RTAS_RESET_MSI_FN	2
> +
> +
> +/* RTAS Helpers */
> +
> +static int rtas_change_msi(struct pci_dn *pdn, u32 function, u32 num_irqs)
> +{
> +	u32 addr, seq_num, rtas_ret[2];
> +	unsigned long buid;
> +	int rc;
> +
> +	addr = rtas_config_addr(pdn->busno, pdn->devfn, 0);
> +	buid = pdn->phb->buid;
> +
> +	seq_num = 1;
> +	do {
> +		rc = rtas_call(change_token, 6, 3, rtas_ret, addr,
> +				BUID_HI(buid), BUID_LO(buid),
> +				function, num_irqs, seq_num);

This call is still currently broken in firmware.  Hopefully we'll have a
resolution soon.

> +
> +		seq_num = rtas_ret[1];
> +	} while (rtas_busy_delay(rc));
> +
> +	if (rc) {
> +		printk(KERN_WARNING "Error[%d]: getting the number of"
> +			" MSI interrupts for %s\n", rc, pci_name(pdn->pcidev));
> +		return rc;
> +	}
> +
> +	return rtas_ret[0];
> +}
> +
> +static int rtas_query_irq_number(struct pci_dn *pdn, int offset)
> +{
> +	u32 addr, rtas_ret[2];
> +	unsigned long buid;
> +	int rc;
> +
> +	addr = rtas_config_addr(pdn->busno, pdn->devfn, 0);
> +	buid = pdn->phb->buid;
> +
> +	do {
> +		rc = rtas_call(query_token, 4, 3, rtas_ret, addr,
> +			       BUID_HI(buid), BUID_LO(buid), offset);
> +	} while (rtas_busy_delay(rc));
> +
> +	if (rc) {
> +		printk(KERN_WARNING "Error[%d]: Querying irq source number "
> +				"for %s\n", rc, pci_name(pdn->pcidev));
> +		return rc;
> +	}
> +
> +	return rtas_ret[0];
> +}
> +
> +/*
> + * The spec gives firmware the option to enable either MSI or MSI-X,
> + * this doesn't wash with the Linux API. For the time beinging, we
> + * kludge around that by checking ourselves the right type is enabled.
> + */
> +static int check_msi_type(struct pci_dev *pdev, int type)
> +{
> +	int pos, msi_enabled, msix_enabled;
> +	u16 reg;
> +
> +	pos = pci_find_capability(pdev, PCI_CAP_ID_MSI);
> +	if (!pos)
> +		return -1;
> +
> +	pci_read_config_word(pdev, pos + PCI_MSI_FLAGS, &reg);
> +
> +	msi_enabled = msix_enabled = 0;
> +
> +	if (reg & PCI_MSI_FLAGS_ENABLE)
> +		msi_enabled = 1;
> +

This is not being set correctly by firmware either. I have them looking
into the problem.

> +	if (reg & PCI_MSIX_FLAGS_ENABLE)
> +		msix_enabled = 1;
> +
> +	if (type == PCI_CAP_ID_MSI && (msix_enabled || !msi_enabled)) {
> +		pr_debug("check_msi_type: Expected MSI but got %s.\n",
> +			msix_enabled ? "MSI-X" : "none");
> +		return -1;
> +	}
> +
> +	if (type == PCI_CAP_ID_MSIX && (msi_enabled || !msix_enabled)) {
> +		pr_debug("check_msi_type: Expected MSI-X but got %s.\n",
> +			msi_enabled ? "MSI" : "none");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static void msi_rtas_free(struct pci_dev *pdev, int num,
> +			struct msix_entry *entries, int type)
> +{
> +	struct device_node *dn;
> +	struct pci_dn *pdn;
> +	int i;
> +
> +	dn = pci_device_to_OF_node(pdev);
> +	if (!dn) {
> +		pr_debug("msi_rtas_free: No OF device node for %s\n",
> +				pci_name(pdev));
> +		return;
> +	}
> +
> +	pdn = PCI_DN(dn);
> +	if (!pdn) {
> +		pr_debug("msi_rtas_free: No PCI DN for %s\n",
> +				pci_name(pdev));
> +		return;
> +	}
> +
> +	for (i = 0; i < num; i++) {
> +		irq_dispose_mapping(entries[i].vector);
> +	}
> +
> +	/* XXX can we do anything sane if this fails? */
> +	rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, 0);
> +}
> +
> +static int msi_rtas_check(struct pci_dev *pdev, int num,
> +			struct msix_entry *entries, int type)
> +{
> +	struct device_node *dn;
> +	int i;
> +
> +	dn = pci_device_to_OF_node(pdev);
> +
> +	if (!of_find_property(dn, "ibm,req#msi", NULL)) {
> +		pr_debug("msi_rtas_check: No ibm,req#msi for %s\n",
> +				pci_name(pdev));
> +		return -1;
> +	}
> +
> +	/*
> +	 * Firmware gives us no control over which entries are allocated
> +	 * for MSI-X, it seems to assume we want 0 - n. For now just insist
> +	 * that the entries array entry members are 0 - n.
> +	 */
> +	for (i = 0; i < num; i++) {
> +		if (entries[i].entry != i) {
> +			pr_debug("msi_rtas_check: entries[i].entry != i\n");
> +			return -1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int msi_rtas_alloc(struct pci_dev *pdev, int num,
> +			struct msix_entry *entries, int type)
> +{
> +	struct pci_dn *pdn;
> +	int hwirq, virq, i;
> +
> +	pdn = PCI_DN(pci_device_to_OF_node(pdev));
> +
> +	/*
> +	 * In the case of an error it's not clear whether the device is left
> +	 * with MSI enabled or not, I think we should explicitly disable.
> +	 */
> +	if (rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, num) != num)
> +		goto out_free;
> +
> +	if (check_msi_type(pdev, type))
> +		goto out_free;
> +
> +	for (i = 0; i < num; i++) {
> +		hwirq = rtas_query_irq_number(pdn, i);
> +		if (hwirq < 0)
> +			goto out_free;
> +
> +		virq = irq_create_mapping(NULL, hwirq);
> +
> +		if (virq == NO_IRQ) {
> +			pr_debug("msi_rtas_alloc: Failed mapping hwirq %d\n",
> +				hwirq);
> +			goto out_free;
> +		}
> +
> +		entries[i].vector = virq;
> +	}
> +
> +	return 0;
> +
> + out_free:
> +	msi_rtas_free(pdev, num, entries, type);

Shouldn't this be:

	msi_rtas_free(pdev, i,.......

Otherwise you'll try freeing unallocated entries.


> +	return -1;
> +}
> +
> +static struct ppc_msi_ops rtas_msi_ops = {
> +	.check = msi_rtas_check,
> +	.alloc = msi_rtas_alloc,
> +	.free  = msi_rtas_free
> +};
> +
> +static struct ppc_msi_ops *rtas_get_msi_ops(struct pci_dev *pdev)
> +{
> +	return &rtas_msi_ops;
> +}
> +
> +static int msi_rtas_init(void)
> +{
> +	query_token  = rtas_token("ibm,query-interrupt-source-number");
> +	change_token = rtas_token("ibm,change-msi");
> +
> +	if ((query_token == RTAS_UNKNOWN_SERVICE) ||
> +			(change_token == RTAS_UNKNOWN_SERVICE)) {
> +		pr_debug("rtas_msi_init: Couldn't find RTAS tokens, no "
> +				"MSI support available.\n");
> +		return 0;
> +	}
> +
> +	pr_debug("rtas_msi_init: Registering RTAS MSI ops.\n");
> +
> +	ppc_md.get_msi_ops = rtas_get_msi_ops;
> +
> +	return 0;
> +}
> +__initcall(msi_rtas_init);

Re: [RFC/PATCH 3/7] Powerpc MSI ops layer

[Jake Moilanen]

On Fri, 2006-09-29 at 07:53 +1000, Michael Ellerman wrote:
> Powerpc MSI ops layer.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
> 
>  arch/powerpc/kernel/msi.c        |  347 +++++++++++++++++++++++++++++++++++++++
>  include/asm-powerpc/machdep.h    |    6 
>  include/asm-powerpc/msi.h        |  175 +++++++++++++++++++
>  include/asm-powerpc/pci-bridge.h |    4 
>  4 files changed, 532 insertions(+)
> 
> Index: to-merge/arch/powerpc/kernel/msi.c
> ===================================================================
> --- /dev/null
> +++ to-merge/arch/powerpc/kernel/msi.c
> @@ -0,0 +1,347 @@
> +/*
> + * Copyright 2006 (C), 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.
> + */
> +
> +#undef DEBUG
> +
> +#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)
> +{
> +	no_msi = 1;
> +}
> +
> +
> +/* msi_info helpers */
> +
> +static struct pci_dn *get_pdn(struct pci_dev *pdev)
> +{
> +	struct device_node *dn;
> +	struct pci_dn *pdn;
> +
> +	dn = pci_device_to_OF_node(pdev);
> +	if (!dn) {
> +		pr_debug("get_pdn: no dn found for %s\n", pci_name(pdev));
> +		return NULL;
> +	}
> +
> +	pdn = PCI_DN(dn);
> +	if (!pdn) {
> +		pr_debug("get_pdn: no pci_dn found for %s\n", pci_name(pdev));
> +		return NULL;
> +	}
> +
> +	return pdn;
> +}
> +
> +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;
> +	struct pci_dn *pdn;
> +
> +	entries_size = sizeof(struct msix_entry) * num;
> +
> +	info = kzalloc(sizeof(struct msi_info) + entries_size, GFP_KERNEL);

Shouldn't you do a second kzalloc for info->entries, and not just add on
the size to the end?

> +	if (!info) {
> +		pr_debug("alloc_msi_info: kzalloc failed for %s\n",
> +				pci_name(pdev));
> +		return -ENOMEM;
> +	}
> +
> +	info->type = type;
> +	info->num = num;
> +	memcpy(info->entries, entries, entries_size);
> +
> +	pdn = get_pdn(pdev);
> +	if (!pdn || pdn->msi_info)	/* don't leak info structs */
> +		BUG();
> +
> +	pdn->msi_info = info;
> +
> +	return 0;
> +}
> +

Re: [PATCH] Preliminary MPIC MSI backend

[Michael Ellerman]

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

On Sat, 2006-09-30 at 10:43 +0200, Segher Boessenkool wrote:
> > A pretty hackish MPIC backend, just enough to flesh out the design.
> > Based on code from Segher.
> 
> It's pretty alright, and very hackish ;-)  I'll sign off on it,
> but some comments...
> 
> Signed-off-by: Segher Boessenkool <segher@kernel.crashing.org>
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> 
> > +static int msi_mpic_check(struct pci_dev *pdev, int num,
> > +			struct msix_entry *entries, int type)
> > +{
> > +	/* The irq allocator needs more work to support MSI-X/multi-MSI */
> > +	if (type == PCI_CAP_ID_MSIX || num != 1)
> > +		return 1;
> 
> I never tested any MSI-X, so maybe keep MSI-X disabled completely
> for now?

Yeah it is, that's an ||.

> > +static int msi_mpic_alloc(struct pci_dev *pdev, int num,
> > +			struct msix_entry *entries, int type)
> > +{
> > +	irq_hw_number_t hwirq;
> > +	unsigned int virq;
> > +
> > +	/* We need a smarter allocator for MSI-X/multi-MSI */
> > +	hwirq = irq_map[pdev->irq].hwirq;
> > +	hwirq += 100;
> 
> Yep, that's the main problem with this code.  A sanity check to
> make sure the number isn't >= 120 would be good, too.

Talking to Ben, we decided for the moment we'll just reuse the currently
assigned irq, in the medium term we'll come up with some way to find the
unassigned irqs on mpic and write an allocator.

> > +	set_irq_type(virq, IRQ_TYPE_EDGE_RISING);
> 
> I also had some code to show MSI IRQs as "MSI" instead of "EDGE"
> in /proc/interrupts, maybe you want to add a generic version of
> that?  Or maybe you have, and I judt didn't see it.

I lost that along the way somewhere, I'll try and find it and resurrect
it.

> > +static int msi_mpic_setup_msi_msg(struct pci_dev *pdev,
> > +		struct msix_entry *entry, struct msi_msg *msg, int type)
> > +{
> > +	msg->address_lo = 0xfee00000;	/* XXX What is this value? */
> > +	msg->address_hi = 0;
> > +	msg->data = pdev->irq | 0x8000;
> 
> Lose the | 0x8000 part, that was an old experiment to work around
> U3/U4 MPIC brokenness (and it didn't work).

OK.

> > +static int msi_mpic_init(void)
> > +{
> > +	/* XXX Do this in mpic_init ? */
> > +	pr_debug("mpic_msi_init: Registering MPIC MSI ops.\n");
> > +	ppc_md.get_msi_ops = mpic_get_msi_ops;
> 
> It's best to do this in the platform code I think.

Yeah probably, I'll leave that up to Ben.

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: 191 bytes --]

Re: [RFC/PATCH 3/7] Powerpc MSI ops layer

[Michael Ellerman]

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

On Tue, 2006-10-03 at 16:54 -0500, Jake Moilanen wrote:
> On Fri, 2006-09-29 at 07:53 +1000, Michael Ellerman wrote:
> > Powerpc MSI ops layer.
> > 
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> > ---
> > 
> >  arch/powerpc/kernel/msi.c        |  347 +++++++++++++++++++++++++++++++++++++++
> >  include/asm-powerpc/machdep.h    |    6 
> >  include/asm-powerpc/msi.h        |  175 +++++++++++++++++++
> >  include/asm-powerpc/pci-bridge.h |    4 
> >  4 files changed, 532 insertions(+)
> > 
> > Index: to-merge/arch/powerpc/kernel/msi.c
> > ===================================================================
> > --- /dev/null
> > +++ to-merge/arch/powerpc/kernel/msi.c
> > @@ -0,0 +1,347 @@
> > +/*
> > + * Copyright 2006 (C), 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.
> > + */
> > +
> > +#undef DEBUG
> > +
> > +#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)
> > +{
> > +	no_msi = 1;
> > +}
> > +
> > +
> > +/* msi_info helpers */
> > +
> > +static struct pci_dn *get_pdn(struct pci_dev *pdev)
> > +{
> > +	struct device_node *dn;
> > +	struct pci_dn *pdn;
> > +
> > +	dn = pci_device_to_OF_node(pdev);
> > +	if (!dn) {
> > +		pr_debug("get_pdn: no dn found for %s\n", pci_name(pdev));
> > +		return NULL;
> > +	}
> > +
> > +	pdn = PCI_DN(dn);
> > +	if (!pdn) {
> > +		pr_debug("get_pdn: no pci_dn found for %s\n", pci_name(pdev));
> > +		return NULL;
> > +	}
> > +
> > +	return pdn;
> > +}
> > +
> > +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;
> > +	struct pci_dn *pdn;
> > +
> > +	entries_size = sizeof(struct msix_entry) * num;
> > +
> > +	info = kzalloc(sizeof(struct msi_info) + entries_size, GFP_KERNEL);
> 
> Shouldn't you do a second kzalloc for info->entries, and not just add on
> the size to the end?

We could, but I don't see why it's better. It's a little sneaky to tack
the entries on the end but I don't see a problem with it?

There is a bug in there that I don't set the entries pointer before
doing the memcpy, but I've fixed that - or is that what you meant ? :)

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: 191 bytes --]

Re: [RFC/PATCH 6/7] RTAS MSI implementation

[Michael Ellerman]

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

On Tue, 2006-10-03 at 16:53 -0500, Jake Moilanen wrote:
> On Fri, 2006-09-29 at 07:53 +1000, Michael Ellerman wrote:
> > Powerpc MSI support via RTAS. Based on Jake's code.
> > 
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> > ---
> > 
> >  arch/powerpc/kernel/Makefile   |    1 
> >  arch/powerpc/kernel/msi-rtas.c |  246 +++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 247 insertions(+)
> > 
> > Index: to-merge/arch/powerpc/kernel/Makefile
> > ===================================================================
> > --- to-merge.orig/arch/powerpc/kernel/Makefile
> > +++ to-merge/arch/powerpc/kernel/Makefile
> > @@ -69,6 +69,7 @@ pci32-$(CONFIG_PPC32)		:= pci_32.o
> >  obj-$(CONFIG_PCI)		+= $(pci64-y) $(pci32-y)
> >  
> >  msiobj-y			:= msi.o
> > +msiobj-$(CONFIG_PPC_PSERIES)	+= msi-rtas.o
> >  obj-$(CONFIG_PCI_MSI)		+= $(msiobj-y)
> >  
> >  kexec-$(CONFIG_PPC64)		:= machine_kexec_64.o
> > Index: to-merge/arch/powerpc/kernel/msi-rtas.c
> > ===================================================================
> > --- /dev/null
> > +++ to-merge/arch/powerpc/kernel/msi-rtas.c
> > @@ -0,0 +1,246 @@
> > +/*
> > + * Copyright (C) 2006 Jake Moilanen <moilanen@austin.ibm.com>, IBM Corp.
> > + * Copyright (C) 2006 Michael Ellerman, IBM Corp.
> > + *
> > + * 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.
> > + *
> > + */
> > +
> > +#define DEBUG 1
> > +
> > +#include <linux/irq.h>
> > +#include <asm/msi.h>
> > +#include <asm/rtas.h>
> > +#include <asm/hw_irq.h>
> > +#include <asm/ppc-pci.h>
> > +
> > +static int query_token, change_token;
> > +
> > +#define RTAS_QUERY_MSI_FN	0
> > +#define RTAS_CHANGE_MSI_FN	1
> > +#define RTAS_RESET_MSI_FN	2
> > +
> > +
> > +/* RTAS Helpers */
> > +
> > +static int rtas_change_msi(struct pci_dn *pdn, u32 function, u32 num_irqs)
> > +{
> > +	u32 addr, seq_num, rtas_ret[2];
> > +	unsigned long buid;
> > +	int rc;
> > +
> > +	addr = rtas_config_addr(pdn->busno, pdn->devfn, 0);
> > +	buid = pdn->phb->buid;
> > +
> > +	seq_num = 1;
> > +	do {
> > +		rc = rtas_call(change_token, 6, 3, rtas_ret, addr,
> > +				BUID_HI(buid), BUID_LO(buid),
> > +				function, num_irqs, seq_num);
> 
> This call is still currently broken in firmware.  Hopefully we'll have a
> resolution soon.

Any word on that?

> > +
> > +		seq_num = rtas_ret[1];
> > +	} while (rtas_busy_delay(rc));
> > +
> > +	if (rc) {
> > +		printk(KERN_WARNING "Error[%d]: getting the number of"
> > +			" MSI interrupts for %s\n", rc, pci_name(pdn->pcidev));
> > +		return rc;
> > +	}
> > +
> > +	return rtas_ret[0];
> > +}
> > +
> > +static int rtas_query_irq_number(struct pci_dn *pdn, int offset)
> > +{
> > +	u32 addr, rtas_ret[2];
> > +	unsigned long buid;
> > +	int rc;
> > +
> > +	addr = rtas_config_addr(pdn->busno, pdn->devfn, 0);
> > +	buid = pdn->phb->buid;
> > +
> > +	do {
> > +		rc = rtas_call(query_token, 4, 3, rtas_ret, addr,
> > +			       BUID_HI(buid), BUID_LO(buid), offset);
> > +	} while (rtas_busy_delay(rc));
> > +
> > +	if (rc) {
> > +		printk(KERN_WARNING "Error[%d]: Querying irq source number "
> > +				"for %s\n", rc, pci_name(pdn->pcidev));
> > +		return rc;
> > +	}
> > +
> > +	return rtas_ret[0];
> > +}
> > +
> > +/*
> > + * The spec gives firmware the option to enable either MSI or MSI-X,
> > + * this doesn't wash with the Linux API. For the time beinging, we
> > + * kludge around that by checking ourselves the right type is enabled.
> > + */
> > +static int check_msi_type(struct pci_dev *pdev, int type)
> > +{
> > +	int pos, msi_enabled, msix_enabled;
> > +	u16 reg;
> > +
> > +	pos = pci_find_capability(pdev, PCI_CAP_ID_MSI);
> > +	if (!pos)
> > +		return -1;
> > +
> > +	pci_read_config_word(pdev, pos + PCI_MSI_FLAGS, &reg);
> > +
> > +	msi_enabled = msix_enabled = 0;
> > +
> > +	if (reg & PCI_MSI_FLAGS_ENABLE)
> > +		msi_enabled = 1;
> > +
> 
> This is not being set correctly by firmware either. I have them looking
> into the problem.

Are you sure? I don't see how MSI could possibly work if they don't
enable it on the device itself. Or is it completely broken at the
moment?

> 
> > +	if (reg & PCI_MSIX_FLAGS_ENABLE)
> > +		msix_enabled = 1;
> > +
> > +	if (type == PCI_CAP_ID_MSI && (msix_enabled || !msi_enabled)) {
> > +		pr_debug("check_msi_type: Expected MSI but got %s.\n",
> > +			msix_enabled ? "MSI-X" : "none");
> > +		return -1;
> > +	}
> > +
> > +	if (type == PCI_CAP_ID_MSIX && (msi_enabled || !msix_enabled)) {
> > +		pr_debug("check_msi_type: Expected MSI-X but got %s.\n",
> > +			msi_enabled ? "MSI" : "none");
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void msi_rtas_free(struct pci_dev *pdev, int num,
> > +			struct msix_entry *entries, int type)
> > +{
> > +	struct device_node *dn;
> > +	struct pci_dn *pdn;
> > +	int i;
> > +
> > +	dn = pci_device_to_OF_node(pdev);
> > +	if (!dn) {
> > +		pr_debug("msi_rtas_free: No OF device node for %s\n",
> > +				pci_name(pdev));
> > +		return;
> > +	}
> > +
> > +	pdn = PCI_DN(dn);
> > +	if (!pdn) {
> > +		pr_debug("msi_rtas_free: No PCI DN for %s\n",
> > +				pci_name(pdev));
> > +		return;
> > +	}
> > +
> > +	for (i = 0; i < num; i++) {
> > +		irq_dispose_mapping(entries[i].vector);
> > +	}
> > +
> > +	/* XXX can we do anything sane if this fails? */
> > +	rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, 0);
> > +}
> > +
> > +static int msi_rtas_check(struct pci_dev *pdev, int num,
> > +			struct msix_entry *entries, int type)
> > +{
> > +	struct device_node *dn;
> > +	int i;
> > +
> > +	dn = pci_device_to_OF_node(pdev);
> > +
> > +	if (!of_find_property(dn, "ibm,req#msi", NULL)) {
> > +		pr_debug("msi_rtas_check: No ibm,req#msi for %s\n",
> > +				pci_name(pdev));
> > +		return -1;
> > +	}
> > +
> > +	/*
> > +	 * Firmware gives us no control over which entries are allocated
> > +	 * for MSI-X, it seems to assume we want 0 - n. For now just insist
> > +	 * that the entries array entry members are 0 - n.
> > +	 */
> > +	for (i = 0; i < num; i++) {
> > +		if (entries[i].entry != i) {
> > +			pr_debug("msi_rtas_check: entries[i].entry != i\n");
> > +			return -1;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int msi_rtas_alloc(struct pci_dev *pdev, int num,
> > +			struct msix_entry *entries, int type)
> > +{
> > +	struct pci_dn *pdn;
> > +	int hwirq, virq, i;
> > +
> > +	pdn = PCI_DN(pci_device_to_OF_node(pdev));
> > +
> > +	/*
> > +	 * In the case of an error it's not clear whether the device is left
> > +	 * with MSI enabled or not, I think we should explicitly disable.
> > +	 */
> > +	if (rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, num) != num)
> > +		goto out_free;
> > +
> > +	if (check_msi_type(pdev, type))
> > +		goto out_free;
> > +
> > +	for (i = 0; i < num; i++) {
> > +		hwirq = rtas_query_irq_number(pdn, i);
> > +		if (hwirq < 0)
> > +			goto out_free;
> > +
> > +		virq = irq_create_mapping(NULL, hwirq);
> > +
> > +		if (virq == NO_IRQ) {
> > +			pr_debug("msi_rtas_alloc: Failed mapping hwirq %d\n",
> > +				hwirq);
> > +			goto out_free;
> > +		}
> > +
> > +		entries[i].vector = virq;
> > +	}
> > +
> > +	return 0;
> > +
> > + out_free:
> > +	msi_rtas_free(pdev, num, entries, type);
> 
> Shouldn't this be:
> 
> 	msi_rtas_free(pdev, i,.......
> 
> Otherwise you'll try freeing unallocated entries.

Yeah maybe. I think I was intending that free be written in such a way
that it could cope with tearing down a partial setup, but I never got
round to writing the patch for irq_dispose_mapping() to make that work -
I should do that.

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: 191 bytes --]

Re: [RFC/PATCH 3/7] Powerpc MSI ops layer

[Jake Moilanen]

On Tue, 2006-10-24 at 11:51 +1000, Michael Ellerman wrote:
> On Tue, 2006-10-03 at 16:54 -0500, Jake Moilanen wrote:
> > On Fri, 2006-09-29 at 07:53 +1000, Michael Ellerman wrote:
> > > Powerpc MSI ops layer.
> > > 
> > > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> > > ---
> > > 
> > >  arch/powerpc/kernel/msi.c        |  347 +++++++++++++++++++++++++++++++++++++++
> > >  include/asm-powerpc/machdep.h    |    6 
> > >  include/asm-powerpc/msi.h        |  175 +++++++++++++++++++
> > >  include/asm-powerpc/pci-bridge.h |    4 
> > >  4 files changed, 532 insertions(+)
> > > 
> > > Index: to-merge/arch/powerpc/kernel/msi.c
> > > ===================================================================
> > > --- /dev/null
> > > +++ to-merge/arch/powerpc/kernel/msi.c
> > > @@ -0,0 +1,347 @@
> > > +/*
> > > + * Copyright 2006 (C), 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.
> > > + */
> > > +
> > > +#undef DEBUG
> > > +
> > > +#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)
> > > +{
> > > +	no_msi = 1;
> > > +}
> > > +
> > > +
> > > +/* msi_info helpers */
> > > +
> > > +static struct pci_dn *get_pdn(struct pci_dev *pdev)
> > > +{
> > > +	struct device_node *dn;
> > > +	struct pci_dn *pdn;
> > > +
> > > +	dn = pci_device_to_OF_node(pdev);
> > > +	if (!dn) {
> > > +		pr_debug("get_pdn: no dn found for %s\n", pci_name(pdev));
> > > +		return NULL;
> > > +	}
> > > +
> > > +	pdn = PCI_DN(dn);
> > > +	if (!pdn) {
> > > +		pr_debug("get_pdn: no pci_dn found for %s\n", pci_name(pdev));
> > > +		return NULL;
> > > +	}
> > > +
> > > +	return pdn;
> > > +}
> > > +
> > > +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;
> > > +	struct pci_dn *pdn;
> > > +
> > > +	entries_size = sizeof(struct msix_entry) * num;
> > > +
> > > +	info = kzalloc(sizeof(struct msi_info) + entries_size, GFP_KERNEL);
> > 
> > Shouldn't you do a second kzalloc for info->entries, and not just add on
> > the size to the end?
> 
> We could, but I don't see why it's better. It's a little sneaky to tack
> the entries on the end but I don't see a problem with it?
> 
> There is a bug in there that I don't set the entries pointer before
> doing the memcpy, but I've fixed that - or is that what you meant ? :)

Yeah...I was just pointing out there was a bug.  :)