[PATCH 0/2] Fixes for recent x86/boot rip-relative addressing changes

[PATCH 0/2] Fixes for recent x86/boot rip-relative addressing changes

[Tom Lendacky]

This patch series provides fixes for the recent x86/boot rip-relative
addressing changes that causes system crashes when booting with 5-level
pagetables and SME active.

I thought I had tested 5-level paging with SME, but must have missed
it. There are two patches to fix the issues that can be squashed into
a single patch with multiple Fixes: tags if desired.

The second patch is sort of a revert, but instead uses the newer
RIP_REL_REF() macro instead of reverting the fix and continuing to use
the fixup_pointer() support.

---

Patches based on:
  https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master
  30052fd948a3 ("Merge branch into tip/master: 'x86/shstk'")

Tom Lendacky (2):
  x86/boot/64: Apply encryption mask to 5-level pagetable update
  x86/boot/64: Move 5-level paging global variable assignments back

 arch/x86/kernel/head64.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

-- 
2.43.2

[PATCH 1/2] x86/boot/64: Apply encryption mask to 5-level pagetable update

[Tom Lendacky]

When running with 5-level page tables, the kernel mapping PGD entry is
updated to point to the P4D table. The assignment uses _PAGE_TABLE_NOENC,
which, when SME is active (mem_encrypt=on), results in a page table
entry without the encryption mask set, causing the system to crash on
boot.

Change the assignment to use _PAGE_TABLE instead of _PAGE_TABLE_NOENC so
that the encryption mask is set for the PGD entry.

Fixes: 533568e06b15 ("x86/boot/64: Use RIP_REL_REF() to access early_top_pgt[]")
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/kernel/head64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 212e8e06aeba..7d2eb035b6a3 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -175,7 +175,7 @@ unsigned long __head __startup_64(unsigned long physaddr,
 		p4d = (p4dval_t *)&RIP_REL_REF(level4_kernel_pgt);
 		p4d[MAX_PTRS_PER_P4D - 1] += load_delta;
 
-		pgd[pgd_index(__START_KERNEL_map)] = (pgdval_t)p4d | _PAGE_TABLE_NOENC;
+		pgd[pgd_index(__START_KERNEL_map)] = (pgdval_t)p4d | _PAGE_TABLE;
 	}
 
 	RIP_REL_REF(level3_kernel_pgt)[PTRS_PER_PUD - 2].pud += load_delta;
-- 
2.43.2

[PATCH 2/2] x86/boot/64: Move 5-level paging global variable assignments back

[Tom Lendacky]

Commit 63bed9660420 ("x86/startup_64: Defer assignment of 5-level paging
global variables") moved assignment of 5-level global variables to later
in the boot in order to avoid having to use RIP relative addressing in
order to set them. However, when running with 5-level paging and SME
active (mem_encrypt=on), the variables are needed as part of the page
table setup needed to encrypt the kernel (using pgd_none(), p4d_offset(),
etc.). Since the variables haven't been set, the page table manipulation
is done as if 4-level paging is active, causing the system to crash on
boot.

While only a subset of the assignments that were moved need to be set
early, move all of the assignments back into check_la57_support() so that
these assignments aren't spread between two locations. Instead of just
reverting the fix, this uses the new RIP_REL_REF() macro when assigning
the variables.

Fixes: 63bed9660420 ("x86/startup_64: Defer assignment of 5-level paging global variables")
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/kernel/head64.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 7d2eb035b6a3..a817ed0724d1 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -81,6 +81,13 @@ static inline bool check_la57_support(void)
 	if (!(native_read_cr4() & X86_CR4_LA57))
 		return false;
 
+	RIP_REL_REF(__pgtable_l5_enabled)	= 1;
+	RIP_REL_REF(pgdir_shift)		= 48;
+	RIP_REL_REF(ptrs_per_p4d)		= 512;
+	RIP_REL_REF(page_offset_base)		= __PAGE_OFFSET_BASE_L5;
+	RIP_REL_REF(vmalloc_base)		= __VMALLOC_BASE_L5;
+	RIP_REL_REF(vmemmap_base)		= __VMEMMAP_BASE_L5;
+
 	return true;
 }
 
@@ -431,15 +438,6 @@ asmlinkage __visible void __init __noreturn x86_64_start_kernel(char * real_mode
 				(__START_KERNEL & PGDIR_MASK)));
 	BUILD_BUG_ON(__fix_to_virt(__end_of_fixed_addresses) <= MODULES_END);
 
-	if (check_la57_support()) {
-		__pgtable_l5_enabled	= 1;
-		pgdir_shift		= 48;
-		ptrs_per_p4d		= 512;
-		page_offset_base	= __PAGE_OFFSET_BASE_L5;
-		vmalloc_base		= __VMALLOC_BASE_L5;
-		vmemmap_base		= __VMEMMAP_BASE_L5;
-	}
-
 	cr4_init_shadow();
 
 	/* Kill off the identity-map trampoline */
-- 
2.43.2

Re: [PATCH 0/2] Fixes for recent x86/boot rip-relative addressing changes

[Ard Biesheuvel]

On Fri, 22 Mar 2024 at 17:41, Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> This patch series provides fixes for the recent x86/boot rip-relative
> addressing changes that causes system crashes when booting with 5-level
> pagetables and SME active.
>
> I thought I had tested 5-level paging with SME, but must have missed
> it. There are two patches to fix the issues that can be squashed into
> a single patch with multiple Fixes: tags if desired.
>

Perhaps you tested the entire series, where the final patch removed
those variables altogether?

> The second patch is sort of a revert, but instead uses the newer
> RIP_REL_REF() macro instead of reverting the fix and continuing to use
> the fixup_pointer() support.
>

Thanks for fixing this.

Series

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

Re: [PATCH 0/2] Fixes for recent x86/boot rip-relative addressing changes

[Tom Lendacky]

On 3/22/24 11:10, Ard Biesheuvel wrote:
> On Fri, 22 Mar 2024 at 17:41, Tom Lendacky <thomas.lendacky@amd.com> wrote:
>>
>> This patch series provides fixes for the recent x86/boot rip-relative
>> addressing changes that causes system crashes when booting with 5-level
>> pagetables and SME active.
>>
>> I thought I had tested 5-level paging with SME, but must have missed
>> it. There are two patches to fix the issues that can be squashed into
>> a single patch with multiple Fixes: tags if desired.
>>
> 
> Perhaps you tested the entire series, where the final patch removed
> those variables altogether?

Maybe, but that wouldn't explain the first fix in the series. I should've 
encountered an issue no matter what. I probably used the wrong config file 
or ...  who knows at this point.

Thanks,
Tom

> 
>> The second patch is sort of a revert, but instead uses the newer
>> RIP_REL_REF() macro instead of reverting the fix and continuing to use
>> the fixup_pointer() support.
>>
> 
> Thanks for fixing this.
> 
> Series
> 
> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>