[PATCH v1 15/15] xen/riscv: init tasklet subsystem

Oleksii Kurochko posted 15 patches 1 month, 2 weeks ago
[PATCH v1 15/15] xen/riscv: init tasklet subsystem
Posted by Oleksii Kurochko 1 month, 2 weeks ago
As the tasklet subsystem is now initialized, it is necessary to implement
sync_local_execstate(), since it is invoked when something calls
tasklet_softirq_action(), which is registered in tasklet_subsys_init().

After introducing sync_local_execstate(), the following linker issue occurs:
  riscv64-linux-gnu-ld: prelink.o: in function `bitmap_and':
    /build/xen/./include/xen/bitmap.h:147: undefined reference to
                                           `sync_vcpu_execstate'
  riscv64-linux-gnu-ld: ./.xen-syms.0: hidden symbol
                        `sync_vcpu_execstate' isn't defined
  riscv64-linux-gnu-ld: final link failed: bad value
Therefore, an implementation of sync_vcpu_execstate() is provided, based on
the Arm code.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/domain.c | 23 +++++++++++++++++++++++
 xen/arch/riscv/setup.c  |  3 +++
 xen/arch/riscv/stubs.c  | 10 ----------
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/xen/arch/riscv/domain.c b/xen/arch/riscv/domain.c
index 8a010ae5b47e..574a5a34a389 100644
--- a/xen/arch/riscv/domain.c
+++ b/xen/arch/riscv/domain.c
@@ -141,6 +141,29 @@ void vcpu_kick(struct vcpu *v)
     }
 }
 
+void sync_local_execstate(void)
+{
+    /* Nothing to do -- no lazy switching */
+}
+
+void sync_vcpu_execstate(struct vcpu *v)
+{
+    /*
+     * We don't support lazy switching.
+     *
+     * However the context may have been saved from a remote pCPU so we
+     * need a barrier to ensure it is observed before continuing.
+     *
+     * Per vcpu_context_saved(), the context can be observed when
+     * v->is_running is false (the caller should check it before calling
+     * this function).
+     *
+     * Note this is a full barrier to also prevent update of the context
+     * to happen before it was observed.
+     */
+    smp_mb();
+}
+
 int vcpu_set_interrupt(struct vcpu *v, const unsigned int irq)
 {
     /*
diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index 9b4835960d20..e8dbd55ce79e 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -12,6 +12,7 @@
 #include <xen/serial.h>
 #include <xen/shutdown.h>
 #include <xen/smp.h>
+#include <xen/tasklet.h>
 #include <xen/timer.h>
 #include <xen/vmap.h>
 #include <xen/xvmalloc.h>
@@ -133,6 +134,8 @@ void __init noreturn start_xen(unsigned long bootcpu_id,
         panic("Booting using ACPI isn't supported\n");
     }
 
+    tasklet_subsys_init();
+
     init_IRQ();
 
     riscv_fill_hwcap();
diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
index d120274af2fe..2b3eb01bf03c 100644
--- a/xen/arch/riscv/stubs.c
+++ b/xen/arch/riscv/stubs.c
@@ -91,16 +91,6 @@ void continue_running(struct vcpu *same)
     BUG_ON("unimplemented");
 }
 
-void sync_local_execstate(void)
-{
-    BUG_ON("unimplemented");
-}
-
-void sync_vcpu_execstate(struct vcpu *v)
-{
-    BUG_ON("unimplemented");
-}
-
 void startup_cpu_idle_loop(void)
 {
     BUG_ON("unimplemented");
-- 
2.52.0
Re: [PATCH v1 15/15] xen/riscv: init tasklet subsystem
Posted by Jan Beulich 3 weeks, 6 days ago
On 24.12.2025 18:03, Oleksii Kurochko wrote:
> As the tasklet subsystem is now initialized, it is necessary to implement
> sync_local_execstate(), since it is invoked when something calls
> tasklet_softirq_action(), which is registered in tasklet_subsys_init().
> 
> After introducing sync_local_execstate(), the following linker issue occurs:
>   riscv64-linux-gnu-ld: prelink.o: in function `bitmap_and':
>     /build/xen/./include/xen/bitmap.h:147: undefined reference to
>                                            `sync_vcpu_execstate'
>   riscv64-linux-gnu-ld: ./.xen-syms.0: hidden symbol
>                         `sync_vcpu_execstate' isn't defined
>   riscv64-linux-gnu-ld: final link failed: bad value

How that when ...

> --- a/xen/arch/riscv/stubs.c
> +++ b/xen/arch/riscv/stubs.c
> @@ -91,16 +91,6 @@ void continue_running(struct vcpu *same)
>      BUG_ON("unimplemented");
>  }
>  
> -void sync_local_execstate(void)
> -{
> -    BUG_ON("unimplemented");
> -}
> -
> -void sync_vcpu_execstate(struct vcpu *v)
> -{
> -    BUG_ON("unimplemented");
> -}

... there was a (stub) implementation? (The code changes look okay, it's just
that I can't make sense of that part of the description.)

Jan
Re: [PATCH v1 15/15] xen/riscv: init tasklet subsystem
Posted by Oleksii Kurochko 3 weeks, 5 days ago
On 1/12/26 5:20 PM, Jan Beulich wrote:
> On 24.12.2025 18:03, Oleksii Kurochko wrote:
>> As the tasklet subsystem is now initialized, it is necessary to implement
>> sync_local_execstate(), since it is invoked when something calls
>> tasklet_softirq_action(), which is registered in tasklet_subsys_init().
>>
>> After introducing sync_local_execstate(), the following linker issue occurs:
>>    riscv64-linux-gnu-ld: prelink.o: in function `bitmap_and':
>>      /build/xen/./include/xen/bitmap.h:147: undefined reference to
>>                                             `sync_vcpu_execstate'
>>    riscv64-linux-gnu-ld: ./.xen-syms.0: hidden symbol
>>                          `sync_vcpu_execstate' isn't defined
>>    riscv64-linux-gnu-ld: final link failed: bad value
> How that when ...
>
>> --- a/xen/arch/riscv/stubs.c
>> +++ b/xen/arch/riscv/stubs.c
>> @@ -91,16 +91,6 @@ void continue_running(struct vcpu *same)
>>       BUG_ON("unimplemented");
>>   }
>>   
>> -void sync_local_execstate(void)
>> -{
>> -    BUG_ON("unimplemented");
>> -}
>> -
>> -void sync_vcpu_execstate(struct vcpu *v)
>> -{
>> -    BUG_ON("unimplemented");
>> -}
> ... there was a (stub) implementation? (The code changes look okay, it's just
> that I can't make sense of that part of the description.)

I haven’t investigated this further. I wanted to look into it now, but I can’t
reproduce the issue anymore. I reverted|sync_vcpu_execstate()| to a stub and no
longer see the problem.

I will move the introduction of|sync_vcpu_execstate()|. It doesn’t seem to be
really needed at the moment, but since it is already introduced and there are no
specific comments against it, I think it can be added as a separate patch in this
series.

Thanks.

~ Olesii


Re: [PATCH v1 15/15] xen/riscv: init tasklet subsystem
Posted by Jan Beulich 3 weeks, 4 days ago
On 13.01.2026 18:03, Oleksii Kurochko wrote:
> 
> On 1/12/26 5:20 PM, Jan Beulich wrote:
>> On 24.12.2025 18:03, Oleksii Kurochko wrote:
>>> As the tasklet subsystem is now initialized, it is necessary to implement
>>> sync_local_execstate(), since it is invoked when something calls
>>> tasklet_softirq_action(), which is registered in tasklet_subsys_init().
>>>
>>> After introducing sync_local_execstate(), the following linker issue occurs:
>>>    riscv64-linux-gnu-ld: prelink.o: in function `bitmap_and':
>>>      /build/xen/./include/xen/bitmap.h:147: undefined reference to
>>>                                             `sync_vcpu_execstate'
>>>    riscv64-linux-gnu-ld: ./.xen-syms.0: hidden symbol
>>>                          `sync_vcpu_execstate' isn't defined
>>>    riscv64-linux-gnu-ld: final link failed: bad value
>> How that when ...
>>
>>> --- a/xen/arch/riscv/stubs.c
>>> +++ b/xen/arch/riscv/stubs.c
>>> @@ -91,16 +91,6 @@ void continue_running(struct vcpu *same)
>>>       BUG_ON("unimplemented");
>>>   }
>>>   
>>> -void sync_local_execstate(void)
>>> -{
>>> -    BUG_ON("unimplemented");
>>> -}
>>> -
>>> -void sync_vcpu_execstate(struct vcpu *v)
>>> -{
>>> -    BUG_ON("unimplemented");
>>> -}
>> ... there was a (stub) implementation? (The code changes look okay, it's just
>> that I can't make sense of that part of the description.)
> 
> I haven’t investigated this further. I wanted to look into it now, but I can’t
> reproduce the issue anymore. I reverted|sync_vcpu_execstate()| to a stub and no
> longer see the problem.
> 
> I will move the introduction of|sync_vcpu_execstate()|. It doesn’t seem to be
> really needed at the moment, but since it is already introduced and there are no
> specific comments against it, I think it can be added as a separate patch in this
> series.

Just to mention: Moving it right here looks to make sense to me. It's just that
the description of the change was irritating.

Jan