Re: [PATCH] x86/pkeys: Disable PKU when XFEATURE_PKRU is missing

[Ingo Molnar]

* Akihiro Suda <suda.gitsendemail@xxxxxxxxx> wrote:

> Even when X86_FEATURE_PKU and X86_FEATURE_OSPKE are available,
> XFEATURE_PKRU can be missing.
> In such a case, pkeys has to be disabled to avoid hanging up.
> 
>   WARNING: CPU: 0 PID: 1 at arch/x86/kernel/fpu/xstate.c:1003 get_xsave_addr_user+0x28/0x40
>   (...)
>   Call Trace:
>    <TASK>
>    ? get_xsave_addr_user+0x28/0x40
>    ? __warn.cold+0x8e/0xea
>    ? get_xsave_addr_user+0x28/0x40
>    ? report_bug+0xff/0x140
>    ? handle_bug+0x3b/0x70
>    ? exc_invalid_op+0x17/0x70
>    ? asm_exc_invalid_op+0x1a/0x20
>    ? get_xsave_addr_user+0x28/0x40
>    copy_fpstate_to_sigframe+0x1be/0x380
>    ? __put_user_8+0x11/0x20
>    get_sigframe+0xf1/0x280
>    x64_setup_rt_frame+0x67/0x2c0
>    arch_do_signal_or_restart+0x1b3/0x240
>    syscall_exit_to_user_mode+0xb0/0x130
>    do_syscall_64+0xab/0x1a0
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> This fix is known to be needed on Apple Virtualization.
> Tested with macOS 13.5.2 running on MacBook Pro 2020 with
> Intel(R) Core(TM) i7-1068NG7 CPU @ 2.30GHz.
> 
> Fixes: 70044df250d0 ("x86/pkeys: Update PKRU to enable all pkeys before XSAVE")
> Link: https://lore.kernel.org/regressions/CAG8fp8QvH71Wi_y7b7tgFp7knK38rfrF7rRHh-gFKqeS0gxY6Q@xxxxxxxxxxxxxx/T/#u
> Link: https://github.com/lima-vm/lima/issues/3334
> 
> Signed-off-by: Akihiro Suda <akihiro.suda.cz@xxxxxxxxxxxxx>
> ---
>  arch/x86/kernel/cpu/common.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index e9464fe411ac..4c2c268af214 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -517,7 +517,8 @@ static bool pku_disabled;
>  static __always_inline void setup_pku(struct cpuinfo_x86 *c)
>  {
>  	if (c == &boot_cpu_data) {
> -		if (pku_disabled || !cpu_feature_enabled(X86_FEATURE_PKU))
> +		if (pku_disabled || !cpu_feature_enabled(X86_FEATURE_PKU) ||
> +		    !cpu_has_xfeatures(XFEATURE_PKRU, NULL))
>  			return;

Note that silent quirks are counterproductive, as they don't give VM 
vendors any incentives to fix their VM for such bugs.

So I changed your quirk to be:

--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -519,6 +519,17 @@ static __always_inline void setup_pku(struct cpuinfo_x86 *c)
 	if (c == &boot_cpu_data) {
 		if (pku_disabled || !cpu_feature_enabled(X86_FEATURE_PKU))
 			return;
+		if (!cpu_has_xfeatures(XFEATURE_PKRU, NULL)) {
+			/*
+			 * Missing XFEATURE_PKRU is not really a valid CPU
+			 * configuration at this point, but apparently
+			 * Apple Virtualization is affected by this,
+			 * so return with a FW warning instead of crashing
+			 * the bootup:
+			 */
+			WARN_ONCE(1, FW_BUG "Invalid XFEATURE_PKRU configuration.\n");
+			return;
+		}
 		/*
 		 * Setting CR4.PKE will cause the X86_FEATURE_OSPKE cpuid
 		 * bit to be set.  Enforce it.

This is noisy in the syslog, but it's a WARN_ONCE() and it doesn't 
crash the bootup.

Thanks,

	Ingo