1. Patchew
  2. Xen
  3. [PATCH for-4.19 0/2] arch/irq: Untangle no_irq_type
  4. View patch

[PATCH 1/2] arch/irq: Make irq_ack_none() mandatory

Andrew Cooper posted 2 patches 1 year, 8 months ago
[PATCH 1/2] arch/irq: Make irq_ack_none() mandatory
Posted by Andrew Cooper 1 year, 8 months ago 
Any non-stub implementation of these is going to have to do something here.

irq_end_none() is more complicated and has arch-specific interactions with
irq_ack_none(), so make it optional.

For PPC, introduce a stub irq_ack_none().

For ARM and x86, export the existing {ack,end}_none() helpers, gaining an irq_
prefix for consisntency with everything else in no_irq_type.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
CC: Shawn Anastasio <sanastasio@raptorengineering.com>
---
 xen/arch/arm/include/asm/irq.h | 3 +++
 xen/arch/arm/irq.c             | 8 ++++----
 xen/arch/ppc/stubs.c           | 6 ++++++
 xen/arch/x86/irq.c             | 4 ++--
 xen/include/xen/irq.h          | 6 ++++++
 5 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/include/asm/irq.h b/xen/arch/arm/include/asm/irq.h
index 1bae5388878e..ec437add0971 100644
--- a/xen/arch/arm/include/asm/irq.h
+++ b/xen/arch/arm/include/asm/irq.h
@@ -98,6 +98,9 @@ void irq_set_affinity(struct irq_desc *desc, const cpumask_t *mask);
  */
 bool irq_type_set_by_domain(const struct domain *d);
 
+void irq_end_none(struct irq_desc *irq);
+#define irq_end_none irq_end_none
+
 #endif /* _ASM_HW_IRQ_H */
 /*
  * Local variables:
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index bcce80a4d624..7138f9e7c283 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -31,12 +31,12 @@ struct irq_guest
     unsigned int virq;
 };
 
-static void ack_none(struct irq_desc *irq)
+void irq_ack_none(struct irq_desc *irq)
 {
     printk("unexpected IRQ trap at irq %02x\n", irq->irq);
 }
 
-static void end_none(struct irq_desc *irq)
+void irq_end_none(struct irq_desc *irq)
 {
     /*
      * Still allow a CPU to end an interrupt if we receive a spurious
@@ -51,8 +51,8 @@ hw_irq_controller no_irq_type = {
     .shutdown = irq_shutdown_none,
     .enable = irq_enable_none,
     .disable = irq_disable_none,
-    .ack = ack_none,
-    .end = end_none
+    .ack = irq_ack_none,
+    .end = irq_end_none
 };
 
 static irq_desc_t irq_desc[NR_IRQS];
diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
index da193839bd09..4e03428e071a 100644
--- a/xen/arch/ppc/stubs.c
+++ b/xen/arch/ppc/stubs.c
@@ -134,12 +134,18 @@ void pirq_set_affinity(struct domain *d, int pirq, const cpumask_t *mask)
     BUG_ON("unimplemented");
 }
 
+void irq_ack_none(struct irq_desc *desc)
+{
+    BUG_ON("unimplemented");
+}
+
 hw_irq_controller no_irq_type = {
     .typename = "none",
     .startup = irq_startup_none,
     .shutdown = irq_shutdown_none,
     .enable = irq_enable_none,
     .disable = irq_disable_none,
+    .ack = irq_ack_none,
 };
 
 int arch_init_one_irq_desc(struct irq_desc *desc)
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index c16205a9beb6..cfd7a08479d2 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -467,7 +467,7 @@ int __init init_irq_data(void)
     return 0;
 }
 
-static void cf_check ack_none(struct irq_desc *desc)
+void cf_check irq_ack_none(struct irq_desc *desc)
 {
     ack_bad_irq(desc->irq);
 }
@@ -478,7 +478,7 @@ hw_irq_controller no_irq_type = {
     irq_shutdown_none,
     irq_enable_none,
     irq_disable_none,
-    ack_none,
+    irq_ack_none,
 };
 
 static vmask_t *irq_get_used_vector_mask(int irq)
diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
index 89f7a8317a87..b71f65db8621 100644
--- a/xen/include/xen/irq.h
+++ b/xen/include/xen/irq.h
@@ -130,6 +130,12 @@ void cf_check irq_actor_none(struct irq_desc *desc);
 #define irq_disable_none irq_actor_none
 #define irq_enable_none irq_actor_none
 
+/*
+ * irq_ack_none() must be provided by the architecture.
+ * irq_end_none() is optional, and opted into using a define.
+ */
+void irq_ack_none(struct irq_desc *irq);
+
 /*
  * Per-cpu interrupted context register state - the inner-most interrupt frame
  * on the stack.
-- 
2.30.2
Re: [PATCH 1/2] arch/irq: Make irq_ack_none() mandatory
Posted by Shawn Anastasio 1 year, 8 months ago 
Hi Andrew,

On 5/30/24 1:40 PM, Andrew Cooper wrote:
> Any non-stub implementation of these is going to have to do something here.
> 
> irq_end_none() is more complicated and has arch-specific interactions with
> irq_ack_none(), so make it optional.
> 
> For PPC, introduce a stub irq_ack_none().
> 
> For ARM and x86, export the existing {ack,end}_none() helpers, gaining an irq_
> prefix for consisntency with everything else in no_irq_type.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

For the PPC parts:

Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>a

Thanks,
Shawn
Re: [PATCH 1/2] arch/irq: Make irq_ack_none() mandatory
Posted by Julien Grall 1 year, 8 months ago 
Hi,

On 30/05/2024 19:40, Andrew Cooper wrote:
> Any non-stub implementation of these is going to have to do something here.
> 
> irq_end_none() is more complicated and has arch-specific interactions with
> irq_ack_none(), so make it optional.
> 
> For PPC, introduce a stub irq_ack_none().
> 
> For ARM and x86, export the existing {ack,end}_none() helpers, gaining an irq_
> prefix for consisntency with everything else in no_irq_type.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall
Re: [PATCH 1/2] arch/irq: Make irq_ack_none() mandatory
Posted by Jan Beulich 1 year, 8 months ago 
On 30.05.2024 20:40, Andrew Cooper wrote:
> Any non-stub implementation of these is going to have to do something here.

For whatever definition of "something", seeing ...

> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -31,12 +31,12 @@ struct irq_guest
>      unsigned int virq;
>  };
>  
> -static void ack_none(struct irq_desc *irq)
> +void irq_ack_none(struct irq_desc *irq)
>  {
>      printk("unexpected IRQ trap at irq %02x\n", irq->irq);
>  }

... this, which - perhaps apart from the word "trap" - is entirely Arm-
independent, and could hence quite well live in a common code fallback
implementation. Nevertheless with patch 2 clearly being an improvement,
both patches:
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan
Re: [PATCH 1/2] arch/irq: Make irq_ack_none() mandatory
Posted by Andrew Cooper 1 year, 8 months ago 
On 31/05/2024 7:42 am, Jan Beulich wrote:
> On 30.05.2024 20:40, Andrew Cooper wrote:
>> Any non-stub implementation of these is going to have to do something here.
> For whatever definition of "something", seeing ...
>
>> --- a/xen/arch/arm/irq.c
>> +++ b/xen/arch/arm/irq.c
>> @@ -31,12 +31,12 @@ struct irq_guest
>>      unsigned int virq;
>>  };
>>  
>> -static void ack_none(struct irq_desc *irq)
>> +void irq_ack_none(struct irq_desc *irq)
>>  {
>>      printk("unexpected IRQ trap at irq %02x\n", irq->irq);
>>  }
> ... this, which - perhaps apart from the word "trap" - is entirely Arm-
> independent, and could hence quite well live in a common code fallback
> implementation.

Not really.

On ARM, ack()+end() are both mandatory and it's end() which is taking
the useful action.

On x86, ack() has the effect and end() is optional (and has a different
prototype even!)


>  Nevertheless with patch 2 clearly being an improvement,
> both patches:
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

~Andrew
Re: [PATCH 1/2] arch/irq: Make irq_ack_none() mandatory
Posted by Oleksii K. 1 year, 8 months ago 
On Thu, 2024-05-30 at 19:40 +0100, Andrew Cooper wrote:
> Any non-stub implementation of these is going to have to do something
> here.
> 
> irq_end_none() is more complicated and has arch-specific interactions
> with
> irq_ack_none(), so make it optional.
> 
> For PPC, introduce a stub irq_ack_none().
> 
> For ARM and x86, export the existing {ack,end}_none() helpers,
> gaining an irq_
> prefix for consisntency with everything else in no_irq_type.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
LGTM: Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

~ Oleksii
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien@xen.org>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> CC: Bertrand Marquis <bertrand.marquis@arm.com>
> CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> CC: Shawn Anastasio <sanastasio@raptorengineering.com>
> ---
>  xen/arch/arm/include/asm/irq.h | 3 +++
>  xen/arch/arm/irq.c             | 8 ++++----
>  xen/arch/ppc/stubs.c           | 6 ++++++
>  xen/arch/x86/irq.c             | 4 ++--
>  xen/include/xen/irq.h          | 6 ++++++
>  5 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/irq.h
> b/xen/arch/arm/include/asm/irq.h
> index 1bae5388878e..ec437add0971 100644
> --- a/xen/arch/arm/include/asm/irq.h
> +++ b/xen/arch/arm/include/asm/irq.h
> @@ -98,6 +98,9 @@ void irq_set_affinity(struct irq_desc *desc, const
> cpumask_t *mask);
>   */
>  bool irq_type_set_by_domain(const struct domain *d);
>  
> +void irq_end_none(struct irq_desc *irq);
> +#define irq_end_none irq_end_none
> +
>  #endif /* _ASM_HW_IRQ_H */
>  /*
>   * Local variables:
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index bcce80a4d624..7138f9e7c283 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -31,12 +31,12 @@ struct irq_guest
>      unsigned int virq;
>  };
>  
> -static void ack_none(struct irq_desc *irq)
> +void irq_ack_none(struct irq_desc *irq)
>  {
>      printk("unexpected IRQ trap at irq %02x\n", irq->irq);
>  }
>  
> -static void end_none(struct irq_desc *irq)
> +void irq_end_none(struct irq_desc *irq)
>  {
>      /*
>       * Still allow a CPU to end an interrupt if we receive a
> spurious
> @@ -51,8 +51,8 @@ hw_irq_controller no_irq_type = {
>      .shutdown = irq_shutdown_none,
>      .enable = irq_enable_none,
>      .disable = irq_disable_none,
> -    .ack = ack_none,
> -    .end = end_none
> +    .ack = irq_ack_none,
> +    .end = irq_end_none
>  };
>  
>  static irq_desc_t irq_desc[NR_IRQS];
> diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
> index da193839bd09..4e03428e071a 100644
> --- a/xen/arch/ppc/stubs.c
> +++ b/xen/arch/ppc/stubs.c
> @@ -134,12 +134,18 @@ void pirq_set_affinity(struct domain *d, int
> pirq, const cpumask_t *mask)
>      BUG_ON("unimplemented");
>  }
>  
> +void irq_ack_none(struct irq_desc *desc)
> +{
> +    BUG_ON("unimplemented");
> +}
> +
>  hw_irq_controller no_irq_type = {
>      .typename = "none",
>      .startup = irq_startup_none,
>      .shutdown = irq_shutdown_none,
>      .enable = irq_enable_none,
>      .disable = irq_disable_none,
> +    .ack = irq_ack_none,
>  };
>  
>  int arch_init_one_irq_desc(struct irq_desc *desc)
> diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
> index c16205a9beb6..cfd7a08479d2 100644
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -467,7 +467,7 @@ int __init init_irq_data(void)
>      return 0;
>  }
>  
> -static void cf_check ack_none(struct irq_desc *desc)
> +void cf_check irq_ack_none(struct irq_desc *desc)
>  {
>      ack_bad_irq(desc->irq);
>  }
> @@ -478,7 +478,7 @@ hw_irq_controller no_irq_type = {
>      irq_shutdown_none,
>      irq_enable_none,
>      irq_disable_none,
> -    ack_none,
> +    irq_ack_none,
>  };
>  
>  static vmask_t *irq_get_used_vector_mask(int irq)
> diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
> index 89f7a8317a87..b71f65db8621 100644
> --- a/xen/include/xen/irq.h
> +++ b/xen/include/xen/irq.h
> @@ -130,6 +130,12 @@ void cf_check irq_actor_none(struct irq_desc
> *desc);
>  #define irq_disable_none irq_actor_none
>  #define irq_enable_none irq_actor_none
>  
> +/*
> + * irq_ack_none() must be provided by the architecture.
> + * irq_end_none() is optional, and opted into using a define.
> + */
> +void irq_ack_none(struct irq_desc *irq);
> +
>  /*
>   * Per-cpu interrupted context register state - the inner-most
> interrupt frame
>   * on the stack.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.