[PATCH v3 0/4] MSI, of, irqchip/gicv3-its: Handle "msi-map" properties.

[PATCH v3 0/4] MSI, of, irqchip/gicv3-its: Handle "msi-map" properties.

[David Daney]

From: David Daney <david.daney@cavium.com>

The first patch from Mark Rutland adds the OF device tree binding
description, which explains what we are attempting to do here.  For
MSI messages on GICv3 systems there is some side-band data that
accompanies the message, this data is specified in the OF device tree
"msi-map" property of the PCI host driver.

The second patch adds a parser to get the required information out of
the device tree.

The third patch creates the pci_msi_domain_get_msi_rid() function to
make it more convenient to generate the proper PCI/MSI requester id.

The final patch converts gicv3-its to use the new infrastructure.

Changes from v1: Factor out the device tree access code to a separate
function in drivers/of/irq.c

Changes from v2: Added the pci_msi_domain_get_msi_rid() patch to
generalize the support a bit more.

David Daney (3):
  of/irq: Add new function of_msi_map_rid()
  PCI/MSI:  Add helper function pci_msi_domain_get_msi_rid().
  irqchip/gic-v3-its:  Add handling of PCI requester id.

Mark Rutland (1):
  Docs: dt: Add PCI MSI map bindings

 Documentation/devicetree/bindings/pci/pci-msi.txt | 220 ++++++++++++++++++++++
 drivers/irqchip/irq-gic-v3-its-pci-msi.c          |   4 +-
 drivers/of/irq.c                                  |  84 +++++++++
 drivers/pci/msi.c                                 |  31 +++
 include/linux/msi.h                               |   1 +
 include/linux/of_irq.h                            |   7 +
 6 files changed, 344 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/pci-msi.txt

-- 
1.9.1

[PATCH v3 1/4] Docs: dt: Add PCI MSI map bindings

[David Daney]

From: Mark Rutland <mark.rutland@arm.com>

Currently msi-parent is used by a few bindings to describe the
relationship between a PCI root complex and a single MSI controller, but
this property does not have a generic binding document.

Additionally, msi-parent is insufficient to describe more complex
relationships between MSI controllers and devices under a root complex,
where devices may be able to target multiple MSI controllers, or where
MSI controllers use (non-probeable) sideband information to distinguish
devices.

This patch adds a generic binding for mapping PCI devices to MSI
controllers. This document covers msi-parent, and a new msi-map property
(specific to PCI*) which may be used to map devices (identified by their
Requester ID) to sideband data for each MSI controller that they may
target.

Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: David Daney <david.daney@cavium.com>
---
 Documentation/devicetree/bindings/pci/pci-msi.txt | 220 ++++++++++++++++++++++
 1 file changed, 220 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/pci-msi.txt

diff --git a/Documentation/devicetree/bindings/pci/pci-msi.txt b/Documentation/devicetree/bindings/pci/pci-msi.txt
new file mode 100644
index 0000000..9b3cc81
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/pci-msi.txt
@@ -0,0 +1,220 @@
+This document describes the generic device tree binding for describing the
+relationship between PCI devices and MSI controllers.
+
+Each PCI device under a root complex is uniquely identified by its Requester ID
+(AKA RID). A Requester ID is a triplet of a Bus number, Device number, and
+Function number.
+
+For the purpose of this document, when treated as a numeric value, a RID is
+formatted such that:
+
+* Bits [15:8] are the Bus number.
+* Bits [7:3] are the Device number.
+* Bits [2:0] are the Function number.
+* Any other bits required for padding must be zero.
+
+MSIs may be distinguished in part through the use of sideband data accompanying
+writes. In the case of PCI devices, this sideband data may be derived from the
+Requester ID. A mechanism is required to associate a device with both the MSI
+controllers it can address, and the sideband data that will be associated with
+its writes to those controllers.
+
+For generic MSI bindings, see
+Documentation/devicetree/bindings/interrupt-controller/msi.txt.
+
+
+PCI root complex
+================
+
+Optional properties
+-------------------
+
+- msi-map: Maps a Requester ID to an MSI controller and associated
+  msi-specifier data. The property is an arbitrary number of tuples of
+  (rid-base,msi-controller,msi-base,length), where:
+
+  * rid-base is a single cell describing the first RID matched by the entry.
+
+  * msi-controller is a single phandle to an MSI controller
+
+  * msi-base is an msi-specifier describing the msi-specifier produced for the
+    first RID matched by the entry.
+
+  * length is a single cell describing how many consecutive RIDs are matched
+    following the rid-base.
+
+  Any RID r in the interval [rid-base, rid-base + length) is associated with
+  the listed msi-controller, with the msi-specifier (r - rid-base + msi-base).
+
+- msi-map-mask: A mask to be applied to each Requester ID prior to being mapped
+  to an msi-specifier per the msi-map property.
+
+- msi-parent: Describes the MSI parent of the root complex itself. Where
+  the root complex and MSI controller do not pass sideband data with MSI
+  writes, this property may be used to describe the MSI controller(s)
+  used by PCI devices under the root complex, if defined as such in the
+  binding for the root complex.
+
+
+Example (1)
+===========
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	msi: msi-controller@a {
+		reg = <0xa 0x1>;
+		compatible = "vendor,some-controller";
+		msi-controller;
+		#msi-cells = <1>;
+	};
+
+	pci: pci@f {
+		reg = <0xf 0x1>;
+		compatible = "vendor,pcie-root-complex";
+		device_type = "pci";
+
+		/*
+		 * The sideband data provided to the MSI controller is
+		 * the RID, identity-mapped.
+		 */
+		msi-map = <0x0 &msi_a 0x0 0x10000>,
+	};
+};
+
+
+Example (2)
+===========
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	msi: msi-controller@a {
+		reg = <0xa 0x1>;
+		compatible = "vendor,some-controller";
+		msi-controller;
+		#msi-cells = <1>;
+	};
+
+	pci: pci@f {
+		reg = <0xf 0x1>;
+		compatible = "vendor,pcie-root-complex";
+		device_type = "pci";
+
+		/*
+		 * The sideband data provided to the MSI controller is
+		 * the RID, masked to only the device and function bits.
+		 */
+		msi-map = <0x0 &msi_a 0x0 0x100>,
+		msi-map-mask = <0xff>
+	};
+};
+
+
+Example (3)
+===========
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	msi: msi-controller@a {
+		reg = <0xa 0x1>;
+		compatible = "vendor,some-controller";
+		msi-controller;
+		#msi-cells = <1>;
+	};
+
+	pci: pci@f {
+		reg = <0xf 0x1>;
+		compatible = "vendor,pcie-root-complex";
+		device_type = "pci";
+
+		/*
+		 * The sideband data provided to the MSI controller is
+		 * the RID, but the high bit of the bus number is
+		 * ignored.
+		 */
+		msi-map = <0x0000 &msi 0x0000 0x8000>,
+			  <0x8000 &msi 0x0000 0x8000>;
+	};
+};
+
+
+Example (4)
+===========
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	msi: msi-controller@a {
+		reg = <0xa 0x1>;
+		compatible = "vendor,some-controller";
+		msi-controller;
+		#msi-cells = <1>;
+	};
+
+	pci: pci@f {
+		reg = <0xf 0x1>;
+		compatible = "vendor,pcie-root-complex";
+		device_type = "pci";
+
+		/*
+		 * The sideband data provided to the MSI controller is
+		 * the RID, but the high bit of the bus number is
+		 * negated.
+		 */
+		msi-map = <0x0000 &msi 0x8000 0x8000>,
+			  <0x8000 &msi 0x0000 0x8000>;
+	};
+};
+
+
+Example (5)
+===========
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	msi_a: msi-controller@a {
+		reg = <0xa 0x1>;
+		compatible = "vendor,some-controller";
+		msi-controller;
+		#msi-cells = <1>;
+	};
+
+	msi_b: msi-controller@b {
+		reg = <0xb 0x1>;
+		compatible = "vendor,some-controller";
+		msi-controller;
+		#msi-cells = <1>;
+	};
+
+	msi_c: msi-controller@c {
+		reg = <0xc 0x1>;
+		compatible = "vendor,some-controller";
+		msi-controller;
+		#msi-cells = <1>;
+	};
+
+	pci: pci@c {
+		reg = <0xf 0x1>;
+		compatible = "vendor,pcie-root-complex";
+		device_type = "pci";
+
+		/*
+		 * The sideband data provided to MSI controller a is the
+		 * RID, but the high bit of the bus number is negated.
+		 * The sideband data provided to MSI controller b is the
+		 * RID, identity-mapped.
+		 * MSI controller c is not addressable.
+		 */
+		msi-map = <0x0000 &msi_a 0x8000 0x08000>,
+			  <0x8000 &msi_a 0x0000 0x08000>,
+			  <0x0000 &msi_b 0x0000 0x10000>;
+	};
+};
-- 
1.9.1

Re: [PATCH v3 1/4] Docs: dt: Add PCI MSI map bindings

[Rob Herring]

On Wed, Sep 30, 2015 at 5:46 PM, David Daney <ddaney.cavm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>
> Currently msi-parent is used by a few bindings to describe the
> relationship between a PCI root complex and a single MSI controller, but
> this property does not have a generic binding document.
>
> Additionally, msi-parent is insufficient to describe more complex
> relationships between MSI controllers and devices under a root complex,
> where devices may be able to target multiple MSI controllers, or where
> MSI controllers use (non-probeable) sideband information to distinguish
> devices.
>
> This patch adds a generic binding for mapping PCI devices to MSI
> controllers. This document covers msi-parent, and a new msi-map property
> (specific to PCI*) which may be used to map devices (identified by their
> Requester ID) to sideband data for each MSI controller that they may
> target.
>
> Acked-by: Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>
> Signed-off-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

> ---
>  Documentation/devicetree/bindings/pci/pci-msi.txt | 220 ++++++++++++++++++++++
>  1 file changed, 220 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/pci-msi.txt
>
> diff --git a/Documentation/devicetree/bindings/pci/pci-msi.txt b/Documentation/devicetree/bindings/pci/pci-msi.txt
> new file mode 100644
> index 0000000..9b3cc81
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/pci-msi.txt
> @@ -0,0 +1,220 @@
> +This document describes the generic device tree binding for describing the
> +relationship between PCI devices and MSI controllers.
> +
> +Each PCI device under a root complex is uniquely identified by its Requester ID
> +(AKA RID). A Requester ID is a triplet of a Bus number, Device number, and
> +Function number.
> +
> +For the purpose of this document, when treated as a numeric value, a RID is
> +formatted such that:
> +
> +* Bits [15:8] are the Bus number.
> +* Bits [7:3] are the Device number.
> +* Bits [2:0] are the Function number.
> +* Any other bits required for padding must be zero.
> +
> +MSIs may be distinguished in part through the use of sideband data accompanying
> +writes. In the case of PCI devices, this sideband data may be derived from the
> +Requester ID. A mechanism is required to associate a device with both the MSI
> +controllers it can address, and the sideband data that will be associated with
> +its writes to those controllers.
> +
> +For generic MSI bindings, see
> +Documentation/devicetree/bindings/interrupt-controller/msi.txt.
> +
> +
> +PCI root complex
> +================
> +
> +Optional properties
> +-------------------
> +
> +- msi-map: Maps a Requester ID to an MSI controller and associated
> +  msi-specifier data. The property is an arbitrary number of tuples of
> +  (rid-base,msi-controller,msi-base,length), where:
> +
> +  * rid-base is a single cell describing the first RID matched by the entry.
> +
> +  * msi-controller is a single phandle to an MSI controller
> +
> +  * msi-base is an msi-specifier describing the msi-specifier produced for the
> +    first RID matched by the entry.
> +
> +  * length is a single cell describing how many consecutive RIDs are matched
> +    following the rid-base.
> +
> +  Any RID r in the interval [rid-base, rid-base + length) is associated with
> +  the listed msi-controller, with the msi-specifier (r - rid-base + msi-base).
> +
> +- msi-map-mask: A mask to be applied to each Requester ID prior to being mapped
> +  to an msi-specifier per the msi-map property.
> +
> +- msi-parent: Describes the MSI parent of the root complex itself. Where
> +  the root complex and MSI controller do not pass sideband data with MSI
> +  writes, this property may be used to describe the MSI controller(s)
> +  used by PCI devices under the root complex, if defined as such in the
> +  binding for the root complex.
> +
> +
> +Example (1)
> +===========
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       msi: msi-controller@a {
> +               reg = <0xa 0x1>;
> +               compatible = "vendor,some-controller";
> +               msi-controller;
> +               #msi-cells = <1>;
> +       };
> +
> +       pci: pci@f {
> +               reg = <0xf 0x1>;
> +               compatible = "vendor,pcie-root-complex";
> +               device_type = "pci";
> +
> +               /*
> +                * The sideband data provided to the MSI controller is
> +                * the RID, identity-mapped.
> +                */
> +               msi-map = <0x0 &msi_a 0x0 0x10000>,
> +       };
> +};
> +
> +
> +Example (2)
> +===========
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       msi: msi-controller@a {
> +               reg = <0xa 0x1>;
> +               compatible = "vendor,some-controller";
> +               msi-controller;
> +               #msi-cells = <1>;
> +       };
> +
> +       pci: pci@f {
> +               reg = <0xf 0x1>;
> +               compatible = "vendor,pcie-root-complex";
> +               device_type = "pci";
> +
> +               /*
> +                * The sideband data provided to the MSI controller is
> +                * the RID, masked to only the device and function bits.
> +                */
> +               msi-map = <0x0 &msi_a 0x0 0x100>,
> +               msi-map-mask = <0xff>
> +       };
> +};
> +
> +
> +Example (3)
> +===========
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       msi: msi-controller@a {
> +               reg = <0xa 0x1>;
> +               compatible = "vendor,some-controller";
> +               msi-controller;
> +               #msi-cells = <1>;
> +       };
> +
> +       pci: pci@f {
> +               reg = <0xf 0x1>;
> +               compatible = "vendor,pcie-root-complex";
> +               device_type = "pci";
> +
> +               /*
> +                * The sideband data provided to the MSI controller is
> +                * the RID, but the high bit of the bus number is
> +                * ignored.
> +                */
> +               msi-map = <0x0000 &msi 0x0000 0x8000>,
> +                         <0x8000 &msi 0x0000 0x8000>;
> +       };
> +};
> +
> +
> +Example (4)
> +===========
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       msi: msi-controller@a {
> +               reg = <0xa 0x1>;
> +               compatible = "vendor,some-controller";
> +               msi-controller;
> +               #msi-cells = <1>;
> +       };
> +
> +       pci: pci@f {
> +               reg = <0xf 0x1>;
> +               compatible = "vendor,pcie-root-complex";
> +               device_type = "pci";
> +
> +               /*
> +                * The sideband data provided to the MSI controller is
> +                * the RID, but the high bit of the bus number is
> +                * negated.
> +                */
> +               msi-map = <0x0000 &msi 0x8000 0x8000>,
> +                         <0x8000 &msi 0x0000 0x8000>;
> +       };
> +};
> +
> +
> +Example (5)
> +===========
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       msi_a: msi-controller@a {
> +               reg = <0xa 0x1>;
> +               compatible = "vendor,some-controller";
> +               msi-controller;
> +               #msi-cells = <1>;
> +       };
> +
> +       msi_b: msi-controller@b {
> +               reg = <0xb 0x1>;
> +               compatible = "vendor,some-controller";
> +               msi-controller;
> +               #msi-cells = <1>;
> +       };
> +
> +       msi_c: msi-controller@c {
> +               reg = <0xc 0x1>;
> +               compatible = "vendor,some-controller";
> +               msi-controller;
> +               #msi-cells = <1>;
> +       };
> +
> +       pci: pci@c {
> +               reg = <0xf 0x1>;
> +               compatible = "vendor,pcie-root-complex";
> +               device_type = "pci";
> +
> +               /*
> +                * The sideband data provided to MSI controller a is the
> +                * RID, but the high bit of the bus number is negated.
> +                * The sideband data provided to MSI controller b is the
> +                * RID, identity-mapped.
> +                * MSI controller c is not addressable.
> +                */
> +               msi-map = <0x0000 &msi_a 0x8000 0x08000>,
> +                         <0x8000 &msi_a 0x0000 0x08000>,
> +                         <0x0000 &msi_b 0x0000 0x10000>;
> +       };
> +};
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 3/4] PCI/MSI: Add helper function pci_msi_domain_get_msi_rid().

[David Daney]

From: David Daney <david.daney@cavium.com>

Add pci_msi_domain_get_msi_rid() to return the MSI requester id (RID).
Initially needed by gic-v3 based systems. It will be used by follow on
patch to drivers/irqchip/irq-gic-v3-its-pci-msi.c

Initially supports mapping the RID via OF device tree.  In the future,
this could be extended to use ACPI _IORT tables as well.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 drivers/pci/msi.c   | 31 +++++++++++++++++++++++++++++++
 include/linux/msi.h |  1 +
 2 files changed, 32 insertions(+)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index d449714..92b6dc9 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -20,6 +20,7 @@
 #include <linux/io.h>
 #include <linux/slab.h>
 #include <linux/irqdomain.h>
+#include <linux/of_irq.h>
 
 #include "pci.h"
 
@@ -1327,4 +1328,34 @@ struct irq_domain *pci_msi_create_default_irq_domain(struct device_node *node,
 
 	return domain;
 }
+
+struct get_mis_id_data {
+	u32	alias;
+};
+
+static int get_msi_id_cb(struct pci_dev *pdev, u16 alias, void *data)
+{
+	struct get_mis_id_data *s = data;
+
+	s->alias = alias;
+	return 0;
+}
+/**
+ * pci_msi_domain_get_msi_rid - Get the MSI requester id (RID)
+ * @domain:	The interrupt domain
+ * @pdev:	The PCI device.
+ *
+ * The RID for a device is formed from the alias, with a firmware
+ * supplied mapping applied
+ *
+ * Returns: The RID.
+ */
+u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev)
+{
+	struct get_mis_id_data d;
+
+	d.alias = 0;
+	pci_for_each_dma_alias(pdev, get_msi_id_cb, &d);
+	return of_msi_map_rid(&pdev->dev, domain->of_node, d.alias);
+}
 #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
diff --git a/include/linux/msi.h b/include/linux/msi.h
index ad939d0..56e3b76 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -293,6 +293,7 @@ irq_hw_number_t pci_msi_domain_calc_hwirq(struct pci_dev *dev,
 					  struct msi_desc *desc);
 int pci_msi_domain_check_cap(struct irq_domain *domain,
 			     struct msi_domain_info *info, struct device *dev);
+u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev);
 #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
 
 #endif /* LINUX_MSI_H */
-- 
1.9.1

Re: [PATCH v3 3/4] PCI/MSI: Add helper function pci_msi_domain_get_msi_rid().

[Marc Zyngier]

Hi David,

On 30/09/15 23:47, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> Add pci_msi_domain_get_msi_rid() to return the MSI requester id (RID).
> Initially needed by gic-v3 based systems. It will be used by follow on
> patch to drivers/irqchip/irq-gic-v3-its-pci-msi.c
> 
> Initially supports mapping the RID via OF device tree.  In the future,
> this could be extended to use ACPI _IORT tables as well.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  drivers/pci/msi.c   | 31 +++++++++++++++++++++++++++++++
>  include/linux/msi.h |  1 +
>  2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index d449714..92b6dc9 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -20,6 +20,7 @@
>  #include <linux/io.h>
>  #include <linux/slab.h>
>  #include <linux/irqdomain.h>
> +#include <linux/of_irq.h>
>  
>  #include "pci.h"
>  
> @@ -1327,4 +1328,34 @@ struct irq_domain *pci_msi_create_default_irq_domain(struct device_node *node,
>  
>  	return domain;
>  }
> +
> +struct get_mis_id_data {
> +	u32	alias;
> +};
> +
> +static int get_msi_id_cb(struct pci_dev *pdev, u16 alias, void *data)
> +{
> +	struct get_mis_id_data *s = data;
> +
> +	s->alias = alias;
> +	return 0;
> +}

Why not use a naked u32, since you only have a single field in this
structure? Or is it that you are anticipating other fields there?

> +/**
> + * pci_msi_domain_get_msi_rid - Get the MSI requester id (RID)
> + * @domain:	The interrupt domain
> + * @pdev:	The PCI device.
> + *
> + * The RID for a device is formed from the alias, with a firmware
> + * supplied mapping applied
> + *
> + * Returns: The RID.
> + */
> +u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev)
> +{
> +	struct get_mis_id_data d;
> +
> +	d.alias = 0;
> +	pci_for_each_dma_alias(pdev, get_msi_id_cb, &d);
> +	return of_msi_map_rid(&pdev->dev, domain->of_node, d.alias);

Should you check whether domain->of_node is NULL first? I don't think
of_msi_map_rid would have any problem with that, but a domain that is
not backed by an of_node makes me feel a bit uneasy and would tend to
indicate that we're not using DT.

> +}
>  #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index ad939d0..56e3b76 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -293,6 +293,7 @@ irq_hw_number_t pci_msi_domain_calc_hwirq(struct pci_dev *dev,
>  					  struct msi_desc *desc);
>  int pci_msi_domain_check_cap(struct irq_domain *domain,
>  			     struct msi_domain_info *info, struct device *dev);
> +u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev);
>  #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
>  
>  #endif /* LINUX_MSI_H */
> 

Otherwise looks good to me.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v3 3/4] PCI/MSI: Add helper function pci_msi_domain_get_msi_rid().

[David Daney]

On 10/01/2015 02:24 AM, Marc Zyngier wrote:
> Hi David,
>
> On 30/09/15 23:47, David Daney wrote:
>> From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>>
>> Add pci_msi_domain_get_msi_rid() to return the MSI requester id (RID).
>> Initially needed by gic-v3 based systems. It will be used by follow on
>> patch to drivers/irqchip/irq-gic-v3-its-pci-msi.c
>>
>> Initially supports mapping the RID via OF device tree.  In the future,
>> this could be extended to use ACPI _IORT tables as well.
>>
>> Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>> ---
>>   drivers/pci/msi.c   | 31 +++++++++++++++++++++++++++++++
>>   include/linux/msi.h |  1 +
>>   2 files changed, 32 insertions(+)
>>
>> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
>> index d449714..92b6dc9 100644
>> --- a/drivers/pci/msi.c
>> +++ b/drivers/pci/msi.c
>> @@ -20,6 +20,7 @@
>>   #include <linux/io.h>
>>   #include <linux/slab.h>
>>   #include <linux/irqdomain.h>
>> +#include <linux/of_irq.h>
>>
>>   #include "pci.h"
>>
>> @@ -1327,4 +1328,34 @@ struct irq_domain *pci_msi_create_default_irq_domain(struct device_node *node,
>>
>>   	return domain;
>>   }
>> +
>> +struct get_mis_id_data {
>> +	u32	alias;
>> +};
>> +
>> +static int get_msi_id_cb(struct pci_dev *pdev, u16 alias, void *data)
>> +{
>> +	struct get_mis_id_data *s = data;
>> +
>> +	s->alias = alias;
>> +	return 0;
>> +}
>
> Why not use a naked u32, since you only have a single field in this
> structure? Or is it that you are anticipating other fields there?

In this case, I think using a pointer to u32 is a good idea.  It would 
simplify the source code somewhat.  Although, I think the generated 
binary would likely be the same.  I don't foresee adding things to this 
structure.  If it becomes necessary in the future, we can just go back 
to using a pointer to a structure.

>
>> +/**
>> + * pci_msi_domain_get_msi_rid - Get the MSI requester id (RID)
>> + * @domain:	The interrupt domain
>> + * @pdev:	The PCI device.
>> + *
>> + * The RID for a device is formed from the alias, with a firmware
>> + * supplied mapping applied
>> + *
>> + * Returns: The RID.
>> + */
>> +u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev)
>> +{
>> +	struct get_mis_id_data d;
>> +
>> +	d.alias = 0;
>> +	pci_for_each_dma_alias(pdev, get_msi_id_cb, &d);
>> +	return of_msi_map_rid(&pdev->dev, domain->of_node, d.alias);
>
> Should you check whether domain->of_node is NULL first? I don't think
> of_msi_map_rid would have any problem with that, but a domain that is
> not backed by an of_node makes me feel a bit uneasy and would tend to
> indicate that we're not using DT.

Yes, that makes sense.  As you observe, I think it probably works as is, 
but it would be good to make it more clear.  This is especially true 
when we add ACPI support.  We will want to be clear on which of 
device-tree or ACPI we are using.


>
>> +}
>>   #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
>> diff --git a/include/linux/msi.h b/include/linux/msi.h
>> index ad939d0..56e3b76 100644
>> --- a/include/linux/msi.h
>> +++ b/include/linux/msi.h
>> @@ -293,6 +293,7 @@ irq_hw_number_t pci_msi_domain_calc_hwirq(struct pci_dev *dev,
>>   					  struct msi_desc *desc);
>>   int pci_msi_domain_check_cap(struct irq_domain *domain,
>>   			     struct msi_domain_info *info, struct device *dev);
>> +u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev);
>>   #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
>>
>>   #endif /* LINUX_MSI_H */
>>
>
> Otherwise looks good to me.

I will send what I hope is the final revision of the patches later today.

David Daney


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/4] PCI/MSI: Add helper function pci_msi_domain_get_msi_rid().

[Marc Zyngier]

On 01/10/15 17:13, David Daney wrote:
> On 10/01/2015 02:24 AM, Marc Zyngier wrote:
>> Hi David,
>>
>> On 30/09/15 23:47, David Daney wrote:
>>> From: David Daney <david.daney@cavium.com>
>>>
>>> Add pci_msi_domain_get_msi_rid() to return the MSI requester id (RID).
>>> Initially needed by gic-v3 based systems. It will be used by follow on
>>> patch to drivers/irqchip/irq-gic-v3-its-pci-msi.c
>>>
>>> Initially supports mapping the RID via OF device tree.  In the future,
>>> this could be extended to use ACPI _IORT tables as well.
>>>
>>> Signed-off-by: David Daney <david.daney@cavium.com>
>>> ---
>>>   drivers/pci/msi.c   | 31 +++++++++++++++++++++++++++++++
>>>   include/linux/msi.h |  1 +
>>>   2 files changed, 32 insertions(+)
>>>
>>> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
>>> index d449714..92b6dc9 100644
>>> --- a/drivers/pci/msi.c
>>> +++ b/drivers/pci/msi.c
>>> @@ -20,6 +20,7 @@
>>>   #include <linux/io.h>
>>>   #include <linux/slab.h>
>>>   #include <linux/irqdomain.h>
>>> +#include <linux/of_irq.h>
>>>
>>>   #include "pci.h"
>>>
>>> @@ -1327,4 +1328,34 @@ struct irq_domain *pci_msi_create_default_irq_domain(struct device_node *node,
>>>
>>>   	return domain;
>>>   }
>>> +
>>> +struct get_mis_id_data {
>>> +	u32	alias;
>>> +};
>>> +
>>> +static int get_msi_id_cb(struct pci_dev *pdev, u16 alias, void *data)
>>> +{
>>> +	struct get_mis_id_data *s = data;
>>> +
>>> +	s->alias = alias;
>>> +	return 0;
>>> +}
>>
>> Why not use a naked u32, since you only have a single field in this
>> structure? Or is it that you are anticipating other fields there?
> 
> In this case, I think using a pointer to u32 is a good idea.  It would 
> simplify the source code somewhat.  Although, I think the generated 
> binary would likely be the same.  I don't foresee adding things to this 
> structure.  If it becomes necessary in the future, we can just go back 
> to using a pointer to a structure.
> 
>>
>>> +/**
>>> + * pci_msi_domain_get_msi_rid - Get the MSI requester id (RID)
>>> + * @domain:	The interrupt domain
>>> + * @pdev:	The PCI device.
>>> + *
>>> + * The RID for a device is formed from the alias, with a firmware
>>> + * supplied mapping applied
>>> + *
>>> + * Returns: The RID.
>>> + */
>>> +u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev)
>>> +{
>>> +	struct get_mis_id_data d;
>>> +
>>> +	d.alias = 0;
>>> +	pci_for_each_dma_alias(pdev, get_msi_id_cb, &d);
>>> +	return of_msi_map_rid(&pdev->dev, domain->of_node, d.alias);
>>
>> Should you check whether domain->of_node is NULL first? I don't think
>> of_msi_map_rid would have any problem with that, but a domain that is
>> not backed by an of_node makes me feel a bit uneasy and would tend to
>> indicate that we're not using DT.
> 
> Yes, that makes sense.  As you observe, I think it probably works as is, 
> but it would be good to make it more clear.  This is especially true 
> when we add ACPI support.  We will want to be clear on which of 
> device-tree or ACPI we are using.
> 
> 
>>
>>> +}
>>>   #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
>>> diff --git a/include/linux/msi.h b/include/linux/msi.h
>>> index ad939d0..56e3b76 100644
>>> --- a/include/linux/msi.h
>>> +++ b/include/linux/msi.h
>>> @@ -293,6 +293,7 @@ irq_hw_number_t pci_msi_domain_calc_hwirq(struct pci_dev *dev,
>>>   					  struct msi_desc *desc);
>>>   int pci_msi_domain_check_cap(struct irq_domain *domain,
>>>   			     struct msi_domain_info *info, struct device *dev);
>>> +u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev);
>>>   #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
>>>
>>>   #endif /* LINUX_MSI_H */
>>>
>>
>> Otherwise looks good to me.
> 
> I will send what I hope is the final revision of the patches later today.

Excellent. In related news, I've rebased my msi-parent stuff on top of
this series, and extended it to also deal with msi-map for matching MSI
domains.

With the two series, we should now have something vaguely coherent that
deals with both the old version of msi-parent, its new definition, and
msi-map in its whole glory. Fun times!

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH v3 2/4] of/irq: Add new function of_msi_map_rid()

[David Daney]

From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

The device tree property "msi-map" specifies how to create the PCI
requester id used in some MSI controllers.  Add a new function
of_msi_map_rid() that finds the msi-map property and applies its
translation to a given requester id.

Reviewed-by: Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
---
 drivers/of/irq.c       | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of_irq.h |  7 +++++
 2 files changed, 91 insertions(+)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 55317fa..c90bd4e 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -598,3 +598,87 @@ void of_msi_configure(struct device *dev, struct device_node *np)
 		d = irq_find_host(msi_np);
 	dev_set_msi_domain(dev, d);
 }
+
+/**
+ * of_msi_map_rid - Map a MSI requester ID for a device.
+ * @dev: device for which the mapping is to be done.
+ * @msi_np: device node of the expected msi controller.
+ * @rid_in: unmapped MSI requester ID for the device.
+ *
+ * Walk up the device hierarchy looking for devices with a "msi-map"
+ * property.  If found, apply the mapping to @rid_in.
+ *
+ * Returns the mapped MSI requester ID.
+ */
+u32 of_msi_map_rid(struct device *dev, struct device_node *msi_np, u32 rid_in)
+{
+	struct device *parent_dev;
+	struct device_node *msi_controller_node;
+	u32 map_mask, masked_rid, rid_base, msi_base, rid_len, phandle;
+	int msi_map_len;
+	bool matched;
+	u32 rid_out = rid_in;
+	const __be32 *msi_map = NULL;
+
+	/*
+	 * Walk up the device parent links looking for one with a
+	 * "msi-map" property.
+	 */
+	for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent) {
+		if (!parent_dev->of_node)
+			continue;
+
+		msi_map = of_get_property(parent_dev->of_node,
+					  "msi-map", &msi_map_len);
+		if (!msi_map)
+			continue;
+
+		if (msi_map_len % (4 * sizeof(__be32))) {
+			dev_err(parent_dev, "Error: Bad msi-map length: %d\n",
+				msi_map_len);
+			return rid_out;
+		}
+		/* We have a good parent_dev and msi_map, let's use them. */
+		break;
+	}
+	if (!msi_map)
+		return rid_out;
+
+	/* The default is to select all bits. */
+	map_mask = 0xffffffff;
+
+	/*
+	 * Can be overridden by "msi-map-mask" property.  If
+	 * of_property_read_u32() fails, the default is used.
+	 */
+	of_property_read_u32(parent_dev->of_node, "msi-map-mask", &map_mask);
+
+	masked_rid = map_mask & rid_in;
+	matched = false;
+	while (!matched && msi_map_len >= 4 * sizeof(__be32)) {
+		rid_base = be32_to_cpup(msi_map + 0);
+		phandle = be32_to_cpup(msi_map + 1);
+		msi_base = be32_to_cpup(msi_map + 2);
+		rid_len = be32_to_cpup(msi_map + 3);
+
+		msi_controller_node = of_find_node_by_phandle(phandle);
+
+		matched = masked_rid >= rid_base &&
+			masked_rid < rid_base + rid_len &&
+			msi_np == msi_controller_node;
+
+		of_node_put(msi_controller_node);
+		msi_map_len -= 4 * sizeof(__be32);
+		msi_map += 4;
+	}
+	if (!matched)
+		return rid_out;
+
+	rid_out = masked_rid + msi_base;
+	dev_dbg(dev,
+		"msi-map at: %s, using mask %08x, rid-base: %08x, msi-base: %08x, length: %08x, rid: %08x -> %08x\n",
+		dev_name(parent_dev), map_mask, rid_base, msi_base,
+		rid_len, rid_in, rid_out);
+
+	return rid_out;
+}
diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
index 4bcbd58..8cd9334 100644
--- a/include/linux/of_irq.h
+++ b/include/linux/of_irq.h
@@ -75,6 +75,7 @@ static inline int of_irq_to_resource_table(struct device_node *dev,
 extern unsigned int irq_of_parse_and_map(struct device_node *node, int index);
 extern struct device_node *of_irq_find_parent(struct device_node *child);
 extern void of_msi_configure(struct device *dev, struct device_node *np);
+u32 of_msi_map_rid(struct device *dev, struct device_node *msi_np, u32 rid_in);
 
 #else /* !CONFIG_OF */
 static inline unsigned int irq_of_parse_and_map(struct device_node *dev,
@@ -87,6 +88,12 @@ static inline void *of_irq_find_parent(struct device_node *child)
 {
 	return NULL;
 }
+
+static inline u32 of_msi_map_rid(struct device *dev,
+				 struct device_node *msi_np, u32 rid_in)
+{
+	return rid_in;
+}
 #endif /* !CONFIG_OF */
 
 #endif /* __OF_IRQ_H */
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/4] of/irq: Add new function of_msi_map_rid()

[Rob Herring]

On Wed, Sep 30, 2015 at 5:47 PM, David Daney <ddaney.cavm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>
> The device tree property "msi-map" specifies how to create the PCI
> requester id used in some MSI controllers.  Add a new function
> of_msi_map_rid() that finds the msi-map property and applies its
> translation to a given requester id.
>
> Reviewed-by: Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>
> Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

I'm assuming this will go thru irqchip or PCI trees? It helps if To is
who you want to merge this.

Rob

> ---
>  drivers/of/irq.c       | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of_irq.h |  7 +++++
>  2 files changed, 91 insertions(+)
>
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 55317fa..c90bd4e 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -598,3 +598,87 @@ void of_msi_configure(struct device *dev, struct device_node *np)
>                 d = irq_find_host(msi_np);
>         dev_set_msi_domain(dev, d);
>  }
> +
> +/**
> + * of_msi_map_rid - Map a MSI requester ID for a device.
> + * @dev: device for which the mapping is to be done.
> + * @msi_np: device node of the expected msi controller.
> + * @rid_in: unmapped MSI requester ID for the device.
> + *
> + * Walk up the device hierarchy looking for devices with a "msi-map"
> + * property.  If found, apply the mapping to @rid_in.
> + *
> + * Returns the mapped MSI requester ID.
> + */
> +u32 of_msi_map_rid(struct device *dev, struct device_node *msi_np, u32 rid_in)
> +{
> +       struct device *parent_dev;
> +       struct device_node *msi_controller_node;
> +       u32 map_mask, masked_rid, rid_base, msi_base, rid_len, phandle;
> +       int msi_map_len;
> +       bool matched;
> +       u32 rid_out = rid_in;
> +       const __be32 *msi_map = NULL;
> +
> +       /*
> +        * Walk up the device parent links looking for one with a
> +        * "msi-map" property.
> +        */
> +       for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent) {
> +               if (!parent_dev->of_node)
> +                       continue;
> +
> +               msi_map = of_get_property(parent_dev->of_node,
> +                                         "msi-map", &msi_map_len);
> +               if (!msi_map)
> +                       continue;
> +
> +               if (msi_map_len % (4 * sizeof(__be32))) {
> +                       dev_err(parent_dev, "Error: Bad msi-map length: %d\n",
> +                               msi_map_len);
> +                       return rid_out;
> +               }
> +               /* We have a good parent_dev and msi_map, let's use them. */
> +               break;
> +       }
> +       if (!msi_map)
> +               return rid_out;
> +
> +       /* The default is to select all bits. */
> +       map_mask = 0xffffffff;
> +
> +       /*
> +        * Can be overridden by "msi-map-mask" property.  If
> +        * of_property_read_u32() fails, the default is used.
> +        */
> +       of_property_read_u32(parent_dev->of_node, "msi-map-mask", &map_mask);
> +
> +       masked_rid = map_mask & rid_in;
> +       matched = false;
> +       while (!matched && msi_map_len >= 4 * sizeof(__be32)) {
> +               rid_base = be32_to_cpup(msi_map + 0);
> +               phandle = be32_to_cpup(msi_map + 1);
> +               msi_base = be32_to_cpup(msi_map + 2);
> +               rid_len = be32_to_cpup(msi_map + 3);
> +
> +               msi_controller_node = of_find_node_by_phandle(phandle);
> +
> +               matched = masked_rid >= rid_base &&
> +                       masked_rid < rid_base + rid_len &&
> +                       msi_np == msi_controller_node;
> +
> +               of_node_put(msi_controller_node);
> +               msi_map_len -= 4 * sizeof(__be32);
> +               msi_map += 4;
> +       }
> +       if (!matched)
> +               return rid_out;
> +
> +       rid_out = masked_rid + msi_base;
> +       dev_dbg(dev,
> +               "msi-map at: %s, using mask %08x, rid-base: %08x, msi-base: %08x, length: %08x, rid: %08x -> %08x\n",
> +               dev_name(parent_dev), map_mask, rid_base, msi_base,
> +               rid_len, rid_in, rid_out);
> +
> +       return rid_out;
> +}
> diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
> index 4bcbd58..8cd9334 100644
> --- a/include/linux/of_irq.h
> +++ b/include/linux/of_irq.h
> @@ -75,6 +75,7 @@ static inline int of_irq_to_resource_table(struct device_node *dev,
>  extern unsigned int irq_of_parse_and_map(struct device_node *node, int index);
>  extern struct device_node *of_irq_find_parent(struct device_node *child);
>  extern void of_msi_configure(struct device *dev, struct device_node *np);
> +u32 of_msi_map_rid(struct device *dev, struct device_node *msi_np, u32 rid_in);
>
>  #else /* !CONFIG_OF */
>  static inline unsigned int irq_of_parse_and_map(struct device_node *dev,
> @@ -87,6 +88,12 @@ static inline void *of_irq_find_parent(struct device_node *child)
>  {
>         return NULL;
>  }
> +
> +static inline u32 of_msi_map_rid(struct device *dev,
> +                                struct device_node *msi_np, u32 rid_in)
> +{
> +       return rid_in;
> +}
>  #endif /* !CONFIG_OF */
>
>  #endif /* __OF_IRQ_H */
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 4/4] irqchip/gic-v3-its: Add handling of PCI requester id.

[David Daney]

From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

Replace open coded generation PCI/MSI requester id with call to the
new function pci_msi_domain_get_msi_rid() which applies the "msi-map"
to the id value.

Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
---
 drivers/irqchip/irq-gic-v3-its-pci-msi.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
index cf351c6..7bbf64a 100644
--- a/drivers/irqchip/irq-gic-v3-its-pci-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
@@ -42,7 +42,6 @@ static struct irq_chip its_msi_irq_chip = {
 
 struct its_pci_alias {
 	struct pci_dev	*pdev;
-	u32		dev_id;
 	u32		count;
 };
 
@@ -60,7 +59,6 @@ static int its_get_pci_alias(struct pci_dev *pdev, u16 alias, void *data)
 {
 	struct its_pci_alias *dev_alias = data;
 
-	dev_alias->dev_id = alias;
 	if (pdev != dev_alias->pdev)
 		dev_alias->count += its_pci_msi_vec_count(dev_alias->pdev);
 
@@ -86,7 +84,7 @@ static int its_pci_msi_prepare(struct irq_domain *domain, struct device *dev,
 	pci_for_each_dma_alias(pdev, its_get_pci_alias, &dev_alias);
 
 	/* ITS specific DeviceID, as the core ITS ignores dev. */
-	info->scratchpad[0].ul = dev_alias.dev_id;
+	info->scratchpad[0].ul = pci_msi_domain_get_msi_rid(domain, pdev);
 
 	return msi_info->ops->msi_prepare(domain->parent,
 					  dev, dev_alias.count, info);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html