[PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[Michal Schmidt]

new_rcp->recipe_bitmap was written to as if it were an aligned bitmap.
It is an 8-byte array, but aligned only to 4.
Use put_unaligned to set its value.

Additionally, values in ice commands are typically in little-endian.
I assume the recipe bitmap should be too, so use the *_le64 conversion.
I don't have a big-endian system with ice to test this.

I tested that the driver does not crash when probing on aarch64 anymore,
which is good enough for me. I don't know if the LAG feature actually
works.

This is what the crash looked like without the fix:
[   17.599009] Unable to handle kernel paging request at virtual address ffff07ff9c6dc004
[   17.599011] Mem abort info:
[   17.599011]   ESR = 0x0000000096000021
[   17.599012]   EC = 0x25: DABT (current EL), IL = 32 bits
[   17.599013]   SET = 0, FnV = 0
[   17.599014]   EA = 0, S1PTW = 0
[   17.599014]   FSC = 0x21: alignment fault
[   17.599015] Data abort info:
[   17.599016]   ISV = 0, ISS = 0x00000021, ISS2 = 0x00000000
[   17.599016]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[   17.599017]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[   17.599019] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000080dd6bd0000
[   17.599020] [ffff07ff9c6dc004] pgd=1800083fffacf003, p4d=1800083fffacf003, pud=1800083ffface003, pmd=1800083fff9ea003, pte=006808001c6dcf07
[   17.599025] Internal error: Oops: 0000000096000021 [#1] SMP
[   17.599027] Modules linked in: crct10dif_ce ghash_ce sha2_ce sha256_arm64 mlx5_core sha1_ce sbsa_gwdt ice(+) nvme nvme_core mlxfw igb tls nvme_common psample i2c_algo_bit gnss pci_hyperv_intf i2c_designware_platform i2c_designware_core xgene_hwmon dm_mirror dm_region_hash dm_log dm_mod
[   17.599043] CPU: 0 PID: 18 Comm: kworker/0:1 Not tainted 5.14.0-407.el9.aarch64 #1
[   17.599044] Hardware name: GIGABYTE R272-P31-00/MP32-AR1-00, BIOS F31L (SCP: 2.10.20220531) 09/29/2022
[   17.599046] Workqueue: events work_for_cpu_fn
[   17.599051] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   17.599053] pc : ice_create_lag_recipe.constprop.0+0xbc/0x11c [ice]
[   17.599091] lr : ice_create_lag_recipe.constprop.0+0x54/0x11c [ice]
[   17.599121] sp : ffff8000084a3c50
[   17.599122] x29: ffff8000084a3c50 x28: ffffabc4a6790f00 x27: ffffabc4a6200fa0
[   17.599124] x26: ffff07ff809e5c34 x25: ffff083e5f41980d x24: ffff07ff8610a0c0
[   17.599126] x23: 0000000000000000 x22: ffff07ff9fe894c0 x21: ffff07ffb771a460
[   17.599128] x20: ffff07ff9c6dc000 x19: 0000000000000000 x18: 0000000000000014
[   17.599130] x17: 00000000c3142fa2 x16: 000000007e77e163 x15: 0000000018c66856
[   17.599132] x14: 00000000b8afd426 x13: 000000007e8b3b19 x12: 000000004a34fdf7
[   17.599134] x11: 00000000a7cb2fcc x10: 00000000ffffff8a x9 : 0000000000000000
[   17.599136] x8 : 0000002000000005 x7 : 0000000000000001 x6 : ffffabc487a054d8
[   17.599138] x5 : ffff07ff9c6dc004 x4 : 000000000000000a x3 : 0000000000000000
[   17.599140] x2 : 0000000000000000 x1 : 0000000000000400 x0 : ffff07ff9c6dc004
[   17.599142] Call trace:
[   17.599143]  ice_create_lag_recipe.constprop.0+0xbc/0x11c [ice]
[   17.599172]  ice_init_lag+0xcc/0x22c [ice]
[   17.599201]  ice_init_features+0x160/0x2b4 [ice]
[   17.599230]  ice_probe+0x2d0/0x30c [ice]
[   17.599258]  local_pci_probe+0x58/0xb0
[   17.599262]  work_for_cpu_fn+0x20/0x30
[   17.599264]  process_one_work+0x1e4/0x4c0
[   17.599266]  worker_thread+0x220/0x450
[   17.599268]  kthread+0xe8/0xf4
[   17.599270]  ret_from_fork+0x10/0x20
[   17.599273] Code: 380044a4 f800429f 8b000ca0 d503201f (f821301f)
[   17.599274] ---[ end trace 168d79e2ecf9f7e3 ]---
[   17.599275] Kernel panic - not syncing: Oops: Fatal exception
[   17.893321] SMP: stopping secondary CPUs
[   17.897374] Kernel Offset: 0x2bc49c400000 from 0xffff800008000000
[   17.903453] PHYS_OFFSET: 0x80000000
[   17.906928] CPU features: 0x0,00000001,70028143,1041720b
[   17.912226] Memory Limit: none
[   17.915268] ---[ end Kernel panic - not syncing: Oops: Fatal exception ]---

Fixes: 1e0f9881ef79 ("ice: Flesh out implementation of support for SRIOV on bonded interface")
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
 drivers/net/ethernet/intel/ice/ice_lag.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c
index 2a25323105e5..d4848f6fe919 100644
--- a/drivers/net/ethernet/intel/ice/ice_lag.c
+++ b/drivers/net/ethernet/intel/ice/ice_lag.c
@@ -1829,9 +1829,7 @@ static int ice_create_lag_recipe(struct ice_hw *hw, u16 *rid,
 	new_rcp->content.act_ctrl_fwd_priority = prio;
 	new_rcp->content.rid = *rid | ICE_AQ_RECIPE_ID_IS_ROOT;
 	new_rcp->recipe_indx = *rid;
-	bitmap_zero((unsigned long *)new_rcp->recipe_bitmap,
-		    ICE_MAX_NUM_RECIPES);
-	set_bit(*rid, (unsigned long *)new_rcp->recipe_bitmap);
+	put_unaligned_le64(BIT_ULL(*rid), new_rcp->recipe_bitmap);
 
 	err = ice_aq_add_recipe(hw, new_rcp, 1, NULL);
 	if (err)
-- 
2.43.0

Re: [PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[Jiri Pirko]

Wed, Jan 31, 2024 at 12:58:23PM CET, mschmidt@redhat.com wrote:
>new_rcp->recipe_bitmap was written to as if it were an aligned bitmap.
>It is an 8-byte array, but aligned only to 4.
>Use put_unaligned to set its value.
>
>Additionally, values in ice commands are typically in little-endian.
>I assume the recipe bitmap should be too, so use the *_le64 conversion.
>I don't have a big-endian system with ice to test this.
>
>I tested that the driver does not crash when probing on aarch64 anymore,
>which is good enough for me. I don't know if the LAG feature actually
>works.
>
>This is what the crash looked like without the fix:
>[   17.599009] Unable to handle kernel paging request at virtual address ffff07ff9c6dc004
>[   17.599011] Mem abort info:
>[   17.599011]   ESR = 0x0000000096000021
>[   17.599012]   EC = 0x25: DABT (current EL), IL = 32 bits
>[   17.599013]   SET = 0, FnV = 0
>[   17.599014]   EA = 0, S1PTW = 0
>[   17.599014]   FSC = 0x21: alignment fault
>[   17.599015] Data abort info:
>[   17.599016]   ISV = 0, ISS = 0x00000021, ISS2 = 0x00000000
>[   17.599016]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>[   17.599017]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>[   17.599019] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000080dd6bd0000
>[   17.599020] [ffff07ff9c6dc004] pgd=1800083fffacf003, p4d=1800083fffacf003, pud=1800083ffface003, pmd=1800083fff9ea003, pte=006808001c6dcf07
>[   17.599025] Internal error: Oops: 0000000096000021 [#1] SMP
>[   17.599027] Modules linked in: crct10dif_ce ghash_ce sha2_ce sha256_arm64 mlx5_core sha1_ce sbsa_gwdt ice(+) nvme nvme_core mlxfw igb tls nvme_common psample i2c_algo_bit gnss pci_hyperv_intf i2c_designware_platform i2c_designware_core xgene_hwmon dm_mirror dm_region_hash dm_log dm_mod
>[   17.599043] CPU: 0 PID: 18 Comm: kworker/0:1 Not tainted 5.14.0-407.el9.aarch64 #1
>[   17.599044] Hardware name: GIGABYTE R272-P31-00/MP32-AR1-00, BIOS F31L (SCP: 2.10.20220531) 09/29/2022
>[   17.599046] Workqueue: events work_for_cpu_fn
>[   17.599051] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>[   17.599053] pc : ice_create_lag_recipe.constprop.0+0xbc/0x11c [ice]
>[   17.599091] lr : ice_create_lag_recipe.constprop.0+0x54/0x11c [ice]
>[   17.599121] sp : ffff8000084a3c50
>[   17.599122] x29: ffff8000084a3c50 x28: ffffabc4a6790f00 x27: ffffabc4a6200fa0
>[   17.599124] x26: ffff07ff809e5c34 x25: ffff083e5f41980d x24: ffff07ff8610a0c0
>[   17.599126] x23: 0000000000000000 x22: ffff07ff9fe894c0 x21: ffff07ffb771a460
>[   17.599128] x20: ffff07ff9c6dc000 x19: 0000000000000000 x18: 0000000000000014
>[   17.599130] x17: 00000000c3142fa2 x16: 000000007e77e163 x15: 0000000018c66856
>[   17.599132] x14: 00000000b8afd426 x13: 000000007e8b3b19 x12: 000000004a34fdf7
>[   17.599134] x11: 00000000a7cb2fcc x10: 00000000ffffff8a x9 : 0000000000000000
>[   17.599136] x8 : 0000002000000005 x7 : 0000000000000001 x6 : ffffabc487a054d8
>[   17.599138] x5 : ffff07ff9c6dc004 x4 : 000000000000000a x3 : 0000000000000000
>[   17.599140] x2 : 0000000000000000 x1 : 0000000000000400 x0 : ffff07ff9c6dc004
>[   17.599142] Call trace:
>[   17.599143]  ice_create_lag_recipe.constprop.0+0xbc/0x11c [ice]
>[   17.599172]  ice_init_lag+0xcc/0x22c [ice]
>[   17.599201]  ice_init_features+0x160/0x2b4 [ice]
>[   17.599230]  ice_probe+0x2d0/0x30c [ice]
>[   17.599258]  local_pci_probe+0x58/0xb0
>[   17.599262]  work_for_cpu_fn+0x20/0x30
>[   17.599264]  process_one_work+0x1e4/0x4c0
>[   17.599266]  worker_thread+0x220/0x450
>[   17.599268]  kthread+0xe8/0xf4
>[   17.599270]  ret_from_fork+0x10/0x20
>[   17.599273] Code: 380044a4 f800429f 8b000ca0 d503201f (f821301f)
>[   17.599274] ---[ end trace 168d79e2ecf9f7e3 ]---
>[   17.599275] Kernel panic - not syncing: Oops: Fatal exception
>[   17.893321] SMP: stopping secondary CPUs
>[   17.897374] Kernel Offset: 0x2bc49c400000 from 0xffff800008000000
>[   17.903453] PHYS_OFFSET: 0x80000000
>[   17.906928] CPU features: 0x0,00000001,70028143,1041720b
>[   17.912226] Memory Limit: none
>[   17.915268] ---[ end Kernel panic - not syncing: Oops: Fatal exception ]---
>
>Fixes: 1e0f9881ef79 ("ice: Flesh out implementation of support for SRIOV on bonded interface")
>Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
>---
> drivers/net/ethernet/intel/ice/ice_lag.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
>diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c
>index 2a25323105e5..d4848f6fe919 100644
>--- a/drivers/net/ethernet/intel/ice/ice_lag.c
>+++ b/drivers/net/ethernet/intel/ice/ice_lag.c
>@@ -1829,9 +1829,7 @@ static int ice_create_lag_recipe(struct ice_hw *hw, u16 *rid,
> 	new_rcp->content.act_ctrl_fwd_priority = prio;
> 	new_rcp->content.rid = *rid | ICE_AQ_RECIPE_ID_IS_ROOT;
> 	new_rcp->recipe_indx = *rid;
>-	bitmap_zero((unsigned long *)new_rcp->recipe_bitmap,
>-		    ICE_MAX_NUM_RECIPES);
>-	set_bit(*rid, (unsigned long *)new_rcp->recipe_bitmap);
>+	put_unaligned_le64(BIT_ULL(*rid), new_rcp->recipe_bitmap);

Looks like there might be another incorrect bitmap usage for this in
ice_add_sw_recipe(). Care to fix it there as well?

Otherwise, the patch looks fine.


> 
> 	err = ice_aq_add_recipe(hw, new_rcp, 1, NULL);
> 	if (err)
>-- 
>2.43.0
>
>

Re: [Intel-wired-lan] [PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[Alexander Lobakin]

From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 31 Jan 2024 13:17:44 +0100

> Wed, Jan 31, 2024 at 12:58:23PM CET, mschmidt@redhat.com wrote:
>> new_rcp->recipe_bitmap was written to as if it were an aligned bitmap.
>> It is an 8-byte array, but aligned only to 4.
>> Use put_unaligned to set its value.
>>
>> Additionally, values in ice commands are typically in little-endian.
>> I assume the recipe bitmap should be too, so use the *_le64 conversion.
>> I don't have a big-endian system with ice to test this.
>>
>> I tested that the driver does not crash when probing on aarch64 anymore,
>> which is good enough for me. I don't know if the LAG feature actually
>> works.
>>
>> This is what the crash looked like without the fix:
>> [   17.599009] Unable to handle kernel paging request at virtual address ffff07ff9c6dc004
>> [   17.599011] Mem abort info:
>> [   17.599011]   ESR = 0x0000000096000021
>> [   17.599012]   EC = 0x25: DABT (current EL), IL = 32 bits
>> [   17.599013]   SET = 0, FnV = 0
>> [   17.599014]   EA = 0, S1PTW = 0
>> [   17.599014]   FSC = 0x21: alignment fault
>> [   17.599015] Data abort info:
>> [   17.599016]   ISV = 0, ISS = 0x00000021, ISS2 = 0x00000000
>> [   17.599016]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>> [   17.599017]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>> [   17.599019] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000080dd6bd0000
>> [   17.599020] [ffff07ff9c6dc004] pgd=1800083fffacf003, p4d=1800083fffacf003, pud=1800083ffface003, pmd=1800083fff9ea003, pte=006808001c6dcf07
>> [   17.599025] Internal error: Oops: 0000000096000021 [#1] SMP
>> [   17.599027] Modules linked in: crct10dif_ce ghash_ce sha2_ce sha256_arm64 mlx5_core sha1_ce sbsa_gwdt ice(+) nvme nvme_core mlxfw igb tls nvme_common psample i2c_algo_bit gnss pci_hyperv_intf i2c_designware_platform i2c_designware_core xgene_hwmon dm_mirror dm_region_hash dm_log dm_mod
>> [   17.599043] CPU: 0 PID: 18 Comm: kworker/0:1 Not tainted 5.14.0-407.el9.aarch64 #1
>> [   17.599044] Hardware name: GIGABYTE R272-P31-00/MP32-AR1-00, BIOS F31L (SCP: 2.10.20220531) 09/29/2022
>> [   17.599046] Workqueue: events work_for_cpu_fn
>> [   17.599051] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> [   17.599053] pc : ice_create_lag_recipe.constprop.0+0xbc/0x11c [ice]
>> [   17.599091] lr : ice_create_lag_recipe.constprop.0+0x54/0x11c [ice]
>> [   17.599121] sp : ffff8000084a3c50
>> [   17.599122] x29: ffff8000084a3c50 x28: ffffabc4a6790f00 x27: ffffabc4a6200fa0
>> [   17.599124] x26: ffff07ff809e5c34 x25: ffff083e5f41980d x24: ffff07ff8610a0c0
>> [   17.599126] x23: 0000000000000000 x22: ffff07ff9fe894c0 x21: ffff07ffb771a460
>> [   17.599128] x20: ffff07ff9c6dc000 x19: 0000000000000000 x18: 0000000000000014
>> [   17.599130] x17: 00000000c3142fa2 x16: 000000007e77e163 x15: 0000000018c66856
>> [   17.599132] x14: 00000000b8afd426 x13: 000000007e8b3b19 x12: 000000004a34fdf7
>> [   17.599134] x11: 00000000a7cb2fcc x10: 00000000ffffff8a x9 : 0000000000000000
>> [   17.599136] x8 : 0000002000000005 x7 : 0000000000000001 x6 : ffffabc487a054d8
>> [   17.599138] x5 : ffff07ff9c6dc004 x4 : 000000000000000a x3 : 0000000000000000
>> [   17.599140] x2 : 0000000000000000 x1 : 0000000000000400 x0 : ffff07ff9c6dc004
>> [   17.599142] Call trace:
>> [   17.599143]  ice_create_lag_recipe.constprop.0+0xbc/0x11c [ice]
>> [   17.599172]  ice_init_lag+0xcc/0x22c [ice]
>> [   17.599201]  ice_init_features+0x160/0x2b4 [ice]
>> [   17.599230]  ice_probe+0x2d0/0x30c [ice]
>> [   17.599258]  local_pci_probe+0x58/0xb0
>> [   17.599262]  work_for_cpu_fn+0x20/0x30
>> [   17.599264]  process_one_work+0x1e4/0x4c0
>> [   17.599266]  worker_thread+0x220/0x450
>> [   17.599268]  kthread+0xe8/0xf4
>> [   17.599270]  ret_from_fork+0x10/0x20
>> [   17.599273] Code: 380044a4 f800429f 8b000ca0 d503201f (f821301f)
>> [   17.599274] ---[ end trace 168d79e2ecf9f7e3 ]---
>> [   17.599275] Kernel panic - not syncing: Oops: Fatal exception
>> [   17.893321] SMP: stopping secondary CPUs
>> [   17.897374] Kernel Offset: 0x2bc49c400000 from 0xffff800008000000
>> [   17.903453] PHYS_OFFSET: 0x80000000
>> [   17.906928] CPU features: 0x0,00000001,70028143,1041720b
>> [   17.912226] Memory Limit: none
>> [   17.915268] ---[ end Kernel panic - not syncing: Oops: Fatal exception ]---
>>
>> Fixes: 1e0f9881ef79 ("ice: Flesh out implementation of support for SRIOV on bonded interface")
>> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
>> ---
>> drivers/net/ethernet/intel/ice/ice_lag.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c
>> index 2a25323105e5..d4848f6fe919 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
>> @@ -1829,9 +1829,7 @@ static int ice_create_lag_recipe(struct ice_hw *hw, u16 *rid,
>> 	new_rcp->content.act_ctrl_fwd_priority = prio;
>> 	new_rcp->content.rid = *rid | ICE_AQ_RECIPE_ID_IS_ROOT;
>> 	new_rcp->recipe_indx = *rid;
>> -	bitmap_zero((unsigned long *)new_rcp->recipe_bitmap,
>> -		    ICE_MAX_NUM_RECIPES);
>> -	set_bit(*rid, (unsigned long *)new_rcp->recipe_bitmap);
>> +	put_unaligned_le64(BIT_ULL(*rid), new_rcp->recipe_bitmap);
> 
> Looks like there might be another incorrect bitmap usage for this in
> ice_add_sw_recipe(). Care to fix it there as well?

Those are already fixed in one switchdev series and will be sent to IWL
soon.
I believe this patch would also make no sense after it's sent.

> 
> Otherwise, the patch looks fine.
> 
> 
>>
>> 	err = ice_aq_add_recipe(hw, new_rcp, 1, NULL);
>> 	if (err)
>> -- 
>> 2.43.0

Thanks,
Olek

Re: [Intel-wired-lan] [PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[Michal Schmidt]

On 1/31/24 17:59, Alexander Lobakin wrote:
> From: Jiri Pirko <jiri@resnulli.us>
> Date: Wed, 31 Jan 2024 13:17:44 +0100
> 
>> Wed, Jan 31, 2024 at 12:58:23PM CET, mschmidt@redhat.com wrote:
>>> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c
>>> index 2a25323105e5..d4848f6fe919 100644
>>> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
>>> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
>>> @@ -1829,9 +1829,7 @@ static int ice_create_lag_recipe(struct ice_hw *hw, u16 *rid,
>>> 	new_rcp->content.act_ctrl_fwd_priority = prio;
>>> 	new_rcp->content.rid = *rid | ICE_AQ_RECIPE_ID_IS_ROOT;
>>> 	new_rcp->recipe_indx = *rid;
>>> -	bitmap_zero((unsigned long *)new_rcp->recipe_bitmap,
>>> -		    ICE_MAX_NUM_RECIPES);
>>> -	set_bit(*rid, (unsigned long *)new_rcp->recipe_bitmap);
>>> +	put_unaligned_le64(BIT_ULL(*rid), new_rcp->recipe_bitmap);
>>
>> Looks like there might be another incorrect bitmap usage for this in
>> ice_add_sw_recipe(). Care to fix it there as well?
> 
> Those are already fixed in one switchdev series and will be sent to IWL
> soon.
> I believe this patch would also make no sense after it's sent.

Hi Alexander,
When will the series be sent?
The bug causes a kernel panic. Will the series target net.git?
Michal

Re: [Intel-wired-lan] [PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[Alexander Lobakin]

From: Michal Schmidt <mschmidt@redhat.com>
Date: Thu, 1 Feb 2024 19:40:17 +0100

> On 1/31/24 17:59, Alexander Lobakin wrote:
>> From: Jiri Pirko <jiri@resnulli.us>
>> Date: Wed, 31 Jan 2024 13:17:44 +0100
>>
>>> Wed, Jan 31, 2024 at 12:58:23PM CET, mschmidt@redhat.com wrote:
>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c
>>>> b/drivers/net/ethernet/intel/ice/ice_lag.c
>>>> index 2a25323105e5..d4848f6fe919 100644
>>>> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
>>>> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
>>>> @@ -1829,9 +1829,7 @@ static int ice_create_lag_recipe(struct ice_hw
>>>> *hw, u16 *rid,
>>>>     new_rcp->content.act_ctrl_fwd_priority = prio;
>>>>     new_rcp->content.rid = *rid | ICE_AQ_RECIPE_ID_IS_ROOT;
>>>>     new_rcp->recipe_indx = *rid;
>>>> -    bitmap_zero((unsigned long *)new_rcp->recipe_bitmap,
>>>> -            ICE_MAX_NUM_RECIPES);
>>>> -    set_bit(*rid, (unsigned long *)new_rcp->recipe_bitmap);
>>>> +    put_unaligned_le64(BIT_ULL(*rid), new_rcp->recipe_bitmap);
>>>
>>> Looks like there might be another incorrect bitmap usage for this in
>>> ice_add_sw_recipe(). Care to fix it there as well?
>>
>> Those are already fixed in one switchdev series and will be sent to IWL
>> soon.
>> I believe this patch would also make no sense after it's sent.
> 
> Hi Alexander,
> When will the series be sent?
> The bug causes a kernel panic. Will the series target net.git?

The global fix is here: [0]
It's targeting net-next.

I don't know what the best way here would be. Target net instead or pick
your fix to net and then fix it properly in net-next?

> Michal

Thanks,
Olek

Re: [Intel-wired-lan] [PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[Alexander Lobakin]

From: Alexander Lobakin <aleksander.lobakin@intel.com>
Date: Fri, 2 Feb 2024 13:39:28 +0100

> From: Michal Schmidt <mschmidt@redhat.com>
> Date: Thu, 1 Feb 2024 19:40:17 +0100
> 
>> On 1/31/24 17:59, Alexander Lobakin wrote:
>>> From: Jiri Pirko <jiri@resnulli.us>
>>> Date: Wed, 31 Jan 2024 13:17:44 +0100
>>>
>>>> Wed, Jan 31, 2024 at 12:58:23PM CET, mschmidt@redhat.com wrote:
>>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>> b/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>> index 2a25323105e5..d4848f6fe919 100644
>>>>> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>> @@ -1829,9 +1829,7 @@ static int ice_create_lag_recipe(struct ice_hw
>>>>> *hw, u16 *rid,
>>>>>     new_rcp->content.act_ctrl_fwd_priority = prio;
>>>>>     new_rcp->content.rid = *rid | ICE_AQ_RECIPE_ID_IS_ROOT;
>>>>>     new_rcp->recipe_indx = *rid;
>>>>> -    bitmap_zero((unsigned long *)new_rcp->recipe_bitmap,
>>>>> -            ICE_MAX_NUM_RECIPES);
>>>>> -    set_bit(*rid, (unsigned long *)new_rcp->recipe_bitmap);
>>>>> +    put_unaligned_le64(BIT_ULL(*rid), new_rcp->recipe_bitmap);
>>>>
>>>> Looks like there might be another incorrect bitmap usage for this in
>>>> ice_add_sw_recipe(). Care to fix it there as well?
>>>
>>> Those are already fixed in one switchdev series and will be sent to IWL
>>> soon.
>>> I believe this patch would also make no sense after it's sent.
>>
>> Hi Alexander,
>> When will the series be sent?
>> The bug causes a kernel panic. Will the series target net.git?
> 
> The global fix is here: [0]
> It's targeting net-next.
> 
> I don't know what the best way here would be. Target net instead or pick
> your fix to net and then fix it properly in net-next?

Sorry, forgot to paste the link :clownface:

[0]
https://lore.kernel.org/intel-wired-lan/20240130025146.30265-2-steven.zou@intel.com

Thanks,
Olek

Re: [Intel-wired-lan] [PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[Jiri Pirko]

Fri, Feb 02, 2024 at 01:40:18PM CET, aleksander.lobakin@intel.com wrote:
>From: Alexander Lobakin <aleksander.lobakin@intel.com>
>Date: Fri, 2 Feb 2024 13:39:28 +0100
>
>> From: Michal Schmidt <mschmidt@redhat.com>
>> Date: Thu, 1 Feb 2024 19:40:17 +0100
>> 
>>> On 1/31/24 17:59, Alexander Lobakin wrote:
>>>> From: Jiri Pirko <jiri@resnulli.us>
>>>> Date: Wed, 31 Jan 2024 13:17:44 +0100
>>>>
>>>>> Wed, Jan 31, 2024 at 12:58:23PM CET, mschmidt@redhat.com wrote:
>>>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>>> b/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>>> index 2a25323105e5..d4848f6fe919 100644
>>>>>> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>>> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>>> @@ -1829,9 +1829,7 @@ static int ice_create_lag_recipe(struct ice_hw
>>>>>> *hw, u16 *rid,
>>>>>>     new_rcp->content.act_ctrl_fwd_priority = prio;
>>>>>>     new_rcp->content.rid = *rid | ICE_AQ_RECIPE_ID_IS_ROOT;
>>>>>>     new_rcp->recipe_indx = *rid;
>>>>>> -    bitmap_zero((unsigned long *)new_rcp->recipe_bitmap,
>>>>>> -            ICE_MAX_NUM_RECIPES);
>>>>>> -    set_bit(*rid, (unsigned long *)new_rcp->recipe_bitmap);
>>>>>> +    put_unaligned_le64(BIT_ULL(*rid), new_rcp->recipe_bitmap);
>>>>>
>>>>> Looks like there might be another incorrect bitmap usage for this in
>>>>> ice_add_sw_recipe(). Care to fix it there as well?
>>>>
>>>> Those are already fixed in one switchdev series and will be sent to IWL
>>>> soon.
>>>> I believe this patch would also make no sense after it's sent.
>>>
>>> Hi Alexander,
>>> When will the series be sent?
>>> The bug causes a kernel panic. Will the series target net.git?
>> 
>> The global fix is here: [0]
>> It's targeting net-next.
>> 
>> I don't know what the best way here would be. Target net instead or pick
>> your fix to net and then fix it properly in net-next?
>
>Sorry, forgot to paste the link :clownface:
>
>[0]
>https://lore.kernel.org/intel-wired-lan/20240130025146.30265-2-steven.zou@intel.com

Just put this into -net, no? I don't see why not.

>
>Thanks,
>Olek

Re: [Intel-wired-lan] [PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[Maciej Fijalkowski]

On Fri, Feb 02, 2024 at 01:40:18PM +0100, Alexander Lobakin wrote:
> From: Alexander Lobakin <aleksander.lobakin@intel.com>
> Date: Fri, 2 Feb 2024 13:39:28 +0100
> 
> > From: Michal Schmidt <mschmidt@redhat.com>
> > Date: Thu, 1 Feb 2024 19:40:17 +0100
> > 
> >> On 1/31/24 17:59, Alexander Lobakin wrote:
> >>> From: Jiri Pirko <jiri@resnulli.us>
> >>> Date: Wed, 31 Jan 2024 13:17:44 +0100
> >>>
> >>>> Wed, Jan 31, 2024 at 12:58:23PM CET, mschmidt@redhat.com wrote:
> >>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c
> >>>>> b/drivers/net/ethernet/intel/ice/ice_lag.c
> >>>>> index 2a25323105e5..d4848f6fe919 100644
> >>>>> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
> >>>>> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
> >>>>> @@ -1829,9 +1829,7 @@ static int ice_create_lag_recipe(struct ice_hw
> >>>>> *hw, u16 *rid,
> >>>>>     new_rcp->content.act_ctrl_fwd_priority = prio;
> >>>>>     new_rcp->content.rid = *rid | ICE_AQ_RECIPE_ID_IS_ROOT;
> >>>>>     new_rcp->recipe_indx = *rid;
> >>>>> -    bitmap_zero((unsigned long *)new_rcp->recipe_bitmap,
> >>>>> -            ICE_MAX_NUM_RECIPES);
> >>>>> -    set_bit(*rid, (unsigned long *)new_rcp->recipe_bitmap);
> >>>>> +    put_unaligned_le64(BIT_ULL(*rid), new_rcp->recipe_bitmap);
> >>>>
> >>>> Looks like there might be another incorrect bitmap usage for this in
> >>>> ice_add_sw_recipe(). Care to fix it there as well?
> >>>
> >>> Those are already fixed in one switchdev series and will be sent to IWL
> >>> soon.
> >>> I believe this patch would also make no sense after it's sent.
> >>
> >> Hi Alexander,
> >> When will the series be sent?
> >> The bug causes a kernel panic. Will the series target net.git?
> > 
> > The global fix is here: [0]
> > It's targeting net-next.
> > 
> > I don't know what the best way here would be. Target net instead or pick
> > your fix to net and then fix it properly in net-next?
> 
> Sorry, forgot to paste the link :clownface:

IMHO 1/2 should go to net. Then you would have to wait for it to got
accepted and get merged to -next and then you come back with 2/2. You know
the deal.

> 
> [0]
> https://lore.kernel.org/intel-wired-lan/20240130025146.30265-2-steven.zou@intel.com
> 
> Thanks,
> Olek
> 

Re: [Intel-wired-lan] [PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[Alexander Lobakin]

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Date: Fri, 2 Feb 2024 14:00:03 +0100

> On Fri, Feb 02, 2024 at 01:40:18PM +0100, Alexander Lobakin wrote:
>> From: Alexander Lobakin <aleksander.lobakin@intel.com>
>> Date: Fri, 2 Feb 2024 13:39:28 +0100
>>
>>> From: Michal Schmidt <mschmidt@redhat.com>
>>> Date: Thu, 1 Feb 2024 19:40:17 +0100
>>>
>>>> On 1/31/24 17:59, Alexander Lobakin wrote:
>>>>> From: Jiri Pirko <jiri@resnulli.us>
>>>>> Date: Wed, 31 Jan 2024 13:17:44 +0100
>>>>>
>>>>>> Wed, Jan 31, 2024 at 12:58:23PM CET, mschmidt@redhat.com wrote:
>>>>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>>>> b/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>>>> index 2a25323105e5..d4848f6fe919 100644
>>>>>>> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>>>> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>>>> @@ -1829,9 +1829,7 @@ static int ice_create_lag_recipe(struct ice_hw
>>>>>>> *hw, u16 *rid,
>>>>>>>     new_rcp->content.act_ctrl_fwd_priority = prio;
>>>>>>>     new_rcp->content.rid = *rid | ICE_AQ_RECIPE_ID_IS_ROOT;
>>>>>>>     new_rcp->recipe_indx = *rid;
>>>>>>> -    bitmap_zero((unsigned long *)new_rcp->recipe_bitmap,
>>>>>>> -            ICE_MAX_NUM_RECIPES);
>>>>>>> -    set_bit(*rid, (unsigned long *)new_rcp->recipe_bitmap);
>>>>>>> +    put_unaligned_le64(BIT_ULL(*rid), new_rcp->recipe_bitmap);
>>>>>>
>>>>>> Looks like there might be another incorrect bitmap usage for this in
>>>>>> ice_add_sw_recipe(). Care to fix it there as well?
>>>>>
>>>>> Those are already fixed in one switchdev series and will be sent to IWL
>>>>> soon.
>>>>> I believe this patch would also make no sense after it's sent.
>>>>
>>>> Hi Alexander,
>>>> When will the series be sent?
>>>> The bug causes a kernel panic. Will the series target net.git?
>>>
>>> The global fix is here: [0]
>>> It's targeting net-next.
>>>
>>> I don't know what the best way here would be. Target net instead or pick
>>> your fix to net and then fix it properly in net-next?
>>
>> Sorry, forgot to paste the link :clownface:
> 
> IMHO 1/2 should go to net. Then you would have to wait for it to got
> accepted and get merged to -next and then you come back with 2/2. You know
> the deal.

Agree!

Hi Steve,

Could you please send the first patch from your series to net instead of
net-next?

(and add "Fixes:" tag with the blamed commit)

> 
>>
>> [0]
>> https://lore.kernel.org/intel-wired-lan/20240130025146.30265-2-steven.zou@intel.com
>>
>> Thanks,
>> Olek

Thanks,
Olek

Re: [Intel-wired-lan] [PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[Zou, Steven]

On 2/2/2024 9:01 PM, Alexander Lobakin wrote:
> From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Date: Fri, 2 Feb 2024 14:00:03 +0100
>
>> On Fri, Feb 02, 2024 at 01:40:18PM +0100, Alexander Lobakin wrote:
>>> From: Alexander Lobakin <aleksander.lobakin@intel.com>
>>> Date: Fri, 2 Feb 2024 13:39:28 +0100
>>>
>>>> From: Michal Schmidt <mschmidt@redhat.com>
>>>> Date: Thu, 1 Feb 2024 19:40:17 +0100
>>>>
>>>>> On 1/31/24 17:59, Alexander Lobakin wrote:
>>>>>> From: Jiri Pirko <jiri@resnulli.us>
>>>>>> Date: Wed, 31 Jan 2024 13:17:44 +0100
>>>>>>
>>>>>>> Wed, Jan 31, 2024 at 12:58:23PM CET, mschmidt@redhat.com wrote:
>>>>>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>>>>> b/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>>>>> index 2a25323105e5..d4848f6fe919 100644
>>>>>>>> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>>>>> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>>>>> @@ -1829,9 +1829,7 @@ static int ice_create_lag_recipe(struct ice_hw
>>>>>>>> *hw, u16 *rid,
>>>>>>>>      new_rcp->content.act_ctrl_fwd_priority = prio;
>>>>>>>>      new_rcp->content.rid = *rid | ICE_AQ_RECIPE_ID_IS_ROOT;
>>>>>>>>      new_rcp->recipe_indx = *rid;
>>>>>>>> -    bitmap_zero((unsigned long *)new_rcp->recipe_bitmap,
>>>>>>>> -            ICE_MAX_NUM_RECIPES);
>>>>>>>> -    set_bit(*rid, (unsigned long *)new_rcp->recipe_bitmap);
>>>>>>>> +    put_unaligned_le64(BIT_ULL(*rid), new_rcp->recipe_bitmap);
>>>>>>> Looks like there might be another incorrect bitmap usage for this in
>>>>>>> ice_add_sw_recipe(). Care to fix it there as well?
>>>>>> Those are already fixed in one switchdev series and will be sent to IWL
>>>>>> soon.
>>>>>> I believe this patch would also make no sense after it's sent.
>>>>> Hi Alexander,
>>>>> When will the series be sent?
>>>>> The bug causes a kernel panic. Will the series target net.git?
>>>> The global fix is here: [0]
>>>> It's targeting net-next.
>>>>
>>>> I don't know what the best way here would be. Target net instead or pick
>>>> your fix to net and then fix it properly in net-next?
>>> Sorry, forgot to paste the link :clownface:
>> IMHO 1/2 should go to net. Then you would have to wait for it to got
>> accepted and get merged to -next and then you come back with 2/2. You know
>> the deal.
> Agree!
>
> Hi Steve,
>
> Could you please send the first patch from your series to net instead of
> net-next?
>
> (and add "Fixes:" tag with the blamed commit)

Hi Olek,
Sure, I will do it soon.

>
>>> [0]
>>> https://lore.kernel.org/intel-wired-lan/20240130025146.30265-2-steven.zou@intel.com
>>>
>>> Thanks,
>>> Olek
> Thanks,
> Olek
-- 

Best Regards,
Steven

Re: [Intel-wired-lan] [PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[Zou, Steven]

On 2/7/2024 8:44 AM, Zou, Steven wrote:
>
> On 2/2/2024 9:01 PM, Alexander Lobakin wrote:
>> From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>> Date: Fri, 2 Feb 2024 14:00:03 +0100
>>
>>> On Fri, Feb 02, 2024 at 01:40:18PM +0100, Alexander Lobakin wrote:
>>>> From: Alexander Lobakin <aleksander.lobakin@intel.com>
>>>> Date: Fri, 2 Feb 2024 13:39:28 +0100
>>>>
>>>>> From: Michal Schmidt <mschmidt@redhat.com>
>>>>> Date: Thu, 1 Feb 2024 19:40:17 +0100
>>>>>
>>>>>> On 1/31/24 17:59, Alexander Lobakin wrote:
>>>>>>> From: Jiri Pirko <jiri@resnulli.us>
>>>>>>> Date: Wed, 31 Jan 2024 13:17:44 +0100
>>>>>>>
>>>>>>>> Wed, Jan 31, 2024 at 12:58:23PM CET, mschmidt@redhat.com wrote:
>>>>>>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>>>>>> b/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>>>>>> index 2a25323105e5..d4848f6fe919 100644
>>>>>>>>> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>>>>>> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
>>>>>>>>> @@ -1829,9 +1829,7 @@ static int ice_create_lag_recipe(struct 
>>>>>>>>> ice_hw
>>>>>>>>> *hw, u16 *rid,
>>>>>>>>>      new_rcp->content.act_ctrl_fwd_priority = prio;
>>>>>>>>>      new_rcp->content.rid = *rid | ICE_AQ_RECIPE_ID_IS_ROOT;
>>>>>>>>>      new_rcp->recipe_indx = *rid;
>>>>>>>>> -    bitmap_zero((unsigned long *)new_rcp->recipe_bitmap,
>>>>>>>>> -            ICE_MAX_NUM_RECIPES);
>>>>>>>>> -    set_bit(*rid, (unsigned long *)new_rcp->recipe_bitmap);
>>>>>>>>> +    put_unaligned_le64(BIT_ULL(*rid), new_rcp->recipe_bitmap);
>>>>>>>> Looks like there might be another incorrect bitmap usage for 
>>>>>>>> this in
>>>>>>>> ice_add_sw_recipe(). Care to fix it there as well?
>>>>>>> Those are already fixed in one switchdev series and will be sent 
>>>>>>> to IWL
>>>>>>> soon.
>>>>>>> I believe this patch would also make no sense after it's sent.
>>>>>> Hi Alexander,
>>>>>> When will the series be sent?
>>>>>> The bug causes a kernel panic. Will the series target net.git?
>>>>> The global fix is here: [0]
>>>>> It's targeting net-next.
>>>>>
>>>>> I don't know what the best way here would be. Target net instead 
>>>>> or pick
>>>>> your fix to net and then fix it properly in net-next?
>>>> Sorry, forgot to paste the link :clownface:
>>> IMHO 1/2 should go to net. Then you would have to wait for it to got
>>> accepted and get merged to -next and then you come back with 2/2. 
>>> You know
>>> the deal.
>> Agree!
>>
>> Hi Steve,
>>
>> Could you please send the first patch from your series to net instead of
>> net-next?
>>
>> (and add "Fixes:" tag with the blamed commit)
>
> Hi Olek,
> Sure, I will do it soon.

Hi team,
The patch has been rebased and submitted on:
https://lore.kernel.org/intel-wired-lan/20240207014959.24012-1-steven.zou@intel.com/
https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20240207014959.24012-1-steven.zou@intel.com/

Thanks,
Steven

>
>>
>>>> [0]
>>>> https://lore.kernel.org/intel-wired-lan/20240130025146.30265-2-steven.zou@intel.com 
>>>>
>>>>
>>>> Thanks,
>>>> Olek
>> Thanks,
>> Olek

Re: [PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[Hongchen Zhang]

Hi Michal,
On 2024/1/31 pm 7:58, Michal Schmidt wrote:
> new_rcp->recipe_bitmap was written to as if it were an aligned bitmap.
> It is an 8-byte array, but aligned only to 4.
> Use put_unaligned to set its value.
> 
> Additionally, values in ice commands are typically in little-endian.
> I assume the recipe bitmap should be too, so use the *_le64 conversion.
> I don't have a big-endian system with ice to test this.
> 
> I tested that the driver does not crash when probing on aarch64 anymore,
> which is good enough for me. I don't know if the LAG feature actually
> works.
> 
> This is what the crash looked like without the fix:
> [   17.599009] Unable to handle kernel paging request at virtual address ffff07ff9c6dc004
> [   17.599011] Mem abort info:
> [   17.599011]   ESR = 0x0000000096000021
> [   17.599012]   EC = 0x25: DABT (current EL), IL = 32 bits
> [   17.599013]   SET = 0, FnV = 0
> [   17.599014]   EA = 0, S1PTW = 0
> [   17.599014]   FSC = 0x21: alignment fault
> [   17.599015] Data abort info:
> [   17.599016]   ISV = 0, ISS = 0x00000021, ISS2 = 0x00000000
> [   17.599016]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> [   17.599017]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [   17.599019] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000080dd6bd0000
> [   17.599020] [ffff07ff9c6dc004] pgd=1800083fffacf003, p4d=1800083fffacf003, pud=1800083ffface003, pmd=1800083fff9ea003, pte=006808001c6dcf07
> [   17.599025] Internal error: Oops: 0000000096000021 [#1] SMP
> [   17.599027] Modules linked in: crct10dif_ce ghash_ce sha2_ce sha256_arm64 mlx5_core sha1_ce sbsa_gwdt ice(+) nvme nvme_core mlxfw igb tls nvme_common psample i2c_algo_bit gnss pci_hyperv_intf i2c_designware_platform i2c_designware_core xgene_hwmon dm_mirror dm_region_hash dm_log dm_mod
> [   17.599043] CPU: 0 PID: 18 Comm: kworker/0:1 Not tainted 5.14.0-407.el9.aarch64 #1
> [   17.599044] Hardware name: GIGABYTE R272-P31-00/MP32-AR1-00, BIOS F31L (SCP: 2.10.20220531) 09/29/2022
> [   17.599046] Workqueue: events work_for_cpu_fn
> [   17.599051] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [   17.599053] pc : ice_create_lag_recipe.constprop.0+0xbc/0x11c [ice]
> [   17.599091] lr : ice_create_lag_recipe.constprop.0+0x54/0x11c [ice]
> [   17.599121] sp : ffff8000084a3c50
> [   17.599122] x29: ffff8000084a3c50 x28: ffffabc4a6790f00 x27: ffffabc4a6200fa0
> [   17.599124] x26: ffff07ff809e5c34 x25: ffff083e5f41980d x24: ffff07ff8610a0c0
> [   17.599126] x23: 0000000000000000 x22: ffff07ff9fe894c0 x21: ffff07ffb771a460
> [   17.599128] x20: ffff07ff9c6dc000 x19: 0000000000000000 x18: 0000000000000014
> [   17.599130] x17: 00000000c3142fa2 x16: 000000007e77e163 x15: 0000000018c66856
> [   17.599132] x14: 00000000b8afd426 x13: 000000007e8b3b19 x12: 000000004a34fdf7
> [   17.599134] x11: 00000000a7cb2fcc x10: 00000000ffffff8a x9 : 0000000000000000
> [   17.599136] x8 : 0000002000000005 x7 : 0000000000000001 x6 : ffffabc487a054d8
> [   17.599138] x5 : ffff07ff9c6dc004 x4 : 000000000000000a x3 : 0000000000000000
> [   17.599140] x2 : 0000000000000000 x1 : 0000000000000400 x0 : ffff07ff9c6dc004
> [   17.599142] Call trace:
> [   17.599143]  ice_create_lag_recipe.constprop.0+0xbc/0x11c [ice]
> [   17.599172]  ice_init_lag+0xcc/0x22c [ice]
> [   17.599201]  ice_init_features+0x160/0x2b4 [ice]
> [   17.599230]  ice_probe+0x2d0/0x30c [ice]
> [   17.599258]  local_pci_probe+0x58/0xb0
> [   17.599262]  work_for_cpu_fn+0x20/0x30
> [   17.599264]  process_one_work+0x1e4/0x4c0
> [   17.599266]  worker_thread+0x220/0x450
> [   17.599268]  kthread+0xe8/0xf4
> [   17.599270]  ret_from_fork+0x10/0x20
> [   17.599273] Code: 380044a4 f800429f 8b000ca0 d503201f (f821301f)
> [   17.599274] ---[ end trace 168d79e2ecf9f7e3 ]---
> [   17.599275] Kernel panic - not syncing: Oops: Fatal exception
> [   17.893321] SMP: stopping secondary CPUs
> [   17.897374] Kernel Offset: 0x2bc49c400000 from 0xffff800008000000
> [   17.903453] PHYS_OFFSET: 0x80000000
> [   17.906928] CPU features: 0x0,00000001,70028143,1041720b
> [   17.912226] Memory Limit: none
> [   17.915268] ---[ end Kernel panic - not syncing: Oops: Fatal exception ]---
> 
> Fixes: 1e0f9881ef79 ("ice: Flesh out implementation of support for SRIOV on bonded interface")
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_lag.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c
> index 2a25323105e5..d4848f6fe919 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
> @@ -1829,9 +1829,7 @@ static int ice_create_lag_recipe(struct ice_hw *hw, u16 *rid,
>   	new_rcp->content.act_ctrl_fwd_priority = prio;
>   	new_rcp->content.rid = *rid | ICE_AQ_RECIPE_ID_IS_ROOT;
>   	new_rcp->recipe_indx = *rid;
> -	bitmap_zero((unsigned long *)new_rcp->recipe_bitmap,
> -		    ICE_MAX_NUM_RECIPES);
> -	set_bit(*rid, (unsigned long *)new_rcp->recipe_bitmap);
> +	put_unaligned_le64(BIT_ULL(*rid), new_rcp->recipe_bitmap);
>   
>   	err = ice_aq_add_recipe(hw, new_rcp, 1, NULL);
>   	if (err)
> 
I encountered the same problem on a LoongArch LS3C6000 machine. Can this 
patch be merged now?

-- 
Best Regards
Hongchen Zhang

Re: [PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[Przemek Kitszel]

On 1/8/25 04:09, Hongchen Zhang wrote:
> Hi Michal,
> On 2024/1/31 pm 7:58, Michal Schmidt wrote:
>> new_rcp->recipe_bitmap was written to as if it were an aligned bitmap.
>> It is an 8-byte array, but aligned only to 4.
>> Use put_unaligned to set its value.
>>
>> Additionally, values in ice commands are typically in little-endian.
>> I assume the recipe bitmap should be too, so use the *_le64 conversion.
>> I don't have a big-endian system with ice to test this.
>>
>> I tested that the driver does not crash when probing on aarch64 anymore,
>> which is good enough for me. I don't know if the LAG feature actually
>> works.
>>
>> This is what the crash looked like without the fix:

>> [   17.599142] Call trace:
>> [   17.599143]  ice_create_lag_recipe.constprop.0+0xbc/0x11c [ice]
>> [   17.599172]  ice_init_lag+0xcc/0x22c [ice]
>> [   17.599201]  ice_init_features+0x160/0x2b4 [ice]
>> [   17.599230]  ice_probe+0x2d0/0x30c [ice]
>> [   17.599258]  local_pci_probe+0x58/0xb0
>> [   17.599262]  work_for_cpu_fn+0x20/0x30

> I encountered the same problem on a LoongArch LS3C6000 machine. Can this 
> patch be merged now?

What kernel base do you use?, we have merged the Steven Patches long ago

Re: [PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[Hongchen Zhang]

Hi Przemek,
On 2025/1/8 下午4:59, Przemek Kitszel wrote:
> On 1/8/25 04:09, Hongchen Zhang wrote:

>> Hi Michal,
>> On 2024/1/31 pm 7:58, Michal Schmidt wrote:
>>> new_rcp->recipe_bitmap was written to as if it were an aligned bitmap.
>>> It is an 8-byte array, but aligned only to 4.
>>> Use put_unaligned to set its value.
>>>
>>> Additionally, values in ice commands are typically in little-endian.
>>> I assume the recipe bitmap should be too, so use the *_le64 conversion.
>>> I don't have a big-endian system with ice to test this.
>>>
>>> I tested that the driver does not crash when probing on aarch64 anymore,
>>> which is good enough for me. I don't know if the LAG feature actually
>>> works.
>>>
>>> This is what the crash looked like without the fix:
> 
>>> [   17.599142] Call trace:
>>> [   17.599143]  ice_create_lag_recipe.constprop.0+0xbc/0x11c [ice]
>>> [   17.599172]  ice_init_lag+0xcc/0x22c [ice]
>>> [   17.599201]  ice_init_features+0x160/0x2b4 [ice]
>>> [   17.599230]  ice_probe+0x2d0/0x30c [ice]
>>> [   17.599258]  local_pci_probe+0x58/0xb0
>>> [   17.599262]  work_for_cpu_fn+0x20/0x30
> 
>> I encountered the same problem on a LoongArch LS3C6000 machine. Can 
>> this patch be merged now?
> 
> What kernel base do you use?, we have merged the Steven Patches long ago
My test is based on 6.6.61 which contains Steven's patch:
  8ec08ba97fab 2024-05-07  ice: Refactor FW data type and fix bitmap 
casting issue [Steven Zou]

It seems that Steven's patch can not solve the unaligned access problem 
caused by new_rcp->recipe_bitmap, So is Michal's patch (may need some 
change in ice_add_sw_recipe()) still needed?

-- 
Best Regards
Hongchen Zhang

Re: [PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[Przemek Kitszel]

On 1/9/25 02:54, Hongchen Zhang wrote:
> Hi Przemek,
> On 2025/1/8 下午4:59, Przemek Kitszel wrote:
>> On 1/8/25 04:09, Hongchen Zhang wrote:
> 
>>> Hi Michal,
>>> On 2024/1/31 pm 7:58, Michal Schmidt wrote:
>>>> new_rcp->recipe_bitmap was written to as if it were an aligned bitmap.
>>>> It is an 8-byte array, but aligned only to 4.
>>>> Use put_unaligned to set its value.
>>>>
>>>> Additionally, values in ice commands are typically in little-endian.
>>>> I assume the recipe bitmap should be too, so use the *_le64 conversion.
>>>> I don't have a big-endian system with ice to test this.
>>>>
>>>> I tested that the driver does not crash when probing on aarch64 
>>>> anymore,
>>>> which is good enough for me. I don't know if the LAG feature actually
>>>> works.
>>>>
>>>> This is what the crash looked like without the fix:
>>
>>>> [   17.599142] Call trace:
>>>> [   17.599143]  ice_create_lag_recipe.constprop.0+0xbc/0x11c [ice]
>>>> [   17.599172]  ice_init_lag+0xcc/0x22c [ice]
>>>> [   17.599201]  ice_init_features+0x160/0x2b4 [ice]
>>>> [   17.599230]  ice_probe+0x2d0/0x30c [ice]
>>>> [   17.599258]  local_pci_probe+0x58/0xb0
>>>> [   17.599262]  work_for_cpu_fn+0x20/0x30
>>
>>> I encountered the same problem on a LoongArch LS3C6000 machine. Can 
>>> this patch be merged now?
>>
>> What kernel base do you use?, we have merged the Steven Patches long ago
> My test is based on 6.6.61 which contains Steven's patch:
>   8ec08ba97fab 2024-05-07  ice: Refactor FW data type and fix bitmap 
> casting issue [Steven Zou]
> 
> It seems that Steven's patch can not solve the unaligned access problem 
> caused by new_rcp->recipe_bitmap, So is Michal's patch (may need some 
> change in ice_add_sw_recipe()) still needed?
> 

thank you, I see now

I agree that ice_aqc_recipe_data_elem::recipe_bitmap[8] should be
changed to __le64, together with updated accesses. Best way to do so
will be as in Steven's patch.

@Michal, will you be OK with us reimplementing this, or you want to
follow up?

Re: [PATCH net] ice: fix unaligned access in ice_create_lag_recipe

[Alexander Lobakin]

From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Date: Thu, 9 Jan 2025 10:31:35 +0100

> On 1/9/25 02:54, Hongchen Zhang wrote:
>> Hi Przemek,
>> On 2025/1/8 下午4:59, Przemek Kitszel wrote:
>>> On 1/8/25 04:09, Hongchen Zhang wrote:
>>
>>>> Hi Michal,
>>>> On 2024/1/31 pm 7:58, Michal Schmidt wrote:
>>>>> new_rcp->recipe_bitmap was written to as if it were an aligned bitmap.
>>>>> It is an 8-byte array, but aligned only to 4.
>>>>> Use put_unaligned to set its value.
>>>>>
>>>>> Additionally, values in ice commands are typically in little-endian.
>>>>> I assume the recipe bitmap should be too, so use the *_le64
>>>>> conversion.
>>>>> I don't have a big-endian system with ice to test this.
>>>>>
>>>>> I tested that the driver does not crash when probing on aarch64
>>>>> anymore,
>>>>> which is good enough for me. I don't know if the LAG feature actually
>>>>> works.
>>>>>
>>>>> This is what the crash looked like without the fix:
>>>
>>>>> [   17.599142] Call trace:
>>>>> [   17.599143]  ice_create_lag_recipe.constprop.0+0xbc/0x11c [ice]
>>>>> [   17.599172]  ice_init_lag+0xcc/0x22c [ice]
>>>>> [   17.599201]  ice_init_features+0x160/0x2b4 [ice]
>>>>> [   17.599230]  ice_probe+0x2d0/0x30c [ice]
>>>>> [   17.599258]  local_pci_probe+0x58/0xb0
>>>>> [   17.599262]  work_for_cpu_fn+0x20/0x30
>>>
>>>> I encountered the same problem on a LoongArch LS3C6000 machine. Can
>>>> this patch be merged now?
>>>
>>> What kernel base do you use?, we have merged the Steven Patches long ago
>> My test is based on 6.6.61 which contains Steven's patch:
>>   8ec08ba97fab 2024-05-07  ice: Refactor FW data type and fix bitmap
>> casting issue [Steven Zou]
>>
>> It seems that Steven's patch can not solve the unaligned access
>> problem caused by new_rcp->recipe_bitmap, So is Michal's patch (may
>> need some change in ice_add_sw_recipe()) still needed?
>>
> 
> thank you, I see now
> 
> I agree that ice_aqc_recipe_data_elem::recipe_bitmap[8] should be
> changed to __le64, together with updated accesses. Best way to do so

Too bad I didn't notice that in ice_create_lag_recipe(), the cast is
still here. It's not valid to cast 1-byte array to a naturally aligned
one (of unsigned longs).

You can't simply change it to __le64 as 8-byte vars are
naturally-aligned, while here its offset is 4 bytes.

The best solution would be to change it to two __le32s and avoid using
bitmap helpers like set_bit() at all -- just manually assign bits there.
Alternatively, if you want -- you can use __le64, but then pack the
structure by 4 bytes, but I don't think it would give any benefit
comparing to the former.

> will be as in Steven's patch.
> 
> @Michal, will you be OK with us reimplementing this, or you want to
> follow up?

Thanks,
Olek