Implement init_IRQ() to initalize various IRQs.

Currently, this function initializes the irq_desc[] array,
which stores IRQ descriptors containing various information
about each IRQ, such as the type of hardware handling, whether
the IRQ is disabled, etc.
The initialization is basic at this point and includes setting
IRQ_TYPE_INVALID as the IRQ type, assigning the IRQ number ( which
is just a consequent index of irq_desc[] array ) to
desc->irq, and setting desc->action to NULL.

Additionally, the function init_irq_data() is introduced to
initialize the IRQ descriptors for all IRQs in the system.

Reuse defines of IRQ_TYPE_* from asm-generic/irq-dt.h.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v2:
 - Move an introduction of IRQ_TYPE_* defines to the separate patch.
 - Reuse asm-generic/irq-dt.h.
 - Use 'unsigned int' for local irq variable inside init_irq_data().
---
 xen/arch/riscv/Makefile             |  1 +
 xen/arch/riscv/include/asm/Makefile |  1 +
 xen/arch/riscv/include/asm/irq.h    |  7 +++++
 xen/arch/riscv/irq.c                | 45 +++++++++++++++++++++++++++++
 xen/arch/riscv/setup.c              |  3 ++
 xen/arch/riscv/stubs.c              |  5 ----
 6 files changed, 57 insertions(+), 5 deletions(-)
 create mode 100644 xen/arch/riscv/irq.c

diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index f42cf3dfa6..a1c145c506 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -3,6 +3,7 @@ obj-y += cpufeature.o
 obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
 obj-y += entry.o
 obj-y += intc.o
+obj-y += irq.o
 obj-y += mm.o
 obj-y += pt.o
 obj-$(CONFIG_RISCV_64) += riscv64/
diff --git a/xen/arch/riscv/include/asm/Makefile b/xen/arch/riscv/include/asm/Makefile
index c989a7f89b..bfdf186c68 100644
--- a/xen/arch/riscv/include/asm/Makefile
+++ b/xen/arch/riscv/include/asm/Makefile
@@ -5,6 +5,7 @@ generic-y += div64.h
 generic-y += hardirq.h
 generic-y += hypercall.h
 generic-y += iocap.h
+generic-y += irq-dt.h
 generic-y += paging.h
 generic-y += percpu.h
 generic-y += perfc_defn.h
diff --git a/xen/arch/riscv/include/asm/irq.h b/xen/arch/riscv/include/asm/irq.h
index 2a48da2651..f609df466e 100644
--- a/xen/arch/riscv/include/asm/irq.h
+++ b/xen/arch/riscv/include/asm/irq.h
@@ -3,6 +3,11 @@
 #define ASM__RISCV__IRQ_H
 
 #include <xen/bug.h>
+#include <xen/device_tree.h>
+
+#include <asm/irq-dt.h>
+
+#define NR_IRQS 1024
 
 /* TODO */
 #define nr_irqs 0U
@@ -25,6 +30,8 @@ static inline void arch_move_irqs(struct vcpu *v)
     BUG_ON("unimplemented");
 }
 
+void init_IRQ(void);
+
 #endif /* ASM__RISCV__IRQ_H */
 
 /*
diff --git a/xen/arch/riscv/irq.c b/xen/arch/riscv/irq.c
new file mode 100644
index 0000000000..26a8556b2c
--- /dev/null
+++ b/xen/arch/riscv/irq.c
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+/*
+ * RISC-V Trap handlers
+ *
+ * Copyright (c) 2024 Vates
+ */
+
+#include <xen/bug.h>
+#include <xen/init.h>
+#include <xen/irq.h>
+
+static irq_desc_t irq_desc[NR_IRQS];
+
+int arch_init_one_irq_desc(struct irq_desc *desc)
+{
+    desc->arch.type = IRQ_TYPE_INVALID;
+
+    return 0;
+}
+
+static int __init init_irq_data(void)
+{
+    unsigned int irq;
+
+    for ( irq = 0; irq < NR_IRQS; irq++ )
+    {
+        struct irq_desc *desc = irq_to_desc(irq);
+        int rc = init_one_irq_desc(desc);
+
+        if ( rc )
+            return rc;
+
+        desc->irq = irq;
+        desc->action  = NULL;
+    }
+
+    return 0;
+}
+
+void __init init_IRQ(void)
+{
+    if ( init_irq_data() < 0 )
+        panic("initialization of IRQ data failed\n");
+}
diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index 2a564512d7..82f8839eff 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -6,6 +6,7 @@
 #include <xen/compile.h>
 #include <xen/device_tree.h>
 #include <xen/init.h>
+#include <xen/irq.h>
 #include <xen/mm.h>
 #include <xen/shutdown.h>
 #include <xen/smp.h>
@@ -127,6 +128,8 @@ void __init noreturn start_xen(unsigned long bootcpu_id,
         panic("Booting using ACPI isn't supported\n");
     }
 
+    init_IRQ();
+
     riscv_fill_hwcap();
 
     preinit_xen_time();
diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
index fdcf91054e..e396b67cd3 100644
--- a/xen/arch/riscv/stubs.c
+++ b/xen/arch/riscv/stubs.c
@@ -107,11 +107,6 @@ void irq_ack_none(struct irq_desc *desc)
     BUG_ON("unimplemented");
 }
 
-int arch_init_one_irq_desc(struct irq_desc *desc)
-{
-    BUG_ON("unimplemented");
-}
-
 void smp_send_state_dump(unsigned int cpu)
 {
     BUG_ON("unimplemented");
-- 
2.49.0

On 06.05.2025 18:51, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/include/asm/irq.h
> +++ b/xen/arch/riscv/include/asm/irq.h
> @@ -3,6 +3,11 @@
>  #define ASM__RISCV__IRQ_H
>  
>  #include <xen/bug.h>
> +#include <xen/device_tree.h>
> +
> +#include <asm/irq-dt.h>
> +
> +#define NR_IRQS 1024

Is this arbitrary or arch-induced? Imo it wants saying in a brief comment either
way. Then again maybe it's entirely obvious for a RISC-V person ...

> --- /dev/null
> +++ b/xen/arch/riscv/irq.c
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +/*
> + * RISC-V Trap handlers
> + *
> + * Copyright (c) 2024 Vates
> + */
> +
> +#include <xen/bug.h>
> +#include <xen/init.h>
> +#include <xen/irq.h>
> +
> +static irq_desc_t irq_desc[NR_IRQS];
> +
> +int arch_init_one_irq_desc(struct irq_desc *desc)
> +{
> +    desc->arch.type = IRQ_TYPE_INVALID;
> +
> +    return 0;
> +}
> +
> +static int __init init_irq_data(void)
> +{
> +    unsigned int irq;
> +
> +    for ( irq = 0; irq < NR_IRQS; irq++ )
> +    {
> +        struct irq_desc *desc = irq_to_desc(irq);
> +        int rc = init_one_irq_desc(desc);
> +
> +        if ( rc )
> +            return rc;
> +
> +        desc->irq = irq;
> +        desc->action  = NULL;

Nit: You copied a stray blank from Arm code. Actually I wonder why it isn't
init_one_irq_desc() that clears this field. It also feels like ->irq would
better be set ahead of calling that function, like x86 has it.

Jan

On 5/14/25 4:49 PM, Jan Beulich wrote:
> On 06.05.2025 18:51, Oleksii Kurochko wrote:
>> --- a/xen/arch/riscv/include/asm/irq.h
>> +++ b/xen/arch/riscv/include/asm/irq.h
>> @@ -3,6 +3,11 @@
>>   #define ASM__RISCV__IRQ_H
>>   
>>   #include <xen/bug.h>
>> +#include <xen/device_tree.h>
>> +
>> +#include <asm/irq-dt.h>
>> +
>> +#define NR_IRQS 1024
> Is this arbitrary or arch-induced? Imo it wants saying in a brief comment either
> way. Then again maybe it's entirely obvious for a RISC-V person ...

1024 it is number of interrupt sources for PLIC and APLIC. I will add the comment above:
/*
  * According to the AIA spec:
  *   The maximum number of interrupt sources an APLIC may support is 1023.
  *
  * The same is true for PLIC.
  *
  * Interrupt Source 0 is reserved and shall never generate an interrupt.
  */
#define NR_CPUS 1024

Probably, it makes sense to change it to 1023 as interrupt 0 isn't really used.

>
>> --- /dev/null
>> +++ b/xen/arch/riscv/irq.c
>> @@ -0,0 +1,45 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +
>> +/*
>> + * RISC-V Trap handlers
>> + *
>> + * Copyright (c) 2024 Vates
>> + */
>> +
>> +#include <xen/bug.h>
>> +#include <xen/init.h>
>> +#include <xen/irq.h>
>> +
>> +static irq_desc_t irq_desc[NR_IRQS];
>> +
>> +int arch_init_one_irq_desc(struct irq_desc *desc)
>> +{
>> +    desc->arch.type = IRQ_TYPE_INVALID;
>> +
>> +    return 0;
>> +}
>> +
>> +static int __init init_irq_data(void)
>> +{
>> +    unsigned int irq;
>> +
>> +    for ( irq = 0; irq < NR_IRQS; irq++ )
>> +    {
>> +        struct irq_desc *desc = irq_to_desc(irq);
>> +        int rc = init_one_irq_desc(desc);
>> +
>> +        if ( rc )
>> +            return rc;
>> +
>> +        desc->irq = irq;
>> +        desc->action  = NULL;
> Nit: You copied a stray blank from Arm code. Actually I wonder why it isn't
> init_one_irq_desc() that clears this field.

I can come up with the patch which does desc->action = NULL in init_one_irq_desc().
But considering that irq_desc[] is zero-initialized then perhaps there is no any
sense for desc->action = NULL, at all.

> It also feels like ->irq would
> better be set ahead of calling that function, like x86 has it.

But what is the difference with an order of writing a value to ->irq? It isn't
really used in init_one_irq_desc(). Or could ->irq be used in arch_init_one_irq_desc()
for something?

~ Oleksii

On 16.05.2025 13:53, Oleksii Kurochko wrote:
> On 5/14/25 4:49 PM, Jan Beulich wrote:
>> On 06.05.2025 18:51, Oleksii Kurochko wrote:
>>> --- a/xen/arch/riscv/include/asm/irq.h
>>> +++ b/xen/arch/riscv/include/asm/irq.h
>>> @@ -3,6 +3,11 @@
>>>   #define ASM__RISCV__IRQ_H
>>>   
>>>   #include <xen/bug.h>
>>> +#include <xen/device_tree.h>
>>> +
>>> +#include <asm/irq-dt.h>
>>> +
>>> +#define NR_IRQS 1024
>> Is this arbitrary or arch-induced? Imo it wants saying in a brief comment either
>> way. Then again maybe it's entirely obvious for a RISC-V person ...
> 
> 1024 it is number of interrupt sources for PLIC and APLIC. I will add the comment above:
> /*
>   * According to the AIA spec:
>   *   The maximum number of interrupt sources an APLIC may support is 1023.
>   *
>   * The same is true for PLIC.
>   *
>   * Interrupt Source 0 is reserved and shall never generate an interrupt.
>   */
> #define NR_CPUS 1024

s/CPU/IRQ I suppose.

> Probably, it makes sense to change it to 1023 as interrupt 0 isn't really used.

I'd recommend against that. It would likely make things more difficult when
range-checking IRQ numbers.

>>> --- /dev/null
>>> +++ b/xen/arch/riscv/irq.c
>>> @@ -0,0 +1,45 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>>> +
>>> +/*
>>> + * RISC-V Trap handlers
>>> + *
>>> + * Copyright (c) 2024 Vates
>>> + */
>>> +
>>> +#include <xen/bug.h>
>>> +#include <xen/init.h>
>>> +#include <xen/irq.h>
>>> +
>>> +static irq_desc_t irq_desc[NR_IRQS];
>>> +
>>> +int arch_init_one_irq_desc(struct irq_desc *desc)
>>> +{
>>> +    desc->arch.type = IRQ_TYPE_INVALID;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int __init init_irq_data(void)
>>> +{
>>> +    unsigned int irq;
>>> +
>>> +    for ( irq = 0; irq < NR_IRQS; irq++ )
>>> +    {
>>> +        struct irq_desc *desc = irq_to_desc(irq);
>>> +        int rc = init_one_irq_desc(desc);
>>> +
>>> +        if ( rc )
>>> +            return rc;
>>> +
>>> +        desc->irq = irq;
>>> +        desc->action  = NULL;
>> Nit: You copied a stray blank from Arm code. Actually I wonder why it isn't
>> init_one_irq_desc() that clears this field.
> 
> I can come up with the patch which does desc->action = NULL in init_one_irq_desc().
> But considering that irq_desc[] is zero-initialized then perhaps there is no any
> sense for desc->action = NULL, at all.

Oh, yes, indeed.

>> It also feels like ->irq would
>> better be set ahead of calling that function, like x86 has it.
> 
> But what is the difference with an order of writing a value to ->irq? It isn't
> really used in init_one_irq_desc().

That's the present state of things, yes. There or ...

> Or could ->irq be used in arch_init_one_irq_desc()
> for something?

... there it could be used e.g. for a log message. Maybe even just a transient
debugging one. And there's no (proper) way to re-establish the IRQ number from
the desc pointer, at least not in arch-independent code.

Jan