[v5] x86/sev: Fix making shared pages private during kdump

[PATCH v5] x86/sev: Fix making shared pages private during kdump

Posted by Ashish Kalra 9 months ago

From: Ashish Kalra <ashish.kalra@amd.com>

When the shared pages are being made private during kdump preparation
there are additional checks to handle shared GHCB pages.

These additional checks include handling the case of GHCB page being
contained within a huge page.

The check for handling the case of GHCB contained within a huge
page incorrectly skips a page just below the GHCB page from being
transitioned back to private during kdump preparation.

This skipped page causes a 0x404 #VC exception when it is accessed
later while dumping guest memory during vmcore generation via kdump.

Correct the range to be checked for GHCB contained in a huge page.
Also ensure that the skipped huge page containing the GHCB page is
transitioned back to private by applying the correct address mask
later when changing GHCBs to private at end of kdump preparation.

Fixes: 3074152e56c9 ("x86/sev: Convert shared memory back to private on kexec")
Cc: stable@vger.kernel.org
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 arch/x86/coco/sev/core.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index d35fec7b164a..30b74e4e4e88 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -1019,7 +1019,8 @@ static void unshare_all_memory(void)
 			data = per_cpu(runtime_data, cpu);
 			ghcb = (unsigned long)&data->ghcb_page;
 
-			if (addr <= ghcb && ghcb <= addr + size) {
+			/* Handle the case of a huge page containing the GHCB page */
+			if (addr <= ghcb && ghcb < addr + size) {
 				skipped_addr = true;
 				break;
 			}
@@ -1131,8 +1132,8 @@ static void shutdown_all_aps(void)
 void snp_kexec_finish(void)
 {
 	struct sev_es_runtime_data *data;
+	unsigned long size, addr;
 	unsigned int level, cpu;
-	unsigned long size;
 	struct ghcb *ghcb;
 	pte_t *pte;
 
@@ -1160,8 +1161,10 @@ void snp_kexec_finish(void)
 		ghcb = &data->ghcb_page;
 		pte = lookup_address((unsigned long)ghcb, &level);
 		size = page_level_size(level);
-		set_pte_enc(pte, level, (void *)ghcb);
-		snp_set_memory_private((unsigned long)ghcb, (size / PAGE_SIZE));
+		/* Handle the case of a huge page containing the GHCB page */
+		addr = (unsigned long)ghcb & page_level_mask(level);
+		set_pte_enc(pte, level, (void *)addr);
+		snp_set_memory_private(addr, (size / PAGE_SIZE));
 	}
 }
 
-- 
2.34.1

Re: [PATCH v5] x86/sev: Fix making shared pages private during kdump

Posted by Tom Lendacky 9 months ago

On 5/6/25 13:35, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> When the shared pages are being made private during kdump preparation
> there are additional checks to handle shared GHCB pages.
> 
> These additional checks include handling the case of GHCB page being
> contained within a huge page.
> 
> The check for handling the case of GHCB contained within a huge
> page incorrectly skips a page just below the GHCB page from being
> transitioned back to private during kdump preparation.
> 
> This skipped page causes a 0x404 #VC exception when it is accessed
> later while dumping guest memory during vmcore generation via kdump.
> 
> Correct the range to be checked for GHCB contained in a huge page.
> Also ensure that the skipped huge page containing the GHCB page is
> transitioned back to private by applying the correct address mask
> later when changing GHCBs to private at end of kdump preparation.
> 
> Fixes: 3074152e56c9 ("x86/sev: Convert shared memory back to private on kexec")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  arch/x86/coco/sev/core.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index d35fec7b164a..30b74e4e4e88 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -1019,7 +1019,8 @@ static void unshare_all_memory(void)
>  			data = per_cpu(runtime_data, cpu);
>  			ghcb = (unsigned long)&data->ghcb_page;
>  
> -			if (addr <= ghcb && ghcb <= addr + size) {
> +			/* Handle the case of a huge page containing the GHCB page */
> +			if (addr <= ghcb && ghcb < addr + size) {
>  				skipped_addr = true;
>  				break;
>  			}
> @@ -1131,8 +1132,8 @@ static void shutdown_all_aps(void)
>  void snp_kexec_finish(void)
>  {
>  	struct sev_es_runtime_data *data;
> +	unsigned long size, addr;
>  	unsigned int level, cpu;
> -	unsigned long size;
>  	struct ghcb *ghcb;
>  	pte_t *pte;
>  
> @@ -1160,8 +1161,10 @@ void snp_kexec_finish(void)
>  		ghcb = &data->ghcb_page;
>  		pte = lookup_address((unsigned long)ghcb, &level);
>  		size = page_level_size(level);
> -		set_pte_enc(pte, level, (void *)ghcb);
> -		snp_set_memory_private((unsigned long)ghcb, (size / PAGE_SIZE));
> +		/* Handle the case of a huge page containing the GHCB page */
> +		addr = (unsigned long)ghcb & page_level_mask(level);
> +		set_pte_enc(pte, level, (void *)addr);
> +		snp_set_memory_private(addr, (size / PAGE_SIZE));
>  	}
>  }
>

Re: [PATCH v5] x86/sev: Fix making shared pages private during kdump

Posted by Borislav Petkov 9 months ago

On Tue, May 06, 2025 at 06:35:29PM +0000, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> When the shared pages are being made private during kdump preparation
> there are additional checks to handle shared GHCB pages.
> 
> These additional checks include handling the case of GHCB page being
> contained within a huge page.
> 
> The check for handling the case of GHCB contained within a huge
> page incorrectly skips a page just below the GHCB page from being
> transitioned back to private during kdump preparation.
> 
> This skipped page causes a 0x404 #VC exception when it is accessed
> later while dumping guest memory during vmcore generation via kdump.
> 
> Correct the range to be checked for GHCB contained in a huge page.
> Also ensure that the skipped huge page containing the GHCB page is
> transitioned back to private by applying the correct address mask
> later when changing GHCBs to private at end of kdump preparation.
> 
> Fixes: 3074152e56c9 ("x86/sev: Convert shared memory back to private on kexec")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  arch/x86/coco/sev/core.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)

Ok, I've pushed both patches here:

https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=tip-x86-urgent-sev

Please have those who are affected by the issues test and report back.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v5] x86/sev: Fix making shared pages private during kdump

Posted by Aithal, Srikanth 8 months, 4 weeks ago

On 5/7/2025 3:12 PM, Borislav Petkov wrote:
> On Tue, May 06, 2025 at 06:35:29PM +0000, Ashish Kalra wrote:
>> From: Ashish Kalra <ashish.kalra@amd.com>
>>
>> When the shared pages are being made private during kdump preparation
>> there are additional checks to handle shared GHCB pages.
>>
>> These additional checks include handling the case of GHCB page being
>> contained within a huge page.
>>
>> The check for handling the case of GHCB contained within a huge
>> page incorrectly skips a page just below the GHCB page from being
>> transitioned back to private during kdump preparation.
>>
>> This skipped page causes a 0x404 #VC exception when it is accessed
>> later while dumping guest memory during vmcore generation via kdump.
>>
>> Correct the range to be checked for GHCB contained in a huge page.
>> Also ensure that the skipped huge page containing the GHCB page is
>> transitioned back to private by applying the correct address mask
>> later when changing GHCBs to private at end of kdump preparation.
>>
>> Fixes: 3074152e56c9 ("x86/sev: Convert shared memory back to private on kexec")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>> ---
>>   arch/x86/coco/sev/core.c | 11 +++++++----
>>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> Ok, I've pushed both patches here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=tip-x86-urgent-sev
> 
> Please have those who are affected by the issues test and report back.
> 
> Thx.
> 

I tested the failure scenario with the patch hosted in the referenced 
Git tree [1]. The patch resolves the issue.

Tested-by: Srikanth Aithal <sraithal@amd.com>

[1]: 
https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=tip-x86-urgent-sev

[tip: x86/urgent] x86/sev: Make sure pages are not skipped during kdump

Posted by tip-bot2 for Ashish Kalra 8 months, 4 weeks ago

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     82b7f88f2316c5442708daeb0b5ec5aa54c8ff7f
Gitweb:        https://git.kernel.org/tip/82b7f88f2316c5442708daeb0b5ec5aa54c8ff7f
Author:        Ashish Kalra <ashish.kalra@amd.com>
AuthorDate:    Tue, 06 May 2025 18:35:29 
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Tue, 13 May 2025 19:47:48 +02:00

x86/sev: Make sure pages are not skipped during kdump

When shared pages are being converted to private during kdump, additional
checks are performed. They include handling the case of a GHCB page being
contained within a huge page.

Currently, this check incorrectly skips a page just below the GHCB page from
being transitioned back to private during kdump preparation.

This skipped page causes a 0x404 #VC exception when it is accessed later while
dumping guest memory for vmcore generation.

Correct the range to be checked for GHCB contained in a huge page.  Also,
ensure that the skipped huge page containing the GHCB page is transitioned
back to private by applying the correct address mask later when changing GHCBs
to private at end of kdump preparation.

  [ bp: Massage commit message. ]

Fixes: 3074152e56c9 ("x86/sev: Convert shared memory back to private on kexec")
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Tested-by: Srikanth Aithal <sraithal@amd.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/20250506183529.289549-1-Ashish.Kalra@amd.com
---
 arch/x86/coco/sev/core.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index 41060ba..36beaac 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -1101,7 +1101,8 @@ static void unshare_all_memory(void)
 			data = per_cpu(runtime_data, cpu);
 			ghcb = (unsigned long)&data->ghcb_page;
 
-			if (addr <= ghcb && ghcb <= addr + size) {
+			/* Handle the case of a huge page containing the GHCB page */
+			if (addr <= ghcb && ghcb < addr + size) {
 				skipped_addr = true;
 				break;
 			}
@@ -1213,8 +1214,8 @@ static void shutdown_all_aps(void)
 void snp_kexec_finish(void)
 {
 	struct sev_es_runtime_data *data;
+	unsigned long size, addr;
 	unsigned int level, cpu;
-	unsigned long size;
 	struct ghcb *ghcb;
 	pte_t *pte;
 
@@ -1242,8 +1243,10 @@ void snp_kexec_finish(void)
 		ghcb = &data->ghcb_page;
 		pte = lookup_address((unsigned long)ghcb, &level);
 		size = page_level_size(level);
-		set_pte_enc(pte, level, (void *)ghcb);
-		snp_set_memory_private((unsigned long)ghcb, (size / PAGE_SIZE));
+		/* Handle the case of a huge page containing the GHCB page */
+		addr = (unsigned long)ghcb & page_level_mask(level);
+		set_pte_enc(pte, level, (void *)addr);
+		snp_set_memory_private(addr, (size / PAGE_SIZE));
 	}
 }