[PATCH] arm: Use HTPIDR to point to per-CPU state

[PATCH] arm: Use HTPIDR to point to per-CPU state

[Tim Deegan]

# HG changeset patch
# User Tim Deegan <tim@xen.org>
# Date 1333030787 -3600
# Node ID dad4fc193c4b160ac55df0354d4deb925c0c18b2
# Parent  bf12b6236d32cc69e6ffd0f21a178849a263fa2e
arm: Use HTPIDR to point to per-CPU state.

Rather than having the per-VCPU stack contain a pointer to the
per-PCPU state, use the CPU's hypervisor thread ID register for that.

Signed-off-by: Tim Deegan <tim@xen.org>

diff -r bf12b6236d32 -r dad4fc193c4b xen/arch/arm/setup.c
--- a/xen/arch/arm/setup.c	Thu Mar 29 12:08:41 2012 +0100
+++ b/xen/arch/arm/setup.c	Thu Mar 29 15:19:47 2012 +0100
@@ -167,7 +167,7 @@ void __init start_xen(unsigned long boot
 
     percpu_init_areas();
     set_processor_id(0); /* needed early, for smp_processor_id() */
-    __set_current((struct vcpu *)0xfffff000); /* debug sanity */
+    set_current((struct vcpu *)0xfffff000); /* debug sanity */
     idle_vcpu[0] = current;
 
     smp_prepare_cpus(cpus);
diff -r bf12b6236d32 -r dad4fc193c4b xen/arch/arm/smpboot.c
--- a/xen/arch/arm/smpboot.c	Thu Mar 29 12:08:41 2012 +0100
+++ b/xen/arch/arm/smpboot.c	Thu Mar 29 15:19:47 2012 +0100
@@ -49,6 +49,9 @@ static bool_t cpu_is_dead = 0;
 /* Number of non-boot CPUs ready to enter C */
 unsigned long __initdata ready_cpus = 0;
 
+/* ID of the PCPU we're running on */
+DEFINE_PER_CPU(unsigned int, cpu_id);
+
 void __init
 smp_prepare_cpus (unsigned int max_cpus)
 {
@@ -102,8 +105,6 @@ void __cpuinit start_secondary(unsigned 
     /* Setup Hyp vector base */
     WRITE_CP32((uint32_t) hyp_traps_vector, HVBAR);
 
-    dprintk(XENLOG_DEBUG, "CPU %li awake.\n", cpuid);
-
     mmu_init_secondary_cpu();
     gic_init_secondary_cpu();
 
@@ -120,7 +121,7 @@ void __cpuinit start_secondary(unsigned 
 
     local_irq_enable();
 
-    dprintk(XENLOG_DEBUG, "CPU %li booted.\n", cpuid);
+    dprintk(XENLOG_DEBUG, "CPU %u booted.\n", smp_processor_id());
 
     startup_cpu_idle_loop();
 }
diff -r bf12b6236d32 -r dad4fc193c4b xen/include/asm-arm/cpregs.h
--- a/xen/include/asm-arm/cpregs.h	Thu Mar 29 12:08:41 2012 +0100
+++ b/xen/include/asm-arm/cpregs.h	Thu Mar 29 15:19:47 2012 +0100
@@ -198,6 +198,7 @@
 /* CP15 CR13:  */
 #define FCSEIDR         p15,0,c13,c0,0  /* FCSE Process ID Register */
 #define CONTEXTIDR      p15,0,c13,c0,1  /* Context ID Register */
+#define HTPIDR          p15,4,c13,c0,2  /* Hyp. Software Thread ID Register */
 
 /* CP15 CR14:  */
 #define CNTPCT          p15,0,c14       /* Time counter value */
diff -r bf12b6236d32 -r dad4fc193c4b xen/include/asm-arm/current.h
--- a/xen/include/asm-arm/current.h	Thu Mar 29 12:08:41 2012 +0100
+++ b/xen/include/asm-arm/current.h	Thu Mar 29 15:19:47 2012 +0100
@@ -11,17 +11,16 @@
 
 struct vcpu;
 
-/*
- * Which VCPU is "current" on this PCPU.
- */
+/* Which VCPU is "current" on this PCPU. */
 DECLARE_PER_CPU(struct vcpu *, curr_vcpu);
 
+#define current            (this_cpu(curr_vcpu))
+#define set_current(vcpu)  do { current = (vcpu); } while (0)
+
+/* Per-VCPU state that lives at the top of the stack */
 struct cpu_info {
     struct cpu_user_regs guest_cpu_user_regs;
     unsigned long elr;
-    /* The following are valid iff this VCPU is current */
-    unsigned int processor_id;
-    unsigned long per_cpu_offset;
     unsigned int pad;
 };
 
@@ -31,22 +30,6 @@ static inline struct cpu_info *get_cpu_i
     return (struct cpu_info *)((sp & ~(STACK_SIZE - 1)) + STACK_SIZE - sizeof(struct cpu_info));
 }
 
-#define get_processor_id()    (get_cpu_info()->processor_id)
-#define set_processor_id(id)  do {                                      \
-    struct cpu_info *ci__ = get_cpu_info();                             \
-    ci__->per_cpu_offset = __per_cpu_offset[ci__->processor_id = (id)]; \
-} while (0)
-
-#define get_current()         (this_cpu(curr_vcpu))
-#define __set_current(vcpu)   (this_cpu(curr_vcpu) = (vcpu))
-#define set_current(vcpu)     do {                                      \
-    int cpu = get_processor_id();                                       \
-    vcpu->arch.cpu_info->processor_id = cpu;                            \
-    vcpu->arch.cpu_info->per_cpu_offset = __per_cpu_offset[cpu];        \
-    __set_current(vcpu);                                                \
-} while (0)
-#define current               (get_current())
-
 #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
 
 #define switch_stack_and_jump(stack, fn)                                \
diff -r bf12b6236d32 -r dad4fc193c4b xen/include/asm-arm/percpu.h
--- a/xen/include/asm-arm/percpu.h	Thu Mar 29 12:08:41 2012 +0100
+++ b/xen/include/asm-arm/percpu.h	Thu Mar 29 15:19:47 2012 +0100
@@ -5,7 +5,6 @@
 extern char __per_cpu_start[], __per_cpu_data_end[];
 extern unsigned long __per_cpu_offset[NR_CPUS];
 void percpu_init_areas(void);
-#endif
 
 /* Separate out the type, so (int[3], foo) works. */
 #define __DEFINE_PER_CPU(type, name, suffix)                    \
@@ -16,10 +15,18 @@ void percpu_init_areas(void);
 #define per_cpu(var, cpu)  \
     (*RELOC_HIDE(&per_cpu__##var, __per_cpu_offset[cpu]))
 #define __get_cpu_var(var) \
-    (*RELOC_HIDE(&per_cpu__##var, get_cpu_info()->per_cpu_offset))
+    (*RELOC_HIDE(&per_cpu__##var, READ_CP32(HTPIDR)))
 
 #define DECLARE_PER_CPU(type, name) extern __typeof__(type) per_cpu__##name
 
+DECLARE_PER_CPU(unsigned int, cpu_id);
+#define get_processor_id()    (this_cpu(cpu_id))
+#define set_processor_id(id)  do {                      \
+    WRITE_CP32(__per_cpu_offset[cpuid], HTPIDR);        \
+    this_cpu(cpu_id) = (id);                            \
+} while(0)
+#endif
+
 #endif /* __ARM_PERCPU_H__ */
 /*
  * Local variables:

Re: [PATCH] arm: Use HTPIDR to point to per-CPU state

[Ian Campbell]

On Thu, 2012-03-29 at 15:20 +0100, Tim Deegan wrote:
> # HG changeset patch
> # User Tim Deegan <tim@xen.org>
> # Date 1333030787 -3600
> # Node ID dad4fc193c4b160ac55df0354d4deb925c0c18b2
> # Parent  bf12b6236d32cc69e6ffd0f21a178849a263fa2e
> arm: Use HTPIDR to point to per-CPU state.
> 
> Rather than having the per-VCPU stack contain a pointer to the
> per-PCPU state, use the CPU's hypervisor thread ID register for that.
> 
> Signed-off-by: Tim Deegan <tim@xen.org>
> 
> diff -r bf12b6236d32 -r dad4fc193c4b xen/arch/arm/setup.c
> --- a/xen/arch/arm/setup.c	Thu Mar 29 12:08:41 2012 +0100
> +++ b/xen/arch/arm/setup.c	Thu Mar 29 15:19:47 2012 +0100
> @@ -167,7 +167,7 @@ void __init start_xen(unsigned long boot
>  
>      percpu_init_areas();
>      set_processor_id(0); /* needed early, for smp_processor_id() */
> -    __set_current((struct vcpu *)0xfffff000); /* debug sanity */
> +    set_current((struct vcpu *)0xfffff000); /* debug sanity */
>      idle_vcpu[0] = current;
>  
>      smp_prepare_cpus(cpus);
> diff -r bf12b6236d32 -r dad4fc193c4b xen/arch/arm/smpboot.c
> --- a/xen/arch/arm/smpboot.c	Thu Mar 29 12:08:41 2012 +0100
> +++ b/xen/arch/arm/smpboot.c	Thu Mar 29 15:19:47 2012 +0100
> @@ -49,6 +49,9 @@ static bool_t cpu_is_dead = 0;
>  /* Number of non-boot CPUs ready to enter C */
>  unsigned long __initdata ready_cpus = 0;
>  
> +/* ID of the PCPU we're running on */
> +DEFINE_PER_CPU(unsigned int, cpu_id);
> +
>  void __init
>  smp_prepare_cpus (unsigned int max_cpus)
>  {
> @@ -102,8 +105,6 @@ void __cpuinit start_secondary(unsigned 
>      /* Setup Hyp vector base */
>      WRITE_CP32((uint32_t) hyp_traps_vector, HVBAR);
>  
> -    dprintk(XENLOG_DEBUG, "CPU %li awake.\n", cpuid);
> -
>      mmu_init_secondary_cpu();
>      gic_init_secondary_cpu();
>  
> @@ -120,7 +121,7 @@ void __cpuinit start_secondary(unsigned 
>  
>      local_irq_enable();
>  
> -    dprintk(XENLOG_DEBUG, "CPU %li booted.\n", cpuid);
> +    dprintk(XENLOG_DEBUG, "CPU %u booted.\n", smp_processor_id());
>  
>      startup_cpu_idle_loop();
>  }
> diff -r bf12b6236d32 -r dad4fc193c4b xen/include/asm-arm/cpregs.h
> --- a/xen/include/asm-arm/cpregs.h	Thu Mar 29 12:08:41 2012 +0100
> +++ b/xen/include/asm-arm/cpregs.h	Thu Mar 29 15:19:47 2012 +0100
> @@ -198,6 +198,7 @@
>  /* CP15 CR13:  */
>  #define FCSEIDR         p15,0,c13,c0,0  /* FCSE Process ID Register */
>  #define CONTEXTIDR      p15,0,c13,c0,1  /* Context ID Register */
> +#define HTPIDR          p15,4,c13,c0,2  /* Hyp. Software Thread ID Register */
>  
>  /* CP15 CR14:  */
>  #define CNTPCT          p15,0,c14       /* Time counter value */
> diff -r bf12b6236d32 -r dad4fc193c4b xen/include/asm-arm/current.h
> --- a/xen/include/asm-arm/current.h	Thu Mar 29 12:08:41 2012 +0100
> +++ b/xen/include/asm-arm/current.h	Thu Mar 29 15:19:47 2012 +0100
> @@ -11,17 +11,16 @@
>  
>  struct vcpu;
>  
> -/*
> - * Which VCPU is "current" on this PCPU.
> - */
> +/* Which VCPU is "current" on this PCPU. */
>  DECLARE_PER_CPU(struct vcpu *, curr_vcpu);
>  
> +#define current            (this_cpu(curr_vcpu))
> +#define set_current(vcpu)  do { current = (vcpu); } while (0)
> +
> +/* Per-VCPU state that lives at the top of the stack */
>  struct cpu_info {
>      struct cpu_user_regs guest_cpu_user_regs;
>      unsigned long elr;
> -    /* The following are valid iff this VCPU is current */
> -    unsigned int processor_id;
> -    unsigned long per_cpu_offset;
>      unsigned int pad;
>  };
>  
> @@ -31,22 +30,6 @@ static inline struct cpu_info *get_cpu_i
>      return (struct cpu_info *)((sp & ~(STACK_SIZE - 1)) + STACK_SIZE - sizeof(struct cpu_info));
>  }
>  
> -#define get_processor_id()    (get_cpu_info()->processor_id)
> -#define set_processor_id(id)  do {                                      \
> -    struct cpu_info *ci__ = get_cpu_info();                             \
> -    ci__->per_cpu_offset = __per_cpu_offset[ci__->processor_id = (id)]; \
> -} while (0)
> -
> -#define get_current()         (this_cpu(curr_vcpu))
> -#define __set_current(vcpu)   (this_cpu(curr_vcpu) = (vcpu))
> -#define set_current(vcpu)     do {                                      \
> -    int cpu = get_processor_id();                                       \
> -    vcpu->arch.cpu_info->processor_id = cpu;                            \
> -    vcpu->arch.cpu_info->per_cpu_offset = __per_cpu_offset[cpu];        \
> -    __set_current(vcpu);                                                \
> -} while (0)
> -#define current               (get_current())
> -
>  #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
>  
>  #define switch_stack_and_jump(stack, fn)                                \
> diff -r bf12b6236d32 -r dad4fc193c4b xen/include/asm-arm/percpu.h
> --- a/xen/include/asm-arm/percpu.h	Thu Mar 29 12:08:41 2012 +0100
> +++ b/xen/include/asm-arm/percpu.h	Thu Mar 29 15:19:47 2012 +0100
> @@ -5,7 +5,6 @@
>  extern char __per_cpu_start[], __per_cpu_data_end[];
>  extern unsigned long __per_cpu_offset[NR_CPUS];
>  void percpu_init_areas(void);
> -#endif
>  
>  /* Separate out the type, so (int[3], foo) works. */
>  #define __DEFINE_PER_CPU(type, name, suffix)                    \
> @@ -16,10 +15,18 @@ void percpu_init_areas(void);
>  #define per_cpu(var, cpu)  \
>      (*RELOC_HIDE(&per_cpu__##var, __per_cpu_offset[cpu]))
>  #define __get_cpu_var(var) \
> -    (*RELOC_HIDE(&per_cpu__##var, get_cpu_info()->per_cpu_offset))
> +    (*RELOC_HIDE(&per_cpu__##var, READ_CP32(HTPIDR)))
>  
>  #define DECLARE_PER_CPU(type, name) extern __typeof__(type) per_cpu__##name
>  
> +DECLARE_PER_CPU(unsigned int, cpu_id);
> +#define get_processor_id()    (this_cpu(cpu_id))
> +#define set_processor_id(id)  do {                      \
> +    WRITE_CP32(__per_cpu_offset[cpuid], HTPIDR);        \
> +    this_cpu(cpu_id) = (id);                            \

This macro uses id, cpu_id and cpuid all at the same time...

The one I'm not really sure about is cpuid, where did he come from?
Looks like it might be a local variable in the two places which call
set_processor_id, and also happens to be the id argument in one of those
cases (the other using literal 0).

> +} while(0)
> +#endif
> +
>  #endif /* __ARM_PERCPU_H__ */
>  /*
>   * Local variables:

Re: [PATCH] arm: Use HTPIDR to point to per-CPU state

[Tim Deegan]

At 17:13 +0100 on 30 Mar (1333127612), Ian Campbell wrote:
> > +DECLARE_PER_CPU(unsigned int, cpu_id);
> > +#define get_processor_id()    (this_cpu(cpu_id))
> > +#define set_processor_id(id)  do {                      \
> > +    WRITE_CP32(__per_cpu_offset[cpuid], HTPIDR);        \
> > +    this_cpu(cpu_id) = (id);                            \
> 
> This macro uses id, cpu_id and cpuid all at the same time...
> 
> The one I'm not really sure about is cpuid, where did he come from?
> Looks like it might be a local variable in the two places which call
> set_processor_id, and also happens to be the id argument in one of those
> cases (the other using literal 0).

Yes, that's a silly mistake that just happens to be OK in all callers. 
Yet another reason to use static inlines rather than macros. :)
Should I resubmit with s/cpuid/id/ or can you fix up as you apply?

Tim.

Re: [PATCH] arm: Use HTPIDR to point to per-CPU state

[Ian Campbell]

On Fri, 2012-03-30 at 17:37 +0100, Tim Deegan wrote:
> At 17:13 +0100 on 30 Mar (1333127612), Ian Campbell wrote:
> > > +DECLARE_PER_CPU(unsigned int, cpu_id);
> > > +#define get_processor_id()    (this_cpu(cpu_id))
> > > +#define set_processor_id(id)  do {                      \
> > > +    WRITE_CP32(__per_cpu_offset[cpuid], HTPIDR);        \
> > > +    this_cpu(cpu_id) = (id);                            \
> > 
> > This macro uses id, cpu_id and cpuid all at the same time...
> > 
> > The one I'm not really sure about is cpuid, where did he come from?
> > Looks like it might be a local variable in the two places which call
> > set_processor_id, and also happens to be the id argument in one of those
> > cases (the other using literal 0).
> 
> Yes, that's a silly mistake that just happens to be OK in all callers. 
> Yet another reason to use static inlines rather than macros. :)
> Should I resubmit with s/cpuid/id/ or can you fix up as you apply?

If it's a simple as that I'll fix it as I apply.

Ian.

Re: [PATCH] arm: Use HTPIDR to point to per-CPU state

[Ian Campbell]

On Fri, 2012-03-30 at 20:35 +0100, Ian Campbell wrote:
> If it's a simple as that I'll fix it as I apply.

Applied:

arm: remove code that sets current to itself
arm: missing unlock in GIC error path
arm: Use HTPIDR to point to per-CPU state (modified as discussed)

Thanks.