Re: [PATCH] platform/x86: ISST: Correct command storage data length

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Fri, 28 Mar 2025, Srinivas Pandruvada wrote:

> After resume/online turbo limit ratio (TRL) is restored partially if
> the admin explicitly changed TRL from user space.
> 
> A hash table is used to store SST mail box and MSR settings when modified
> to restore those settings after resume or online. This uses a struct
> isst_cmd field "data" to store these settings. This is a 64 bit field.
> But isst_store_new_cmd() is only assigning as u32. This results in
> truncation of 32 bits.
> 
> Change the argument to u64 from u32.
> 
> Fixes: f607874f35cb ("platform/x86: ISST: Restore state on resume")
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: stable@vger.kernel.org

Thanks, I've applied this to the review-ilpo-fixes branch.

While reviewing this, I noticed full_cmd is currently typed as int but I 
think it should be u32 to match the type in the struct.

The construction of full_cmd also open codes FIELD_PREP() and doesn't use 
named defines for the fields. And ->cmd is also decomposed in 
isst_mbox_resume_command() which should use FIELD_GET() and the same 
defines.

> ---
>  drivers/platform/x86/intel/speed_select_if/isst_if_common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
> index dbcd3087aaa4..31239a93dd71 100644
> --- a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
> +++ b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
> @@ -84,7 +84,7 @@ static DECLARE_HASHTABLE(isst_hash, 8);
>  static DEFINE_MUTEX(isst_hash_lock);
>  
>  static int isst_store_new_cmd(int cmd, u32 cpu, int mbox_cmd_type, u32 param,
> -			      u32 data)
> +			      u64 data)
>  {
>  	struct isst_cmd *sst_cmd;

-- 
 i.