| Message ID | ce3460b3857cca9e6f3072a8ddd50b31cb46b855.1744126720.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | riscv: introduce basic UART support and interrupts for hypervisor mode | expand |
On 08.04.2025 17:57, Oleksii Kurochko wrote: > Initialize cpu_{possible, online, present}_map by using smp_clear_cpu_maps(). > > Drop DEFINE_PER_CPU(unsigned int, cpu_id) from stubs.c as this variable isn't > expected to be used in RISC-V at all. > > Move declaration of cpu_{possible,online,present}_map from stubs.c to smpboot.c > as now smpboot.c is now introduced. > Other defintions keep in stubs.c as they are not initialized and not needed, at > the moment. > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > --- > xen/arch/riscv/Makefile | 1 + > xen/arch/riscv/include/asm/smp.h | 2 ++ > xen/arch/riscv/setup.c | 2 ++ > xen/arch/riscv/smpboot.c | 15 +++++++++++++++ > xen/arch/riscv/stubs.c | 6 ------ > 5 files changed, 20 insertions(+), 6 deletions(-) > create mode 100644 xen/arch/riscv/smpboot.c > > diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile > index 0c6c4a38a3..f551bf32a2 100644 > --- a/xen/arch/riscv/Makefile > +++ b/xen/arch/riscv/Makefile > @@ -10,6 +10,7 @@ obj-y += sbi.o > obj-y += setup.o > obj-y += shutdown.o > obj-y += smp.o > +obj-y += smpboot.o > obj-y += stubs.o > obj-y += time.o > obj-y += traps.o > diff --git a/xen/arch/riscv/include/asm/smp.h b/xen/arch/riscv/include/asm/smp.h > index 5e170b57b3..188c033718 100644 > --- a/xen/arch/riscv/include/asm/smp.h > +++ b/xen/arch/riscv/include/asm/smp.h > @@ -26,6 +26,8 @@ static inline void set_cpuid_to_hartid(unsigned long cpuid, > > void setup_tp(unsigned int cpuid); > > +void smp_clear_cpu_maps(void); > + > #endif > > /* > diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c > index 4e416f6e44..7f68f3f5b7 100644 > --- a/xen/arch/riscv/setup.c > +++ b/xen/arch/riscv/setup.c > @@ -72,6 +72,8 @@ void __init noreturn start_xen(unsigned long bootcpu_id, > > remove_identity_mapping(); > > + smp_clear_cpu_maps(); > + > set_processor_id(0); > > set_cpuid_to_hartid(0, bootcpu_id); > diff --git a/xen/arch/riscv/smpboot.c b/xen/arch/riscv/smpboot.c > new file mode 100644 > index 0000000000..0f4dcc28e1 > --- /dev/null > +++ b/xen/arch/riscv/smpboot.c > @@ -0,0 +1,15 @@ > +#include <xen/cpumask.h> > +#include <xen/init.h> > + > +cpumask_t cpu_online_map; > +cpumask_t cpu_present_map; > +cpumask_t cpu_possible_map; __read_mostly for all of them, perhaps (if CPU hotplug isn't expected to be supported) even __ro_after_init for the latter two? As to cpu_possible_map - do you predict that you'll actually use it? Arm does (and instead has only a fake cpu_present_map), but on x86 we get away without. > +void __init smp_clear_cpu_maps(void) > +{ > + cpumask_clear(&cpu_possible_map); > + cpumask_clear(&cpu_online_map); What's the point of these? All three maps start out fully zeroed. > + cpumask_set_cpu(0, &cpu_possible_map); > + cpumask_set_cpu(0, &cpu_online_map); These are contradicting the name of the function. The somewhat equivalent function we have on x86 is smp_prepare_boot_cpu(). > + cpumask_copy(&cpu_present_map, &cpu_possible_map); Another cpumask_set_cpu() is probably cheaper here then. Jan
On 4/10/25 3:10 PM, Jan Beulich wrote: > On 08.04.2025 17:57, Oleksii Kurochko wrote: >> Initialize cpu_{possible, online, present}_map by using smp_clear_cpu_maps(). >> >> Drop DEFINE_PER_CPU(unsigned int, cpu_id) from stubs.c as this variable isn't >> expected to be used in RISC-V at all. >> >> Move declaration of cpu_{possible,online,present}_map from stubs.c to smpboot.c >> as now smpboot.c is now introduced. >> Other defintions keep in stubs.c as they are not initialized and not needed, at >> the moment. >> >> Signed-off-by: Oleksii Kurochko<oleksii.kurochko@gmail.com> >> --- >> xen/arch/riscv/Makefile | 1 + >> xen/arch/riscv/include/asm/smp.h | 2 ++ >> xen/arch/riscv/setup.c | 2 ++ >> xen/arch/riscv/smpboot.c | 15 +++++++++++++++ >> xen/arch/riscv/stubs.c | 6 ------ >> 5 files changed, 20 insertions(+), 6 deletions(-) >> create mode 100644 xen/arch/riscv/smpboot.c >> >> diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile >> index 0c6c4a38a3..f551bf32a2 100644 >> --- a/xen/arch/riscv/Makefile >> +++ b/xen/arch/riscv/Makefile >> @@ -10,6 +10,7 @@ obj-y += sbi.o >> obj-y += setup.o >> obj-y += shutdown.o >> obj-y += smp.o >> +obj-y += smpboot.o >> obj-y += stubs.o >> obj-y += time.o >> obj-y += traps.o >> diff --git a/xen/arch/riscv/include/asm/smp.h b/xen/arch/riscv/include/asm/smp.h >> index 5e170b57b3..188c033718 100644 >> --- a/xen/arch/riscv/include/asm/smp.h >> +++ b/xen/arch/riscv/include/asm/smp.h >> @@ -26,6 +26,8 @@ static inline void set_cpuid_to_hartid(unsigned long cpuid, >> >> void setup_tp(unsigned int cpuid); >> >> +void smp_clear_cpu_maps(void); >> + >> #endif >> >> /* >> diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c >> index 4e416f6e44..7f68f3f5b7 100644 >> --- a/xen/arch/riscv/setup.c >> +++ b/xen/arch/riscv/setup.c >> @@ -72,6 +72,8 @@ void __init noreturn start_xen(unsigned long bootcpu_id, >> >> remove_identity_mapping(); >> >> + smp_clear_cpu_maps(); >> + >> set_processor_id(0); >> >> set_cpuid_to_hartid(0, bootcpu_id); >> diff --git a/xen/arch/riscv/smpboot.c b/xen/arch/riscv/smpboot.c >> new file mode 100644 >> index 0000000000..0f4dcc28e1 >> --- /dev/null >> +++ b/xen/arch/riscv/smpboot.c >> @@ -0,0 +1,15 @@ >> +#include <xen/cpumask.h> >> +#include <xen/init.h> >> + >> +cpumask_t cpu_online_map; >> +cpumask_t cpu_present_map; >> +cpumask_t cpu_possible_map; > __read_mostly for all of them, perhaps (if CPU hotplug isn't expected to > be supported) even __ro_after_init for the latter two? We have been living without CPU hotplug support for a long time in the downstream branch, but I can't say whether it is expected to be supported in the future or not. To ensure we can add such an option later without changing the attributes of cpu_online_map variable, I prefer to use|__read_mostly| here and __ro_after_init for cpu_possible_map. > > As to cpu_possible_map - do you predict that you'll actually use it? Arm > does (and instead has only a fake cpu_present_map), but on x86 we get away > without. I checked how it is used now in downstream latest branch and it isn't really used only during initialization smp_clear_cpu_maps() and smp_prepare_cpus() so we can skip it for RISC-V too. > >> +void __init smp_clear_cpu_maps(void) >> +{ >> + cpumask_clear(&cpu_possible_map); >> + cpumask_clear(&cpu_online_map); > What's the point of these? All three maps start out fully zeroed. It could be really dropped. I saw your patch for Arm, I'll align the current patch with that changes. > >> + cpumask_set_cpu(0, &cpu_possible_map); >> + cpumask_set_cpu(0, &cpu_online_map); > These are contradicting the name of the function. The somewhat equivalent > function we have on x86 is smp_prepare_boot_cpu(). > >> + cpumask_copy(&cpu_present_map, &cpu_possible_map); > Another cpumask_set_cpu() is probably cheaper here then. What do you mean by cheaper here? ~ Oleksii
On 14.04.2025 17:05, Oleksii Kurochko wrote: > On 4/10/25 3:10 PM, Jan Beulich wrote: >> On 08.04.2025 17:57, Oleksii Kurochko wrote: >>> +void __init smp_clear_cpu_maps(void) >>> +{ >>> + cpumask_clear(&cpu_possible_map); >>> + cpumask_clear(&cpu_online_map); >> What's the point of these? All three maps start out fully zeroed. > > It could be really dropped. I saw your patch for Arm, I'll align the current > patch with that changes. > >>> + cpumask_set_cpu(0, &cpu_possible_map); >>> + cpumask_set_cpu(0, &cpu_online_map); >> These are contradicting the name of the function. The somewhat equivalent >> function we have on x86 is smp_prepare_boot_cpu(). >> >>> + cpumask_copy(&cpu_present_map, &cpu_possible_map); >> Another cpumask_set_cpu() is probably cheaper here then. > > What do you mean by cheaper here? Less code to execute to achieve the same effect. Jan
diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile index 0c6c4a38a3..f551bf32a2 100644 --- a/xen/arch/riscv/Makefile +++ b/xen/arch/riscv/Makefile @@ -10,6 +10,7 @@ obj-y += sbi.o obj-y += setup.o obj-y += shutdown.o obj-y += smp.o +obj-y += smpboot.o obj-y += stubs.o obj-y += time.o obj-y += traps.o diff --git a/xen/arch/riscv/include/asm/smp.h b/xen/arch/riscv/include/asm/smp.h index 5e170b57b3..188c033718 100644 --- a/xen/arch/riscv/include/asm/smp.h +++ b/xen/arch/riscv/include/asm/smp.h @@ -26,6 +26,8 @@ static inline void set_cpuid_to_hartid(unsigned long cpuid, void setup_tp(unsigned int cpuid); +void smp_clear_cpu_maps(void); + #endif /* diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c index 4e416f6e44..7f68f3f5b7 100644 --- a/xen/arch/riscv/setup.c +++ b/xen/arch/riscv/setup.c @@ -72,6 +72,8 @@ void __init noreturn start_xen(unsigned long bootcpu_id, remove_identity_mapping(); + smp_clear_cpu_maps(); + set_processor_id(0); set_cpuid_to_hartid(0, bootcpu_id); diff --git a/xen/arch/riscv/smpboot.c b/xen/arch/riscv/smpboot.c new file mode 100644 index 0000000000..0f4dcc28e1 --- /dev/null +++ b/xen/arch/riscv/smpboot.c @@ -0,0 +1,15 @@ +#include <xen/cpumask.h> +#include <xen/init.h> + +cpumask_t cpu_online_map; +cpumask_t cpu_present_map; +cpumask_t cpu_possible_map; + +void __init smp_clear_cpu_maps(void) +{ + cpumask_clear(&cpu_possible_map); + cpumask_clear(&cpu_online_map); + cpumask_set_cpu(0, &cpu_possible_map); + cpumask_set_cpu(0, &cpu_online_map); + cpumask_copy(&cpu_present_map, &cpu_possible_map); +} diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c index 83416d3350..fdcf91054e 100644 --- a/xen/arch/riscv/stubs.c +++ b/xen/arch/riscv/stubs.c @@ -11,12 +11,6 @@ /* smpboot.c */ -cpumask_t cpu_online_map; -cpumask_t cpu_present_map; -cpumask_t cpu_possible_map; - -/* ID of the PCPU we're running on */ -DEFINE_PER_CPU(unsigned int, cpu_id); /* XXX these seem awfully x86ish... */ /* representing HT siblings of each logical CPU */ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_sibling_mask);
Initialize cpu_{possible, online, present}_map by using smp_clear_cpu_maps(). Drop DEFINE_PER_CPU(unsigned int, cpu_id) from stubs.c as this variable isn't expected to be used in RISC-V at all. Move declaration of cpu_{possible,online,present}_map from stubs.c to smpboot.c as now smpboot.c is now introduced. Other defintions keep in stubs.c as they are not initialized and not needed, at the moment. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- xen/arch/riscv/Makefile | 1 + xen/arch/riscv/include/asm/smp.h | 2 ++ xen/arch/riscv/setup.c | 2 ++ xen/arch/riscv/smpboot.c | 15 +++++++++++++++ xen/arch/riscv/stubs.c | 6 ------ 5 files changed, 20 insertions(+), 6 deletions(-) create mode 100644 xen/arch/riscv/smpboot.c