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

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

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?