All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4 for-4.5] xen: arm: xgene bug fixes + support for McDivitt
@ 2014-11-18 16:44 Ian Campbell
  2014-11-18 16:44 ` [PATCH for-4.5 1/4] xen: arm: Add earlyprintk " Ian Campbell
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Ian Campbell @ 2014-11-18 16:44 UTC (permalink / raw
  To: xen-devel
  Cc: Julien Grall, Tim Deegan, Stefano Stabellini, Clark Laughlin,
	Pranavkumar Sawargaonkar

These patches:

      * fix up an off by one bug in the xgene mapping of additional PCI
        bus resources, which would cause an additional extra page to be
        mapped
      * correct the size of the mapped regions to match the docs
      * adds support for the other 4 PCI buses on the chip, which
        enables mcdivitt and presumably most other Xgene based platforms
        which uses PCI buses other than pcie0.
      * adds earlyprintk for the mcdivitt platform

McDivitt is the X-Gene based HP Moonshot cartridge (McDivitt is the code
name, I think the product is called m400, not quite sure).

Other than the bug fixes I'd like to see the mcdivitt support
(specifically the "other 4 PCI buses" one) in 4.5 because Moonshot is an
interesting and exciting platform for arm64. It is also being used for
ongoing work on Xen on ARM on Openstack in Linaro. The earlyprintk patch
is totally harmless unless it's explicitly enabled at compile time, IMHO
if we are taking the rest we may as well throw it in...

The risk here is that we break the existing support for the Mustang
platform, which would be the most likely failure case for the second
patch. I've tested these on a Mustang, including firing up a PCI NIC
device. The new mappings are a superset of the existing ones so the
potential for breakage should be quite small.

I've also successfully tested on a McDivitt.

Ian.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH for-4.5 1/4] xen: arm: Add earlyprintk for McDivitt.
  2014-11-18 16:44 [PATCH 0/4 for-4.5] xen: arm: xgene bug fixes + support for McDivitt Ian Campbell
@ 2014-11-18 16:44 ` Ian Campbell
  2014-11-18 16:59   ` Julien Grall
  2014-11-18 16:44 ` [PATCH for-4.5 2/4] xen: arm: correct off by one in xgene-storm's map_one_mmio Ian Campbell
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Ian Campbell @ 2014-11-18 16:44 UTC (permalink / raw
  To: xen-devel
  Cc: Ian Campbell, julien.grall, tim, Clark Laughlin,
	stefano.stabellini, Pranavkumar Sawargaonkar

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

diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index 572d854..ef887a5 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -95,6 +95,12 @@ EARLY_PRINTK_BAUD := 115200
 EARLY_UART_BASE_ADDRESS := 0x1c020000
 EARLY_UART_REG_SHIFT := 2
 endif
+ifeq ($(CONFIG_EARLY_PRINTK), xgene-mcdivitt)
+EARLY_PRINTK_INC := 8250
+EARLY_PRINTK_BAUD := 9600
+EARLY_UART_BASE_ADDRESS := 0x1c021000
+EARLY_UART_REG_SHIFT := 2
+endif
 ifeq ($(CONFIG_EARLY_PRINTK), juno)
 EARLY_PRINTK_INC := pl011
 EARLY_PRINTK_BAUD := 115200
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH for-4.5 2/4] xen: arm: correct off by one in xgene-storm's map_one_mmio
  2014-11-18 16:44 [PATCH 0/4 for-4.5] xen: arm: xgene bug fixes + support for McDivitt Ian Campbell
  2014-11-18 16:44 ` [PATCH for-4.5 1/4] xen: arm: Add earlyprintk " Ian Campbell
@ 2014-11-18 16:44 ` Ian Campbell
  2014-11-18 17:01   ` Julien Grall
  2014-11-19 21:11   ` Konrad Rzeszutek Wilk
  2014-11-18 16:44 ` [PATCH for-4.5 3/4] xen: arm: correct specific mappings for PCIE0 on X-Gene Ian Campbell
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 20+ messages in thread
From: Ian Campbell @ 2014-11-18 16:44 UTC (permalink / raw
  To: xen-devel
  Cc: Ian Campbell, julien.grall, tim, Clark Laughlin,
	stefano.stabellini, Pranavkumar Sawargaonkar

The callers pass the end as the pfn immediately *after* the last page to be
mapped, therefore adding one is incorrect and causes an additional page to be
mapped.

At the same time correct the printing of the mfn values, zero-padding them to
16 digits as for a paddr when they are frame numbers is just confusing.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/platforms/xgene-storm.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
index 29c4752..38674cd 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -45,9 +45,9 @@ static int map_one_mmio(struct domain *d, const char *what,
 {
     int ret;
 
-    printk("Additional MMIO %"PRIpaddr"-%"PRIpaddr" (%s)\n",
+    printk("Additional MMIO %lx-%lx (%s)\n",
            start, end, what);
-    ret = map_mmio_regions(d, start, end - start + 1, start);
+    ret = map_mmio_regions(d, start, end - start, start);
     if ( ret )
         printk("Failed to map %s @ %"PRIpaddr" to dom%d\n",
                what, start, d->domain_id);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH for-4.5 3/4] xen: arm: correct specific mappings for PCIE0 on X-Gene
  2014-11-18 16:44 [PATCH 0/4 for-4.5] xen: arm: xgene bug fixes + support for McDivitt Ian Campbell
  2014-11-18 16:44 ` [PATCH for-4.5 1/4] xen: arm: Add earlyprintk " Ian Campbell
  2014-11-18 16:44 ` [PATCH for-4.5 2/4] xen: arm: correct off by one in xgene-storm's map_one_mmio Ian Campbell
@ 2014-11-18 16:44 ` Ian Campbell
  2014-11-18 17:04   ` Julien Grall
  2014-11-18 16:44 ` [PATCH for-4.5 4/4] xen: arm: Support the other 4 PCI buses on Xgene Ian Campbell
  2014-11-18 16:51 ` [PATCH 0/4 for-4.5] xen: arm: xgene bug fixes + support for McDivitt Ian Campbell
  4 siblings, 1 reply; 20+ messages in thread
From: Ian Campbell @ 2014-11-18 16:44 UTC (permalink / raw
  To: xen-devel
  Cc: Ian Campbell, julien.grall, tim, Clark Laughlin,
	stefano.stabellini, Pranavkumar Sawargaonkar

The region assigned to PCIE0, according to the docs, is 0x0e000000000 to
0x10000000000. They make no distinction between PCI CFG and PCI IO mem within
this range (in fact, I'm not sure that isn't up to the driver).

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/platforms/xgene-storm.c |   18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
index 38674cd..6c432a1 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -89,22 +89,8 @@ 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));
+    ret = map_one_mmio(d, "PCI MEMORY", paddr_to_pfn(0x0e000000000UL),
+                                        paddr_to_pfn(0x01000000000UL));
     if ( ret )
         goto err;
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH for-4.5 4/4] xen: arm: Support the other 4 PCI buses on Xgene
  2014-11-18 16:44 [PATCH 0/4 for-4.5] xen: arm: xgene bug fixes + support for McDivitt Ian Campbell
                   ` (2 preceding siblings ...)
  2014-11-18 16:44 ` [PATCH for-4.5 3/4] xen: arm: correct specific mappings for PCIE0 on X-Gene Ian Campbell
@ 2014-11-18 16:44 ` Ian Campbell
  2014-11-18 17:15   ` Julien Grall
  2014-11-18 16:51 ` [PATCH 0/4 for-4.5] xen: arm: xgene bug fixes + support for McDivitt Ian Campbell
  4 siblings, 1 reply; 20+ messages in thread
From: Ian Campbell @ 2014-11-18 16:44 UTC (permalink / raw
  To: xen-devel
  Cc: Ian Campbell, julien.grall, tim, Clark Laughlin,
	stefano.stabellini, Pranavkumar Sawargaonkar

Currently we only establish specific mappings for pcie0, which is
used on the Mustang platform. However at least McDivitt uses pcie3.
So wire up all the others, based on whether the corresponding DT node
is marked as available.

This results in no change for Mustang.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/platforms/xgene-storm.c |   84 ++++++++++++++++++++++++++++------
 1 file changed, 71 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
index 6c432a1..926c325 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -78,35 +78,31 @@ static int map_one_spi(struct domain *d, const char *what,
     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)
+/* Creates MMIO mappings base..end as well as 4 SPIs from the given base. */
+static int xgene_storm_pcie_specific_mapping(struct domain *d,
+                                             paddr_t base, paddr_t end,
+                                             int base_spi)
 {
     int ret;
 
     /* Map the PCIe bus resources */
-    ret = map_one_mmio(d, "PCI MEMORY", paddr_to_pfn(0x0e000000000UL),
-                                        paddr_to_pfn(0x01000000000UL));
+    ret = map_one_mmio(d, "PCI MEMORY", paddr_to_pfn(base), paddr_to_pfn(end));
     if ( ret )
         goto err;
 
-    ret = map_one_spi(d, "PCI#INTA", 0xc2, DT_IRQ_TYPE_LEVEL_HIGH);
+    ret = map_one_spi(d, "PCI#INTA", base_spi+0, DT_IRQ_TYPE_LEVEL_HIGH);
     if ( ret )
         goto err;
 
-    ret = map_one_spi(d, "PCI#INTB", 0xc3, DT_IRQ_TYPE_LEVEL_HIGH);
+    ret = map_one_spi(d, "PCI#INTB", base_spi+1, DT_IRQ_TYPE_LEVEL_HIGH);
     if ( ret )
         goto err;
 
-    ret = map_one_spi(d, "PCI#INTC", 0xc4, DT_IRQ_TYPE_LEVEL_HIGH);
+    ret = map_one_spi(d, "PCI#INTC", base_spi+2, DT_IRQ_TYPE_LEVEL_HIGH);
     if ( ret )
         goto err;
 
-    ret = map_one_spi(d, "PCI#INTD", 0xc5, DT_IRQ_TYPE_LEVEL_HIGH);
+    ret = map_one_spi(d, "PCI#INTD", base_spi+3, DT_IRQ_TYPE_LEVEL_HIGH);
     if ( ret )
         goto err;
 
@@ -115,6 +111,68 @@ err:
     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)
+{
+    struct dt_device_node *node = NULL;
+    int ret;
+
+    while ( (node = dt_find_compatible_node(node, "pci", "apm,xgene-pcie")) )
+    {
+        u64 addr;
+
+        /* Identify the bus via it's control register address */
+        ret = dt_device_get_address(node, 0, &addr, NULL);
+        if ( ret < 0 )
+            return ret;
+
+        if ( !dt_device_is_available(node) )
+            continue;
+
+       switch ( addr )
+        {
+        case 0x1f2b0000: /* PCIe0 */
+            ret = xgene_storm_pcie_specific_mapping(d,
+                0x0e000000000UL, 0x10000000000UL, 0xc2);
+            break;
+        case 0x1f2c0000: /* PCIe1 */
+            ret = xgene_storm_pcie_specific_mapping(d,
+                0x0d000000000UL, 0x0e000000000UL, 0xc8);
+            break;
+        case 0x1f2d0000: /* PCIe2 */
+            ret = xgene_storm_pcie_specific_mapping(d,
+                0x09000000000UL, 0x0a000000000UL, 0xce);
+            break;
+        case 0x1f500000: /* PCIe3 */
+            ret = xgene_storm_pcie_specific_mapping(d,
+                0x0a000000000UL, 0x0c000000000UL, 0xd4);
+            break;
+        case 0x1f510000: /* PCIe4 */
+            ret = xgene_storm_pcie_specific_mapping(d,
+                0x0c000000000UL, 0x0d000000000UL, 0xda);
+            break;
+
+        default:
+            /* Ignore unknown PCI busses */
+            ret = 0;
+            break;
+        }
+
+        if ( ret < 0 )
+            return ret;
+
+        printk("Mapped additional regions for PCIe device at 0x%"PRIx64"\n",
+               addr);
+    }
+
+    return 0;
+}
+
 static void xgene_storm_reset(void)
 {
     void __iomem *addr;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/4 for-4.5] xen: arm: xgene bug fixes + support for McDivitt
  2014-11-18 16:44 [PATCH 0/4 for-4.5] xen: arm: xgene bug fixes + support for McDivitt Ian Campbell
                   ` (3 preceding siblings ...)
  2014-11-18 16:44 ` [PATCH for-4.5 4/4] xen: arm: Support the other 4 PCI buses on Xgene Ian Campbell
@ 2014-11-18 16:51 ` Ian Campbell
  2014-11-19 21:18   ` Konrad Rzeszutek Wilk
  4 siblings, 1 reply; 20+ messages in thread
From: Ian Campbell @ 2014-11-18 16:51 UTC (permalink / raw
  To: xen-devel
  Cc: Stefano Stabellini, Clark Laughlin, Julien Grall, Tim Deegan,
	Pranavkumar Sawargaonkar

On Tue, 2014-11-18 at 16:44 +0000, Ian Campbell wrote:
> These patches:

... which are also at
        git://xenbits.xen.org/people/ianc/xen.git mcdivitt-v1

Ian.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH for-4.5 1/4] xen: arm: Add earlyprintk for McDivitt.
  2014-11-18 16:44 ` [PATCH for-4.5 1/4] xen: arm: Add earlyprintk " Ian Campbell
@ 2014-11-18 16:59   ` Julien Grall
  2014-11-19  9:54     ` Ian Campbell
  0 siblings, 1 reply; 20+ messages in thread
From: Julien Grall @ 2014-11-18 16:59 UTC (permalink / raw
  To: Ian Campbell, xen-devel
  Cc: Pranavkumar Sawargaonkar, Clark Laughlin, tim, stefano.stabellini

Hi Ian,

On 11/18/2014 04:44 PM, Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/arch/arm/Rules.mk |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index 572d854..ef887a5 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -95,6 +95,12 @@ EARLY_PRINTK_BAUD := 115200
>  EARLY_UART_BASE_ADDRESS := 0x1c020000
>  EARLY_UART_REG_SHIFT := 2
>  endif
> +ifeq ($(CONFIG_EARLY_PRINTK), xgene-mcdivitt)
> +EARLY_PRINTK_INC := 8250
> +EARLY_PRINTK_BAUD := 9600

EARLY_PRINTK_BAUD is not necessary as we don't use the initialization
function (EARLY_PRINTK_INIT_UART is not set).

With the EARLY_PRINTK_BAUD dropped, this could be merged with the
xgene-storm  early printk (I didn't really understand why the baud rate
is different). But I don't think it's 4.5 material.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH for-4.5 2/4] xen: arm: correct off by one in xgene-storm's map_one_mmio
  2014-11-18 16:44 ` [PATCH for-4.5 2/4] xen: arm: correct off by one in xgene-storm's map_one_mmio Ian Campbell
@ 2014-11-18 17:01   ` Julien Grall
  2014-11-19  9:54     ` Ian Campbell
  2014-11-19 21:11   ` Konrad Rzeszutek Wilk
  1 sibling, 1 reply; 20+ messages in thread
From: Julien Grall @ 2014-11-18 17:01 UTC (permalink / raw
  To: Ian Campbell, xen-devel
  Cc: Pranavkumar Sawargaonkar, Clark Laughlin, tim, stefano.stabellini

Hi Ian,

On 11/18/2014 04:44 PM, Ian Campbell wrote:
> The callers pass the end as the pfn immediately *after* the last page to be
> mapped, therefore adding one is incorrect and causes an additional page to be
> mapped.
> 
> At the same time correct the printing of the mfn values, zero-padding them to
> 16 digits as for a paddr when they are frame numbers is just confusing.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/arch/arm/platforms/xgene-storm.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
> index 29c4752..38674cd 100644
> --- a/xen/arch/arm/platforms/xgene-storm.c
> +++ b/xen/arch/arm/platforms/xgene-storm.c
> @@ -45,9 +45,9 @@ static int map_one_mmio(struct domain *d, const char *what,
>  {
>      int ret;
>  
> -    printk("Additional MMIO %"PRIpaddr"-%"PRIpaddr" (%s)\n",
> +    printk("Additional MMIO %lx-%lx (%s)\n",
>             start, end, what);
> -    ret = map_mmio_regions(d, start, end - start + 1, start);
> +    ret = map_mmio_regions(d, start, end - start, start);
>      if ( ret )
>          printk("Failed to map %s @ %"PRIpaddr" to dom%d\n",
>                 what, start, d->domain_id);

As you fixed the previous printf format. I would fix this one too.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH for-4.5 3/4] xen: arm: correct specific mappings for PCIE0 on X-Gene
  2014-11-18 16:44 ` [PATCH for-4.5 3/4] xen: arm: correct specific mappings for PCIE0 on X-Gene Ian Campbell
@ 2014-11-18 17:04   ` Julien Grall
  0 siblings, 0 replies; 20+ messages in thread
From: Julien Grall @ 2014-11-18 17:04 UTC (permalink / raw
  To: Ian Campbell, xen-devel
  Cc: Pranavkumar Sawargaonkar, Clark Laughlin, tim, stefano.stabellini

Hi Ian,

On 11/18/2014 04:44 PM, Ian Campbell wrote:
> The region assigned to PCIE0, according to the docs, is 0x0e000000000 to
> 0x10000000000. They make no distinction between PCI CFG and PCI IO mem within
> this range (in fact, I'm not sure that isn't up to the driver).
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH for-4.5 4/4] xen: arm: Support the other 4 PCI buses on Xgene
  2014-11-18 16:44 ` [PATCH for-4.5 4/4] xen: arm: Support the other 4 PCI buses on Xgene Ian Campbell
@ 2014-11-18 17:15   ` Julien Grall
  2014-11-19  9:56     ` Ian Campbell
  0 siblings, 1 reply; 20+ messages in thread
From: Julien Grall @ 2014-11-18 17:15 UTC (permalink / raw
  To: Ian Campbell, xen-devel
  Cc: Pranavkumar Sawargaonkar, Clark Laughlin, tim, stefano.stabellini

Hi Ian,

On 11/18/2014 04:44 PM, Ian Campbell wrote:
> Currently we only establish specific mappings for pcie0, which is
> used on the Mustang platform. However at least McDivitt uses pcie3.
> So wire up all the others, based on whether the corresponding DT node
> is marked as available.
> 
> This results in no change for Mustang.

Hopefully, we will support PCI DT parsing in Xen 4.6!

> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> +/*
> + * 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)
> +{
> +    struct dt_device_node *node = NULL;

NIT: const struct

> +    int ret;
> +
> +    while ( (node = dt_find_compatible_node(node, "pci", "apm,xgene-pcie")) )
> +    {
> +        u64 addr;
> +
> +        /* Identify the bus via it's control register address */
> +        ret = dt_device_get_address(node, 0, &addr, NULL);
> +        if ( ret < 0 )
> +            return ret;
> +
> +        if ( !dt_device_is_available(node) )
> +            continue;
> +
> +       switch ( addr )
> +        {
> +        case 0x1f2b0000: /* PCIe0 */
> +            ret = xgene_storm_pcie_specific_mapping(d,
> +                0x0e000000000UL, 0x10000000000UL, 0xc2);
> +            break;
> +        case 0x1f2c0000: /* PCIe1 */
> +            ret = xgene_storm_pcie_specific_mapping(d,
> +                0x0d000000000UL, 0x0e000000000UL, 0xc8);
> +            break;
> +        case 0x1f2d0000: /* PCIe2 */
> +            ret = xgene_storm_pcie_specific_mapping(d,
> +                0x09000000000UL, 0x0a000000000UL, 0xce);
> +            break;
> +        case 0x1f500000: /* PCIe3 */
> +            ret = xgene_storm_pcie_specific_mapping(d,
> +                0x0a000000000UL, 0x0c000000000UL, 0xd4);
> +            break;
> +        case 0x1f510000: /* PCIe4 */
> +            ret = xgene_storm_pcie_specific_mapping(d,
> +                0x0c000000000UL, 0x0d000000000UL, 0xda);
> +            break;
> +
> +        default:
> +            /* Ignore unknown PCI busses */

I would add a
printk("Ignoring PCI busses %s\n", dt_node_full_name(dev));

> +            ret = 0;
> +            break;

continue? You can't assume the order of the PCI busses in the device tree.

> +        }
> +
> +        if ( ret < 0 )
> +            return ret;
> +
> +        printk("Mapped additional regions for PCIe device at 0x%"PRIx64"\n",
> +               addr);

Printing the device tree path would be more helpful than the address.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH for-4.5 1/4] xen: arm: Add earlyprintk for McDivitt.
  2014-11-18 16:59   ` Julien Grall
@ 2014-11-19  9:54     ` Ian Campbell
  2014-11-19 10:02       ` Julien Grall
  0 siblings, 1 reply; 20+ messages in thread
From: Ian Campbell @ 2014-11-19  9:54 UTC (permalink / raw
  To: Julien Grall
  Cc: stefano.stabellini, tim, xen-devel, Clark Laughlin,
	Pranavkumar Sawargaonkar

On Tue, 2014-11-18 at 16:59 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 11/18/2014 04:44 PM, Ian Campbell wrote:
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  xen/arch/arm/Rules.mk |    6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> > index 572d854..ef887a5 100644
> > --- a/xen/arch/arm/Rules.mk
> > +++ b/xen/arch/arm/Rules.mk
> > @@ -95,6 +95,12 @@ EARLY_PRINTK_BAUD := 115200
> >  EARLY_UART_BASE_ADDRESS := 0x1c020000
> >  EARLY_UART_REG_SHIFT := 2
> >  endif
> > +ifeq ($(CONFIG_EARLY_PRINTK), xgene-mcdivitt)
> > +EARLY_PRINTK_INC := 8250
> > +EARLY_PRINTK_BAUD := 9600
> 
> EARLY_PRINTK_BAUD is not necessary as we don't use the initialization
> function (EARLY_PRINTK_INIT_UART is not set).

Oh yes, oops. Also the baud is not even what is actually used, so it's
not even serving a documentary purpose.

> With the EARLY_PRINTK_BAUD dropped, this could be merged with the
> xgene-storm  early printk

It's at a different base address. Long term I either want to make this
(somewhat) runtime configurable or at least to rationalise the options
into the form <soc/soc-family>-uart<N>, or perhaps even <8250|pl011|
etc>@<address>[,<rate><settings>], if it's not to skanky to arrange to
parse that somewhere in the build system. Not for 4.5 though.

> (I didn't really understand why the baud rate
> is different).

Different hardware might potentially have different baud rates
configured in firmware which we would want to seemlessly follow, but
it's moot since the right thing to do in most cases is leave the
bootloader provided cfg alone.

> But I don't think it's 4.5 material.

You mean the patch generally or the merging?

Ian.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH for-4.5 2/4] xen: arm: correct off by one in xgene-storm's map_one_mmio
  2014-11-18 17:01   ` Julien Grall
@ 2014-11-19  9:54     ` Ian Campbell
  0 siblings, 0 replies; 20+ messages in thread
From: Ian Campbell @ 2014-11-19  9:54 UTC (permalink / raw
  To: Julien Grall
  Cc: Clark Laughlin, Pranavkumar Sawargaonkar, tim, stefano.stabellini,
	xen-devel

On Tue, 2014-11-18 at 17:01 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 11/18/2014 04:44 PM, Ian Campbell wrote:
> > The callers pass the end as the pfn immediately *after* the last page to be
> > mapped, therefore adding one is incorrect and causes an additional page to be
> > mapped.
> > 
> > At the same time correct the printing of the mfn values, zero-padding them to
> > 16 digits as for a paddr when they are frame numbers is just confusing.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  xen/arch/arm/platforms/xgene-storm.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
> > index 29c4752..38674cd 100644
> > --- a/xen/arch/arm/platforms/xgene-storm.c
> > +++ b/xen/arch/arm/platforms/xgene-storm.c
> > @@ -45,9 +45,9 @@ static int map_one_mmio(struct domain *d, const char *what,
> >  {
> >      int ret;
> >  
> > -    printk("Additional MMIO %"PRIpaddr"-%"PRIpaddr" (%s)\n",
> > +    printk("Additional MMIO %lx-%lx (%s)\n",
> >             start, end, what);
> > -    ret = map_mmio_regions(d, start, end - start + 1, start);
> > +    ret = map_mmio_regions(d, start, end - start, start);
> >      if ( ret )
> >          printk("Failed to map %s @ %"PRIpaddr" to dom%d\n",
> >                 what, start, d->domain_id);
> 
> As you fixed the previous printf format. I would fix this one too.

Yes, good idea.

Ian.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH for-4.5 4/4] xen: arm: Support the other 4 PCI buses on Xgene
  2014-11-18 17:15   ` Julien Grall
@ 2014-11-19  9:56     ` Ian Campbell
  2014-11-19 10:06       ` Julien Grall
  0 siblings, 1 reply; 20+ messages in thread
From: Ian Campbell @ 2014-11-19  9:56 UTC (permalink / raw
  To: Julien Grall
  Cc: Clark Laughlin, Pranavkumar Sawargaonkar, tim, stefano.stabellini,
	xen-devel

On Tue, 2014-11-18 at 17:15 +0000, Julien Grall wrote:
> > +        default:
> > +            /* Ignore unknown PCI busses */
> 
> I would add a
> printk("Ignoring PCI busses %s\n", dt_node_full_name(dev));
> 
> > +            ret = 0;
> > +            break;
> 
> continue?

Yes, that makes sense (probably the ret = is then unnecessary).

>  You can't assume the order of the PCI busses in the device tree.

But, I don't understand what this has to do with using continue.

> 
> > +        }
> > +
> > +        if ( ret < 0 )
> > +            return ret;
> > +
> > +        printk("Mapped additional regions for PCIe device at 0x%"PRIx64"\n",
> > +               addr);
> 
> Printing the device tree path would be more helpful than the address.

OK.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH for-4.5 1/4] xen: arm: Add earlyprintk for McDivitt.
  2014-11-19  9:54     ` Ian Campbell
@ 2014-11-19 10:02       ` Julien Grall
  0 siblings, 0 replies; 20+ messages in thread
From: Julien Grall @ 2014-11-19 10:02 UTC (permalink / raw
  To: Ian Campbell
  Cc: stefano.stabellini, tim, xen-devel, Clark Laughlin,
	Pranavkumar Sawargaonkar

Hi,

On 19/11/2014 09:54, Ian Campbell wrote:
> On Tue, 2014-11-18 at 16:59 +0000, Julien Grall wrote:
>> Hi Ian,
>>
>> On 11/18/2014 04:44 PM, Ian Campbell wrote:
>>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>>> ---
>>>   xen/arch/arm/Rules.mk |    6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
>>> index 572d854..ef887a5 100644
>>> --- a/xen/arch/arm/Rules.mk
>>> +++ b/xen/arch/arm/Rules.mk
>>> @@ -95,6 +95,12 @@ EARLY_PRINTK_BAUD := 115200
>>>   EARLY_UART_BASE_ADDRESS := 0x1c020000
>>>   EARLY_UART_REG_SHIFT := 2
>>>   endif
>>> +ifeq ($(CONFIG_EARLY_PRINTK), xgene-mcdivitt)
>>> +EARLY_PRINTK_INC := 8250
>>> +EARLY_PRINTK_BAUD := 9600
>>
>> EARLY_PRINTK_BAUD is not necessary as we don't use the initialization
>> function (EARLY_PRINTK_INIT_UART is not set).
>
> Oh yes, oops. Also the baud is not even what is actually used, so it's
> not even serving a documentary purpose.
>
>> With the EARLY_PRINTK_BAUD dropped, this could be merged with the
>> xgene-storm  early printk
>
> It's at a different base address. Long term I either want to make this
> (somewhat) runtime configurable or at least to rationalise the options
> into the form <soc/soc-family>-uart<N>, or perhaps even <8250|pl011|
> etc>@<address>[,<rate><settings>], if it's not to skanky to arrange to
> parse that somewhere in the build system. Not for 4.5 though.

> You mean the patch generally or the merging?

I meant rationalise the number of early printk. This patch looks fine 
for me.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH for-4.5 4/4] xen: arm: Support the other 4 PCI buses on Xgene
  2014-11-19  9:56     ` Ian Campbell
@ 2014-11-19 10:06       ` Julien Grall
  2014-11-19 10:18         ` Ian Campbell
  0 siblings, 1 reply; 20+ messages in thread
From: Julien Grall @ 2014-11-19 10:06 UTC (permalink / raw
  To: Ian Campbell
  Cc: Clark Laughlin, Pranavkumar Sawargaonkar, tim, stefano.stabellini,
	xen-devel

Hi Ian,

On 19/11/2014 09:56, Ian Campbell wrote:
> On Tue, 2014-11-18 at 17:15 +0000, Julien Grall wrote:
>>> +        default:
>>> +            /* Ignore unknown PCI busses */
>>
>> I would add a
>> printk("Ignoring PCI busses %s\n", dt_node_full_name(dev));
>>
>>> +            ret = 0;
>>> +            break;
>>
>> continue?
>
> Yes, that makes sense (probably the ret = is then unnecessary).
>
>>   You can't assume the order of the PCI busses in the device tree.
>
> But, I don't understand what this has to do with using continue.

The current xgene-storm DTS has the different PCI busses ordered. So as 
soon as you don't find the PCI range, it means there is no more PCI busses.

Without the continue, this patch gives the impression that you rely on 
the node order on the device tree.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH for-4.5 4/4] xen: arm: Support the other 4 PCI buses on Xgene
  2014-11-19 10:06       ` Julien Grall
@ 2014-11-19 10:18         ` Ian Campbell
  2014-11-19 10:30           ` Julien Grall
  0 siblings, 1 reply; 20+ messages in thread
From: Ian Campbell @ 2014-11-19 10:18 UTC (permalink / raw
  To: Julien Grall
  Cc: Clark Laughlin, Pranavkumar Sawargaonkar, tim, stefano.stabellini,
	xen-devel

On Wed, 2014-11-19 at 10:06 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 19/11/2014 09:56, Ian Campbell wrote:
> > On Tue, 2014-11-18 at 17:15 +0000, Julien Grall wrote:
> >>> +        default:
> >>> +            /* Ignore unknown PCI busses */
> >>
> >> I would add a
> >> printk("Ignoring PCI busses %s\n", dt_node_full_name(dev));
> >>
> >>> +            ret = 0;
> >>> +            break;
> >>
> >> continue?
> >
> > Yes, that makes sense (probably the ret = is then unnecessary).
> >
> >>   You can't assume the order of the PCI busses in the device tree.
> >
> > But, I don't understand what this has to do with using continue.
> 
> The current xgene-storm DTS has the different PCI busses ordered. So as 
> soon as you don't find the PCI range, it means there is no more PCI busses.

I don't think it does, the patch iterates over all of the buses, even
ones we don't understand, we don't give up at the first one we don't
grok.

> Without the continue, this patch gives the impression that you rely on 
> the node order on the device tree.



> 
> Regards,
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH for-4.5 4/4] xen: arm: Support the other 4 PCI buses on Xgene
  2014-11-19 10:18         ` Ian Campbell
@ 2014-11-19 10:30           ` Julien Grall
  0 siblings, 0 replies; 20+ messages in thread
From: Julien Grall @ 2014-11-19 10:30 UTC (permalink / raw
  To: Ian Campbell
  Cc: Clark Laughlin, Pranavkumar Sawargaonkar, tim, stefano.stabellini,
	xen-devel



On 19/11/2014 10:18, Ian Campbell wrote:
> On Wed, 2014-11-19 at 10:06 +0000, Julien Grall wrote:
>> Hi Ian,
>>
>> On 19/11/2014 09:56, Ian Campbell wrote:
>>> On Tue, 2014-11-18 at 17:15 +0000, Julien Grall wrote:
>>>>> +        default:
>>>>> +            /* Ignore unknown PCI busses */
>>>>
>>>> I would add a
>>>> printk("Ignoring PCI busses %s\n", dt_node_full_name(dev));
>>>>
>>>>> +            ret = 0;
>>>>> +            break;
>>>>
>>>> continue?
>>>
>>> Yes, that makes sense (probably the ret = is then unnecessary).
>>>
>>>>    You can't assume the order of the PCI busses in the device tree.
>>>
>>> But, I don't understand what this has to do with using continue.
>>
>> The current xgene-storm DTS has the different PCI busses ordered. So as
>> soon as you don't find the PCI range, it means there is no more PCI busses.
>
> I don't think it does, the patch iterates over all of the buses, even
> ones we don't understand, we don't give up at the first one we don't
> grok.

Hrmm you are right. I don't know why I though the break were bound to 
the loop and not the switch.

Sorry for the noise.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH for-4.5 2/4] xen: arm: correct off by one in xgene-storm's map_one_mmio
  2014-11-18 16:44 ` [PATCH for-4.5 2/4] xen: arm: correct off by one in xgene-storm's map_one_mmio Ian Campbell
  2014-11-18 17:01   ` Julien Grall
@ 2014-11-19 21:11   ` Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 20+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-11-19 21:11 UTC (permalink / raw
  To: Ian Campbell
  Cc: stefano.stabellini, julien.grall, tim, xen-devel, Clark Laughlin,
	Pranavkumar Sawargaonkar

On Tue, Nov 18, 2014 at 04:44:46PM +0000, Ian Campbell wrote:
> The callers pass the end as the pfn immediately *after* the last page to be
> mapped, therefore adding one is incorrect and causes an additional page to be
> mapped.
> 
> At the same time correct the printing of the mfn values, zero-padding them to
> 16 digits as for a paddr when they are frame numbers is just confusing.

HA! I was just looking at that today and thought it was odd.

Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/arch/arm/platforms/xgene-storm.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
> index 29c4752..38674cd 100644
> --- a/xen/arch/arm/platforms/xgene-storm.c
> +++ b/xen/arch/arm/platforms/xgene-storm.c
> @@ -45,9 +45,9 @@ static int map_one_mmio(struct domain *d, const char *what,
>  {
>      int ret;
>  
> -    printk("Additional MMIO %"PRIpaddr"-%"PRIpaddr" (%s)\n",
> +    printk("Additional MMIO %lx-%lx (%s)\n",
>             start, end, what);
> -    ret = map_mmio_regions(d, start, end - start + 1, start);
> +    ret = map_mmio_regions(d, start, end - start, start);
>      if ( ret )
>          printk("Failed to map %s @ %"PRIpaddr" to dom%d\n",
>                 what, start, d->domain_id);
> -- 
> 1.7.10.4
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/4 for-4.5] xen: arm: xgene bug fixes + support for McDivitt
  2014-11-18 16:51 ` [PATCH 0/4 for-4.5] xen: arm: xgene bug fixes + support for McDivitt Ian Campbell
@ 2014-11-19 21:18   ` Konrad Rzeszutek Wilk
  2014-11-20  9:02     ` Ian Campbell
  0 siblings, 1 reply; 20+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-11-19 21:18 UTC (permalink / raw
  To: Ian Campbell
  Cc: Julien Grall, Tim Deegan, xen-devel, Stefano Stabellini,
	Clark Laughlin, Pranavkumar Sawargaonkar

On Tue, Nov 18, 2014 at 04:51:42PM +0000, Ian Campbell wrote:
> On Tue, 2014-11-18 at 16:44 +0000, Ian Campbell wrote:
> > These patches:
> 
> ... which are also at
>         git://xenbits.xen.org/people/ianc/xen.git mcdivitt-v1

I presume you are going to post v2 with Julian's feedback rolled in?

I took a look at the code and it looks Xen 4.5 material so I am
OK with it rolling in, but would appreciate another posting just
to make sure that nothing is amiss.

Thank you!
> 
> Ian.
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/4 for-4.5] xen: arm: xgene bug fixes + support for McDivitt
  2014-11-19 21:18   ` Konrad Rzeszutek Wilk
@ 2014-11-20  9:02     ` Ian Campbell
  0 siblings, 0 replies; 20+ messages in thread
From: Ian Campbell @ 2014-11-20  9:02 UTC (permalink / raw
  To: Konrad Rzeszutek Wilk
  Cc: Julien Grall, Tim Deegan, xen-devel, Stefano Stabellini,
	Clark Laughlin, Pranavkumar Sawargaonkar

On Wed, 2014-11-19 at 16:18 -0500, Konrad Rzeszutek Wilk wrote:
> On Tue, Nov 18, 2014 at 04:51:42PM +0000, Ian Campbell wrote:
> > On Tue, 2014-11-18 at 16:44 +0000, Ian Campbell wrote:
> > > These patches:
> > 
> > ... which are also at
> >         git://xenbits.xen.org/people/ianc/xen.git mcdivitt-v1
> 
> I presume you are going to post v2 with Julian's feedback rolled in?

Yes, see <1416410868.29243.39.camel@citrix.com>.

> I took a look at the code and it looks Xen 4.5 material so I am
> OK with it rolling in, but would appreciate another posting just
> to make sure that nothing is amiss.
> 
> Thank you!
> > 
> > Ian.
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2014-11-20  9:02 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-18 16:44 [PATCH 0/4 for-4.5] xen: arm: xgene bug fixes + support for McDivitt Ian Campbell
2014-11-18 16:44 ` [PATCH for-4.5 1/4] xen: arm: Add earlyprintk " Ian Campbell
2014-11-18 16:59   ` Julien Grall
2014-11-19  9:54     ` Ian Campbell
2014-11-19 10:02       ` Julien Grall
2014-11-18 16:44 ` [PATCH for-4.5 2/4] xen: arm: correct off by one in xgene-storm's map_one_mmio Ian Campbell
2014-11-18 17:01   ` Julien Grall
2014-11-19  9:54     ` Ian Campbell
2014-11-19 21:11   ` Konrad Rzeszutek Wilk
2014-11-18 16:44 ` [PATCH for-4.5 3/4] xen: arm: correct specific mappings for PCIE0 on X-Gene Ian Campbell
2014-11-18 17:04   ` Julien Grall
2014-11-18 16:44 ` [PATCH for-4.5 4/4] xen: arm: Support the other 4 PCI buses on Xgene Ian Campbell
2014-11-18 17:15   ` Julien Grall
2014-11-19  9:56     ` Ian Campbell
2014-11-19 10:06       ` Julien Grall
2014-11-19 10:18         ` Ian Campbell
2014-11-19 10:30           ` Julien Grall
2014-11-18 16:51 ` [PATCH 0/4 for-4.5] xen: arm: xgene bug fixes + support for McDivitt Ian Campbell
2014-11-19 21:18   ` Konrad Rzeszutek Wilk
2014-11-20  9:02     ` Ian Campbell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.