Re: [PATCH v4] x86/boot: Don't return encryption mask from __startup_64()

["Huang, Kai" <kai.huang@intel.com>]

On Thu, 2025-06-19 at 07:36 +0000, Khalid Ali wrote:
> --- a/arch/x86/kernel/head_64.S
> +++ b/arch/x86/kernel/head_64.S
> @@ -106,18 +106,16 @@ SYM_CODE_START_NOALIGN(startup_64)
>  
>  	/*
>  	 * Perform pagetable fixups. Additionally, if SME is active, encrypt
> -	 * the kernel and retrieve the modifier (SME encryption mask if SME
> -	 * is active) to be added to the initial pgdir entry that will be
> -	 * programmed into CR3.
> -	 */
> +	 * the kernel.
> +	 /

Sigh... this comment is broken, since there's no '*' before the last '/'.

>  	movq	%r15, %rsi
>  	call	__startup_64

Here's how I find it:

So I went to see why this patch caused early boot failure, since the code
change doesn't seem wrong to me.

After staring at the code for half hour and yet unable to see any issue, I
went to disassemble the kernel image, then I found the above two lines of
code wasn't there at all.

Then looking at this again, it's obvious that the reason is the change to
the comment is broken, leading the above two lines of code being commented
out.

With this fixed, I can boot the kernel in a normal VM (both w/ and w/o
CONFIG_AMD_MEM_ENCRYPT).

And this patch has other style issues too like the broken indent of function
parameters after changing returning value from 'unsigned int' to 'void', and
...
	
>  
>  	/* Form the CR3 value being sure to include the CR3 modifier */
> -	leaq	early_top_pgt(%rip), %rcx
> -	addq	%rcx, %rax
> -
> +	leaq	early_top_pgt(%rip), %rax
> +	
... a tailing whitespace here.

So, please, before posting patches, test them, and run
./scripts/checkpatch.pl against them.

And I would suggest AMD guys to avoid looking into this until those issues
are fixed.