* [kvm-unit-tests PATCH v3 1/4] x86 EFI: Bypass call to fdt_check_header()
@ 2024-03-28 15:21 Pavan Kumar Paluri
2024-03-28 15:21 ` [kvm-unit-tests PATCH v3 2/4] x86/efi: Retry call to efi exit boot services Pavan Kumar Paluri
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Pavan Kumar Paluri @ 2024-03-28 15:21 UTC (permalink / raw
To: kvm
Cc: andrew.jones, pbonzini, thomas.lendacky, michael.roth,
Pavan Kumar Paluri
Issuing a call to fdt_check_header() prevents running any of x86 UEFI
enabled tests. Bypass this call for x86 and also calls to
efi_load_image(), efi_grow_buffer(), efi_get_var() 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>
Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
---
lib/efi.c | 7 +++++++
1 file changed, 7 insertions(+)
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;
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [kvm-unit-tests PATCH v3 2/4] x86/efi: Retry call to efi exit boot services
2024-03-28 15:21 [kvm-unit-tests PATCH v3 1/4] x86 EFI: Bypass call to fdt_check_header() Pavan Kumar Paluri
@ 2024-03-28 15:21 ` Pavan Kumar Paluri
2024-03-28 16:33 ` Andrew Jones
2024-03-28 15:21 ` [kvm-unit-tests PATCH v3 3/4] x86 AMD SEV-ES: Set GHCB page attributes for a new page table Pavan Kumar Paluri
2024-03-28 15:21 ` [kvm-unit-tests PATCH v3 4/4] x86 AMD SEV-ES: Setup a new page table and install level-1 PTEs Pavan Kumar Paluri
2 siblings, 1 reply; 6+ messages in thread
From: Pavan Kumar Paluri @ 2024-03-28 15:21 UTC (permalink / raw
To: kvm
Cc: andrew.jones, pbonzini, thomas.lendacky, michael.roth,
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 keep trying to update the memory map and calls to exit boot
services as long as case status is EFI_INVALID_PARAMETER. Keep freeing
the old memory map before obtaining new memory map via
efi_get_memory_map() in case of exit boot services failure.
Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
---
lib/efi.c | 34 ++++++++++++++++++++--------------
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/lib/efi.c b/lib/efi.c
index 8a74a22834a4..d2569b22b4f2 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -406,8 +406,8 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
efi_system_table = sys_tab;
/* Memory map struct values */
- efi_memory_desc_t *map = NULL;
- unsigned long map_size = 0, desc_size = 0, key = 0, buff_size = 0;
+ efi_memory_desc_t *map;
+ unsigned long map_size, desc_size, key, buff_size;
u32 desc_ver;
/* Helper variables needed to get the cmdline */
@@ -446,13 +446,6 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
efi_bootinfo.mem_map.key_ptr = &key;
efi_bootinfo.mem_map.buff_size = &buff_size;
- /* Get EFI memory map */
- status = efi_get_memory_map(&efi_bootinfo.mem_map);
- if (status != EFI_SUCCESS) {
- printf("Failed to get memory map\n");
- goto efi_main_error;
- }
-
#ifdef __riscv
status = efi_get_boot_hartid();
if (status != EFI_SUCCESS) {
@@ -461,11 +454,24 @@ 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
- */
- status = efi_exit_boot_services(handle, &efi_bootinfo.mem_map);
+ status = EFI_INVALID_PARAMETER;
+ while (status == EFI_INVALID_PARAMETER) {
+ /* Get EFI memory map */
+ status = efi_get_memory_map(&efi_bootinfo.mem_map);
+ if (status != EFI_SUCCESS) {
+ printf("Failed to get memory map\n");
+ goto efi_main_error;
+ }
+ /*
+ * Exit EFI boot services, let kvm-unit-tests take full
+ * control of the guest.
+ */
+ status = efi_exit_boot_services(handle,
+ &efi_bootinfo.mem_map);
+ if (status == EFI_INVALID_PARAMETER)
+ efi_free_pool(*efi_bootinfo.mem_map.map);
+ }
+
if (status != EFI_SUCCESS) {
printf("Failed to exit boot services\n");
goto efi_main_error;
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [kvm-unit-tests PATCH v3 3/4] x86 AMD SEV-ES: Set GHCB page attributes for a new page table
2024-03-28 15:21 [kvm-unit-tests PATCH v3 1/4] x86 EFI: Bypass call to fdt_check_header() Pavan Kumar Paluri
2024-03-28 15:21 ` [kvm-unit-tests PATCH v3 2/4] x86/efi: Retry call to efi exit boot services Pavan Kumar Paluri
@ 2024-03-28 15:21 ` Pavan Kumar Paluri
2024-03-28 15:21 ` [kvm-unit-tests PATCH v3 4/4] x86 AMD SEV-ES: Setup a new page table and install level-1 PTEs Pavan Kumar Paluri
2 siblings, 0 replies; 6+ messages in thread
From: Pavan Kumar Paluri @ 2024-03-28 15:21 UTC (permalink / raw
To: kvm
Cc: andrew.jones, pbonzini, thomas.lendacky, michael.roth,
Pavan Kumar Paluri
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, for a new page table that will be setup as a part
of page allocation for SEV-ES/SNP tests later on.
Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
---
lib/x86/vm.c | 6 ++++++
1 file changed, 6 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);
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [kvm-unit-tests PATCH v3 4/4] x86 AMD SEV-ES: Setup a new page table and install level-1 PTEs
2024-03-28 15:21 [kvm-unit-tests PATCH v3 1/4] x86 EFI: Bypass call to fdt_check_header() Pavan Kumar Paluri
2024-03-28 15:21 ` [kvm-unit-tests PATCH v3 2/4] x86/efi: Retry call to efi exit boot services Pavan Kumar Paluri
2024-03-28 15:21 ` [kvm-unit-tests PATCH v3 3/4] x86 AMD SEV-ES: Set GHCB page attributes for a new page table Pavan Kumar Paluri
@ 2024-03-28 15:21 ` Pavan Kumar Paluri
2 siblings, 0 replies; 6+ messages in thread
From: Pavan Kumar Paluri @ 2024-03-28 15:21 UTC (permalink / raw
To: kvm
Cc: andrew.jones, pbonzini, thomas.lendacky, michael.roth,
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().
Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
---
x86/amd_sev.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/x86/amd_sev.c b/x86/amd_sev.c
index 7757d4f85b7a..bdf14055e46a 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH v3 2/4] x86/efi: Retry call to efi exit boot services
2024-03-28 15:21 ` [kvm-unit-tests PATCH v3 2/4] x86/efi: Retry call to efi exit boot services Pavan Kumar Paluri
@ 2024-03-28 16:33 ` Andrew Jones
2024-03-28 21:48 ` Paluri, PavanKumar
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Jones @ 2024-03-28 16:33 UTC (permalink / raw
To: Pavan Kumar Paluri; +Cc: kvm, pbonzini, thomas.lendacky, michael.roth
On Thu, Mar 28, 2024 at 10:21:10AM -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 keep trying to update the memory map and calls to exit boot
> services as long as case status is EFI_INVALID_PARAMETER. Keep freeing
> the old memory map before obtaining new memory map via
> efi_get_memory_map() in case of exit boot services failure.
>
> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
> ---
> lib/efi.c | 34 ++++++++++++++++++++--------------
> 1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/lib/efi.c b/lib/efi.c
> index 8a74a22834a4..d2569b22b4f2 100644
> --- a/lib/efi.c
> +++ b/lib/efi.c
> @@ -406,8 +406,8 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> efi_system_table = sys_tab;
>
> /* Memory map struct values */
> - efi_memory_desc_t *map = NULL;
> - unsigned long map_size = 0, desc_size = 0, key = 0, buff_size = 0;
> + efi_memory_desc_t *map;
> + unsigned long map_size, desc_size, key, buff_size;
> u32 desc_ver;
>
> /* Helper variables needed to get the cmdline */
> @@ -446,13 +446,6 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> efi_bootinfo.mem_map.key_ptr = &key;
> efi_bootinfo.mem_map.buff_size = &buff_size;
>
> - /* Get EFI memory map */
> - status = efi_get_memory_map(&efi_bootinfo.mem_map);
> - if (status != EFI_SUCCESS) {
> - printf("Failed to get memory map\n");
> - goto efi_main_error;
> - }
> -
> #ifdef __riscv
> status = efi_get_boot_hartid();
> if (status != EFI_SUCCESS) {
> @@ -461,11 +454,24 @@ 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
> - */
> - status = efi_exit_boot_services(handle, &efi_bootinfo.mem_map);
> + status = EFI_INVALID_PARAMETER;
> + while (status == EFI_INVALID_PARAMETER) {
> + /* Get EFI memory map */
I tried to get rid of this comment since it states the exact same thing
as the function name below does.
> + status = efi_get_memory_map(&efi_bootinfo.mem_map);
> + if (status != EFI_SUCCESS) {
> + printf("Failed to get memory map\n");
> + goto efi_main_error;
> + }
> + /*
> + * Exit EFI boot services, let kvm-unit-tests take full
> + * control of the guest.
> + */
> + status = efi_exit_boot_services(handle,
> + &efi_bootinfo.mem_map);
We have 100 char lines (and that's just a soft limit) so this would look
better sticking out.
> + if (status == EFI_INVALID_PARAMETER)
> + efi_free_pool(*efi_bootinfo.mem_map.map);
> + }
> +
> if (status != EFI_SUCCESS) {
> printf("Failed to exit boot services\n");
> goto efi_main_error;
> --
> 2.34.1
>
Besides the nits,
Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
(A general comment for the series is that we're on v3 but there's no
changelog anywhere. Please use cover letters for a series and then
put the changelog in the cover letter.)
Thanks,
drew
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH v3 2/4] x86/efi: Retry call to efi exit boot services
2024-03-28 16:33 ` Andrew Jones
@ 2024-03-28 21:48 ` Paluri, PavanKumar
0 siblings, 0 replies; 6+ messages in thread
From: Paluri, PavanKumar @ 2024-03-28 21:48 UTC (permalink / raw
To: Andrew Jones; +Cc: kvm, pbonzini, thomas.lendacky, michael.roth
On 3/28/2024 11:33 AM, Andrew Jones wrote:
> On Thu, Mar 28, 2024 at 10:21:10AM -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 keep trying to update the memory map and calls to exit boot
>> services as long as case status is EFI_INVALID_PARAMETER. Keep freeing
>> the old memory map before obtaining new memory map via
>> efi_get_memory_map() in case of exit boot services failure.
>>
>> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
>> ---
>> lib/efi.c | 34 ++++++++++++++++++++--------------
>> 1 file changed, 20 insertions(+), 14 deletions(-)
>>
>> diff --git a/lib/efi.c b/lib/efi.c
>> index 8a74a22834a4..d2569b22b4f2 100644
>> --- a/lib/efi.c
>> +++ b/lib/efi.c
>> @@ -406,8 +406,8 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
>> efi_system_table = sys_tab;
>>
>> /* Memory map struct values */
>> - efi_memory_desc_t *map = NULL;
>> - unsigned long map_size = 0, desc_size = 0, key = 0, buff_size = 0;
>> + efi_memory_desc_t *map;
>> + unsigned long map_size, desc_size, key, buff_size;
>> u32 desc_ver;
>>
>> /* Helper variables needed to get the cmdline */
>> @@ -446,13 +446,6 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
>> efi_bootinfo.mem_map.key_ptr = &key;
>> efi_bootinfo.mem_map.buff_size = &buff_size;
>>
>> - /* Get EFI memory map */
>> - status = efi_get_memory_map(&efi_bootinfo.mem_map);
>> - if (status != EFI_SUCCESS) {
>> - printf("Failed to get memory map\n");
>> - goto efi_main_error;
>> - }
>> -
>> #ifdef __riscv
>> status = efi_get_boot_hartid();
>> if (status != EFI_SUCCESS) {
>> @@ -461,11 +454,24 @@ 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
>> - */
>> - status = efi_exit_boot_services(handle, &efi_bootinfo.mem_map);
>> + status = EFI_INVALID_PARAMETER;
>> + while (status == EFI_INVALID_PARAMETER) {
>> + /* Get EFI memory map */
>
> I tried to get rid of this comment since it states the exact same thing
> as the function name below does.
>
Will make the change.
>> + status = efi_get_memory_map(&efi_bootinfo.mem_map);
>> + if (status != EFI_SUCCESS) {
>> + printf("Failed to get memory map\n");
>> + goto efi_main_error;
>> + }
>> + /*
>> + * Exit EFI boot services, let kvm-unit-tests take full
>> + * control of the guest.
>> + */
>> + status = efi_exit_boot_services(handle,
>> + &efi_bootinfo.mem_map);
>
> We have 100 char lines (and that's just a soft limit) so this would look
> better sticking out.
>
Will do.
>> + if (status == EFI_INVALID_PARAMETER)
>> + efi_free_pool(*efi_bootinfo.mem_map.map);
>> + }
>> +
>> if (status != EFI_SUCCESS) {
>> printf("Failed to exit boot services\n");
>> goto efi_main_error;
>> --
>> 2.34.1
>>
>
> Besides the nits,
>
> Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
>
> (A general comment for the series is that we're on v3 but there's no
> changelog anywhere. Please use cover letters for a series and then
> put the changelog in the cover letter.)
>
Ah, I missed out on that. I will include a cover-letter for v4 and also
plan to drop patches 3 & 4 and send them separately as they aren't very
relevant to UEFI fixes and for easier upstreaming.
Thanks,
Pavan
> Thanks,
> drew
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-03-28 21:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-28 15:21 [kvm-unit-tests PATCH v3 1/4] x86 EFI: Bypass call to fdt_check_header() Pavan Kumar Paluri
2024-03-28 15:21 ` [kvm-unit-tests PATCH v3 2/4] x86/efi: Retry call to efi exit boot services Pavan Kumar Paluri
2024-03-28 16:33 ` Andrew Jones
2024-03-28 21:48 ` Paluri, PavanKumar
2024-03-28 15:21 ` [kvm-unit-tests PATCH v3 3/4] x86 AMD SEV-ES: Set GHCB page attributes for a new page table Pavan Kumar Paluri
2024-03-28 15:21 ` [kvm-unit-tests PATCH v3 4/4] x86 AMD SEV-ES: Setup a new page table and install level-1 PTEs Pavan Kumar Paluri
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).