[kvm-unit-tests RFC PATCH 1/3] x86 EFI: Bypass call to fdt_check_header()

[kvm-unit-tests RFC PATCH 1/3] x86 EFI: Bypass call to fdt_check_header()

[Pavan Kumar Paluri]

Issuing a call to fdt_check_header() prevents running any of x86 UEFI
enabled tests. Bypass this call for x86 in order to enable UEFI
supported tests for KUT x86 arch.

Fixes: 9632ce446b8f ("arm64: efi: Improve device tree discovery")
Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
---
 lib/efi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/efi.c b/lib/efi.c
index 5314eaa81e66..124e77685230 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -328,6 +328,10 @@ static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
 		return NULL;
 	}
 
+#ifdef __x86_64__
+	return fdt;
+#endif
+
 	return fdt_check_header(fdt) == 0 ? fdt : NULL;
 }
 
-- 
2.34.1

[kvm-unit-tests RFC PATCH 2/3] x86/efi: Retry call to efi exit boot services

[Pavan Kumar Paluri]

In some cases, KUT guest might fail to exit boot services due to a
possible memory map update that might have taken place between
efi_get_memory_map() and efi_exit_boot_services() calls. As per UEFI
spec 2.10 (Section 7.4.6 EFI_BOOT_SERVICES.ExitBootServices()), we need
to update the memory map and retry call to exit boot
services.

Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
---
 lib/efi.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/lib/efi.c b/lib/efi.c
index 124e77685230..9d066bfad0b6 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -458,14 +458,27 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
 	}
 #endif
 
-	/* 
+	/*
 	 * Exit EFI boot services, let kvm-unit-tests take full control of the
-	 * guest
+	 * guest.
 	 */
 	status = efi_exit_boot_services(handle, &efi_bootinfo.mem_map);
-	if (status != EFI_SUCCESS) {
-		printf("Failed to exit boot services\n");
-		goto efi_main_error;
+
+	/*
+	 * There is a possibility that memory map might have changed
+	 * between efi_get_memory_map() and efi_exit_boot_services in
+	 * which case status is EFI_INVALID_PARAMETER. As per UEFI spec
+	 * 2.10, we need to get the updated memory map and try again.
+	 */
+	if (status == EFI_INVALID_PARAMETER) {
+		efi_get_memory_map(&efi_bootinfo.mem_map);
+
+		status = efi_exit_boot_services(handle,
+						&efi_bootinfo.mem_map);
+		if (status != EFI_SUCCESS) {
+			printf("Failed to exit boot services\n");
+			goto efi_main_error;
+		}
 	}
 
 	/* Set up arch-specific resources */
-- 
2.34.1

[kvm-unit-tests RFC PATCH 3/3] x86 AMD SEV-ES: Setup a new page table and install level 1 PTEs

[Pavan Kumar Paluri]

KUT's UEFI tests don't currently have support for page allocation.
SEV-ES/SNP tests will need this later, so the support for page
allocation is provided via setup_vm().

SEV-ES/SNP guest uses GHCB page to communicate with the host. Such a
page should remain unencrypted (its c-bit should be unset). Therefore,
call setup_ghcb_pte() in the path of setup_vm() to make sure c-bit of
GHCB's pte is unset.

Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
---
 lib/x86/vm.c  | 6 ++++++
 x86/amd_sev.c | 7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/lib/x86/vm.c b/lib/x86/vm.c
index 90f73fbb2dfd..ce2063aee75d 100644
--- a/lib/x86/vm.c
+++ b/lib/x86/vm.c
@@ -3,6 +3,7 @@
 #include "vmalloc.h"
 #include "alloc_page.h"
 #include "smp.h"
+#include "amd_sev.h"
 
 static pteval_t pte_opt_mask;
 
@@ -197,6 +198,11 @@ void *setup_mmu(phys_addr_t end_of_memory, void *opt_mask)
     init_alloc_vpage((void*)(3ul << 30));
 #endif
 
+#ifdef CONFIG_EFI
+	if (amd_sev_es_enabled())
+		setup_ghcb_pte(cr3);
+#endif
+
     write_cr3(virt_to_phys(cr3));
 #ifndef __x86_64__
     write_cr4(X86_CR4_PSE);
diff --git a/x86/amd_sev.c b/x86/amd_sev.c
index 7757d4f85b7a..03636e581dfe 100644
--- a/x86/amd_sev.c
+++ b/x86/amd_sev.c
@@ -14,6 +14,8 @@
 #include "x86/processor.h"
 #include "x86/amd_sev.h"
 #include "msr.h"
+#include "x86/vm.h"
+#include "alloc_page.h"
 
 #define EXIT_SUCCESS 0
 #define EXIT_FAILURE 1
@@ -89,9 +91,14 @@ static void test_stringio(void)
 int main(void)
 {
 	int rtn;
+	unsigned long *vaddr;
 	rtn = test_sev_activation();
 	report(rtn == EXIT_SUCCESS, "SEV activation test.");
 	test_sev_es_activation();
 	test_stringio();
+	setup_vm();
+	vaddr = alloc_page();
+	if (!vaddr)
+		assert_msg(vaddr, "Page allocation Failure");
 	return report_summary();
 }
-- 
2.34.1

Re: [kvm-unit-tests RFC PATCH 1/3] x86 EFI: Bypass call to fdt_check_header()

[Andrew Jones]

On Mon, Mar 25, 2024 at 04:36:21PM -0500, Pavan Kumar Paluri wrote:
> Issuing a call to fdt_check_header() prevents running any of x86 UEFI
> enabled tests. Bypass this call for x86 in order to enable UEFI
> supported tests for KUT x86 arch.

Ouch! Sorry about that. I think I prefer something like below, though.

Thanks,
drew

diff --git a/lib/efi.c b/lib/efi.c
index 5314eaa81e66..335b66d26092 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -312,6 +312,7 @@ static void* efi_get_var(efi_handle_t handle, struct efi_loaded_image_64 *image,
        return val;
 }
 
+#if defined(__aarch64__) || defined(__riscv)
 static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
 {
        efi_char16_t var[] = ENV_VARNAME_DTBFILE;
@@ -330,6 +331,12 @@ static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
 
        return fdt_check_header(fdt) == 0 ? fdt : NULL;
 }
+#else
+static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
+{
+       return NULL;
+}
+#endif
 
 static const struct {
        struct efi_vendor_dev_path      vendor;

> 
> Fixes: 9632ce446b8f ("arm64: efi: Improve device tree discovery")
> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
> ---
>  lib/efi.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/efi.c b/lib/efi.c
> index 5314eaa81e66..124e77685230 100644
> --- a/lib/efi.c
> +++ b/lib/efi.c
> @@ -328,6 +328,10 @@ static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
>  		return NULL;
>  	}
>  
> +#ifdef __x86_64__
> +	return fdt;
> +#endif
> +
>  	return fdt_check_header(fdt) == 0 ? fdt : NULL;
>  }
>  
> -- 
> 2.34.1
> 

Re: [kvm-unit-tests RFC PATCH 2/3] x86/efi: Retry call to efi exit boot services

[Andrew Jones]

On Mon, Mar 25, 2024 at 04:36:22PM -0500, Pavan Kumar Paluri wrote:
> In some cases, KUT guest might fail to exit boot services due to a
> possible memory map update that might have taken place between
> efi_get_memory_map() and efi_exit_boot_services() calls. As per UEFI
> spec 2.10 (Section 7.4.6 EFI_BOOT_SERVICES.ExitBootServices()), we need
> to update the memory map and retry call to exit boot
> services.
> 
> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
> ---
>  lib/efi.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/efi.c b/lib/efi.c
> index 124e77685230..9d066bfad0b6 100644
> --- a/lib/efi.c
> +++ b/lib/efi.c
> @@ -458,14 +458,27 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
>  	}
>  #endif
>  
> -	/* 
> +	/*
>  	 * Exit EFI boot services, let kvm-unit-tests take full control of the
> -	 * guest
> +	 * guest.
>  	 */
>  	status = efi_exit_boot_services(handle, &efi_bootinfo.mem_map);
> -	if (status != EFI_SUCCESS) {
> -		printf("Failed to exit boot services\n");
> -		goto efi_main_error;
> +
> +	/*
> +	 * There is a possibility that memory map might have changed
> +	 * between efi_get_memory_map() and efi_exit_boot_services in
> +	 * which case status is EFI_INVALID_PARAMETER. As per UEFI spec
> +	 * 2.10, we need to get the updated memory map and try again.
> +	 */
> +	if (status == EFI_INVALID_PARAMETER) {

Shouldn't we loop on this? The spec doesn't make it clear that the second
try should always work.

> +		efi_get_memory_map(&efi_bootinfo.mem_map);
> +
> +		status = efi_exit_boot_services(handle,
> +						&efi_bootinfo.mem_map);
> +		if (status != EFI_SUCCESS) {
> +			printf("Failed to exit boot services\n");
> +			goto efi_main_error;
> +		}
>  	}
>  
>  	/* Set up arch-specific resources */
> -- 
> 2.34.1
>

Thanks,
drew

Re: [kvm-unit-tests RFC PATCH 1/3] x86 EFI: Bypass call to fdt_check_header()

[Paluri, PavanKumar]

On 3/26/2024 3:51 AM, Andrew Jones wrote:
> On Mon, Mar 25, 2024 at 04:36:21PM -0500, Pavan Kumar Paluri wrote:
>> Issuing a call to fdt_check_header() prevents running any of x86 UEFI
>> enabled tests. Bypass this call for x86 in order to enable UEFI
>> supported tests for KUT x86 arch.
> 
> Ouch! Sorry about that. I think I prefer something like below, though.
> 
> Thanks,
> drew
> 
> diff --git a/lib/efi.c b/lib/efi.c
> index 5314eaa81e66..335b66d26092 100644
> --- a/lib/efi.c
> +++ b/lib/efi.c
> @@ -312,6 +312,7 @@ static void* efi_get_var(efi_handle_t handle, struct efi_loaded_image_64 *image,
>         return val;
>  }
>  
> +#if defined(__aarch64__) || defined(__riscv)
>  static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
>  {
>         efi_char16_t var[] = ENV_VARNAME_DTBFILE;
> @@ -330,6 +331,12 @@ static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
>  
>         return fdt_check_header(fdt) == 0 ? fdt : NULL;
>  }
> +#else
> +static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
> +{
> +       return NULL;
> +}
> +#endif
>  
>  static const struct {
>         struct efi_vendor_dev_path      vendor;
> 

Thanks for the review, this looks better. I will soon send out a v2 with
the changes.

Thanks,
Pavan
>>
>> Fixes: 9632ce446b8f ("arm64: efi: Improve device tree discovery")
>> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
>> ---
>>  lib/efi.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/lib/efi.c b/lib/efi.c
>> index 5314eaa81e66..124e77685230 100644
>> --- a/lib/efi.c
>> +++ b/lib/efi.c
>> @@ -328,6 +328,10 @@ static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
>>  		return NULL;
>>  	}
>>  
>> +#ifdef __x86_64__
>> +	return fdt;
>> +#endif
>> +
>>  	return fdt_check_header(fdt) == 0 ? fdt : NULL;
>>  }
>>  
>> -- 
>> 2.34.1
>>

Re: [kvm-unit-tests RFC PATCH 2/3] x86/efi: Retry call to efi exit boot services

[Paluri, PavanKumar]

On 3/26/2024 3:57 AM, Andrew Jones wrote:
> On Mon, Mar 25, 2024 at 04:36:22PM -0500, Pavan Kumar Paluri wrote:
>> In some cases, KUT guest might fail to exit boot services due to a
>> possible memory map update that might have taken place between
>> efi_get_memory_map() and efi_exit_boot_services() calls. As per UEFI
>> spec 2.10 (Section 7.4.6 EFI_BOOT_SERVICES.ExitBootServices()), we need
>> to update the memory map and retry call to exit boot
>> services.
>>
>> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
>> ---
>>  lib/efi.c | 23 ++++++++++++++++++-----
>>  1 file changed, 18 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/efi.c b/lib/efi.c
>> index 124e77685230..9d066bfad0b6 100644
>> --- a/lib/efi.c
>> +++ b/lib/efi.c
>> @@ -458,14 +458,27 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
>>  	}
>>  #endif
>>  
>> -	/* 
>> +	/*
>>  	 * Exit EFI boot services, let kvm-unit-tests take full control of the
>> -	 * guest
>> +	 * guest.
>>  	 */
>>  	status = efi_exit_boot_services(handle, &efi_bootinfo.mem_map);
>> -	if (status != EFI_SUCCESS) {
>> -		printf("Failed to exit boot services\n");
>> -		goto efi_main_error;
>> +
>> +	/*
>> +	 * There is a possibility that memory map might have changed
>> +	 * between efi_get_memory_map() and efi_exit_boot_services in
>> +	 * which case status is EFI_INVALID_PARAMETER. As per UEFI spec
>> +	 * 2.10, we need to get the updated memory map and try again.
>> +	 */
>> +	if (status == EFI_INVALID_PARAMETER) {
> 
> Shouldn't we loop on this? The spec doesn't make it clear that the second
> try should always work.
> 
Indeed, will plug in a do-while to loop on it.

Thanks,
Pavan
>> +		efi_get_memory_map(&efi_bootinfo.mem_map);
>> +
>> +		status = efi_exit_boot_services(handle,
>> +						&efi_bootinfo.mem_map);
>> +		if (status != EFI_SUCCESS) {
>> +			printf("Failed to exit boot services\n");
>> +			goto efi_main_error;
>> +		}
>>  	}
>>  
>>  	/* Set up arch-specific resources */
>> -- 
>> 2.34.1
>>
> 
> Thanks,
> drew

Re: [kvm-unit-tests RFC PATCH 2/3] x86/efi: Retry call to efi exit boot services

[Michael Roth]

On Mon, Mar 25, 2024 at 04:36:22PM -0500, Pavan Kumar Paluri wrote:
> In some cases, KUT guest might fail to exit boot services due to a
> possible memory map update that might have taken place between
> efi_get_memory_map() and efi_exit_boot_services() calls. As per UEFI
> spec 2.10 (Section 7.4.6 EFI_BOOT_SERVICES.ExitBootServices()), we need
> to update the memory map and retry call to exit boot
> services.
> 
> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
> ---
>  lib/efi.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/efi.c b/lib/efi.c
> index 124e77685230..9d066bfad0b6 100644
> --- a/lib/efi.c
> +++ b/lib/efi.c
> @@ -458,14 +458,27 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
>  	}
>  #endif
>  
> -	/* 
> +	/*
>  	 * Exit EFI boot services, let kvm-unit-tests take full control of the
> -	 * guest
> +	 * guest.
>  	 */
>  	status = efi_exit_boot_services(handle, &efi_bootinfo.mem_map);
> -	if (status != EFI_SUCCESS) {
> -		printf("Failed to exit boot services\n");
> -		goto efi_main_error;

With this change, error codes other than EFI_INVALID_PARAMETER are only
handled if the first failure is EFI_INVALID_PARAMETER. Need to to re-add
the previous handling for when the first EBS failure is something other
than EFI_INVALID_PARAMETER.

-Mike

> +
> +	/*
> +	 * There is a possibility that memory map might have changed
> +	 * between efi_get_memory_map() and efi_exit_boot_services in
> +	 * which case status is EFI_INVALID_PARAMETER. As per UEFI spec
> +	 * 2.10, we need to get the updated memory map and try again.
> +	 */
> +	if (status == EFI_INVALID_PARAMETER) {
> +		efi_get_memory_map(&efi_bootinfo.mem_map);
> +
> +		status = efi_exit_boot_services(handle,
> +						&efi_bootinfo.mem_map);
> +		if (status != EFI_SUCCESS) {
> +			printf("Failed to exit boot services\n");
> +			goto efi_main_error;
> +		}
>  	}
>  
>  	/* Set up arch-specific resources */
> -- 
> 2.34.1
> 

Re: [kvm-unit-tests RFC PATCH 2/3] x86/efi: Retry call to efi exit boot services

[Paluri, PavanKumar]

Hi Mike,

On 3/26/2024 8:38 AM, Michael Roth wrote:
> On Mon, Mar 25, 2024 at 04:36:22PM -0500, Pavan Kumar Paluri wrote:
>> In some cases, KUT guest might fail to exit boot services due to a
>> possible memory map update that might have taken place between
>> efi_get_memory_map() and efi_exit_boot_services() calls. As per UEFI
>> spec 2.10 (Section 7.4.6 EFI_BOOT_SERVICES.ExitBootServices()), we need
>> to update the memory map and retry call to exit boot
>> services.
>>
>> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
>> ---
>>  lib/efi.c | 23 ++++++++++++++++++-----
>>  1 file changed, 18 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/efi.c b/lib/efi.c
>> index 124e77685230..9d066bfad0b6 100644
>> --- a/lib/efi.c
>> +++ b/lib/efi.c
>> @@ -458,14 +458,27 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
>>  	}
>>  #endif
>>  
>> -	/* 
>> +	/*
>>  	 * Exit EFI boot services, let kvm-unit-tests take full control of the
>> -	 * guest
>> +	 * guest.
>>  	 */
>>  	status = efi_exit_boot_services(handle, &efi_bootinfo.mem_map);
>> -	if (status != EFI_SUCCESS) {
>> -		printf("Failed to exit boot services\n");
>> -		goto efi_main_error;
> 
> With this change, error codes other than EFI_INVALID_PARAMETER are only
> handled if the first failure is EFI_INVALID_PARAMETER. Need to to re-add
> the previous handling for when the first EBS failure is something other
> than EFI_INVALID_PARAMETER.
> 
But the status codes that could be returned from
efi_exit_boot_services() are only EFI_INVALID_PARAMETER/EFI_SUCCESS [1].

[1] UEFI 2.10 Section 7.4.6.

Thanks,
Pavan

> -Mike
> 
>> +
>> +	/*
>> +	 * There is a possibility that memory map might have changed
>> +	 * between efi_get_memory_map() and efi_exit_boot_services in
>> +	 * which case status is EFI_INVALID_PARAMETER. As per UEFI spec
>> +	 * 2.10, we need to get the updated memory map and try again.
>> +	 */
>> +	if (status == EFI_INVALID_PARAMETER) {
>> +		efi_get_memory_map(&efi_bootinfo.mem_map);
>> +
>> +		status = efi_exit_boot_services(handle,
>> +						&efi_bootinfo.mem_map);
>> +		if (status != EFI_SUCCESS) {
>> +			printf("Failed to exit boot services\n");
>> +			goto efi_main_error;
>> +		}
>>  	}
>>  
>>  	/* Set up arch-specific resources */
>> -- 
>> 2.34.1
>>

Re: [kvm-unit-tests RFC PATCH 2/3] x86/efi: Retry call to efi exit boot services

[Michael Roth]

On Tue, Mar 26, 2024 at 08:45:53AM -0500, Paluri, PavanKumar wrote:
> Hi Mike,
> 
> On 3/26/2024 8:38 AM, Michael Roth wrote:
> > On Mon, Mar 25, 2024 at 04:36:22PM -0500, Pavan Kumar Paluri wrote:
> >> In some cases, KUT guest might fail to exit boot services due to a
> >> possible memory map update that might have taken place between
> >> efi_get_memory_map() and efi_exit_boot_services() calls. As per UEFI
> >> spec 2.10 (Section 7.4.6 EFI_BOOT_SERVICES.ExitBootServices()), we need
> >> to update the memory map and retry call to exit boot
> >> services.
> >>
> >> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
> >> ---
> >>  lib/efi.c | 23 ++++++++++++++++++-----
> >>  1 file changed, 18 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/lib/efi.c b/lib/efi.c
> >> index 124e77685230..9d066bfad0b6 100644
> >> --- a/lib/efi.c
> >> +++ b/lib/efi.c
> >> @@ -458,14 +458,27 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> >>  	}
> >>  #endif
> >>  
> >> -	/* 
> >> +	/*
> >>  	 * Exit EFI boot services, let kvm-unit-tests take full control of the
> >> -	 * guest
> >> +	 * guest.
> >>  	 */
> >>  	status = efi_exit_boot_services(handle, &efi_bootinfo.mem_map);
> >> -	if (status != EFI_SUCCESS) {
> >> -		printf("Failed to exit boot services\n");
> >> -		goto efi_main_error;
> > 
> > With this change, error codes other than EFI_INVALID_PARAMETER are only
> > handled if the first failure is EFI_INVALID_PARAMETER. Need to to re-add
> > the previous handling for when the first EBS failure is something other
> > than EFI_INVALID_PARAMETER.
> > 
> But the status codes that could be returned from
> efi_exit_boot_services() are only EFI_INVALID_PARAMETER/EFI_SUCCESS [1].
> 
> [1] UEFI 2.10 Section 7.4.6.

New error codes can be added over time. Also, the block you added handles
!EFI_SUCCESS generally for the 2nd call, so for consistency it makes sense
to continue handling it similarly for the 1st call as well.

-Mike

> 
> Thanks,
> Pavan
> 
> > -Mike
> > 
> >> +
> >> +	/*
> >> +	 * There is a possibility that memory map might have changed
> >> +	 * between efi_get_memory_map() and efi_exit_boot_services in
> >> +	 * which case status is EFI_INVALID_PARAMETER. As per UEFI spec
> >> +	 * 2.10, we need to get the updated memory map and try again.
> >> +	 */
> >> +	if (status == EFI_INVALID_PARAMETER) {
> >> +		efi_get_memory_map(&efi_bootinfo.mem_map);
> >> +
> >> +		status = efi_exit_boot_services(handle,
> >> +						&efi_bootinfo.mem_map);
> >> +		if (status != EFI_SUCCESS) {
> >> +			printf("Failed to exit boot services\n");
> >> +			goto efi_main_error;
> >> +		}
> >>  	}
> >>  
> >>  	/* Set up arch-specific resources */
> >> -- 
> >> 2.34.1
> >>

Re: [kvm-unit-tests RFC PATCH 3/3] x86 AMD SEV-ES: Setup a new page table and install level 1 PTEs

[Tom Lendacky]

On 3/25/24 16:36, Pavan Kumar Paluri wrote:
> KUT's UEFI tests don't currently have support for page allocation.
> SEV-ES/SNP tests will need this later, so the support for page
> allocation is provided via setup_vm().
> 
> SEV-ES/SNP guest uses GHCB page to communicate with the host. Such a
> page should remain unencrypted (its c-bit should be unset). Therefore,
> call setup_ghcb_pte() in the path of setup_vm() to make sure c-bit of
> GHCB's pte is unset.

This looks like it should be 2 separate patches. One for supporting page 
allocation and one for setting the GHCB page attributes.

Thanks,
Tom

> 
> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
> ---
>   lib/x86/vm.c  | 6 ++++++
>   x86/amd_sev.c | 7 +++++++
>   2 files changed, 13 insertions(+)
> 
> diff --git a/lib/x86/vm.c b/lib/x86/vm.c
> index 90f73fbb2dfd..ce2063aee75d 100644
> --- a/lib/x86/vm.c
> +++ b/lib/x86/vm.c
> @@ -3,6 +3,7 @@
>   #include "vmalloc.h"
>   #include "alloc_page.h"
>   #include "smp.h"
> +#include "amd_sev.h"
>   
>   static pteval_t pte_opt_mask;
>   
> @@ -197,6 +198,11 @@ void *setup_mmu(phys_addr_t end_of_memory, void *opt_mask)
>       init_alloc_vpage((void*)(3ul << 30));
>   #endif
>   
> +#ifdef CONFIG_EFI
> +	if (amd_sev_es_enabled())
> +		setup_ghcb_pte(cr3);
> +#endif
> +
>       write_cr3(virt_to_phys(cr3));
>   #ifndef __x86_64__
>       write_cr4(X86_CR4_PSE);
> diff --git a/x86/amd_sev.c b/x86/amd_sev.c
> index 7757d4f85b7a..03636e581dfe 100644
> --- a/x86/amd_sev.c
> +++ b/x86/amd_sev.c
> @@ -14,6 +14,8 @@
>   #include "x86/processor.h"
>   #include "x86/amd_sev.h"
>   #include "msr.h"
> +#include "x86/vm.h"
> +#include "alloc_page.h"
>   
>   #define EXIT_SUCCESS 0
>   #define EXIT_FAILURE 1
> @@ -89,9 +91,14 @@ static void test_stringio(void)
>   int main(void)
>   {
>   	int rtn;
> +	unsigned long *vaddr;
>   	rtn = test_sev_activation();
>   	report(rtn == EXIT_SUCCESS, "SEV activation test.");
>   	test_sev_es_activation();
>   	test_stringio();
> +	setup_vm();
> +	vaddr = alloc_page();
> +	if (!vaddr)
> +		assert_msg(vaddr, "Page allocation Failure");
>   	return report_summary();
>   }

Re: [kvm-unit-tests RFC PATCH 1/3] x86 EFI: Bypass call to fdt_check_header()

[Paluri, PavanKumar]

On 3/26/2024 3:51 AM, Andrew Jones wrote:
> On Mon, Mar 25, 2024 at 04:36:21PM -0500, Pavan Kumar Paluri wrote:
>> Issuing a call to fdt_check_header() prevents running any of x86 UEFI
>> enabled tests. Bypass this call for x86 in order to enable UEFI
>> supported tests for KUT x86 arch.
> 
> Ouch! Sorry about that. I think I prefer something like below, though.
> 
> Thanks,
> drew
> 
> diff --git a/lib/efi.c b/lib/efi.c
> index 5314eaa81e66..335b66d26092 100644
> --- a/lib/efi.c
> +++ b/lib/efi.c
> @@ -312,6 +312,7 @@ static void* efi_get_var(efi_handle_t handle, struct efi_loaded_image_64 *image,
>         return val;
>  }
>  
> +#if defined(__aarch64__) || defined(__riscv)

I just realized, I had to move this line all the way upto efi_get_var in
order to avoid the following compilation warnings/errors (for x86 build):

lib/efi.c:299:14: error: ‘efi_get_var’ defined but not used
[-Werror=unused-function]
  299 | static void* efi_get_var(efi_handle_t handle, struct
efi_loaded_image_64 *image, efi_char16_t *var)
      |              ^~~~~~~~~~~
lib/efi.c:210:13: error: ‘efi_load_image’ defined but not used
[-Werror=unused-function]
  210 | static void efi_load_image(efi_handle_t handle, struct
efi_loaded_image_64 *image, void **data,
      |             ^~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Diff after applying the change below:

diff --git a/lib/efi.c b/lib/efi.c
index 5314eaa81e66..8a74a22834a4 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -204,6 +204,7 @@ static char *efi_convert_cmdline(struct
efi_loaded_image_64 *image, int *cmd_lin
        return (char *)cmdline_addr;
 }

+#if defined(__aarch64__) || defined(__riscv)
 /*
  * Open the file and read it into a buffer.
  */
@@ -330,6 +331,12 @@ static void *efi_get_fdt(efi_handle_t handle,
struct efi_loaded_image_64 *image)

        return fdt_check_header(fdt) == 0 ? fdt : NULL;
 }
+#else
+static void *efi_get_fdt(efi_handle_t handle, struct
efi_loaded_image_64 *image)
+{
+       return NULL;
+}
+#endif

 static const struct {
        struct efi_vendor_dev_path      vendor;


>  static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
>  {
>         efi_char16_t var[] = ENV_VARNAME_DTBFILE;
> @@ -330,6 +331,12 @@ static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
>  
>         return fdt_check_header(fdt) == 0 ? fdt : NULL;
>  }
> +#else
> +static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
> +{
> +       return NULL;
> +}
> +#endif
>  
>  static const struct {
>         struct efi_vendor_dev_path      vendor;
> 

Thanks,
Pavan

Re: [kvm-unit-tests RFC PATCH 3/3] x86 AMD SEV-ES: Setup a new page table and install level 1 PTEs

[Paluri, PavanKumar]

On 3/26/2024 9:01 AM, Tom Lendacky wrote:
> On 3/25/24 16:36, Pavan Kumar Paluri wrote:
>> KUT's UEFI tests don't currently have support for page allocation.
>> SEV-ES/SNP tests will need this later, so the support for page
>> allocation is provided via setup_vm().
>>
>> SEV-ES/SNP guest uses GHCB page to communicate with the host. Such a
>> page should remain unencrypted (its c-bit should be unset). Therefore,
>> call setup_ghcb_pte() in the path of setup_vm() to make sure c-bit of
>> GHCB's pte is unset.
> 
> This looks like it should be 2 separate patches. One for supporting page
> allocation and one for setting the GHCB page attributes.
> 

Sure, I will separate this into 2 patches, the GHCB page attribute patch
followed by the page allocation support.

Thanks,
Pavan
> Thanks,
> Tom
>