Bug in xenomai 3.2.x on ARM

Bug in xenomai 3.2.x on ARM

[François Legal]

Hello,

trying to port a running Xenomai 3.1.x/Linux 4.4.x system to Xenomai 3.2.x/Linux 4.4.x running on Zynq7000, I found a (maybe) bug in intr.c

In xnintr_attach, the call to ipipe_set_irq_affinity (when in SMP) expects a 0 return code. However, on my platform, I end up in irq_gic.c in gic_set_affinity() which returns IRQ_SET_MASK_OK_DONE (2) when everything is fine.

This IRQ_SET_MASK_OK_DONE does not seem very common in the kernel (I could find a few occurences, on Gic v3 so maybe AARCH64 has the same problem, plus some PCI devices).

I was wondering if this was something to fix in ipipe or xenomai ?

François

Re: Bug in xenomai 3.2.x on ARM

[Greg Gallagher]

On Tue, Mar 22, 2022 at 12:49 PM François Legal via Xenomai <
xenomai@xenomai.org> wrote:

> Hello,
>
> trying to port a running Xenomai 3.1.x/Linux 4.4.x system to Xenomai
> 3.2.x/Linux 4.4.x running on Zynq7000, I found a (maybe) bug in intr.c
>
> In xnintr_attach, the call to ipipe_set_irq_affinity (when in SMP) expects
> a 0 return code. However, on my platform, I end up in irq_gic.c in
> gic_set_affinity() which returns IRQ_SET_MASK_OK_DONE (2) when everything
> is fine.
>
> This IRQ_SET_MASK_OK_DONE does not seem very common in the kernel (I could
> find a few occurences, on Gic v3 so maybe AARCH64 has the same problem,
> plus some PCI devices).
>
> I was wondering if this was something to fix in ipipe or xenomai ?
>
> François


Do the newer kernels have the same return value? Do we test 3.2.x on the
older kernels? Maybe just cip ?

-Greg

>
>

Re: Bug in xenomai 3.2.x on ARM

[Jan Kiszka]

On 22.03.22 18:31, Greg Gallagher via Xenomai wrote:
> On Tue, Mar 22, 2022 at 12:49 PM François Legal via Xenomai <
> xenomai@xenomai.org> wrote:
> 
>> Hello,
>>
>> trying to port a running Xenomai 3.1.x/Linux 4.4.x system to Xenomai
>> 3.2.x/Linux 4.4.x running on Zynq7000, I found a (maybe) bug in intr.c
>>
>> In xnintr_attach, the call to ipipe_set_irq_affinity (when in SMP) expects
>> a 0 return code. However, on my platform, I end up in irq_gic.c in
>> gic_set_affinity() which returns IRQ_SET_MASK_OK_DONE (2) when everything
>> is fine.
>>
>> This IRQ_SET_MASK_OK_DONE does not seem very common in the kernel (I could
>> find a few occurences, on Gic v3 so maybe AARCH64 has the same problem,
>> plus some PCI devices).
>>
>> I was wondering if this was something to fix in ipipe or xenomai ?
>>
>> François
> 
> 
> Do the newer kernels have the same return value? Do we test 3.2.x on the
> older kernels? Maybe just cip ?
> 

We limited testing to 4.4-cip with 3.0.x. Newer kernels require newer
Xenomai, and vice versa.

But let's first understand if the issue if 4.4-specific.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

Re: Bug in xenomai 3.2.x on ARM

[François Legal]

Le Mardi, Mars 22, 2022 19:57 CET, Jan Kiszka <jan.kiszka@siemens.com> a écrit:

> On 22.03.22 18:31, Greg Gallagher via Xenomai wrote:
> > On Tue, Mar 22, 2022 at 12:49 PM François Legal via Xenomai <
> > xenomai@xenomai.org> wrote:
> >
> >> Hello,
> >>
> >> trying to port a running Xenomai 3.1.x/Linux 4.4.x system to Xenomai
> >> 3.2.x/Linux 4.4.x running on Zynq7000, I found a (maybe) bug in intr.c
> >>
> >> In xnintr_attach, the call to ipipe_set_irq_affinity (when in SMP) expects
> >> a 0 return code. However, on my platform, I end up in irq_gic.c in
> >> gic_set_affinity() which returns IRQ_SET_MASK_OK_DONE (2) when everything
> >> is fine.
> >>
> >> This IRQ_SET_MASK_OK_DONE does not seem very common in the kernel (I could
> >> find a few occurences, on Gic v3 so maybe AARCH64 has the same problem,
> >> plus some PCI devices).
> >>
> >> I was wondering if this was something to fix in ipipe or xenomai ?
> >>
> >> François
> >
> >
> > Do the newer kernels have the same return value? Do we test 3.2.x on the
> > older kernels? Maybe just cip ?
> >
>
> We limited testing to 4.4-cip with 3.0.x. Newer kernels require newer
> Xenomai, and vice versa.
>
> But let's first understand if the issue if 4.4-specific.
>
> Jan
>
> --
> Siemens AG, Technology
> Competence Center Embedded Linux

 In 4.4, gic_set_affinity returns IRQ_SET_MASK_OK (aka 0). In 4.19 is already returns IRQ_SET_MASK_OK_DONE , but in Xenomai 3.1.x, there were no call to ipipe_set_irq_affinity AFAIK.

François

Re: Bug in xenomai 3.2.x on ARM

[François Legal]

Le Mardi, Mars 22, 2022 19:57 CET, Jan Kiszka <jan.kiszka@siemens.com> a écrit:

> On 22.03.22 18:31, Greg Gallagher via Xenomai wrote:
> > On Tue, Mar 22, 2022 at 12:49 PM François Legal via Xenomai <
> > xenomai@xenomai.org> wrote:
> >
> >> Hello,
> >>
> >> trying to port a running Xenomai 3.1.x/Linux 4.4.x system to Xenomai
> >> 3.2.x/Linux 4.4.x running on Zynq7000, I found a (maybe) bug in intr.c
> >>
> >> In xnintr_attach, the call to ipipe_set_irq_affinity (when in SMP) expects
> >> a 0 return code. However, on my platform, I end up in irq_gic.c in
> >> gic_set_affinity() which returns IRQ_SET_MASK_OK_DONE (2) when everything
> >> is fine.
> >>
> >> This IRQ_SET_MASK_OK_DONE does not seem very common in the kernel (I could
> >> find a few occurences, on Gic v3 so maybe AARCH64 has the same problem,
> >> plus some PCI devices).
> >>
> >> I was wondering if this was something to fix in ipipe or xenomai ?
> >>
> >> François
> >
> >
> > Do the newer kernels have the same return value? Do we test 3.2.x on the
> > older kernels? Maybe just cip ?
> >
>
> We limited testing to 4.4-cip with 3.0.x. Newer kernels require newer
> Xenomai, and vice versa.
>
> But let's first understand if the issue if 4.4-specific.
>
> Jan
>
> --
> Siemens AG, Technology
> Competence Center Embedded Linux

I just found out, re reading the message that I made a typo in my original message. I'm not trying to port the system to xenomai 3.2/linux 4.4 but linux 5.4

Also, I was wondering how I could make this topic move forward.

Thanks in advance

François

Re: Bug in xenomai 3.2.x on ARM

[Gunter Grau]

We also saw this issue when porting out application to Xenomai 3.2 with imx6.
Looks like the return value in upstream is still defined to be not always 0 on success. So we wrote a patch.
It looks like this happend when dovetail was introduced.

The patch only applies to Xenomai 3.2 since ipipe is not part of the
master anymore.

Regards, Gunter

[PATCH] cobalt: ipipe: intr: Fix return value check of ipipe_set_irq_affinity

[Gunter Grau]

From: Julian Haller <julian.haller@philips.com>

ipipe_set_irq_affinity directly returns the value of the regular
irq_set_affinity method. As described in irq.h in the linux kernel,
the following return values indicate a success:

/*
 * Return value for chip->irq_set_affinity()
 *
 * IRQ_SET_MASK_OK      - OK, core updates irq_common_data.affinity
 * IRQ_SET_MASK_NOCPY   - OK, chip did update irq_common_data.affinity
 * IRQ_SET_MASK_OK_DONE - Same as IRQ_SET_MASK_OK for core. Special code to
 *                        support stacked irqchips, which indicates skipping
 *                        all descendent irqchips.
 */
enum {
        IRQ_SET_MASK_OK = 0,
        IRQ_SET_MASK_OK_NOCOPY,
        IRQ_SET_MASK_OK_DONE,
};

As one example, the GIC in i.MX6 devices returns IRQ_SET_MASK_OK_DONE
on success. Fix the xintr_attach function by treating all positive
return values as success.

Signed-off-by: Gunter Grau <gunter.grau@philips.com>
---
 kernel/cobalt/ipipe/intr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/cobalt/ipipe/intr.c b/kernel/cobalt/ipipe/intr.c
index 378c7f07d..cb15597f7 100644
--- a/kernel/cobalt/ipipe/intr.c
+++ b/kernel/cobalt/ipipe/intr.c
@@ -869,7 +869,7 @@ int xnintr_attach(struct xnintr *intr, void *cookie, const cpumask_t *cpumask)
 			return -EINVAL;
 	}
 	ret = ipipe_set_irq_affinity(intr->irq, *effective_mask);
-	if (ret)
+	if (ret < 0)
 		return ret;
 #endif /* CONFIG_SMP */
 
-- 
2.25.1

RE: [PATCH] cobalt: ipipe: intr: Fix return value check of ipipe_set_irq_affinity

[Grau, Gunter]

Hi,

Sorry, looks like something went wrong with the reply-to.
It belongs to this issue: https://xenomai.org/pipermail/xenomai/2022-April/047624.html

Thanks, Gunter


> -----Original Message-----
> From: Xenomai <xenomai-bounces@xenomai.org> On Behalf Of Gunter
> Grau via Xenomai
> Sent: Freitag, 20. Mai 2022 16:29
> To: xenomai@xenomai.org
> Cc: Gunter Grau <guntgrau@bbl.ma.philips.com>
> Subject: [PATCH] cobalt: ipipe: intr: Fix return value check of
> ipipe_set_irq_affinity
>
> Caution: This e-mail originated from outside of Philips, be careful for
> phishing.
>
>
> From: Julian Haller <julian.haller@philips.com>
>
> ipipe_set_irq_affinity directly returns the value of the regular
> irq_set_affinity method. As described in irq.h in the linux kernel, the
> following return values indicate a success:
>
> /*
>  * Return value for chip->irq_set_affinity()
>  *
>  * IRQ_SET_MASK_OK      - OK, core updates irq_common_data.affinity
>  * IRQ_SET_MASK_NOCPY   - OK, chip did update irq_common_data.affinity
>  * IRQ_SET_MASK_OK_DONE - Same as IRQ_SET_MASK_OK for core. Special
> code to
>  *                        support stacked irqchips, which indicates skipping
>  *                        all descendent irqchips.
>  */
> enum {
>         IRQ_SET_MASK_OK = 0,
>         IRQ_SET_MASK_OK_NOCOPY,
>         IRQ_SET_MASK_OK_DONE,
> };
>
> As one example, the GIC in i.MX6 devices returns IRQ_SET_MASK_OK_DONE
> on success. Fix the xintr_attach function by treating all positive return values
> as success.
>
> Signed-off-by: Gunter Grau <gunter.grau@philips.com>
> ---
>  kernel/cobalt/ipipe/intr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/cobalt/ipipe/intr.c b/kernel/cobalt/ipipe/intr.c index
> 378c7f07d..cb15597f7 100644
> --- a/kernel/cobalt/ipipe/intr.c
> +++ b/kernel/cobalt/ipipe/intr.c
> @@ -869,7 +869,7 @@ int xnintr_attach(struct xnintr *intr, void *cookie,
> const cpumask_t *cpumask)
>                         return -EINVAL;
>         }
>         ret = ipipe_set_irq_affinity(intr->irq, *effective_mask);
> -       if (ret)
> +       if (ret < 0)
>                 return ret;
>  #endif /* CONFIG_SMP */
>
> --
> 2.25.1
>


________________________________
The information contained in this message may be confidential and legally protected under applicable law. The message is intended solely for the addressee(s). If you are not the intended recipient, you are hereby notified that any use, forwarding, dissemination, or reproduction of this message is strictly prohibited and may be unlawful. If you are not the intended recipient, please contact the sender by return e-mail and destroy all copies of the original message.

Re: [PATCH] cobalt: ipipe: intr: Fix return value check of ipipe_set_irq_affinity

[Jan Kiszka]

On 20.05.22 16:29, Gunter Grau via Xenomai wrote:
> From: Julian Haller <julian.haller@philips.com>
> 
> ipipe_set_irq_affinity directly returns the value of the regular
> irq_set_affinity method. As described in irq.h in the linux kernel,
> the following return values indicate a success:
> 
> /*
>  * Return value for chip->irq_set_affinity()
>  *
>  * IRQ_SET_MASK_OK      - OK, core updates irq_common_data.affinity
>  * IRQ_SET_MASK_NOCPY   - OK, chip did update irq_common_data.affinity
>  * IRQ_SET_MASK_OK_DONE - Same as IRQ_SET_MASK_OK for core. Special code to
>  *                        support stacked irqchips, which indicates skipping
>  *                        all descendent irqchips.
>  */
> enum {
>         IRQ_SET_MASK_OK = 0,
>         IRQ_SET_MASK_OK_NOCOPY,
>         IRQ_SET_MASK_OK_DONE,
> };
> 
> As one example, the GIC in i.MX6 devices returns IRQ_SET_MASK_OK_DONE
> on success. Fix the xintr_attach function by treating all positive
> return values as success.
> 
> Signed-off-by: Gunter Grau <gunter.grau@philips.com>
> ---
>  kernel/cobalt/ipipe/intr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/cobalt/ipipe/intr.c b/kernel/cobalt/ipipe/intr.c
> index 378c7f07d..cb15597f7 100644
> --- a/kernel/cobalt/ipipe/intr.c
> +++ b/kernel/cobalt/ipipe/intr.c
> @@ -869,7 +869,7 @@ int xnintr_attach(struct xnintr *intr, void *cookie, const cpumask_t *cpumask)
>  			return -EINVAL;
>  	}
>  	ret = ipipe_set_irq_affinity(intr->irq, *effective_mask);
> -	if (ret)
> +	if (ret < 0)
>  		return ret;
>  #endif /* CONFIG_SMP */
>  

Thanks, applied to the 3.2 stable tree. Other versions are not affected.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux