Ian Campbell
2013-Nov-20 14:45 UTC
[PATCH+RFC+HACK 00/16] xen: arm initial support for xgene arm64 platform
I''m afraid this series is rather a grab bag and it is distressingly
large at this stage. With this series I can boot an Xgene board until it
fails to find its SATA controller. This is a dom0 issue for which
patches are pending from APM (/me nudges Anup).
As well as the APM specific platform stuff there are also some generic
improvements which were either necessary or useful during this work.
Towards the tail end are some hacks and rfcs which need more work and/or
discussion. I mostly posting now because I''m aware that I''ve
been
negligent in not sending these out sooner.
WRT the freeze I think that the baseline stuff is all plausible for 4.4.
Firstly because I''m inclined to give new platform enablement a fairly
loose reign at least for the time being (and much of it was posted ages
ago by Anup/Pranavkumar). Secondly the non-platform related bits (other
than the aforementioned hacks/rfcs) fall mostly either into two buckets:
Either they are bugfixes or they are mostly obviously safe (additional
debug prints etc).
Blow by blow analysis:
xen: arm64: Add 8250 earlyprintk support
New early uart driver. It is enabled as a build time
debug option and is totally harmless to platforms which
don''t use it.
xen: arm64: Add Basic Platform support for APM X-Gene Storm.
xen: arm64: Add APM implementor id to processor implementers.
xen: arm: include ns16550 driver on arm64 too
xen: arm: Enable 1:1 workaround for APM X-Gene Storm.
Support for the new platform. Enable an existing driver
used by that platform (already on for arm32).
xen: arm: early logging of command line
Pretty safe & very useful IMNSVHO.
xen: arm: Handle cpus nodes with #address-calls > 1
xen: arm: Make register bit definitions unsigned.
xen: arm: explicitly map 64 bit release address
Bug fixes.
xen: arm: enable synchronous console while starting secondary CPUs
Improves logging in a useful way. Pretty safe.
xen: arm: Add debug keyhandler to dump the physical GIC state.
Useful debug functionality. Harmless unless you
deliberately trigger the particular debug key.
xen: arm: improve early memory map readability
Cosmetic, but safe.
RFC: xen: arm: handle 40-bit addresses in the p2m
RFC: xen: arm: allow platform code to select dom0 event channel irq
These should be considered for cleanup review and
eventual commit for 4.4. The rest of the platform
enablement is pretty pointless without these.
HACK: xen: arm: GICC_DIR register at offset 0x10000 instead of 0x1000
Should be properly implemented with a view to being
accepted for 4.4. Again things are rather pointless
without.
Could plausibly be reimplemented as a platform quirk,
which might be safer for 4.4.
HACK: xen: arm: map PCI controller ranges region MMIOs to dom0.
I think this one is likely to be a step too far for 4.4.
Even if it worked (it doesn''t) it is quite a big and
potentially complex change. I''m considering the option
of implementing the hardcoded list (which is here as a
HACK, see the commit message for more) via the
platform->specific_mapping callback for 4.4. In that
case it would only impact Xgene if it were broken.
Ian.
From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> Extracted from "Basic Platform support for APM X-Gene Storm." Signed-off-by: Anup Patel <anup.patel@linaro.org> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> Reworked into generic 8250 driver, use EARLY_UART_REG_SHIFT. While there observe a missing shift in the arm32 version (UART_THR is zero so it doesn''t really matter). Changed for consistency. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- xen/arch/arm/Rules.mk | 6 +++++ xen/arch/arm/arm64/debug-8250.inc | 52 +++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 xen/arch/arm/arm64/debug-8250.inc diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk index c27c2eb..aaa203e 100644 --- a/xen/arch/arm/Rules.mk +++ b/xen/arch/arm/Rules.mk @@ -82,6 +82,12 @@ EARLY_PRINTK_INC := 8250 EARLY_UART_BASE_ADDRESS := 0xF0406B00 EARLY_UART_REG_SHIFT := 2 endif +ifeq ($(CONFIG_EARLY_PRINTK), xgene-storm) +EARLY_PRINTK_INC := 8250 +EARLY_PRINTK_BAUD := 115200 +EARLY_UART_BASE_ADDRESS := 0x1c020000 +EARLY_UART_REG_SHIFT := 2 +endif ifneq ($(EARLY_PRINTK_INC),) EARLY_PRINTK := y diff --git a/xen/arch/arm/arm64/debug-8250.inc b/xen/arch/arm/arm64/debug-8250.inc new file mode 100644 index 0000000..5858354 --- /dev/null +++ b/xen/arch/arm/arm64/debug-8250.inc @@ -0,0 +1,52 @@ +/* + * xen/arch/arm/arm64/debug-8250.inc + * + * 8250 specific debug code + * + * Copyright (c) 2013 Applied Micro. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <xen/8250-uart.h> + +/* UART initialization + * rb: register which contains the UART base address + * rc: scratch register 1 + * rd: scratch register 2 */ +.macro early_uart_init rb rc rd +.endm + +/* UART wait UART to be ready to transmit + * xb: register which contains the UART base address + * c: scratch register */ +.macro early_uart_ready xb c +1: + ldrb w\c, [\xb, #UART_LSR << EARLY_UART_REG_SHIFT] + and w\c, w\c, #UART_LSR_THRE + cmp w\c, #UART_LSR_THRE + b.ne 1b +.endm + +/* UART transmit character + * xb: register which contains the UART base address + * wt: register which contains the character to transmit */ +.macro early_uart_transmit xb wt + /* UART_THR transmit holding */ + strb \wt, [\xb, #UART_THR << EARLY_UART_REG_SHIFT] +.endm + +/* + * Local variables: + * mode: ASM + * indent-tabs-mode: nil + * End: + */ -- 1.7.10.4
Ian Campbell
2013-Nov-20 14:48 UTC
[PATCH 02/16] xen: arm64: Add Basic Platform support for APM X-Gene Storm.
From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
This patch adds initial platform stubs for APM X-Gene.
Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
Drop earlyprintk (split into earlier patch). Only build on ARM64.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/platforms/Makefile | 1 +
xen/arch/arm/platforms/xgene-storm.c | 57 ++++++++++++++++++++++++++++++++++
2 files changed, 58 insertions(+)
create mode 100644 xen/arch/arm/platforms/xgene-storm.c
diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
index f0dd72c..680364f 100644
--- a/xen/arch/arm/platforms/Makefile
+++ b/xen/arch/arm/platforms/Makefile
@@ -3,3 +3,4 @@ obj-$(CONFIG_ARM_32) += exynos5.o
obj-$(CONFIG_ARM_32) += midway.o
obj-$(CONFIG_ARM_32) += omap5.o
obj-$(CONFIG_ARM_32) += sunxi.o
+obj-$(CONFIG_ARM_64) += xgene-storm.o
diff --git a/xen/arch/arm/platforms/xgene-storm.c
b/xen/arch/arm/platforms/xgene-storm.c
new file mode 100644
index 0000000..8e2b3b6
--- /dev/null
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -0,0 +1,57 @@
+/*
+ * xen/arch/arm/platforms/xgene-storm.c
+ *
+ * Applied Micro''s X-Gene specific settings
+ *
+ * Pranavkumar Sawargaonkar <psawargaonkar@apm.com>
+ * Anup Patel <apatel@apm.com>
+ * Copyright (c) 2013 Applied Micro.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <xen/config.h>
+#include <xen/device_tree.h>
+#include <xen/domain_page.h>
+#include <xen/mm.h>
+#include <xen/vmap.h>
+#include <asm/platform.h>
+#include <asm/early_printk.h>
+
+static void xgene_storm_reset(void)
+{
+}
+
+static int xgene_storm_init(void)
+{
+ return 0;
+}
+
+static const char const *xgene_storm_dt_compat[] __initdata +{
+ "apm,xgene-storm",
+ NULL
+};
+
+PLATFORM_START(xgene_storm, "APM X-GENE STORM")
+ .compatible = xgene_storm_dt_compat,
+ .init = xgene_storm_init,
+ .reset = xgene_storm_reset,
+PLATFORM_END
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
1.7.10.4
Ian Campbell
2013-Nov-20 14:48 UTC
[PATCH 03/16] xen: arm64: Add APM implementor id to processor implementers.
From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
This patch updates the list of processor implementers with APM implementor id.
Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
xen/arch/arm/setup.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index cdcc2e7..49e1b5c 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -64,6 +64,7 @@ static const char * __initdata processor_implementers[] = {
[''B''] = "Broadcom Corporation",
[''D''] = "Digital Equipment Corp",
[''M''] = "Motorola, Freescale Semiconductor
Inc.",
+ [''P''] = "Applied Micro",
[''Q''] = "Qualcomm Inc.",
[''V''] = "Marvell Semiconductor Inc.",
[''i''] = "Intel Corporation",
--
1.7.10.4
Ian Campbell
2013-Nov-20 14:48 UTC
[PATCH 04/16] xen: arm: include ns16550 driver on arm64 too
Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- config/arm64.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/config/arm64.mk b/config/arm64.mk index 49055fa..15b57a4 100644 --- a/config/arm64.mk +++ b/config/arm64.mk @@ -7,6 +7,7 @@ CONFIG_XEN_INSTALL_SUFFIX : CFLAGS += #-marm -march= -mcpu= etc HAS_PL011 := y +HAS_NS16550 := y # Use only if calling $(LD) directly. LDFLAGS_DIRECT += -EL -- 1.7.10.4
Ian Campbell
2013-Nov-20 14:48 UTC
[PATCH 05/16] xen: arm: Enable 1:1 workaround for APM X-Gene Storm.
There is no SMMU on this platform.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/platforms/xgene-storm.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/xen/arch/arm/platforms/xgene-storm.c
b/xen/arch/arm/platforms/xgene-storm.c
index 8e2b3b6..23986a9 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -35,6 +35,12 @@ static int xgene_storm_init(void)
return 0;
}
+static uint32_t xgene_storm_quirks(void)
+{
+ return PLATFORM_QUIRK_DOM0_MAPPING_11;
+}
+
+
static const char const *xgene_storm_dt_compat[] __initdata {
"apm,xgene-storm",
@@ -45,6 +51,7 @@ PLATFORM_START(xgene_storm, "APM X-GENE STORM")
.compatible = xgene_storm_dt_compat,
.init = xgene_storm_init,
.reset = xgene_storm_reset,
+ .quirks = xgene_storm_quirks,
PLATFORM_END
/*
--
1.7.10.4
Helpful for diagnosis of bad console= parameters.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/setup.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 49e1b5c..d252131 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -597,6 +597,7 @@ void __init start_xen(unsigned long boot_phys_offset,
{
size_t fdt_size;
int cpus, i;
+ const char *cmdline;
setup_cache();
@@ -610,7 +611,9 @@ void __init start_xen(unsigned long boot_phys_offset,
+ (fdt_paddr & ((1 << SECOND_SHIFT) - 1));
fdt_size = device_tree_early_init(device_tree_flattened, fdt_paddr);
- cmdline_parse(device_tree_bootargs(device_tree_flattened));
+ cmdline = device_tree_bootargs(device_tree_flattened);
+ early_printk("Command line: %s\n", cmdline);
+ cmdline_parse(cmdline);
setup_pagetables(boot_phys_offset, get_xen_paddr());
setup_mm(fdt_paddr, fdt_size);
--
1.7.10.4
Ian Campbell
2013-Nov-20 14:48 UTC
[PATCH 07/16] xen: arm: Handle cpus nodes with #address-calls > 1
The APM X-Gene Mustang board DTS has #address-cells = 2.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/smpboot.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 6c90fa6..ce832ae 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -125,18 +125,35 @@ void __init smp_init_cpus(void)
dt_for_each_child_node( cpus, cpu )
{
+ const __be32 *prop;
+ u64 addr, size;
u32 hwid;
if ( !dt_device_type_is_equal(cpu, "cpu") )
continue;
- if ( !dt_property_read_u32(cpu, "reg", &hwid) )
+ if ( dt_n_size_cells(cpu) != 0 )
+ printk(XENLOG_WARNING "cpu node `%s`: #size-cells %d\n",
+ dt_node_full_name(cpu), dt_n_size_cells(cpu));
+
+ prop = dt_get_property(cpu, "reg", NULL);
+ if ( !prop )
{
- printk(XENLOG_WARNING "cpu node `%s`: missing reg
property\n",
+ printk(XENLOG_WARNING "cpu node `%s`: has no reg
property\n",
dt_node_full_name(cpu));
continue;
}
+ dt_get_range(&prop, cpu, &addr, &size);
+
+ hwid = addr;
+ if ( hwid != addr )
+ {
+ printk(XENLOG_WARNING "cpu node `%s`: hwid overflow
%"PRIx64"\n",
+ dt_node_full_name(cpu), addr);
+ continue;
+ }
+
/*
* 8 MSBs must be set to 0 in the DT since the reg property
* defines the MPIDR[23:0]
@@ -159,8 +176,8 @@ void __init smp_init_cpus(void)
if ( tmp_map[j] == hwid )
{
printk(XENLOG_WARNING
- "cpu node `%s`: duplicate /cpu reg properties in
the DT\n",
- dt_node_full_name(cpu));
+ "cpu node `%s`: duplicate /cpu reg properties
%"PRIx32" in the DT\n",
+ dt_node_full_name(cpu), hwid);
break;
}
}
--
1.7.10.4
Ian Campbell
2013-Nov-20 14:48 UTC
[PATCH 08/16] xen: arm: Make register bit definitions unsigned.
Otherwise the results of the shifting can be undefined and/or sign extended.
Pointed out in the context of HCR_* by Pranavkumar Sawargaonkar.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Pranavkumar Sawargaonkar <psawargaonkar@apm.com>
---
xen/include/asm-arm/processor.h | 144 +++++++++++++++++++--------------------
1 file changed, 72 insertions(+), 72 deletions(-)
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 3da3a3d..7cd2d3e 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -18,71 +18,71 @@
#define MPIDR_INVALID (~MPIDR_HWID_MASK)
/* TTBCR Translation Table Base Control Register */
-#define TTBCR_EAE 0x80000000
-#define TTBCR_N_MASK 0x07
-#define TTBCR_N_16KB 0x00
-#define TTBCR_N_8KB 0x01
-#define TTBCR_N_4KB 0x02
-#define TTBCR_N_2KB 0x03
-#define TTBCR_N_1KB 0x04
+#define TTBCR_EAE _AC(0x80000000,U)
+#define TTBCR_N_MASK _AC(0x07,U)
+#define TTBCR_N_16KB _AC(0x00,U)
+#define TTBCR_N_8KB _AC(0x01,U)
+#define TTBCR_N_4KB _AC(0x02,U)
+#define TTBCR_N_2KB _AC(0x03,U)
+#define TTBCR_N_1KB _AC(0x04,U)
/* SCTLR System Control Register. */
/* HSCTLR is a subset of this. */
-#define SCTLR_TE (1<<30)
-#define SCTLR_AFE (1<<29)
-#define SCTLR_TRE (1<<28)
-#define SCTLR_NMFI (1<<27)
-#define SCTLR_EE (1<<25)
-#define SCTLR_VE (1<<24)
-#define SCTLR_U (1<<22)
-#define SCTLR_FI (1<<21)
-#define SCTLR_WXN (1<<19)
-#define SCTLR_HA (1<<17)
-#define SCTLR_RR (1<<14)
-#define SCTLR_V (1<<13)
-#define SCTLR_I (1<<12)
-#define SCTLR_Z (1<<11)
-#define SCTLR_SW (1<<10)
-#define SCTLR_B (1<<7)
-#define SCTLR_C (1<<2)
-#define SCTLR_A (1<<1)
-#define SCTLR_M (1<<0)
-
-#define HSCTLR_BASE 0x30c51878
+#define SCTLR_TE (_AC(1,U)<<30)
+#define SCTLR_AFE (_AC(1,U)<<29)
+#define SCTLR_TRE (_AC(1,U)<<28)
+#define SCTLR_NMFI (_AC(1,U)<<27)
+#define SCTLR_EE (_AC(1,U)<<25)
+#define SCTLR_VE (_AC(1,U)<<24)
+#define SCTLR_U (_AC(1,U)<<22)
+#define SCTLR_FI (_AC(1,U)<<21)
+#define SCTLR_WXN (_AC(1,U)<<19)
+#define SCTLR_HA (_AC(1,U)<<17)
+#define SCTLR_RR (_AC(1,U)<<14)
+#define SCTLR_V (_AC(1,U)<<13)
+#define SCTLR_I (_AC(1,U)<<12)
+#define SCTLR_Z (_AC(1,U)<<11)
+#define SCTLR_SW (_AC(1,U)<<10)
+#define SCTLR_B (_AC(1,U)<<7)
+#define SCTLR_C (_AC(1,U)<<2)
+#define SCTLR_A (_AC(1,U)<<1)
+#define SCTLR_M (_AC(1,U)<<0)
+
+#define HSCTLR_BASE _AC(0x30c51878,U)
/* HCR Hyp Configuration Register */
-#define HCR_RW (1<<31) /* Register Width, ARM64 only */
-#define HCR_TGE (1<<27) /* Trap General Exceptions */
-#define HCR_TVM (1<<26) /* Trap Virtual Memory Controls */
-#define HCR_TTLB (1<<25) /* Trap TLB Maintenance Operations */
-#define HCR_TPU (1<<24) /* Trap Cache Maintenance Operations to
PoU */
-#define HCR_TPC (1<<23) /* Trap Cache Maintenance Operations to
PoC */
-#define HCR_TSW (1<<22) /* Trap Set/Way Cache Maintenance
Operations */
-#define HCR_TAC (1<<21) /* Trap ACTLR Accesses */
-#define HCR_TIDCP (1<<20) /* Trap lockdown */
-#define HCR_TSC (1<<19) /* Trap SMC instruction */
-#define HCR_TID3 (1<<18) /* Trap ID Register Group 3 */
-#define HCR_TID2 (1<<17) /* Trap ID Register Group 2 */
-#define HCR_TID1 (1<<16) /* Trap ID Register Group 1 */
-#define HCR_TID0 (1<<15) /* Trap ID Register Group 0 */
-#define HCR_TWE (1<<14) /* Trap WFE instruction */
-#define HCR_TWI (1<<13) /* Trap WFI instruction */
-#define HCR_DC (1<<12) /* Default cacheable */
-#define HCR_BSU_MASK (3<<10) /* Barrier Shareability Upgrade */
-#define HCR_BSU_NONE (0<<10)
-#define HCR_BSU_INNER (1<<10)
-#define HCR_BSU_OUTER (2<<10)
-#define HCR_BSU_FULL (3<<10)
-#define HCR_FB (1<<9) /* Force Broadcast of Cache/BP/TLB
operations */
-#define HCR_VA (1<<8) /* Virtual Asynchronous Abort */
-#define HCR_VI (1<<7) /* Virtual IRQ */
-#define HCR_VF (1<<6) /* Virtual FIQ */
-#define HCR_AMO (1<<5) /* Override CPSR.A */
-#define HCR_IMO (1<<4) /* Override CPSR.I */
-#define HCR_FMO (1<<3) /* Override CPSR.F */
-#define HCR_PTW (1<<2) /* Protected Walk */
-#define HCR_SWIO (1<<1) /* Set/Way Invalidation Override */
-#define HCR_VM (1<<0) /* Virtual MMU Enable */
+#define HCR_RW (_AC(1,U)<<31) /* Register Width, ARM64 only */
+#define HCR_TGE (_AC(1,U)<<27) /* Trap General Exceptions */
+#define HCR_TVM (_AC(1,U)<<26) /* Trap Virtual Memory Controls */
+#define HCR_TTLB (_AC(1,U)<<25) /* Trap TLB Maintenance Operations
*/
+#define HCR_TPU (_AC(1,U)<<24) /* Trap Cache Maintenance
Operations to PoU */
+#define HCR_TPC (_AC(1,U)<<23) /* Trap Cache Maintenance
Operations to PoC */
+#define HCR_TSW (_AC(1,U)<<22) /* Trap Set/Way Cache Maintenance
Operations */
+#define HCR_TAC (_AC(1,U)<<21) /* Trap ACTLR Accesses */
+#define HCR_TIDCP (_AC(1,U)<<20) /* Trap lockdown */
+#define HCR_TSC (_AC(1,U)<<19) /* Trap SMC instruction */
+#define HCR_TID3 (_AC(1,U)<<18) /* Trap ID Register Group 3 */
+#define HCR_TID2 (_AC(1,U)<<17) /* Trap ID Register Group 2 */
+#define HCR_TID1 (_AC(1,U)<<16) /* Trap ID Register Group 1 */
+#define HCR_TID0 (_AC(1,U)<<15) /* Trap ID Register Group 0 */
+#define HCR_TWE (_AC(1,U)<<14) /* Trap WFE instruction */
+#define HCR_TWI (_AC(1,U)<<13) /* Trap WFI instruction */
+#define HCR_DC (_AC(1,U)<<12) /* Default cacheable */
+#define HCR_BSU_MASK (_AC(3,U)<<10) /* Barrier Shareability Upgrade */
+#define HCR_BSU_NONE (_AC(0,U)<<10)
+#define HCR_BSU_INNER (_AC(1,U)<<10)
+#define HCR_BSU_OUTER (_AC(2,U)<<10)
+#define HCR_BSU_FULL (_AC(3,U)<<10)
+#define HCR_FB (_AC(1,U)<<9) /* Force Broadcast of Cache/BP/TLB
operations */
+#define HCR_VA (_AC(1,U)<<8) /* Virtual Asynchronous Abort */
+#define HCR_VI (_AC(1,U)<<7) /* Virtual IRQ */
+#define HCR_VF (_AC(1,U)<<6) /* Virtual FIQ */
+#define HCR_AMO (_AC(1,U)<<5) /* Override CPSR.A */
+#define HCR_IMO (_AC(1,U)<<4) /* Override CPSR.I */
+#define HCR_FMO (_AC(1,U)<<3) /* Override CPSR.F */
+#define HCR_PTW (_AC(1,U)<<2) /* Protected Walk */
+#define HCR_SWIO (_AC(1,U)<<1) /* Set/Way Invalidation Override */
+#define HCR_VM (_AC(1,U)<<0) /* Virtual MMU Enable */
#define HSR_EC_UNKNOWN 0x00
#define HSR_EC_WFI_WFE 0x01
@@ -346,20 +346,20 @@ union hsr {
HSR_SYSREG_OP2_MASK)
/* Physical Address Register */
-#define PAR_F (1<<0)
+#define PAR_F (_AC(1,U)<<0)
/* .... If F == 1 */
#define PAR_FSC_SHIFT (1)
-#define PAR_FSC_MASK (0x3f<<PAR_FSC_SHIFT)
-#define PAR_STAGE21 (1<<8) /* Stage 2 Fault During Stage 1 Walk
*/
-#define PAR_STAGE2 (1<<9) /* Stage 2 Fault */
+#define PAR_FSC_MASK (_AC(0x3f,U)<<PAR_FSC_SHIFT)
+#define PAR_STAGE21 (_AC(1,U)<<8) /* Stage 2 Fault During Stage 1
Walk */
+#define PAR_STAGE2 (_AC(1,U)<<9) /* Stage 2 Fault */
/* If F == 0 */
#define PAR_MAIR_SHIFT 56 /* Memory Attributes */
#define PAR_MAIR_MASK (0xffLL<<PAR_MAIR_SHIFT)
-#define PAR_NS (1<<9) /* Non-Secure */
+#define PAR_NS (_AC(1,U)<<9) /* Non-Secure */
#define PAR_SH_SHIFT 7 /* Shareability */
-#define PAR_SH_MASK (3<<PAR_SH_SHIFT)
+#define PAR_SH_MASK (_AC(3,U)<<PAR_SH_SHIFT)
/* Fault Status Register */
/*
@@ -372,11 +372,11 @@ union hsr {
* 10xxxx -- Other
* 11xxxx -- Implementation Defined
*/
-#define FSC_TYPE_MASK (0x3<<4)
-#define FSC_TYPE_FAULT (0x00<<4)
-#define FSC_TYPE_ABT (0x01<<4)
-#define FSC_TYPE_OTH (0x02<<4)
-#define FSC_TYPE_IMPL (0x03<<4)
+#define FSC_TYPE_MASK (_AC(0x3,U)<<4)
+#define FSC_TYPE_FAULT (_AC(0x00,U)<<4)
+#define FSC_TYPE_ABT (_AC(0x01,U)<<4)
+#define FSC_TYPE_OTH (_AC(0x02,U)<<4)
+#define FSC_TYPE_IMPL (_AC(0x03,U)<<4)
#define FSC_FLT_TRANS (0x04)
#define FSC_FLT_ACCESS (0x08)
@@ -391,7 +391,7 @@ union hsr {
#define FSC_LKD (0x34) /* Lockdown Abort */
#define FSC_CPR (0x3a) /* Coprocossor Abort */
-#define FSC_LL_MASK (0x03<<0)
+#define FSC_LL_MASK (_AC(0x03,U)<<0)
/* Time counter hypervisor control register */
#define CNTHCTL_PA (1u<<0) /* Kernel/user access to physical
counter */
--
1.7.10.4
Ian Campbell
2013-Nov-20 14:48 UTC
[PATCH 09/16] xen: arm: explicitly map 64 bit release address
In case it is outside visible ram.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/arm64/smpboot.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/arm64/smpboot.c b/xen/arch/arm/arm64/smpboot.c
index 8239590..8696ed6 100644
--- a/xen/arch/arm/arm64/smpboot.c
+++ b/xen/arch/arm/arm64/smpboot.c
@@ -4,6 +4,8 @@
#include <xen/errno.h>
#include <xen/mm.h>
#include <xen/smp.h>
+#include <xen/vmap.h>
+#include <asm/io.h>
struct smp_enable_ops {
int (*prepare_cpu)(int);
@@ -14,7 +16,7 @@ static struct smp_enable_ops smp_enable_ops[NR_CPUS];
static int __init smp_spin_table_cpu_up(int cpu)
{
- paddr_t *release;
+ paddr_t __iomem *release;
if (!cpu_release_addr[cpu])
{
@@ -22,12 +24,20 @@ static int __init smp_spin_table_cpu_up(int cpu)
return -ENODEV;
}
- release = __va(cpu_release_addr[cpu]);
+ release = ioremap_nocache(cpu_release_addr[cpu], 8);
+ if ( !release )
+ {
+ dprintk(XENLOG_ERR, "CPU%d: Unable to map release address\n",
cpu);
+ return -EFAULT;
+ }
release[0] = __pa(init_secondary);
flush_xen_data_tlb_range_va((vaddr_t)release, sizeof(*release));
+ iounmap(release);
+
sev();
+
return 0;
}
--
1.7.10.4
Ian Campbell
2013-Nov-20 14:48 UTC
[PATCH 10/16] xen: arm: enable synchronous console while starting secondary CPUs
If CONFIG_EARLY_PRINTK is in use then this gets all interleaved with the
standard logging otherwise.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/smpboot.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index ce832ae..9b58345 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -28,6 +28,7 @@
#include <xen/softirq.h>
#include <xen/timer.h>
#include <xen/irq.h>
+#include <xen/console.h>
#include <asm/gic.h>
cpumask_t cpu_online_map;
@@ -349,6 +350,8 @@ int __cpu_up(unsigned int cpu)
if ( rc < 0 )
return rc;
+ console_start_sync(); /* Secondary may use early_printk */
+
/* Tell the remote CPU which stack to boot on. */
init_data.stack = idle_vcpu[cpu]->arch.stack;
@@ -361,6 +364,8 @@ int __cpu_up(unsigned int cpu)
rc = arch_cpu_up(cpu);
+ console_end_sync();
+
if ( rc < 0 )
{
printk("Failed to bring up CPU%d\n", cpu);
--
1.7.10.4
Ian Campbell
2013-Nov-20 14:48 UTC
[PATCH 11/16] xen: arm: Add debug keyhandler to dump the physical GIC state.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/gic.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index ab49106..185a6b8 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -28,6 +28,7 @@
#include <xen/softirq.h>
#include <xen/list.h>
#include <xen/device_tree.h>
+#include <xen/keyhandler.h>
#include <asm/p2m.h>
#include <asm/domain.h>
@@ -385,6 +386,77 @@ int gic_irq_xlate(const u32 *intspec, unsigned int intsize,
return 0;
}
+
+static void do_dump_gic(unsigned char key)
+{
+ int irq;
+ printk("''%c'' pressed -> dumping GIC
state\n", key);
+
+ for ( irq = 0; irq < gic.lines; irq++ )
+ {
+ const char *type;
+ int type_nr, enable, pend, active, priority, target;
+ struct irq_desc *desc = irq_to_desc(irq);
+ uint8_t *bytereg;
+ uint32_t wordreg;
+
+ bytereg = (uint8_t *) (GICD + GICD_ITARGETSR);
+ target = bytereg[irq];
+
+ bytereg = (uint8_t *) (GICD + GICD_IPRIORITYR);
+ priority = bytereg[irq];
+
+ switch ( irq )
+ {
+ case 0 ... 15:
+ type = "SGI";
+ type_nr = irq;
+ target = 0x00; /* these are per-CPU */
+ break;
+ case 16 ... 31:
+ type = "PPI";
+ type_nr = irq - 16;
+ break;
+ default:
+ type = "SPI";
+ type_nr = irq - 32;
+ break;
+ }
+
+ wordreg = GICD[GICD_ISENABLER + irq / 32];
+ enable = !!(wordreg & (1u << (irq % 32)));
+ wordreg = GICD[GICD_ISPENDR + irq / 32];
+ pend = !!(wordreg & (1u << (irq % 32)));
+ wordreg = GICD[GICD_ISACTIVER + irq / 32];
+ active = !!(wordreg & (1u << (irq % 32)));
+
+ printk("IRQ%d %s%d: %c%c%c pri:%02x tgt:%02x ",
+ irq, type, type_nr,
+ enable ? ''e'' : ''-'',
+ pend ? ''p'' : ''-'',
+ active ? ''a'' : ''-'',
+ priority, target);
+
+ if ( desc->status & IRQ_GUEST )
+ {
+ struct domain *d = desc->action->dev_id;
+ printk("dom%d %s", d->domain_id,
desc->action->name);
+ }
+ else
+ {
+ printk("Xen");
+ }
+ printk("\n");
+ }
+
+}
+
+static struct keyhandler dump_gic_keyhandler = {
+ .irq_callback = 0,
+ .u.fn = do_dump_gic,
+ .desc = "dump GIC state"
+};
+
/* Set up the GIC */
void __init gic_init(void)
{
@@ -456,6 +528,9 @@ void __init gic_init(void)
gic_hyp_init();
spin_unlock(&gic.lock);
+
+ register_keyhandler(''G'', &dump_gic_keyhandler);
+
}
void send_SGI_mask(const cpumask_t *cpumask, enum gic_sgi sgi)
--
1.7.10.4
Ian Campbell
2013-Nov-20 14:48 UTC
[PATCH 12/16] xen: arm: improve early memory map readability
Purely cosmetic, put a blank line after the early memory map to separate it
from the subsequent information. This looks better since hte memory map is
preceded by a blank line too.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/common/device_tree.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 44253da..943b181 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -452,6 +452,7 @@ static void __init early_print_info(void)
early_printk(" RESVD[%d]: %"PRIpaddr" -
%"PRIpaddr"\n",
i, s, e);
}
+ early_printk("\n");
}
/**
--
1.7.10.4
Ian Campbell
2013-Nov-20 14:48 UTC
[PATCH 13/16] RFC: xen: arm: handle 40-bit addresses in the p2m
On the X-gene platform there are resources up this high which must be mapped
to dom0.
I''m becoming more convinced that p2m first level pages should be mapped
from
the xenheap so we can avoid all this faff with figuring out which page is
needed.
Remove the first level page from the p2m->pages list since it is actually two
pages and must be freed as such. Do so in p2m_teardown.
I''ve also punted on the implementation of dump_p2m_lookup for high
addresses...
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
This is a little bit RFC...
---
xen/arch/arm/p2m.c | 60 +++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 48 insertions(+), 12 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 82dda65..af32511 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -7,6 +7,10 @@
#include <asm/flushtlb.h>
#include <asm/gic.h>
+/* First level P2M is 2 consecutive pages */
+#define P2M_FIRST_ORDER 1
+#define P2M_FIRST_ENTRIES (LPAE_ENTRIES<<P2M_FIRST_ORDER)
+
void dump_p2m_lookup(struct domain *d, paddr_t addr)
{
struct p2m_domain *p2m = &d->arch.p2m;
@@ -14,6 +18,12 @@ void dump_p2m_lookup(struct domain *d, paddr_t addr)
printk("dom%d IPA 0x%"PRIpaddr"\n", d->domain_id,
addr);
+ if ( first_linear_offset(addr) > LPAE_ENTRIES )
+ {
+ printk("Cannot dump addresses in second of first level
pages...\n");
+ return;
+ }
+
printk("P2M @ %p mfn:0x%lx\n",
p2m->first_level, page_to_mfn(p2m->first_level));
@@ -31,6 +41,30 @@ void p2m_load_VTTBR(struct domain *d)
isb(); /* Ensure update is visible */
}
+static int p2m_first_level_index(paddr_t addr)
+{
+ /*
+ * 1st pages are concatenated so zeroeth offset gives us the
+ * index of the 1st page
+ */
+ return zeroeth_table_offset(addr);
+}
+
+/*
+ * Map whichever of the first pages contain addr. The caller should
+ * then use first_table_offset as an index.
+ */
+static lpae_t *p2m_map_first(struct p2m_domain *p2m, paddr_t addr)
+{
+ struct page_info *page;
+
+ BUG_ON(first_linear_offset(addr) > P2M_FIRST_ENTRIES);
+
+ page = p2m->first_level + p2m_first_level_index(addr);
+
+ return __map_domain_page(page);
+}
+
/*
* Lookup the MFN corresponding to a domain''s PFN.
*
@@ -45,7 +79,7 @@ paddr_t p2m_lookup(struct domain *d, paddr_t paddr)
spin_lock(&p2m->lock);
- first = __map_domain_page(p2m->first_level);
+ first = p2m_map_first(p2m, paddr);
pte = first[first_table_offset(paddr)];
if ( !pte.p2m.valid || !pte.p2m.table )
@@ -135,18 +169,21 @@ static int create_p2m_entries(struct domain *d,
struct p2m_domain *p2m = &d->arch.p2m;
lpae_t *first = NULL, *second = NULL, *third = NULL;
paddr_t addr;
- unsigned long cur_first_offset = ~0, cur_second_offset = ~0;
+ unsigned long cur_first_page = ~0,
+ cur_first_offset = ~0,
+ cur_second_offset = ~0;
spin_lock(&p2m->lock);
- /* XXX Don''t actually handle 40 bit guest physical addresses */
- BUG_ON(start_gpaddr & 0x8000000000ULL);
- BUG_ON(end_gpaddr & 0x8000000000ULL);
-
- first = __map_domain_page(p2m->first_level);
-
for(addr = start_gpaddr; addr < end_gpaddr; addr += PAGE_SIZE)
{
+ if ( cur_first_page != p2m_first_level_index(addr) )
+ {
+ if ( first ) unmap_domain_page(first);
+ first = p2m_map_first(p2m, addr);
+ cur_first_page = p2m_first_level_index(addr);
+ }
+
if ( !first[first_table_offset(addr)].p2m.valid )
{
rc = p2m_create_table(d, &first[first_table_offset(addr)]);
@@ -279,15 +316,12 @@ int p2m_alloc_table(struct domain *d)
struct page_info *page;
void *p;
- /* First level P2M is 2 consecutive pages */
- page = alloc_domheap_pages(NULL, 1, 0);
+ page = alloc_domheap_pages(NULL, P2M_FIRST_ORDER, 0);
if ( page == NULL )
return -ENOMEM;
spin_lock(&p2m->lock);
- page_list_add(page, &p2m->pages);
-
/* Clear both first level pages */
p = __map_domain_page(page);
clear_page(p);
@@ -380,6 +414,8 @@ void p2m_teardown(struct domain *d)
while ( (pg = page_list_remove_head(&p2m->pages)) )
free_domheap_page(pg);
+ free_domheap_pages(p2m->first_level, P2M_FIRST_ORDER);
+
p2m->first_level = NULL;
p2m_free_vmid(d);
--
1.7.10.4
Ian Campbell
2013-Nov-20 14:48 UTC
[PATCH 14/16] RFC: xen: arm: allow platform code to select dom0 event channel irq
Currently the hardcoded use of GUEST_EVTCHN_PPI is problematic if that is a
real PPI on the platform.
We really need to be smarter about selecting an unused PPI but in the meantime
we can at least give the platform code the option of hardcoding a number which
works for the platform.
Hardcode a suitable PPI on the Xgene platform.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
RFC: I''m not all that happy with this, but it seems better than nothing
for
4.4...
If other existing platforms also have this issue I can extend the patch if
told what a suitable PPI is.
---
xen/arch/arm/domain.c | 7 +++++--
xen/arch/arm/platform.c | 7 +++++++
xen/arch/arm/platforms/xgene-storm.c | 1 +
xen/include/asm-arm/platform.h | 5 +++++
4 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 2f57d01..52d2403 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -31,6 +31,7 @@
#include <asm/processor-ca15.h>
#include <asm/gic.h>
+#include <asm/platform.h>
#include "vtimer.h"
#include "vuart.h"
@@ -526,8 +527,10 @@ int arch_domain_create(struct domain *d, unsigned int
domcr_flags)
if ( (rc = vcpu_domain_init(d)) != 0 )
goto fail;
- /* XXX dom0 needs more intelligent selection of PPI */
- d->arch.evtchn_irq = GUEST_EVTCHN_PPI;
+ if ( d->domain_id )
+ d->arch.evtchn_irq = GUEST_EVTCHN_PPI;
+ else
+ d->arch.evtchn_irq = platform_dom0_evtchn_ppi();
/*
* Virtual UART is only used by linux early printk and decompress code.
diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
index 0fbbdc7..a7f9ee4 100644
--- a/xen/arch/arm/platform.c
+++ b/xen/arch/arm/platform.c
@@ -156,6 +156,13 @@ bool_t platform_device_is_blacklisted(const struct
dt_device_node *node)
return dt_match_node(blacklist, node);
}
+unsigned int platform_dom0_evtchn_ppi(void)
+{
+ if ( platform && platform->dom0_evtchn_ppi )
+ return platform->dom0_evtchn_ppi;
+ return GUEST_EVTCHN_PPI;
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/arch/arm/platforms/xgene-storm.c
b/xen/arch/arm/platforms/xgene-storm.c
index 23986a9..b1b850b 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -52,6 +52,7 @@ PLATFORM_START(xgene_storm, "APM X-GENE STORM")
.init = xgene_storm_init,
.reset = xgene_storm_reset,
.quirks = xgene_storm_quirks,
+ .dom0_evtchn_ppi = 24,
PLATFORM_END
/*
diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
index c282b30..92b954d 100644
--- a/xen/include/asm-arm/platform.h
+++ b/xen/include/asm-arm/platform.h
@@ -37,6 +37,10 @@ struct platform_desc {
* List of devices which must not pass-through to a guest
*/
const struct dt_device_match *blacklist_dev;
+ /*
+ * The IRQ (PPI) to use to inject event channels to dom0.
+ */
+ unsigned int dom0_evtchn_ppi;
};
/*
@@ -56,6 +60,7 @@ void platform_reset(void);
void platform_poweroff(void);
bool_t platform_has_quirk(uint32_t quirk);
bool_t platform_device_is_blacklisted(const struct dt_device_node *node);
+unsigned int platform_dom0_evtchn_ppi(void);
#define PLATFORM_START(_name, _namestr) \
static const struct platform_desc __plat_desc_##_name __used \
--
1.7.10.4
Ian Campbell
2013-Nov-20 14:48 UTC
[PATCH 15/16] HACK: xen: arm: GICC_DIR register at offset 0x10000 instead of 0x1000
FIXME: Should implement device tree extensions from
http://www.spinics.net/lists/devicetree/msg10473.html
especially
http://www.spinics.net/lists/devicetree/msg10478.html
---
xen/arch/arm/gic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 185a6b8..0a8cf09 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -516,7 +516,7 @@ void __init gic_init(void)
BUILD_BUG_ON(FIXMAP_ADDR(FIXMAP_GICC1) !
FIXMAP_ADDR(FIXMAP_GICC2)-PAGE_SIZE);
set_fixmap(FIXMAP_GICC1, gic.cbase >> PAGE_SHIFT, DEV_SHARED);
- set_fixmap(FIXMAP_GICC2, (gic.cbase >> PAGE_SHIFT) + 1, DEV_SHARED);
+ set_fixmap(FIXMAP_GICC2, (gic.cbase >> PAGE_SHIFT) + 0x10,
DEV_SHARED);
set_fixmap(FIXMAP_GICH, gic.hbase >> PAGE_SHIFT, DEV_SHARED);
/* Global settings: interrupt distributor */
--
1.7.10.4
Ian Campbell
2013-Nov-20 14:48 UTC
[PATCH 16/16] HACK: xen: arm: map PCI controller ranges region MMIOs to dom0.
The ranges property of a node with device_type = "pci" is defined in
ePAPR
2.3.8. Map the appropriate MMIO regions through to dom0.
This is a hack/PoC since it actually crashes for some reason. Hence it
contains a hacked in hardcoded list suitable for Xgene while I figure this
out.
This should also eventually handle the interrupt-map and (ePAPR 2.4.3.1) and
possibly dma-ranges (ePAPR 2.3.9) and msi-ranges (unspeciifed?) too.
---
xen/arch/arm/domain_build.c | 103 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 103 insertions(+)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index aa7e3d2..e778c06 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -702,6 +702,69 @@ static int make_xen_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_property *ranges)
+{
+ const __be32 *cells;
+
+ int size_cells, addr_cells, i, nr;
+
+ u32 pci_space;
+ u64 child_addr;
+ u64 host_addr;
+ u64 length;
+
+ printk("%s(%p, %p, %p)\n", __func__, d, dev, ranges);
+ printk("%s device %s\n", __func__, dt_node_full_name(dev));
+ return 0;
+
+ cells = ranges->value;
+ printk("%s ranges at %p, length %d\n", __func__, cells,
ranges->length);
+ size_cells = dt_n_size_cells(dev);
+ addr_cells = dt_n_addr_cells(dev);
+
+ /*
+ * Range is child address, host address (#address-cells), length
+ * (#size-cells),see ePAPR 2.3.8.
+ *
+ * PCI child address is u32 space + u64 address, see ePAPR 6.2.2.
+ *
+ */
+ nr = ranges->length / ( 3 + size_cells + addr_cells );
+ printk("PCI device %s: #address-cells %d, #size-cells %d. len %d,
entries %d\n",
+ dt_node_name(dev), addr_cells, size_cells, ranges->length, nr);
+
+ for ( i = 0; i < nr ; i++ )
+ {
+ pci_space = (u32)dt_next_cell(1, &cells);
+ child_addr = dt_next_cell(2, &cells);
+ host_addr = dt_next_cell(addr_cells, &cells);
+ length = dt_next_cell(size_cells, &cells);
+ printk("PCI SPACE 0x%08x, 0x%"PRIx64" maps to
0x%"PRIx64" size 0x%"PRIx64"\n",
+ pci_space, child_addr, host_addr, length);
+ }
+ return 0;
+}
+
+static int map_device_ranges(struct domain *d, const struct dt_device_node
*dev)
+{
+ const struct dt_property *ranges;
+ u32 len;
+
+ ranges = dt_get_property(dev, "ranges", &len);
+ /* No ranges, nothing to do */
+ if ( !ranges )
+ return 0;
+
+ if ( dt_device_type_is_equal(dev, "pci") )
+ return map_pci_device_ranges(d, dev, ranges);
+
+ printk("Cannot handle ranges for non-PCI device type %s\n",
dev->type);
+ /* Lets not worry for now... */
+ return 0;
+}
+
/* Map the device in the domain */
static int map_device(struct domain *d, const struct dt_device_node *dev)
{
@@ -767,6 +830,9 @@ static int map_device(struct domain *d, const struct
dt_device_node *dev)
DPRINT("addr %u = 0x%"PRIx64" -
0x%"PRIx64"\n",
i, addr, addr + size - 1);
+ if ( size == 0 )
+ continue;
+
res = map_mmio_regions(d, addr & PAGE_MASK,
PAGE_ALIGN(addr + size) - 1,
addr & PAGE_MASK);
@@ -779,6 +845,8 @@ static int map_device(struct domain *d, const struct
dt_device_node *dev)
}
}
+ res = map_device_ranges(d, dev);
+
return 0;
}
@@ -903,6 +971,41 @@ static int prepare_dtb(struct domain *d, struct kernel_info
*kinfo)
if ( ret )
goto err;
+ {
+ struct dt_irq irq;
+
+ ret = map_mmio_regions(d,
+ 0xe000000000UL,
+ 0xe00fffffffUL,
+ 0xe000000000UL);
+ if (ret) printk("PCI REGION 0 failed\n");
+ ret = map_mmio_regions(d,
+ 0xe080000000UL,
+ 0xe08fffffffUL,
+ 0xe080000000UL);
+ if (ret) printk("PCI REGION 1 failed\n");
+ ret = map_mmio_regions(d,
+ 0xe010000000UL,
+ 0xe010000000UL,
+ 0xe01000ffffUL);
+ if (ret) printk("PCI REGION 2 failed\n");
+
+ irq.type = 0x4;
+
+ irq.irq = 0xc2 + 32;
+ ret = gic_route_irq_to_guest(d, &irq, "PCI#INTA");
+ if (ret) printk("PCI INTA failed\n");
+ irq.irq = 0xc3 + 32;
+ ret = gic_route_irq_to_guest(d, &irq, "PCI#INTB");
+ if (ret) printk("PCI INTB failed\n");
+ irq.irq = 0xc4 + 32;
+ ret = gic_route_irq_to_guest(d, &irq, "PCI#INTC");
+ if (ret) printk("PCI INTC failed\n");
+ irq.irq = 0xc5 + 32;
+ ret = gic_route_irq_to_guest(d, &irq, "PCI#INTD");
+ if (ret) printk("PCI INTD failed\n");
+ }
+
ret = fdt_finish(kinfo->fdt);
if ( ret < 0 )
goto err;
--
1.7.10.4
Julien Grall
2013-Nov-20 16:17 UTC
Re: [PATCH 01/16] xen: arm64: Add 8250 earlyprintk support
On 11/20/2013 02:48 PM, Ian Campbell wrote:> From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > > Extracted from "Basic Platform support for APM X-Gene Storm." > > Signed-off-by: Anup Patel <anup.patel@linaro.org> > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > > Reworked into generic 8250 driver, use EARLY_UART_REG_SHIFT. > > While there observe a missing shift in the arm32 version (UART_THR is zero so > it doesn''t really matter). Changed for consistency. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > --- > xen/arch/arm/Rules.mk | 6 +++++ > xen/arch/arm/arm64/debug-8250.inc | 52 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 58 insertions(+) > create mode 100644 xen/arch/arm/arm64/debug-8250.inc > > diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk > index c27c2eb..aaa203e 100644 > --- a/xen/arch/arm/Rules.mk > +++ b/xen/arch/arm/Rules.mk > @@ -82,6 +82,12 @@ EARLY_PRINTK_INC := 8250 > EARLY_UART_BASE_ADDRESS := 0xF0406B00 > EARLY_UART_REG_SHIFT := 2 > endif > +ifeq ($(CONFIG_EARLY_PRINTK), xgene-storm) > +EARLY_PRINTK_INC := 8250 > +EARLY_PRINTK_BAUD := 115200 > +EARLY_UART_BASE_ADDRESS := 0x1c020000 > +EARLY_UART_REG_SHIFT := 2 > +endif > > ifneq ($(EARLY_PRINTK_INC),) > EARLY_PRINTK := y > diff --git a/xen/arch/arm/arm64/debug-8250.inc b/xen/arch/arm/arm64/debug-8250.inc > new file mode 100644 > index 0000000..5858354 > --- /dev/null > +++ b/xen/arch/arm/arm64/debug-8250.inc > @@ -0,0 +1,52 @@ > +/* > + * xen/arch/arm/arm64/debug-8250.inc > + * > + * 8250 specific debug code > + * > + * Copyright (c) 2013 Applied Micro. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <xen/8250-uart.h> > + > +/* UART initialization > + * rb: register which contains the UART base address > + * rc: scratch register 1 > + * rd: scratch register 2 */ > +.macro early_uart_init rb rc rd > +.endmYou don''t need to define early_uart_init. This macro is only used if EARLY_PRINTK_INIT_UART is defined, which it''s not the case here. -- Julien Grall
Julien Grall
2013-Nov-20 16:23 UTC
Re: [PATCH 02/16] xen: arm64: Add Basic Platform support for APM X-Gene Storm.
On 11/20/2013 02:48 PM, Ian Campbell wrote:> From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > > This patch adds initial platform stubs for APM X-Gene. > > Signed-off-by: Anup Patel <anup.patel@linaro.org> > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > > Drop earlyprintk (split into earlier patch). Only build on ARM64. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > --- > xen/arch/arm/platforms/Makefile | 1 + > xen/arch/arm/platforms/xgene-storm.c | 57 ++++++++++++++++++++++++++++++++++ > 2 files changed, 58 insertions(+) > create mode 100644 xen/arch/arm/platforms/xgene-storm.c > > diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile > index f0dd72c..680364f 100644 > --- a/xen/arch/arm/platforms/Makefile > +++ b/xen/arch/arm/platforms/Makefile > @@ -3,3 +3,4 @@ obj-$(CONFIG_ARM_32) += exynos5.o > obj-$(CONFIG_ARM_32) += midway.o > obj-$(CONFIG_ARM_32) += omap5.o > obj-$(CONFIG_ARM_32) += sunxi.o > +obj-$(CONFIG_ARM_64) += xgene-storm.o > diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c > new file mode 100644 > index 0000000..8e2b3b6 > --- /dev/null > +++ b/xen/arch/arm/platforms/xgene-storm.c > @@ -0,0 +1,57 @@ > +/* > + * xen/arch/arm/platforms/xgene-storm.c > + * > + * Applied Micro''s X-Gene specific settings > + * > + * Pranavkumar Sawargaonkar <psawargaonkar@apm.com> > + * Anup Patel <apatel@apm.com> > + * Copyright (c) 2013 Applied Micro. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <xen/config.h> > +#include <xen/device_tree.h> > +#include <xen/domain_page.h> > +#include <xen/mm.h> > +#include <xen/vmap.h> > +#include <asm/platform.h> > +#include <asm/early_printk.h> > + > +static void xgene_storm_reset(void) > +{ > +} > + > +static int xgene_storm_init(void) > +{ > + return 0; > +}There is no need to create callbacks if they are empty. Perhaps, this patch should be merge with #5 "Enable 1:1 Workaround .."? -- Julien Grall
Julien Grall
2013-Nov-20 16:23 UTC
Re: [PATCH 03/16] xen: arm64: Add APM implementor id to processor implementers.
On 11/20/2013 02:48 PM, Ian Campbell wrote:> From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > > This patch updates the list of processor implementers with APM implementor id. > > Signed-off-by: Anup Patel <anup.patel@linaro.org> > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>Acked-by: Julien Grall <julien.grall@linaro.org>> --- > xen/arch/arm/setup.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index cdcc2e7..49e1b5c 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -64,6 +64,7 @@ static const char * __initdata processor_implementers[] = { > [''B''] = "Broadcom Corporation", > [''D''] = "Digital Equipment Corp", > [''M''] = "Motorola, Freescale Semiconductor Inc.", > + [''P''] = "Applied Micro", > [''Q''] = "Qualcomm Inc.", > [''V''] = "Marvell Semiconductor Inc.", > [''i''] = "Intel Corporation", >-- Julien Grall
Julien Grall
2013-Nov-20 16:24 UTC
Re: [PATCH 04/16] xen: arm: include ns16550 driver on arm64 too
On 11/20/2013 02:48 PM, Ian Campbell wrote:> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>Acked-by: Julien Grall <julien.grall@linaro.org>> --- > config/arm64.mk | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/config/arm64.mk b/config/arm64.mk > index 49055fa..15b57a4 100644 > --- a/config/arm64.mk > +++ b/config/arm64.mk > @@ -7,6 +7,7 @@ CONFIG_XEN_INSTALL_SUFFIX :> CFLAGS += #-marm -march= -mcpu= etc > > HAS_PL011 := y > +HAS_NS16550 := y > > # Use only if calling $(LD) directly. > LDFLAGS_DIRECT += -EL >-- Julien Grall
Julien Grall
2013-Nov-20 16:25 UTC
Re: [PATCH 06/16] xen: arm: early logging of command line
On 11/20/2013 02:48 PM, Ian Campbell wrote:> Helpful for diagnosis of bad console= parameters. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>Thanks, I wanted to create a similar patch ! :) I think it can be apply now without the other patches. Acked-by: Julien Grall <julien.grall@linaro.org>> --- > xen/arch/arm/setup.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index 49e1b5c..d252131 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -597,6 +597,7 @@ void __init start_xen(unsigned long boot_phys_offset, > { > size_t fdt_size; > int cpus, i; > + const char *cmdline; > > setup_cache(); > > @@ -610,7 +611,9 @@ void __init start_xen(unsigned long boot_phys_offset, > + (fdt_paddr & ((1 << SECOND_SHIFT) - 1)); > fdt_size = device_tree_early_init(device_tree_flattened, fdt_paddr); > > - cmdline_parse(device_tree_bootargs(device_tree_flattened)); > + cmdline = device_tree_bootargs(device_tree_flattened); > + early_printk("Command line: %s\n", cmdline); > + cmdline_parse(cmdline); > > setup_pagetables(boot_phys_offset, get_xen_paddr()); > setup_mm(fdt_paddr, fdt_size); >-- Julien Grall
Julien Grall
2013-Nov-20 16:31 UTC
Re: [PATCH 07/16] xen: arm: Handle cpus nodes with #address-calls > 1
On 11/20/2013 02:48 PM, Ian Campbell wrote:> The APM X-Gene Mustang board DTS has #address-cells = 2. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > --- > xen/arch/arm/smpboot.c | 25 +++++++++++++++++++++---- > 1 file changed, 21 insertions(+), 4 deletions(-) > > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c > index 6c90fa6..ce832ae 100644 > --- a/xen/arch/arm/smpboot.c > +++ b/xen/arch/arm/smpboot.c > @@ -125,18 +125,35 @@ void __init smp_init_cpus(void) > > dt_for_each_child_node( cpus, cpu ) > { > + const __be32 *prop; > + u64 addr, size; > u32 hwid; > > if ( !dt_device_type_is_equal(cpu, "cpu") ) > continue; > > - if ( !dt_property_read_u32(cpu, "reg", &hwid) ) > + if ( dt_n_size_cells(cpu) != 0 ) > + printk(XENLOG_WARNING "cpu node `%s`: #size-cells %d\n", > + dt_node_full_name(cpu), dt_n_size_cells(cpu)); > + > + prop = dt_get_property(cpu, "reg", NULL); > + if ( !prop ) > { > - printk(XENLOG_WARNING "cpu node `%s`: missing reg property\n", > + printk(XENLOG_WARNING "cpu node `%s`: has no reg property\n", > dt_node_full_name(cpu)); > continue; > } > > + dt_get_range(&prop, cpu, &addr, &size);You can use dt_read_number here.> + > + hwid = addr; > + if ( hwid != addr ) > + { > + printk(XENLOG_WARNING "cpu node `%s`: hwid overflow %"PRIx64"\n", > + dt_node_full_name(cpu), addr); > + continue; > + } > + > /* > * 8 MSBs must be set to 0 in the DT since the reg property > * defines the MPIDR[23:0] > @@ -159,8 +176,8 @@ void __init smp_init_cpus(void) > if ( tmp_map[j] == hwid ) > { > printk(XENLOG_WARNING > - "cpu node `%s`: duplicate /cpu reg properties in the DT\n", > - dt_node_full_name(cpu)); > + "cpu node `%s`: duplicate /cpu reg properties %"PRIx32" in the DT\n", > + dt_node_full_name(cpu), hwid); > break; > } > } >-- Julien Grall
Ian Campbell
2013-Nov-20 16:37 UTC
Re: [PATCH 07/16] xen: arm: Handle cpus nodes with #address-calls > 1
On Wed, 2013-11-20 at 16:31 +0000, Julien Grall wrote:> On 11/20/2013 02:48 PM, Ian Campbell wrote: > > The APM X-Gene Mustang board DTS has #address-cells = 2. > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > --- > > xen/arch/arm/smpboot.c | 25 +++++++++++++++++++++---- > > 1 file changed, 21 insertions(+), 4 deletions(-) > > > > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c > > index 6c90fa6..ce832ae 100644 > > --- a/xen/arch/arm/smpboot.c > > +++ b/xen/arch/arm/smpboot.c > > @@ -125,18 +125,35 @@ void __init smp_init_cpus(void) > > > > dt_for_each_child_node( cpus, cpu ) > > { > > + const __be32 *prop; > > + u64 addr, size; > > u32 hwid; > > > > if ( !dt_device_type_is_equal(cpu, "cpu") ) > > continue; > > > > - if ( !dt_property_read_u32(cpu, "reg", &hwid) ) > > + if ( dt_n_size_cells(cpu) != 0 ) > > + printk(XENLOG_WARNING "cpu node `%s`: #size-cells %d\n", > > + dt_node_full_name(cpu), dt_n_size_cells(cpu)); > > + > > + prop = dt_get_property(cpu, "reg", NULL); > > + if ( !prop ) > > { > > - printk(XENLOG_WARNING "cpu node `%s`: missing reg property\n", > > + printk(XENLOG_WARNING "cpu node `%s`: has no reg property\n", > > dt_node_full_name(cpu)); > > continue; > > } > > > > + dt_get_range(&prop, cpu, &addr, &size); > > You can use dt_read_number here.As in dt_read_number(&prop, dt_n_addr_cells(cpu) ? Is there a helper which will take (&prop, cpu, &addr) and figure out the size etc internally, that''s really what I was looking for. Ian.
Julien Grall
2013-Nov-20 16:46 UTC
Re: [PATCH 07/16] xen: arm: Handle cpus nodes with #address-calls > 1
On 11/20/2013 04:37 PM, Ian Campbell wrote:> On Wed, 2013-11-20 at 16:31 +0000, Julien Grall wrote: >> On 11/20/2013 02:48 PM, Ian Campbell wrote: >>> The APM X-Gene Mustang board DTS has #address-cells = 2. >>> >>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com> >>> --- >>> xen/arch/arm/smpboot.c | 25 +++++++++++++++++++++---- >>> 1 file changed, 21 insertions(+), 4 deletions(-) >>> >>> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c >>> index 6c90fa6..ce832ae 100644 >>> --- a/xen/arch/arm/smpboot.c >>> +++ b/xen/arch/arm/smpboot.c >>> @@ -125,18 +125,35 @@ void __init smp_init_cpus(void) >>> >>> dt_for_each_child_node( cpus, cpu ) >>> { >>> + const __be32 *prop; >>> + u64 addr, size; >>> u32 hwid; >>> >>> if ( !dt_device_type_is_equal(cpu, "cpu") ) >>> continue; >>> >>> - if ( !dt_property_read_u32(cpu, "reg", &hwid) ) >>> + if ( dt_n_size_cells(cpu) != 0 ) >>> + printk(XENLOG_WARNING "cpu node `%s`: #size-cells %d\n", >>> + dt_node_full_name(cpu), dt_n_size_cells(cpu)); >>> + >>> + prop = dt_get_property(cpu, "reg", NULL); >>> + if ( !prop ) >>> { >>> - printk(XENLOG_WARNING "cpu node `%s`: missing reg property\n", >>> + printk(XENLOG_WARNING "cpu node `%s`: has no reg property\n", >>> dt_node_full_name(cpu)); >>> continue; >>> } >>> >>> + dt_get_range(&prop, cpu, &addr, &size); >> >> You can use dt_read_number here. > > As in > dt_read_number(&prop, dt_n_addr_cells(cpu) > ? Is there a helper which will take (&prop, cpu, &addr) and figure out > the size etc internally, that''s really what I was looking for.Except dt_get_range, no. I would prefer the solution with dt_read_number because it more clear that we want only a number. A cpu is not defined by a range. In the both case, you will also need to check the size of the property. A bad crafted device tree can have reg smaller than #address-cells + #size-cells. -- Julien Grall
Julien Grall
2013-Nov-20 17:16 UTC
Re: [PATCH 12/16] xen: arm: improve early memory map readability
On 11/20/2013 02:48 PM, Ian Campbell wrote:> Purely cosmetic, put a blank line after the early memory map to separate it > from the subsequent information. This looks better since hte memory map iss/hte/the/ Except this minor typo: Acked-by: Julien Grall <julien.grall@linaro.org>> preceded by a blank line too. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > --- > xen/common/device_tree.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c > index 44253da..943b181 100644 > --- a/xen/common/device_tree.c > +++ b/xen/common/device_tree.c > @@ -452,6 +452,7 @@ static void __init early_print_info(void) > early_printk(" RESVD[%d]: %"PRIpaddr" - %"PRIpaddr"\n", > i, s, e); > } > + early_printk("\n"); > } > > /** >-- Julien Grall
Julien Grall
2013-Nov-20 17:31 UTC
Re: [PATCH 10/16] xen: arm: enable synchronous console while starting secondary CPUs
On 11/20/2013 02:48 PM, Ian Campbell wrote:> If CONFIG_EARLY_PRINTK is in use then this gets all interleaved with the > standard logging otherwise. >Before the discussion IRL, it was not clear to me that you want to directly "flush" the console buffer when prinkt is called. Can you improve your commit message?> Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > --- > xen/arch/arm/smpboot.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c > index ce832ae..9b58345 100644 > --- a/xen/arch/arm/smpboot.c > +++ b/xen/arch/arm/smpboot.c > @@ -28,6 +28,7 @@ > #include <xen/softirq.h> > #include <xen/timer.h> > #include <xen/irq.h> > +#include <xen/console.h> > #include <asm/gic.h> > > cpumask_t cpu_online_map; > @@ -349,6 +350,8 @@ int __cpu_up(unsigned int cpu) > if ( rc < 0 ) > return rc; > > + console_start_sync(); /* Secondary may use early_printk */ > +If it''s only for early printk, can you surround console_*_sync by #ifdef CONFIG_EARLY_PRINTK?> /* Tell the remote CPU which stack to boot on. */ > init_data.stack = idle_vcpu[cpu]->arch.stack; > > @@ -361,6 +364,8 @@ int __cpu_up(unsigned int cpu) > > rc = arch_cpu_up(cpu); > > + console_end_sync(); > + > if ( rc < 0 ) > { > printk("Failed to bring up CPU%d\n", cpu); >-- Julien Grall
Julien Grall
2013-Nov-20 17:36 UTC
Re: [PATCH 11/16] xen: arm: Add debug keyhandler to dump the physical GIC state.
On 11/20/2013 02:48 PM, Ian Campbell wrote:> Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > --- > xen/arch/arm/gic.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 75 insertions(+) > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index ab49106..185a6b8 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -28,6 +28,7 @@ > #include <xen/softirq.h> > #include <xen/list.h> > #include <xen/device_tree.h> > +#include <xen/keyhandler.h> > #include <asm/p2m.h> > #include <asm/domain.h> > > @@ -385,6 +386,77 @@ int gic_irq_xlate(const u32 *intspec, unsigned int intsize, > return 0; > } > > + > +static void do_dump_gic(unsigned char key) > +{ > + int irq; > + printk("''%c'' pressed -> dumping GIC state\n", key); > + > + for ( irq = 0; irq < gic.lines; irq++ ) > + { > + const char *type; > + int type_nr, enable, pend, active, priority, target; > + struct irq_desc *desc = irq_to_desc(irq); > + uint8_t *bytereg; > + uint32_t wordreg; > + > + bytereg = (uint8_t *) (GICD + GICD_ITARGETSR); > + target = bytereg[irq]; > + > + bytereg = (uint8_t *) (GICD + GICD_IPRIORITYR); > + priority = bytereg[irq]; > + > + switch ( irq ) > + { > + case 0 ... 15: > + type = "SGI"; > + type_nr = irq; > + target = 0x00; /* these are per-CPU */ > + break; > + case 16 ... 31: > + type = "PPI"; > + type_nr = irq - 16; > + break;I think it''s a bit stupid to print SGI and PPI as it''s per-CPU interrupt. With your solution, you don''t know which CPU call the keyhandler. Perhaps, you need to an SGI to each CPU? -- Julien Grall
Ian Campbell
2013-Nov-20 17:37 UTC
Re: [PATCH 10/16] xen: arm: enable synchronous console while starting secondary CPUs
On Wed, 2013-11-20 at 17:31 +0000, Julien Grall wrote:> On 11/20/2013 02:48 PM, Ian Campbell wrote: > > If CONFIG_EARLY_PRINTK is in use then this gets all interleaved with the > > standard logging otherwise. > > > > Before the discussion IRL, it was not clear to me that you want to > directly "flush" the console buffer when prinkt is called. Can you > improve your commit message?Sure. How about: xen: arm: enable synchronous console while starting secondary CPUs Setting synchronous console ensures that any printk hits the buffer immediately and that any outstanding queued log messages are flushed. This ensures that such log messages are not being printed while the secondary CPU may be using early_printk during early bringup. Signed-off-by: Ian Campbell <ian.campbell@citrix.com>> > --- > > xen/arch/arm/smpboot.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c > > index ce832ae..9b58345 100644 > > --- a/xen/arch/arm/smpboot.c > > +++ b/xen/arch/arm/smpboot.c > > @@ -28,6 +28,7 @@ > > #include <xen/softirq.h> > > #include <xen/timer.h> > > #include <xen/irq.h> > > +#include <xen/console.h> > > #include <asm/gic.h> > > > > cpumask_t cpu_online_map; > > @@ -349,6 +350,8 @@ int __cpu_up(unsigned int cpu) > > if ( rc < 0 ) > > return rc; > > > > + console_start_sync(); /* Secondary may use early_printk */ > > + > > If it''s only for early printk, can you surround console_*_sync by #ifdef > CONFIG_EARLY_PRINTK? > > > /* Tell the remote CPU which stack to boot on. */ > > init_data.stack = idle_vcpu[cpu]->arch.stack; > > > > @@ -361,6 +364,8 @@ int __cpu_up(unsigned int cpu) > > > > rc = arch_cpu_up(cpu); > > > > + console_end_sync(); > > + > > if ( rc < 0 ) > > { > > printk("Failed to bring up CPU%d\n", cpu); > > >
Ian Campbell
2013-Nov-20 17:48 UTC
Re: [PATCH 11/16] xen: arm: Add debug keyhandler to dump the physical GIC state.
On Wed, 2013-11-20 at 17:36 +0000, Julien Grall wrote:> On 11/20/2013 02:48 PM, Ian Campbell wrote: > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > --- > > xen/arch/arm/gic.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 75 insertions(+) > > > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > > index ab49106..185a6b8 100644 > > --- a/xen/arch/arm/gic.c > > +++ b/xen/arch/arm/gic.c > > @@ -28,6 +28,7 @@ > > #include <xen/softirq.h> > > #include <xen/list.h> > > #include <xen/device_tree.h> > > +#include <xen/keyhandler.h> > > #include <asm/p2m.h> > > #include <asm/domain.h> > > > > @@ -385,6 +386,77 @@ int gic_irq_xlate(const u32 *intspec, unsigned int intsize, > > return 0; > > } > > > > + > > +static void do_dump_gic(unsigned char key) > > +{ > > + int irq; > > + printk("''%c'' pressed -> dumping GIC state\n", key); > > + > > + for ( irq = 0; irq < gic.lines; irq++ ) > > + { > > + const char *type; > > + int type_nr, enable, pend, active, priority, target; > > + struct irq_desc *desc = irq_to_desc(irq); > > + uint8_t *bytereg; > > + uint32_t wordreg; > > + > > + bytereg = (uint8_t *) (GICD + GICD_ITARGETSR); > > + target = bytereg[irq]; > > + > > + bytereg = (uint8_t *) (GICD + GICD_IPRIORITYR); > > + priority = bytereg[irq]; > > + > > + switch ( irq ) > > + { > > + case 0 ... 15: > > + type = "SGI"; > > + type_nr = irq; > > + target = 0x00; /* these are per-CPU */ > > + break; > > + case 16 ... 31: > > + type = "PPI"; > > + type_nr = irq - 16; > > + break; > > I think it''s a bit stupid to print SGI and PPI as it''s per-CPU > interrupt. With your solution, you don''t know which CPU call the keyhandler.Most of the things I was interested don''t vary across the CPUs, specifically who is receiving the interrupt (dom0 or Xen) and the priority (which is either static or consistent across the CPUs by design). The enable/pend/act flags are a bit bogus but that didn''t really matter to me at the time.> Perhaps, you need to an SGI to each CPU?I suppose I could but I didn''t need this for my debugging so I didn''t implement it. I''d rather leave this until there is a need for that functionality. I don''t much care if this patch goes in as is or not but I don''t want to spend lots more time on it. Ian.
Stefano Stabellini
2013-Nov-20 19:06 UTC
Re: [PATCH 06/16] xen: arm: early logging of command line
On Wed, 20 Nov 2013, Ian Campbell wrote:> Helpful for diagnosis of bad console= parameters. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>> xen/arch/arm/setup.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index 49e1b5c..d252131 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -597,6 +597,7 @@ void __init start_xen(unsigned long boot_phys_offset, > { > size_t fdt_size; > int cpus, i; > + const char *cmdline; > > setup_cache(); > > @@ -610,7 +611,9 @@ void __init start_xen(unsigned long boot_phys_offset, > + (fdt_paddr & ((1 << SECOND_SHIFT) - 1)); > fdt_size = device_tree_early_init(device_tree_flattened, fdt_paddr); > > - cmdline_parse(device_tree_bootargs(device_tree_flattened)); > + cmdline = device_tree_bootargs(device_tree_flattened); > + early_printk("Command line: %s\n", cmdline); > + cmdline_parse(cmdline); > > setup_pagetables(boot_phys_offset, get_xen_paddr()); > setup_mm(fdt_paddr, fdt_size); > -- > 1.7.10.4 >
Stefano Stabellini
2013-Nov-20 19:07 UTC
Re: [PATCH 02/16] xen: arm64: Add Basic Platform support for APM X-Gene Storm.
On Wed, 20 Nov 2013, Ian Campbell wrote:> From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > > This patch adds initial platform stubs for APM X-Gene. > > Signed-off-by: Anup Patel <anup.patel@linaro.org> > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > > Drop earlyprintk (split into earlier patch). Only build on ARM64. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>> xen/arch/arm/platforms/Makefile | 1 + > xen/arch/arm/platforms/xgene-storm.c | 57 ++++++++++++++++++++++++++++++++++ > 2 files changed, 58 insertions(+) > create mode 100644 xen/arch/arm/platforms/xgene-storm.c > > diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile > index f0dd72c..680364f 100644 > --- a/xen/arch/arm/platforms/Makefile > +++ b/xen/arch/arm/platforms/Makefile > @@ -3,3 +3,4 @@ obj-$(CONFIG_ARM_32) += exynos5.o > obj-$(CONFIG_ARM_32) += midway.o > obj-$(CONFIG_ARM_32) += omap5.o > obj-$(CONFIG_ARM_32) += sunxi.o > +obj-$(CONFIG_ARM_64) += xgene-storm.o > diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c > new file mode 100644 > index 0000000..8e2b3b6 > --- /dev/null > +++ b/xen/arch/arm/platforms/xgene-storm.c > @@ -0,0 +1,57 @@ > +/* > + * xen/arch/arm/platforms/xgene-storm.c > + * > + * Applied Micro''s X-Gene specific settings > + * > + * Pranavkumar Sawargaonkar <psawargaonkar@apm.com> > + * Anup Patel <apatel@apm.com> > + * Copyright (c) 2013 Applied Micro. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <xen/config.h> > +#include <xen/device_tree.h> > +#include <xen/domain_page.h> > +#include <xen/mm.h> > +#include <xen/vmap.h> > +#include <asm/platform.h> > +#include <asm/early_printk.h> > + > +static void xgene_storm_reset(void) > +{ > +} > + > +static int xgene_storm_init(void) > +{ > + return 0; > +} > + > +static const char const *xgene_storm_dt_compat[] __initdata > +{ > + "apm,xgene-storm", > + NULL > +}; > + > +PLATFORM_START(xgene_storm, "APM X-GENE STORM") > + .compatible = xgene_storm_dt_compat, > + .init = xgene_storm_init, > + .reset = xgene_storm_reset, > +PLATFORM_END > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ > -- > 1.7.10.4 >
Stefano Stabellini
2013-Nov-20 19:10 UTC
Re: [PATCH 04/16] xen: arm: include ns16550 driver on arm64 too
On Wed, 20 Nov 2013, Ian Campbell wrote:> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>> config/arm64.mk | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/config/arm64.mk b/config/arm64.mk > index 49055fa..15b57a4 100644 > --- a/config/arm64.mk > +++ b/config/arm64.mk > @@ -7,6 +7,7 @@ CONFIG_XEN_INSTALL_SUFFIX :> CFLAGS += #-marm -march= -mcpu= etc > > HAS_PL011 := y > +HAS_NS16550 := y > > # Use only if calling $(LD) directly. > LDFLAGS_DIRECT += -EL > -- > 1.7.10.4 >
Stefano Stabellini
2013-Nov-20 19:10 UTC
Re: [PATCH 03/16] xen: arm64: Add APM implementor id to processor implementers.
On Wed, 20 Nov 2013, Ian Campbell wrote:> From: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > > This patch updates the list of processor implementers with APM implementor id. > > Signed-off-by: Anup Patel <anup.patel@linaro.org> > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>> xen/arch/arm/setup.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index cdcc2e7..49e1b5c 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -64,6 +64,7 @@ static const char * __initdata processor_implementers[] = { > [''B''] = "Broadcom Corporation", > [''D''] = "Digital Equipment Corp", > [''M''] = "Motorola, Freescale Semiconductor Inc.", > + [''P''] = "Applied Micro", > [''Q''] = "Qualcomm Inc.", > [''V''] = "Marvell Semiconductor Inc.", > [''i''] = "Intel Corporation", > -- > 1.7.10.4 >
Stefano Stabellini
2013-Nov-20 19:10 UTC
Re: [PATCH 05/16] xen: arm: Enable 1:1 workaround for APM X-Gene Storm.
On Wed, 20 Nov 2013, Ian Campbell wrote:> There is no SMMU on this platform. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>> xen/arch/arm/platforms/xgene-storm.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c > index 8e2b3b6..23986a9 100644 > --- a/xen/arch/arm/platforms/xgene-storm.c > +++ b/xen/arch/arm/platforms/xgene-storm.c > @@ -35,6 +35,12 @@ static int xgene_storm_init(void) > return 0; > } > > +static uint32_t xgene_storm_quirks(void) > +{ > + return PLATFORM_QUIRK_DOM0_MAPPING_11; > +} > + > + > static const char const *xgene_storm_dt_compat[] __initdata > { > "apm,xgene-storm", > @@ -45,6 +51,7 @@ PLATFORM_START(xgene_storm, "APM X-GENE STORM") > .compatible = xgene_storm_dt_compat, > .init = xgene_storm_init, > .reset = xgene_storm_reset, > + .quirks = xgene_storm_quirks, > PLATFORM_END > > /* > -- > 1.7.10.4 >
Stefano Stabellini
2013-Nov-20 19:17 UTC
Re: [PATCH 11/16] xen: arm: Add debug keyhandler to dump the physical GIC state.
On Wed, 20 Nov 2013, Ian Campbell wrote:> Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > --- > xen/arch/arm/gic.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 75 insertions(+) > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index ab49106..185a6b8 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -28,6 +28,7 @@ > #include <xen/softirq.h> > #include <xen/list.h> > #include <xen/device_tree.h> > +#include <xen/keyhandler.h> > #include <asm/p2m.h> > #include <asm/domain.h> > > @@ -385,6 +386,77 @@ int gic_irq_xlate(const u32 *intspec, unsigned int intsize, > return 0; > } > > + > +static void do_dump_gic(unsigned char key)It might be worth renaming the current gic_dump_info to gic_dump_info_guest to avoid confusions.> + int irq; > + printk("''%c'' pressed -> dumping GIC state\n", key); > + > + for ( irq = 0; irq < gic.lines; irq++ ) > + { > + const char *type; > + int type_nr, enable, pend, active, priority, target; > + struct irq_desc *desc = irq_to_desc(irq); > + uint8_t *bytereg; > + uint32_t wordreg; > + > + bytereg = (uint8_t *) (GICD + GICD_ITARGETSR); > + target = bytereg[irq]; > + > + bytereg = (uint8_t *) (GICD + GICD_IPRIORITYR); > + priority = bytereg[irq]; > + > + switch ( irq ) > + { > + case 0 ... 15: > + type = "SGI"; > + type_nr = irq; > + target = 0x00; /* these are per-CPU */ > + break; > + case 16 ... 31: > + type = "PPI"; > + type_nr = irq - 16; > + break; > + default: > + type = "SPI"; > + type_nr = irq - 32; > + break; > + } > + > + wordreg = GICD[GICD_ISENABLER + irq / 32]; > + enable = !!(wordreg & (1u << (irq % 32))); > + wordreg = GICD[GICD_ISPENDR + irq / 32]; > + pend = !!(wordreg & (1u << (irq % 32))); > + wordreg = GICD[GICD_ISACTIVER + irq / 32]; > + active = !!(wordreg & (1u << (irq % 32))); > + > + printk("IRQ%d %s%d: %c%c%c pri:%02x tgt:%02x ", > + irq, type, type_nr, > + enable ? ''e'' : ''-'', > + pend ? ''p'' : ''-'', > + active ? ''a'' : ''-'', > + priority, target); > + > + if ( desc->status & IRQ_GUEST ) > + { > + struct domain *d = desc->action->dev_id; > + printk("dom%d %s", d->domain_id, desc->action->name); > + } > + else > + { > + printk("Xen"); > + } > + printk("\n"); > + } > + > +} > + > +static struct keyhandler dump_gic_keyhandler = { > + .irq_callback = 0, > + .u.fn = do_dump_gic, > + .desc = "dump GIC state" > +}; > + > /* Set up the GIC */ > void __init gic_init(void) > { > @@ -456,6 +528,9 @@ void __init gic_init(void) > gic_hyp_init(); > > spin_unlock(&gic.lock); > + > + register_keyhandler(''G'', &dump_gic_keyhandler); > + > } > > void send_SGI_mask(const cpumask_t *cpumask, enum gic_sgi sgi) > -- > 1.7.10.4 >
Stefano Stabellini
2013-Nov-20 19:22 UTC
Re: [PATCH 10/16] xen: arm: enable synchronous console while starting secondary CPUs
On Wed, 20 Nov 2013, Ian Campbell wrote:> If CONFIG_EARLY_PRINTK is in use then this gets all interleaved with the > standard logging otherwise. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>I am concerned because in this case console_start_sync and console_end_sync would be called before console_endboot and I can''t tell whether it is a supported.> xen/arch/arm/smpboot.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c > index ce832ae..9b58345 100644 > --- a/xen/arch/arm/smpboot.c > +++ b/xen/arch/arm/smpboot.c > @@ -28,6 +28,7 @@ > #include <xen/softirq.h> > #include <xen/timer.h> > #include <xen/irq.h> > +#include <xen/console.h> > #include <asm/gic.h> > > cpumask_t cpu_online_map; > @@ -349,6 +350,8 @@ int __cpu_up(unsigned int cpu) > if ( rc < 0 ) > return rc; > > + console_start_sync(); /* Secondary may use early_printk */ > + > /* Tell the remote CPU which stack to boot on. */ > init_data.stack = idle_vcpu[cpu]->arch.stack; > > @@ -361,6 +364,8 @@ int __cpu_up(unsigned int cpu) > > rc = arch_cpu_up(cpu); > > + console_end_sync(); > + > if ( rc < 0 ) > { > printk("Failed to bring up CPU%d\n", cpu); > -- > 1.7.10.4 >
Stefano Stabellini
2013-Nov-20 19:29 UTC
Re: [PATCH 08/16] xen: arm: Make register bit definitions unsigned.
On Wed, 20 Nov 2013, Ian Campbell wrote:> Otherwise the results of the shifting can be undefined and/or sign extended. > > Pointed out in the context of HCR_* by Pranavkumar Sawargaonkar. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > Cc: Pranavkumar Sawargaonkar <psawargaonkar@apm.com>Do you realize that by using U instead of UL, it would be 4 bytes even on ARMv8? Is that expected? If so maybe adding a line to the commit message would be good.> xen/include/asm-arm/processor.h | 144 +++++++++++++++++++-------------------- > 1 file changed, 72 insertions(+), 72 deletions(-) > > diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h > index 3da3a3d..7cd2d3e 100644 > --- a/xen/include/asm-arm/processor.h > +++ b/xen/include/asm-arm/processor.h > @@ -18,71 +18,71 @@ > #define MPIDR_INVALID (~MPIDR_HWID_MASK) > > /* TTBCR Translation Table Base Control Register */ > -#define TTBCR_EAE 0x80000000 > -#define TTBCR_N_MASK 0x07 > -#define TTBCR_N_16KB 0x00 > -#define TTBCR_N_8KB 0x01 > -#define TTBCR_N_4KB 0x02 > -#define TTBCR_N_2KB 0x03 > -#define TTBCR_N_1KB 0x04 > +#define TTBCR_EAE _AC(0x80000000,U) > +#define TTBCR_N_MASK _AC(0x07,U) > +#define TTBCR_N_16KB _AC(0x00,U) > +#define TTBCR_N_8KB _AC(0x01,U) > +#define TTBCR_N_4KB _AC(0x02,U) > +#define TTBCR_N_2KB _AC(0x03,U) > +#define TTBCR_N_1KB _AC(0x04,U) > > /* SCTLR System Control Register. */ > /* HSCTLR is a subset of this. */ > -#define SCTLR_TE (1<<30) > -#define SCTLR_AFE (1<<29) > -#define SCTLR_TRE (1<<28) > -#define SCTLR_NMFI (1<<27) > -#define SCTLR_EE (1<<25) > -#define SCTLR_VE (1<<24) > -#define SCTLR_U (1<<22) > -#define SCTLR_FI (1<<21) > -#define SCTLR_WXN (1<<19) > -#define SCTLR_HA (1<<17) > -#define SCTLR_RR (1<<14) > -#define SCTLR_V (1<<13) > -#define SCTLR_I (1<<12) > -#define SCTLR_Z (1<<11) > -#define SCTLR_SW (1<<10) > -#define SCTLR_B (1<<7) > -#define SCTLR_C (1<<2) > -#define SCTLR_A (1<<1) > -#define SCTLR_M (1<<0) > - > -#define HSCTLR_BASE 0x30c51878 > +#define SCTLR_TE (_AC(1,U)<<30) > +#define SCTLR_AFE (_AC(1,U)<<29) > +#define SCTLR_TRE (_AC(1,U)<<28) > +#define SCTLR_NMFI (_AC(1,U)<<27) > +#define SCTLR_EE (_AC(1,U)<<25) > +#define SCTLR_VE (_AC(1,U)<<24) > +#define SCTLR_U (_AC(1,U)<<22) > +#define SCTLR_FI (_AC(1,U)<<21) > +#define SCTLR_WXN (_AC(1,U)<<19) > +#define SCTLR_HA (_AC(1,U)<<17) > +#define SCTLR_RR (_AC(1,U)<<14) > +#define SCTLR_V (_AC(1,U)<<13) > +#define SCTLR_I (_AC(1,U)<<12) > +#define SCTLR_Z (_AC(1,U)<<11) > +#define SCTLR_SW (_AC(1,U)<<10) > +#define SCTLR_B (_AC(1,U)<<7) > +#define SCTLR_C (_AC(1,U)<<2) > +#define SCTLR_A (_AC(1,U)<<1) > +#define SCTLR_M (_AC(1,U)<<0) > + > +#define HSCTLR_BASE _AC(0x30c51878,U) > > /* HCR Hyp Configuration Register */ > -#define HCR_RW (1<<31) /* Register Width, ARM64 only */ > -#define HCR_TGE (1<<27) /* Trap General Exceptions */ > -#define HCR_TVM (1<<26) /* Trap Virtual Memory Controls */ > -#define HCR_TTLB (1<<25) /* Trap TLB Maintenance Operations */ > -#define HCR_TPU (1<<24) /* Trap Cache Maintenance Operations to PoU */ > -#define HCR_TPC (1<<23) /* Trap Cache Maintenance Operations to PoC */ > -#define HCR_TSW (1<<22) /* Trap Set/Way Cache Maintenance Operations */ > -#define HCR_TAC (1<<21) /* Trap ACTLR Accesses */ > -#define HCR_TIDCP (1<<20) /* Trap lockdown */ > -#define HCR_TSC (1<<19) /* Trap SMC instruction */ > -#define HCR_TID3 (1<<18) /* Trap ID Register Group 3 */ > -#define HCR_TID2 (1<<17) /* Trap ID Register Group 2 */ > -#define HCR_TID1 (1<<16) /* Trap ID Register Group 1 */ > -#define HCR_TID0 (1<<15) /* Trap ID Register Group 0 */ > -#define HCR_TWE (1<<14) /* Trap WFE instruction */ > -#define HCR_TWI (1<<13) /* Trap WFI instruction */ > -#define HCR_DC (1<<12) /* Default cacheable */ > -#define HCR_BSU_MASK (3<<10) /* Barrier Shareability Upgrade */ > -#define HCR_BSU_NONE (0<<10) > -#define HCR_BSU_INNER (1<<10) > -#define HCR_BSU_OUTER (2<<10) > -#define HCR_BSU_FULL (3<<10) > -#define HCR_FB (1<<9) /* Force Broadcast of Cache/BP/TLB operations */ > -#define HCR_VA (1<<8) /* Virtual Asynchronous Abort */ > -#define HCR_VI (1<<7) /* Virtual IRQ */ > -#define HCR_VF (1<<6) /* Virtual FIQ */ > -#define HCR_AMO (1<<5) /* Override CPSR.A */ > -#define HCR_IMO (1<<4) /* Override CPSR.I */ > -#define HCR_FMO (1<<3) /* Override CPSR.F */ > -#define HCR_PTW (1<<2) /* Protected Walk */ > -#define HCR_SWIO (1<<1) /* Set/Way Invalidation Override */ > -#define HCR_VM (1<<0) /* Virtual MMU Enable */ > +#define HCR_RW (_AC(1,U)<<31) /* Register Width, ARM64 only */ > +#define HCR_TGE (_AC(1,U)<<27) /* Trap General Exceptions */ > +#define HCR_TVM (_AC(1,U)<<26) /* Trap Virtual Memory Controls */ > +#define HCR_TTLB (_AC(1,U)<<25) /* Trap TLB Maintenance Operations */ > +#define HCR_TPU (_AC(1,U)<<24) /* Trap Cache Maintenance Operations to PoU */ > +#define HCR_TPC (_AC(1,U)<<23) /* Trap Cache Maintenance Operations to PoC */ > +#define HCR_TSW (_AC(1,U)<<22) /* Trap Set/Way Cache Maintenance Operations */ > +#define HCR_TAC (_AC(1,U)<<21) /* Trap ACTLR Accesses */ > +#define HCR_TIDCP (_AC(1,U)<<20) /* Trap lockdown */ > +#define HCR_TSC (_AC(1,U)<<19) /* Trap SMC instruction */ > +#define HCR_TID3 (_AC(1,U)<<18) /* Trap ID Register Group 3 */ > +#define HCR_TID2 (_AC(1,U)<<17) /* Trap ID Register Group 2 */ > +#define HCR_TID1 (_AC(1,U)<<16) /* Trap ID Register Group 1 */ > +#define HCR_TID0 (_AC(1,U)<<15) /* Trap ID Register Group 0 */ > +#define HCR_TWE (_AC(1,U)<<14) /* Trap WFE instruction */ > +#define HCR_TWI (_AC(1,U)<<13) /* Trap WFI instruction */ > +#define HCR_DC (_AC(1,U)<<12) /* Default cacheable */ > +#define HCR_BSU_MASK (_AC(3,U)<<10) /* Barrier Shareability Upgrade */ > +#define HCR_BSU_NONE (_AC(0,U)<<10) > +#define HCR_BSU_INNER (_AC(1,U)<<10) > +#define HCR_BSU_OUTER (_AC(2,U)<<10) > +#define HCR_BSU_FULL (_AC(3,U)<<10) > +#define HCR_FB (_AC(1,U)<<9) /* Force Broadcast of Cache/BP/TLB operations */ > +#define HCR_VA (_AC(1,U)<<8) /* Virtual Asynchronous Abort */ > +#define HCR_VI (_AC(1,U)<<7) /* Virtual IRQ */ > +#define HCR_VF (_AC(1,U)<<6) /* Virtual FIQ */ > +#define HCR_AMO (_AC(1,U)<<5) /* Override CPSR.A */ > +#define HCR_IMO (_AC(1,U)<<4) /* Override CPSR.I */ > +#define HCR_FMO (_AC(1,U)<<3) /* Override CPSR.F */ > +#define HCR_PTW (_AC(1,U)<<2) /* Protected Walk */ > +#define HCR_SWIO (_AC(1,U)<<1) /* Set/Way Invalidation Override */ > +#define HCR_VM (_AC(1,U)<<0) /* Virtual MMU Enable */ > > #define HSR_EC_UNKNOWN 0x00 > #define HSR_EC_WFI_WFE 0x01 > @@ -346,20 +346,20 @@ union hsr { > HSR_SYSREG_OP2_MASK) > > /* Physical Address Register */ > -#define PAR_F (1<<0) > +#define PAR_F (_AC(1,U)<<0) > > /* .... If F == 1 */ > #define PAR_FSC_SHIFT (1) > -#define PAR_FSC_MASK (0x3f<<PAR_FSC_SHIFT) > -#define PAR_STAGE21 (1<<8) /* Stage 2 Fault During Stage 1 Walk */ > -#define PAR_STAGE2 (1<<9) /* Stage 2 Fault */ > +#define PAR_FSC_MASK (_AC(0x3f,U)<<PAR_FSC_SHIFT) > +#define PAR_STAGE21 (_AC(1,U)<<8) /* Stage 2 Fault During Stage 1 Walk */ > +#define PAR_STAGE2 (_AC(1,U)<<9) /* Stage 2 Fault */ > > /* If F == 0 */ > #define PAR_MAIR_SHIFT 56 /* Memory Attributes */ > #define PAR_MAIR_MASK (0xffLL<<PAR_MAIR_SHIFT) > -#define PAR_NS (1<<9) /* Non-Secure */ > +#define PAR_NS (_AC(1,U)<<9) /* Non-Secure */ > #define PAR_SH_SHIFT 7 /* Shareability */ > -#define PAR_SH_MASK (3<<PAR_SH_SHIFT) > +#define PAR_SH_MASK (_AC(3,U)<<PAR_SH_SHIFT) > > /* Fault Status Register */ > /* > @@ -372,11 +372,11 @@ union hsr { > * 10xxxx -- Other > * 11xxxx -- Implementation Defined > */ > -#define FSC_TYPE_MASK (0x3<<4) > -#define FSC_TYPE_FAULT (0x00<<4) > -#define FSC_TYPE_ABT (0x01<<4) > -#define FSC_TYPE_OTH (0x02<<4) > -#define FSC_TYPE_IMPL (0x03<<4) > +#define FSC_TYPE_MASK (_AC(0x3,U)<<4) > +#define FSC_TYPE_FAULT (_AC(0x00,U)<<4) > +#define FSC_TYPE_ABT (_AC(0x01,U)<<4) > +#define FSC_TYPE_OTH (_AC(0x02,U)<<4) > +#define FSC_TYPE_IMPL (_AC(0x03,U)<<4) > > #define FSC_FLT_TRANS (0x04) > #define FSC_FLT_ACCESS (0x08) > @@ -391,7 +391,7 @@ union hsr { > #define FSC_LKD (0x34) /* Lockdown Abort */ > #define FSC_CPR (0x3a) /* Coprocossor Abort */ > > -#define FSC_LL_MASK (0x03<<0) > +#define FSC_LL_MASK (_AC(0x03,U)<<0) > > /* Time counter hypervisor control register */ > #define CNTHCTL_PA (1u<<0) /* Kernel/user access to physical counter */ > -- > 1.7.10.4 >
Stefano Stabellini
2013-Nov-20 19:31 UTC
Re: [PATCH 09/16] xen: arm: explicitly map 64 bit release address
On Wed, 20 Nov 2013, Ian Campbell wrote:> In case it is outside visible ram. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>> xen/arch/arm/arm64/smpboot.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/arm64/smpboot.c b/xen/arch/arm/arm64/smpboot.c > index 8239590..8696ed6 100644 > --- a/xen/arch/arm/arm64/smpboot.c > +++ b/xen/arch/arm/arm64/smpboot.c > @@ -4,6 +4,8 @@ > #include <xen/errno.h> > #include <xen/mm.h> > #include <xen/smp.h> > +#include <xen/vmap.h> > +#include <asm/io.h> > > struct smp_enable_ops { > int (*prepare_cpu)(int); > @@ -14,7 +16,7 @@ static struct smp_enable_ops smp_enable_ops[NR_CPUS]; > > static int __init smp_spin_table_cpu_up(int cpu) > { > - paddr_t *release; > + paddr_t __iomem *release; > > if (!cpu_release_addr[cpu]) > { > @@ -22,12 +24,20 @@ static int __init smp_spin_table_cpu_up(int cpu) > return -ENODEV; > } > > - release = __va(cpu_release_addr[cpu]); > + release = ioremap_nocache(cpu_release_addr[cpu], 8); > + if ( !release ) > + { > + dprintk(XENLOG_ERR, "CPU%d: Unable to map release address\n", cpu); > + return -EFAULT; > + } > > release[0] = __pa(init_secondary); > flush_xen_data_tlb_range_va((vaddr_t)release, sizeof(*release)); > > + iounmap(release); > + > sev(); > + > return 0; > } > > -- > 1.7.10.4 >
Ian Campbell
2013-Nov-21 10:24 UTC
Re: [PATCH 05/16] xen: arm: Enable 1:1 workaround for APM X-Gene Storm.
On Wed, 2013-11-20 at 19:10 +0000, Stefano Stabellini wrote:> On Wed, 20 Nov 2013, Ian Campbell wrote: > > There is no SMMU on this platform. > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>Thanks. I ended up merging this into "Add Basic Platform support for APM X-Gene Storm." for the (not yet posted) v2. Since you acked both I have added your ack to the result, hope that''s ok. Ian.
Ian Campbell
2013-Nov-21 10:29 UTC
Re: [PATCH 08/16] xen: arm: Make register bit definitions unsigned.
On Wed, 2013-11-20 at 19:29 +0000, Stefano Stabellini wrote:> On Wed, 20 Nov 2013, Ian Campbell wrote: > > Otherwise the results of the shifting can be undefined and/or sign extended. > > > > Pointed out in the context of HCR_* by Pranavkumar Sawargaonkar. > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > Cc: Pranavkumar Sawargaonkar <psawargaonkar@apm.com> > > Do you realize that by using U instead of UL, it would be 4 bytes even > on ARMv8? Is that expected? If so maybe adding a line to the commit > message would be good.Hrm yes. For most of the registers that is expected/desired since the actual register is 32-bits, but for HCR in particular it should be 32 bits on 32-bit and 64 bits on 64-bits, which might be tricky to arrange with these macros. I''ll see what I can do. We only actually use bits 0..31 since bits 32+33 aren''t useful to us (at least not now) so I might end up fudging it somehow...
Ian Campbell
2013-Nov-21 10:32 UTC
Re: [PATCH 10/16] xen: arm: enable synchronous console while starting secondary CPUs
On Wed, 2013-11-20 at 19:22 +0000, Stefano Stabellini wrote:> On Wed, 20 Nov 2013, Ian Campbell wrote: > > If CONFIG_EARLY_PRINTK is in use then this gets all interleaved with the > > standard logging otherwise. > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > I am concerned because in this case console_start_sync and > console_end_sync would be called before console_endboot and I can''t tell > whether it is a supported.I believe it is. Or at least I can''t see anything which would prevent it and it works in practice. The synchronousness of the console is reference counted so it doesn''t interfere with e..g the sync_console option etc. Keir CCd in case he has an opinion...> > > > xen/arch/arm/smpboot.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c > > index ce832ae..9b58345 100644 > > --- a/xen/arch/arm/smpboot.c > > +++ b/xen/arch/arm/smpboot.c > > @@ -28,6 +28,7 @@ > > #include <xen/softirq.h> > > #include <xen/timer.h> > > #include <xen/irq.h> > > +#include <xen/console.h> > > #include <asm/gic.h> > > > > cpumask_t cpu_online_map; > > @@ -349,6 +350,8 @@ int __cpu_up(unsigned int cpu) > > if ( rc < 0 ) > > return rc; > > > > + console_start_sync(); /* Secondary may use early_printk */ > > + > > /* Tell the remote CPU which stack to boot on. */ > > init_data.stack = idle_vcpu[cpu]->arch.stack; > > > > @@ -361,6 +364,8 @@ int __cpu_up(unsigned int cpu) > > > > rc = arch_cpu_up(cpu); > > > > + console_end_sync(); > > + > > if ( rc < 0 ) > > { > > printk("Failed to bring up CPU%d\n", cpu); > > -- > > 1.7.10.4 > >
Ian Campbell
2013-Nov-21 10:35 UTC
Re: [PATCH 11/16] xen: arm: Add debug keyhandler to dump the physical GIC state.
On Wed, 2013-11-20 at 19:17 +0000, Stefano Stabellini wrote:> On Wed, 20 Nov 2013, Ian Campbell wrote: > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > --- > > xen/arch/arm/gic.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 75 insertions(+) > > > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > > index ab49106..185a6b8 100644 > > --- a/xen/arch/arm/gic.c > > +++ b/xen/arch/arm/gic.c > > @@ -28,6 +28,7 @@ > > #include <xen/softirq.h> > > #include <xen/list.h> > > #include <xen/device_tree.h> > > +#include <xen/keyhandler.h> > > #include <asm/p2m.h> > > #include <asm/domain.h> > > > > @@ -385,6 +386,77 @@ int gic_irq_xlate(const u32 *intspec, unsigned int intsize, > > return 0; > > } > > > > + > > +static void do_dump_gic(unsigned char key) > > It might be worth renaming the current gic_dump_info to > gic_dump_info_guest to avoid confusions.Done although based on Julien''s comments about SGI/PPI printing I''m considering putting this patch to one side for the time being.
Julien Grall
2013-Nov-21 13:40 UTC
Re: [PATCH 10/16] xen: arm: enable synchronous console while starting secondary CPUs
On 11/20/2013 05:37 PM, Ian Campbell wrote:> On Wed, 2013-11-20 at 17:31 +0000, Julien Grall wrote: >> On 11/20/2013 02:48 PM, Ian Campbell wrote: >>> If CONFIG_EARLY_PRINTK is in use then this gets all interleaved with the >>> standard logging otherwise. >>> >> >> Before the discussion IRL, it was not clear to me that you want to >> directly "flush" the console buffer when prinkt is called. Can you >> improve your commit message? > > Sure. How about: > > xen: arm: enable synchronous console while starting secondary > CPUs > > Setting synchronous console ensures that any printk hits the > buffer immediately and that any outstanding queued log messages > are flushed. This ensures that such log messages are not being > printed while the secondary CPU may be using early_printk during > early bringup. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>This commit message looks better for me. -- Julien Grall
Julien Grall
2013-Nov-21 14:32 UTC
Re: [PATCH 16/16] HACK: xen: arm: map PCI controller ranges region MMIOs to dom0.
On 11/20/2013 02:48 PM, Ian Campbell wrote:> The ranges property of a node with device_type = "pci" is defined in ePAPR > 2.3.8. Map the appropriate MMIO regions through to dom0. > > This is a hack/PoC since it actually crashes for some reason. Hence it > contains a hacked in hardcoded list suitable for Xgene while I figure this > out. > > This should also eventually handle the interrupt-map and (ePAPR 2.4.3.1) and > possibly dma-ranges (ePAPR 2.3.9) and msi-ranges (unspeciifed?) too.For pci ranges, you can add a new bus in dt_bues. So you won''t need specific code in domain_build.c and we will be able to use it later. You can take a look to linux/drivers/of/address.c Mapping interrupt should be OK. I don''t see any reason that will fail with the current code because IRQ are all under the GIC controller. -- Julien Grall
Ian Campbell
2013-Nov-21 14:57 UTC
Re: [PATCH 16/16] HACK: xen: arm: map PCI controller ranges region MMIOs to dom0.
On Thu, 2013-11-21 at 14:32 +0000, Julien Grall wrote:> > On 11/20/2013 02:48 PM, Ian Campbell wrote: > > The ranges property of a node with device_type = "pci" is defined in ePAPR > > 2.3.8. Map the appropriate MMIO regions through to dom0. > > > > This is a hack/PoC since it actually crashes for some reason. Hence it > > contains a hacked in hardcoded list suitable for Xgene while I figure this > > out. > > > > This should also eventually handle the interrupt-map and (ePAPR 2.4.3.1) and > > possibly dma-ranges (ePAPR 2.3.9) and msi-ranges (unspeciifed?) too. > > > For pci ranges, you can add a new bus in dt_bues. So you won''t need > specific code in domain_build.c and we will be able to use it later. > You can take a look to linux/drivers/of/address.cWon''t we still need to handle the resulting extra MMIO addresses which are not part of the reg region?> Mapping interrupt should be OK. I don''t see any reason that will fail > with the current code because IRQ are all under the GIC controller.We need to parse interrupt-map because the child interrupts are not listed in the regular interrupts property. Ian.
George Dunlap
2013-Nov-21 15:05 UTC
Re: [PATCH+RFC+HACK 00/16] xen: arm initial support for xgene arm64 platform
On Wed, Nov 20, 2013 at 2:45 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:> I''m afraid this series is rather a grab bag and it is distressingly > large at this stage. With this series I can boot an Xgene board until it > fails to find its SATA controller. This is a dom0 issue for which > patches are pending from APM (/me nudges Anup). > > As well as the APM specific platform stuff there are also some generic > improvements which were either necessary or useful during this work. > Towards the tail end are some hacks and rfcs which need more work and/or > discussion. I mostly posting now because I''m aware that I''ve been > negligent in not sending these out sooner. > > WRT the freeze I think that the baseline stuff is all plausible for 4.4. > Firstly because I''m inclined to give new platform enablement a fairly > loose reign at least for the time being (and much of it was posted ages > ago by Anup/Pranavkumar). Secondly the non-platform related bits (other > than the aforementioned hacks/rfcs) fall mostly either into two buckets: > Either they are bugfixes or they are mostly obviously safe (additional > debug prints etc). > > Blow by blow analysis: > > xen: arm64: Add 8250 earlyprintk support > > New early uart driver. It is enabled as a build time > debug option and is totally harmless to platforms which > don''t use it. > > xen: arm64: Add Basic Platform support for APM X-Gene Storm. > xen: arm64: Add APM implementor id to processor implementers. > xen: arm: include ns16550 driver on arm64 too > xen: arm: Enable 1:1 workaround for APM X-Gene Storm. > > Support for the new platform. Enable an existing driver > used by that platform (already on for arm32). > > xen: arm: early logging of command line > > Pretty safe & very useful IMNSVHO.All of these look fine from a release perspective.> > xen: arm: Handle cpus nodes with #address-calls > 1So we need to make a distinction here with "bug fixes": from a release perspective, bugs are errors in the code that affect working features. What is the likelihood that any currently-supported architectures might problems without this patch? It''s not clear from looking at the patch. If it''s low-to-none, then this wouldn''t really qualify for a bug fix exemption to the code freeze.> xen: arm: Make register bit definitions unsigned. > xen: arm: explicitly map 64 bit release address > > Bug fixes.These two look fine.> > xen: arm: enable synchronous console while starting secondary CPUs > > Improves logging in a useful way. Pretty safe. > > xen: arm: Add debug keyhandler to dump the physical GIC state. > > Useful debug functionality. Harmless unless you > deliberately trigger the particular debug key. > > xen: arm: improve early memory map readability > > Cosmetic, but safe.These all look fine as well.> > RFC: xen: arm: handle 40-bit addresses in the p2m > RFC: xen: arm: allow platform code to select dom0 event channel irq > > These should be considered for cleanup review and > eventual commit for 4.4. The rest of the platform > enablement is pretty pointless without these.Hmm... it seems a bit late for RFC stuff in fairly core code. These look like they might possibly be extending functionality for currently-supported architectures as well; but without concrete examples, this would come under "new feature" rather than "bug fix". On the other hand, both of these look pretty obvious and low-risk improvements.> > HACK: xen: arm: GICC_DIR register at offset 0x10000 instead of 0x1000 > > Should be properly implemented with a view to being > accepted for 4.4. Again things are rather pointless > without. > > Could plausibly be reimplemented as a platform quirk, > which might be safer for 4.4. > > HACK: xen: arm: map PCI controller ranges region MMIOs to dom0. > > I think this one is likely to be a step too far for 4.4. > Even if it worked (it doesn''t) it is quite a big and > potentially complex change. I''m considering the option > of implementing the hardcoded list (which is here as a > HACK, see the commit message for more) via the > platform->specific_mapping callback for 4.4. In that > case it would only impact Xgene if it were broken.This is starting to look like an awful lot of "to be sorted out". (Although it looks like Julien has a simple solution that makes this last patch unnecessary?) You address risks, but you don''t address the fundamental benefit of including it now, rather than waiting to check it in for 4.5. At the moment, unless there is some compelling strategic reason for including this in 4.4, I''m inclined to say it should just wait. -George
Stefano Stabellini
2013-Nov-21 15:27 UTC
Re: [PATCH+RFC+HACK 00/16] xen: arm initial support for xgene arm64 platform
On Thu, 21 Nov 2013, George Dunlap wrote:> > HACK: xen: arm: GICC_DIR register at offset 0x10000 instead of 0x1000 > > > > Should be properly implemented with a view to being > > accepted for 4.4. Again things are rather pointless > > without. > > > > Could plausibly be reimplemented as a platform quirk, > > which might be safer for 4.4. > > > > HACK: xen: arm: map PCI controller ranges region MMIOs to dom0. > > > > I think this one is likely to be a step too far for 4.4. > > Even if it worked (it doesn''t) it is quite a big and > > potentially complex change. I''m considering the option > > of implementing the hardcoded list (which is here as a > > HACK, see the commit message for more) via the > > platform->specific_mapping callback for 4.4. In that > > case it would only impact Xgene if it were broken. > > This is starting to look like an awful lot of "to be sorted out". > (Although it looks like Julien has a simple solution that makes this > last patch unnecessary?) > > You address risks, but you don''t address the fundamental benefit of > including it now, rather than waiting to check it in for 4.5. At the > moment, unless there is some compelling strategic reason for including > this in 4.4, I''m inclined to say it should just wait.Would you be OK with accepting an alternative implementation of these two patches via platform specific hooks, as Ian suggested?
Ian Campbell
2013-Nov-21 15:38 UTC
Re: [PATCH+RFC+HACK 00/16] xen: arm initial support for xgene arm64 platform
On Thu, 2013-11-21 at 15:05 +0000, George Dunlap wrote:> On Wed, Nov 20, 2013 at 2:45 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > I''m afraid this series is rather a grab bag and it is distressingly > > large at this stage. With this series I can boot an Xgene board until it > > fails to find its SATA controller. This is a dom0 issue for which > > patches are pending from APM (/me nudges Anup). > > > > As well as the APM specific platform stuff there are also some generic > > improvements which were either necessary or useful during this work. > > Towards the tail end are some hacks and rfcs which need more work and/or > > discussion. I mostly posting now because I''m aware that I''ve been > > negligent in not sending these out sooner. > > > > WRT the freeze I think that the baseline stuff is all plausible for 4.4. > > Firstly because I''m inclined to give new platform enablement a fairly > > loose reign at least for the time being (and much of it was posted ages > > ago by Anup/Pranavkumar). Secondly the non-platform related bits (other > > than the aforementioned hacks/rfcs) fall mostly either into two buckets: > > Either they are bugfixes or they are mostly obviously safe (additional > > debug prints etc). > > > > Blow by blow analysis:Pulling your last comment first, since it informs many of my other answers:> You address risks, but you don''t address the fundamental benefit of > including it now, rather than waiting to check it in for 4.5. At the > moment, unless there is some compelling strategic reason for including > this in 4.4, I''m inclined to say it should just wait.The primary benefit is that this is the first real (i.e. non emulated) 64-bit ARM server platform on the market. Having Xen running on that early in the new year would be awesome. As well as the currently supported platforms and this one new one we should also account for people who want to port Xen 4.4 to their own platform. The closer we can make this to "drop a file in xen/arch/arm/platforms/ and add it to the Makefile" the better IMHO. Most of the patches below (i.e. the ones which haven''t already been okayed) are relevant in this light.> > xen: arm: Handle cpus nodes with #address-cells > 1 > > So we need to make a distinction here with "bug fixes": from a release > perspective, bugs are errors in the code that affect working features. > What is the likelihood that any currently-supported architectures > might problems without this patch? It''s not clear from looking at the > patch. If it''s low-to-none, then this wouldn''t really qualify for a > bug fix exemption to the code freeze.In principal it''s entirely possible that someone might rewrite the dts of such a platform this way. It''s a bit unlikely but the main reason would because as well as the cpu number #address-cells also influences things like the cpu spin address property (where it is present), which could conceivably be about 4G (albeit unlikely on 32-bit). But ultimately this is a Xen bug because it does not correctly parse a valid device tree file.> > > > RFC: xen: arm: handle 40-bit addresses in the p2m > > RFC: xen: arm: allow platform code to select dom0 event channel irq > > > > These should be considered for cleanup review and > > eventual commit for 4.4. The rest of the platform > > enablement is pretty pointless without these. > > Hmm... it seems a bit late for RFC stuff in fairly core code. These > look like they might possibly be extending functionality for > currently-supported architectures as well; but without concrete > examples, this would come under "new feature" rather than "bug fix".The 40-bit address thing is an issue on 32-bit too, it''s just that the platforms don''t typically have anything mapped up that high (MMIO tends to be lower to support non-LPAE kernels and they don''t typically have TBs of RAM). On the plus side the new case would never hit on the existing platforms. If nothing else there is currently a BUG_ON checking for that. The dom0 event channel thing is an issue for all platforms, I think we''ve just been lucky that they mostly don''t use the current hardcoded number for something, although I had it in mind that one of them did and was being hacked around.> On the other hand, both of these look pretty obvious and low-risk improvements. > > > > > HACK: xen: arm: GICC_DIR register at offset 0x10000 instead of 0x1000 > > > > Should be properly implemented with a view to being > > accepted for 4.4. Again things are rather pointless > > without. > > > > Could plausibly be reimplemented as a platform quirk, > > which might be safer for 4.4. > > > > HACK: xen: arm: map PCI controller ranges region MMIOs to dom0. > > > > I think this one is likely to be a step too far for 4.4. > > Even if it worked (it doesn''t) it is quite a big and > > potentially complex change. I''m considering the option > > of implementing the hardcoded list (which is here as a > > HACK, see the commit message for more) via the > > platform->specific_mapping callback for 4.4. In that > > case it would only impact Xgene if it were broken. > > This is starting to look like an awful lot of "to be sorted out".Yes. I was a bit unhappy to find I had these in my tree -- I thought what was outstanding was less intrusive.> (Although it looks like Julien has a simple solution that makes this > last patch unnecessary?)I don''t think Julien is right about that simpler solution being workable, but regardless I think it would be better to err on the side of caution and reimplement both of these as platform hooks for 4.4. The first would be a new quirk (only implemented by this platform) and the second would be using an existing per-platform hook. Unless that sounds obviously bogus to you I''ll put something together for you to pass judgement on. Ian.
Julien Grall
2013-Nov-21 15:42 UTC
Re: [PATCH 16/16] HACK: xen: arm: map PCI controller ranges region MMIOs to dom0.
On 11/21/2013 02:57 PM, Ian Campbell wrote:> On Thu, 2013-11-21 at 14:32 +0000, Julien Grall wrote: >> >> On 11/20/2013 02:48 PM, Ian Campbell wrote: >>> The ranges property of a node with device_type = "pci" is defined in ePAPR >>> 2.3.8. Map the appropriate MMIO regions through to dom0. >>> >>> This is a hack/PoC since it actually crashes for some reason. Hence it >>> contains a hacked in hardcoded list suitable for Xgene while I figure this >>> out. >>> >>> This should also eventually handle the interrupt-map and (ePAPR 2.4.3.1) and >>> possibly dma-ranges (ePAPR 2.3.9) and msi-ranges (unspeciifed?) too. >> >> >> For pci ranges, you can add a new bus in dt_bues. So you won''t need >> specific code in domain_build.c and we will be able to use it later. >> You can take a look to linux/drivers/of/address.c > > Won''t we still need to handle the resulting extra MMIO addresses which > are not part of the reg region?Which MMIO addresses are you talking about? I though all the ranges are described by "reg"? If the modification is too hard to implement for Xen 4.4, you can implement specific_mapping callback for APM.> >> Mapping interrupt should be OK. I don''t see any reason that will fail >> with the current code because IRQ are all under the GIC controller. > > We need to parse interrupt-map because the child interrupts are not > listed in the regular interrupts property.Right, I didn''t notice that the interrupt-map is difference for pci. -- Julien Grall
Ian Campbell
2013-Nov-21 15:53 UTC
Re: [PATCH 16/16] HACK: xen: arm: map PCI controller ranges region MMIOs to dom0.
On Thu, 2013-11-21 at 15:42 +0000, Julien Grall wrote:> > On 11/21/2013 02:57 PM, Ian Campbell wrote: > > On Thu, 2013-11-21 at 14:32 +0000, Julien Grall wrote: > >> > >> On 11/20/2013 02:48 PM, Ian Campbell wrote: > >>> The ranges property of a node with device_type = "pci" is defined in ePAPR > >>> 2.3.8. Map the appropriate MMIO regions through to dom0. > >>> > >>> This is a hack/PoC since it actually crashes for some reason. Hence it > >>> contains a hacked in hardcoded list suitable for Xgene while I figure this > >>> out. > >>> > >>> This should also eventually handle the interrupt-map and (ePAPR 2.4.3.1) and > >>> possibly dma-ranges (ePAPR 2.3.9) and msi-ranges (unspeciifed?) too. > >> > >> > >> For pci ranges, you can add a new bus in dt_bues. So you won''t need > >> specific code in domain_build.c and we will be able to use it later. > >> You can take a look to linux/drivers/of/address.c > > > > Won''t we still need to handle the resulting extra MMIO addresses which > > are not part of the reg region? > > Which MMIO addresses are you talking about? I though all the ranges are > described by "reg"?Please check ePAPR 2.3.8 -- the ranges property is used in bus nodes to define the MMIO regions used by the child devices on that bus. The reg property only covers the MMIO used by the bus controller itself. The interrupts-map is similar.> If the modification is too hard to implement for Xen 4.4, you can > implement specific_mapping callback for APM.Yes, this is what I proposed already and am looking into. Ian.
George Dunlap
2013-Nov-21 17:14 UTC
Re: [PATCH+RFC+HACK 00/16] xen: arm initial support for xgene arm64 platform
On 21/11/13 15:38, Ian Campbell wrote:> On Thu, 2013-11-21 at 15:05 +0000, George Dunlap wrote: >> On Wed, Nov 20, 2013 at 2:45 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote: >>> I''m afraid this series is rather a grab bag and it is distressingly >>> large at this stage. With this series I can boot an Xgene board until it >>> fails to find its SATA controller. This is a dom0 issue for which >>> patches are pending from APM (/me nudges Anup). >>> >>> As well as the APM specific platform stuff there are also some generic >>> improvements which were either necessary or useful during this work. >>> Towards the tail end are some hacks and rfcs which need more work and/or >>> discussion. I mostly posting now because I''m aware that I''ve been >>> negligent in not sending these out sooner. >>> >>> WRT the freeze I think that the baseline stuff is all plausible for 4.4. >>> Firstly because I''m inclined to give new platform enablement a fairly >>> loose reign at least for the time being (and much of it was posted ages >>> ago by Anup/Pranavkumar). Secondly the non-platform related bits (other >>> than the aforementioned hacks/rfcs) fall mostly either into two buckets: >>> Either they are bugfixes or they are mostly obviously safe (additional >>> debug prints etc). >>> >>> Blow by blow analysis: > Pulling your last comment first, since it informs many of my other > answers: >> You address risks, but you don''t address the fundamental benefit of >> including it now, rather than waiting to check it in for 4.5. At the >> moment, unless there is some compelling strategic reason for including >> this in 4.4, I''m inclined to say it should just wait. > The primary benefit is that this is the first real (i.e. non emulated) > 64-bit ARM server platform on the market. Having Xen running on that > early in the new year would be awesome. > > As well as the currently supported platforms and this one new one we > should also account for people who want to port Xen 4.4 to their own > platform. The closer we can make this to "drop a file in > xen/arch/arm/platforms/ and add it to the Makefile" the better IMHO. > Most of the patches below (i.e. the ones which haven''t already been > okayed) are relevant in this light.Right, so this would be for people shipping "4.4+vendor patches". If we didn''t accept these, they''d have to fix or backport all these things themselves. That sounds like a pretty compelling strategic reason. :-) And it justifies a number of the more potentially risky patches as improvements in their own right (i.e., not just for xgene, but for other bigger platforms).>>> xen: arm: Handle cpus nodes with #address-cells > 1 >> So we need to make a distinction here with "bug fixes": from a release >> perspective, bugs are errors in the code that affect working features. >> What is the likelihood that any currently-supported architectures >> might problems without this patch? It''s not clear from looking at the >> patch. If it''s low-to-none, then this wouldn''t really qualify for a >> bug fix exemption to the code freeze. > In principal it''s entirely possible that someone might rewrite the dts > of such a platform this way. It''s a bit unlikely but the main reason > would because as well as the cpu number #address-cells also influences > things like the cpu spin address property (where it is present), which > could conceivably be about 4G (albeit unlikely on 32-bit). > > But ultimately this is a Xen bug because it does not correctly parse a > valid device tree file.So just to step back and talk about principles here: Sure, and I didn''t say it wasn''t a bug. But I don''t think that the freeze exemption is to just fix bugs per se; it''s to fix broken functionality in supported features. So just the fact that something is in theory wrong with Xen isn''t enough; it has to impact features that are actually supported. Now in this case I think this distinction is unnecessary, since I buy your argument that one of the things we want to support is the "4.4+vendor patches" model; so it does impact features that we actually want to support. But if it weren''t for that, I wouldn''t accept it just on the basis that it''s a bug in Xen, if it didn''t actually affect any of the supported functionality.> >>> RFC: xen: arm: handle 40-bit addresses in the p2m >>> RFC: xen: arm: allow platform code to select dom0 event channel irq >>> >>> These should be considered for cleanup review and >>> eventual commit for 4.4. The rest of the platform >>> enablement is pretty pointless without these. >> Hmm... it seems a bit late for RFC stuff in fairly core code. These >> look like they might possibly be extending functionality for >> currently-supported architectures as well; but without concrete >> examples, this would come under "new feature" rather than "bug fix". > The 40-bit address thing is an issue on 32-bit too, it''s just that the > platforms don''t typically have anything mapped up that high (MMIO tends > to be lower to support non-LPAE kernels and they don''t typically have > TBs of RAM). > > On the plus side the new case would never hit on the existing platforms. > If nothing else there is currently a BUG_ON checking for that.Oh, right -- it looked like you might be increasing the number of pages allocated, but now I see that you''ve just replaced a ''1'' with P2M_FIRST_ORDER (which is different to P2M_FIRST_ENTRIES). That makes more sense then.>> (Although it looks like Julien has a simple solution that makes this >> last patch unnecessary?) > I don''t think Julien is right about that simpler solution being > workable, but regardless I think it would be better to err on the side > of caution and reimplement both of these as platform hooks for 4.4. The > first would be a new quirk (only implemented by this platform) and the > second would be using an existing per-platform hook. > > Unless that sounds obviously bogus to you I''ll put something together > for you to pass judgement on.Sounds good. -George
Stefano Stabellini
2013-Nov-21 18:44 UTC
Re: [PATCH 14/16] RFC: xen: arm: allow platform code to select dom0 event channel irq
On Wed, 20 Nov 2013, Ian Campbell wrote:> Currently the hardcoded use of GUEST_EVTCHN_PPI is problematic if that is a > real PPI on the platform. > > We really need to be smarter about selecting an unused PPI but in the meantime > we can at least give the platform code the option of hardcoding a number which > works for the platform. > > Hardcode a suitable PPI on the Xgene platform. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>I think this is a reasonable approach for 4.4. Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>> RFC: I''m not all that happy with this, but it seems better than nothing for > 4.4... > > If other existing platforms also have this issue I can extend the patch if > told what a suitable PPI is. > --- > xen/arch/arm/domain.c | 7 +++++-- > xen/arch/arm/platform.c | 7 +++++++ > xen/arch/arm/platforms/xgene-storm.c | 1 + > xen/include/asm-arm/platform.h | 5 +++++ > 4 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 2f57d01..52d2403 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -31,6 +31,7 @@ > #include <asm/processor-ca15.h> > > #include <asm/gic.h> > +#include <asm/platform.h> > #include "vtimer.h" > #include "vuart.h" > > @@ -526,8 +527,10 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags) > if ( (rc = vcpu_domain_init(d)) != 0 ) > goto fail; > > - /* XXX dom0 needs more intelligent selection of PPI */ > - d->arch.evtchn_irq = GUEST_EVTCHN_PPI; > + if ( d->domain_id ) > + d->arch.evtchn_irq = GUEST_EVTCHN_PPI; > + else > + d->arch.evtchn_irq = platform_dom0_evtchn_ppi(); > > /* > * Virtual UART is only used by linux early printk and decompress code. > diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c > index 0fbbdc7..a7f9ee4 100644 > --- a/xen/arch/arm/platform.c > +++ b/xen/arch/arm/platform.c > @@ -156,6 +156,13 @@ bool_t platform_device_is_blacklisted(const struct dt_device_node *node) > return dt_match_node(blacklist, node); > } > > +unsigned int platform_dom0_evtchn_ppi(void) > +{ > + if ( platform && platform->dom0_evtchn_ppi ) > + return platform->dom0_evtchn_ppi; > + return GUEST_EVTCHN_PPI; > +} > + > /* > * Local variables: > * mode: C > diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c > index 23986a9..b1b850b 100644 > --- a/xen/arch/arm/platforms/xgene-storm.c > +++ b/xen/arch/arm/platforms/xgene-storm.c > @@ -52,6 +52,7 @@ PLATFORM_START(xgene_storm, "APM X-GENE STORM") > .init = xgene_storm_init, > .reset = xgene_storm_reset, > .quirks = xgene_storm_quirks, > + .dom0_evtchn_ppi = 24, > PLATFORM_END > > /* > diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h > index c282b30..92b954d 100644 > --- a/xen/include/asm-arm/platform.h > +++ b/xen/include/asm-arm/platform.h > @@ -37,6 +37,10 @@ struct platform_desc { > * List of devices which must not pass-through to a guest > */ > const struct dt_device_match *blacklist_dev; > + /* > + * The IRQ (PPI) to use to inject event channels to dom0. > + */ > + unsigned int dom0_evtchn_ppi; > }; > > /* > @@ -56,6 +60,7 @@ void platform_reset(void); > void platform_poweroff(void); > bool_t platform_has_quirk(uint32_t quirk); > bool_t platform_device_is_blacklisted(const struct dt_device_node *node); > +unsigned int platform_dom0_evtchn_ppi(void); > > #define PLATFORM_START(_name, _namestr) \ > static const struct platform_desc __plat_desc_##_name __used \ > -- > 1.7.10.4 >
Stefano Stabellini
2013-Nov-21 19:17 UTC
Re: [PATCH 13/16] RFC: xen: arm: handle 40-bit addresses in the p2m
On Wed, 20 Nov 2013, Ian Campbell wrote:> On the X-gene platform there are resources up this high which must be mapped > to dom0. > > I''m becoming more convinced that p2m first level pages should be mapped from > the xenheap so we can avoid all this faff with figuring out which page is > needed. > > Remove the first level page from the p2m->pages list since it is actually two > pages and must be freed as such. Do so in p2m_teardown. > > I''ve also punted on the implementation of dump_p2m_lookup for high > addresses... > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > --- > This is a little bit RFC...The patch looks good but the commit message is very unclear. It is best to clarify what this patch intends to do and then separately what could be a future development.> xen/arch/arm/p2m.c | 60 +++++++++++++++++++++++++++++++++++++++++----------- > 1 file changed, 48 insertions(+), 12 deletions(-) > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index 82dda65..af32511 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -7,6 +7,10 @@ > #include <asm/flushtlb.h> > #include <asm/gic.h> > > +/* First level P2M is 2 consecutive pages */ > +#define P2M_FIRST_ORDER 1 > +#define P2M_FIRST_ENTRIES (LPAE_ENTRIES<<P2M_FIRST_ORDER) > + > void dump_p2m_lookup(struct domain *d, paddr_t addr) > { > struct p2m_domain *p2m = &d->arch.p2m; > @@ -14,6 +18,12 @@ void dump_p2m_lookup(struct domain *d, paddr_t addr) > > printk("dom%d IPA 0x%"PRIpaddr"\n", d->domain_id, addr); > > + if ( first_linear_offset(addr) > LPAE_ENTRIES ) > + { > + printk("Cannot dump addresses in second of first level pages...\n"); > + return; > + } > + > printk("P2M @ %p mfn:0x%lx\n", > p2m->first_level, page_to_mfn(p2m->first_level)); > > @@ -31,6 +41,30 @@ void p2m_load_VTTBR(struct domain *d) > isb(); /* Ensure update is visible */ > } > > +static int p2m_first_level_index(paddr_t addr) > +{ > + /* > + * 1st pages are concatenated so zeroeth offset gives us the > + * index of the 1st page > + */ > + return zeroeth_table_offset(addr); > +} > + > +/* > + * Map whichever of the first pages contain addr. The caller should > + * then use first_table_offset as an index. > + */ > +static lpae_t *p2m_map_first(struct p2m_domain *p2m, paddr_t addr) > +{ > + struct page_info *page; > + > + BUG_ON(first_linear_offset(addr) > P2M_FIRST_ENTRIES); > + > + page = p2m->first_level + p2m_first_level_index(addr); > + > + return __map_domain_page(page); > +} > + > /* > * Lookup the MFN corresponding to a domain''s PFN. > * > @@ -45,7 +79,7 @@ paddr_t p2m_lookup(struct domain *d, paddr_t paddr) > > spin_lock(&p2m->lock); > > - first = __map_domain_page(p2m->first_level); > + first = p2m_map_first(p2m, paddr); > > pte = first[first_table_offset(paddr)]; > if ( !pte.p2m.valid || !pte.p2m.table ) > @@ -135,18 +169,21 @@ static int create_p2m_entries(struct domain *d, > struct p2m_domain *p2m = &d->arch.p2m; > lpae_t *first = NULL, *second = NULL, *third = NULL; > paddr_t addr; > - unsigned long cur_first_offset = ~0, cur_second_offset = ~0; > + unsigned long cur_first_page = ~0, > + cur_first_offset = ~0, > + cur_second_offset = ~0; > > spin_lock(&p2m->lock); > > - /* XXX Don''t actually handle 40 bit guest physical addresses */ > - BUG_ON(start_gpaddr & 0x8000000000ULL); > - BUG_ON(end_gpaddr & 0x8000000000ULL); > - > - first = __map_domain_page(p2m->first_level); > - > for(addr = start_gpaddr; addr < end_gpaddr; addr += PAGE_SIZE) > { > + if ( cur_first_page != p2m_first_level_index(addr) ) > + { > + if ( first ) unmap_domain_page(first); > + first = p2m_map_first(p2m, addr); > + cur_first_page = p2m_first_level_index(addr); > + } > + > if ( !first[first_table_offset(addr)].p2m.valid ) > { > rc = p2m_create_table(d, &first[first_table_offset(addr)]); > @@ -279,15 +316,12 @@ int p2m_alloc_table(struct domain *d) > struct page_info *page; > void *p; > > - /* First level P2M is 2 consecutive pages */ > - page = alloc_domheap_pages(NULL, 1, 0); > + page = alloc_domheap_pages(NULL, P2M_FIRST_ORDER, 0); > if ( page == NULL ) > return -ENOMEM; > > spin_lock(&p2m->lock); > > - page_list_add(page, &p2m->pages); > - > /* Clear both first level pages */ > p = __map_domain_page(page); > clear_page(p); > @@ -380,6 +414,8 @@ void p2m_teardown(struct domain *d) > while ( (pg = page_list_remove_head(&p2m->pages)) ) > free_domheap_page(pg); > > + free_domheap_pages(p2m->first_level, P2M_FIRST_ORDER); > + > p2m->first_level = NULL; > > p2m_free_vmid(d); > -- > 1.7.10.4 >
Ian Campbell
2013-Nov-22 09:49 UTC
Re: [PATCH 13/16] RFC: xen: arm: handle 40-bit addresses in the p2m
On Thu, 2013-11-21 at 19:17 +0000, Stefano Stabellini wrote:> On Wed, 20 Nov 2013, Ian Campbell wrote: > > On the X-gene platform there are resources up this high which must be mapped > > to dom0. > > > > I''m becoming more convinced that p2m first level pages should be mapped from > > the xenheap so we can avoid all this faff with figuring out which page is > > needed. > > > > Remove the first level page from the p2m->pages list since it is actually two > > pages and must be freed as such. Do so in p2m_teardown. > > > > I''ve also punted on the implementation of dump_p2m_lookup for high > > addresses... > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > --- > > This is a little bit RFC... > > The patch looks good but the commit message is very unclear. It is > best to clarify what this patch intends to do and then separately what > could be a future development.Yeah, it is a bit stream of consciousness isn''t it. I''ve dropped the paragraph "I''m becoming..." which I think isn''t really necessary in the commit log. I think without that the commit message is ok, what do you think?
Possibly Parallel Threads
- [PATCH v3 00/13] xen: arm initial support for xgene arm64 platform
- [PATCH v2 02/15] xen: arm64: Add Basic Platform support for APM X-Gene Storm.
- [PATCH v5 0/7] Dissociate logical and gic/hardware CPU ID
- [PATCH] xen: arm: Fixing ttbcr (TCR_EL1 for AArch64) size.
- [PATCH v2 13/15] xen: arm: Add debug keyhandler to dump the physical GIC state.