[RFC for-4.5 0/5] xen/arm: Remove processor specific bits in code

[RFC for-4.5 0/5] xen/arm: Remove processor specific bits in code

[Julien Grall]

Hello,

This patch series follow a patch I have sent few months ago see:
https://patches.linaro.org/19617/.

I took a new approach and introduced specific processors callback which will
be called at least during vcpu initialization. In the future, we can extend
the structure to add new callbacks.

This patch series also removes xen/include/asm-arm/processor-ca{15,7}.h, both
headers are not used in Xen.

Sincerely yours,

Julien Grall (5):
  xen/arm32: head.S: Remove CA15 and CA7 specific includes
  xen/arm32: Introduce lookup_processor_type
  xen/arm64: Implement lookup_processor_type as a dummy function
  xen/arm: Remove processor specific setup in vcpu_initialise
  xen/arm: Remove asm-arm/processor-ca{15,7}.h headers

 xen/arch/arm/Makefile                |    1 +
 xen/arch/arm/arm32/Makefile          |    2 +-
 xen/arch/arm/arm32/head.S            |   59 +++++++++++++++++++++++++---------
 xen/arch/arm/arm32/proc-v7-c.c       |   32 ++++++++++++++++++
 xen/arch/arm/arm32/proc-v7.S         |    3 ++
 xen/arch/arm/arm64/head.S            |    7 ++++
 xen/arch/arm/domain.c                |    8 ++---
 xen/arch/arm/processor.c             |   49 ++++++++++++++++++++++++++++
 xen/arch/arm/setup.c                 |    3 ++
 xen/include/asm-arm/processor-ca15.h |   42 ------------------------
 xen/include/asm-arm/processor-ca7.h  |   20 ------------
 xen/include/asm-arm/procinfo.h       |   17 ++++++++--
 12 files changed, 156 insertions(+), 87 deletions(-)
 create mode 100644 xen/arch/arm/arm32/proc-v7-c.c
 create mode 100644 xen/arch/arm/processor.c
 delete mode 100644 xen/include/asm-arm/processor-ca15.h
 delete mode 100644 xen/include/asm-arm/processor-ca7.h

-- 
1.7.10.4

[RFC for-4.5 1/5] xen/arm32: head.S: Remove CA15 and CA7 specific includes

[Julien Grall]

head.S only contains generic code. It should not include headers for a
specific processor.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/arm32/head.S |    2 --
 1 file changed, 2 deletions(-)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 1b1801b..77f5518 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -19,8 +19,6 @@
 
 #include <asm/config.h>
 #include <asm/page.h>
-#include <asm/processor-ca15.h>
-#include <asm/processor-ca7.h>
 #include <asm/asm_defns.h>
 #include <asm/early_printk.h>
 
-- 
1.7.10.4

[RFC for-4.5 2/5] xen/arm32: Introduce lookup_processor_type

[Julien Grall]

Looking for a specific proc_info structure is already implemented in assembly.
Implement lookup_processor_type to avoid duplicate code between C and
assembly.

This function searches the proc_info_list structure following the processor
ID. If the search fail, it will return NULL, otherwise a pointer to this
structure for the specific processor.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/arm32/head.S |   57 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 43 insertions(+), 14 deletions(-)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 77f5518..68fb499 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -198,26 +198,16 @@ skip_bss:
         PRINT("- Setting up control registers -\r\n")
 
         /* Get processor specific proc info into r1 */
-        mrc   CP32(r0, MIDR)                /* r0 := our cpu id */
-        ldr   r1, = __proc_info_start
-        add   r1, r1, r10                   /* r1 := paddr of table (start) */
-        ldr   r2, = __proc_info_end
-        add   r2, r2, r10                   /* r2 := paddr of table (end) */
-1:      ldr   r3, [r1, #PROCINFO_cpu_mask]
-        and   r4, r0, r3                    /* r4 := our cpu id with mask */
-        ldr   r3, [r1, #PROCINFO_cpu_val]   /* r3 := cpu val in current proc info */
-        teq   r4, r3
-        beq   2f                            /* Match => exit, or try next proc info */
-        add   r1, r1, #PROCINFO_sizeof
-        cmp   r1, r2
-        blo   1b
+        bl    __lookup_processor_type
+        teq   r1, #0
+        bne   1f
         mov   r4, r0
         PRINT("- Missing processor info: ")
         mov   r0, r4
         bl    putn
         PRINT(" -\r\n")
         b     fail
-2:
+1:
 
         /* Jump to cpu_init */
         ldr   r1, [r1, #PROCINFO_cpu_init]  /* r1 := vaddr(init func) */
@@ -545,6 +535,45 @@ putn:   mov   pc, lr
 
 #endif /* !CONFIG_EARLY_PRINTK */
 
+/* This provides a C-API version of __lookup_processor_type */
+GLOBAL(lookup_processor_type)
+        stmfd sp!, {r4, r10, lr}
+        mov   r10, #0                   /* r10 := offset between virt&phys */
+        bl    __lookup_processor_type
+        mov r0, r1
+        ldmfd sp!, {r4, r10, pc}
+
+/* Read processor ID register (CP#15, CR0), and Look up in the linker-built
+ * supported processor list. Note that we can't use the absolute addresses for
+ * the __proc_info lists since we aren't running with the MMU on (and therefore,
+ * we are not in correct address space). We have to calculate the offset.
+ *
+ * r10: offset between virt&phys
+ *
+ * Returns:
+ * r0: CPUID
+ * r1: proc_info pointer
+ * Clobbers r2-r4
+ */
+__lookup_processor_type:
+        mrc   CP32(r0, MIDR)                /* r0 := our cpu id */
+        ldr   r1, = __proc_info_start
+        add   r1, r1, r10                   /* r1 := paddr of table (start) */
+        ldr   r2, = __proc_info_end
+        add   r2, r2, r10                   /* r2 := paddr of table (end) */
+1:      ldr   r3, [r1, #PROCINFO_cpu_mask]
+        and   r4, r0, r3                    /* r4 := our cpu id with mask */
+        ldr   r3, [r1, #PROCINFO_cpu_val]   /* r3 := cpu val in current proc info */
+        teq   r4, r3
+        beq   2f                            /* Match => exit, or try next proc info */
+        add   r1, r1, #PROCINFO_sizeof
+        cmp   r1, r2
+        blo   1b
+        /* We failed to find the proc_info, return NULL */
+        mov   r1, #0
+2:
+        mov   pc, lr
+
 /*
  * Local variables:
  * mode: ASM
-- 
1.7.10.4

[RFC for-4.5 3/5] xen/arm64: Implement lookup_processor_type as a dummy function

[Julien Grall]

ARM64 implementation doesn't yet have specific code per processor.
This function will be used in a later patch.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/arm64/head.S |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index c97c194..e1d3103 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -546,6 +546,13 @@ putn:   ret
 
 #endif /* !CONFIG_EARLY_PRINTK */
 
+/* This provides a C-API version of __lookup_processor_type
+ * TODO: For now, the implementation return NULL every time
+ */
+GLOBAL(lookup_processor_type)
+        mov  x0, #0
+        ret
+
 /*
  * Local variables:
  * mode: ASM
-- 
1.7.10.4

[RFC for-4.5 4/5] xen/arm: Remove processor specific setup in vcpu_initialise

[Julien Grall]

This patch introduces the possibility to have specific processor callbacks
that can be called in various place.

Currently VCPU initialization code contains processor specific setup (for
Cortex A7 and Cortex A15) for the ACTRL registers. It's possible to have
processor with a different layout for this register.

Move this setup in a specific callback for ARM v7 processor.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/Makefile          |    1 +
 xen/arch/arm/arm32/Makefile    |    2 +-
 xen/arch/arm/arm32/proc-v7-c.c |   32 ++++++++++++++++++++++++++
 xen/arch/arm/arm32/proc-v7.S   |    3 +++
 xen/arch/arm/domain.c          |    8 ++-----
 xen/arch/arm/processor.c       |   49 ++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/setup.c           |    3 +++
 xen/include/asm-arm/procinfo.h |   17 ++++++++++++--
 8 files changed, 106 insertions(+), 9 deletions(-)
 create mode 100644 xen/arch/arm/arm32/proc-v7-c.c
 create mode 100644 xen/arch/arm/processor.c

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index d70f6d5..63e0460 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -32,6 +32,7 @@ obj-y += vuart.o
 obj-y += hvm.o
 obj-y += device.o
 obj-y += decode.o
+obj-y += processor.o
 
 #obj-bin-y += ....o
 
diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index 65ecff4..0e7c3b4 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -1,7 +1,7 @@
 subdir-y += lib
 
 obj-y += entry.o
-obj-y += proc-v7.o
+obj-y += proc-v7.o proc-v7-c.o
 
 obj-y += traps.o
 obj-y += domain.o
diff --git a/xen/arch/arm/arm32/proc-v7-c.c b/xen/arch/arm/arm32/proc-v7-c.c
new file mode 100644
index 0000000..a3b94a2
--- /dev/null
+++ b/xen/arch/arm/arm32/proc-v7-c.c
@@ -0,0 +1,32 @@
+/*
+ * xen/arch/arm/arm32/proc-v7-c.c
+ *
+ * arm v7 specific initializations (C part)
+ *
+ * Julien Grall <julien.grall@linaro.org>
+ * Copyright (c) 2014 Linaro Limited.
+ *
+ * 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 <asm/procinfo.h>
+#include <asm/processor.h>
+
+static void armv7_vcpu_initialize(struct vcpu *v)
+{
+    if ( v->domain->max_vcpus > 1 )
+        v->arch.actlr |= ACTLR_V7_SMP;
+    else
+        v->arch.actlr &= ~ACTLR_V7_SMP;
+}
+
+const struct processor armv7_processor = {
+    .vcpu_initialize = armv7_vcpu_initialize,
+};
diff --git a/xen/arch/arm/arm32/proc-v7.S b/xen/arch/arm/arm32/proc-v7.S
index 2c8cb9c..d7d2318 100644
--- a/xen/arch/arm/arm32/proc-v7.S
+++ b/xen/arch/arm/arm32/proc-v7.S
@@ -33,6 +33,7 @@ __v7_ca15mp_proc_info:
         .long 0x410FC0F0             /* Cortex-A15 */
         .long 0xFF0FFFF0             /* Mask */
         .long v7_init
+        .long armv7_processor
         .size __v7_ca15mp_proc_info, . - __v7_ca15mp_proc_info
 
         .section ".init.proc.info", #alloc, #execinstr
@@ -41,6 +42,7 @@ __v7_ca7mp_proc_info:
         .long 0x410FC070             /* Cortex-A7 */
         .long 0xFF0FFFF0             /* Mask */
         .long v7_init
+        .long armv7_processor
         .size __v7_ca7mp_proc_info, . - __v7_ca7mp_proc_info
 
         .section ".init.proc.info", #alloc, #execinstr
@@ -49,6 +51,7 @@ __v7_brahma15mp_proc_info:
         .long 0x420F00F2             /* Broadcom Brahma-B15 */
         .long 0xFF0FFFFF             /* Mask */
         .long v7_init
+        .long armv7_processor
         .size __v7_brahma15mp_proc_info, . - __v7_brahma15mp_proc_info
 
 /*
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index c279a27..df23147 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -29,7 +29,7 @@
 #include <asm/irq.h>
 #include <asm/cpufeature.h>
 #include <asm/vfp.h>
-#include <asm/processor-ca15.h>
+#include <asm/procinfo.h>
 
 #include <asm/gic.h>
 #include <asm/platform.h>
@@ -487,11 +487,7 @@ int vcpu_initialise(struct vcpu *v)
 
     v->arch.actlr = READ_SYSREG32(ACTLR_EL1);
 
-    /* XXX: Handle other than CA15 cpus */
-    if ( v->domain->max_vcpus > 1 )
-        v->arch.actlr |= ACTLR_CA15_SMP;
-    else
-        v->arch.actlr &= ~ACTLR_CA15_SMP;
+    processor_vcpu_initialize(v);
 
     if ( (rc = vcpu_vgic_init(v)) != 0 )
         return rc;
diff --git a/xen/arch/arm/processor.c b/xen/arch/arm/processor.c
new file mode 100644
index 0000000..b06fcc5
--- /dev/null
+++ b/xen/arch/arm/processor.c
@@ -0,0 +1,49 @@
+/*
+ * xen/arch/arm/processor.c
+ *
+ * Helpers to execute processor specific code.
+ *
+ * Julien Grall <julien.grall@linaro.org>
+ * Copyright (C) 2014 Linaro Limited.
+ *
+ * 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 <asm/procinfo.h>
+
+static const struct processor *processor = NULL;
+
+void __init processor_setup(void)
+{
+    const struct proc_info_list *procinfo;
+
+    procinfo = lookup_processor_type();
+    if ( !procinfo )
+        return;
+
+    processor = procinfo->processor;
+}
+
+void processor_vcpu_initialize(struct vcpu *v)
+{
+    if ( !processor || !processor->vcpu_initialize )
+        return;
+
+    processor->vcpu_initialize(v);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 1f6d713..5529cb1 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -42,6 +42,7 @@
 #include <asm/gic.h>
 #include <asm/cpufeature.h>
 #include <asm/platform.h>
+#include <asm/procinfo.h>
 
 struct cpuinfo_arm __read_mostly boot_cpu_data;
 
@@ -148,6 +149,8 @@ static void __init processor_id(void)
     {
         printk("32-bit Execution: Unsupported\n");
     }
+
+    processor_setup();
 }
 
 static void dt_unreserved_regions(paddr_t s, paddr_t e,
diff --git a/xen/include/asm-arm/procinfo.h b/xen/include/asm-arm/procinfo.h
index 9d3feb7..9cb7a86 100644
--- a/xen/include/asm-arm/procinfo.h
+++ b/xen/include/asm-arm/procinfo.h
@@ -21,10 +21,23 @@
 #ifndef __ASM_ARM_PROCINFO_H
 #define __ASM_ARM_PROCINFO_H
 
+#include <xen/sched.h>
+
+struct processor {
+    /* Initialize specific processor register for the new VPCU*/
+    void (*vcpu_initialize)(struct vcpu *v);
+};
+
 struct proc_info_list {
-	unsigned int		cpu_val;
-	unsigned int		cpu_mask;
+    unsigned int        cpu_val;
+    unsigned int        cpu_mask;
     void                (*cpu_init)(void);
+    struct processor    *processor;
 };
 
+const __init struct proc_info_list *lookup_processor_type(void);
+
+void __init processor_setup(void);
+void processor_vcpu_initialize(struct vcpu *v);
+
 #endif
-- 
1.7.10.4

[RFC for-4.5 5/5] xen/arm: Remove asm-arm/processor-ca{15, 7}.h headers

[Julien Grall]

Theses headers are not in the right directory and are not used anymore.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/include/asm-arm/processor-ca15.h |   42 ----------------------------------
 xen/include/asm-arm/processor-ca7.h  |   20 ----------------
 2 files changed, 62 deletions(-)
 delete mode 100644 xen/include/asm-arm/processor-ca15.h
 delete mode 100644 xen/include/asm-arm/processor-ca7.h

diff --git a/xen/include/asm-arm/processor-ca15.h b/xen/include/asm-arm/processor-ca15.h
deleted file mode 100644
index f65f40a..0000000
--- a/xen/include/asm-arm/processor-ca15.h
+++ /dev/null
@@ -1,42 +0,0 @@
-#ifndef __ASM_ARM_PROCESSOR_CA15_H
-#define __ASM_ARM_PROCESSOR_CA15_H
-
-/* ACTLR Auxiliary Control Register, Cortex A15 */
-#define ACTLR_CA15_SNOOP_DELAYED      (1<<31)
-#define ACTLR_CA15_MAIN_CLOCK         (1<<30)
-#define ACTLR_CA15_NEON_CLOCK         (1<<29)
-#define ACTLR_CA15_NONCACHE           (1<<24)
-#define ACTLR_CA15_INORDER_REQ        (1<<23)
-#define ACTLR_CA15_INORDER_LOAD       (1<<22)
-#define ACTLR_CA15_L2_TLB_PREFETCH    (1<<21)
-#define ACTLR_CA15_L2_IPA_PA_CACHE    (1<<20)
-#define ACTLR_CA15_L2_CACHE           (1<<19)
-#define ACTLR_CA15_L2_PA_CACHE        (1<<18)
-#define ACTLR_CA15_TLB                (1<<17)
-#define ACTLR_CA15_STRONGY_ORDERED    (1<<16)
-#define ACTLR_CA15_INORDER            (1<<15)
-#define ACTLR_CA15_FORCE_LIM          (1<<14)
-#define ACTLR_CA15_CP_FLUSH           (1<<13)
-#define ACTLR_CA15_CP_PUSH            (1<<12)
-#define ACTLR_CA15_LIM                (1<<11)
-#define ACTLR_CA15_SER                (1<<10)
-#define ACTLR_CA15_OPT                (1<<9)
-#define ACTLR_CA15_WFI                (1<<8)
-#define ACTLR_CA15_WFE                (1<<7)
-#define ACTLR_CA15_SMP                (1<<6)
-#define ACTLR_CA15_PLD                (1<<5)
-#define ACTLR_CA15_IP                 (1<<4)
-#define ACTLR_CA15_MICRO_BTB          (1<<3)
-#define ACTLR_CA15_LOOP_ONE           (1<<2)
-#define ACTLR_CA15_LOOP_DISABLE       (1<<1)
-#define ACTLR_CA15_BTB                (1<<0)
-
-#endif /* __ASM_ARM_PROCESSOR_CA15_H */
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/include/asm-arm/processor-ca7.h b/xen/include/asm-arm/processor-ca7.h
deleted file mode 100644
index 5048a95..0000000
--- a/xen/include/asm-arm/processor-ca7.h
+++ /dev/null
@@ -1,20 +0,0 @@
-#ifndef __ASM_ARM_PROCESSOR_CA7_H
-#define __ASM_ARM_PROCESSOR_CA7_H
-
-/* ACTLR Auxiliary Control Register, Cortex A7 */
-#define ACTLR_CA7_DDI                 (1<<28)
-#define ACTLR_CA7_DDVM                (1<<15)
-#define ACTLR_CA7_L1RADIS             (1<<12)
-#define ACTLR_CA7_L2RADIS             (1<<11)
-#define ACTLR_CA7_DODMBS              (1<<10)
-#define ACTLR_CA7_SMP                 (1<<6)
-
-#endif /* __ASM_ARM_PROCESSOR_CA7_H */
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
-- 
1.7.10.4

Re: [RFC for-4.5 1/5] xen/arm32: head.S: Remove CA15 and CA7 specific includes

[Ian Campbell]

On Tue, 2014-02-11 at 20:04 +0000, Julien Grall wrote:
> head.S only contains generic code. It should not include headers for a
> specific processor.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

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

Re: [RFC for-4.5 2/5] xen/arm32: Introduce lookup_processor_type

[Ian Campbell]

On Tue, 2014-02-11 at 20:04 +0000, Julien Grall wrote:
> Looking for a specific proc_info structure is already implemented in assembly.
> Implement lookup_processor_type to avoid duplicate code between C and
> assembly.
> 
> This function searches the proc_info_list structure following the processor
> ID. If the search fail, it will return NULL, otherwise a pointer to this
> structure for the specific processor.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

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

> ---
>  xen/arch/arm/arm32/head.S |   57 ++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 43 insertions(+), 14 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 77f5518..68fb499 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -198,26 +198,16 @@ skip_bss:
>          PRINT("- Setting up control registers -\r\n")
>  
>          /* Get processor specific proc info into r1 */
> -        mrc   CP32(r0, MIDR)                /* r0 := our cpu id */
> -        ldr   r1, = __proc_info_start
> -        add   r1, r1, r10                   /* r1 := paddr of table (start) */
> -        ldr   r2, = __proc_info_end
> -        add   r2, r2, r10                   /* r2 := paddr of table (end) */
> -1:      ldr   r3, [r1, #PROCINFO_cpu_mask]
> -        and   r4, r0, r3                    /* r4 := our cpu id with mask */
> -        ldr   r3, [r1, #PROCINFO_cpu_val]   /* r3 := cpu val in current proc info */
> -        teq   r4, r3
> -        beq   2f                            /* Match => exit, or try next proc info */
> -        add   r1, r1, #PROCINFO_sizeof
> -        cmp   r1, r2
> -        blo   1b
> +        bl    __lookup_processor_type
> +        teq   r1, #0
> +        bne   1f
>          mov   r4, r0
>          PRINT("- Missing processor info: ")
>          mov   r0, r4
>          bl    putn
>          PRINT(" -\r\n")
>          b     fail
> -2:
> +1:
>  
>          /* Jump to cpu_init */
>          ldr   r1, [r1, #PROCINFO_cpu_init]  /* r1 := vaddr(init func) */
> @@ -545,6 +535,45 @@ putn:   mov   pc, lr
>  
>  #endif /* !CONFIG_EARLY_PRINTK */
>  
> +/* This provides a C-API version of __lookup_processor_type */
> +GLOBAL(lookup_processor_type)
> +        stmfd sp!, {r4, r10, lr}
> +        mov   r10, #0                   /* r10 := offset between virt&phys */
> +        bl    __lookup_processor_type
> +        mov r0, r1
> +        ldmfd sp!, {r4, r10, pc}
> +
> +/* Read processor ID register (CP#15, CR0), and Look up in the linker-built
> + * supported processor list. Note that we can't use the absolute addresses for
> + * the __proc_info lists since we aren't running with the MMU on (and therefore,
> + * we are not in correct address space). We have to calculate the offset.
> + *
> + * r10: offset between virt&phys
> + *
> + * Returns:
> + * r0: CPUID
> + * r1: proc_info pointer
> + * Clobbers r2-r4
> + */
> +__lookup_processor_type:
> +        mrc   CP32(r0, MIDR)                /* r0 := our cpu id */
> +        ldr   r1, = __proc_info_start
> +        add   r1, r1, r10                   /* r1 := paddr of table (start) */
> +        ldr   r2, = __proc_info_end
> +        add   r2, r2, r10                   /* r2 := paddr of table (end) */
> +1:      ldr   r3, [r1, #PROCINFO_cpu_mask]
> +        and   r4, r0, r3                    /* r4 := our cpu id with mask */
> +        ldr   r3, [r1, #PROCINFO_cpu_val]   /* r3 := cpu val in current proc info */
> +        teq   r4, r3
> +        beq   2f                            /* Match => exit, or try next proc info */
> +        add   r1, r1, #PROCINFO_sizeof
> +        cmp   r1, r2
> +        blo   1b
> +        /* We failed to find the proc_info, return NULL */
> +        mov   r1, #0
> +2:
> +        mov   pc, lr
> +
>  /*
>   * Local variables:
>   * mode: ASM

Re: [RFC for-4.5 3/5] xen/arm64: Implement lookup_processor_type as a dummy function

[Ian Campbell]

On Tue, 2014-02-11 at 20:04 +0000, Julien Grall wrote:
> ARM64 implementation doesn't yet have specific code per processor.
> This function will be used in a later patch.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

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

> ---
>  xen/arch/arm/arm64/head.S |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index c97c194..e1d3103 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -546,6 +546,13 @@ putn:   ret
>  
>  #endif /* !CONFIG_EARLY_PRINTK */
>  
> +/* This provides a C-API version of __lookup_processor_type
> + * TODO: For now, the implementation return NULL every time
> + */
> +GLOBAL(lookup_processor_type)
> +        mov  x0, #0
> +        ret
> +
>  /*
>   * Local variables:
>   * mode: ASM

Re: [RFC for-4.5 4/5] xen/arm: Remove processor specific setup in vcpu_initialise

[Ian Campbell]

On Tue, 2014-02-11 at 20:04 +0000, Julien Grall wrote:
> diff --git a/xen/arch/arm/arm32/proc-v7-c.c b/xen/arch/arm/arm32/proc-v7-c.c
> new file mode 100644
> index 0000000..a3b94a2
> --- /dev/null
> +++ b/xen/arch/arm/arm32/proc-v7-c.c
> @@ -0,0 +1,32 @@
> +/*
> + * xen/arch/arm/arm32/proc-v7-c.c
> + *
> + * arm v7 specific initializations (C part)

I think strictly speaking this is actually cortex a{7,15} specific.

Calling this file "proc-v7-ca15.c" or something (core-cortex.c?) would
be nicer than the ugly -c suffix...

> + *
> + * Julien Grall <julien.grall@linaro.org>
> + * Copyright (c) 2014 Linaro Limited.
> + *
> + * 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 <asm/procinfo.h>
> +#include <asm/processor.h>
> +
> +static void armv7_vcpu_initialize(struct vcpu *v)
> +{
> +    if ( v->domain->max_vcpus > 1 )
> +        v->arch.actlr |= ACTLR_V7_SMP;
> +    else
> +        v->arch.actlr &= ~ACTLR_V7_SMP;
> +}
> +
> +const struct processor armv7_processor = {

__rodata? (or whatever it is called)

> +    .vcpu_initialize = armv7_vcpu_initialize,
> +};

Re: [RFC for-4.5 5/5] xen/arm: Remove asm-arm/processor-ca{15, 7}.h headers

[Ian Campbell]

On Tue, 2014-02-11 at 20:04 +0000, Julien Grall wrote:
> Theses headers are not in the right directory and are not used anymore.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

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

Although perhaps proc-v7-c.c ought to be using some of these? e.g.
ACTLR_CA15_SMP instead of ACTLR_V7_SMP, which now looks to be badly
named to me...

Re: [RFC for-4.5 4/5] xen/arm: Remove processor specific setup in vcpu_initialise

[Julien Grall]

Hi Ian,

On 02/19/2014 12:18 PM, Ian Campbell wrote:
> On Tue, 2014-02-11 at 20:04 +0000, Julien Grall wrote:
>> diff --git a/xen/arch/arm/arm32/proc-v7-c.c b/xen/arch/arm/arm32/proc-v7-c.c
>> new file mode 100644
>> index 0000000..a3b94a2
>> --- /dev/null
>> +++ b/xen/arch/arm/arm32/proc-v7-c.c
>> @@ -0,0 +1,32 @@
>> +/*
>> + * xen/arch/arm/arm32/proc-v7-c.c
>> + *
>> + * arm v7 specific initializations (C part)
> 
> I think strictly speaking this is actually cortex a{7,15} specific.
> Calling this file "proc-v7-ca15.c" or something (core-cortex.c?) would
> be nicer than the ugly -c suffix...

Right, but it's possible to have some specific quirk written in C for
other ARMv7 CPU.

I think we need to have a file where we can store v7 code (as
proc-v7.S). Creating a file per specific processor for only one function
seems odd. I'm open to any other generic name for this file.

Cheers,

-- 
Julien Grall

Re: [RFC for-4.5 5/5] xen/arm: Remove asm-arm/processor-ca{15, 7}.h headers

[Julien Grall]

Hi Ian,

On 02/19/2014 12:19 PM, Ian Campbell wrote:
> On Tue, 2014-02-11 at 20:04 +0000, Julien Grall wrote:
>> Theses headers are not in the right directory and are not used anymore.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Although perhaps proc-v7-c.c ought to be using some of these? e.g.
> ACTLR_CA15_SMP instead of ACTLR_V7_SMP, which now looks to be badly
> named to me...

The name ACTLR_CA15_SMP gives the impression that it's only for Cortex
A15, rather than A7.

ACTLR_CORTEX_SMP is not better because we may have other cortex in
future that do not have the SMP bit in the future.

What about hardcoding the bit, as it will be only use here?

Cheers,

-- 
Julien Grall

Re: [RFC for-4.5 5/5] xen/arm: Remove asm-arm/processor-ca{15, 7}.h headers

[Ian Campbell]

On Thu, 2014-02-20 at 16:49 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 02/19/2014 12:19 PM, Ian Campbell wrote:
> > On Tue, 2014-02-11 at 20:04 +0000, Julien Grall wrote:
> >> Theses headers are not in the right directory and are not used anymore.
> >>
> >> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > Although perhaps proc-v7-c.c ought to be using some of these? e.g.
> > ACTLR_CA15_SMP instead of ACTLR_V7_SMP, which now looks to be badly
> > named to me...
> 
> The name ACTLR_CA15_SMP gives the impression that it's only for Cortex
> A15, rather than A7.
> 
> ACTLR_CORTEX_SMP is not better because we may have other cortex in
> future that do not have the SMP bit in the future.
> 
> What about hardcoding the bit, as it will be only use here?

Along with a suitable comment I suppose that would be OK in this
instance.
Ian.

Re: [RFC for-4.5 4/5] xen/arm: Remove processor specific setup in vcpu_initialise

[Julien Grall]

On 02/19/2014 12:18 PM, Ian Campbell wrote:

>> + *
>> + * Julien Grall <julien.grall@linaro.org>
>> + * Copyright (c) 2014 Linaro Limited.
>> + *
>> + * 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 <asm/procinfo.h>
>> +#include <asm/processor.h>
>> +
>> +static void armv7_vcpu_initialize(struct vcpu *v)
>> +{
>> +    if ( v->domain->max_vcpus > 1 )
>> +        v->arch.actlr |= ACTLR_V7_SMP;
>> +    else
>> +        v->arch.actlr &= ~ACTLR_V7_SMP;
>> +}
>> +
>> +const struct processor armv7_processor = {
> 
> __rodata? (or whatever it is called)

I forgot to answer to this part. The compiler will put it by default in
rodata. Did you want to say __initconst? If so, we can't because I use a
pointer to this structure in arch/arm/processor.c

If we want to save space, we can copy it in another variable in
processor_setup.

-- 
Julien Grall

Re: [RFC for-4.5 4/5] xen/arm: Remove processor specific setup in vcpu_initialise

[Ian Campbell]

On Thu, 2014-02-20 at 19:43 +0000, Julien Grall wrote:
> On 02/19/2014 12:18 PM, Ian Campbell wrote:
> 
> >> + *
> >> + * Julien Grall <julien.grall@linaro.org>
> >> + * Copyright (c) 2014 Linaro Limited.
> >> + *
> >> + * 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 <asm/procinfo.h>
> >> +#include <asm/processor.h>
> >> +
> >> +static void armv7_vcpu_initialize(struct vcpu *v)
> >> +{
> >> +    if ( v->domain->max_vcpus > 1 )
> >> +        v->arch.actlr |= ACTLR_V7_SMP;
> >> +    else
> >> +        v->arch.actlr &= ~ACTLR_V7_SMP;
> >> +}
> >> +
> >> +const struct processor armv7_processor = {
> > 
> > __rodata? (or whatever it is called)
> 
> I forgot to answer to this part. The compiler will put it by default in
> rodata. Did you want to say __initconst? If so, we can't because I use a
> pointer to this structure in arch/arm/processor.c

I meant __initconst yes. Leaving it in rodata is fine.

__init indeed doesn't make sense now you make me think of it.

> If we want to save space, we can copy it in another variable in
> processor_setup.

No need IMHO (at least not yet, maybe if we end up with dozens of
these...).

 Thanks.

Ian.