Re: [PATCH v5 2/5] x86/kexec: do unconditional WBINVD for bare-metal in relocate_kernel()
From: Huang, Kai
Date: Thu Aug 15 2024 - 19:45:34 EST
>
> #define ARCH_HAS_KIMAGE_ARCH
> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> index 9c9ac606893e..07ca9d3361a3 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -392,7 +392,7 @@ void machine_kexec(struct kimage *image)
> (unsigned long)page_list,
> image->start,
> image->preserve_context,
> - host_mem_enc_active);
> + !boot_cpu_has(X86_FEATURE_HYPERVISOR));
>
>
LKP reported below warning:
All warnings (new ones prefixed by >>):
arch/x86/kernel/machine_kexec_64.c: In function 'machine_kexec':
>> arch/x86/kernel/machine_kexec_64.c:325:22: warning: variable
'host_mem_enc_active' set but not used [-Wunused-but-set-variable]
325 | unsigned int host_mem_enc_active;
| ^~~~~~~~~~~~~~~~~~~
This is due to while rebasing I didn't pay enough attention to the recent code
from commit
93c1800b3799f ("x86/kexec: Fix bug with call depth tracking")
which introduced the host_mem_enc_active variable in order to avoid
cc_platform_has() function call after load_segments() to resolve a problem
when call depth tracking is on.
A 100% safe way is to replace
host_mem_enc_active = cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT);
... with
bare_metal = !boot_cpu_has(X86_FEATURE_HYPERVISOR);
but I think we can just remove that variable and directly use
!boot_cpu_has(X86_FEATURE_HYPERVISOR)
as the last argument of calling the relocate_kernel(), because AFAICT the
above X86_FEATURE_HYPERVISOR bit test will always generate inline code thus
there will be no additional CALL/RET.
The incremental diff will be:
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -331,16 +331,9 @@ static void kexec_save_processor_start(struct kimage
*image)
void machine_kexec(struct kimage *image)
{
unsigned long page_list[PAGES_NR];
- unsigned int host_mem_enc_active;
int save_ftrace_enabled;
void *control_page;
- /*
- * This must be done before load_segments() since if call depth
tracking
- * is used then GS must be valid to make any function calls.
- */
- host_mem_enc_active = cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT);
-
Am I missing anything?
I'll send out a new version with the above and put some explanation to the
changelog if I don't see any other feedback on the rest TDX patches in the
coming days. Thanks!