| Message ID | 461a246e3a54345578556821f2c7dbf01e348a05.1726242605.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | device tree mapping | expand |
On 13.09.2024 17:57, Oleksii Kurochko wrote: > Introduce struct pcpu_info to store pCPU-related information. > Initially, it includes only processor_id and hart id, but it > will be extended to include guest CPU information and > temporary variables for saving/restoring vCPU registers. > > Add set_processor_id() function to set processor_id stored in > pcpu_info. > > Define smp_processor_id() to provide accurate information, > replacing the previous "dummy" value of 0. > > Initialize tp registers to point to pcpu_info[0]. > Set processor_id to 0 for logical CPU 0 and store the physical > CPU ID in pcpu_info[0]. > > Introduce helpers for getting hart_id ( physical CPU id in RISC-V > terms ) from Xen CPU id. > > Removing of <asm/processor.h> inclusion leads to the following > compilation error: > common/keyhandler.c: In function 'dump_registers': > common/keyhandler.c:200:13: error: implicit declaration of function > 'cpu_relax' [-Werror=implicit-function-declaration] > 200 | cpu_relax(); What is this paragraph about? It may be stale, or it may be lacking information / context on what it tries to explain. > @@ -14,6 +16,22 @@ DECLARE_PER_CPU(cpumask_var_t, cpu_core_mask); > */ > #define park_offline_cpus false > > +/* > + * Mapping between Xen logical cpu index and hartid. > + */ > +static inline unsigned long cpuid_to_hartid(unsigned long cpuid) > +{ > + return pcpu_info[cpuid].hart_id; > +} > + > +static inline void map_cpuid_to_hartid(unsigned long cpuid, > + unsigned long hartid) > +{ > + pcpu_info[cpuid].hart_id = hartid; > +} "map" is ambiguous - it may mean both "get" or "set". May I ask that this become "set", just like for the processor-ID helper? Jan
On Mon, 2024-09-16 at 08:48 +0200, Jan Beulich wrote: > On 13.09.2024 17:57, Oleksii Kurochko wrote: > > Introduce struct pcpu_info to store pCPU-related information. > > Initially, it includes only processor_id and hart id, but it > > will be extended to include guest CPU information and > > temporary variables for saving/restoring vCPU registers. > > > > Add set_processor_id() function to set processor_id stored in > > pcpu_info. > > > > Define smp_processor_id() to provide accurate information, > > replacing the previous "dummy" value of 0. > > > > Initialize tp registers to point to pcpu_info[0]. > > Set processor_id to 0 for logical CPU 0 and store the physical > > CPU ID in pcpu_info[0]. > > > > Introduce helpers for getting hart_id ( physical CPU id in RISC-V > > terms ) from Xen CPU id. > > > > Removing of <asm/processor.h> inclusion leads to the following > > compilation error: > > common/keyhandler.c: In function 'dump_registers': > > common/keyhandler.c:200:13: error: implicit declaration of > > function > > 'cpu_relax' [-Werror=implicit-function-declaration] > > 200 | cpu_relax(); > > What is this paragraph about? It may be stale, or it may be lacking > information / context on what it tries to explain. When I moved pcpu_info[], set_processor_id(), and smp_processor_id() fromasm/processor.h to asm/current.h and began cleaning up the header inclusions, the mentioned compiler error appeared. The inclusion of <asm/processor.h> isn’t necessary anymore at the moment, but I assume it will be included eventually, so I decided to add this explanation to the commit message to clarify why it wasn’t dropped. Initially, I marked it as a TODO in <asm/current.h>, but I realized this comment would be removed once something from <asm/processor.h> is required. So, I opted to document it in the commit message instead. > > > @@ -14,6 +16,22 @@ DECLARE_PER_CPU(cpumask_var_t, cpu_core_mask); > > */ > > #define park_offline_cpus false > > > > +/* > > + * Mapping between Xen logical cpu index and hartid. > > + */ > > +static inline unsigned long cpuid_to_hartid(unsigned long cpuid) > > +{ > > + return pcpu_info[cpuid].hart_id; > > +} > > + > > +static inline void map_cpuid_to_hartid(unsigned long cpuid, > > + unsigned long hartid) > > +{ > > + pcpu_info[cpuid].hart_id = hartid; > > +} > > "map" is ambiguous - it may mean both "get" or "set". May I ask that > this become "set", just like for the processor-ID helper? Sure, I will update that in the next patch series version. ~ Oleksii
diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile index d192be7b55..6832549133 100644 --- a/xen/arch/riscv/Makefile +++ b/xen/arch/riscv/Makefile @@ -5,6 +5,7 @@ obj-$(CONFIG_RISCV_64) += riscv64/ obj-y += sbi.o obj-y += setup.o obj-y += shutdown.o +obj-y += smp.o obj-y += stubs.o obj-y += traps.o obj-y += vm_event.o diff --git a/xen/arch/riscv/include/asm/current.h b/xen/arch/riscv/include/asm/current.h index aedb6dc732..6f1ec4e190 100644 --- a/xen/arch/riscv/include/asm/current.h +++ b/xen/arch/riscv/include/asm/current.h @@ -3,12 +3,37 @@ #ifndef __ASM_CURRENT_H #define __ASM_CURRENT_H -#include <xen/lib.h> +#include <xen/bug.h> +#include <xen/cache.h> #include <xen/percpu.h> + #include <asm/processor.h> #ifndef __ASSEMBLY__ +register struct pcpu_info *tp asm ( "tp" ); + +struct pcpu_info { + unsigned int processor_id; /* Xen CPU id */ + unsigned long hart_id; /* physical CPU id */ +} __cacheline_aligned; + +/* tp points to one of these */ +extern struct pcpu_info pcpu_info[NR_CPUS]; + +#define set_processor_id(id) do { \ + tp->processor_id = (id); \ +} while (0) + +static inline unsigned int smp_processor_id(void) +{ + unsigned int id = tp->processor_id; + + BUG_ON(id >= NR_CPUS); + + return id; +} + /* Which VCPU is "current" on this PCPU. */ DECLARE_PER_CPU(struct vcpu *, curr_vcpu); diff --git a/xen/arch/riscv/include/asm/processor.h b/xen/arch/riscv/include/asm/processor.h index 3ae164c265..e42b353b4c 100644 --- a/xen/arch/riscv/include/asm/processor.h +++ b/xen/arch/riscv/include/asm/processor.h @@ -12,9 +12,6 @@ #ifndef __ASSEMBLY__ -/* TODO: need to be implemeted */ -#define smp_processor_id() 0 - /* On stack VCPU state */ struct cpu_user_regs { diff --git a/xen/arch/riscv/include/asm/smp.h b/xen/arch/riscv/include/asm/smp.h index b1ea91b1eb..7cb8b86144 100644 --- a/xen/arch/riscv/include/asm/smp.h +++ b/xen/arch/riscv/include/asm/smp.h @@ -5,6 +5,8 @@ #include <xen/cpumask.h> #include <xen/percpu.h> +#include <asm/current.h> + DECLARE_PER_CPU(cpumask_var_t, cpu_sibling_mask); DECLARE_PER_CPU(cpumask_var_t, cpu_core_mask); @@ -14,6 +16,22 @@ DECLARE_PER_CPU(cpumask_var_t, cpu_core_mask); */ #define park_offline_cpus false +/* + * Mapping between Xen logical cpu index and hartid. + */ +static inline unsigned long cpuid_to_hartid(unsigned long cpuid) +{ + return pcpu_info[cpuid].hart_id; +} + +static inline void map_cpuid_to_hartid(unsigned long cpuid, + unsigned long hartid) +{ + pcpu_info[cpuid].hart_id = hartid; +} + +void setup_tp(unsigned int cpuid); + #endif /* diff --git a/xen/arch/riscv/riscv64/asm-offsets.c b/xen/arch/riscv/riscv64/asm-offsets.c index 9f663b9510..3b5daf3b36 100644 --- a/xen/arch/riscv/riscv64/asm-offsets.c +++ b/xen/arch/riscv/riscv64/asm-offsets.c @@ -1,5 +1,6 @@ #define COMPILE_OFFSETS +#include <asm/current.h> #include <asm/processor.h> #include <xen/types.h> @@ -50,4 +51,6 @@ void asm_offsets(void) OFFSET(CPU_USER_REGS_SSTATUS, struct cpu_user_regs, sstatus); OFFSET(CPU_USER_REGS_PREGS, struct cpu_user_regs, pregs); BLANK(); + DEFINE(PCPU_INFO_SIZE, sizeof(struct pcpu_info)); + BLANK(); } diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S index 3261e9fce8..2a1b3dad91 100644 --- a/xen/arch/riscv/riscv64/head.S +++ b/xen/arch/riscv/riscv64/head.S @@ -1,4 +1,5 @@ #include <asm/asm.h> +#include <asm/asm-offsets.h> #include <asm/riscv_encoding.h> .section .text.header, "ax", %progbits @@ -55,6 +56,10 @@ FUNC(start) */ jal reset_stack + /* Xen's boot cpu id is equal to 0 so setup TP register for it */ + li a0, 0 + jal setup_tp + /* restore hart_id ( bootcpu_id ) and dtb address */ mv a0, s0 mv a1, s1 @@ -72,6 +77,15 @@ FUNC(reset_stack) ret END(reset_stack) +/* void setup_tp(unsigned int xen_cpuid); */ +FUNC(setup_tp) + la t0, pcpu_info + li t1, PCPU_INFO_SIZE + mul t1, a0, t1 + add tp, t0, t1 + ret +END(setup_tp) + .section .text.ident, "ax", %progbits FUNC(turn_on_mmu) diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c index 82c5752da1..0fd6c37b61 100644 --- a/xen/arch/riscv/setup.c +++ b/xen/arch/riscv/setup.c @@ -9,6 +9,7 @@ #include <public/version.h> #include <asm/early_printk.h> +#include <asm/smp.h> #include <asm/traps.h> void arch_get_xen_caps(xen_capabilities_info_t *info) @@ -41,6 +42,10 @@ void __init noreturn start_xen(unsigned long bootcpu_id, { remove_identity_mapping(); + set_processor_id(0); + + map_cpuid_to_hartid(0, bootcpu_id); + trap_init(); #ifdef CONFIG_SELF_TESTS diff --git a/xen/arch/riscv/smp.c b/xen/arch/riscv/smp.c new file mode 100644 index 0000000000..4ca6a4e892 --- /dev/null +++ b/xen/arch/riscv/smp.c @@ -0,0 +1,15 @@ +#include <xen/smp.h> + +/* + * FIXME: make pcpu_info[] dynamically allocated when necessary + * functionality will be ready + */ +/* + * tp points to one of these per cpu. + * + * hart_id would be valid (no matter which value) if its + * processor_id field is valid (less than NR_CPUS). + */ +struct pcpu_info pcpu_info[NR_CPUS] = { [0 ... NR_CPUS - 1] = { + .processor_id = NR_CPUS, +}};
Introduce struct pcpu_info to store pCPU-related information. Initially, it includes only processor_id and hart id, but it will be extended to include guest CPU information and temporary variables for saving/restoring vCPU registers. Add set_processor_id() function to set processor_id stored in pcpu_info. Define smp_processor_id() to provide accurate information, replacing the previous "dummy" value of 0. Initialize tp registers to point to pcpu_info[0]. Set processor_id to 0 for logical CPU 0 and store the physical CPU ID in pcpu_info[0]. Introduce helpers for getting hart_id ( physical CPU id in RISC-V terms ) from Xen CPU id. Removing of <asm/processor.h> inclusion leads to the following compilation error: common/keyhandler.c: In function 'dump_registers': common/keyhandler.c:200:13: error: implicit declaration of function 'cpu_relax' [-Werror=implicit-function-declaration] 200 | cpu_relax(); Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in V7: - remove get_processor_id(). - move definition of tp variable, struct pcpu_info, pcpu_info[], set_processor_id() and smp_processor_id() from asm/processor.h to asm/current.h. (1) - change xen/lib.h to xen/bug.h in current.h, for BUG_ON() it is enough xen/bug.h - update BUG_ON() from BUG_ON(id > (NR_CPUS-1)) to BUG_ON(id >= NR_CPUS) in smp_processor_id(). - update the comment above cpuid_to_hartid(). - refactor setup_tp() to the way suggested by Jan B. - add helpers to get and set cpuid to hartid. - update the commit message: add information that removing of <asm/processor.h> from <asm/current.h> leads to compilation error. --- Changes in V6: - update the commit message ( drop outdated information ). - s/FIXME commit/FIXME comment in "changes in V5". - code style fixes. - refactoring of smp_processor_id() and fix BUG_ON() condition inside it. - change "mv a0,x0" to "li a0, 0". - add __cacheline_aligned to the struct pcpu_info. - drop smp_set_bootcpu_id() and smpboot.c as it has only smp_set_bootcpu_id() defined at the moment. - re-write setup_tp() to assembler. --- Changes in V5: - add hart_id to pcpu_info; - add comments to pcpu_info members. - define INVALID_HARTID as ULONG_MAX as mhart_id register has MXLEN which is equal to 32 for RV-32 and 64 for RV-64. - add hart_id to pcpu_info structure. - drop cpuid_to_hartid_map[] and use pcpu_info[] for the same purpose. - introduce new function setup_tp(cpuid). - add the FIXME comment on top of pcpu_info[]. - setup TP register before start_xen() being called. - update the commit message. - change "commit message" to "comment" in "Changes in V4" in "update the comment above the code of TP..." --- Changes in V4: - wrap id with () inside set_processor_id(). - code style fixes - update BUG_ON(id > NR_CPUS) in smp_processor_id() and drop the comment above BUG_ON(). - s/__cpuid_to_hartid_map/cpuid_to_hartid_map - s/cpuid_to_hartid_map/cpuid_to_harti ( here cpuid_to_hartid_map is the name of the macros ). - update the comment above the code of TP register initialization in start_xen(). - s/smp_setup_processor_id/smp_setup_bootcpu_id - update the commit message. - cleanup headers which are included in <asm/processor.h> --- Changes in V3: - new patch. --- xen/arch/riscv/Makefile | 1 + xen/arch/riscv/include/asm/current.h | 27 +++++++++++++++++++++++++- xen/arch/riscv/include/asm/processor.h | 3 --- xen/arch/riscv/include/asm/smp.h | 18 +++++++++++++++++ xen/arch/riscv/riscv64/asm-offsets.c | 3 +++ xen/arch/riscv/riscv64/head.S | 14 +++++++++++++ xen/arch/riscv/setup.c | 5 +++++ xen/arch/riscv/smp.c | 15 ++++++++++++++ 8 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 xen/arch/riscv/smp.c