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;
+
+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);
--
2.49.0On 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
© 2016 - 2025 Red Hat, Inc.