On 7/21/25 16:20, Huang, Kai wrote: > On Mon, 2025-07-21 at 09:40 -0500, Tom Lendacky wrote: >> On 7/17/25 16:46, Kai Huang wrote: >>> During kexec, the kernel jumps to the new kernel in relocate_kernel(), >>> which is implemented in assembly and both 32-bit and 64-bit have their >>> own version. >>> >>> Currently, for both 32-bit and 64-bit, the last two parameters of the >>> relocate_kernel() are both 'unsigned int' but actually they only convey >>> a boolean, i.e., one bit information. The 'unsigned int' has enough >>> space to carry two bits information therefore there's no need to pass >>> the two booleans in two separate 'unsigned int'. >>> >>> Consolidate the last two function parameters of relocate_kernel() into a >>> single 'unsigned int' and pass flags instead. >>> >>> Only consolidate the 64-bit version albeit the similar optimization can >>> be done for the 32-bit version too. Don't bother changing the 32-bit >>> version while it is working (since assembly code change is required). >>> >>> Signed-off-by: Kai Huang <kai.huang@xxxxxxxxx> >>> --- >>> arch/x86/include/asm/kexec.h | 12 ++++++++++-- >>> arch/x86/kernel/machine_kexec_64.c | 22 +++++++++++++--------- >>> arch/x86/kernel/relocate_kernel_64.S | 19 +++++++++---------- >>> 3 files changed, 32 insertions(+), 21 deletions(-) >>> >>> @@ -204,7 +202,7 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped) >>> * entries that will conflict with the now unencrypted memory >>> * used by kexec. Flush the caches before copying the kernel. >>> */ >>> - testq %r8, %r8 >>> + testq $RELOC_KERNEL_HOST_MEM_ACTIVE, %r11 >> >> Hmmm... can't both bits be set at the same time? If so, then this will >> fail. This should be doing bit tests now. > > TEST instruction performs logical AND of the two operands, therefore the > above equals to: > > set ZF if "R11 AND BIT(1) == 0". > > Whether there's any other bits set in R11 doesn't impact the above, right? > Doh! My bad, yes, not sure what I was thinking there. Thanks, Tom >> >>> jz .Lsme_off >>> wbinvd >>> .Lsme_off: >>> @@ -220,7 +218,7 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped) >>> movq %cr3, %rax >>> movq %rax, %cr3 >>> >>> - testq %r11, %r11 /* preserve_context */ >>> + testq $RELOC_KERNEL_PRESERVE_CONTEXT, %r11 >>> jnz .Lrelocate >>> >>> /* >>> @@ -273,7 +271,7 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped) >>> ANNOTATE_NOENDBR >>> andq $PAGE_MASK, %r8 >>> lea PAGE_SIZE(%r8), %rsp >>> - movl $1, %r11d /* Ensure preserve_context flag is set */ >>> + movl $RELOC_KERNEL_PRESERVE_CONTEXT, %r11d /* Ensure preserve_context flag is set */ >> >> And this will clear any value that was in r11 vs setting a single bit. >> Not sure it currently has any effect because r8 (where the memory >> encryption setting was held) is modified just before this. But if any >> bits are added in the future that are needed past here, this will be a >> problem. > > Right. It's just for the > > call swap_pages > > right after it. Nothing else later uses RELOC_KERNEL_PRESERVE_CONTEXT or > RELOC_KERNEL_HOST_MEM_ACTIVE. > > Maybe we can add a comment to remind that all other flags are not restored > so if someone wants to add a new bit and use it at a later he/she can see? > > /* > * Ensure RELOC_KERNEL_PRESERVE_CONTEXT flag is set so swap_pages > * can do things correctly. Note this doesn't restore any other > * RELOC_KERNEL_* flags that were passed to relocate_kernel(). > */ >> >>> call swap_pages >>> movq kexec_va_control_page(%rip), %rax >>> 0: addq $virtual_mapped - 0b, %rax >>> @@ -321,7 +319,7 @@ SYM_CODE_START_LOCAL_NOALIGN(swap_pages) >>> UNWIND_HINT_END_OF_STACK >>> /* >>> * %rdi indirection page >>> - * %r11 preserve_context >>> + * %r11 flags: RELOC_KERNEL_* >>> */ >>> movq %rdi, %rcx /* Put the indirection_page in %rcx */ >>> xorl %edi, %edi >>> @@ -357,7 +355,8 @@ SYM_CODE_START_LOCAL_NOALIGN(swap_pages) >>> movq %rdi, %rdx /* Save destination page to %rdx */ >>> movq %rsi, %rax /* Save source page to %rax */ >>> >>> - testq %r11, %r11 /* Only actually swap for ::preserve_context */ >>> + /* Only actually swap for ::preserve_context */ >>> + testq $RELOC_KERNEL_PRESERVE_CONTEXT, %r11 >> >> Ditto here on the bit testing. > > I don't see any problem? Please see above.
