[PATCH] hw/net/can/sja1000: fix bug for single acceptance filer and standard frame

[PATCH] hw/net/can/sja1000: fix bug for single acceptance filer and standard frame

[Pavel Pisa]

From: Pavel Pisa <pisa@cmp.felk.cvut.cz>

A CAN sja1000 standard frame filter mask has been computed and applied
incorrectly for standard frames when single Acceptance Filter Mode
(MOD_AFM = 1) has been selected. The problem has not been found
by Linux kernel testing because it uses dual filter mode (MOD_AFM = 0)
and leaves falters fully open.

The problem has been noticed by Grant Ramsay when testing with Zephyr
RTOS which uses single filter mode.

Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Reported-by: Grant Ramsay <gramsay@enphaseenergy.com>
---
 hw/net/can/can_sja1000.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
index 73201f9139..575df7d2f8 100644
--- a/hw/net/can/can_sja1000.c
+++ b/hw/net/can/can_sja1000.c
@@ -108,7 +108,7 @@ void can_sja_single_filter(struct qemu_can_filter *filter,
         }
 
         filter->can_mask = (uint32_t)amr[0] << 3;
-        filter->can_mask |= (uint32_t)amr[1] << 5;
+        filter->can_mask |= (uint32_t)amr[1] >> 5;
         filter->can_mask = ~filter->can_mask & QEMU_CAN_SFF_MASK;
         if (!(amr[1] & 0x10)) {
             filter->can_mask |= QEMU_CAN_RTR_FLAG;
-- 
2.39.2

Re: [PATCH] hw/net/can/sja1000: fix bug for single acceptance filer and standard frame

[Philippe Mathieu-Daudé]

On 14/12/23 11:46, Pavel Pisa wrote:
> From: Pavel Pisa <pisa@cmp.felk.cvut.cz>
> 
> A CAN sja1000 standard frame filter mask has been computed and applied
> incorrectly for standard frames when single Acceptance Filter Mode
> (MOD_AFM = 1) has been selected. The problem has not been found
> by Linux kernel testing because it uses dual filter mode (MOD_AFM = 0)
> and leaves falters fully open.
> 
> The problem has been noticed by Grant Ramsay when testing with Zephyr
> RTOS which uses single filter mode.
> 

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2028
Fixes: 733210e754 ("hw/net/can: SJA1000 chip register level emulation")

> Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
> Reported-by: Grant Ramsay <gramsay@enphaseenergy.com>
> ---
>   hw/net/can/can_sja1000.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
> index 73201f9139..575df7d2f8 100644
> --- a/hw/net/can/can_sja1000.c
> +++ b/hw/net/can/can_sja1000.c
> @@ -108,7 +108,7 @@ void can_sja_single_filter(struct qemu_can_filter *filter,
>           }
>   
>           filter->can_mask = (uint32_t)amr[0] << 3;
> -        filter->can_mask |= (uint32_t)amr[1] << 5;
> +        filter->can_mask |= (uint32_t)amr[1] >> 5;
>           filter->can_mask = ~filter->can_mask & QEMU_CAN_SFF_MASK;
>           if (!(amr[1] & 0x10)) {
>               filter->can_mask |= QEMU_CAN_RTR_FLAG;

Re: [PATCH] hw/net/can/sja1000: fix bug for single acceptance filer and standard frame

[Bernhard Beschow]

Typo in commit subject: s/filer/filter/

Am 14. Dezember 2023 10:46:23 UTC schrieb Pavel Pisa <pisa@fel.cvut.cz>:
>From: Pavel Pisa <pisa@cmp.felk.cvut.cz>
>
>A CAN sja1000 standard frame filter mask has been computed and applied
>incorrectly for standard frames when single Acceptance Filter Mode
>(MOD_AFM = 1) has been selected. The problem has not been found
>by Linux kernel testing because it uses dual filter mode (MOD_AFM = 0)
>and leaves falters fully open.
>
>The problem has been noticed by Grant Ramsay when testing with Zephyr
>RTOS which uses single filter mode.
>
>Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
>Reported-by: Grant Ramsay <gramsay@enphaseenergy.com>
>---
> hw/net/can/can_sja1000.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
>index 73201f9139..575df7d2f8 100644
>--- a/hw/net/can/can_sja1000.c
>+++ b/hw/net/can/can_sja1000.c
>@@ -108,7 +108,7 @@ void can_sja_single_filter(struct qemu_can_filter *filter,
>         }
> 
>         filter->can_mask = (uint32_t)amr[0] << 3;
>-        filter->can_mask |= (uint32_t)amr[1] << 5;
>+        filter->can_mask |= (uint32_t)amr[1] >> 5;
>         filter->can_mask = ~filter->can_mask & QEMU_CAN_SFF_MASK;
>         if (!(amr[1] & 0x10)) {
>             filter->can_mask |= QEMU_CAN_RTR_FLAG;