platform_get_irq() recieves information about device's irq ( type
and irq number ) from device tree node and using this information
update irq descriptor in irq_desc[] array.
Introduce dt_irq_xlate and initialize with aplic_irq_xlate() as
it is used by dt_device_get_irq() which is called by
platform_get_irq().
Co-developed-by: Romain Caritey <Romain.Caritey@microchip.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
- Add cf_check for aplic_irq_xlate().
- Ident label in irq_set_type().
- Return proper -E... values for platform_get_irq().
---
xen/arch/riscv/aplic.c | 20 +++++++++++++++
xen/arch/riscv/include/asm/irq.h | 3 +++
xen/arch/riscv/irq.c | 42 ++++++++++++++++++++++++++++++++
3 files changed, 65 insertions(+)
diff --git a/xen/arch/riscv/aplic.c b/xen/arch/riscv/aplic.c
index caba8f8993..10ae81f7ac 100644
--- a/xen/arch/riscv/aplic.c
+++ b/xen/arch/riscv/aplic.c
@@ -11,6 +11,7 @@
#include <xen/errno.h>
#include <xen/init.h>
+#include <xen/irq.h>
#include <xen/sections.h>
#include <xen/types.h>
@@ -21,6 +22,23 @@ static struct intc_info __ro_after_init aplic_info = {
.hw_version = INTC_APLIC,
};
+static int cf_check aplic_irq_xlate(const uint32_t *intspec,
+ unsigned int intsize,
+ unsigned int *out_hwirq,
+ unsigned int *out_type)
+{
+ if ( intsize < 2 )
+ return -EINVAL;
+
+ /* Mapping 1:1 */
+ *out_hwirq = intspec[0];
+
+ if ( out_type )
+ *out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
+
+ return 0;
+}
+
static int __init aplic_preinit(struct dt_device_node *node, const void *dat)
{
if ( aplic_info.node )
@@ -35,6 +53,8 @@ static int __init aplic_preinit(struct dt_device_node *node, const void *dat)
aplic_info.node = node;
+ dt_irq_xlate = aplic_irq_xlate;
+
return 0;
}
diff --git a/xen/arch/riscv/include/asm/irq.h b/xen/arch/riscv/include/asm/irq.h
index f609df466e..6223bbbed5 100644
--- a/xen/arch/riscv/include/asm/irq.h
+++ b/xen/arch/riscv/include/asm/irq.h
@@ -30,6 +30,9 @@ static inline void arch_move_irqs(struct vcpu *v)
BUG_ON("unimplemented");
}
+struct dt_device_node;
+int platform_get_irq(const struct dt_device_node *device, int index);
+
void init_IRQ(void);
#endif /* ASM__RISCV__IRQ_H */
diff --git a/xen/arch/riscv/irq.c b/xen/arch/riscv/irq.c
index 26a8556b2c..4c518bbd97 100644
--- a/xen/arch/riscv/irq.c
+++ b/xen/arch/riscv/irq.c
@@ -7,11 +7,53 @@
*/
#include <xen/bug.h>
+#include <xen/device_tree.h>
+#include <xen/errno.h>
#include <xen/init.h>
#include <xen/irq.h>
static irq_desc_t irq_desc[NR_IRQS];
+static bool irq_validate_new_type(unsigned int curr, unsigned int new)
+{
+ return (curr == IRQ_TYPE_INVALID || curr == new );
+}
+
+static int irq_set_type(unsigned int irq, unsigned int type)
+{
+ unsigned long flags;
+ struct irq_desc *desc = irq_to_desc(irq);
+ int ret = -EBUSY;
+
+ spin_lock_irqsave(&desc->lock, flags);
+
+ if ( !irq_validate_new_type(desc->arch.type, type) )
+ goto err;
+
+ desc->arch.type = type;
+
+ ret = 0;
+
+ err:
+ spin_unlock_irqrestore(&desc->lock, flags);
+
+ return ret;
+}
+
+int platform_get_irq(const struct dt_device_node *device, int index)
+{
+ struct dt_irq dt_irq;
+ int ret;
+
+ if ( (ret = dt_device_get_irq(device, index, &dt_irq)) != 0 )
+ return ret;
+
+ if ( (ret = irq_set_type(dt_irq.irq, dt_irq.type)) != 0 )
+ return ret;
+
+ return dt_irq.irq;
+}
+
int arch_init_one_irq_desc(struct irq_desc *desc)
{
desc->arch.type = IRQ_TYPE_INVALID;
--
2.49.0On 06.05.2025 18:51, Oleksii Kurochko wrote:
> platform_get_irq() recieves information about device's irq ( type
> and irq number ) from device tree node and using this information
> update irq descriptor in irq_desc[] array.
>
> Introduce dt_irq_xlate and initialize with aplic_irq_xlate() as
> it is used by dt_device_get_irq() which is called by
> platform_get_irq().
>
> Co-developed-by: Romain Caritey <Romain.Caritey@microchip.com>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in V2:
> - Add cf_check for aplic_irq_xlate().
> - Ident label in irq_set_type().
> - Return proper -E... values for platform_get_irq().
> ---
> xen/arch/riscv/aplic.c | 20 +++++++++++++++
> xen/arch/riscv/include/asm/irq.h | 3 +++
> xen/arch/riscv/irq.c | 42 ++++++++++++++++++++++++++++++++
> 3 files changed, 65 insertions(+)
>
> diff --git a/xen/arch/riscv/aplic.c b/xen/arch/riscv/aplic.c
> index caba8f8993..10ae81f7ac 100644
> --- a/xen/arch/riscv/aplic.c
> +++ b/xen/arch/riscv/aplic.c
> @@ -11,6 +11,7 @@
>
> #include <xen/errno.h>
> #include <xen/init.h>
> +#include <xen/irq.h>
> #include <xen/sections.h>
> #include <xen/types.h>
>
> @@ -21,6 +22,23 @@ static struct intc_info __ro_after_init aplic_info = {
> .hw_version = INTC_APLIC,
> };
>
> +static int cf_check aplic_irq_xlate(const uint32_t *intspec,
> + unsigned int intsize,
> + unsigned int *out_hwirq,
> + unsigned int *out_type)
> +{
> + if ( intsize < 2 )
> + return -EINVAL;
> +
> + /* Mapping 1:1 */
> + *out_hwirq = intspec[0];
> +
> + if ( out_type )
> + *out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
> +
> + return 0;
> +}
> +
> static int __init aplic_preinit(struct dt_device_node *node, const void *dat)
> {
> if ( aplic_info.node )
> @@ -35,6 +53,8 @@ static int __init aplic_preinit(struct dt_device_node *node, const void *dat)
>
> aplic_info.node = node;
>
> + dt_irq_xlate = aplic_irq_xlate;
> +
> return 0;
> }
>
> diff --git a/xen/arch/riscv/include/asm/irq.h b/xen/arch/riscv/include/asm/irq.h
> index f609df466e..6223bbbed5 100644
> --- a/xen/arch/riscv/include/asm/irq.h
> +++ b/xen/arch/riscv/include/asm/irq.h
> @@ -30,6 +30,9 @@ static inline void arch_move_irqs(struct vcpu *v)
> BUG_ON("unimplemented");
> }
>
> +struct dt_device_node;
> +int platform_get_irq(const struct dt_device_node *device, int index);
> +
> void init_IRQ(void);
>
> #endif /* ASM__RISCV__IRQ_H */
> diff --git a/xen/arch/riscv/irq.c b/xen/arch/riscv/irq.c
> index 26a8556b2c..4c518bbd97 100644
> --- a/xen/arch/riscv/irq.c
> +++ b/xen/arch/riscv/irq.c
> @@ -7,11 +7,53 @@
> */
>
> #include <xen/bug.h>
> +#include <xen/device_tree.h>
> +#include <xen/errno.h>
> #include <xen/init.h>
> #include <xen/irq.h>
>
> static irq_desc_t irq_desc[NR_IRQS];
>
> +static bool irq_validate_new_type(unsigned int curr, unsigned int new)
> +{
> + return (curr == IRQ_TYPE_INVALID || curr == new );
Nit: Stray blank. In fact you could omit the parentheses as well.
> +}
> +
> +static int irq_set_type(unsigned int irq, unsigned int type)
> +{
> + unsigned long flags;
> + struct irq_desc *desc = irq_to_desc(irq);
> + int ret = -EBUSY;
> +
> + spin_lock_irqsave(&desc->lock, flags);
> +
> + if ( !irq_validate_new_type(desc->arch.type, type) )
> + goto err;
> +
> + desc->arch.type = type;
> +
> + ret = 0;
> +
> + err:
> + spin_unlock_irqrestore(&desc->lock, flags);
> +
> + return ret;
> +}
> +
> +int platform_get_irq(const struct dt_device_node *device, int index)
> +{
> + struct dt_irq dt_irq;
> + int ret;
> +
> + if ( (ret = dt_device_get_irq(device, index, &dt_irq)) != 0 )
> + return ret;
> +
> + if ( (ret = irq_set_type(dt_irq.irq, dt_irq.type)) != 0 )
> + return ret;
> +
> + return dt_irq.irq;
What guarantees the value to be at most INT_MAX (i.e. no silent conversion to
a negative value, signaling an error to the caller)? Actually, looking at
irq_set_type(), what guarantees irq_to_desc() there to not overrun irq_desc[]?
There are no bounds checks in aplic_irq_xlate().
Jan
On 5/15/25 9:33 AM, Jan Beulich wrote:
>> +int platform_get_irq(const struct dt_device_node *device, int index)
>> +{
>> + struct dt_irq dt_irq;
>> + int ret;
>> +
>> + if ( (ret = dt_device_get_irq(device, index, &dt_irq)) != 0 )
>> + return ret;
>> +
>> + if ( (ret = irq_set_type(dt_irq.irq, dt_irq.type)) != 0 )
>> + return ret;
>> +
>> + return dt_irq.irq;
> What guarantees the value to be at most INT_MAX (i.e. no silent conversion to
> a negative value, signaling an error to the caller)? Actually, looking at
> irq_set_type(), what guarantees irq_to_desc() there to not overrun irq_desc[]?
> There are no bounds checks in aplic_irq_xlate().
I'm afraid that both aren't guaranteed. I think to have the following in platform_get_irq()
should be enough:
BUILD_BUG_ON(NR_IRQS > INT_MAX);
if ( dt_irq.irq >= NR_IRQS )
panic("irq%d is bigger then NR_IRQS(%d)\n", dt_irq.irq, NR_IRQS);
Probably, the first could be dropped as I'm not sure that anyone will use such big
number for NR_IRQS.
~ Oleksii
On 16.05.2025 16:04, Oleksii Kurochko wrote:
>
> On 5/15/25 9:33 AM, Jan Beulich wrote:
>>> +int platform_get_irq(const struct dt_device_node *device, int index)
>>> +{
>>> + struct dt_irq dt_irq;
>>> + int ret;
>>> +
>>> + if ( (ret = dt_device_get_irq(device, index, &dt_irq)) != 0 )
>>> + return ret;
>>> +
>>> + if ( (ret = irq_set_type(dt_irq.irq, dt_irq.type)) != 0 )
>>> + return ret;
>>> +
>>> + return dt_irq.irq;
>> What guarantees the value to be at most INT_MAX (i.e. no silent conversion to
>> a negative value, signaling an error to the caller)? Actually, looking at
>> irq_set_type(), what guarantees irq_to_desc() there to not overrun irq_desc[]?
>> There are no bounds checks in aplic_irq_xlate().
>
> I'm afraid that both aren't guaranteed. I think to have the following in platform_get_irq()
> should be enough:
> BUILD_BUG_ON(NR_IRQS > INT_MAX);
>
> if ( dt_irq.irq >= NR_IRQS )
> panic("irq%d is bigger then NR_IRQS(%d)\n", dt_irq.irq, NR_IRQS);
>
> Probably, the first could be dropped as I'm not sure that anyone will use such big
> number for NR_IRQS.
I'd say better keep it, even if largely for doc purposes.
Jan
© 2016 - 2025 Red Hat, Inc.