xen: arm: xgene bug fixes + support for McDivitt
cover xen: arm: xgene bug fixes + support for McDivitt
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.
PATCH 1/4 xen: arm: Add earlyprintk for McDivitt.
--- xen/arch/arm/Rules.mk | 6 ++++++ 1 file changed, 6 insertions(+)
xen/arch/arm/Rules.mk +6 -0
@@ -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
Signature · 1 line
1.7.10.4
PATCH 2/4 xen: arm: correct off by one in xgene-storm's map_one_mmio
In reply to Ian CampbellThe 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.
--- xen/arch/arm/platforms/xgene-storm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
xen/arch/arm/platforms/xgene-storm.c +2 -2
@@ -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);
Signature · 1 line
1.7.10.4
PATCH 3/4 xen: arm: correct specific mappings for PCIE0 on X-Gene
In reply to Ian CampbellThe 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).
--- xen/arch/arm/platforms/xgene-storm.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-)
xen/arch/arm/platforms/xgene-storm.c +2 -16
@@ -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;
Signature · 1 line
1.7.10.4
PATCH 4/4 xen: arm: Support the other 4 PCI buses on Xgene
In reply to Ian CampbellCurrently 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.
--- xen/arch/arm/platforms/xgene-storm.c | 84 ++++++++++++++++++++++++++++------ 1 file changed, 71 insertions(+), 13 deletions(-)
xen/arch/arm/platforms/xgene-storm.c +71 -13
@@ -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;
Signature · 1 line
1.7.10.4
Quoted 1 line · Ian Campbell
These patches:
... which are also at
git://xenbits.xen.org/people/ianc/xen.git mcdivitt-v1
Ian.
Re: [PATCH for-4.5 1/4] xen: arm: Add earlyprintk for McDivitt.
In reply to Ian CampbellHi Ian,
Quoted 10 lines · Ian Campbell
Signed-off-by Ian Campbell <[email protected]>--- xen/arch/arm/Rules.mk | 6 ++++++ 1 file changed, 6 insertions(+)xen/arch/arm/Rules.mk +0
-0
xen/arch/arm/Rules.mk EARLY_PRINTK_BAUD := 115200 · line 100
Quoted 6 lines
xen/arch/arm/Rules.mk +3
-0@@ -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,
Signature · 1 line
Julien Grall
Re: [PATCH for-4.5 2/4] xen: arm: correct off by one in xgene-storm's map_one_mmio
In reply to Ian CampbellHi Ian,
Quoted 18 lines · Ian Campbell
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 <[email protected]>--- xen/arch/arm/platforms/xgene-storm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)xen/arch/arm/platforms/xgene-storm.c +0
-0@@ -45,9 +45,9 @@ static int map_one_mmio(struct domain *d, const char *what, {
xen/arch/arm/platforms/xgene-storm.c static int map_one_mmio(struct domain *d, const char *what, · line 53
Quoted 10 lines
xen/arch/arm/platforms/xgene-storm.c +2
-2int 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,
Signature · 1 line
Julien Grall
Re: [PATCH for-4.5 3/4] xen: arm: correct specific mappings for PCIE0 on X-Gene
In reply to Ian CampbellHi Ian,
Quoted 5 lines · Ian Campbell
The region assigned to PCIE0, according to the docs, is
0x0e000000000to0x10000000000. 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 <[email protected]>
Regards,
Signature · 1 line
Julien Grall
Re: [PATCH for-4.5 4/4] xen: arm: Support the other 4 PCI buses on Xgene
In reply to Ian CampbellHi Ian,
Quoted 6 lines · Ian Campbell
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!
xen/arch/arm/platforms/xgene-storm.c err: · line 122
Quoted 11 lines
Signed-off-by Ian Campbell <[email protected]>---
xen/arch/arm/platforms/xgene-storm.c +9
-0+/* +* 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
Quoted 29 lines
xen/arch/arm/platforms/xgene-storm.c +29
-0+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 */
xen/arch/arm/platforms/xgene-storm.c err: · line 161
Quoted 10 lines
xen/arch/arm/platforms/xgene-storm.c +10
-0+ 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 aprintk("Ignoring PCI busses %s\n", dt_node_full_name(dev));
xen/arch/arm/platforms/xgene-storm.c err: · line 163
Quoted 2 lines
xen/arch/arm/platforms/xgene-storm.c +2
-0+ ret =0 ; +break ;
continue? You can't assume the order of the PCI busses in the device tree.
xen/arch/arm/platforms/xgene-storm.c err: · line 170
Quoted 7 lines
xen/arch/arm/platforms/xgene-storm.c +7
-0+ } + +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,
Signature · 1 line
Julien Grall
Re: [PATCH for-4.5 1/4] xen: arm: Add earlyprintk for McDivitt.
In reply to Julien GrallQuoted 8 lines · Julien Grall
Hi Ian,
Quoted 5 lines · Ian Campbell
Signed-off-by Ian Campbell <[email protected]>--- xen/arch/arm/Rules.mk | 6 ++++++ 1 file changed, 6 insertions(+)
Quoted 14 lines
xen/arch/arm/Rules.mk EARLY_PRINTK_BAUD := 115200 · line 100
Quoted 11 lines
xen/arch/arm/Rules.mk +3
-0@@ -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_BAUDis not necessary as we don't use the initialization function (EARLY_PRINTK_INIT_UARTis not set).
Oh yes, oops. Also the baud is not even what is actually used, so it's not even serving a documentary purpose.
Quoted 2 lines
With the
EARLY_PRINTK_BAUDdropped, 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.
Quoted 2 lines
(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.
Quoted 1 line
But I don't think it's 4.5 material.
You mean the patch generally or the merging?
Ian.
Re: [PATCH for-4.5 2/4] xen: arm: correct off by one in xgene-storm's map_one_mmio
In reply to Julien GrallQuoted 23 lines · Julien Grall
Hi Ian,
Quoted 20 lines · Ian Campbell
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 <[email protected]>--- xen/arch/arm/platforms/xgene-storm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)xen/arch/arm/platforms/xgene-storm.c +0
-0@@ -45,9 +45,9 @@ static int map_one_mmio(struct domain *d, const char *what, {int ret;
Quoted 10 lines
xen/arch/arm/platforms/xgene-storm.c static int map_one_mmio(struct domain *d, const char *what, · line 53
Quoted 8 lines
xen/arch/arm/platforms/xgene-storm.c +2
-2-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.
Re: [PATCH for-4.5 4/4] xen: arm: Support the other 4 PCI buses on Xgene
In reply to Julien GrallQuoted 10 lines · Julien Grall
xen/arch/arm/platforms/xgene-storm.c err: · line 161
Quoted 2 lines
xen/arch/arm/platforms/xgene-storm.c +2
-0+default : +/* Ignore unknown PCI busses */ I would add a
printk("Ignoring PCI busses %s\n", dt_node_full_name(dev));xen/arch/arm/platforms/xgene-storm.c err: · line 163
Quoted 2 lines
xen/arch/arm/platforms/xgene-storm.c +2
-0+ ret =0 ; +break ;continue?
Yes, that makes sense (probably the ret = is then unnecessary).
Quoted 1 line
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.
Quoted 10 lines
xen/arch/arm/platforms/xgene-storm.c err: · line 170
Quoted 8 lines
xen/arch/arm/platforms/xgene-storm.c +7
-0+ } + +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.
Re: [PATCH for-4.5 1/4] xen: arm: Add earlyprintk for McDivitt.
In reply to Ian CampbellHi,
Quoted 30 lines · Ian Campbell
Quoted 8 lines · Julien Grall
Hi Ian,
Quoted 5 lines · Ian Campbell
Signed-off-by Ian Campbell <[email protected]>--- xen/arch/arm/Rules.mk | 6 ++++++ 1 file changed, 6 insertions(+)Quoted 14 lines
xen/arch/arm/Rules.mk EARLY_PRINTK_BAUD := 115200 · line 100
Quoted 11 lines
xen/arch/arm/Rules.mk +3
-0@@ -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_BAUDis not necessary as we don't use the initialization function (EARLY_PRINTK_INIT_UARTis not set).Oh yes, oops. Also the baud is not even what is actually used, so it's not even serving a documentary purpose.
Quoted 2 lines
With the
EARLY_PRINTK_BAUDdropped, this could be merged with the xgene-storm early printk
Quoted 7 lines
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,
Signature · 1 line
Julien Grall
Re: [PATCH for-4.5 4/4] xen: arm: Support the other 4 PCI buses on Xgene
In reply to Ian CampbellHi Ian,
Quoted 10 lines · Ian Campbell
Quoted 9 lines · Julien Grall
xen/arch/arm/platforms/xgene-storm.c err: · line 161
Quoted 2 lines
xen/arch/arm/platforms/xgene-storm.c +2
-0+default : +/* Ignore unknown PCI busses */ I would add a
printk("Ignoring PCI busses %s\n", dt_node_full_name(dev));Quoted 2 lines
xen/arch/arm/platforms/xgene-storm.c +2
-0+ ret =0 ; +break ;
Quoted 7 lines
Quoted 1 line
continue?
Yes, that makes sense (probably the ret = is then unnecessary).
Quoted 1 line
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,
Signature · 1 line
Julien Grall
Re: [PATCH for-4.5 4/4] xen: arm: Support the other 4 PCI buses on Xgene
Quoted 15 lines · Julien Grall
Hi Ian,
Quoted 12 lines · Ian Campbell
Quoted 10 lines · Julien Grall
xen/arch/arm/platforms/xgene-storm.c err: · line 161
Quoted 2 lines
xen/arch/arm/platforms/xgene-storm.c +2
-0+default : +/* Ignore unknown PCI busses */ I would add a
printk("Ignoring PCI busses %s\n", dt_node_full_name(dev));xen/arch/arm/platforms/xgene-storm.c err: · line 163
Quoted 2 lines
xen/arch/arm/platforms/xgene-storm.c +2
-0+ ret =0 ; +break ;continue?
Quoted 8 lines
Quoted 5 lines
Yes, that makes sense (probably the ret = is then unnecessary).
Quoted 1 line
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.
Quoted 8 lines
Without the continue, this patch gives the impression that you rely on the node order on the device tree.
Regards,
Re: [PATCH for-4.5 4/4] xen: arm: Support the other 4 PCI buses on Xgene
Quoted 20 lines · Ian Campbell
Quoted 19 lines · Julien Grall
Hi Ian,
Quoted 16 lines · Ian Campbell
Quoted 10 lines · Julien Grall
xen/arch/arm/platforms/xgene-storm.c err: · line 161
Quoted 2 lines
xen/arch/arm/platforms/xgene-storm.c +2
-0+default : +/* Ignore unknown PCI busses */ I would add a
printk("Ignoring PCI busses %s\n", dt_node_full_name(dev));xen/arch/arm/platforms/xgene-storm.c err: · line 163
Quoted 2 lines
xen/arch/arm/platforms/xgene-storm.c +2
-0+ ret =0 ; +break ;continue?
Yes, that makes sense (probably the ret = is then unnecessary).
Quoted 1 line
You can't assume the order of the PCI busses in the device tree.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
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,
Signature · 1 line
Julien Grall
Re: [PATCH for-4.5 2/4] xen: arm: correct off by one in xgene-storm's map_one_mmio
In reply to Ian CampbellQuoted 6 lines · Ian Campbell
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.
Quoted 25 lines
Signed-off-by Ian Campbell <[email protected]>--- xen/arch/arm/platforms/xgene-storm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)xen/arch/arm/platforms/xgene-storm.c +2
-2@@ -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);Signature · 1 line
1.7.10.4
Quoted 5 lines · Ian Campbell
Quoted 1 line · Ian Campbell
These patches:
... which are also at
git://xenbits.xen.org/people/ianc/xen.gitmcdivitt-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!
Quoted 8 lines
Ian.
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Quoted 8 lines · Konrad Rzeszutek Wilk
Quoted 5 lines · Ian Campbell
Quoted 1 line · Ian Campbell
These patches:
... which are also at
git://xenbits.xen.org/people/ianc/xen.gitmcdivitt-v1I presume you are going to post v2 with Julian's feedback rolled in?
Yes, see <[email protected]>.
Quoted 13 lines
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!
Quoted 8 lines
Ian.
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel