1. Patchew
  2. Xen
  3. View series

[PATCH v2] xen: move vcpu_kick() declaration to common header

Oleksii Kurochko posted 1 patch 2 weeks, 1 day ago
There is a newer version of this series
xen/arch/arm/include/asm/event.h | 1 -
xen/arch/ppc/include/asm/event.h | 1 -
xen/arch/x86/cpu/mcheck/vmce.c   | 1 +
xen/arch/x86/include/asm/event.h | 1 -
xen/arch/x86/pv/traps.c          | 1 +
xen/include/xen/sched.h          | 1 +
6 files changed, 3 insertions(+), 3 deletions(-)
[PATCH v2] xen: move vcpu_kick() declaration to common header
Posted by Oleksii Kurochko 2 weeks, 1 day ago 
The vcpu_kick() declaration is duplicated across multiple
architecture-specific event.h headers (ARM, x86, PPC).

Remove the redundant declarations and move vcpu_kick() into
the common xen/include/xen/sched.h header.

Drop the definition of vcpu_kick() from ppc/include/asm/event.h,
as it is already provided in ppc/stubs.c.

Add inclusion of xen/sched.h in the files where vcpu_kick() is
used.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
CI tests:
 https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2185404470
---
Changes in v2:
 - Move vcpu_kick() declaration to xen/sched.h instead of xen/event.h
 - Revert changes connected to switching asm/event.h to xen/event.h as vcpu_kick() 
   is now living in xen/sched.h.
 - Add inclusion of xen/sched.h because of moved vcpu_kick() declaration to
   xen/sched.h.
 - Update the commit message.
---
 xen/arch/arm/include/asm/event.h | 1 -
 xen/arch/ppc/include/asm/event.h | 1 -
 xen/arch/x86/cpu/mcheck/vmce.c   | 1 +
 xen/arch/x86/include/asm/event.h | 1 -
 xen/arch/x86/pv/traps.c          | 1 +
 xen/include/xen/sched.h          | 1 +
 6 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/include/asm/event.h b/xen/arch/arm/include/asm/event.h
index 509157b2b3..e036ab7fb8 100644
--- a/xen/arch/arm/include/asm/event.h
+++ b/xen/arch/arm/include/asm/event.h
@@ -3,7 +3,6 @@
 
 #include <asm/domain.h>
 
-void vcpu_kick(struct vcpu *v);
 void vcpu_mark_events_pending(struct vcpu *v);
 void vcpu_update_evtchn_irq(struct vcpu *v);
 void vcpu_block_unless_event_pending(struct vcpu *v);
diff --git a/xen/arch/ppc/include/asm/event.h b/xen/arch/ppc/include/asm/event.h
index 0f475c4b89..565eee1439 100644
--- a/xen/arch/ppc/include/asm/event.h
+++ b/xen/arch/ppc/include/asm/event.h
@@ -5,7 +5,6 @@
 #include <xen/lib.h>
 
 /* TODO: implement */
-static inline void vcpu_kick(struct vcpu *v) { BUG_ON("unimplemented"); }
 static inline void vcpu_mark_events_pending(struct vcpu *v) { BUG_ON("unimplemented"); }
 static inline void vcpu_update_evtchn_irq(struct vcpu *v) { BUG_ON("unimplemented"); }
 static inline void vcpu_block_unless_event_pending(struct vcpu *v) { BUG_ON("unimplemented"); }
diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index 1a7e92506a..5e89c61238 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -12,6 +12,7 @@
 #include <xen/event.h>
 #include <xen/kernel.h>
 #include <xen/delay.h>
+#include <xen/sched.h>
 #include <xen/smp.h>
 #include <xen/mm.h>
 #include <asm/hvm/save.h>
diff --git a/xen/arch/x86/include/asm/event.h b/xen/arch/x86/include/asm/event.h
index 434f65007e..d13ce28167 100644
--- a/xen/arch/x86/include/asm/event.h
+++ b/xen/arch/x86/include/asm/event.h
@@ -11,7 +11,6 @@
 
 #include <xen/shared.h>
 
-void vcpu_kick(struct vcpu *v);
 void vcpu_mark_events_pending(struct vcpu *v);
 
 static inline int vcpu_event_delivery_is_enabled(struct vcpu *v)
diff --git a/xen/arch/x86/pv/traps.c b/xen/arch/x86/pv/traps.c
index c3c0976c44..21340eb0e9 100644
--- a/xen/arch/x86/pv/traps.c
+++ b/xen/arch/x86/pv/traps.c
@@ -10,6 +10,7 @@
 #include <xen/event.h>
 #include <xen/hypercall.h>
 #include <xen/lib.h>
+#include <xen/sched.h>
 #include <xen/softirq.h>
 
 #include <asm/debugreg.h>
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 52090b4f70..1f77e0869b 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -877,6 +877,7 @@ void vcpu_wake(struct vcpu *v);
 long vcpu_yield(void);
 void vcpu_sleep_nosync(struct vcpu *v);
 void vcpu_sleep_sync(struct vcpu *v);
+void vcpu_kick(struct vcpu *v);
 
 /*
  * Force synchronisation of given VCPU's state. If it is currently descheduled,
-- 
2.52.0
Re: [PATCH v2] xen: move vcpu_kick() declaration to common header
Posted by Jan Beulich 1 week, 5 days ago 
On 28.11.2025 17:23, Oleksii Kurochko wrote:
> The vcpu_kick() declaration is duplicated across multiple
> architecture-specific event.h headers (ARM, x86, PPC).
> 
> Remove the redundant declarations and move vcpu_kick() into
> the common xen/include/xen/sched.h header.
> 
> Drop the definition of vcpu_kick() from ppc/include/asm/event.h,
> as it is already provided in ppc/stubs.c.
> 
> Add inclusion of xen/sched.h in the files where vcpu_kick() is
> used.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
albeit preferably with at least ...

> --- a/xen/arch/x86/cpu/mcheck/vmce.c
> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
> @@ -12,6 +12,7 @@
>  #include <xen/event.h>
>  #include <xen/kernel.h>
>  #include <xen/delay.h>
> +#include <xen/sched.h>
>  #include <xen/smp.h>
>  #include <xen/mm.h>
>  #include <asm/hvm/save.h>

... this change omitted. This file includes the private "mce.h", which in turn
includes xen/sched.h.

> --- a/xen/arch/x86/pv/traps.c
> +++ b/xen/arch/x86/pv/traps.c
> @@ -10,6 +10,7 @@
>  #include <xen/event.h>
>  #include <xen/hypercall.h>
>  #include <xen/lib.h>
> +#include <xen/sched.h>
>  #include <xen/softirq.h>

Somewhat similarly here, xen/event.h includes xen/sched.h. That's less obviously
guaranteed, though, so making the include explicit here is likely okay.

Jan
Re: [PATCH v2] xen: move vcpu_kick() declaration to common header
Posted by Oleksii Kurochko 1 week, 5 days ago 
On 12/1/25 9:40 AM, Jan Beulich wrote:
> On 28.11.2025 17:23, Oleksii Kurochko wrote:
>> The vcpu_kick() declaration is duplicated across multiple
>> architecture-specific event.h headers (ARM, x86, PPC).
>>
>> Remove the redundant declarations and move vcpu_kick() into
>> the common xen/include/xen/sched.h header.
>>
>> Drop the definition of vcpu_kick() from ppc/include/asm/event.h,
>> as it is already provided in ppc/stubs.c.
>>
>> Add inclusion of xen/sched.h in the files where vcpu_kick() is
>> used.
>>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>

Thanks.


> albeit preferably with at least ...
>
>> --- a/xen/arch/x86/cpu/mcheck/vmce.c
>> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
>> @@ -12,6 +12,7 @@
>>   #include <xen/event.h>
>>   #include <xen/kernel.h>
>>   #include <xen/delay.h>
>> +#include <xen/sched.h>
>>   #include <xen/smp.h>
>>   #include <xen/mm.h>
>>   #include <asm/hvm/save.h>
> ... this change omitted. This file includes the private "mce.h", which in turn
> includes xen/sched.h.
>
>> --- a/xen/arch/x86/pv/traps.c
>> +++ b/xen/arch/x86/pv/traps.c
>> @@ -10,6 +10,7 @@
>>   #include <xen/event.h>
>>   #include <xen/hypercall.h>
>>   #include <xen/lib.h>
>> +#include <xen/sched.h>
>>   #include <xen/softirq.h>
> Somewhat similarly here, xen/event.h includes xen/sched.h. That's less obviously
> guaranteed, though, so making the include explicit here is likely okay.

I am generally okay with not adding what is probably an unnecessary new header
inclusion, but it is unclear to me why we should avoid including a header just
because it is already included by another one. In other words, if one day someone
removes "xen/sched.h" from "mce.h", is it acceptable for this to result in a
compilation error? How do we decide when such an error is acceptable and when
it is not?

Should the default behavior be that if header X is already included indirectly
through other headers, there is no need to include header X directly?

~ Oleksii
Re: [PATCH v2] xen: move vcpu_kick() declaration to common header
Posted by Jan Beulich 1 week, 5 days ago 
On 01.12.2025 10:24, Oleksii Kurochko wrote:
> 
> On 12/1/25 9:40 AM, Jan Beulich wrote:
>> On 28.11.2025 17:23, Oleksii Kurochko wrote:
>>> The vcpu_kick() declaration is duplicated across multiple
>>> architecture-specific event.h headers (ARM, x86, PPC).
>>>
>>> Remove the redundant declarations and move vcpu_kick() into
>>> the common xen/include/xen/sched.h header.
>>>
>>> Drop the definition of vcpu_kick() from ppc/include/asm/event.h,
>>> as it is already provided in ppc/stubs.c.
>>>
>>> Add inclusion of xen/sched.h in the files where vcpu_kick() is
>>> used.
>>>
>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> Thanks.
> 
> 
>> albeit preferably with at least ...
>>
>>> --- a/xen/arch/x86/cpu/mcheck/vmce.c
>>> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
>>> @@ -12,6 +12,7 @@
>>>   #include <xen/event.h>
>>>   #include <xen/kernel.h>
>>>   #include <xen/delay.h>
>>> +#include <xen/sched.h>
>>>   #include <xen/smp.h>
>>>   #include <xen/mm.h>
>>>   #include <asm/hvm/save.h>
>> ... this change omitted. This file includes the private "mce.h", which in turn
>> includes xen/sched.h.
>>
>>> --- a/xen/arch/x86/pv/traps.c
>>> +++ b/xen/arch/x86/pv/traps.c
>>> @@ -10,6 +10,7 @@
>>>   #include <xen/event.h>
>>>   #include <xen/hypercall.h>
>>>   #include <xen/lib.h>
>>> +#include <xen/sched.h>
>>>   #include <xen/softirq.h>
>> Somewhat similarly here, xen/event.h includes xen/sched.h. That's less obviously
>> guaranteed, though, so making the include explicit here is likely okay.
> 
> I am generally okay with not adding what is probably an unnecessary new header
> inclusion, but it is unclear to me why we should avoid including a header just
> because it is already included by another one. In other words, if one day someone
> removes "xen/sched.h" from "mce.h", is it acceptable for this to result in a
> compilation error? How do we decide when such an error is acceptable and when
> it is not?

This is precisely why I made the distinction between global vs private headers.
Relying on private headers to have certain #include-s is imo always okay. For
global headers that's less clear, ...

> Should the default behavior be that if header X is already included indirectly
> through other headers, there is no need to include header X directly?

... and hence I wouldn't want to give too much of a rule of thumb. One may say
that if a (global) header, to fulfill its purpose, absolutely has to include
some other header, then one can rely on this when using that header. But in
most cases the situation is somewhat blurred, so it's case-by-case decision.

What commonly happens is that #include-s are added to keep the build working
in all configurations. But #include-s typically wouldn't be added "just because".

Jan

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.