Re: [PATCH] drm/xe/hwmon: Store mailbox temperature data as bytes

["Poosa, Karthik" <karthik.poosa@intel.com>]

On 28-10-2026 18:36, Mallesh, Koujalagi wrote:
>
> On 28-10-2026 05:06 pm, Poosa, Karthik wrote:
>>
>> On 26-10-2026 17:24, Mallesh, Koujalagi wrote:
>>>
>>> On 26-10-2026 04:47 pm, Poosa, Karthik wrote:
>>>>
>>>> On 23-10-2026 16:06, Mallesh Koujalagi wrote:
>>>>> Store pcode mailbox temperature data directly in the byte array
>>>>> using put_unaligned_le32() instead of accessing it through
>>>>> different views of the same storage.
>>>>>
>>>>> This keeps the layout unchanged and makes the data access
>>>>> consistent and endian-safe.
>>>>
>>>> 1. Is this patch addressing any issue reported by a static analysis 
>>>> tool?
>>>>
>>> Yes, it's reported by static analysis tool.
>>>> 2. Given that |cpu_to_le3()| already handles endianness, do we 
>>>> still need this change?
>>>
>>> Yes, cpu_to_le32() already handles endianness correctly. The main 
>>> reason for this change is not
>>>
>>> to fix an endianness issue, however to address the static analysis 
>>> warning about inconsistent union access.
>>>
>>> The code writes data through temp.dword[] and later reads it through 
>>> temp.value[], which was flagged as
>>>
>>> inconsistent access.
>>>
>>> To avoid that used put_unaligned_le32 writes directly into the byte 
>>> array that is later
>>>
>>> read, making the access pattern consistent.
>> I understand the static-analysis warning about writing through 
>> |dword[]| and reading through |value[]|. However, since |value[]| is 
>> accessed as |u8|, there should be no alignment issue, and 
>> |cpu_to_le32()| already provides the expected byte representation. Is 
>> there any known functional issue with the current code, or is this 
>> change solely to address the static-analysis warning?
>
> Agree, no alignment or endianness issue since we are using 
> cpu_to_le32() which provides the correct byte representation. This is 
> purely a code-hygiene cleanup to silence the static analysis warning 
> via put_unaligned_le32 so I'll drop the Fixes: tag and add no 
> functional change. Please let me know.
>
>
> Thanks,
>
> -/Mallesh
If there is no function issue we can drop this patch and mark issue as 
false positive.
>
>>>
>>>
>>> Thanks,
>>>
>>> -/Mallesh
>>>
>>>>
>>>>>
>>>>> Fixes: ca6e33701de8 ("drm/xe/hwmon: Fix thermal sensor data 
>>>>> handling")
>>>>> Signed-off-by: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
>>>>> ---
>>>>>   drivers/gpu/drm/xe/xe_hwmon.c | 15 ++++++++-------
>>>>>   1 file changed, 8 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c 
>>>>> b/drivers/gpu/drm/xe/xe_hwmon.c
>>>>> index 42acfeb8818c..7bb1223dc30b 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_hwmon.c
>>>>> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
>>>>> @@ -7,6 +7,7 @@
>>>>>   #include <linux/hwmon.h>
>>>>>   #include <linux/jiffies.h>
>>>>>   #include <linux/types.h>
>>>>> +#include <linux/unaligned.h>
>>>>>   #include <linux/units.h>
>>>>>     #include <drm/drm_managed.h>
>>>>> @@ -165,12 +166,12 @@ struct xe_hwmon_thermal_info {
>>>>>       bool available[CHANNEL_MAX];
>>>>>       /** @msu_mask: VRAM subsystem enable mask read from MMIO */
>>>>>       u32 msu_mask;
>>>>> -    union {
>>>>> -        /** @value: per-sensor raw mailbox temperature; 
>>>>> bit7=sign, bits6:0=magnitude */
>>>>> -        u8 value[U8_MAX + 1];
>>>>> -        /** @dword: sensor values as little-endian dwords, 
>>>>> u32-aligned for pcode reads */
>>>>> -        __le32 dword[DIV_ROUND_UP(U8_MAX + 1, sizeof(u32))];
>>>>> -    };
>>>>> +    /**
>>>>> +     * @value: per-sensor raw mailbox temperature; bit7=sign, 
>>>>> bits6:0=magnitude.
>>>>> +     * Written one dword at a time via put_unaligned_le32() to
>>>> Written -> Write
>>>>> +     * avoid inconsistent union access between dword and byte views.
>>>>> +     */
>>>>> +    u8 value[U8_MAX + 1];
>>>>>       /** @vram_label: vram label names, dynamically allocated 
>>>>> based on vram_count */
>>>>>       char (*vram_label)[MAX_LABEL_SIZE];
>>>>>   };
>>>>> @@ -995,7 +996,7 @@ static int get_mc_temp(struct xe_hwmon *hwmon, 
>>>>> long *val)
>>>>>           if (ret)
>>>>>               return ret;
>>>>>           drm_dbg(&hwmon->xe->drm, "thermal data for group %d val 
>>>>> 0x%x\n", i, data);
>>>>> -        hwmon->temp.dword[i] = cpu_to_le32(data);
>>>>> +        put_unaligned_le32(data, &hwmon->temp.value[i * 
>>>>> sizeof(u32)]);
>>>>>       }
>>>>>         for (i = start; i < end; i++) {