[PATCH v5 0/8] Refactor DT specific codes preparing for ACPI support on ARM64

[PATCH v5 0/8] Refactor DT specific codes preparing for ACPI support on ARM64

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

These patches are Part 2 of the previous patch set I sent which adds
ACPI support for arm64 on Xen[1]. Split them as an individual set for
convenient reviewing.

The first patch import kconfig.h from Linux to support the use of
IS_ENABLED().
The second patch ports changes from Linux to avoid doing traditional
BIOS table scan for ARM64.
The third patch refactor acpi_os_map_memory to be architecturally
independent.
The last five patches refactor some ARM codes into generic and DT
specific parts.

CC: Doug Goldstein <cardoe@cardoe.com>
CC: Jan Beulich <jbeulich@suse.com>

Thanks,
Shannon
[1] http://lists.xenproject.org/archives/html/xen-devel/2015-11/msg01831.html


Graeme Gregory (1):
  ACPI: add config for BIOS table scan

Shannon Zhao (7):
  Kconfig: import kconfig.h from Linux 4.3
  acpi: Refactor acpi_os_map_memory to be architecturally independent
  arm/smpboot: Move dt specific code in smp to seperate functions
  arm/gic-v2: Refactor gicv2_init into generic and dt specific parts
  arm/gic-v3: Refactor gicv3_init into generic and dt specific parts
  arm/uart: Rename dt-uart.c to arm-uart.c
  pl011: Refactor pl011 driver to dt and common initialization parts

 MAINTAINERS                                |   2 +-
 xen/arch/arm/arm64/smpboot.c               |   7 +-
 xen/arch/arm/gic-v2.c                      |  21 ++++--
 xen/arch/arm/gic-v3.c                      | 114 +++++++++++++++--------------
 xen/arch/arm/smpboot.c                     |  29 +++++---
 xen/arch/x86/Kconfig                       |   1 +
 xen/drivers/acpi/Kconfig                   |   3 +
 xen/drivers/acpi/osl.c                     |  15 ++--
 xen/drivers/char/Makefile                  |   2 +-
 xen/drivers/char/{dt-uart.c => arm-uart.c} |   2 +-
 xen/drivers/char/pl011.c                   |  64 +++++++++-------
 xen/include/asm-x86/acpi.h                 |   2 +
 xen/include/xen/config.h                   |   2 +-
 xen/include/xen/kconfig.h                  |  54 ++++++++++++++
 14 files changed, 211 insertions(+), 107 deletions(-)
 rename xen/drivers/char/{dt-uart.c => arm-uart.c} (98%)
 create mode 100644 xen/include/xen/kconfig.h

-- 
2.0.4

[PATCH v5 1/8] Kconfig: import kconfig.h from Linux 4.3

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

To support using CONFIG_ options in C/CPP expressions, import kconfig.h
from the Linux v4.3 tag (commit id
6a13feb9c82803e2b815eca72fa7a9f5561d7861).

CC: Doug Goldstein <cardoe@cardoe.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 xen/include/xen/config.h  |  2 +-
 xen/include/xen/kconfig.h | 54 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 xen/include/xen/kconfig.h

diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
index 7595599..eeb49db 100644
--- a/xen/include/xen/config.h
+++ b/xen/include/xen/config.h
@@ -7,7 +7,7 @@
 #ifndef __XEN_CONFIG_H__
 #define __XEN_CONFIG_H__
 
-#include <generated/autoconf.h>
+#include <xen/kconfig.h>
 
 #ifndef __ASSEMBLY__
 #include <xen/compiler.h>
diff --git a/xen/include/xen/kconfig.h b/xen/include/xen/kconfig.h
new file mode 100644
index 0000000..d68a7ed
--- /dev/null
+++ b/xen/include/xen/kconfig.h
@@ -0,0 +1,54 @@
+#ifndef __XEN_KCONFIG_H
+#define __XEN_KCONFIG_H
+
+#include <generated/autoconf.h>
+
+/*
+ * Helper macros to use CONFIG_ options in C/CPP expressions. Note that
+ * these only work with boolean and tristate options.
+ */
+
+/*
+ * Getting something that works in C and CPP for an arg that may or may
+ * not be defined is tricky.  Here, if we have "#define CONFIG_BOOGER 1"
+ * we match on the placeholder define, insert the "0," for arg1 and generate
+ * the triplet (0, 1, 0).  Then the last step cherry picks the 2nd arg (a one).
+ * When CONFIG_BOOGER is not defined, we generate a (... 1, 0) pair, and when
+ * the last step cherry picks the 2nd arg, we get a zero.
+ */
+#define __ARG_PLACEHOLDER_1 0,
+#define config_enabled(cfg) _config_enabled(cfg)
+#define _config_enabled(value) __config_enabled(__ARG_PLACEHOLDER_##value)
+#define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk 1, 0)
+#define ___config_enabled(__ignored, val, ...) val
+
+/*
+ * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
+ * otherwise. For boolean options, this is equivalent to
+ * IS_ENABLED(CONFIG_FOO).
+ */
+#define IS_BUILTIN(option) config_enabled(option)
+
+/*
+ * IS_MODULE(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'm', 0
+ * otherwise.
+ */
+#define IS_MODULE(option) config_enabled(option##_MODULE)
+
+/*
+ * IS_REACHABLE(CONFIG_FOO) evaluates to 1 if the currently compiled
+ * code can call a function defined in code compiled based on CONFIG_FOO.
+ * This is similar to IS_ENABLED(), but returns false when invoked from
+ * built-in code when CONFIG_FOO is set to 'm'.
+ */
+#define IS_REACHABLE(option) (config_enabled(option) || \
+		 (config_enabled(option##_MODULE) && config_enabled(MODULE)))
+
+/*
+ * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
+ * 0 otherwise.
+ */
+#define IS_ENABLED(option) \
+	(IS_BUILTIN(option) || IS_MODULE(option))
+
+#endif /* __XEN_KCONFIG_H */
-- 
2.0.4

[PATCH v5 2/8] ACPI: add config for BIOS table scan

[Shannon Zhao]

From: Graeme Gregory <graeme.gregory@linaro.org>

With the addition of ARM64 that does not have a traditional BIOS to
scan, add a config option which is selected on x86 (ia64 doesn't need
it either, it is EFI/UEFI based system) to do the traditional BIOS
scanning for tables.

Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
[Linux commit 8a1664be0b922dd6afd60eca96a992ef5ec22c40]
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
Cc: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/Kconfig     | 1 +
 xen/drivers/acpi/Kconfig | 3 +++
 xen/drivers/acpi/osl.c   | 4 +++-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 7d2ed96..3a25288 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -5,6 +5,7 @@ config X86
 	def_bool y
 	select COMPAT
 	select HAS_ACPI
+	select ACPI_LEGACY_TABLES_LOOKUP if HAS_ACPI
 	select HAS_CPUFREQ
 	select HAS_EHCI
 	select HAS_GDBSX
diff --git a/xen/drivers/acpi/Kconfig b/xen/drivers/acpi/Kconfig
index 11ab5e4..82d73ca 100644
--- a/xen/drivers/acpi/Kconfig
+++ b/xen/drivers/acpi/Kconfig
@@ -2,3 +2,6 @@
 # Select HAS_ACPI if ACPI is supported
 config HAS_ACPI
 	bool
+
+config ACPI_LEGACY_TABLES_LOOKUP
+	bool
diff --git a/xen/drivers/acpi/osl.c b/xen/drivers/acpi/osl.c
index ce15470..a2fc8c4 100644
--- a/xen/drivers/acpi/osl.c
+++ b/xen/drivers/acpi/osl.c
@@ -75,12 +75,14 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
 			       "System description tables not found\n");
 			return 0;
 		}
-	} else {
+	} else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) {
 		acpi_physical_address pa = 0;
 
 		acpi_find_root_pointer(&pa);
 		return pa;
 	}
+
+	return 0;
 }
 
 void __iomem *
-- 
2.0.4

[PATCH v5 3/8] acpi: Refactor acpi_os_map_memory to be architecturally independent

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

The first Mb handling is not necessary and the attribute of __vmap() is
different for ARM. Factor the first Mb handling only for x86 and define
a mapping attribute for each architecture.

Cc: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v5: only factor the first Mb handling and the attribute of __vmap()
---
 xen/drivers/acpi/osl.c     | 11 +++++++----
 xen/include/asm-x86/acpi.h |  2 ++
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/xen/drivers/acpi/osl.c b/xen/drivers/acpi/osl.c
index a2fc8c4..63a4b1f 100644
--- a/xen/drivers/acpi/osl.c
+++ b/xen/drivers/acpi/osl.c
@@ -92,11 +92,14 @@ acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
 		mfn_t mfn = _mfn(PFN_DOWN(phys));
 		unsigned int offs = phys & (PAGE_SIZE - 1);
 
-		/* The low first Mb is always mapped. */
-		if ( !((phys + size - 1) >> 20) )
-			return __va(phys);
+		if (IS_ENABLED(CONFIG_X86)) {
+			/* The low first Mb is always mapped. */
+			if ( !((phys + size - 1) >> 20) )
+				return __va(phys);
+		}
+
 		return __vmap(&mfn, PFN_UP(offs + size), 1, 1,
-			      PAGE_HYPERVISOR_NOCACHE) + offs;
+			      ACPI_MAP_MEM_ATTR) + offs;
 	}
 	return __acpi_map_table(phys, size);
 }
diff --git a/xen/include/asm-x86/acpi.h b/xen/include/asm-x86/acpi.h
index d3bde78..d532e3d 100644
--- a/xen/include/asm-x86/acpi.h
+++ b/xen/include/asm-x86/acpi.h
@@ -163,4 +163,6 @@ void hvm_acpi_sleep_button(struct domain *d);
 void save_rest_processor_state(void);
 void restore_rest_processor_state(void);
 
+#define ACPI_MAP_MEM_ATTR	PAGE_HYPERVISOR_NOCACHE
+
 #endif /*__X86_ASM_ACPI_H*/
-- 
2.0.4

[PATCH v5 4/8] arm/smpboot: Move dt specific code in smp to seperate functions

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Partition smp initialization functions into generic and dt specific
parts, this will be useful when introducing new functions for smp
initialization based on acpi.

Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/arm64/smpboot.c |  7 ++++++-
 xen/arch/arm/smpboot.c       | 29 ++++++++++++++++++-----------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/xen/arch/arm/arm64/smpboot.c b/xen/arch/arm/arm64/smpboot.c
index 62e6abb..7928f69 100644
--- a/xen/arch/arm/arm64/smpboot.c
+++ b/xen/arch/arm/arm64/smpboot.c
@@ -70,7 +70,7 @@ int __init arch_smp_init(void)
     return 0;
 }
 
-int __init arch_cpu_init(int cpu, struct dt_device_node *dn)
+static int __init dt_arch_cpu_init(int cpu, struct dt_device_node *dn)
 {
     const char *enable_method;
 
@@ -94,6 +94,11 @@ int __init arch_cpu_init(int cpu, struct dt_device_node *dn)
     return 0;
 }
 
+int __init arch_cpu_init(int cpu, struct dt_device_node *dn)
+{
+    return dt_arch_cpu_init(cpu, dn);
+}
+
 int __init arch_cpu_up(int cpu)
 {
     if ( !smp_enable_ops[cpu].prepare_cpu )
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 00b2b2a..b6119d1 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -92,7 +92,7 @@ smp_clear_cpu_maps (void)
  * MPIDR values related to logical cpus
  * Code base on Linux arch/arm/kernel/devtree.c
  */
-void __init smp_init_cpus(void)
+static void __init dt_smp_init_cpus(void)
 {
     register_t mpidr;
     struct dt_device_node *cpus = dt_find_node_by_path("/cpus");
@@ -106,16 +106,6 @@ void __init smp_init_cpus(void)
     bool_t bootcpu_valid = 0;
     int rc;
 
-    /* scan the DTB for a PSCI node and set a global variable */
-    psci_init();
-
-    if ( (rc = arch_smp_init()) < 0 )
-    {
-        printk(XENLOG_WARNING "SMP init failed (%d)\n"
-               "Using only 1 CPU\n", rc);
-        return;
-    }
-
     mpidr = boot_cpu_data.mpidr.bits & MPIDR_HWID_MASK;
 
     if ( !cpus )
@@ -243,6 +233,23 @@ void __init smp_init_cpus(void)
     }
 }
 
+void __init smp_init_cpus(void)
+{
+    int rc;
+
+    /* initialize PSCI and set a global variable */
+    psci_init();
+
+    if ( (rc = arch_smp_init()) < 0 )
+    {
+        printk(XENLOG_WARNING "SMP init failed (%d)\n"
+               "Using only 1 CPU\n", rc);
+        return;
+    }
+
+    dt_smp_init_cpus();
+}
+
 int __init
 smp_get_max_cpus (void)
 {
-- 
2.0.4

[PATCH v5 5/8] arm/gic-v2: Refactor gicv2_init into generic and dt specific parts

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Refactor gic-v2 related functions into dt and generic parts. This will be
helpful when adding acpi support for gic.

Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/gic-v2.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 793dca7..3fb5823 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -628,13 +628,12 @@ static bool_t gicv2_is_aliased(paddr_t cbase, paddr_t csize)
     return ((val_low & 0xfff0fff) == 0x0202043B && val_low == val_high);
 }
 
-static int __init gicv2_init(void)
+static paddr_t __initdata hbase, dbase, cbase, csize, vbase;
+
+static void __init gicv2_dt_init(void)
 {
     int res;
-    paddr_t hbase, dbase;
-    paddr_t cbase, csize;
-    paddr_t vbase, vsize;
-    uint32_t aliased_offset = 0;
+    paddr_t vsize;
     const struct dt_device_node *node = gicv2_info.node;
 
     res = dt_device_get_address(node, 0, &dbase, NULL);
@@ -680,6 +679,13 @@ static int __init gicv2_init(void)
     if ( csize != vsize )
         panic("GICv2: Sizes of GICC (%#"PRIpaddr") and GICV (%#"PRIpaddr") don't match\n",
                csize, vsize);
+}
+
+static int __init gicv2_init(void)
+{
+    uint32_t aliased_offset = 0;
+
+    gicv2_dt_init();
 
     printk("GICv2 initialization:\n"
               "        gic_dist_addr=%"PRIpaddr"\n"
@@ -765,7 +771,8 @@ const static struct gic_hw_operations gicv2_ops = {
 };
 
 /* Set up the GIC */
-static int __init gicv2_preinit(struct dt_device_node *node, const void *data)
+static int __init gicv2_dt_preinit(struct dt_device_node *node,
+                                   const void *data)
 {
     gicv2_info.hw_version = GIC_V2;
     gicv2_info.node = node;
@@ -783,7 +790,7 @@ static const struct dt_device_match gicv2_dt_match[] __initconst =
 
 DT_DEVICE_START(gicv2, "GICv2", DEVICE_GIC)
         .dt_match = gicv2_dt_match,
-        .init = gicv2_preinit,
+        .init = gicv2_dt_preinit,
 DT_DEVICE_END
 
 /*
-- 
2.0.4

[PATCH v5 6/8] arm/gic-v3: Refactor gicv3_init into generic and dt specific parts

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Refactor gic-v3 related functions into dt and generic parts. This will be
helpful when adding acpi support for gic-v3.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v5: none
v4: Use INVALID_PADDR and move ioremap to common init function
---
 xen/arch/arm/gic-v3.c | 114 +++++++++++++++++++++++++++-----------------------
 1 file changed, 61 insertions(+), 53 deletions(-)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index a245b56..65a4de6 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1138,41 +1138,14 @@ static int __init cmp_rdist(const void *a, const void *b)
     return ( l->base < r->base) ? -1 : 0;
 }
 
+static paddr_t __initdata dbase = INVALID_PADDR, vbase = INVALID_PADDR;
+static paddr_t __initdata cbase = INVALID_PADDR, csize = INVALID_PADDR;
+
 /* If the GICv3 supports GICv2, initialize it */
-static void __init gicv3_init_v2(const struct dt_device_node *node,
-                                 paddr_t dbase)
+static void __init gicv3_init_v2(void)
 {
-    int res;
-    paddr_t cbase, csize;
-    paddr_t vbase, vsize;
-
-    /*
-     * For GICv3 supporting GICv2, GICC and GICV base address will be
-     * provided.
-     */
-    res = dt_device_get_address(node, 1 + gicv3.rdist_count,
-                                &cbase, &csize);
-    if ( res )
-        return;
-
-    res = dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
-                                &vbase, &vsize);
-    if ( res )
-        return;
-
-    /*
-     * We emulate a vGICv2 using a GIC CPU interface of GUEST_GICC_SIZE.
-     * So only support GICv2 on GICv3 when the virtual CPU interface is
-     * at least GUEST_GICC_SIZE.
-     */
-    if ( vsize < GUEST_GICC_SIZE )
-    {
-        printk(XENLOG_WARNING
-               "GICv3: WARNING: Not enabling support for GICv2 compat mode.\n"
-               "Size of GICV (%#"PRIpaddr") must at least be %#llx.\n",
-               vsize, GUEST_GICC_SIZE);
+    if ( cbase == INVALID_PADDR || vbase == INVALID_PADDR )
         return;
-    }
 
     printk("GICv3 compatible with GICv2 cbase %#"PRIpaddr" vbase %#"PRIpaddr"\n",
            cbase, vbase);
@@ -1180,20 +1153,12 @@ static void __init gicv3_init_v2(const struct dt_device_node *node,
     vgic_v2_setup_hw(dbase, cbase, csize, vbase, 0);
 }
 
-/* Set up the GIC */
-static int __init gicv3_init(void)
+static void __init gicv3_dt_init(void)
 {
     struct rdist_region *rdist_regs;
     int res, i;
-    uint32_t reg;
     const struct dt_device_node *node = gicv3_info.node;
-    paddr_t dbase;
-
-    if ( !cpu_has_gicv3 )
-    {
-        dprintk(XENLOG_ERR, "GICv3: driver requires system register support\n");
-        return -ENODEV;
-    }
+    paddr_t vsize;
 
     res = dt_device_get_address(node, 0, &dbase, NULL);
     if ( res )
@@ -1203,14 +1168,6 @@ static int __init gicv3_init(void)
         panic("GICv3:  Found unaligned distributor address %"PRIpaddr"",
               dbase);
 
-    gicv3.map_dbase = ioremap_nocache(dbase, SZ_64K);
-    if ( !gicv3.map_dbase )
-        panic("GICv3: Failed to ioremap for GIC distributor\n");
-
-    reg = readl_relaxed(GICD + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
-    if ( reg != GIC_PIDR2_ARCH_GICv3 && reg != GIC_PIDR2_ARCH_GICv4 )
-         panic("GICv3: no distributor detected\n");
-
     if ( !dt_property_read_u32(node, "#redistributor-regions",
                 &gicv3.rdist_count) )
         gicv3.rdist_count = 1;
@@ -1248,6 +1205,57 @@ static int __init gicv3_init(void)
         panic("GICv3: Cannot find the maintenance IRQ");
     gicv3_info.maintenance_irq = res;
 
+    /*
+     * For GICv3 supporting GICv2, GICC and GICV base address will be
+     * provided.
+     */
+    res = dt_device_get_address(node, 1 + gicv3.rdist_count,
+                                &cbase, &csize);
+    if ( res )
+        return;
+
+    res = dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
+                                &vbase, &vsize);
+    if ( res )
+        return;
+
+    /*
+     * We emulate a vGICv2 using a GIC CPU interface of GUEST_GICC_SIZE.
+     * So only support GICv2 on GICv3 when the virtual CPU interface is
+     * at least GUEST_GICC_SIZE.
+     */
+    if ( vsize < GUEST_GICC_SIZE )
+    {
+        printk(XENLOG_WARNING
+               "GICv3: WARNING: Not enabling support for GICv2 compat mode.\n"
+               "Size of GICV (%#"PRIpaddr") must at least be %#llx.\n",
+               vsize, GUEST_GICC_SIZE);
+        return;
+    }
+}
+
+/* Set up the GIC */
+static int __init gicv3_init(void)
+{
+    int res, i;
+    uint32_t reg;
+
+    if ( !cpu_has_gicv3 )
+    {
+        dprintk(XENLOG_ERR, "GICv3: driver requires system register support\n");
+        return -ENODEV;
+    }
+
+    gicv3_dt_init();
+
+    gicv3.map_dbase = ioremap_nocache(dbase, SZ_64K);
+    if ( !gicv3.map_dbase )
+        panic("GICv3: Failed to ioremap for GIC distributor\n");
+
+    reg = readl_relaxed(GICD + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
+    if ( reg != GIC_PIDR2_ARCH_GICv3 && reg != GIC_PIDR2_ARCH_GICv4 )
+         panic("GICv3: no distributor detected\n");
+
     for ( i = 0; i < gicv3.rdist_count; i++ )
     {
         /* map dbase & rdist regions */
@@ -1277,7 +1285,7 @@ static int __init gicv3_init(void)
 
     vgic_v3_setup_hw(dbase, gicv3.rdist_count, gicv3.rdist_regions,
                      gicv3.rdist_stride);
-    gicv3_init_v2(node, dbase);
+    gicv3_init_v2();
 
     spin_lock_init(&gicv3.lock);
 
@@ -1317,7 +1325,7 @@ static const struct gic_hw_operations gicv3_ops = {
     .make_hwdom_dt_node  = gicv3_make_hwdom_dt_node,
 };
 
-static int __init gicv3_preinit(struct dt_device_node *node, const void *data)
+static int __init gicv3_dt_preinit(struct dt_device_node *node, const void *data)
 {
     gicv3_info.hw_version = GIC_V3;
     gicv3_info.node = node;
@@ -1335,7 +1343,7 @@ static const struct dt_device_match gicv3_dt_match[] __initconst =
 
 DT_DEVICE_START(gicv3, "GICv3", DEVICE_GIC)
         .dt_match = gicv3_dt_match,
-        .init = gicv3_preinit,
+        .init = gicv3_dt_preinit,
 DT_DEVICE_END
 
 /*
-- 
2.0.4

[PATCH v5 7/8] arm/uart: Rename dt-uart.c to arm-uart.c

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Since we will add ACPI initialization for UART in this file later,
rename it with a generic name.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v5: make the patch readable
v4: split the original patch to renaming this and adding ACPI parts.
---
 MAINTAINERS                                | 2 +-
 xen/drivers/char/Makefile                  | 2 +-
 xen/drivers/char/{dt-uart.c => arm-uart.c} | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
 rename xen/drivers/char/{dt-uart.c => arm-uart.c} (98%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 09fe823..5d2b354 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -133,7 +133,7 @@ S:	Supported
 L:	xen-devel@lists.xen.org
 F:	xen/arch/arm/
 F:	xen/include/asm-arm/
-F:	xen/drivers/char/dt-uart.c
+F:	xen/drivers/char/arm-uart.c
 F:	xen/drivers/char/exynos4210-uart.c
 F:	xen/drivers/char/omap-uart.c
 F:	xen/drivers/char/pl011.c
diff --git a/xen/drivers/char/Makefile b/xen/drivers/char/Makefile
index aa620fc..aa169d7 100644
--- a/xen/drivers/char/Makefile
+++ b/xen/drivers/char/Makefile
@@ -6,5 +6,5 @@ obj-$(CONFIG_HAS_EXYNOS4210) += exynos4210-uart.o
 obj-$(CONFIG_HAS_OMAP) += omap-uart.o
 obj-$(CONFIG_HAS_SCIF) += scif-uart.o
 obj-$(CONFIG_HAS_EHCI) += ehci-dbgp.o
-obj-$(CONFIG_ARM) += dt-uart.o
+obj-$(CONFIG_ARM) += arm-uart.o
 obj-y += serial.o
diff --git a/xen/drivers/char/dt-uart.c b/xen/drivers/char/arm-uart.c
similarity index 98%
rename from xen/drivers/char/dt-uart.c
rename to xen/drivers/char/arm-uart.c
index d599322..883e615 100644
--- a/xen/drivers/char/dt-uart.c
+++ b/xen/drivers/char/arm-uart.c
@@ -1,5 +1,5 @@
 /*
- * xen/drivers/char/dt-uart.c
+ * xen/drivers/char/arm-uart.c
  *
  * Generic uart retrieved via the device tree
  *
-- 
2.0.4

[PATCH v5 8/8] pl011: Refactor pl011 driver to dt and common initialization parts

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Refactor pl011 driver to dt and common initialization parts. This will
be useful later when acpi specific uart initialization function is
introduced.

Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/drivers/char/pl011.c | 64 ++++++++++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 26 deletions(-)

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 67e6df5..7e16294 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -226,12 +226,42 @@ static struct uart_driver __read_mostly pl011_driver = {
     .vuart_info   = pl011_vuart,
 };
 
+static int __init pl011_uart_init(int irq, u64 addr, u64 size)
+{
+    struct pl011 *uart;
+
+    uart = &pl011_com;
+    uart->irq       = irq;
+    uart->clock_hz  = 0x16e3600;
+    uart->baud      = BAUD_AUTO;
+    uart->data_bits = 8;
+    uart->parity    = PARITY_NONE;
+    uart->stop_bits = 1;
+
+    uart->regs = ioremap_nocache(addr, size);
+    if ( !uart->regs )
+    {
+        printk("pl011: Unable to map the UART memory\n");
+        return -ENOMEM;
+    }
+
+    uart->vuart.base_addr = addr;
+    uart->vuart.size = size;
+    uart->vuart.data_off = DR;
+    uart->vuart.status_off = FR;
+    uart->vuart.status = 0;
+
+    /* Register with generic serial driver. */
+    serial_register_uart(SERHND_DTUART, &pl011_driver, uart);
+
+    return 0;
+}
+
 /* TODO: Parse UART config from the command line */
-static int __init pl011_uart_init(struct dt_device_node *dev,
-                                  const void *data)
+static int __init pl011_dt_uart_init(struct dt_device_node *dev,
+                                     const void *data)
 {
     const char *config = data;
-    struct pl011 *uart;
     int res;
     u64 addr, size;
 
@@ -240,14 +270,6 @@ static int __init pl011_uart_init(struct dt_device_node *dev,
         printk("WARNING: UART configuration is not supported\n");
     }
 
-    uart = &pl011_com;
-
-    uart->clock_hz  = 0x16e3600;
-    uart->baud      = BAUD_AUTO;
-    uart->data_bits = 8;
-    uart->parity    = PARITY_NONE;
-    uart->stop_bits = 1;
-
     res = dt_device_get_address(dev, 0, &addr, &size);
     if ( res )
     {
@@ -262,24 +284,14 @@ static int __init pl011_uart_init(struct dt_device_node *dev,
         printk("pl011: Unable to retrieve the IRQ\n");
         return -EINVAL;
     }
-    uart->irq = res;
 
-    uart->regs = ioremap_nocache(addr, size);
-    if ( !uart->regs )
+    res = pl011_uart_init(res, addr, size);
+    if ( res < 0 )
     {
-        printk("pl011: Unable to map the UART memory\n");
-        return -ENOMEM;
+        printk("pl011: Unable to initialize\n");
+        return res;
     }
 
-    uart->vuart.base_addr = addr;
-    uart->vuart.size = size;
-    uart->vuart.data_off = DR;
-    uart->vuart.status_off = FR;
-    uart->vuart.status = 0;
-
-    /* Register with generic serial driver. */
-    serial_register_uart(SERHND_DTUART, &pl011_driver, uart);
-
     dt_device_set_used_by(dev, DOMID_XEN);
 
     return 0;
@@ -293,7 +305,7 @@ static const struct dt_device_match pl011_dt_match[] __initconst =
 
 DT_DEVICE_START(pl011, "PL011 UART", DEVICE_SERIAL)
         .dt_match = pl011_dt_match,
-        .init = pl011_uart_init,
+        .init = pl011_dt_uart_init,
 DT_DEVICE_END
 
 /*
-- 
2.0.4

Re: [PATCH v5 1/8] Kconfig: import kconfig.h from Linux 4.3

[Jonathan Creekmore]

Shannon Zhao writes:

> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> To support using CONFIG_ options in C/CPP expressions, import kconfig.h
> from the Linux v4.3 tag (commit id
> 6a13feb9c82803e2b815eca72fa7a9f5561d7861).
>
> CC: Doug Goldstein <cardoe@cardoe.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  xen/include/xen/config.h  |  2 +-
>  xen/include/xen/kconfig.h | 54 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+), 1 deletion(-)
>  create mode 100644 xen/include/xen/kconfig.h
>
> diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
> index 7595599..eeb49db 100644
> --- a/xen/include/xen/config.h
> +++ b/xen/include/xen/config.h
> @@ -7,7 +7,7 @@
>  #ifndef __XEN_CONFIG_H__
>  #define __XEN_CONFIG_H__
>
> -#include <generated/autoconf.h>
> +#include <xen/kconfig.h>
>
>  #ifndef __ASSEMBLY__
>  #include <xen/compiler.h>
> diff --git a/xen/include/xen/kconfig.h b/xen/include/xen/kconfig.h
> new file mode 100644
> index 0000000..d68a7ed
> --- /dev/null
> +++ b/xen/include/xen/kconfig.h
> @@ -0,0 +1,54 @@
> +#ifndef __XEN_KCONFIG_H
> +#define __XEN_KCONFIG_H
> +
> +#include <generated/autoconf.h>
> +
> +/*
> + * Helper macros to use CONFIG_ options in C/CPP expressions. Note that
> + * these only work with boolean and tristate options.
> + */
> +
> +/*
> + * Getting something that works in C and CPP for an arg that may or may
> + * not be defined is tricky.  Here, if we have "#define CONFIG_BOOGER 1"
> + * we match on the placeholder define, insert the "0," for arg1 and generate
> + * the triplet (0, 1, 0).  Then the last step cherry picks the 2nd arg (a one).
> + * When CONFIG_BOOGER is not defined, we generate a (... 1, 0) pair, and when
> + * the last step cherry picks the 2nd arg, we get a zero.
> + */
> +#define __ARG_PLACEHOLDER_1 0,
> +#define config_enabled(cfg) _config_enabled(cfg)
> +#define _config_enabled(value) __config_enabled(__ARG_PLACEHOLDER_##value)
> +#define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk 1, 0)
> +#define ___config_enabled(__ignored, val, ...) val
> +
> +/*
> + * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
> + * otherwise. For boolean options, this is equivalent to
> + * IS_ENABLED(CONFIG_FOO).
> + */
> +#define IS_BUILTIN(option) config_enabled(option)
> +
> +/*
> + * IS_MODULE(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'm', 0
> + * otherwise.
> + */
> +#define IS_MODULE(option) config_enabled(option##_MODULE)
> +
> +/*
> + * IS_REACHABLE(CONFIG_FOO) evaluates to 1 if the currently compiled
> + * code can call a function defined in code compiled based on CONFIG_FOO.
> + * This is similar to IS_ENABLED(), but returns false when invoked from
> + * built-in code when CONFIG_FOO is set to 'm'.
> + */
> +#define IS_REACHABLE(option) (config_enabled(option) || \
> +		 (config_enabled(option##_MODULE) && config_enabled(MODULE)))
> +
> +/*
> + * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
> + * 0 otherwise.
> + */
> +#define IS_ENABLED(option) \
> +	(IS_BUILTIN(option) || IS_MODULE(option))
> +
> +#endif /* __XEN_KCONFIG_H */

I am not sure that the complexity of this file is necessary since Xen
does not support loadable modules. Essentially, IS_ENABLED(CONFIG_FOO)
is as simple as #ifdef CONFIG_FOO.

Re: [PATCH v5 2/8] ACPI: add config for BIOS table scan

[Jonathan Creekmore]

Shannon Zhao writes:

> From: Graeme Gregory <graeme.gregory@linaro.org>
>
> With the addition of ARM64 that does not have a traditional BIOS to
> scan, add a config option which is selected on x86 (ia64 doesn't need
> it either, it is EFI/UEFI based system) to do the traditional BIOS
> scanning for tables.
>
> Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> [Linux commit 8a1664be0b922dd6afd60eca96a992ef5ec22c40]
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> ---
>  xen/arch/x86/Kconfig     | 1 +
>  xen/drivers/acpi/Kconfig | 3 +++
>  xen/drivers/acpi/osl.c   | 4 +++-
>  3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 7d2ed96..3a25288 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -5,6 +5,7 @@ config X86
>  	def_bool y
>  	select COMPAT
>  	select HAS_ACPI
> +	select ACPI_LEGACY_TABLES_LOOKUP if HAS_ACPI

Since HAS_ACPI is selected right above this, it seems pointless to do
the if HAS_ACPI here. Just select ACPI_LEGACY_TABLES_LOOKUP. Or, see below.

>  	select HAS_CPUFREQ
>  	select HAS_EHCI
>  	select HAS_GDBSX
> diff --git a/xen/drivers/acpi/Kconfig b/xen/drivers/acpi/Kconfig
> index 11ab5e4..82d73ca 100644
> --- a/xen/drivers/acpi/Kconfig
> +++ b/xen/drivers/acpi/Kconfig
> @@ -2,3 +2,6 @@
>  # Select HAS_ACPI if ACPI is supported
>  config HAS_ACPI
>  	bool
> +
> +config ACPI_LEGACY_TABLES_LOOKUP
> +	bool

Or, better, default the value of ACPI_LEGACY_TABLES_LOOKUP based on
HAS_ACPI. That way, you only select HAS_ACPI to default this to on and,
if another platform besides X86 ever enabled HAS_ACPI, it would turn on
this option without you having to select it as well.

> diff --git a/xen/drivers/acpi/osl.c b/xen/drivers/acpi/osl.c
> index ce15470..a2fc8c4 100644
> --- a/xen/drivers/acpi/osl.c
> +++ b/xen/drivers/acpi/osl.c
> @@ -75,12 +75,14 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
>  			       "System description tables not found\n");
>  			return 0;
>  		}
> -	} else {
> +	} else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) {

I would use an #ifdef CONFIG_ACPI_LEGACY_TABLES_LOOKUP instead of using
the kconfig.h IS_ENABLED macro to keep from pulling that file in.

>  		acpi_physical_address pa = 0;
>
>  		acpi_find_root_pointer(&pa);
>  		return pa;
>  	}
> +
> +	return 0;
>  }
>
>  void __iomem *

Re: [PATCH v5 1/8] Kconfig: import kconfig.h from Linux 4.3

[Andrew Cooper]

On 23/01/16 17:14, Jonathan Creekmore wrote:
> Shannon Zhao writes:
>
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> To support using CONFIG_ options in C/CPP expressions, import kconfig.h
>> from the Linux v4.3 tag (commit id
>> 6a13feb9c82803e2b815eca72fa7a9f5561d7861).
>>
>> CC: Doug Goldstein <cardoe@cardoe.com>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  xen/include/xen/config.h  |  2 +-
>>  xen/include/xen/kconfig.h | 54 +++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 55 insertions(+), 1 deletion(-)
>>  create mode 100644 xen/include/xen/kconfig.h
>>
>> diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
>> index 7595599..eeb49db 100644
>> --- a/xen/include/xen/config.h
>> +++ b/xen/include/xen/config.h
>> @@ -7,7 +7,7 @@
>>  #ifndef __XEN_CONFIG_H__
>>  #define __XEN_CONFIG_H__
>>
>> -#include <generated/autoconf.h>
>> +#include <xen/kconfig.h>
>>
>>  #ifndef __ASSEMBLY__
>>  #include <xen/compiler.h>
>> diff --git a/xen/include/xen/kconfig.h b/xen/include/xen/kconfig.h
>> new file mode 100644
>> index 0000000..d68a7ed
>> --- /dev/null
>> +++ b/xen/include/xen/kconfig.h
>> @@ -0,0 +1,54 @@
>> +#ifndef __XEN_KCONFIG_H
>> +#define __XEN_KCONFIG_H
>> +
>> +#include <generated/autoconf.h>
>> +
>> +/*
>> + * Helper macros to use CONFIG_ options in C/CPP expressions. Note that
>> + * these only work with boolean and tristate options.
>> + */
>> +
>> +/*
>> + * Getting something that works in C and CPP for an arg that may or may
>> + * not be defined is tricky.  Here, if we have "#define CONFIG_BOOGER 1"
>> + * we match on the placeholder define, insert the "0," for arg1 and generate
>> + * the triplet (0, 1, 0).  Then the last step cherry picks the 2nd arg (a one).
>> + * When CONFIG_BOOGER is not defined, we generate a (... 1, 0) pair, and when
>> + * the last step cherry picks the 2nd arg, we get a zero.
>> + */
>> +#define __ARG_PLACEHOLDER_1 0,
>> +#define config_enabled(cfg) _config_enabled(cfg)
>> +#define _config_enabled(value) __config_enabled(__ARG_PLACEHOLDER_##value)
>> +#define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk 1, 0)
>> +#define ___config_enabled(__ignored, val, ...) val
>> +
>> +/*
>> + * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
>> + * otherwise. For boolean options, this is equivalent to
>> + * IS_ENABLED(CONFIG_FOO).
>> + */
>> +#define IS_BUILTIN(option) config_enabled(option)
>> +
>> +/*
>> + * IS_MODULE(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'm', 0
>> + * otherwise.
>> + */
>> +#define IS_MODULE(option) config_enabled(option##_MODULE)
>> +
>> +/*
>> + * IS_REACHABLE(CONFIG_FOO) evaluates to 1 if the currently compiled
>> + * code can call a function defined in code compiled based on CONFIG_FOO.
>> + * This is similar to IS_ENABLED(), but returns false when invoked from
>> + * built-in code when CONFIG_FOO is set to 'm'.
>> + */
>> +#define IS_REACHABLE(option) (config_enabled(option) || \
>> +		 (config_enabled(option##_MODULE) && config_enabled(MODULE)))
>> +
>> +/*
>> + * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
>> + * 0 otherwise.
>> + */
>> +#define IS_ENABLED(option) \
>> +	(IS_BUILTIN(option) || IS_MODULE(option))
>> +
>> +#endif /* __XEN_KCONFIG_H */
> I am not sure that the complexity of this file is necessary since Xen
> does not support loadable modules. Essentially, IS_ENABLED(CONFIG_FOO)
> is as simple as #ifdef CONFIG_FOO.

I would like to be able to convert some of our "#ifdef CONFIG_FOO" code
into "if ( IS_ENABLED(CONFIG_FOO) )" to reduce the quantity of code-rot
in often-disabled options.

However, I agree that we don't want all the module complexity.  i.e.
IS_ENABLED() is the only one of these we need.

~Andrew

Re: [PATCH v5 2/8] ACPI: add config for BIOS table scan

[Shannon Zhao]

On 2016/1/24 1:25, Jonathan Creekmore wrote:
> 
> Shannon Zhao writes:
> 
>> From: Graeme Gregory <graeme.gregory@linaro.org>
>>
>> With the addition of ARM64 that does not have a traditional BIOS to
>> scan, add a config option which is selected on x86 (ia64 doesn't need
>> it either, it is EFI/UEFI based system) to do the traditional BIOS
>> scanning for tables.
>>
>> Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> [Linux commit 8a1664be0b922dd6afd60eca96a992ef5ec22c40]
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>> Cc: Jan Beulich <jbeulich@suse.com>
>> ---
>>  xen/arch/x86/Kconfig     | 1 +
>>  xen/drivers/acpi/Kconfig | 3 +++
>>  xen/drivers/acpi/osl.c   | 4 +++-
>>  3 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
>> index 7d2ed96..3a25288 100644
>> --- a/xen/arch/x86/Kconfig
>> +++ b/xen/arch/x86/Kconfig
>> @@ -5,6 +5,7 @@ config X86
>>  	def_bool y
>>  	select COMPAT
>>  	select HAS_ACPI
>> +	select ACPI_LEGACY_TABLES_LOOKUP if HAS_ACPI
> 
> Since HAS_ACPI is selected right above this, it seems pointless to do
> the if HAS_ACPI here. Just select ACPI_LEGACY_TABLES_LOOKUP. Or, see below.
> 
Sure.
>>  	select HAS_CPUFREQ
>>  	select HAS_EHCI
>>  	select HAS_GDBSX
>> diff --git a/xen/drivers/acpi/Kconfig b/xen/drivers/acpi/Kconfig
>> index 11ab5e4..82d73ca 100644
>> --- a/xen/drivers/acpi/Kconfig
>> +++ b/xen/drivers/acpi/Kconfig
>> @@ -2,3 +2,6 @@
>>  # Select HAS_ACPI if ACPI is supported
>>  config HAS_ACPI
>>  	bool
>> +
>> +config ACPI_LEGACY_TABLES_LOOKUP
>> +	bool
> 
> Or, better, default the value of ACPI_LEGACY_TABLES_LOOKUP based on
> HAS_ACPI. That way, you only select HAS_ACPI to default this to on and,
> if another platform besides X86 ever enabled HAS_ACPI, it would turn on
> this option without you having to select it as well.
> 
But it wants other platform(currently is ARM) not to select this option
by default, because it's not necessary to do traditional BIOS scan on ARM64.

>> diff --git a/xen/drivers/acpi/osl.c b/xen/drivers/acpi/osl.c
>> index ce15470..a2fc8c4 100644
>> --- a/xen/drivers/acpi/osl.c
>> +++ b/xen/drivers/acpi/osl.c
>> @@ -75,12 +75,14 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
>>  			       "System description tables not found\n");
>>  			return 0;
>>  		}
>> -	} else {
>> +	} else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) {
> 
> I would use an #ifdef CONFIG_ACPI_LEGACY_TABLES_LOOKUP instead of using
> the kconfig.h IS_ENABLED macro to keep from pulling that file in.
> 
But Jan will not agree with this since I posted this patch like what you
said before, but he NACKed it.

>>  		acpi_physical_address pa = 0;
>>
>>  		acpi_find_root_pointer(&pa);
>>  		return pa;
>>  	}
>> +
>> +	return 0;
>>  }
>>
>>  void __iomem *
> 
> .
> 

-- 
Shannon

Re: [PATCH v5 1/8] Kconfig: import kconfig.h from Linux 4.3

[Shannon Zhao]

On 2016/1/24 2:42, Andrew Cooper wrote:
> On 23/01/16 17:14, Jonathan Creekmore wrote:
>> Shannon Zhao writes:
>>
>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>
>>> To support using CONFIG_ options in C/CPP expressions, import kconfig.h
>>> from the Linux v4.3 tag (commit id
>>> 6a13feb9c82803e2b815eca72fa7a9f5561d7861).
>>>
>>> CC: Doug Goldstein <cardoe@cardoe.com>
>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>> ---
>>>  xen/include/xen/config.h  |  2 +-
>>>  xen/include/xen/kconfig.h | 54 +++++++++++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 55 insertions(+), 1 deletion(-)
>>>  create mode 100644 xen/include/xen/kconfig.h
>>>
>>> diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
>>> index 7595599..eeb49db 100644
>>> --- a/xen/include/xen/config.h
>>> +++ b/xen/include/xen/config.h
>>> @@ -7,7 +7,7 @@
>>>  #ifndef __XEN_CONFIG_H__
>>>  #define __XEN_CONFIG_H__
>>>
>>> -#include <generated/autoconf.h>
>>> +#include <xen/kconfig.h>
>>>
>>>  #ifndef __ASSEMBLY__
>>>  #include <xen/compiler.h>
>>> diff --git a/xen/include/xen/kconfig.h b/xen/include/xen/kconfig.h
>>> new file mode 100644
>>> index 0000000..d68a7ed
>>> --- /dev/null
>>> +++ b/xen/include/xen/kconfig.h
>>> @@ -0,0 +1,54 @@
>>> +#ifndef __XEN_KCONFIG_H
>>> +#define __XEN_KCONFIG_H
>>> +
>>> +#include <generated/autoconf.h>
>>> +
>>> +/*
>>> + * Helper macros to use CONFIG_ options in C/CPP expressions. Note that
>>> + * these only work with boolean and tristate options.
>>> + */
>>> +
>>> +/*
>>> + * Getting something that works in C and CPP for an arg that may or may
>>> + * not be defined is tricky.  Here, if we have "#define CONFIG_BOOGER 1"
>>> + * we match on the placeholder define, insert the "0," for arg1 and generate
>>> + * the triplet (0, 1, 0).  Then the last step cherry picks the 2nd arg (a one).
>>> + * When CONFIG_BOOGER is not defined, we generate a (... 1, 0) pair, and when
>>> + * the last step cherry picks the 2nd arg, we get a zero.
>>> + */
>>> +#define __ARG_PLACEHOLDER_1 0,
>>> +#define config_enabled(cfg) _config_enabled(cfg)
>>> +#define _config_enabled(value) __config_enabled(__ARG_PLACEHOLDER_##value)
>>> +#define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk 1, 0)
>>> +#define ___config_enabled(__ignored, val, ...) val
>>> +
>>> +/*
>>> + * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
>>> + * otherwise. For boolean options, this is equivalent to
>>> + * IS_ENABLED(CONFIG_FOO).
>>> + */
>>> +#define IS_BUILTIN(option) config_enabled(option)
>>> +
>>> +/*
>>> + * IS_MODULE(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'm', 0
>>> + * otherwise.
>>> + */
>>> +#define IS_MODULE(option) config_enabled(option##_MODULE)
>>> +
>>> +/*
>>> + * IS_REACHABLE(CONFIG_FOO) evaluates to 1 if the currently compiled
>>> + * code can call a function defined in code compiled based on CONFIG_FOO.
>>> + * This is similar to IS_ENABLED(), but returns false when invoked from
>>> + * built-in code when CONFIG_FOO is set to 'm'.
>>> + */
>>> +#define IS_REACHABLE(option) (config_enabled(option) || \
>>> +		 (config_enabled(option##_MODULE) && config_enabled(MODULE)))
>>> +
>>> +/*
>>> + * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
>>> + * 0 otherwise.
>>> + */
>>> +#define IS_ENABLED(option) \
>>> +	(IS_BUILTIN(option) || IS_MODULE(option))
>>> +
>>> +#endif /* __XEN_KCONFIG_H */
>> I am not sure that the complexity of this file is necessary since Xen
>> does not support loadable modules. Essentially, IS_ENABLED(CONFIG_FOO)
>> is as simple as #ifdef CONFIG_FOO.
> 
> I would like to be able to convert some of our "#ifdef CONFIG_FOO" code
> into "if ( IS_ENABLED(CONFIG_FOO) )" to reduce the quantity of code-rot
> in often-disabled options.
> 
> However, I agree that we don't want all the module complexity.  i.e.
> IS_ENABLED() is the only one of these we need.
> 
Oh, yes. Will drop other macros.

Thanks,
-- 
Shannon

Re: [PATCH v5 7/8] arm/uart: Rename dt-uart.c to arm-uart.c

[Ian Campbell]

On Sat, 2016-01-23 at 16:00 +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Since we will add ACPI initialization for UART in this file later,
> rename it with a generic name.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [PATCH v5 1/8] Kconfig: import kconfig.h from Linux 4.3

[Jan Beulich]

>>> On 23.01.16 at 09:00, <zhaoshenglong@huawei.com> wrote:
> --- a/xen/include/xen/config.h
> +++ b/xen/include/xen/config.h
> @@ -7,7 +7,7 @@
>  #ifndef __XEN_CONFIG_H__
>  #define __XEN_CONFIG_H__
>  
> -#include <generated/autoconf.h>
> +#include <xen/kconfig.h>

Why? I don't see why all source files need to include this new
header, no matter whether they make use of any of the
definitions therein.

Jan

Re: [PATCH v5 2/8] ACPI: add config for BIOS table scan

[Jan Beulich]

>>> On 23.01.16 at 18:25, <jonathan.creekmore@gmail.com> wrote:
> Shannon Zhao writes:
>> --- a/xen/arch/x86/Kconfig
>> +++ b/xen/arch/x86/Kconfig
>> @@ -5,6 +5,7 @@ config X86
>>  	def_bool y
>>  	select COMPAT
>>  	select HAS_ACPI
>> +	select ACPI_LEGACY_TABLES_LOOKUP if HAS_ACPI
> 
> Since HAS_ACPI is selected right above this, it seems pointless to do
> the if HAS_ACPI here. Just select ACPI_LEGACY_TABLES_LOOKUP. Or, see below.

Indeed. But also beware of the alphabetical sorting here.

>> --- a/xen/drivers/acpi/Kconfig
>> +++ b/xen/drivers/acpi/Kconfig
>> @@ -2,3 +2,6 @@
>>  # Select HAS_ACPI if ACPI is supported
>>  config HAS_ACPI
>>  	bool
>> +
>> +config ACPI_LEGACY_TABLES_LOOKUP
>> +	bool
> 
> Or, better, default the value of ACPI_LEGACY_TABLES_LOOKUP based on
> HAS_ACPI. That way, you only select HAS_ACPI to default this to on and,
> if another platform besides X86 ever enabled HAS_ACPI, it would turn on
> this option without you having to select it as well.

If you're thinking of "def_bool HAS_ACPI", then no, please don't:
This needlessly adds "# CONFIG_ACPI_LEGACY_TABLES_LOOKUP is
not set" to .config, which while only a cosmetic problem here in the
general case preventsprompts to show up once an option obtains a
prompt. And I'd like to avoid setting bad precedents.

>> diff --git a/xen/drivers/acpi/osl.c b/xen/drivers/acpi/osl.c
>> index ce15470..a2fc8c4 100644
>> --- a/xen/drivers/acpi/osl.c
>> +++ b/xen/drivers/acpi/osl.c
>> @@ -75,12 +75,14 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
>>  			       "System description tables not found\n");
>>  			return 0;
>>  		}
>> -	} else {
>> +	} else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) {
> 
> I would use an #ifdef CONFIG_ACPI_LEGACY_TABLES_LOOKUP instead of using
> the kconfig.h IS_ENABLED macro to keep from pulling that file in.

I don't think I objected to this part (and in fact I agree with Andrew
that the non-preprocessor variant, where it can be used without
breaking the build, is preferable). Iirc what I objected to was that
you didn't use Kconfig.

Jan

Re: [PATCH v5 3/8] acpi: Refactor acpi_os_map_memory to be architecturally independent

[Jan Beulich]

>>> On 23.01.16 at 09:00, <zhaoshenglong@huawei.com> wrote:
> v5: only factor the first Mb handling and the attribute of __vmap()

Much better.

> --- a/xen/drivers/acpi/osl.c
> +++ b/xen/drivers/acpi/osl.c
> @@ -92,11 +92,14 @@ acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
>  		mfn_t mfn = _mfn(PFN_DOWN(phys));
>  		unsigned int offs = phys & (PAGE_SIZE - 1);
>  
> -		/* The low first Mb is always mapped. */
> -		if ( !((phys + size - 1) >> 20) )
> -			return __va(phys);
> +		if (IS_ENABLED(CONFIG_X86)) {
> +			/* The low first Mb is always mapped. */
> +			if ( !((phys + size - 1) >> 20) )
> +				return __va(phys);
> +		}

But please combine the two if()-s into one.

Jan

Re: [PATCH v5 1/8] Kconfig: import kconfig.h from Linux 4.3

[Shannon Zhao]

On 2016/1/25 22:35, Jan Beulich wrote:
>>>> On 23.01.16 at 09:00, <zhaoshenglong@huawei.com> wrote:
>> > --- a/xen/include/xen/config.h
>> > +++ b/xen/include/xen/config.h
>> > @@ -7,7 +7,7 @@
>> >  #ifndef __XEN_CONFIG_H__
>> >  #define __XEN_CONFIG_H__
>> >  
>> > -#include <generated/autoconf.h>
>> > +#include <xen/kconfig.h>
> Why? I don't see why all source files need to include this new
> header, no matter whether they make use of any of the
> definitions therein.
Will fix this. Thanks.

-- 
Shannon

Re: [PATCH v5 6/8] arm/gic-v3: Refactor gicv3_init into generic and dt specific parts

[Stefano Stabellini]

On Sat, 23 Jan 2016, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Refactor gic-v3 related functions into dt and generic parts. This will be
> helpful when adding acpi support for gic-v3.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> v5: none
> v4: Use INVALID_PADDR and move ioremap to common init function
> ---
>  xen/arch/arm/gic-v3.c | 114 +++++++++++++++++++++++++++-----------------------
>  1 file changed, 61 insertions(+), 53 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index a245b56..65a4de6 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1138,41 +1138,14 @@ static int __init cmp_rdist(const void *a, const void *b)
>      return ( l->base < r->base) ? -1 : 0;
>  }
>  
> +static paddr_t __initdata dbase = INVALID_PADDR, vbase = INVALID_PADDR;
> +static paddr_t __initdata cbase = INVALID_PADDR, csize = INVALID_PADDR;
> +
>  /* If the GICv3 supports GICv2, initialize it */
> -static void __init gicv3_init_v2(const struct dt_device_node *node,
> -                                 paddr_t dbase)
> +static void __init gicv3_init_v2(void)
>  {
> -    int res;
> -    paddr_t cbase, csize;
> -    paddr_t vbase, vsize;
> -
> -    /*
> -     * For GICv3 supporting GICv2, GICC and GICV base address will be
> -     * provided.
> -     */
> -    res = dt_device_get_address(node, 1 + gicv3.rdist_count,
> -                                &cbase, &csize);
> -    if ( res )
> -        return;
> -
> -    res = dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
> -                                &vbase, &vsize);
> -    if ( res )
> -        return;
> -
> -    /*
> -     * We emulate a vGICv2 using a GIC CPU interface of GUEST_GICC_SIZE.
> -     * So only support GICv2 on GICv3 when the virtual CPU interface is
> -     * at least GUEST_GICC_SIZE.
> -     */
> -    if ( vsize < GUEST_GICC_SIZE )
> -    {
> -        printk(XENLOG_WARNING
> -               "GICv3: WARNING: Not enabling support for GICv2 compat mode.\n"
> -               "Size of GICV (%#"PRIpaddr") must at least be %#llx.\n",
> -               vsize, GUEST_GICC_SIZE);

The vsize < GUEST_GICC_SIZE check needs to remain here because ... 


> +    if ( cbase == INVALID_PADDR || vbase == INVALID_PADDR )
>          return;
> -    }
>  
>      printk("GICv3 compatible with GICv2 cbase %#"PRIpaddr" vbase %#"PRIpaddr"\n",
>             cbase, vbase);
> @@ -1180,20 +1153,12 @@ static void __init gicv3_init_v2(const struct dt_device_node *node,
>      vgic_v2_setup_hw(dbase, cbase, csize, vbase, 0);
>  }
>  
> -/* Set up the GIC */
> -static int __init gicv3_init(void)
> +static void __init gicv3_dt_init(void)
>  {
>      struct rdist_region *rdist_regs;
>      int res, i;
> -    uint32_t reg;
>      const struct dt_device_node *node = gicv3_info.node;
> -    paddr_t dbase;
> -
> -    if ( !cpu_has_gicv3 )
> -    {
> -        dprintk(XENLOG_ERR, "GICv3: driver requires system register support\n");
> -        return -ENODEV;
> -    }
> +    paddr_t vsize;
>  
>      res = dt_device_get_address(node, 0, &dbase, NULL);
>      if ( res )
> @@ -1203,14 +1168,6 @@ static int __init gicv3_init(void)
>          panic("GICv3:  Found unaligned distributor address %"PRIpaddr"",
>                dbase);
>  
> -    gicv3.map_dbase = ioremap_nocache(dbase, SZ_64K);
> -    if ( !gicv3.map_dbase )
> -        panic("GICv3: Failed to ioremap for GIC distributor\n");
> -
> -    reg = readl_relaxed(GICD + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
> -    if ( reg != GIC_PIDR2_ARCH_GICv3 && reg != GIC_PIDR2_ARCH_GICv4 )
> -         panic("GICv3: no distributor detected\n");
> -
>      if ( !dt_property_read_u32(node, "#redistributor-regions",
>                  &gicv3.rdist_count) )
>          gicv3.rdist_count = 1;
> @@ -1248,6 +1205,57 @@ static int __init gicv3_init(void)
>          panic("GICv3: Cannot find the maintenance IRQ");
>      gicv3_info.maintenance_irq = res;
>  
> +    /*
> +     * For GICv3 supporting GICv2, GICC and GICV base address will be
> +     * provided.
> +     */
> +    res = dt_device_get_address(node, 1 + gicv3.rdist_count,
> +                                &cbase, &csize);
> +    if ( res )
> +        return;
> +
> +    res = dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
> +                                &vbase, &vsize);
> +    if ( res )
> +        return;
> +
> +    /*
> +     * We emulate a vGICv2 using a GIC CPU interface of GUEST_GICC_SIZE.
> +     * So only support GICv2 on GICv3 when the virtual CPU interface is
> +     * at least GUEST_GICC_SIZE.
> +     */
> +    if ( vsize < GUEST_GICC_SIZE )
> +    {
> +        printk(XENLOG_WARNING
> +               "GICv3: WARNING: Not enabling support for GICv2 compat mode.\n"
> +               "Size of GICV (%#"PRIpaddr") must at least be %#llx.\n",
> +               vsize, GUEST_GICC_SIZE);
> +        return;

... here it is completely ineffectual.


> +    }
> +}
> +
> +/* Set up the GIC */
> +static int __init gicv3_init(void)
> +{
> +    int res, i;
> +    uint32_t reg;
> +
> +    if ( !cpu_has_gicv3 )
> +    {
> +        dprintk(XENLOG_ERR, "GICv3: driver requires system register support\n");
> +        return -ENODEV;
> +    }
> +
> +    gicv3_dt_init();
> +
> +    gicv3.map_dbase = ioremap_nocache(dbase, SZ_64K);
> +    if ( !gicv3.map_dbase )
> +        panic("GICv3: Failed to ioremap for GIC distributor\n");
> +
> +    reg = readl_relaxed(GICD + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
> +    if ( reg != GIC_PIDR2_ARCH_GICv3 && reg != GIC_PIDR2_ARCH_GICv4 )
> +         panic("GICv3: no distributor detected\n");
> +
>      for ( i = 0; i < gicv3.rdist_count; i++ )
>      {
>          /* map dbase & rdist regions */
> @@ -1277,7 +1285,7 @@ static int __init gicv3_init(void)
>  
>      vgic_v3_setup_hw(dbase, gicv3.rdist_count, gicv3.rdist_regions,
>                       gicv3.rdist_stride);
> -    gicv3_init_v2(node, dbase);
> +    gicv3_init_v2();
>  
>      spin_lock_init(&gicv3.lock);
>  
> @@ -1317,7 +1325,7 @@ static const struct gic_hw_operations gicv3_ops = {
>      .make_hwdom_dt_node  = gicv3_make_hwdom_dt_node,
>  };
>  
> -static int __init gicv3_preinit(struct dt_device_node *node, const void *data)
> +static int __init gicv3_dt_preinit(struct dt_device_node *node, const void *data)
>  {
>      gicv3_info.hw_version = GIC_V3;
>      gicv3_info.node = node;
> @@ -1335,7 +1343,7 @@ static const struct dt_device_match gicv3_dt_match[] __initconst =
>  
>  DT_DEVICE_START(gicv3, "GICv3", DEVICE_GIC)
>          .dt_match = gicv3_dt_match,
> -        .init = gicv3_preinit,
> +        .init = gicv3_dt_preinit,
>  DT_DEVICE_END
>  
>  /*
> -- 
> 2.0.4
> 
> 

Re: [PATCH v5 6/8] arm/gic-v3: Refactor gicv3_init into generic and dt specific parts

[Shannon Zhao]

On 2016/1/27 20:18, Stefano Stabellini wrote:
> On Sat, 23 Jan 2016, Shannon Zhao wrote:
>> >From: Shannon Zhao<shannon.zhao@linaro.org>
>> >
>> >Refactor gic-v3 related functions into dt and generic parts. This will be
>> >helpful when adding acpi support for gic-v3.
>> >
>> >Signed-off-by: Shannon Zhao<shannon.zhao@linaro.org>
>> >---
>> >v5: none
>> >v4: Use INVALID_PADDR and move ioremap to common init function
>> >---
>> >  xen/arch/arm/gic-v3.c | 114 +++++++++++++++++++++++++++-----------------------
>> >  1 file changed, 61 insertions(+), 53 deletions(-)
>> >
>> >diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>> >index a245b56..65a4de6 100644
>> >--- a/xen/arch/arm/gic-v3.c
>> >+++ b/xen/arch/arm/gic-v3.c
>> >@@ -1138,41 +1138,14 @@ static int __init cmp_rdist(const void *a, const void *b)
>> >      return ( l->base < r->base) ? -1 : 0;
>> >  }
>> >
>> >+static paddr_t __initdata dbase = INVALID_PADDR, vbase = INVALID_PADDR;
>> >+static paddr_t __initdata cbase = INVALID_PADDR, csize = INVALID_PADDR;
>> >+
>> >  /* If the GICv3 supports GICv2, initialize it */
>> >-static void __init gicv3_init_v2(const struct dt_device_node *node,
>> >-                                 paddr_t dbase)
>> >+static void __init gicv3_init_v2(void)
>> >  {
>> >-    int res;
>> >-    paddr_t cbase, csize;
>> >-    paddr_t vbase, vsize;
>> >-
>> >-    /*
>> >-     * For GICv3 supporting GICv2, GICC and GICV base address will be
>> >-     * provided.
>> >-     */
>> >-    res = dt_device_get_address(node, 1 + gicv3.rdist_count,
>> >-                                &cbase, &csize);
>> >-    if ( res )
>> >-        return;
>> >-
>> >-    res = dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
>> >-                                &vbase, &vsize);
>> >-    if ( res )
>> >-        return;
>> >-
>> >-    /*
>> >-     * We emulate a vGICv2 using a GIC CPU interface of GUEST_GICC_SIZE.
>> >-     * So only support GICv2 on GICv3 when the virtual CPU interface is
>> >-     * at least GUEST_GICC_SIZE.
>> >-     */
>> >-    if ( vsize < GUEST_GICC_SIZE )
>> >-    {
>> >-        printk(XENLOG_WARNING
>> >-               "GICv3: WARNING: Not enabling support for GICv2 compat mode.\n"
>> >-               "Size of GICV (%#"PRIpaddr") must at least be %#llx.\n",
>> >-               vsize, GUEST_GICC_SIZE);
> The vsize < GUEST_GICC_SIZE check needs to remain here because ...
>
Ah, sorry, didn't notice this. Will fix this. To reduce email traffic 
load I plan to update and send this patch only and hope we could apply 
the last five patches of this series firstly. I'll respin the first 
three patches and send them as an individual series. Is this fine?

Thanks,
-- 
Shannon

Re: [PATCH v5 6/8] arm/gic-v3: Refactor gicv3_init into generic and dt specific parts

[Stefano Stabellini]

On Wed, 27 Jan 2016, Shannon Zhao wrote:
> On 2016/1/27 20:18, Stefano Stabellini wrote:
> > On Sat, 23 Jan 2016, Shannon Zhao wrote:
> > > >From: Shannon Zhao<shannon.zhao@linaro.org>
> > > >
> > > >Refactor gic-v3 related functions into dt and generic parts. This will be
> > > >helpful when adding acpi support for gic-v3.
> > > >
> > > >Signed-off-by: Shannon Zhao<shannon.zhao@linaro.org>
> > > >---
> > > >v5: none
> > > >v4: Use INVALID_PADDR and move ioremap to common init function
> > > >---
> > > >  xen/arch/arm/gic-v3.c | 114
> > > +++++++++++++++++++++++++++-----------------------
> > > >  1 file changed, 61 insertions(+), 53 deletions(-)
> > > >
> > > >diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> > > >index a245b56..65a4de6 100644
> > > >--- a/xen/arch/arm/gic-v3.c
> > > >+++ b/xen/arch/arm/gic-v3.c
> > > >@@ -1138,41 +1138,14 @@ static int __init cmp_rdist(const void *a, const
> > > void *b)
> > > >      return ( l->base < r->base) ? -1 : 0;
> > > >  }
> > > >
> > > >+static paddr_t __initdata dbase = INVALID_PADDR, vbase = INVALID_PADDR;
> > > >+static paddr_t __initdata cbase = INVALID_PADDR, csize = INVALID_PADDR;
> > > >+
> > > >  /* If the GICv3 supports GICv2, initialize it */
> > > >-static void __init gicv3_init_v2(const struct dt_device_node *node,
> > > >-                                 paddr_t dbase)
> > > >+static void __init gicv3_init_v2(void)
> > > >  {
> > > >-    int res;
> > > >-    paddr_t cbase, csize;
> > > >-    paddr_t vbase, vsize;
> > > >-
> > > >-    /*
> > > >-     * For GICv3 supporting GICv2, GICC and GICV base address will be
> > > >-     * provided.
> > > >-     */
> > > >-    res = dt_device_get_address(node, 1 + gicv3.rdist_count,
> > > >-                                &cbase, &csize);
> > > >-    if ( res )
> > > >-        return;
> > > >-
> > > >-    res = dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
> > > >-                                &vbase, &vsize);
> > > >-    if ( res )
> > > >-        return;
> > > >-
> > > >-    /*
> > > >-     * We emulate a vGICv2 using a GIC CPU interface of GUEST_GICC_SIZE.
> > > >-     * So only support GICv2 on GICv3 when the virtual CPU interface is
> > > >-     * at least GUEST_GICC_SIZE.
> > > >-     */
> > > >-    if ( vsize < GUEST_GICC_SIZE )
> > > >-    {
> > > >-        printk(XENLOG_WARNING
> > > >-               "GICv3: WARNING: Not enabling support for GICv2 compat
> > > mode.\n"
> > > >-               "Size of GICV (%#"PRIpaddr") must at least be %#llx.\n",
> > > >-               vsize, GUEST_GICC_SIZE);
> > The vsize < GUEST_GICC_SIZE check needs to remain here because ...
> > 
> Ah, sorry, didn't notice this. Will fix this. To reduce email traffic load I
> plan to update and send this patch only and hope we could apply the last five
> patches of this series firstly. I'll respin the first three patches and send
> them as an individual series. Is this fine?

That's OK for me

[PATCH v6 6/8] arm/gic-v3: Refactor gicv3_init into generic and dt specific parts

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Refactor gic-v3 related functions into dt and generic parts. This will be
helpful when adding acpi support for gic-v3.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v6: keep vsize check in gicv3_init_v2
---
 xen/arch/arm/gic-v3.c | 84 +++++++++++++++++++++++++++------------------------
 1 file changed, 45 insertions(+), 39 deletions(-)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index a245b56..fa61231 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1138,26 +1138,14 @@ static int __init cmp_rdist(const void *a, const void *b)
     return ( l->base < r->base) ? -1 : 0;
 }
 
+static paddr_t __initdata dbase = INVALID_PADDR;
+static paddr_t __initdata vbase = INVALID_PADDR, vsize = 0;
+static paddr_t __initdata cbase = INVALID_PADDR, csize = 0;
+
 /* If the GICv3 supports GICv2, initialize it */
-static void __init gicv3_init_v2(const struct dt_device_node *node,
-                                 paddr_t dbase)
+static void __init gicv3_init_v2(void)
 {
-    int res;
-    paddr_t cbase, csize;
-    paddr_t vbase, vsize;
-
-    /*
-     * For GICv3 supporting GICv2, GICC and GICV base address will be
-     * provided.
-     */
-    res = dt_device_get_address(node, 1 + gicv3.rdist_count,
-                                &cbase, &csize);
-    if ( res )
-        return;
-
-    res = dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
-                                &vbase, &vsize);
-    if ( res )
+    if ( cbase == INVALID_PADDR || vbase == INVALID_PADDR )
         return;
 
     /*
@@ -1180,20 +1168,11 @@ static void __init gicv3_init_v2(const struct dt_device_node *node,
     vgic_v2_setup_hw(dbase, cbase, csize, vbase, 0);
 }
 
-/* Set up the GIC */
-static int __init gicv3_init(void)
+static void __init gicv3_dt_init(void)
 {
     struct rdist_region *rdist_regs;
     int res, i;
-    uint32_t reg;
     const struct dt_device_node *node = gicv3_info.node;
-    paddr_t dbase;
-
-    if ( !cpu_has_gicv3 )
-    {
-        dprintk(XENLOG_ERR, "GICv3: driver requires system register support\n");
-        return -ENODEV;
-    }
 
     res = dt_device_get_address(node, 0, &dbase, NULL);
     if ( res )
@@ -1203,14 +1182,6 @@ static int __init gicv3_init(void)
         panic("GICv3:  Found unaligned distributor address %"PRIpaddr"",
               dbase);
 
-    gicv3.map_dbase = ioremap_nocache(dbase, SZ_64K);
-    if ( !gicv3.map_dbase )
-        panic("GICv3: Failed to ioremap for GIC distributor\n");
-
-    reg = readl_relaxed(GICD + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
-    if ( reg != GIC_PIDR2_ARCH_GICv3 && reg != GIC_PIDR2_ARCH_GICv4 )
-         panic("GICv3: no distributor detected\n");
-
     if ( !dt_property_read_u32(node, "#redistributor-regions",
                 &gicv3.rdist_count) )
         gicv3.rdist_count = 1;
@@ -1248,6 +1219,41 @@ static int __init gicv3_init(void)
         panic("GICv3: Cannot find the maintenance IRQ");
     gicv3_info.maintenance_irq = res;
 
+    /*
+     * For GICv3 supporting GICv2, GICC and GICV base address will be
+     * provided.
+     */
+    res = dt_device_get_address(node, 1 + gicv3.rdist_count,
+                                &cbase, &csize);
+    if ( res )
+        return;
+
+    dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
+                          &vbase, &vsize);
+}
+
+/* Set up the GIC */
+static int __init gicv3_init(void)
+{
+    int res, i;
+    uint32_t reg;
+
+    if ( !cpu_has_gicv3 )
+    {
+        dprintk(XENLOG_ERR, "GICv3: driver requires system register support\n");
+        return -ENODEV;
+    }
+
+    gicv3_dt_init();
+
+    gicv3.map_dbase = ioremap_nocache(dbase, SZ_64K);
+    if ( !gicv3.map_dbase )
+        panic("GICv3: Failed to ioremap for GIC distributor\n");
+
+    reg = readl_relaxed(GICD + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
+    if ( reg != GIC_PIDR2_ARCH_GICv3 && reg != GIC_PIDR2_ARCH_GICv4 )
+         panic("GICv3: no distributor detected\n");
+
     for ( i = 0; i < gicv3.rdist_count; i++ )
     {
         /* map dbase & rdist regions */
@@ -1277,7 +1283,7 @@ static int __init gicv3_init(void)
 
     vgic_v3_setup_hw(dbase, gicv3.rdist_count, gicv3.rdist_regions,
                      gicv3.rdist_stride);
-    gicv3_init_v2(node, dbase);
+    gicv3_init_v2();
 
     spin_lock_init(&gicv3.lock);
 
@@ -1317,7 +1323,7 @@ static const struct gic_hw_operations gicv3_ops = {
     .make_hwdom_dt_node  = gicv3_make_hwdom_dt_node,
 };
 
-static int __init gicv3_preinit(struct dt_device_node *node, const void *data)
+static int __init gicv3_dt_preinit(struct dt_device_node *node, const void *data)
 {
     gicv3_info.hw_version = GIC_V3;
     gicv3_info.node = node;
@@ -1335,7 +1341,7 @@ static const struct dt_device_match gicv3_dt_match[] __initconst =
 
 DT_DEVICE_START(gicv3, "GICv3", DEVICE_GIC)
         .dt_match = gicv3_dt_match,
-        .init = gicv3_preinit,
+        .init = gicv3_dt_preinit,
 DT_DEVICE_END
 
 /*
-- 
2.0.4

Re: [PATCH v6 6/8] arm/gic-v3: Refactor gicv3_init into generic and dt specific parts

[Stefano Stabellini]

On Thu, 28 Jan 2016, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Refactor gic-v3 related functions into dt and generic parts. This will be
> helpful when adding acpi support for gic-v3.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


> v6: keep vsize check in gicv3_init_v2
> ---
>  xen/arch/arm/gic-v3.c | 84 +++++++++++++++++++++++++++------------------------
>  1 file changed, 45 insertions(+), 39 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index a245b56..fa61231 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1138,26 +1138,14 @@ static int __init cmp_rdist(const void *a, const void *b)
>      return ( l->base < r->base) ? -1 : 0;
>  }
>  
> +static paddr_t __initdata dbase = INVALID_PADDR;
> +static paddr_t __initdata vbase = INVALID_PADDR, vsize = 0;
> +static paddr_t __initdata cbase = INVALID_PADDR, csize = 0;
> +
>  /* If the GICv3 supports GICv2, initialize it */
> -static void __init gicv3_init_v2(const struct dt_device_node *node,
> -                                 paddr_t dbase)
> +static void __init gicv3_init_v2(void)
>  {
> -    int res;
> -    paddr_t cbase, csize;
> -    paddr_t vbase, vsize;
> -
> -    /*
> -     * For GICv3 supporting GICv2, GICC and GICV base address will be
> -     * provided.
> -     */
> -    res = dt_device_get_address(node, 1 + gicv3.rdist_count,
> -                                &cbase, &csize);
> -    if ( res )
> -        return;
> -
> -    res = dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
> -                                &vbase, &vsize);
> -    if ( res )
> +    if ( cbase == INVALID_PADDR || vbase == INVALID_PADDR )
>          return;
>  
>      /*
> @@ -1180,20 +1168,11 @@ static void __init gicv3_init_v2(const struct dt_device_node *node,
>      vgic_v2_setup_hw(dbase, cbase, csize, vbase, 0);
>  }
>  
> -/* Set up the GIC */
> -static int __init gicv3_init(void)
> +static void __init gicv3_dt_init(void)
>  {
>      struct rdist_region *rdist_regs;
>      int res, i;
> -    uint32_t reg;
>      const struct dt_device_node *node = gicv3_info.node;
> -    paddr_t dbase;
> -
> -    if ( !cpu_has_gicv3 )
> -    {
> -        dprintk(XENLOG_ERR, "GICv3: driver requires system register support\n");
> -        return -ENODEV;
> -    }
>  
>      res = dt_device_get_address(node, 0, &dbase, NULL);
>      if ( res )
> @@ -1203,14 +1182,6 @@ static int __init gicv3_init(void)
>          panic("GICv3:  Found unaligned distributor address %"PRIpaddr"",
>                dbase);
>  
> -    gicv3.map_dbase = ioremap_nocache(dbase, SZ_64K);
> -    if ( !gicv3.map_dbase )
> -        panic("GICv3: Failed to ioremap for GIC distributor\n");
> -
> -    reg = readl_relaxed(GICD + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
> -    if ( reg != GIC_PIDR2_ARCH_GICv3 && reg != GIC_PIDR2_ARCH_GICv4 )
> -         panic("GICv3: no distributor detected\n");
> -
>      if ( !dt_property_read_u32(node, "#redistributor-regions",
>                  &gicv3.rdist_count) )
>          gicv3.rdist_count = 1;
> @@ -1248,6 +1219,41 @@ static int __init gicv3_init(void)
>          panic("GICv3: Cannot find the maintenance IRQ");
>      gicv3_info.maintenance_irq = res;
>  
> +    /*
> +     * For GICv3 supporting GICv2, GICC and GICV base address will be
> +     * provided.
> +     */
> +    res = dt_device_get_address(node, 1 + gicv3.rdist_count,
> +                                &cbase, &csize);
> +    if ( res )
> +        return;
> +
> +    dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
> +                          &vbase, &vsize);
> +}
> +
> +/* Set up the GIC */
> +static int __init gicv3_init(void)
> +{
> +    int res, i;
> +    uint32_t reg;
> +
> +    if ( !cpu_has_gicv3 )
> +    {
> +        dprintk(XENLOG_ERR, "GICv3: driver requires system register support\n");
> +        return -ENODEV;
> +    }
> +
> +    gicv3_dt_init();
> +
> +    gicv3.map_dbase = ioremap_nocache(dbase, SZ_64K);
> +    if ( !gicv3.map_dbase )
> +        panic("GICv3: Failed to ioremap for GIC distributor\n");
> +
> +    reg = readl_relaxed(GICD + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
> +    if ( reg != GIC_PIDR2_ARCH_GICv3 && reg != GIC_PIDR2_ARCH_GICv4 )
> +         panic("GICv3: no distributor detected\n");
> +
>      for ( i = 0; i < gicv3.rdist_count; i++ )
>      {
>          /* map dbase & rdist regions */
> @@ -1277,7 +1283,7 @@ static int __init gicv3_init(void)
>  
>      vgic_v3_setup_hw(dbase, gicv3.rdist_count, gicv3.rdist_regions,
>                       gicv3.rdist_stride);
> -    gicv3_init_v2(node, dbase);
> +    gicv3_init_v2();
>  
>      spin_lock_init(&gicv3.lock);
>  
> @@ -1317,7 +1323,7 @@ static const struct gic_hw_operations gicv3_ops = {
>      .make_hwdom_dt_node  = gicv3_make_hwdom_dt_node,
>  };
>  
> -static int __init gicv3_preinit(struct dt_device_node *node, const void *data)
> +static int __init gicv3_dt_preinit(struct dt_device_node *node, const void *data)
>  {
>      gicv3_info.hw_version = GIC_V3;
>      gicv3_info.node = node;
> @@ -1335,7 +1341,7 @@ static const struct dt_device_match gicv3_dt_match[] __initconst =
>  
>  DT_DEVICE_START(gicv3, "GICv3", DEVICE_GIC)
>          .dt_match = gicv3_dt_match,
> -        .init = gicv3_preinit,
> +        .init = gicv3_dt_preinit,
>  DT_DEVICE_END
>  
>  /*
> -- 
> 2.0.4
> 
> 

Re: [PATCH v6 6/8] arm/gic-v3: Refactor gicv3_init into generic and dt specific parts

[Shannon Zhao]

On 2016/1/28 18:27, Stefano Stabellini wrote:
> On Thu, 28 Jan 2016, Shannon Zhao wrote:
>> > From: Shannon Zhao <shannon.zhao@linaro.org>
>> > 
>> > Refactor gic-v3 related functions into dt and generic parts. This will be
>> > helpful when adding acpi support for gic-v3.
>> > 
>> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
Thanks a lot!

Hi Ian, Would you please pick the last five patches(4-8)? Regarding the
first three patches, I'll update them and send as an individual set.

Thanks,
-- 
Shannon

Re: [PATCH v6 6/8] arm/gic-v3: Refactor gicv3_init into generic and dt specific parts

[Ian Campbell]

On Sat, 2016-01-30 at 17:03 +0800, Shannon Zhao wrote:
> 
> On 2016/1/28 18:27, Stefano Stabellini wrote:
> > On Thu, 28 Jan 2016, Shannon Zhao wrote:
> > > > From: Shannon Zhao <shannon.zhao@linaro.org>
> > > > 
> > > > Refactor gic-v3 related functions into dt and generic parts. This
> > > > will be
> > > > helpful when adding acpi support for gic-v3.
> > > > 
> > > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> Thanks a lot!
> 
> Hi Ian, Would you please pick the last five patches(4-8)?

Done. I nearly missed the hidden v6 but Stefano pointed it out to me.

0f5f9d8 pl011: Refactor pl011 driver to dt and common initialization parts
57c5953 arm/uart: Rename dt-uart.c to arm-uart.c
b3aeac4 arm/gic-v3: Refactor gicv3_init into generic and dt specific parts
57ab13c arm/gic-v2: Refactor gicv2_init into generic and dt specific parts
eaf1de3 arm/smpboot: Move dt specific code in smp to seperate functions

There was one minor conflict with a MAINTAINERS change, but I trivially
resolved it.