Received-SPF: pass (google.com: domain of linux-kernel+bounces-164801-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Tue, 30 Apr 2024 16:42:25 -0700
From: Ashok Raj <ashok.raj@intel.com>
To: "Chang S. Bae" <chang.seok.bae@intel.com>
CC: <linux-kernel@vger.kernel.org>, <x86@kernel.org>,
	<platform-driver-x86@vger.kernel.org>, <tglx@linutronix.de>,
	<mingo@redhat.com>, <bp@alien8.de>, <dave.hansen@linux.intel.com>,
	<hdegoede@redhat.com>, <ilpo.jarvinen@linux.intel.com>,
	<tony.luck@intel.com>, <jithu.joseph@intel.com>, Ashok Raj
	<ashok.raj@intel.com>
Subject: Re: [PATCH 1/2] x86/fpu: Extend kernel_fpu_begin_mask() to
 initialize AMX state
Message-ID: <ZjGB4WPJWj58TF4r@a4bf019067fa.jf.intel.com>
References: <20240430212508.105117-1-chang.seok.bae@intel.com>
 <20240430212508.105117-2-chang.seok.bae@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240430212508.105117-2-chang.seok.bae@intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?viligf1tSKeLlLsskSNYmqKyiM8baG87ZhpS3W+epbnie3FCws2p/HjBD41e?=
 =?us-ascii?Q?I22F1BItqsbpc8zwxnuhtt4hanP56l8CZo6CYyCDLSxoVwq4tZiZqw4j2fNX?=
 =?us-ascii?Q?1qkiiAdJ22nSm02R+jXLO0LeAYKmXaP7aIKpxZG4nsuF6nuYlrmftc5PonPr?=
 =?us-ascii?Q?elHMAFVtyUFCy+axPwAOxQONdnWAx1uCpiwodWlg7GZCyUNMrxoBSeqLqvtN?=
 =?us-ascii?Q?Tys5eOiDGQ5TRl5VGOUi+BQ79A/IWftFMvCU3Fg0knfrBS+C8PiBUT9H/LXZ?=
 =?us-ascii?Q?SGR+1++KSDXo13LJ4VRpUaW7wd/3FoSYHEjkpYpRXA6NwiLwmJ7ZyOcUr97j?=
 =?us-ascii?Q?oWb2bpm9oW2u+p4a/P153LTAK1M6GqSaucB92v76f6KiBXZzLcuNyS+OCCnV?=
 =?us-ascii?Q?tvXXkK3/RrbT/Ntp/yv1Cug6i9ik8C5riTVxAbnpOn6YSZxFIoklbS1xkxcT?=
 =?us-ascii?Q?rJ0lL4cH5uOeiE/6q9hPdrJisv+1VXa8EYc6bp5l7SiemmG5MB51kqcdizpg?=
 =?us-ascii?Q?p8AJrEmhUZne2ePAbcekzGPpSOlK8fWChfyyYXcJ/XlYQXE3XpepV3cDkL/5?=
 =?us-ascii?Q?3g+z5LoKz0xlAuqdTo3oHe109Lrd6Yrq2/w+rA1wDwf3Y5/ahk3ArR5ecngP?=
 =?us-ascii?Q?noqa383kWJKxhQDtF8wrsJXYCIYToZcMN/rK6FMG/1tUD4dQ+51soWqnlr+i?=
 =?us-ascii?Q?xTZHH4IhbXj8CyM41r5fi/XWMVkHrB3qltkT5KzSxWcD1gFYQscV8cqPgniI?=
 =?us-ascii?Q?+2QWbmxrEWJzXL/WLykWB8G8bkOcw2qtPPsjIyzWaXsuIHds0TbLuu4UWVPd?=
 =?us-ascii?Q?QQn9ihvCC6GHui7wLR+s8qvKa2nmmtQp6Bl5Q/9YXPymIeYfpAWbL7C0d0Xz?=
 =?us-ascii?Q?Jvy6CHCsaOKtAF1wJelxIktmXEFBGtSN20urFRPBU9ZXF1NFR0ah6J0UHQdt?=
 =?us-ascii?Q?+1DfT4k122PaKH2gLU2NegNkqzKej72mM88glk1DvjMZinAm1d/+EP9RVgDN?=
 =?us-ascii?Q?tV2ROjhJocYuFzj39NWFlktt6RSWrXYr/OXUI/ZJ0J57E8k8MTAlAKAd+pQn?=
 =?us-ascii?Q?TmvQen5Wh3rwQ5KVKZITcL2Knss1k0bbChefdK9zYX7IiTzj/NmYjOZD+OFj?=
 =?us-ascii?Q?a5WXyfJDlNOXQQizod6De0W0vQ82HEhGPL7AIf0TXGtVlCOhLE9rVpabH/bd?=
 =?us-ascii?Q?83xB0DqI5kJy/WtfVSpD39v/9MryV0j+6IplZTGkDd5bL6qviBaB2YdCO2xD?=
 =?us-ascii?Q?33fah+OfD90XiCgIcoUh4LpKt2hD4qOIaTiJt5DCGOgfpdjcdWd6+rkigEmE?=
 =?us-ascii?Q?1G1V39EcLZIY0eL8YF+7dHfLwyOoXy/XdAMpLuZ0cOgpzkPTwu/5nDo+mmjL?=
 =?us-ascii?Q?bahDB1RUXT+CgDspW6bHOHcymxweFuFcp8HJWjeF6Jpiu9LEyUhEWeuU2HJg?=
 =?us-ascii?Q?AM1+Ocy0YuL7f6gHCiR34jBjQHzTIPiqpE2ochQH9Gi3RAxJDZBSqe/445OG?=
 =?us-ascii?Q?19wNbDpJ0omwJIRZK5lgIWvojWm63D2I5lToNd8rpUVUAuuBkexFnXYgzPC/?=
 =?us-ascii?Q?FBTmhS+Ddrc2npfvM/WW5K+RINHBIElg8Wx+vgGg?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 2de85443-25b8-4986-4830-08dc696f340c
X-MS-Exchange-CrossTenant-AuthSource: SJ1PR11MB6201.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Apr 2024 23:42:31.6385
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: fzwZztIjPk+ZBB1hhRg2czYkpnbbPyiDq8+I1N10ozgOIEsfcES/tXf0ZNUF2PNlclXA/5k0s0aFHL9y0ZVnwA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB7251
X-OriginatorOrg: intel.com

On Tue, Apr 30, 2024 at 02:25:07PM -0700, Bae, Chang Seok wrote:
> The In-Field Scan [1] test does not start its operation when the CPU is
> unprepared. If the AMX state is uninitialized, the scan test will
> immediately terminate. Therefore, a proper initialization of the AMX
> state is necessary to run the test.

Instead of using new terminology like "unprepared", maybe use the
term "non-initialized state" as in the SDM for clarity? 

	Maintaining AMX state in a non-initialized state will prevent In-field
	Scan test [1] to abort without making progress.

> 
> Although fpu_idle_fpregs() initializes the state, its usage should be
> limited to specialized cases, primarily before entering the sleep state.
> The restore_fpregs_from_fpstate() function offers a suitable mechanism
> for initializing fpstate in general, which remains within the core code.
> 
> Extend kernel_fpu_begin_mask() to include AMX state initialization,
> providing the in-field scan driver code access to the appropriate
> initialization method while maintaining compliance with established FPU
> API semantics.

Maybe simplify the above paragraph with

Extend kernel_fpu_begin_mask() to allow In-field scan driver to initialize
AMX state before running tests.

> 
> [1] https://docs.kernel.org/arch/x86/ifs.html
> Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
> ---
> The necessity for AMX initialization is clarified in the Intel Software
> Development Manual as of March 2024, particularly in Section 18.2
> RECOMMENDATIONS FOR SYSTEM SOFTWARE of Vol. 1.
> 
> Side note: restore_fpregs_from_fpstate() also sets the x87 state to a
> fixed value. However, this only applies to AMD CPUs with the FXSAVE_LEAK
> quirk.
> ---
>  arch/x86/include/asm/fpu/api.h | 1 +
>  arch/x86/kernel/fpu/core.c     | 3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
> index a2be3aefff9f..67887fc45c24 100644
> --- a/arch/x86/include/asm/fpu/api.h
> +++ b/arch/x86/include/asm/fpu/api.h
> @@ -25,6 +25,7 @@
>  /* Kernel FPU states to initialize in kernel_fpu_begin_mask() */
>  #define KFPU_387	_BITUL(0)	/* 387 state will be initialized */
>  #define KFPU_MXCSR	_BITUL(1)	/* MXCSR will be initialized */
> +#define KFPU_AMX	_BITUL(2)	/* AMX will be initialized */
>  
>  extern void kernel_fpu_begin_mask(unsigned int kfpu_mask);
>  extern void kernel_fpu_end(void);
> diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
> index 520deb411a70..0c0235b4a901 100644
> --- a/arch/x86/kernel/fpu/core.c
> +++ b/arch/x86/kernel/fpu/core.c
> @@ -440,6 +440,9 @@ void kernel_fpu_begin_mask(unsigned int kfpu_mask)
>  
>  	if (unlikely(kfpu_mask & KFPU_387) && boot_cpu_has(X86_FEATURE_FPU))
>  		asm volatile ("fninit");
> +
> +	if (unlikely(kfpu_mask & KFPU_AMX) && boot_cpu_has(X86_FEATURE_AMX_TILE))
> +		restore_fpregs_from_fpstate(&init_fpstate, XFEATURE_MASK_XTILE);
>  }
>  EXPORT_SYMBOL_GPL(kernel_fpu_begin_mask);
>  
> -- 
> 2.34.1
> 

-- 
Cheers,
Ashok