| Message ID | 51ce486a825a1654998db01c4e07c127e4b1b38b.1700221559.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce generic headers | expand |
On 17.11.2023 13:24, Oleksii Kurochko wrote: > --- a/xen/arch/ppc/include/asm/current.h > +++ b/xen/arch/ppc/include/asm/current.h > @@ -4,6 +4,8 @@ > > #include <xen/percpu.h> > > +#include <asm/processor.h> > + > #ifndef __ASSEMBLY__ > > struct vcpu; > @@ -38,6 +40,10 @@ static inline struct cpu_info *get_cpu_info(void) > > #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs) > > +#define smp_processor_id() 0 /* TODO: Fix this */ > + > +#define get_per_cpu_offset() smp_processor_id() This 2nd item can't be quite right either, so likely also wants a FIXME comment. Shawn will have the ultimate say here anyway. > --- a/xen/include/xen/percpu.h > +++ b/xen/include/xen/percpu.h > @@ -1,8 +1,6 @@ > #ifndef __XEN_PERCPU_H__ > #define __XEN_PERCPU_H__ > > -#include <asm/percpu.h> > - > #define DECLARE_PER_CPU(type, name) \ > extern __typeof__(type) per_cpu__ ## name > > @@ -29,6 +27,8 @@ > > #define get_per_cpu_var(var) (per_cpu__##var) > > +#include <asm/percpu.h> > + > /* Linux compatibility. */ > #define get_cpu_var(var) this_cpu(var) > #define put_cpu_var(var) While you explain the reason for this movement, it still feels a little fragile to me. Jan
On Tue, 2023-11-21 at 16:56 +0100, Jan Beulich wrote: > On 17.11.2023 13:24, Oleksii Kurochko wrote: > > --- a/xen/arch/ppc/include/asm/current.h > > +++ b/xen/arch/ppc/include/asm/current.h > > @@ -4,6 +4,8 @@ > > > > #include <xen/percpu.h> > > > > +#include <asm/processor.h> > > + > > #ifndef __ASSEMBLY__ > > > > struct vcpu; > > @@ -38,6 +40,10 @@ static inline struct cpu_info > > *get_cpu_info(void) > > > > #define guest_cpu_user_regs() (&get_cpu_info()- > > >guest_cpu_user_regs) > > > > +#define smp_processor_id() 0 /* TODO: Fix this */ > > + > > +#define get_per_cpu_offset() smp_processor_id() > > This 2nd item can't be quite right either, so likely also wants a > FIXME comment. > Shawn will have the ultimate say here anyway. I did so because it is how percpu stuff was implemented before get_per_cpu_offset was introduced. > > > --- a/xen/include/xen/percpu.h > > +++ b/xen/include/xen/percpu.h > > @@ -1,8 +1,6 @@ > > #ifndef __XEN_PERCPU_H__ > > #define __XEN_PERCPU_H__ > > > > -#include <asm/percpu.h> > > - > > #define DECLARE_PER_CPU(type, name) \ > > extern __typeof__(type) per_cpu__ ## name > > > > @@ -29,6 +27,8 @@ > > > > #define get_per_cpu_var(var) (per_cpu__##var) > > > > +#include <asm/percpu.h> > > + > > /* Linux compatibility. */ > > #define get_cpu_var(var) this_cpu(var) > > #define put_cpu_var(var) > > While you explain the reason for this movement, it still feels a > little > fragile to me. The reason for that is #include <asm/processor.h> was added to <asm/percpu.h>. <asm/processor.h> uses DECLARE_PER_CPU(...) so it should be defined before inclusion of <asm/percpu.h>. Otherwise the following error will occur: ./arch/riscv/include/asm/current.h:13:32: error: unknown type name 'curr_vcpu' 13 | DECLARE_PER_CPU(struct vcpu *, curr_vcpu); | ^~~~~~~~~ In file included from ././include/xen/config.h:17, from <command-line>: ./include/xen/sched.h: In function 'rcu_unlock_domain': ./include/asm-generic/percpu.h:19:19: error: 'per_cpu__curr_vcpu' undeclared (first use in this function) 19 | (*RELOC_HIDE(&per_cpu__##var, get_per_cpu_offset())) | ^~~~~~~~~ ./include/xen/compiler.h:146:37: note: in definition of macro 'RELOC_HIDE' 146 | __asm__ ("" : "=r"(__ptr) : "0"(ptr)); \ | ^~~ ./arch/riscv/include/asm/current.h:15:29: note: in expansion of macro 'this_cpu' 15 | #define current (this_cpu(curr_vcpu)) | ^~~~~~~~ ./include/xen/sched.h:726:15: note: in expansion of macro 'current' 726 | if ( d != current->domain ) | ^~~~~~~ ./include/asm-generic/percpu.h:19:19: note: each undeclared identifier is reported only once for each function it appears in 19 | (*RELOC_HIDE(&per_cpu__##var, get_per_cpu_offset())) | ^~~~~~~~~ ./include/xen/compiler.h:146:37: note: in definition of macro 'RELOC_HIDE' 146 | __asm__ ("" : "=r"(__ptr) : "0"(ptr)); \ | ^~~ ./arch/riscv/include/asm/current.h:15:29: note: in expansion of macro 'this_cpu' 15 | #define current (this_cpu(curr_vcpu)) | ^~~~~~~~ ./include/xen/sched.h:726:15: note: in expansion of macro 'current' 726 | if ( d != current->domain ) ~ Oleksii
Hi Oleksii, On 17/11/2023 12:24, Oleksii Kurochko wrote: > The patch introduces generic percpu.h which was based on Arm's version > with the following changes: > * makes __per_cpu_data_end[] constant > * introduce get_per_cpu_offset() for macros this_cpu() and this_cpu_ptr() > * add inclustion of <asm/current.h> as get_per_cpu_offset() is located there. > > Also it was changed a place where <asm/percpu.h> is included in <xen/percpu.h> > because asm-generic version of percpu.h started to include <asm/current.h> which > requires definition of DECLARE_PER_CPU. > > As well the patch switches Arm, PPC and x86 architectures to use asm-generic > version of percpu.h. > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > --- > Changes in V3: > - switch all architectures to asm-generic version of percpu.h > - introduce get_per_cpu_offset() and implement it architectures. > - make __per_cpu_data_end constamt. > - update the commit message. > --- > Changes in V2: > - use smp_processor_id() instead of get_processor_id(). > - update commit message . > --- > xen/arch/arm/include/asm/Makefile | 1 + > xen/arch/arm/include/asm/current.h | 3 +++ The changes for Arm and common looks good to me. I saw Jan thought the include movement is fragile, I don't have any strong opinion about the placement so long it compiles :). So feels free to add my ack: Acked-by: Julien Grall <jgrall@amazon.com> Cheers,
diff --git a/xen/arch/arm/include/asm/Makefile b/xen/arch/arm/include/asm/Makefile index cac6d5e3df..3faf1251ec 100644 --- a/xen/arch/arm/include/asm/Makefile +++ b/xen/arch/arm/include/asm/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-only generic-y += iocap.h generic-y += paging.h +generic-y += percpu.h generic-y += random.h generic-y += vm_event.h diff --git a/xen/arch/arm/include/asm/current.h b/xen/arch/arm/include/asm/current.h index 51d1c8efa8..0be7ad6ef9 100644 --- a/xen/arch/arm/include/asm/current.h +++ b/xen/arch/arm/include/asm/current.h @@ -5,6 +5,7 @@ #include <xen/percpu.h> #include <asm/processor.h> +#include <asm/sysregs.h> /* Tell whether the guest vCPU enabled Workaround 2 (i.e variant 4) */ #define CPUINFO_WORKAROUND_2_FLAG_SHIFT 0 @@ -60,6 +61,8 @@ do { \ this_cpu(cpu_id) = (id); \ } while ( 0 ) +#define get_per_cpu_offset() READ_SYSREG(TPIDR_EL2) + #endif #endif /* __ARM_CURRENT_H__ */ diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile index d8f2a1453c..c0badf5717 100644 --- a/xen/arch/ppc/include/asm/Makefile +++ b/xen/arch/ppc/include/asm/Makefile @@ -2,5 +2,6 @@ generic-y += hypercall.h generic-y += iocap.h generic-y += paging.h +generic-y += percpu.h generic-y += random.h generic-y += vm_event.h diff --git a/xen/arch/ppc/include/asm/current.h b/xen/arch/ppc/include/asm/current.h index 0ca06033f9..3d0d316bae 100644 --- a/xen/arch/ppc/include/asm/current.h +++ b/xen/arch/ppc/include/asm/current.h @@ -4,6 +4,8 @@ #include <xen/percpu.h> +#include <asm/processor.h> + #ifndef __ASSEMBLY__ struct vcpu; @@ -38,6 +40,10 @@ static inline struct cpu_info *get_cpu_info(void) #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs) +#define smp_processor_id() 0 /* TODO: Fix this */ + +#define get_per_cpu_offset() smp_processor_id() + #endif /* __ASSEMBLY__ */ #endif /* __ASM_PPC_CURRENT_H__ */ diff --git a/xen/arch/ppc/include/asm/percpu.h b/xen/arch/ppc/include/asm/percpu.h deleted file mode 100644 index e7c40c0f03..0000000000 --- a/xen/arch/ppc/include/asm/percpu.h +++ /dev/null @@ -1,24 +0,0 @@ -#ifndef __PPC_PERCPU_H__ -#define __PPC_PERCPU_H__ - -#ifndef __ASSEMBLY__ - -extern char __per_cpu_start[], __per_cpu_data_end[]; -extern unsigned long __per_cpu_offset[NR_CPUS]; -void percpu_init_areas(void); - -#define smp_processor_id() 0 /* TODO: Fix this */ - -#define per_cpu(var, cpu) \ - (*RELOC_HIDE(&per_cpu__##var, __per_cpu_offset[cpu])) -#define this_cpu(var) \ - (*RELOC_HIDE(&per_cpu__##var, smp_processor_id())) - -#define per_cpu_ptr(var, cpu) \ - (*RELOC_HIDE(var, __per_cpu_offset[cpu])) -#define this_cpu_ptr(var) \ - (*RELOC_HIDE(var, smp_processor_id())) - -#endif - -#endif /* __PPC_PERCPU_H__ */ diff --git a/xen/arch/x86/include/asm/Makefile b/xen/arch/x86/include/asm/Makefile new file mode 100644 index 0000000000..874429ed30 --- /dev/null +++ b/xen/arch/x86/include/asm/Makefile @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0-only +generic-y += percpu.h diff --git a/xen/arch/x86/include/asm/current.h b/xen/arch/x86/include/asm/current.h index 35cca5cbe4..10950f36cc 100644 --- a/xen/arch/x86/include/asm/current.h +++ b/xen/arch/x86/include/asm/current.h @@ -102,6 +102,8 @@ static inline struct cpu_info *get_cpu_info(void) #define smp_processor_id() (get_cpu_info()->processor_id) #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs) +#define get_per_cpu_offset() (get_cpu_info()->per_cpu_offset) + /* * Get the bottom-of-stack, as stored in the per-CPU TSS. This actually points * into the middle of cpu_info.guest_cpu_user_regs, at the section that diff --git a/xen/arch/x86/include/asm/percpu.h b/xen/arch/x86/include/asm/percpu.h deleted file mode 100644 index 2b0c29a233..0000000000 --- a/xen/arch/x86/include/asm/percpu.h +++ /dev/null @@ -1,22 +0,0 @@ -#ifndef __X86_PERCPU_H__ -#define __X86_PERCPU_H__ - -#ifndef __ASSEMBLY__ -extern char __per_cpu_start[], __per_cpu_data_end[]; -extern unsigned long __per_cpu_offset[NR_CPUS]; -void percpu_init_areas(void); -#endif - -/* var is in discarded region: offset to particular copy we want */ -#define per_cpu(var, cpu) \ - (*RELOC_HIDE(&per_cpu__##var, __per_cpu_offset[cpu])) -#define this_cpu(var) \ - (*RELOC_HIDE(&per_cpu__##var, get_cpu_info()->per_cpu_offset)) - -#define this_cpu_ptr(var) \ - (*RELOC_HIDE(var, get_cpu_info()->per_cpu_offset)) - -#define per_cpu_ptr(var, cpu) \ - (*RELOC_HIDE(var, __per_cpu_offset[cpu])) - -#endif /* __X86_PERCPU_H__ */ diff --git a/xen/arch/arm/include/asm/percpu.h b/xen/include/asm-generic/percpu.h similarity index 57% rename from xen/arch/arm/include/asm/percpu.h rename to xen/include/asm-generic/percpu.h index f1a8768080..60af4f9ff9 100644 --- a/xen/arch/arm/include/asm/percpu.h +++ b/xen/include/asm-generic/percpu.h @@ -1,28 +1,32 @@ -#ifndef __ARM_PERCPU_H__ -#define __ARM_PERCPU_H__ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef __ASM_GENERIC_PERCPU_H__ +#define __ASM_GENERIC_PERCPU_H__ #ifndef __ASSEMBLY__ #include <xen/types.h> -#include <asm/sysregs.h> +#include <asm/current.h> -extern char __per_cpu_start[], __per_cpu_data_end[]; +extern char __per_cpu_start[]; +extern const char __per_cpu_data_end[]; extern unsigned long __per_cpu_offset[NR_CPUS]; void percpu_init_areas(void); #define per_cpu(var, cpu) \ (*RELOC_HIDE(&per_cpu__##var, __per_cpu_offset[cpu])) + #define this_cpu(var) \ - (*RELOC_HIDE(&per_cpu__##var, READ_SYSREG(TPIDR_EL2))) + (*RELOC_HIDE(&per_cpu__##var, get_per_cpu_offset())) #define per_cpu_ptr(var, cpu) \ (*RELOC_HIDE(var, __per_cpu_offset[cpu])) #define this_cpu_ptr(var) \ - (*RELOC_HIDE(var, READ_SYSREG(TPIDR_EL2))) + (*RELOC_HIDE(var, get_per_cpu_offset())) #endif -#endif /* __ARM_PERCPU_H__ */ +#endif /* __ASM_GENERIC_PERCPU_H__ */ + /* * Local variables: * mode: C diff --git a/xen/include/xen/percpu.h b/xen/include/xen/percpu.h index c7bf57cbcd..57522f346b 100644 --- a/xen/include/xen/percpu.h +++ b/xen/include/xen/percpu.h @@ -1,8 +1,6 @@ #ifndef __XEN_PERCPU_H__ #define __XEN_PERCPU_H__ -#include <asm/percpu.h> - #define DECLARE_PER_CPU(type, name) \ extern __typeof__(type) per_cpu__ ## name @@ -29,6 +27,8 @@ #define get_per_cpu_var(var) (per_cpu__##var) +#include <asm/percpu.h> + /* Linux compatibility. */ #define get_cpu_var(var) this_cpu(var) #define put_cpu_var(var)
The patch introduces generic percpu.h which was based on Arm's version with the following changes: * makes __per_cpu_data_end[] constant * introduce get_per_cpu_offset() for macros this_cpu() and this_cpu_ptr() * add inclustion of <asm/current.h> as get_per_cpu_offset() is located there. Also it was changed a place where <asm/percpu.h> is included in <xen/percpu.h> because asm-generic version of percpu.h started to include <asm/current.h> which requires definition of DECLARE_PER_CPU. As well the patch switches Arm, PPC and x86 architectures to use asm-generic version of percpu.h. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in V3: - switch all architectures to asm-generic version of percpu.h - introduce get_per_cpu_offset() and implement it architectures. - make __per_cpu_data_end constamt. - update the commit message. --- Changes in V2: - use smp_processor_id() instead of get_processor_id(). - update commit message . --- xen/arch/arm/include/asm/Makefile | 1 + xen/arch/arm/include/asm/current.h | 3 +++ xen/arch/ppc/include/asm/Makefile | 1 + xen/arch/ppc/include/asm/current.h | 6 +++++ xen/arch/ppc/include/asm/percpu.h | 24 ------------------- xen/arch/x86/include/asm/Makefile | 2 ++ xen/arch/x86/include/asm/current.h | 2 ++ xen/arch/x86/include/asm/percpu.h | 22 ----------------- .../asm => include/asm-generic}/percpu.h | 18 ++++++++------ xen/include/xen/percpu.h | 4 ++-- 10 files changed, 28 insertions(+), 55 deletions(-) delete mode 100644 xen/arch/ppc/include/asm/percpu.h create mode 100644 xen/arch/x86/include/asm/Makefile delete mode 100644 xen/arch/x86/include/asm/percpu.h rename xen/{arch/arm/include/asm => include/asm-generic}/percpu.h (57%)