[PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[Ian Campbell]

This series adds parsing of the DT ranges and interrupt-map properties
for PCI devices, these contain the MMIOs and IRQs used by children on
the bus. This replaces the specific mapping on xgene which should also
mean that it works on xgene devices other than mustang which use a
different PCI root controller (the xgene has several, we only map the
first).

I've tested on Mustang and on a FastModel with virtio-pci based rootfs.

This is *not* for 4.5.

Ian.

[PATCH 1/5] xen: arm: propagate gic's #address-cells property to dom0.

[Ian Campbell]

The interrupt-map property requires that the interrupt-parent node
must have both #address-cells and #interrupt-cells properties (see
ePAPR 2.4.3.1). Therefore propagate the property if it is present.

We must propagate (rather than invent our own value) since this value
is used to size fields within other properties within the tree.

ePAPR strictly speaking requires that the interrupt-parent node
always has these properties. However reality has diverged from this
and implementations will recursively search parents for #*-cells
properties. Hence we only copy if it is present.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/domain_build.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 5aca925..de180d8 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -788,6 +788,8 @@ static int make_gic_node(const struct domain *d, void *fdt,
 {
     const struct dt_device_node *gic = dt_interrupt_controller;
     int res = 0;
+    const void *addrcells;
+    u32 addrcells_len;
 
     /*
      * Xen currently supports only a single GIC. Discard any secondary
@@ -817,6 +819,14 @@ static int make_gic_node(const struct domain *d, void *fdt,
             return res;
     }
 
+    addrcells = dt_get_property(gic, "#address-cells", &addrcells_len);
+    if ( addrcells )
+    {
+        res = fdt_property(fdt, "#address-cells", addrcells, addrcells_len);
+        if ( res )
+            return res;
+    }
+
     res = fdt_end_node(fdt);
 
     return res;
-- 
1.7.10.4

Re: [PATCH 1/5] xen: arm: propagate gic's #address-cells property to dom0.

[Julien Grall]

Hi Ian,

On 24/10/2014 10:58, Ian Campbell wrote:
> The interrupt-map property requires that the interrupt-parent node
> must have both #address-cells and #interrupt-cells properties (see
> ePAPR 2.4.3.1). Therefore propagate the property if it is present.
>
> We must propagate (rather than invent our own value) since this value
> is used to size fields within other properties within the tree.
>
> ePAPR strictly speaking requires that the interrupt-parent node
> always has these properties. However reality has diverged from this
> and implementations will recursively search parents for #*-cells
> properties. Hence we only copy if it is present.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Reviewed-by: Julien Grall <julien.grall@linaro.org>

Without this patch I can't boot Xen on the Foundation model with GIC-v3. 
Is it possible to push this patch for Xen 4.5 rc1?

Regards,

-- 
Julien Grall

Re: [PATCH 1/5] xen: arm: propagate gic's #address-cells property to dom0.

[Ian Campbell]

On Wed, 2014-10-29 at 19:03 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 24/10/2014 10:58, Ian Campbell wrote:
> > The interrupt-map property requires that the interrupt-parent node
> > must have both #address-cells and #interrupt-cells properties (see
> > ePAPR 2.4.3.1). Therefore propagate the property if it is present.
> >
> > We must propagate (rather than invent our own value) since this value
> > is used to size fields within other properties within the tree.
> >
> > ePAPR strictly speaking requires that the interrupt-parent node
> > always has these properties. However reality has diverged from this
> > and implementations will recursively search parents for #*-cells
> > properties. Hence we only copy if it is present.
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Reviewed-by: Julien Grall <julien.grall@linaro.org>
> 
> Without this patch I can't boot Xen on the Foundation model with GIC-v3. 
> Is it possible to push this patch for Xen 4.5 rc1?

rc1 is long gone... I'll push for rc2 though.

Ian.

Re: [PATCH 1/5] xen: arm: propagate gic's #address-cells property to dom0.

[Julien Grall]

On 30/10/2014 10:06, Ian Campbell wrote:
> On Wed, 2014-10-29 at 19:03 +0000, Julien Grall wrote:
>> Hi Ian,
>>
>> On 24/10/2014 10:58, Ian Campbell wrote:
>>> The interrupt-map property requires that the interrupt-parent node
>>> must have both #address-cells and #interrupt-cells properties (see
>>> ePAPR 2.4.3.1). Therefore propagate the property if it is present.
>>>
>>> We must propagate (rather than invent our own value) since this value
>>> is used to size fields within other properties within the tree.
>>>
>>> ePAPR strictly speaking requires that the interrupt-parent node
>>> always has these properties. However reality has diverged from this
>>> and implementations will recursively search parents for #*-cells
>>> properties. Hence we only copy if it is present.
>>>
>>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>>
>> Reviewed-by: Julien Grall <julien.grall@linaro.org>
>>
>> Without this patch I can't boot Xen on the Foundation model with GIC-v3.
>> Is it possible to push this patch for Xen 4.5 rc1?
>
> rc1 is long gone... I'll push for rc2 though.

Hmm right, I though the first version was rc0 not rc1.

-- 
Julien Grall

Re: [PATCH 1/5] xen: arm: propagate gic's #address-cells property to dom0.

[Ian Campbell]

On Wed, 2014-10-29 at 19:03 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 24/10/2014 10:58, Ian Campbell wrote:
> > The interrupt-map property requires that the interrupt-parent node
> > must have both #address-cells and #interrupt-cells properties (see
> > ePAPR 2.4.3.1). Therefore propagate the property if it is present.
> >
> > We must propagate (rather than invent our own value) since this value
> > is used to size fields within other properties within the tree.
> >
> > ePAPR strictly speaking requires that the interrupt-parent node
> > always has these properties. However reality has diverged from this
> > and implementations will recursively search parents for #*-cells
> > properties. Hence we only copy if it is present.
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Reviewed-by: Julien Grall <julien.grall@linaro.org>
> 
> Without this patch I can't boot Xen on the Foundation model with GIC-v3. 
> Is it possible to push this patch for Xen 4.5 rc1?

Konrad, I think this is needed for 4.5 since without it dom0 can fail to
parse certain other constructs within the DT (in bits which we don't
generate and can't easily/don't want to rewrite as we pass them
through).

The risk is that some bit of DT which we do generate relies on this
value being absent (as it was before this patch). I don't believe we
generate any such nodes. The consumers are typically nodes representing
devices which we pass through rather than messing with them.

Ian.

Re: [PATCH 1/5] xen: arm: propagate gic's #address-cells property to dom0.

[Konrad Rzeszutek Wilk]

On Tue, Nov 04, 2014 at 10:23:55AM +0000, Ian Campbell wrote:
> On Wed, 2014-10-29 at 19:03 +0000, Julien Grall wrote:
> > Hi Ian,
> > 
> > On 24/10/2014 10:58, Ian Campbell wrote:
> > > The interrupt-map property requires that the interrupt-parent node
> > > must have both #address-cells and #interrupt-cells properties (see
> > > ePAPR 2.4.3.1). Therefore propagate the property if it is present.
> > >
> > > We must propagate (rather than invent our own value) since this value
> > > is used to size fields within other properties within the tree.
> > >
> > > ePAPR strictly speaking requires that the interrupt-parent node
> > > always has these properties. However reality has diverged from this
> > > and implementations will recursively search parents for #*-cells
> > > properties. Hence we only copy if it is present.
> > >
> > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > Reviewed-by: Julien Grall <julien.grall@linaro.org>
> > 
> > Without this patch I can't boot Xen on the Foundation model with GIC-v3. 
> > Is it possible to push this patch for Xen 4.5 rc1?
> 
> Konrad, I think this is needed for 4.5 since without it dom0 can fail to
> parse certain other constructs within the DT (in bits which we don't
> generate and can't easily/don't want to rewrite as we pass them
> through).

Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>


> 
> The risk is that some bit of DT which we do generate relies on this
> value being absent (as it was before this patch). I don't believe we
> generate any such nodes. The consumers are typically nodes representing
> devices which we pass through rather than messing with them.
> 
> Ian.
> 

Re: [PATCH 1/5] xen: arm: propagate gic's #address-cells property to dom0.

[Ian Campbell]

On Tue, 2014-11-04 at 12:11 -0500, Konrad Rzeszutek Wilk wrote:
> On Tue, Nov 04, 2014 at 10:23:55AM +0000, Ian Campbell wrote:
> > On Wed, 2014-10-29 at 19:03 +0000, Julien Grall wrote:
> > > Hi Ian,
> > > 
> > > On 24/10/2014 10:58, Ian Campbell wrote:
> > > > The interrupt-map property requires that the interrupt-parent node
> > > > must have both #address-cells and #interrupt-cells properties (see
> > > > ePAPR 2.4.3.1). Therefore propagate the property if it is present.
> > > >
> > > > We must propagate (rather than invent our own value) since this value
> > > > is used to size fields within other properties within the tree.
> > > >
> > > > ePAPR strictly speaking requires that the interrupt-parent node
> > > > always has these properties. However reality has diverged from this
> > > > and implementations will recursively search parents for #*-cells
> > > > properties. Hence we only copy if it is present.
> > > >
> > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > > 
> > > Reviewed-by: Julien Grall <julien.grall@linaro.org>
> > > 
> > > Without this patch I can't boot Xen on the Foundation model with GIC-v3. 
> > > Is it possible to push this patch for Xen 4.5 rc1?
> > 
> > Konrad, I think this is needed for 4.5 since without it dom0 can fail to
> > parse certain other constructs within the DT (in bits which we don't
> > generate and can't easily/don't want to rewrite as we pass them
> > through).
> 
> Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Applied, thanks.

Ian.

[PATCH 2/5] xen: device-tree: add accessors for the addr/size-cells of a node's children.

[Ian Campbell]

The existing dt_n_{addr,size}_cells functions tell you which sizes will apply
to this node, which are the #address/size-cells properties of the node's
*parent* (or grandparent, etc).

In some cases we need to know what size applies to a nodes children, which can
include the #address/size-cells properties of the node itself (or its parent).

Refactor the existing function to allow both possibilities.

Clean up the grammar of the existing doc comments a bit.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/common/device_tree.c      |   35 ++++++++++++++++++++++++++----
 xen/include/xen/device_tree.h |   47 ++++++++++++++++++++++++++++++++++-------
 2 files changed, 70 insertions(+), 12 deletions(-)

diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index f72b2e9..a4e4eb5 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -368,13 +368,17 @@ dt_find_matching_node(struct dt_device_node *from,
     return NULL;
 }
 
-int dt_n_addr_cells(const struct dt_device_node *np)
+/* If parent is true then start from the parent. */
+static int dt_n_addr_cells_common(const struct dt_device_node *np, bool parent)
 {
     const __be32 *ip;
 
     do {
-        if ( np->parent )
+        if ( parent && np->parent )
             np = np->parent;
+        else
+            parent = 1;
+
         ip = dt_get_property(np, "#address-cells", NULL);
         if ( ip )
             return be32_to_cpup(ip);
@@ -383,13 +387,26 @@ int dt_n_addr_cells(const struct dt_device_node *np)
     return DT_ROOT_NODE_ADDR_CELLS_DEFAULT;
 }
 
-int dt_n_size_cells(const struct dt_device_node *np)
+int dt_n_addr_cells(const struct dt_device_node *np)
+{
+    return dt_n_addr_cells_common(np, true);
+}
+
+int dt_n_addr_cells_children(const struct dt_device_node *np)
+{
+    return dt_n_addr_cells_common(np, false);
+}
+
+/* If parent is true then start from the parent. */
+static int dt_n_size_cells_common(const struct dt_device_node *np, bool parent)
 {
     const __be32 *ip;
 
     do {
-        if ( np->parent )
+        if ( parent && np->parent )
             np = np->parent;
+        else
+            parent = 1;
         ip = dt_get_property(np, "#size-cells", NULL);
         if ( ip )
             return be32_to_cpup(ip);
@@ -398,6 +415,16 @@ int dt_n_size_cells(const struct dt_device_node *np)
     return DT_ROOT_NODE_SIZE_CELLS_DEFAULT;
 }
 
+int dt_n_size_cells(const struct dt_device_node *np)
+{
+    return dt_n_size_cells_common(np, true);
+}
+int dt_n_size_cells_children(const struct dt_device_node *np)
+{
+    return dt_n_size_cells_common(np, false);
+}
+
+
 /*
  * Default translator (generic bus)
  */
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 08db8bc..ac2e876 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -513,24 +513,55 @@ int dt_device_get_raw_irq(const struct dt_device_node *device, int index,
 int dt_irq_translate(const struct dt_raw_irq *raw, struct dt_irq *out_irq);
 
 /**
- * dt_n_size_cells - Helper to retrieve the number of cell for the size
- * @np: node to get the value
+ * dt_n_size_cells - Helper to retrieve the number of cells used for
+ * size properties of a node (e.g. the size part of a reg property).
+ *
+ * @np: node to get the value for
  *
- * This function retrieves for a give device-tree node the number of
- * cell for the size field.
+ * This function retrieves for a given device-tree node the number of
+ * cells used for size properties, which is the #size-cells property of
+ * the node's parent (or grandparent etc).
  */
 int dt_n_size_cells(const struct dt_device_node *np);
 
 /**
- * dt_n_addr_cells - Helper to retrieve the number of cell for the address
- * @np: node to get the value
+ * dt_n_size_cells_children - Helper to retrieve the number of cells
+ * used for size properties of child nodes.
+ *
+ * @np: node to get the value for
+ *
+ * This function retrieves the #size-cells field which will apply to
+ * this node's children, which may be specified by this node or its
+ * parent, grantparent etc.
+ */
+int dt_n_size_cells_children(const struct dt_device_node *node);
+
+/**
+ * dt_n_addr_cells - Helper to retrieve the number of cells for
+ * address properties of a node (e.g. the adddress part of a reg
+ * property).
+ *
+ * @np: node to get the value for
  *
- * This function retrieves for a give device-tree node the number of
- * cell for the address field.
+ * This function retrieves for a given device-tree node the number of
+ * cells used for address properties, which is the #address-cells
+ * property of the node's parent (or grantparent, etc).
  */
 int dt_n_addr_cells(const struct dt_device_node *np);
 
 /**
+ * dt_n_size_cells_children - Helper to retrieve the number of cells
+ * used for size properties of child nodes.
+ *
+ * @np: node to get the value for
+ *
+ * This function retrieves the #address-cells field which will apply to
+ * this node's children, which may be specified by this node or its
+ * parent, grantparent etc.
+ */
+int dt_n_addr_cells_children(const struct dt_device_node *np);
+
+/**
  * dt_device_is_available - Check if a device is available for use
  *
  * @device: Node to check for availability
-- 
1.7.10.4

[PATCH 3/5] xen: arm: Add DT_NR_GIC_INTERRUPT_CELLS rather than hardcoding 3

[Ian Campbell]

I'm going to need this elsewhere and would prefer to avoid writing 3 again.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/domain_build.c |    2 +-
 xen/arch/arm/gic.c          |    2 +-
 xen/include/asm-arm/gic.h   |    2 ++
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index de180d8..998b6fd 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -500,7 +500,7 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
  * This code is assuming the irq is an PPI.
  */
 
-typedef __be32 gic_interrupt_t[3];
+typedef __be32 gic_interrupt_t[DT_NR_GIC_INTERRUPT_CELLS];
 
 static void set_interrupt_ppi(gic_interrupt_t interrupt, unsigned int irq,
                               unsigned int cpumask, unsigned int level)
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 70d10d6..148f555 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -147,7 +147,7 @@ int gic_irq_xlate(const u32 *intspec, unsigned int intsize,
                   unsigned int *out_hwirq,
                   unsigned int *out_type)
 {
-    if ( intsize < 3 )
+    if ( intsize < DT_NR_GIC_INTERRUPT_CELLS )
         return -EINVAL;
 
     /* Get the interrupt number and add 16 to skip over SGIs */
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 187dc46..d697dd9 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -22,6 +22,8 @@
 #define NR_GIC_SGI         16
 #define MAX_RDIST_COUNT    4
 
+#define DT_NR_GIC_INTERRUPT_CELLS 3
+
 #define GICD_CTLR       (0x000)
 #define GICD_TYPER      (0x004)
 #define GICD_IIDR       (0x008)
-- 
1.7.10.4

[PATCH 4/5] xen: refactor irq_set_type out of platform_get_irq

[Ian Campbell]

When parsing a PCI nodes interrupt-map property I need to be able to configure
an interrupt which has no associated dt_device_node. So refactor the code and
expose.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/irq.c        |   16 ++++++++++------
 xen/include/asm-arm/irq.h |    1 +
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 25ecf1d..084324a 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -520,6 +520,15 @@ unlock:
     return ret;
 }
 
+int irq_set_type(unsigned int irq, unsigned int type)
+{
+    /* Setup the IRQ type */
+    if ( irq < NR_LOCAL_IRQS )
+        return irq_local_set_type(irq, type);
+    else
+        return irq_set_spi_type(irq, type);
+}
+
 int platform_get_irq(const struct dt_device_node *device, int index)
 {
     struct dt_irq dt_irq;
@@ -533,12 +542,7 @@ int platform_get_irq(const struct dt_device_node *device, int index)
     irq = dt_irq.irq;
     type = dt_irq.type;
 
-    /* Setup the IRQ type */
-    if ( irq < NR_LOCAL_IRQS )
-        res = irq_local_set_type(irq, type);
-    else
-        res = irq_set_spi_type(irq, type);
-
+    res = irq_set_type(irq, type);
     if ( res )
             return -1;
 
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index e877334..778e79a 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -47,6 +47,7 @@ void arch_move_irqs(struct vcpu *v);
 /* Set IRQ type for an SPI */
 int irq_set_spi_type(unsigned int spi, unsigned int type);
 
+int irq_set_type(unsigned int irq, unsigned int type);
 int platform_get_irq(const struct dt_device_node *device, int index);
 
 void irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_mask);
-- 
1.7.10.4

[PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

[Ian Campbell]

These properties are defined in ePAPR and the OpenFirmware PCI Bus Binding
Specification (IEEE Std 1275-1994).

This replaces the xgene specific mapping. Tested on Mustang and on a model with
a PCI virtio controller.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/domain_build.c          |  193 ++++++++++++++++++++++++++++++++++
 xen/arch/arm/platforms/xgene-storm.c |   90 ----------------
 xen/include/xen/device_tree.h        |   32 ++++++
 3 files changed, 225 insertions(+), 90 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 998b6fd..00b5e07 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -907,6 +907,191 @@ static int make_timer_node(const struct domain *d, void *fdt,
     return res;
 }
 
+static int map_pci_device_ranges(struct domain *d,
+                                 const struct dt_device_node *dev,
+                                 const struct dt_pci_ranges_entry *ranges,
+                                 const u32 len)
+{
+    int i, nr, res;
+
+    if ( len % sizeof(struct dt_pci_ranges_entry) )
+    {
+        printk(XENLOG_ERR
+               "%s: ranges property length is not a multiple of entry size\n",
+               dt_node_name(dev));
+        return -EINVAL;
+    }
+
+    nr = len / sizeof(*ranges);
+
+    for ( i = 0; i < nr ; i++ )
+    {
+        const struct dt_pci_ranges_entry *range = &ranges[i];
+        u64 addr, len;
+
+        addr = fdt64_to_cpu(range->cpu_addr);
+        len = fdt64_to_cpu(range->length);
+
+        DPRINT("%s: ranges[%d]: 0x%08x.%08x.%08x 0x%"PRIx64" size 0x%"PRIx64"\n",
+               dt_node_name(dev), i,
+               fdt32_to_cpu(range->pci_addr.hi),
+               fdt32_to_cpu(range->pci_addr.mid),
+               fdt32_to_cpu(range->pci_addr.lo),
+               addr, len);
+
+        res = map_mmio_regions(d, paddr_to_pfn(addr & PAGE_MASK),
+                                  DIV_ROUND_UP(len, PAGE_SIZE),
+                                  paddr_to_pfn(addr & PAGE_MASK));
+        if ( res < 0 )
+        {
+            printk(XENLOG_ERR "%s: ranges[%d]: "
+                   "Unable to map 0x%"PRIx64" - 0x%"PRIx64" in domain %d\n",
+                   dt_node_name(dev), i,
+                   addr & PAGE_MASK, PAGE_ALIGN(addr + len) - 1,
+                   d->domain_id);
+            return res;
+        }
+    }
+    return 0;
+}
+
+static int map_device_ranges(struct domain *d, const struct dt_device_node *dev)
+{
+    const void *ranges;
+    u32 len;
+
+    if ( !dev->type )
+        return 0; /* No device type, nothing to do */
+
+    ranges = dt_get_property(dev, "ranges", &len);
+    if ( !ranges )
+        return 0; /* No ranges, nothing to do */
+
+    if ( dt_device_type_is_equal(dev, "pci") )
+        return map_pci_device_ranges(d, dev, ranges, len);
+    else if ( dt_device_type_is_equal(dev, "<NULL>") )
+        return 0; /* "<NULL>" is used for none */
+
+    DPRINT("Cannot handle ranges property for non-PCI device %s type %s\n",
+           dt_node_name(dev), dev->type);
+
+    /* Non-critical. */
+    return 0;
+}
+
+static int map_pci_device_interrupts(struct domain *d,
+                                     const struct dt_device_node *dev,
+                                     const __be32 *cells, const u32 len)
+{
+    const int intc_addr_cells =
+        dt_n_addr_cells_children(dt_interrupt_controller);
+    const size_t entry_size =
+        sizeof(struct dt_interrupt_map_entry) +
+        (intc_addr_cells + DT_NR_GIC_INTERRUPT_CELLS)*sizeof(__be32);
+
+    int i, nr, res;
+    u64 parent_addr;
+
+    if ( len % entry_size )
+    {
+        printk(XENLOG_ERR
+               "%s: interrupts property length is not a multiple of entry size %zd\n",
+               dt_node_name(dev), entry_size);
+        return -EINVAL;
+    }
+
+    nr = len / entry_size;
+
+    for ( i = 0; i < nr ; i++ )
+    {
+        const struct dt_interrupt_map_entry *map = (void *)cells;
+        struct dt_raw_irq dt_raw_irq;
+        struct dt_irq dt_irq;
+        int j;
+
+        if ( fdt32_to_cpu(map->phandle) != dt_interrupt_controller->phandle )
+        {
+            printk("%s: interrupt-map[%d]: Not connected to primary controller.\n",
+                   dt_node_name(dev), i);
+            continue;
+        }
+
+        dt_raw_irq.controller = dt_interrupt_controller;
+        dt_raw_irq.size = DT_NR_GIC_INTERRUPT_CELLS;
+
+        cells = &map->irq_specifier[0];
+
+        parent_addr = dt_next_cell(intc_addr_cells, &cells);
+        for (j = 0; j < DT_NR_GIC_INTERRUPT_CELLS; j++)
+            dt_raw_irq.specifier[j] = dt_next_cell(1, &cells);
+
+        res = dt_irq_translate(&dt_raw_irq, &dt_irq);
+        if ( res < 0 )
+        {
+            printk(XENLOG_ERR
+                   "%s: interrupt-map[%d]: Unable to translate raw IRQ\n",
+                   dt_node_name(dev), i);
+            continue;
+        }
+
+        DPRINT("%s: interrupt-map[%d]: 0x%"PRIx32".%"PRIx32".%"PRIx32" "
+               "pin=%"PRId32" phandle=%"PRIx32" "
+               "paddr=%"PRIx64" type=%"PRIx32" irq=%x\n",
+               dt_node_name(dev), i,
+               fdt32_to_cpu(map->pci_irq_mask.pci_addr.hi),
+               fdt32_to_cpu(map->pci_irq_mask.pci_addr.mid),
+               fdt32_to_cpu(map->pci_irq_mask.pci_addr.lo),
+               fdt32_to_cpu(map->pci_irq_mask.pci_pin),
+               fdt32_to_cpu(map->phandle),
+               parent_addr, dt_irq.type, dt_irq.irq);
+
+        /* Setup the IRQ type */
+        res = irq_set_type(dt_irq.irq, dt_irq.type);
+        if ( res )
+        {
+            printk(XENLOG_ERR
+                   "%s: interrupt-map[%d]: Unable to setup IRQ%"PRId32" to dom%d\n",
+                   dt_node_name(dev), i, dt_irq.irq, d->domain_id);
+            return res;
+        }
+
+        res = route_irq_to_guest(d, dt_irq.irq, dt_node_name(dev));
+        if ( res < 0 )
+        {
+            printk(XENLOG_ERR
+                   "%s: interrupt-map[%d]: Unable to map IRQ%"PRId32" to dom%d\n",
+                   dt_node_name(dev), i, dt_irq.irq, d->domain_id);
+            return res;
+        }
+    }
+    return 0;
+}
+
+static int map_device_interrupts(struct domain *d, const struct dt_device_node *dev)
+{
+    const void *map;
+    u32 len;
+
+    if ( !dev->type )
+        return 0; /* No device type, nothing to do */
+
+    map = dt_get_property(dev, "interrupt-map", &len);
+    if ( !map ) /* nothing to do */
+        return 0;
+
+    if ( dt_device_type_is_equal(dev, "pci") )
+        return map_pci_device_interrupts(d, dev, map, len);
+    else if ( dt_device_type_is_equal(dev, "<NULL>") )
+        return 0; /* "<NULL>" is used for none */
+
+    printk("Cannot handle interrupt-map for non-PCI device %s type %s\n",
+           dt_node_name(dev), dev->type);
+
+    /* Non-critical. */
+    return 0;
+}
+
+
 /* Map the device in the domain */
 static int map_device(struct domain *d, struct dt_device_node *dev)
 {
@@ -1015,6 +1200,14 @@ static int map_device(struct domain *d, struct dt_device_node *dev)
         }
     }
 
+    res = map_device_ranges(d, dev);
+    if ( res )
+        return res;
+
+    res = map_device_interrupts(d, dev);
+    if ( res )
+        return res;
+
     return 0;
 }
 
diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
index 29c4752..f88d306 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -40,95 +40,6 @@ static uint32_t xgene_storm_quirks(void)
     return PLATFORM_QUIRK_GIC_64K_STRIDE|PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI;
 }
 
-static int map_one_mmio(struct domain *d, const char *what,
-                         unsigned long start, unsigned long end)
-{
-    int ret;
-
-    printk("Additional MMIO %"PRIpaddr"-%"PRIpaddr" (%s)\n",
-           start, end, what);
-    ret = map_mmio_regions(d, start, end - start + 1, start);
-    if ( ret )
-        printk("Failed to map %s @ %"PRIpaddr" to dom%d\n",
-               what, start, d->domain_id);
-    return ret;
-}
-
-static int map_one_spi(struct domain *d, const char *what,
-                       unsigned int spi, unsigned int type)
-{
-    unsigned int irq;
-    int ret;
-
-    irq = spi + 32; /* SPIs start at IRQ 32 */
-
-    ret = irq_set_spi_type(irq, type);
-    if ( ret )
-    {
-        printk("Failed to set the type for IRQ%u\n", irq);
-        return ret;
-    }
-
-    printk("Additional IRQ %u (%s)\n", irq, what);
-
-    ret = route_irq_to_guest(d, irq, what);
-    if ( ret )
-        printk("Failed to route %s to dom%d\n", what, d->domain_id);
-
-    return ret;
-}
-
-/*
- * Xen does not currently support mapping MMIO regions and interrupt
- * for bus child devices (referenced via the "ranges" and
- * "interrupt-map" properties to domain 0). Instead for now map the
- * necessary resources manually.
- */
-static int xgene_storm_specific_mapping(struct domain *d)
-{
-    int ret;
-
-    /* Map the PCIe bus resources */
-    ret = map_one_mmio(d, "PCI MEM REGION", paddr_to_pfn(0xe000000000UL),
-                                            paddr_to_pfn(0xe010000000UL));
-    if ( ret )
-        goto err;
-
-    ret = map_one_mmio(d, "PCI IO REGION", paddr_to_pfn(0xe080000000UL),
-                                           paddr_to_pfn(0xe080010000UL));
-    if ( ret )
-        goto err;
-
-    ret = map_one_mmio(d, "PCI CFG REGION", paddr_to_pfn(0xe0d0000000UL),
-                                            paddr_to_pfn(0xe0d0200000UL));
-    if ( ret )
-        goto err;
-    ret = map_one_mmio(d, "PCI MSI REGION", paddr_to_pfn(0xe010000000UL),
-                                            paddr_to_pfn(0xe010800000UL));
-    if ( ret )
-        goto err;
-
-    ret = map_one_spi(d, "PCI#INTA", 0xc2, DT_IRQ_TYPE_LEVEL_HIGH);
-    if ( ret )
-        goto err;
-
-    ret = map_one_spi(d, "PCI#INTB", 0xc3, DT_IRQ_TYPE_LEVEL_HIGH);
-    if ( ret )
-        goto err;
-
-    ret = map_one_spi(d, "PCI#INTC", 0xc4, DT_IRQ_TYPE_LEVEL_HIGH);
-    if ( ret )
-        goto err;
-
-    ret = map_one_spi(d, "PCI#INTD", 0xc5, DT_IRQ_TYPE_LEVEL_HIGH);
-    if ( ret )
-        goto err;
-
-    ret = 0;
-err:
-    return ret;
-}
-
 static void xgene_storm_reset(void)
 {
     void __iomem *addr;
@@ -177,7 +88,6 @@ PLATFORM_START(xgene_storm, "APM X-GENE STORM")
     .init = xgene_storm_init,
     .reset = xgene_storm_reset,
     .quirks = xgene_storm_quirks,
-    .specific_mapping = xgene_storm_specific_mapping,
 
     .dom0_evtchn_ppi = 24,
     .dom0_gnttab_start = 0x1f800000,
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index ac2e876..d60ca94 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -682,6 +682,38 @@ int dt_parse_phandle_with_args(const struct dt_device_node *np,
                                const char *cells_name, int index,
                                struct dt_phandle_args *out_args);
 
+/*
+ * Definitions for implementing parts of the OpenFirmware PCI Bus Binding
+ * Specification (IEEE Std 1275-1994).
+ */
+
+struct dt_pci_unit_address {
+    __be32 hi, mid, lo;
+} __attribute__((packed));
+
+struct dt_pci_irq_mask {
+    struct dt_pci_unit_address    pci_addr;
+    __be32                        pci_pin;
+} __attribute__((packed));
+
+struct dt_pci_ranges_entry {
+    struct dt_pci_unit_address    pci_addr;
+    __be64                        cpu_addr;
+    __be64                        length;
+} __attribute__((packed));
+
+struct dt_interrupt_map_entry {
+    struct dt_pci_irq_mask             pci_irq_mask;
+    __be32                             phandle;
+    __be32			       irq_specifier[0];
+    /*
+     * irq_specifier is: parent-addr and interrupt-descriptor, both
+     * sized according to the interrupt-parent's #address-cells and
+     * #interrupt-cells.
+     */
+} __attribute__((packed));
+
+
 #endif /* __XEN_DEVICE_TREE_H */
 
 /*
-- 
1.7.10.4

Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

[Julien Grall]

Hi Ian,

On 24/10/14 10:58, Ian Campbell wrote:
> These properties are defined in ePAPR and the OpenFirmware PCI Bus Binding
> Specification (IEEE Std 1275-1994).
> 
> This replaces the xgene specific mapping. Tested on Mustang and on a model with
> a PCI virtio controller.

I'm wondering why you choose to map everything at Xen boot time rather
than implementing PHYSDEVOP_pci_device_add to do the job?

This would allow us to re-use most of the interrupts/mmio decoding
provided in the device tree library. It would also avoid missing support
of cascade ranges/interrupt-map.

Regards,

-- 
Julien Grall

Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

[Ian Campbell]

On Tue, 2015-02-17 at 17:33 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 24/10/14 10:58, Ian Campbell wrote:
> > These properties are defined in ePAPR and the OpenFirmware PCI Bus Binding
> > Specification (IEEE Std 1275-1994).
> > 
> > This replaces the xgene specific mapping. Tested on Mustang and on a model with
> > a PCI virtio controller.
> 
> I'm wondering why you choose to map everything at Xen boot time rather
> than implementing PHYSDEVOP_pci_device_add to do the job?

Does pci_device_add contain sufficient information to do so?

The regions which are being mapped are essentially the PCI host
controllers MMIO, IO and CFG "windows" which are then consumed by the
various bars of the devices on the bus.

So mapping based on pci_device_add would require us to go from the SBDF
to a set of BARS which need mapping, which is a whole lot more complex
than just mapping all of the resources owned by the root complex through
to the h/w domain.

Or perhaps I've misunderstood what you were suggesting?

> This would allow us to re-use most of the interrupts/mmio decoding
> provided in the device tree library. It would also avoid missing support
> of cascade ranges/interrupt-map.

I *think* (if I'm remembering right) I decided we don't need to worry
about cascades of these things because the second level resources are
all fully contained within the first (top level) one and so with the
approach I've taken here are all fully mapped already. That's why I made
this patch stop descending into children when such a "bus node" is
found.

Ian.

Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

[Julien Grall]

Hi Ian,

On 18/02/2015 13:50, Ian Campbell wrote:
> On Tue, 2015-02-17 at 17:33 +0000, Julien Grall wrote:
>> Hi Ian,
>>
>> On 24/10/14 10:58, Ian Campbell wrote:
>>> These properties are defined in ePAPR and the OpenFirmware PCI Bus Binding
>>> Specification (IEEE Std 1275-1994).
>>>
>>> This replaces the xgene specific mapping. Tested on Mustang and on a model with
>>> a PCI virtio controller.
>>
>> I'm wondering why you choose to map everything at Xen boot time rather
>> than implementing PHYSDEVOP_pci_device_add to do the job?
>
> Does pci_device_add contain sufficient information to do so?

Hmmm... for the interrupt the SBDF is enough. Although for the MMIO it 
looks like there is no difference between PCI bars.

> The regions which are being mapped are essentially the PCI host
> controllers MMIO, IO and CFG "windows" which are then consumed by the
> various bars of the devices on the bus.
>
> So mapping based on pci_device_add would require us to go from the SBDF
> to a set of BARS which need mapping, which is a whole lot more complex
> than just mapping all of the resources owned by the root complex through
> to the h/w domain.

I gave a look to the code which parse the host bridge resource (see 
of_pci_get_host_bridge_resources). They seem to re-use to the 
of_translate_* function. Would not it be possible to do the same?

> Or perhaps I've misunderstood what you were suggesting?

I was suggesting to do it via pci_add_device but it looks like it's only 
possible for IRQ not MMIO.

>> This would allow us to re-use most of the interrupts/mmio decoding
>> provided in the device tree library. It would also avoid missing support
>> of cascade ranges/interrupt-map.
>
> I *think* (if I'm remembering right) I decided we don't need to worry
> about cascades of these things because the second level resources are
> all fully contained within the first (top level) one and so with the
> approach I've taken here are all fully mapped already. That's why I made
> this patch stop descending into children when such a "bus node" is
> found.

I don't understand this paragraph, sorry.

The address range you decoded via the PCI bus may be an intermediate 
address which needs to be translated in the physical hardware address.

Regards,

-- 
Julien Grall

Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

[Ian Campbell]

On Wed, 2015-02-18 at 14:19 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 18/02/2015 13:50, Ian Campbell wrote:
> > On Tue, 2015-02-17 at 17:33 +0000, Julien Grall wrote:
> >> Hi Ian,
> >>
> >> On 24/10/14 10:58, Ian Campbell wrote:
> >>> These properties are defined in ePAPR and the OpenFirmware PCI Bus Binding
> >>> Specification (IEEE Std 1275-1994).
> >>>
> >>> This replaces the xgene specific mapping. Tested on Mustang and on a model with
> >>> a PCI virtio controller.
> >>
> >> I'm wondering why you choose to map everything at Xen boot time rather
> >> than implementing PHYSDEVOP_pci_device_add to do the job?
> >
> > Does pci_device_add contain sufficient information to do so?
> 
> Hmmm... for the interrupt the SBDF is enough. Although for the MMIO it 
> looks like there is no difference between PCI bars.
> 
> > The regions which are being mapped are essentially the PCI host
> > controllers MMIO, IO and CFG "windows" which are then consumed by the
> > various bars of the devices on the bus.
> >
> > So mapping based on pci_device_add would require us to go from the SBDF
> > to a set of BARS which need mapping, which is a whole lot more complex
> > than just mapping all of the resources owned by the root complex through
> > to the h/w domain.
> 
> I gave a look to the code which parse the host bridge resource (see 
> of_pci_get_host_bridge_resources). They seem to re-use to the 
> of_translate_* function. Would not it be possible to do the same?
> 
> > Or perhaps I've misunderstood what you were suggesting?
> 
> I was suggesting to do it via pci_add_device but it looks like it's only 
> possible for IRQ not MMIO.

I think so, and we probably should consider the two cases separately
since the right answer could reasonably differ for different resource
types.

I am reasonably convinced that for MMIO (+IO+CFG space) we should map
everything as described by the ranges property of the top most node, it
can be considered an analogue to / extension of the reg property of that
node.

For IRQ I'm not so sure, it's possible that routing the IRQ at
pci_add_device time might be better, or fit in better with e.g. the ACPI
architecture, but mapping everything described in interrupt-map at start
of day is also an option and a reasonably simple one, probably.

(My memory is fuzzy, but I think the concerns I had with this patch were
precisely to do with IRQs and how to parse the interrupt-map without a
specific SBDF in hand -- but only because the existing helper functions
assume an SBDF is present)

> >> This would allow us to re-use most of the interrupts/mmio decoding
> >> provided in the device tree library. It would also avoid missing support
> >> of cascade ranges/interrupt-map.
> >
> > I *think* (if I'm remembering right) I decided we don't need to worry
> > about cascades of these things because the second level resources are
> > all fully contained within the first (top level) one and so with the
> > approach I've taken here are all fully mapped already. That's why I made
> > this patch stop descending into children when such a "bus node" is
> > found.
> 
> I don't understand this paragraph, sorry.
> 
> The address range you decoded via the PCI bus may be an intermediate 
> address which needs to be translated in the physical hardware address.

This isn't to do with IPA->PA translations but to do with translations
between different PA addressing regimes. i.e. the different addressing
schemes of difference busses.

Lets say we have a system with a PCI-ROOT device exposing a PCI bus,
which in turn contains a PCI-BRIDGE which for the sake of argument lets
say is a PCI-FOOBUS bridge.

Lets just consider the MMIO hole for now, but IRQ is basically the same.

The ranges property on a node describes a mapping from a "parent"
address space into a "child" address space.

For PCI-ROOT "parent" is the host physical address space and "child" is
the PCI MMIO/IO/CFG address spaces.

For PCI-BRIDGE "parent" is the PCI-ROOT's child address space (i.e. PCI
MMIO/IO/CFG) and "child" is the FOOBUS address space.

The inputs ("parents") of the PCI-BRIDGE ranges property must therefore
by definition be valid outputs of the PCI-ROOT ranges property (i.e. be
"child" addresses).

Therefore if we map all of the input/parent ranges described by
PCI-ROOT's ranges property we do not need to recurse further and
consider PCI-BRIDGE's ranges property -- we've effectively already dealt
with it.

Does that make more sense?

Ian.

Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

[Julien Grall]

On 18/02/2015 14:37, Ian Campbell wrote:
> On Wed, 2015-02-18 at 14:19 +0000, Julien Grall wrote:
> I think so, and we probably should consider the two cases separately
> since the right answer could reasonably differ for different resource
> types.
>
> I am reasonably convinced that for MMIO (+IO+CFG space) we should map
> everything as described by the ranges property of the top most node, it
> can be considered an analogue to / extension of the reg property of that
> node.

Agreed.

> For IRQ I'm not so sure, it's possible that routing the IRQ at
> pci_add_device time might be better, or fit in better with e.g. the ACPI
> architecture, but mapping everything described in interrupt-map at start
> of day is also an option and a reasonably simple one, probably.

I agree that it's simple. Are we sure that we would be able to get a 
"better" solution later without modifying the kernel?

If not, we may need to keep this solution forever.

> This isn't to do with IPA->PA translations but to do with translations
> between different PA addressing regimes. i.e. the different addressing
> schemes of difference busses.

I meant bus address. The name "intermediate address" was misused, sorry.

> Lets say we have a system with a PCI-ROOT device exposing a PCI bus,
> which in turn contains a PCI-BRIDGE which for the sake of argument lets
> say is a PCI-FOOBUS bridge.

> Lets just consider the MMIO hole for now, but IRQ is basically the same.
>
> The ranges property on a node describes a mapping from a "parent"
> address space into a "child" address space.
>
> For PCI-ROOT "parent" is the host physical address space and "child" is
> the PCI MMIO/IO/CFG address spaces.
>
> For PCI-BRIDGE "parent" is the PCI-ROOT's child address space (i.e. PCI
> MMIO/IO/CFG) and "child" is the FOOBUS address space.
>
> The inputs ("parents") of the PCI-BRIDGE ranges property must therefore
> by definition be valid outputs of the PCI-ROOT ranges property (i.e. be
> "child" addresses).
>
> Therefore if we map all of the input/parent ranges described by
> PCI-ROOT's ranges property we do not need to recurse further and
> consider PCI-BRIDGE's ranges property -- we've effectively already dealt
> with it.
>
> Does that make more sense?

I'm still confused, what prevents the PCI-ROOT device to not be 
connected to another bus?

In device tree format, that would give something like:

/ {

   soc {
      ranges = "...";

      pcie {
        ranges = "...";
      }
   }
}

The address retrieved from the PCI-ROOT would be a bus address and not a 
physical address.

Regards,

-- 
Julien Grall

Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

[Julien Grall]

On 18/02/2015 15:05, Julien Grall wrote:
>
>
> On 18/02/2015 14:37, Ian Campbell wrote:
>> On Wed, 2015-02-18 at 14:19 +0000, Julien Grall wrote:
>> I think so, and we probably should consider the two cases separately
>> since the right answer could reasonably differ for different resource
>> types.
>>
>> I am reasonably convinced that for MMIO (+IO+CFG space) we should map
>> everything as described by the ranges property of the top most node, it
>> can be considered an analogue to / extension of the reg property of that
>> node.
>
> Agreed.
>
>> For IRQ I'm not so sure, it's possible that routing the IRQ at
>> pci_add_device time might be better, or fit in better with e.g. the ACPI
>> architecture, but mapping everything described in interrupt-map at start
>> of day is also an option and a reasonably simple one, probably.
>
> I agree that it's simple. Are we sure that we would be able to get a
> "better" solution later without modifying the kernel?
>
> If not, we may need to keep this solution forever.
>
>> This isn't to do with IPA->PA translations but to do with translations
>> between different PA addressing regimes. i.e. the different addressing
>> schemes of difference busses.
>
> I meant bus address. The name "intermediate address" was misused, sorry.
>
>> Lets say we have a system with a PCI-ROOT device exposing a PCI bus,
>> which in turn contains a PCI-BRIDGE which for the sake of argument lets
>> say is a PCI-FOOBUS bridge.
>
>> Lets just consider the MMIO hole for now, but IRQ is basically the same.
>>
>> The ranges property on a node describes a mapping from a "parent"
>> address space into a "child" address space.
>>
>> For PCI-ROOT "parent" is the host physical address space and "child" is
>> the PCI MMIO/IO/CFG address spaces.
>>
>> For PCI-BRIDGE "parent" is the PCI-ROOT's child address space (i.e. PCI
>> MMIO/IO/CFG) and "child" is the FOOBUS address space.
>>
>> The inputs ("parents") of the PCI-BRIDGE ranges property must therefore
>> by definition be valid outputs of the PCI-ROOT ranges property (i.e. be
>> "child" addresses).
>>
>> Therefore if we map all of the input/parent ranges described by
>> PCI-ROOT's ranges property we do not need to recurse further and
>> consider PCI-BRIDGE's ranges property -- we've effectively already dealt
>> with it.
>>
>> Does that make more sense?
>
> I'm still confused, what prevents the PCI-ROOT device to not be
> connected to another bus?
>
> In device tree format, that would give something like:
>
> / {
>
>    soc {
>       ranges = "...";
>
>       pcie {
>         ranges = "...";
>       }
>    }
> }


Actually the device tree of the x-gene board has something similar.

/ {
   soc {
     ranges;

     pcie {
       ranges = "...";
     }
}

"ranges;" means there is not translation necessary. But nothing prevent 
to have a the property "ranges" set.

Regards,

-- 
Julien Grall

Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

[Ian Campbell]

On Wed, 2015-02-18 at 15:16 +0000, Julien Grall wrote:
> "ranges;" means there is not translation necessary. But nothing prevent 
> to have a the property "ranges" set.

I don't suppose you know of a system where this translation is needed do
you? So I've got something to test with...

Ian.

Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

[Julien Grall]

Hi Ian,

On 05/03/2015 14:43, Ian Campbell wrote:
> On Wed, 2015-02-18 at 15:16 +0000, Julien Grall wrote:
>> "ranges;" means there is not translation necessary. But nothing prevent
>> to have a the property "ranges" set.
>
> I don't suppose you know of a system where this translation is needed do
> you? So I've got something to test with...

AFAIR, no. I guess, it should not be to difficult to tweak a device tree 
for testing.

Regards,

-- 
Julien Grall

Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

[Ian Campbell]

On Wed, 2015-02-18 at 15:05 +0000, Julien Grall wrote:
> 
> On 18/02/2015 14:37, Ian Campbell wrote:
> > On Wed, 2015-02-18 at 14:19 +0000, Julien Grall wrote:
> > I think so, and we probably should consider the two cases separately
> > since the right answer could reasonably differ for different resource
> > types.
> >
> > I am reasonably convinced that for MMIO (+IO+CFG space) we should map
> > everything as described by the ranges property of the top most node, it
> > can be considered an analogue to / extension of the reg property of that
> > node.
> 
> Agreed.
> 
> > For IRQ I'm not so sure, it's possible that routing the IRQ at
> > pci_add_device time might be better, or fit in better with e.g. the ACPI
> > architecture, but mapping everything described in interrupt-map at start
> > of day is also an option and a reasonably simple one, probably.
> 
> I agree that it's simple. Are we sure that we would be able to get a 
> "better" solution later without modifying the kernel?
> 
> If not, we may need to keep this solution forever.

True. I suppose feature flags would be one way out, but not a very
convenient one..

> > This isn't to do with IPA->PA translations but to do with translations
> > between different PA addressing regimes. i.e. the different addressing
> > schemes of difference busses.
> 
> I meant bus address. The name "intermediate address" was misused, sorry.
> 
> > Lets say we have a system with a PCI-ROOT device exposing a PCI bus,
> > which in turn contains a PCI-BRIDGE which for the sake of argument lets
> > say is a PCI-FOOBUS bridge.
> 
> > Lets just consider the MMIO hole for now, but IRQ is basically the same.
> >
> > The ranges property on a node describes a mapping from a "parent"
> > address space into a "child" address space.
> >
> > For PCI-ROOT "parent" is the host physical address space and "child" is
> > the PCI MMIO/IO/CFG address spaces.
> >
> > For PCI-BRIDGE "parent" is the PCI-ROOT's child address space (i.e. PCI
> > MMIO/IO/CFG) and "child" is the FOOBUS address space.
> >
> > The inputs ("parents") of the PCI-BRIDGE ranges property must therefore
> > by definition be valid outputs of the PCI-ROOT ranges property (i.e. be
> > "child" addresses).
> >
> > Therefore if we map all of the input/parent ranges described by
> > PCI-ROOT's ranges property we do not need to recurse further and
> > consider PCI-BRIDGE's ranges property -- we've effectively already dealt
> > with it.
> >
> > Does that make more sense?
> 
> I'm still confused, what prevents the PCI-ROOT device to not be 
> connected to another bus?
>
> In device tree format, that would give something like:
> 
> / {
> 
>    soc {
>       ranges = "...";
> 
>       pcie {
>         ranges = "...";
>       }
>    }
> }
> 
> The address retrieved from the PCI-ROOT would be a bus address and not a 
> physical address.

Hrm, nothing, I see what you are getting at now.

Either soc has a device_type property which we understand, in which case
we would handle it and stop recursing or (more likely for an soc) it
does not, in which case we would handle the pcie ranges property, but it
needs to be translated through the ranges property of soc, which the
patch doesn't do and probably it should.

Ian.

Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

[Julien Grall]

On 18/02/2015 15:18, Ian Campbell wrote:
> On Wed, 2015-02-18 at 15:05 +0000, Julien Grall wrote:
>>
>> On 18/02/2015 14:37, Ian Campbell wrote:
>>> On Wed, 2015-02-18 at 14:19 +0000, Julien Grall wrote:
>>> I think so, and we probably should consider the two cases separately
>>> since the right answer could reasonably differ for different resource
>>> types.
>>>
>>> I am reasonably convinced that for MMIO (+IO+CFG space) we should map
>>> everything as described by the ranges property of the top most node, it
>>> can be considered an analogue to / extension of the reg property of that
>>> node.
>>
>> Agreed.
>>
>>> For IRQ I'm not so sure, it's possible that routing the IRQ at
>>> pci_add_device time might be better, or fit in better with e.g. the ACPI
>>> architecture, but mapping everything described in interrupt-map at start
>>> of day is also an option and a reasonably simple one, probably.
>>
>> I agree that it's simple. Are we sure that we would be able to get a
>> "better" solution later without modifying the kernel?
>>
>> If not, we may need to keep this solution forever.
>
> True. I suppose feature flags would be one way out, but not a very
> convenient one..
>
>>> This isn't to do with IPA->PA translations but to do with translations
>>> between different PA addressing regimes. i.e. the different addressing
>>> schemes of difference busses.
>>
>> I meant bus address. The name "intermediate address" was misused, sorry.
>>
>>> Lets say we have a system with a PCI-ROOT device exposing a PCI bus,
>>> which in turn contains a PCI-BRIDGE which for the sake of argument lets
>>> say is a PCI-FOOBUS bridge.
>>
>>> Lets just consider the MMIO hole for now, but IRQ is basically the same.
>>>
>>> The ranges property on a node describes a mapping from a "parent"
>>> address space into a "child" address space.
>>>
>>> For PCI-ROOT "parent" is the host physical address space and "child" is
>>> the PCI MMIO/IO/CFG address spaces.
>>>
>>> For PCI-BRIDGE "parent" is the PCI-ROOT's child address space (i.e. PCI
>>> MMIO/IO/CFG) and "child" is the FOOBUS address space.
>>>
>>> The inputs ("parents") of the PCI-BRIDGE ranges property must therefore
>>> by definition be valid outputs of the PCI-ROOT ranges property (i.e. be
>>> "child" addresses).
>>>
>>> Therefore if we map all of the input/parent ranges described by
>>> PCI-ROOT's ranges property we do not need to recurse further and
>>> consider PCI-BRIDGE's ranges property -- we've effectively already dealt
>>> with it.
>>>
>>> Does that make more sense?
>>
>> I'm still confused, what prevents the PCI-ROOT device to not be
>> connected to another bus?
>>
>> In device tree format, that would give something like:
>>
>> / {
>>
>>     soc {
>>        ranges = "...";
>>
>>        pcie {
>>          ranges = "...";
>>        }
>>     }
>> }
>>
>> The address retrieved from the PCI-ROOT would be a bus address and not a
>> physical address.
>
> Hrm, nothing, I see what you are getting at now.
>
> Either soc has a device_type property which we understand, in which case
> we would handle it and stop recursing or (more likely for an soc) it
> does not, in which case we would handle the pcie ranges property, but it
> needs to be translated through the ranges property of soc, which the
> patch doesn't do and probably it should.

The code to do it is quite complicate and hard to maintain (actually 
it's a copy of the Linux one). It would be good if you can re-use the 
functions to translate in common/device_tree.c.

I think we may have the same problem for interrupts too.

Regards,

-- 
Julien Grall

Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

[Ian Campbell]

On Wed, 2015-02-18 at 15:31 +0000, Julien Grall wrote:

> > Either soc has a device_type property which we understand, in which case
> > we would handle it and stop recursing or (more likely for an soc) it
> > does not, in which case we would handle the pcie ranges property, but it
> > needs to be translated through the ranges property of soc, which the
> > patch doesn't do and probably it should.
> 
> The code to do it is quite complicate and hard to maintain (actually 
> it's a copy of the Linux one). It would be good if you can re-use the 
> functions to translate in common/device_tree.c.

Of course.

> I think we may have the same problem for interrupts too.

Yes. In that case ISTR the existing functions needed an SBDF, so weren't
quite right for the "up front mapping" approach -- but that can surely
be fixed via some refactoring.

Ian.

Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

[Julien Grall]

On 18/02/2015 14:37, Ian Campbell wrote:
> I am reasonably convinced that for MMIO (+IO+CFG space) we should map
> everything as described by the ranges property of the top most node, it
> can be considered an analogue to / extension of the reg property of that
> node.

BTW, the CFG space is part of the "reg" property, which is already 
mapped. The "ranges" property only covers the IO/MMIO BARs.

-- 
Julien Grall

Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

[Ian Campbell]

On Wed, 2015-02-18 at 15:13 +0000, Julien Grall wrote:
> 
> On 18/02/2015 14:37, Ian Campbell wrote:
> > I am reasonably convinced that for MMIO (+IO+CFG space) we should map
> > everything as described by the ranges property of the top most node, it
> > can be considered an analogue to / extension of the reg property of that
> > node.
> 
> BTW, the CFG space is part of the "reg" property, which is already 
> mapped. The "ranges" property only covers the IO/MMIO BARs.

Right, I keep forgetting...

Ian.

Re: [PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[Suravee Suthikulpanit]

On 10/24/2014 04:58 AM, Ian Campbell wrote:
> This message has been archived. View the original item
> <http://ausev2.amd.com/EnterpriseVault/ViewMessage.asp?VaultId=18C5C586ACEF6BE42BD5DBA644C96C94F1110000amdvault.amd.com&SavesetId=201411267046116~201410241000000000~Z~F141820FA2B2A99F4BA9EAD48EEB5211>
> This series adds parsing of the DT ranges and interrupt-map properties
> for PCI devices, these contain the MMIOs and IRQs used by children on
> the bus. This replaces the specific mapping on xgene which should also
> mean that it works on xgene devices other than mustang which use a
> different PCI root controller (the xgene has several, we only map the
> first).
>
> I've tested on Mustang and on a FastModel with virtio-pci based rootfs.
>
> This is *not* for 4.5.
>
> Ian.
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Ian,

I have tested this on Seattle w/ the ToT Xen-4.6 unstable branch (commit 
001324547356af86875fad5003f679571a6b8f1c), and I have PCI working in 
Dom0. Are you planning on committing this series at some point?

Thanks,

Surave

Re: [PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[Julien Grall]

Hi Suravee,

On 16/02/15 03:49, Suravee Suthikulpanit wrote:
> On 10/24/2014 04:58 AM, Ian Campbell wrote:
>> This message has been archived. View the original item
>> <http://ausev2.amd.com/EnterpriseVault/ViewMessage.asp?VaultId=18C5C586ACEF6BE42BD5DBA644C96C94F1110000amdvault.amd.com&SavesetId=201411267046116~201410241000000000~Z~F141820FA2B2A99F4BA9EAD48EEB5211>
>>
>> This series adds parsing of the DT ranges and interrupt-map properties
>> for PCI devices, these contain the MMIOs and IRQs used by children on
>> the bus. This replaces the specific mapping on xgene which should also
>> mean that it works on xgene devices other than mustang which use a
>> different PCI root controller (the xgene has several, we only map the
>> first).
>>
>> I've tested on Mustang and on a FastModel with virtio-pci based rootfs.
>>
>> This is *not* for 4.5.
>>
>> Ian.
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
> 
> Ian,
> 
> I have tested this on Seattle w/ the ToT Xen-4.6 unstable branch (commit
> 001324547356af86875fad5003f679571a6b8f1c), and I have PCI working in
> Dom0. Are you planning on committing this series at some point?

I think the plan is to commit this series.

I will try to review it during the week.

Regards,

-- 
Julien Grall

Re: [PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[Julien Grall]

(CC Jan and Andrew)

Hi Suravee,

On 17/02/15 03:04, Suravee Suthikulanit wrote:
> By the way, looking at the output of "xl dmesg", I saw the following
> message:
> 
> (XEN) DOM0: PCI host bridge /smb/pcie@f0000000 ranges:
> (XEN) DOM0:    IO 0xefff0000..0xefffffff -> 0x00000000
> (XEN) DOM0:   MEM 0x40000000..0xbfffffff -> 0x40000000
> (XEN) DOM0:   MEM 0x100000000..0x7fffffffff -> 0x100000000
> (XEN) DOM0: pci-host-generic f0000000.pcie: PCI host bridge to bus 0000:00
> (XEN) DOM0: pci_bus 0000:00: root bus resource [bus 00-7f]
> (XEN) DOM0: pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
> (XEN) DOM0: pci_bus 0000:00: root bus resource [mem 0x40000000-0xbfffffff]
> (XEN) DOM0: pci_bus 0000:00: root bus resource [mem
> 0x100000000-0x7fffffffff]
> (XEN) DOM0: pci 0000:00:00.0: of_irq_parse_pci() failed with rc=-19
> (XEN) do_physdev_op 16 cmd=25: not implemented yet
> (XEN) do_physdev_op 16 cmd=15: not implemented yet
> (XEN) DOM0: pci 0000:00:00.0: Failed to add - passthrough or MSI/MSI-X
> might fail!
> (XEN) DOM0: pci 0000:00:02.0: of_irq_parse_pci() failed with rc=-19
> (XEN) do_physdev_op 16 cmd=15: not implemented yet
> (XEN) DOM0: pci 0000:00:02.0: Failed to add - passthrough or MSI/MSI-X
> might fail!
> (XEN) do_physdev_op 16 cmd=15: not implemented yet
> (XEN) DOM0: pci 0000:00:02.1: Failed to add - passthrough or MSI/MSI-X
> might fail!
> (XEN) do_physdev_op 16 cmd=15: not implemented yet
> (XEN) DOM0: pci 0000:01:00.0: Failed to add - passthrough or MSI/MSI-X
> might fail!
> (XEN) DOM0: pci 0000:01:00.1: of_irq_parse_pci() failed with rc=-22
> (XEN) do_physdev_op 16 cmd=15: not implemented yet
> (XEN) DOM0: pci 0000:01:00.1: Failed to add - passthrough or MSI/MSI-X
> might fail!
> 
> IIUC, This is because xen_add_device() failed, and it seems to be
> related to some hyper call not implemented. Not sure what is "cmd=15".
> Any ideas?

There is 2 commands not implemented in the log:
	* cmd 15: PHYSDEVOP_manage_pci_add
	* cmd 25: PHYSDEVOP_pci_device_add

Linux fallbacks on the former because the later is not implemented.

AFAICT, PHYSDEVOP_manage_pci_add should not be implemented for ARM
because it doesn't support segment. I suspect that it's kept for legacy
on older Xen x86. Maybe Jan or Andrew have more input on this?

Cheers,

-- 
Julien Grall

Re: [PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[Andrew Cooper]

On 17/02/15 13:43, Julien Grall wrote:
> (CC Jan and Andrew)
>
> Hi Suravee,
>
> On 17/02/15 03:04, Suravee Suthikulanit wrote:
>> By the way, looking at the output of "xl dmesg", I saw the following
>> message:
>>
>> (XEN) DOM0: PCI host bridge /smb/pcie@f0000000 ranges:
>> (XEN) DOM0:    IO 0xefff0000..0xefffffff -> 0x00000000
>> (XEN) DOM0:   MEM 0x40000000..0xbfffffff -> 0x40000000
>> (XEN) DOM0:   MEM 0x100000000..0x7fffffffff -> 0x100000000
>> (XEN) DOM0: pci-host-generic f0000000.pcie: PCI host bridge to bus 0000:00
>> (XEN) DOM0: pci_bus 0000:00: root bus resource [bus 00-7f]
>> (XEN) DOM0: pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
>> (XEN) DOM0: pci_bus 0000:00: root bus resource [mem 0x40000000-0xbfffffff]
>> (XEN) DOM0: pci_bus 0000:00: root bus resource [mem
>> 0x100000000-0x7fffffffff]
>> (XEN) DOM0: pci 0000:00:00.0: of_irq_parse_pci() failed with rc=-19
>> (XEN) do_physdev_op 16 cmd=25: not implemented yet
>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>> (XEN) DOM0: pci 0000:00:00.0: Failed to add - passthrough or MSI/MSI-X
>> might fail!
>> (XEN) DOM0: pci 0000:00:02.0: of_irq_parse_pci() failed with rc=-19
>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>> (XEN) DOM0: pci 0000:00:02.0: Failed to add - passthrough or MSI/MSI-X
>> might fail!
>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>> (XEN) DOM0: pci 0000:00:02.1: Failed to add - passthrough or MSI/MSI-X
>> might fail!
>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>> (XEN) DOM0: pci 0000:01:00.0: Failed to add - passthrough or MSI/MSI-X
>> might fail!
>> (XEN) DOM0: pci 0000:01:00.1: of_irq_parse_pci() failed with rc=-22
>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>> (XEN) DOM0: pci 0000:01:00.1: Failed to add - passthrough or MSI/MSI-X
>> might fail!
>>
>> IIUC, This is because xen_add_device() failed, and it seems to be
>> related to some hyper call not implemented. Not sure what is "cmd=15".
>> Any ideas?
> There is 2 commands not implemented in the log:
> 	* cmd 15: PHYSDEVOP_manage_pci_add
> 	* cmd 25: PHYSDEVOP_pci_device_add
>
> Linux fallbacks on the former because the later is not implemented.
>
> AFAICT, PHYSDEVOP_manage_pci_add should not be implemented for ARM
> because it doesn't support segment. I suspect that it's kept for legacy
> on older Xen x86. Maybe Jan or Andrew have more input on this?

It needs to be kept for backwards compatibility in x86.

All new code should use PHYSDEVOP_pci_device_add.

~Andrew

Re: [PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[Suravee Suthikulanit]

On 2/17/2015 7:50 AM, Andrew Cooper wrote:
> On 17/02/15 13:43, Julien Grall wrote:
>> (CC Jan and Andrew)
>>
>> Hi Suravee,
>>
>> On 17/02/15 03:04, Suravee Suthikulanit wrote:
>>> By the way, looking at the output of "xl dmesg", I saw the following
>>> message:
>>>
>>> (XEN) DOM0: PCI host bridge /smb/pcie@f0000000 ranges:
>>> (XEN) DOM0:    IO 0xefff0000..0xefffffff -> 0x00000000
>>> (XEN) DOM0:   MEM 0x40000000..0xbfffffff -> 0x40000000
>>> (XEN) DOM0:   MEM 0x100000000..0x7fffffffff -> 0x100000000
>>> (XEN) DOM0: pci-host-generic f0000000.pcie: PCI host bridge to bus 0000:00
>>> (XEN) DOM0: pci_bus 0000:00: root bus resource [bus 00-7f]
>>> (XEN) DOM0: pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
>>> (XEN) DOM0: pci_bus 0000:00: root bus resource [mem 0x40000000-0xbfffffff]
>>> (XEN) DOM0: pci_bus 0000:00: root bus resource [mem
>>> 0x100000000-0x7fffffffff]
>>> (XEN) DOM0: pci 0000:00:00.0: of_irq_parse_pci() failed with rc=-19
>>> (XEN) do_physdev_op 16 cmd=25: not implemented yet
>>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>>> (XEN) DOM0: pci 0000:00:00.0: Failed to add - passthrough or MSI/MSI-X
>>> might fail!
>>> (XEN) DOM0: pci 0000:00:02.0: of_irq_parse_pci() failed with rc=-19
>>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>>> (XEN) DOM0: pci 0000:00:02.0: Failed to add - passthrough or MSI/MSI-X
>>> might fail!
>>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>>> (XEN) DOM0: pci 0000:00:02.1: Failed to add - passthrough or MSI/MSI-X
>>> might fail!
>>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>>> (XEN) DOM0: pci 0000:01:00.0: Failed to add - passthrough or MSI/MSI-X
>>> might fail!
>>> (XEN) DOM0: pci 0000:01:00.1: of_irq_parse_pci() failed with rc=-22
>>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>>> (XEN) DOM0: pci 0000:01:00.1: Failed to add - passthrough or MSI/MSI-X
>>> might fail!
>>>
>>> IIUC, This is because xen_add_device() failed, and it seems to be
>>> related to some hyper call not implemented. Not sure what is "cmd=15".
>>> Any ideas?
>> There is 2 commands not implemented in the log:
>> 	* cmd 15: PHYSDEVOP_manage_pci_add
>> 	* cmd 25: PHYSDEVOP_pci_device_add
>>
>> Linux fallbacks on the former because the later is not implemented.
>>
>> AFAICT, PHYSDEVOP_manage_pci_add should not be implemented for ARM
>> because it doesn't support segment. I suspect that it's kept for legacy
>> on older Xen x86. Maybe Jan or Andrew have more input on this?
>
> It needs to be kept for backwards compatibility in x86.
>
> All new code should use PHYSDEVOP_pci_device_add.
>
> ~Andrew
>

Ok, now that I look at the arch/arm/physdev.c, I don't think the code 
for supporting any of the PHYSDEVOP_xxx is there.  That's probably why 
Xen complains. In contrast, arch/x86/physdev.c has most PHYSDEVOP_xxx 
already supported.

My question is, are we supposed to be adding code to put the support in 
here?

Thanks,

Suravee.

Re: [PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[Suravee Suthikulanit]

On 2/17/2015 4:35 PM, Suravee Suthikulanit wrote:
> On 2/17/2015 7:50 AM, Andrew Cooper wrote:
>> On 17/02/15 13:43, Julien Grall wrote:
>>> (CC Jan and Andrew)
>>>
>>> Hi Suravee,
>>>
>>> On 17/02/15 03:04, Suravee Suthikulanit wrote:
>>>> By the way, looking at the output of "xl dmesg", I saw the following
>>>> message:
>>>>
>>>> (XEN) DOM0: PCI host bridge /smb/pcie@f0000000 ranges:
>>>> (XEN) DOM0:    IO 0xefff0000..0xefffffff -> 0x00000000
>>>> (XEN) DOM0:   MEM 0x40000000..0xbfffffff -> 0x40000000
>>>> (XEN) DOM0:   MEM 0x100000000..0x7fffffffff -> 0x100000000
>>>> (XEN) DOM0: pci-host-generic f0000000.pcie: PCI host bridge to bus
>>>> 0000:00
>>>> (XEN) DOM0: pci_bus 0000:00: root bus resource [bus 00-7f]
>>>> (XEN) DOM0: pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
>>>> (XEN) DOM0: pci_bus 0000:00: root bus resource [mem
>>>> 0x40000000-0xbfffffff]
>>>> (XEN) DOM0: pci_bus 0000:00: root bus resource [mem
>>>> 0x100000000-0x7fffffffff]
>>>> (XEN) DOM0: pci 0000:00:00.0: of_irq_parse_pci() failed with rc=-19
>>>> (XEN) do_physdev_op 16 cmd=25: not implemented yet
>>>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>>>> (XEN) DOM0: pci 0000:00:00.0: Failed to add - passthrough or MSI/MSI-X
>>>> might fail!
>>>> (XEN) DOM0: pci 0000:00:02.0: of_irq_parse_pci() failed with rc=-19
>>>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>>>> (XEN) DOM0: pci 0000:00:02.0: Failed to add - passthrough or MSI/MSI-X
>>>> might fail!
>>>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>>>> (XEN) DOM0: pci 0000:00:02.1: Failed to add - passthrough or MSI/MSI-X
>>>> might fail!
>>>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>>>> (XEN) DOM0: pci 0000:01:00.0: Failed to add - passthrough or MSI/MSI-X
>>>> might fail!
>>>> (XEN) DOM0: pci 0000:01:00.1: of_irq_parse_pci() failed with rc=-22
>>>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>>>> (XEN) DOM0: pci 0000:01:00.1: Failed to add - passthrough or MSI/MSI-X
>>>> might fail!
>>>>
>>>> IIUC, This is because xen_add_device() failed, and it seems to be
>>>> related to some hyper call not implemented. Not sure what is "cmd=15".
>>>> Any ideas?
>>> There is 2 commands not implemented in the log:
>>>     * cmd 15: PHYSDEVOP_manage_pci_add
>>>     * cmd 25: PHYSDEVOP_pci_device_add
>>>
>>> Linux fallbacks on the former because the later is not implemented.
>>>
>>> AFAICT, PHYSDEVOP_manage_pci_add should not be implemented for ARM
>>> because it doesn't support segment. I suspect that it's kept for legacy
>>> on older Xen x86. Maybe Jan or Andrew have more input on this?
>>
>> It needs to be kept for backwards compatibility in x86.
>>
>> All new code should use PHYSDEVOP_pci_device_add.
>>
>> ~Andrew
>>
>
> Ok, now that I look at the arch/arm/physdev.c, I don't think the code
> for supporting any of the PHYSDEVOP_xxx is there.  That's probably why
> Xen complains. In contrast, arch/x86/physdev.c has most PHYSDEVOP_xxx
> already supported.
>
> My question is, are we supposed to be adding code to put the support in
> here?
>
> Thanks,
>
> Suravee.

My guess is yes, and that would mean we need to enable building 
drivers/pci.c when building arm code, which then open up a can of worms 
with re-factoring MSI support code from x86 and etc.

Suravee

Re: [PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[Suravee Suthikulanit]

On 2/17/2015 6:31 PM, Suravee Suthikulanit wrote:
> On 2/17/2015 4:35 PM, Suravee Suthikulanit wrote:
>> On 2/17/2015 7:50 AM, Andrew Cooper wrote:
>>> On 17/02/15 13:43, Julien Grall wrote:
>>>> (CC Jan and Andrew)
>>>>
>>>> Hi Suravee,
>>>>
>>>> On 17/02/15 03:04, Suravee Suthikulanit wrote:
>>>>> By the way, looking at the output of "xl dmesg", I saw the following
>>>>> message:
>>>>>
>>>>> (XEN) DOM0: PCI host bridge /smb/pcie@f0000000 ranges:
>>>>> (XEN) DOM0:    IO 0xefff0000..0xefffffff -> 0x00000000
>>>>> (XEN) DOM0:   MEM 0x40000000..0xbfffffff -> 0x40000000
>>>>> (XEN) DOM0:   MEM 0x100000000..0x7fffffffff -> 0x100000000
>>>>> (XEN) DOM0: pci-host-generic f0000000.pcie: PCI host bridge to bus
>>>>> 0000:00
>>>>> (XEN) DOM0: pci_bus 0000:00: root bus resource [bus 00-7f]
>>>>> (XEN) DOM0: pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
>>>>> (XEN) DOM0: pci_bus 0000:00: root bus resource [mem
>>>>> 0x40000000-0xbfffffff]
>>>>> (XEN) DOM0: pci_bus 0000:00: root bus resource [mem
>>>>> 0x100000000-0x7fffffffff]
>>>>> (XEN) DOM0: pci 0000:00:00.0: of_irq_parse_pci() failed with rc=-19
>>>>> (XEN) do_physdev_op 16 cmd=25: not implemented yet
>>>>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>>>>> (XEN) DOM0: pci 0000:00:00.0: Failed to add - passthrough or MSI/MSI-X
>>>>> might fail!
>>>>> (XEN) DOM0: pci 0000:00:02.0: of_irq_parse_pci() failed with rc=-19
>>>>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>>>>> (XEN) DOM0: pci 0000:00:02.0: Failed to add - passthrough or MSI/MSI-X
>>>>> might fail!
>>>>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>>>>> (XEN) DOM0: pci 0000:00:02.1: Failed to add - passthrough or MSI/MSI-X
>>>>> might fail!
>>>>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>>>>> (XEN) DOM0: pci 0000:01:00.0: Failed to add - passthrough or MSI/MSI-X
>>>>> might fail!
>>>>> (XEN) DOM0: pci 0000:01:00.1: of_irq_parse_pci() failed with rc=-22
>>>>> (XEN) do_physdev_op 16 cmd=15: not implemented yet
>>>>> (XEN) DOM0: pci 0000:01:00.1: Failed to add - passthrough or MSI/MSI-X
>>>>> might fail!
>>>>>
>>>>> IIUC, This is because xen_add_device() failed, and it seems to be
>>>>> related to some hyper call not implemented. Not sure what is "cmd=15".
>>>>> Any ideas?
>>>> There is 2 commands not implemented in the log:
>>>>     * cmd 15: PHYSDEVOP_manage_pci_add
>>>>     * cmd 25: PHYSDEVOP_pci_device_add
>>>>
>>>> Linux fallbacks on the former because the later is not implemented.
>>>>
>>>> AFAICT, PHYSDEVOP_manage_pci_add should not be implemented for ARM
>>>> because it doesn't support segment. I suspect that it's kept for legacy
>>>> on older Xen x86. Maybe Jan or Andrew have more input on this?
>>>
>>> It needs to be kept for backwards compatibility in x86.
>>>
>>> All new code should use PHYSDEVOP_pci_device_add.
>>>
>>> ~Andrew
>>>
>>
>> Ok, now that I look at the arch/arm/physdev.c, I don't think the code
>> for supporting any of the PHYSDEVOP_xxx is there.  That's probably why
>> Xen complains. In contrast, arch/x86/physdev.c has most PHYSDEVOP_xxx
>> already supported.
>>
>> My question is, are we supposed to be adding code to put the support in
>> here?
>>
>> Thanks,
>>
>> Suravee.
>
> My guess is yes, and that would mean we need to enable building
> drivers/pci.c when building arm code, which then open up a can of worms
> with re-factoring MSI support code from x86 and etc.
>
> Suravee

Actually, that seems to be more related to the PCI pass-through devices. 
Isn't the Cavium guys already done that work to support their PCI device 
pass-through?

Anyways, at this point, I am able to generated Dom0 device tree with 
correct v2m node, and I can see Dom0 gicv2m driver probing and 
initializing correctly as it would on bare-metal.

# Snippet from /sys/firmware/fdt showing dom0 GIC node
	interrupt-controller {
		compatible = "arm,gic-400", "arm,cortex-a15-gic";
		#interrupt-cells = <0x3>;
		interrupt-controller;
		reg = <0x0 0xe1110000 0x0 0x1000 0x0 0xe112f000 0x0 0x2000>;
		phandle = <0x1>;
		#address-cells = <0x2>;
		#size-cells = <0x2>;
		ranges = <0x0 0x0 0x0 0xe1100000 0x0 0x100000>;

		v2m {
			compatible = "arm,gic-v2m-frame";
			msi-controller;
			arm,msi-base-spi = <0x40>;
			arm,msi-num-spis = <0x100>;
			phandle = <0x5>;
			reg = <0x0 0x80000 0x0 0x1000>;
		};
	};

linux:~ # dmesg | grep v2m
[    0.000000] GICv2m: Overriding V2M MSI_TYPER (base:64, num:256)
[    0.000000] GICv2m: Node v2m: range[0xe1180000:0xe1180fff], SPI[64:320]

So, during setting up v2m in hypervisor, I also call 
route_irq_to_guest() for the all SPIs used for MSI (i.e. 64-320 on 
Seattle), which will force the MSIs to Dom0. However, we would need to 
figure out how to detach and re-route certain interrupts to a specific 
DomU in case of passing through PCI devices in the future.

So, here's what I got:

linux:~ # cat /proc/interrupts
            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 

   0:          0          0          0          0          0          0 
   xen-dyn-event     xenbus
   3:      19872      19872      19870      19861      19866      20034 
       GIC  27  arch_timer
   4:         91          1          1          1          1          1 
       GIC  31  events
  13:       2397          0          0          0          0          0 
       GIC 387  e0300000.sata
  14:          0          0          0          0          0          0 
       GIC 389  e1000000.i2c
  15:          0          0          0          0          0          0 
       GIC 362  pl022
  16:          0          0          0          0          0          0 
       GIC 361  pl022
  46:         90          0          0          0          0          0 
  xen-percpu-virq      hvc_console
  47:        200          0          0          0          0          0 
       MSI 524288  enp1s0f0-TxRx-0
  48:         56          0          0          0          0          0 
       MSI 524289  enp1s0f0-TxRx-1
  49:         47          0          0          0          0          0 
       MSI 524290  enp1s0f0-TxRx-2
  50:         45          0          0          0          0          0 
       MSI 524291  enp1s0f0-TxRx-3
  51:         53          0          0          0          0          0 
       MSI 524292  enp1s0f0-TxRx-4
  52:         48          0          0          0          0          0 
       MSI 524293  enp1s0f0-TxRx-5
  53:          4          0          0          0          0          0 
       MSI 524294  enp1s0f0

linux:~ # ifconfig enp1s0f0
enp1s0f0  Link encap:Ethernet  HWaddr 00:1B:21:55:7F:14
           inet addr:10.236.19.5  Bcast:10.236.19.255  Mask:255.255.254.0
           inet6 addr: fe80::21b:21ff:fe55:7f14/64 Scope:Link
           UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
           RX packets:333 errors:0 dropped:0 overruns:0 frame:0
           TX packets:111 errors:0 dropped:0 overruns:0 carrier:0
           collisions:0 txqueuelen:1000
           RX bytes:33406 (32.6 Kb)  TX bytes:14957 (14.6 Kb)

linux:~ # lspci -vvv -s 1:0.0
01:00.0 Ethernet controller: Intel Corporation 82599ES 10-Gigabit 
SFI/SFP+ Network Connection (rev 01)
         Subsystem: Intel Corporation Ethernet Server Adapter X520-2
         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- 
ParErr- Stepping- SERR- FastB2B- DisINTx+
         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- 
<TAbort- <MAbort- >SERR- <PERR- INTx-
         Latency: 0
         Interrupt: pin A routed to IRQ 44
         Region 0: Memory at bfe80000 (64-bit, prefetchable) [size=512K]
         Region 2: I/O ports at 0020 [size=32]
         Region 4: Memory at bff04000 (64-bit, prefetchable) [size=16K]
         Capabilities: [40] Power Management version 3
                 Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA 
PME(D0+,D1-,D2-,D3hot+,D3cold-)
                 Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=1 PME-
         Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
                 Address: 0000000000000000  Data: 0000
                 Masking: 00000000  Pending: 00000000
         Capabilities: [70] MSI-X: Enable+ Count=64 Masked-
                 Vector table: BAR=4 offset=00000000
                 PBA: BAR=4 offset=00002000
....

And there you have it.... GICv2m MSI(-X) supports in Dom 0 for Seattle 
;)  Thanks to Ian's PCI patch, which makes porting much simpler.

Next, I'll clean up the code and send out Xen patch for review. I'll 
also push Linux changes (for adding ARM64 PCI Generic host controller 
supports and MSI IRQ domain from Marc) into my Linux tree on Github. 
Then you could give it a try on your Seattle box.

Thanks,

Suravee

Re: [PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[Julien Grall]

Hi Suravee,

On 18/02/2015 05:28, Suravee Suthikulanit wrote:
>
> Actually, that seems to be more related to the PCI pass-through devices.
> Isn't the Cavium guys already done that work to support their PCI device
> pass-through?

They were working on it, but so far there is no patch series on the ML. 
It would be nice to come with a common solution (i.e between GICv2m and 
GICv3 ITS) for MSI.

> Anyways, at this point, I am able to generated Dom0 device tree with
> correct v2m node, and I can see Dom0 gicv2m driver probing and
> initializing correctly as it would on bare-metal.
>
> # Snippet from /sys/firmware/fdt showing dom0 GIC node
>      interrupt-controller {
>          compatible = "arm,gic-400", "arm,cortex-a15-gic";
>          #interrupt-cells = <0x3>;
>          interrupt-controller;
>          reg = <0x0 0xe1110000 0x0 0x1000 0x0 0xe112f000 0x0 0x2000>;
>          phandle = <0x1>;
>          #address-cells = <0x2>;
>          #size-cells = <0x2>;
>          ranges = <0x0 0x0 0x0 0xe1100000 0x0 0x100000>;
>
>          v2m {
>              compatible = "arm,gic-v2m-frame";
>              msi-controller;
>              arm,msi-base-spi = <0x40>;
>              arm,msi-num-spis = <0x100>;
>              phandle = <0x5>;
>              reg = <0x0 0x80000 0x0 0x1000>;
>          };
>      };
>
> linux:~ # dmesg | grep v2m
> [    0.000000] GICv2m: Overriding V2M MSI_TYPER (base:64, num:256)
> [    0.000000] GICv2m: Node v2m: range[0xe1180000:0xe1180fff], SPI[64:320]
>
> So, during setting up v2m in hypervisor, I also call
> route_irq_to_guest() for the all SPIs used for MSI (i.e. 64-320 on
> Seattle), which will force the MSIs to Dom0. However, we would need to
> figure out how to detach and re-route certain interrupts to a specific
> DomU in case of passing through PCI devices in the future.

Who decide to assign the MSI n to the SPI x? DOM0 or Xen?

Wouldn't it be possible to route the SPI dynamically when the domain 
decide to use the MSI n? We would need to implement PHYSDEVOP_map_pirq 
for MSI.

[..]

> And there you have it.... GICv2m MSI(-X) supports in Dom 0 for Seattle
> ;)  Thanks to Ian's PCI patch, which makes porting much simpler.
>
> Next, I'll clean up the code and send out Xen patch for review. I'll
> also push Linux changes (for adding ARM64 PCI Generic host controller
> supports and MSI IRQ domain from Marc) into my Linux tree on Github.
> Then you could give it a try on your Seattle box.

Congrats!

-- 
Julien Grall

Re: [PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[Suravee Suthikulanit]

On 2/18/2015 6:48 AM, Julien Grall wrote:
> Hi Suravee,
>
> On 18/02/2015 05:28, Suravee Suthikulanit wrote:
>>
>> Actually, that seems to be more related to the PCI pass-through devices.
>> Isn't the Cavium guys already done that work to support their PCI device
>> pass-through?
>
> They were working on it, but so far there is no patch series on the ML.
> It would be nice to come with a common solution (i.e between GICv2m and
> GICv3 ITS) for MSI.

Agree, although for supporting Dom0 MSI, I don't see anything that could 
be shared since this would totally be two separate drivers/interfaces.

>
>> Anyways, at this point, I am able to generated Dom0 device tree with
>> correct v2m node, and I can see Dom0 gicv2m driver probing and
>> initializing correctly as it would on bare-metal.
>>
>> # Snippet from /sys/firmware/fdt showing dom0 GIC node
>>      interrupt-controller {
>>          compatible = "arm,gic-400", "arm,cortex-a15-gic";
>>          #interrupt-cells = <0x3>;
>>          interrupt-controller;
>>          reg = <0x0 0xe1110000 0x0 0x1000 0x0 0xe112f000 0x0 0x2000>;
>>          phandle = <0x1>;
>>          #address-cells = <0x2>;
>>          #size-cells = <0x2>;
>>          ranges = <0x0 0x0 0x0 0xe1100000 0x0 0x100000>;
>>
>>          v2m {
>>              compatible = "arm,gic-v2m-frame";
>>              msi-controller;
>>              arm,msi-base-spi = <0x40>;
>>              arm,msi-num-spis = <0x100>;
>>              phandle = <0x5>;
>>              reg = <0x0 0x80000 0x0 0x1000>;
>>          };
>>      };
>>
>> linux:~ # dmesg | grep v2m
>> [    0.000000] GICv2m: Overriding V2M MSI_TYPER (base:64, num:256)
>> [    0.000000] GICv2m: Node v2m: range[0xe1180000:0xe1180fff],
>> SPI[64:320]
>>
>> So, during setting up v2m in hypervisor, I also call
>> route_irq_to_guest() for the all SPIs used for MSI (i.e. 64-320 on
>> Seattle), which will force the MSIs to Dom0. However, we would need to
>> figure out how to detach and re-route certain interrupts to a specific
>> DomU in case of passing through PCI devices in the future.
>
> Who decide to assign the MSI n to the SPI x? DOM0 or Xen?

For v2m, each MSI is tied to a specific SPI. The range of SPIs is 
specified in the GIC V2M_MSI_TYPER register. In Xen, we need to make 
sure that these are routed to Dom0 initially since Dom0 GICv2m driver is 
the one handling all MSI assignments.

> Wouldn't it be possible to route the SPI dynamically when the domain
> decide to use the MSI n? We would need to implement PHYSDEVOP_map_pirq
> for MSI.

Enabling MSI is done by each end-point PCI device drivers in the guest. 
In Linux, this would mean that when the driver tries to allocate an MSI 
interrupt, it would need to communicate back to Xen (possibly via 
hypercall as you pointed out) to get the next available SPI. It is not 
necessary for now. I am planning to revisit this when we try to 
implement pass-through support. Lemme know if you think this should be 
handled differently.

Cheers,

Suravee

Re: [PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[Manish]

On 19/02/15 1:43 am, Suravee Suthikulanit wrote:
> On 2/18/2015 6:48 AM, Julien Grall wrote:
>> Hi Suravee,
>>
>> On 18/02/2015 05:28, Suravee Suthikulanit wrote:
>>>
>>> Actually, that seems to be more related to the PCI pass-through 
>>> devices.
>>> Isn't the Cavium guys already done that work to support their PCI 
>>> device
>>> pass-through?
>>
>> They were working on it, but so far there is no patch series on the ML.
>> It would be nice to come with a common solution (i.e between GICv2m and
>> GICv3 ITS) for MSI.
>
> Agree, although for supporting Dom0 MSI, I don't see anything that 
> could be shared since this would totally be two separate 
> drivers/interfaces.
>
>>
>>> Anyways, at this point, I am able to generated Dom0 device tree with
>>> correct v2m node, and I can see Dom0 gicv2m driver probing and
>>> initializing correctly as it would on bare-metal.
>>>
>>> # Snippet from /sys/firmware/fdt showing dom0 GIC node
>>>      interrupt-controller {
>>>          compatible = "arm,gic-400", "arm,cortex-a15-gic";
>>>          #interrupt-cells = <0x3>;
>>>          interrupt-controller;
>>>          reg = <0x0 0xe1110000 0x0 0x1000 0x0 0xe112f000 0x0 0x2000>;
>>>          phandle = <0x1>;
>>>          #address-cells = <0x2>;
>>>          #size-cells = <0x2>;
>>>          ranges = <0x0 0x0 0x0 0xe1100000 0x0 0x100000>;
>>>
>>>          v2m {
>>>              compatible = "arm,gic-v2m-frame";
>>>              msi-controller;
>>>              arm,msi-base-spi = <0x40>;
>>>              arm,msi-num-spis = <0x100>;
>>>              phandle = <0x5>;
>>>              reg = <0x0 0x80000 0x0 0x1000>;
>>>          };
>>>      };
>>>
>>> linux:~ # dmesg | grep v2m
>>> [    0.000000] GICv2m: Overriding V2M MSI_TYPER (base:64, num:256)
>>> [    0.000000] GICv2m: Node v2m: range[0xe1180000:0xe1180fff],
>>> SPI[64:320]
>>>
>>> So, during setting up v2m in hypervisor, I also call
>>> route_irq_to_guest() for the all SPIs used for MSI (i.e. 64-320 on
>>> Seattle), which will force the MSIs to Dom0. However, we would need to
>>> figure out how to detach and re-route certain interrupts to a specific
>>> DomU in case of passing through PCI devices in the future.
>>
>> Who decide to assign the MSI n to the SPI x? DOM0 or Xen?
>
> For v2m, each MSI is tied to a specific SPI. The range of SPIs is 
> specified in the GIC V2M_MSI_TYPER register. In Xen, we need to make 
> sure that these are routed to Dom0 initially since Dom0 GICv2m driver 
> is the one handling all MSI assignments.
>
>> Wouldn't it be possible to route the SPI dynamically when the domain
>> decide to use the MSI n? We would need to implement PHYSDEVOP_map_pirq
>> for MSI.
>
> Enabling MSI is done by each end-point PCI device drivers in the 
> guest. In Linux, this would mean that when the driver tries to 
> allocate an MSI interrupt, it would need to communicate back to Xen 
> (possibly via hypercall as you pointed out) to get the next available 
> SPI. It is not necessary for now. I am planning to revisit this when 
> we try to implement pass-through support. Lemme know if you think this 
> should be handled differently.
>
For cavium we are doing it differently. In the frontend bus we have 
added its. I am planning to send RFC patches soon.
> Cheers,
>
> Suravee
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[Jan Beulich]

>>> On 18.02.15 at 21:13, <suravee.suthikulpanit@amd.com> wrote:
> On 2/18/2015 6:48 AM, Julien Grall wrote:
>> Wouldn't it be possible to route the SPI dynamically when the domain
>> decide to use the MSI n? We would need to implement PHYSDEVOP_map_pirq
>> for MSI.
> 
> Enabling MSI is done by each end-point PCI device drivers in the guest. 
> In Linux, this would mean that when the driver tries to allocate an MSI 
> interrupt, it would need to communicate back to Xen (possibly via 
> hypercall as you pointed out) to get the next available SPI. It is not 
> necessary for now. I am planning to revisit this when we try to 
> implement pass-through support. Lemme know if you think this should be 
> handled differently.

I think guest MSI setup should work at least similarly (if identically
is not possible) to x86, i.e. via the vpci interface.

Jan

Re: [PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[Manish]

On 19/02/15 1:44 pm, Jan Beulich wrote:
>>>> On 18.02.15 at 21:13, <suravee.suthikulpanit@amd.com> wrote:
>> On 2/18/2015 6:48 AM, Julien Grall wrote:
>>> Wouldn't it be possible to route the SPI dynamically when the domain
>>> decide to use the MSI n? We would need to implement PHYSDEVOP_map_pirq
>>> for MSI.
>> Enabling MSI is done by each end-point PCI device drivers in the guest.
>> In Linux, this would mean that when the driver tries to allocate an MSI
>> interrupt, it would need to communicate back to Xen (possibly via
>> hypercall as you pointed out) to get the next available SPI. It is not
>> necessary for now. I am planning to revisit this when we try to
>> implement pass-through support. Lemme know if you think this should be
>> handled differently.
> I think guest MSI setup should work at least similarly (if identically
> is not possible) to x86, i.e. via the vpci interface.
For cavium thunder pci passthrough, we have avoided that. Guest driver 
directly traps into hypervisor. No front end backend communication for 
MSI setup. The front-back communication is limited to reading PCI 
configuration space.
> Jan
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[Julien Grall]

Hi Suravee,

On 18/02/15 20:13, Suravee Suthikulanit wrote:
>> Who decide to assign the MSI n to the SPI x? DOM0 or Xen?
> 
> For v2m, each MSI is tied to a specific SPI. The range of SPIs is
> specified in the GIC V2M_MSI_TYPER register. In Xen, we need to make
> sure that these are routed to Dom0 initially since Dom0 GICv2m driver is
> the one handling all MSI assignments.

I guess you emulate the GICv2m frame? If so, can't you trap access to
the frame in order to configure the SPI?

>> Wouldn't it be possible to route the SPI dynamically when the domain
>> decide to use the MSI n? We would need to implement PHYSDEVOP_map_pirq
>> for MSI.
> 
> Enabling MSI is done by each end-point PCI device drivers in the guest.
> In Linux, this would mean that when the driver tries to allocate an MSI
> interrupt, it would need to communicate back to Xen (possibly via
> hypercall as you pointed out) to get the next available SPI. It is not
> necessary for now. I am planning to revisit this when we try to
> implement pass-through support. Lemme know if you think this should be
> handled differently.

The pass-through support will come quickly. I think it would be better
to support this case now, mainly if we have to add some code in Linux.
It may take some times to upstream it.

Although, I haven't really though about how to do it in Xen.

Regards,

-- 
Julien Grall

Re: [PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[Jan Beulich]

>>> On 18.02.15 at 01:31, <suravee.suthikulpanit@amd.com> wrote:
> On 2/17/2015 4:35 PM, Suravee Suthikulanit wrote:
>> Ok, now that I look at the arch/arm/physdev.c, I don't think the code
>> for supporting any of the PHYSDEVOP_xxx is there.  That's probably why
>> Xen complains. In contrast, arch/x86/physdev.c has most PHYSDEVOP_xxx
>> already supported.
>>
>> My question is, are we supposed to be adding code to put the support in
>> here?
> 
> My guess is yes, and that would mean we need to enable building 
> drivers/pci.c when building arm code, which then open up a can of worms 
> with re-factoring MSI support code from x86 and etc.

As you said later, that's pass-through related, i.e. not immediately
needed. Right now - to get rid of those messages - you'd have to
simply stub out the respective physdevop-s as much as is necessary
to no longer emit log messages and no longer return an error on
valid input.

Jan

Re: [PATCH for-4.6 0/5] xen: arm: Parse PCI DT nodes' ranges and interrupt-map

[Ian Campbell]

On Sun, 2015-02-15 at 21:49 -0600, Suravee Suthikulpanit wrote:
> On 10/24/2014 04:58 AM, Ian Campbell wrote:
> > This message has been archived. View the original item
> > <http://ausev2.amd.com/EnterpriseVault/ViewMessage.asp?VaultId=18C5C586ACEF6BE42BD5DBA644C96C94F1110000amdvault.amd.com&SavesetId=201411267046116~201410241000000000~Z~F141820FA2B2A99F4BA9EAD48EEB5211>
> > This series adds parsing of the DT ranges and interrupt-map properties
> > for PCI devices, these contain the MMIOs and IRQs used by children on
> > the bus. This replaces the specific mapping on xgene which should also
> > mean that it works on xgene devices other than mustang which use a
> > different PCI root controller (the xgene has several, we only map the
> > first).
> >
> > I've tested on Mustang and on a FastModel with virtio-pci based rootfs.
> >
> > This is *not* for 4.5.
> >
> > Ian.
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> 
> Ian,
> 
> I have tested this on Seattle w/ the ToT Xen-4.6 unstable branch (commit 
> 001324547356af86875fad5003f679571a6b8f1c), and I have PCI working in 
> Dom0. Are you planning on committing this series at some point?

At some point, yes.

I originally had some doubts that this was the correct approach, but I
a) can't remember what they were and b) think I've convinced myself this
is the right way to go afterall...

Since this is now in your way I'll bump revisiting it up my stack a bit.

Ian.