[PATCH v3 15/21] x86/xen: Drop xen_irq_ops

Juergen Gross posted 21 patches 3 weeks, 3 days ago
Only 9 patches received!
There is a newer version of this series
[PATCH v3 15/21] x86/xen: Drop xen_irq_ops
Posted by Juergen Gross 3 weeks, 3 days ago
Instead of having a pre-filled array xen_irq_ops for Xen PV paravirt
functions, drop the array and assign each element individually.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- new patch
---
 arch/x86/xen/irq.c    | 20 +++++++-------------
 tools/objtool/check.c |  1 -
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
index 39982f955cfe..d8678c3d3971 100644
--- a/arch/x86/xen/irq.c
+++ b/arch/x86/xen/irq.c
@@ -40,20 +40,14 @@ static void xen_halt(void)
 		xen_safe_halt();
 }
 
-static const typeof(pv_ops) xen_irq_ops __initconst = {
-	.irq = {
-		/* Initial interrupt flag handling only called while interrupts off. */
-		.save_fl = __PV_IS_CALLEE_SAVE(paravirt_ret0),
-		.irq_disable = __PV_IS_CALLEE_SAVE(paravirt_nop),
-		.irq_enable = __PV_IS_CALLEE_SAVE(BUG_func),
-
-		.safe_halt = xen_safe_halt,
-		.halt = xen_halt,
-	},
-};
-
 void __init xen_init_irq_ops(void)
 {
-	pv_ops.irq = xen_irq_ops.irq;
+	/* Initial interrupt flag handling only called while interrupts off. */
+	pv_ops.irq.save_fl = __PV_IS_CALLEE_SAVE(paravirt_ret0);
+	pv_ops.irq.irq_disable = __PV_IS_CALLEE_SAVE(paravirt_nop);
+	pv_ops.irq.irq_enable = __PV_IS_CALLEE_SAVE(BUG_func);
+	pv_ops.irq.safe_halt = xen_safe_halt;
+	pv_ops.irq.halt = xen_halt;
+
 	x86_init.irqs.intr_init = xen_init_IRQ;
 }
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index a72059fcbc83..d66eb37ff294 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -593,7 +593,6 @@ static int init_pv_ops(struct objtool_file *file)
 	static const char *pv_ops_tables[] = {
 		"pv_ops",
 		"xen_cpu_ops",
-		"xen_irq_ops",
 		"xen_mmu_ops",
 		NULL,
 	};
-- 
2.51.0
Re: [PATCH v3 15/21] x86/xen: Drop xen_irq_ops
Posted by Peter Zijlstra 3 weeks, 2 days ago
On Mon, Oct 06, 2025 at 09:46:00AM +0200, Juergen Gross wrote:
> Instead of having a pre-filled array xen_irq_ops for Xen PV paravirt
> functions, drop the array and assign each element individually.

Same comment for the next few patches; this changelog is a little light
on *why*. I mean, I don't mind the change, but supposedly we should
justify things at least a little, right? :-)

> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> V2:
> - new patch
> ---
>  arch/x86/xen/irq.c    | 20 +++++++-------------
>  tools/objtool/check.c |  1 -
>  2 files changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
> index 39982f955cfe..d8678c3d3971 100644
> --- a/arch/x86/xen/irq.c
> +++ b/arch/x86/xen/irq.c
> @@ -40,20 +40,14 @@ static void xen_halt(void)
>  		xen_safe_halt();
>  }
>  
> -static const typeof(pv_ops) xen_irq_ops __initconst = {
> -	.irq = {
> -		/* Initial interrupt flag handling only called while interrupts off. */
> -		.save_fl = __PV_IS_CALLEE_SAVE(paravirt_ret0),
> -		.irq_disable = __PV_IS_CALLEE_SAVE(paravirt_nop),
> -		.irq_enable = __PV_IS_CALLEE_SAVE(BUG_func),
> -
> -		.safe_halt = xen_safe_halt,
> -		.halt = xen_halt,
> -	},
> -};
> -
>  void __init xen_init_irq_ops(void)
>  {
> -	pv_ops.irq = xen_irq_ops.irq;
> +	/* Initial interrupt flag handling only called while interrupts off. */
> +	pv_ops.irq.save_fl = __PV_IS_CALLEE_SAVE(paravirt_ret0);
> +	pv_ops.irq.irq_disable = __PV_IS_CALLEE_SAVE(paravirt_nop);
> +	pv_ops.irq.irq_enable = __PV_IS_CALLEE_SAVE(BUG_func);
> +	pv_ops.irq.safe_halt = xen_safe_halt;
> +	pv_ops.irq.halt = xen_halt;
> +
>  	x86_init.irqs.intr_init = xen_init_IRQ;
>  }
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index a72059fcbc83..d66eb37ff294 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -593,7 +593,6 @@ static int init_pv_ops(struct objtool_file *file)
>  	static const char *pv_ops_tables[] = {
>  		"pv_ops",
>  		"xen_cpu_ops",
> -		"xen_irq_ops",
>  		"xen_mmu_ops",
>  		NULL,
>  	};
> -- 
> 2.51.0
>
Re: [PATCH v3 15/21] x86/xen: Drop xen_irq_ops
Posted by Jürgen Groß 3 weeks, 2 days ago
On 06.10.25 20:55, Peter Zijlstra wrote:
> On Mon, Oct 06, 2025 at 09:46:00AM +0200, Juergen Gross wrote:
>> Instead of having a pre-filled array xen_irq_ops for Xen PV paravirt
>> functions, drop the array and assign each element individually.
> 
> Same comment for the next few patches; this changelog is a little light
> on *why*. I mean, I don't mind the change, but supposedly we should
> justify things at least a little, right? :-)

Would you be fine with the following addition:

   This is in preparation of reducing the paravirt include hell by
   splitting paravirt.h into multiple more fine grained header files,
   which will in turn require to split up the pv_ops vector as well.
   Dropping the pre-filled array makes life easier for objtool to
   detect missing initializers in multiple pv_ops_ arrays.


Juergen
Re: [PATCH v3 15/21] x86/xen: Drop xen_irq_ops
Posted by Peter Zijlstra 3 weeks, 2 days ago
On Tue, Oct 07, 2025 at 09:47:48AM +0200, Jürgen Groß wrote:
> On 06.10.25 20:55, Peter Zijlstra wrote:
> > On Mon, Oct 06, 2025 at 09:46:00AM +0200, Juergen Gross wrote:
> > > Instead of having a pre-filled array xen_irq_ops for Xen PV paravirt
> > > functions, drop the array and assign each element individually.
> > 
> > Same comment for the next few patches; this changelog is a little light
> > on *why*. I mean, I don't mind the change, but supposedly we should
> > justify things at least a little, right? :-)
> 
> Would you be fine with the following addition:
> 
>   This is in preparation of reducing the paravirt include hell by
>   splitting paravirt.h into multiple more fine grained header files,
>   which will in turn require to split up the pv_ops vector as well.
>   Dropping the pre-filled array makes life easier for objtool to
>   detect missing initializers in multiple pv_ops_ arrays.

Yes, that'll do. The latter being the main reason in this case, right?
Re: [PATCH v3 15/21] x86/xen: Drop xen_irq_ops
Posted by Jürgen Groß 3 weeks, 2 days ago
On 07.10.25 12:21, Peter Zijlstra wrote:
> On Tue, Oct 07, 2025 at 09:47:48AM +0200, Jürgen Groß wrote:
>> On 06.10.25 20:55, Peter Zijlstra wrote:
>>> On Mon, Oct 06, 2025 at 09:46:00AM +0200, Juergen Gross wrote:
>>>> Instead of having a pre-filled array xen_irq_ops for Xen PV paravirt
>>>> functions, drop the array and assign each element individually.
>>>
>>> Same comment for the next few patches; this changelog is a little light
>>> on *why*. I mean, I don't mind the change, but supposedly we should
>>> justify things at least a little, right? :-)
>>
>> Would you be fine with the following addition:
>>
>>    This is in preparation of reducing the paravirt include hell by
>>    splitting paravirt.h into multiple more fine grained header files,
>>    which will in turn require to split up the pv_ops vector as well.
>>    Dropping the pre-filled array makes life easier for objtool to
>>    detect missing initializers in multiple pv_ops_ arrays.
> 
> Yes, that'll do. The latter being the main reason in this case, right?
Yes.


Juergen