# 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:
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:
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.
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.
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.