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

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,
+}};
-- 
2.46.0

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