Re: [PATCH 01/10] platform/x86/intel/ifs: Store IFS generation number

[Ilpo Järvinen]

On Wed, 13 Sep 2023, Jithu Joseph wrote:

> IFS generation number is reported via MSR_INTEGRITY_CAPS.

Please use more characters per line, the limit is 72 characters.

> As IFS support gets added to newer CPUs, some differences
> are expected during IFS image loading and test flows.
> 
> Define MSR bitmasks to extract and store the generation in
> driver data, so that driver can modify its MSR interaction
> appropriately.
> 
> Signed-off-by: Jithu Joseph <jithu.joseph@xxxxxxxxx>
> Reviewed-by: Tony Luck <tony.luck@xxxxxxxxx>
> Tested-by: Pengfei Xu <pengfei.xu@xxxxxxxxx>
> ---
>  arch/x86/include/asm/msr-index.h      | 2 ++
>  drivers/platform/x86/intel/ifs/ifs.h  | 2 ++
>  drivers/platform/x86/intel/ifs/core.c | 2 ++
>  3 files changed, 6 insertions(+)
> 
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index 1d111350197f..a71a86a01488 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -222,6 +222,8 @@
>  #define MSR_INTEGRITY_CAPS_ARRAY_BIST          BIT(MSR_INTEGRITY_CAPS_ARRAY_BIST_BIT)
>  #define MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT	4
>  #define MSR_INTEGRITY_CAPS_PERIODIC_BIST	BIT(MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT)
> +#define MSR_INTEGRITY_CAPS_SAF_GEN_REV_SHIFT	9
> +#define MSR_INTEGRITY_CAPS_SAF_GEN_REV_MASK	(0x3ull << MSR_INTEGRITY_CAPS_SAF_GEN_REV_SHIFT)

GENMASK_ULL(), don't add _SHIFT at all as FIELD_GET/PREP() will handle 
it for you.

>  #define MSR_LBR_NHM_FROM		0x00000680
>  #define MSR_LBR_NHM_TO			0x000006c0
> diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
> index 93191855890f..d666aeed20fc 100644
> --- a/drivers/platform/x86/intel/ifs/ifs.h
> +++ b/drivers/platform/x86/intel/ifs/ifs.h
> @@ -229,6 +229,7 @@ struct ifs_test_caps {
>   * @status: it holds simple status pass/fail/untested
>   * @scan_details: opaque scan status code from h/w
>   * @cur_batch: number indicating the currently loaded test file
> + * @generation: IFS test generation enumerated by hardware
>   */
>  struct ifs_data {
>  	int	loaded_version;
> @@ -238,6 +239,7 @@ struct ifs_data {
>  	int	status;
>  	u64	scan_details;
>  	u32	cur_batch;
> +	u32	generation;
>  };
>  
>  struct ifs_work {
> diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
> index 306f886b52d2..88d84aad9334 100644
> --- a/drivers/platform/x86/intel/ifs/core.c
> +++ b/drivers/platform/x86/intel/ifs/core.c
> @@ -94,6 +94,8 @@ static int __init ifs_init(void)
>  	for (i = 0; i < IFS_NUMTESTS; i++) {
>  		if (!(msrval & BIT(ifs_devices[i].test_caps->integrity_cap_bit)))
>  			continue;
> +		ifs_devices[i].rw_data.generation = (msrval & MSR_INTEGRITY_CAPS_SAF_GEN_REV_MASK)
> +							>> MSR_INTEGRITY_CAPS_SAF_GEN_REV_SHIFT;

FIELD_GET(), don't forget to make sure use have the include for it.

>  		ret = misc_register(&ifs_devices[i].misc);
>  		if (ret)
>  			goto err_exit;
> 

-- 
 i.