* [kvm-unit-tests PATCH v2 01/17] x86 UEFI: Copy code from Linux
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-09-20 14:33 ` Paolo Bonzini
2021-09-21 16:33 ` Andrew Jones
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 02/17] x86 UEFI: Implement UEFI function calls Zixuan Wang
` (16 subsequent siblings)
17 siblings, 2 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
From: Varad Gautam <varad.gautam@suse.com>
Copy UEFI-related definitions from Linux, so the follow-up commits can
develop UEFI function calls based on these definitions, without relying
on GNU-EFI library.
Signed-off-by: Varad Gautam <varad.gautam@suse.com>
---
lib/linux/uefi.h | 518 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 518 insertions(+)
create mode 100644 lib/linux/uefi.h
diff --git a/lib/linux/uefi.h b/lib/linux/uefi.h
new file mode 100644
index 0000000..567cddc
--- /dev/null
+++ b/lib/linux/uefi.h
@@ -0,0 +1,518 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Relevant definitions from linux/efi.h. */
+
+#ifndef __LINUX_UEFI_H
+#define __LINUX_UEFI_H
+
+#define BITS_PER_LONG 64
+
+#define EFI_SUCCESS 0
+#define EFI_LOAD_ERROR ( 1 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_INVALID_PARAMETER ( 2 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_UNSUPPORTED ( 3 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_BAD_BUFFER_SIZE ( 4 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_BUFFER_TOO_SMALL ( 5 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_NOT_READY ( 6 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_DEVICE_ERROR ( 7 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_WRITE_PROTECTED ( 8 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_OUT_OF_RESOURCES ( 9 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_NOT_FOUND (14 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_TIMEOUT (18 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_ABORTED (21 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_SECURITY_VIOLATION (26 | (1UL << (BITS_PER_LONG-1)))
+
+typedef unsigned long efi_status_t;
+typedef u8 efi_bool_t;
+typedef u16 efi_char16_t; /* UNICODE character */
+typedef u64 efi_physical_addr_t;
+typedef void *efi_handle_t;
+
+#define __efiapi __attribute__((ms_abi))
+
+/*
+ * The UEFI spec and EDK2 reference implementation both define EFI_GUID as
+ * struct { u32 a; u16; b; u16 c; u8 d[8]; }; and so the implied alignment
+ * is 32 bits not 8 bits like our guid_t. In some cases (i.e., on 32-bit ARM),
+ * this means that firmware services invoked by the kernel may assume that
+ * efi_guid_t* arguments are 32-bit aligned, and use memory accessors that
+ * do not tolerate misalignment. So let's set the minimum alignment to 32 bits.
+ *
+ * Note that the UEFI spec as well as some comments in the EDK2 code base
+ * suggest that EFI_GUID should be 64-bit aligned, but this appears to be
+ * a mistake, given that no code seems to exist that actually enforces that
+ * or relies on it.
+ */
+typedef struct {
+ u8 b[16];
+} guid_t;
+typedef guid_t efi_guid_t;
+
+#define EFI_GUID(a, b, c, d...) (efi_guid_t){ { \
+ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \
+ (b) & 0xff, ((b) >> 8) & 0xff, \
+ (c) & 0xff, ((c) >> 8) & 0xff, d } }
+
+/*
+ * Generic EFI table header
+ */
+typedef struct {
+ u64 signature;
+ u32 revision;
+ u32 headersize;
+ u32 crc32;
+ u32 reserved;
+} efi_table_hdr_t;
+
+/*
+ * Memory map descriptor:
+ */
+
+/* Memory types: */
+#define EFI_RESERVED_TYPE 0
+#define EFI_LOADER_CODE 1
+#define EFI_LOADER_DATA 2
+#define EFI_BOOT_SERVICES_CODE 3
+#define EFI_BOOT_SERVICES_DATA 4
+#define EFI_RUNTIME_SERVICES_CODE 5
+#define EFI_RUNTIME_SERVICES_DATA 6
+#define EFI_CONVENTIONAL_MEMORY 7
+#define EFI_UNUSABLE_MEMORY 8
+#define EFI_ACPI_RECLAIM_MEMORY 9
+#define EFI_ACPI_MEMORY_NVS 10
+#define EFI_MEMORY_MAPPED_IO 11
+#define EFI_MEMORY_MAPPED_IO_PORT_SPACE 12
+#define EFI_PAL_CODE 13
+#define EFI_PERSISTENT_MEMORY 14
+#define EFI_MAX_MEMORY_TYPE 15
+
+/* Attribute values: */
+#define EFI_MEMORY_UC ((u64)0x0000000000000001ULL) /* uncached */
+#define EFI_MEMORY_WC ((u64)0x0000000000000002ULL) /* write-coalescing */
+#define EFI_MEMORY_WT ((u64)0x0000000000000004ULL) /* write-through */
+#define EFI_MEMORY_WB ((u64)0x0000000000000008ULL) /* write-back */
+#define EFI_MEMORY_UCE ((u64)0x0000000000000010ULL) /* uncached, exported */
+#define EFI_MEMORY_WP ((u64)0x0000000000001000ULL) /* write-protect */
+#define EFI_MEMORY_RP ((u64)0x0000000000002000ULL) /* read-protect */
+#define EFI_MEMORY_XP ((u64)0x0000000000004000ULL) /* execute-protect */
+#define EFI_MEMORY_NV ((u64)0x0000000000008000ULL) /* non-volatile */
+#define EFI_MEMORY_MORE_RELIABLE \
+ ((u64)0x0000000000010000ULL) /* higher reliability */
+#define EFI_MEMORY_RO ((u64)0x0000000000020000ULL) /* read-only */
+#define EFI_MEMORY_SP ((u64)0x0000000000040000ULL) /* soft reserved */
+#define EFI_MEMORY_CPU_CRYPTO ((u64)0x0000000000080000ULL) /* supports encryption */
+#define EFI_MEMORY_RUNTIME ((u64)0x8000000000000000ULL) /* range requires runtime mapping */
+#define EFI_MEMORY_DESCRIPTOR_VERSION 1
+
+#define EFI_PAGE_SHIFT 12
+#define EFI_PAGE_SIZE (1UL << EFI_PAGE_SHIFT)
+#define EFI_PAGES_MAX (U64_MAX >> EFI_PAGE_SHIFT)
+
+typedef struct {
+ u32 type;
+ u32 pad;
+ u64 phys_addr;
+ u64 virt_addr;
+ u64 num_pages;
+ u64 attribute;
+} efi_memory_desc_t;
+
+typedef struct {
+ efi_guid_t guid;
+ u32 headersize;
+ u32 flags;
+ u32 imagesize;
+} efi_capsule_header_t;
+
+/*
+ * EFI capsule flags
+ */
+#define EFI_CAPSULE_PERSIST_ACROSS_RESET 0x00010000
+#define EFI_CAPSULE_POPULATE_SYSTEM_TABLE 0x00020000
+#define EFI_CAPSULE_INITIATE_RESET 0x00040000
+
+struct capsule_info {
+ efi_capsule_header_t header;
+ efi_capsule_header_t *capsule;
+ int reset_type;
+ long index;
+ size_t count;
+ size_t total_size;
+ struct page **pages;
+ phys_addr_t *phys;
+ size_t page_bytes_remain;
+};
+
+int __efi_capsule_setup_info(struct capsule_info *cap_info);
+
+/*
+ * Types and defines for Time Services
+ */
+#define EFI_TIME_ADJUST_DAYLIGHT 0x1
+#define EFI_TIME_IN_DAYLIGHT 0x2
+#define EFI_UNSPECIFIED_TIMEZONE 0x07ff
+
+typedef struct {
+ u16 year;
+ u8 month;
+ u8 day;
+ u8 hour;
+ u8 minute;
+ u8 second;
+ u8 pad1;
+ u32 nanosecond;
+ s16 timezone;
+ u8 daylight;
+ u8 pad2;
+} efi_time_t;
+
+typedef struct {
+ u32 resolution;
+ u32 accuracy;
+ u8 sets_to_zero;
+} efi_time_cap_t;
+
+typedef void *efi_event_t;
+/* Note that notifications won't work in mixed mode */
+typedef void (__efiapi *efi_event_notify_t)(efi_event_t, void *);
+
+typedef enum {
+ EfiTimerCancel,
+ EfiTimerPeriodic,
+ EfiTimerRelative
+} EFI_TIMER_DELAY;
+
+/*
+ * EFI Device Path information
+ */
+#define EFI_DEV_HW 0x01
+#define EFI_DEV_PCI 1
+#define EFI_DEV_PCCARD 2
+#define EFI_DEV_MEM_MAPPED 3
+#define EFI_DEV_VENDOR 4
+#define EFI_DEV_CONTROLLER 5
+#define EFI_DEV_ACPI 0x02
+#define EFI_DEV_BASIC_ACPI 1
+#define EFI_DEV_EXPANDED_ACPI 2
+#define EFI_DEV_MSG 0x03
+#define EFI_DEV_MSG_ATAPI 1
+#define EFI_DEV_MSG_SCSI 2
+#define EFI_DEV_MSG_FC 3
+#define EFI_DEV_MSG_1394 4
+#define EFI_DEV_MSG_USB 5
+#define EFI_DEV_MSG_USB_CLASS 15
+#define EFI_DEV_MSG_I20 6
+#define EFI_DEV_MSG_MAC 11
+#define EFI_DEV_MSG_IPV4 12
+#define EFI_DEV_MSG_IPV6 13
+#define EFI_DEV_MSG_INFINIBAND 9
+#define EFI_DEV_MSG_UART 14
+#define EFI_DEV_MSG_VENDOR 10
+#define EFI_DEV_MEDIA 0x04
+#define EFI_DEV_MEDIA_HARD_DRIVE 1
+#define EFI_DEV_MEDIA_CDROM 2
+#define EFI_DEV_MEDIA_VENDOR 3
+#define EFI_DEV_MEDIA_FILE 4
+#define EFI_DEV_MEDIA_PROTOCOL 5
+#define EFI_DEV_BIOS_BOOT 0x05
+#define EFI_DEV_END_PATH 0x7F
+#define EFI_DEV_END_PATH2 0xFF
+#define EFI_DEV_END_INSTANCE 0x01
+#define EFI_DEV_END_ENTIRE 0xFF
+
+struct efi_generic_dev_path {
+ u8 type;
+ u8 sub_type;
+ u16 length;
+} __packed;
+
+typedef struct efi_generic_dev_path efi_device_path_protocol_t;
+
+/*
+ * EFI Boot Services table
+ */
+union efi_boot_services {
+ struct {
+ efi_table_hdr_t hdr;
+ void *raise_tpl;
+ void *restore_tpl;
+ efi_status_t (__efiapi *allocate_pages)(int, int, unsigned long,
+ efi_physical_addr_t *);
+ efi_status_t (__efiapi *free_pages)(efi_physical_addr_t,
+ unsigned long);
+ efi_status_t (__efiapi *get_memory_map)(unsigned long *, void *,
+ unsigned long *,
+ unsigned long *, u32 *);
+ efi_status_t (__efiapi *allocate_pool)(int, unsigned long,
+ void **);
+ efi_status_t (__efiapi *free_pool)(void *);
+ efi_status_t (__efiapi *create_event)(u32, unsigned long,
+ efi_event_notify_t, void *,
+ efi_event_t *);
+ efi_status_t (__efiapi *set_timer)(efi_event_t,
+ EFI_TIMER_DELAY, u64);
+ efi_status_t (__efiapi *wait_for_event)(unsigned long,
+ efi_event_t *,
+ unsigned long *);
+ void *signal_event;
+ efi_status_t (__efiapi *close_event)(efi_event_t);
+ void *check_event;
+ void *install_protocol_interface;
+ void *reinstall_protocol_interface;
+ void *uninstall_protocol_interface;
+ efi_status_t (__efiapi *handle_protocol)(efi_handle_t,
+ efi_guid_t *, void **);
+ void *__reserved;
+ void *register_protocol_notify;
+ efi_status_t (__efiapi *locate_handle)(int, efi_guid_t *,
+ void *, unsigned long *,
+ efi_handle_t *);
+ efi_status_t (__efiapi *locate_device_path)(efi_guid_t *,
+ efi_device_path_protocol_t **,
+ efi_handle_t *);
+ efi_status_t (__efiapi *install_configuration_table)(efi_guid_t *,
+ void *);
+ void *load_image;
+ void *start_image;
+ efi_status_t (__efiapi *exit)(efi_handle_t,
+ efi_status_t,
+ unsigned long,
+ efi_char16_t *);
+ void *unload_image;
+ efi_status_t (__efiapi *exit_boot_services)(efi_handle_t,
+ unsigned long);
+ void *get_next_monotonic_count;
+ efi_status_t (__efiapi *stall)(unsigned long);
+ void *set_watchdog_timer;
+ void *connect_controller;
+ efi_status_t (__efiapi *disconnect_controller)(efi_handle_t,
+ efi_handle_t,
+ efi_handle_t);
+ void *open_protocol;
+ void *close_protocol;
+ void *open_protocol_information;
+ void *protocols_per_handle;
+ void *locate_handle_buffer;
+ efi_status_t (__efiapi *locate_protocol)(efi_guid_t *, void *,
+ void **);
+ void *install_multiple_protocol_interfaces;
+ void *uninstall_multiple_protocol_interfaces;
+ void *calculate_crc32;
+ void *copy_mem;
+ void *set_mem;
+ void *create_event_ex;
+ };
+ struct {
+ efi_table_hdr_t hdr;
+ u32 raise_tpl;
+ u32 restore_tpl;
+ u32 allocate_pages;
+ u32 free_pages;
+ u32 get_memory_map;
+ u32 allocate_pool;
+ u32 free_pool;
+ u32 create_event;
+ u32 set_timer;
+ u32 wait_for_event;
+ u32 signal_event;
+ u32 close_event;
+ u32 check_event;
+ u32 install_protocol_interface;
+ u32 reinstall_protocol_interface;
+ u32 uninstall_protocol_interface;
+ u32 handle_protocol;
+ u32 __reserved;
+ u32 register_protocol_notify;
+ u32 locate_handle;
+ u32 locate_device_path;
+ u32 install_configuration_table;
+ u32 load_image;
+ u32 start_image;
+ u32 exit;
+ u32 unload_image;
+ u32 exit_boot_services;
+ u32 get_next_monotonic_count;
+ u32 stall;
+ u32 set_watchdog_timer;
+ u32 connect_controller;
+ u32 disconnect_controller;
+ u32 open_protocol;
+ u32 close_protocol;
+ u32 open_protocol_information;
+ u32 protocols_per_handle;
+ u32 locate_handle_buffer;
+ u32 locate_protocol;
+ u32 install_multiple_protocol_interfaces;
+ u32 uninstall_multiple_protocol_interfaces;
+ u32 calculate_crc32;
+ u32 copy_mem;
+ u32 set_mem;
+ u32 create_event_ex;
+ } mixed_mode;
+};
+
+typedef union efi_boot_services efi_boot_services_t;
+
+/*
+ * Types and defines for EFI ResetSystem
+ */
+#define EFI_RESET_COLD 0
+#define EFI_RESET_WARM 1
+#define EFI_RESET_SHUTDOWN 2
+
+/*
+ * EFI Runtime Services table
+ */
+#define EFI_RUNTIME_SERVICES_SIGNATURE ((u64)0x5652453544e5552ULL)
+#define EFI_RUNTIME_SERVICES_REVISION 0x00010000
+
+typedef struct {
+ efi_table_hdr_t hdr;
+ u32 get_time;
+ u32 set_time;
+ u32 get_wakeup_time;
+ u32 set_wakeup_time;
+ u32 set_virtual_address_map;
+ u32 convert_pointer;
+ u32 get_variable;
+ u32 get_next_variable;
+ u32 set_variable;
+ u32 get_next_high_mono_count;
+ u32 reset_system;
+ u32 update_capsule;
+ u32 query_capsule_caps;
+ u32 query_variable_info;
+} efi_runtime_services_32_t;
+
+typedef efi_status_t efi_get_time_t (efi_time_t *tm, efi_time_cap_t *tc);
+typedef efi_status_t efi_set_time_t (efi_time_t *tm);
+typedef efi_status_t efi_get_wakeup_time_t (efi_bool_t *enabled, efi_bool_t *pending,
+ efi_time_t *tm);
+typedef efi_status_t efi_set_wakeup_time_t (efi_bool_t enabled, efi_time_t *tm);
+typedef efi_status_t efi_get_variable_t (efi_char16_t *name, efi_guid_t *vendor, u32 *attr,
+ unsigned long *data_size, void *data);
+typedef efi_status_t efi_get_next_variable_t (unsigned long *name_size, efi_char16_t *name,
+ efi_guid_t *vendor);
+typedef efi_status_t efi_set_variable_t (efi_char16_t *name, efi_guid_t *vendor,
+ u32 attr, unsigned long data_size,
+ void *data);
+typedef efi_status_t efi_get_next_high_mono_count_t (u32 *count);
+typedef void efi_reset_system_t (int reset_type, efi_status_t status,
+ unsigned long data_size, efi_char16_t *data);
+typedef efi_status_t efi_set_virtual_address_map_t (unsigned long memory_map_size,
+ unsigned long descriptor_size,
+ u32 descriptor_version,
+ efi_memory_desc_t *virtual_map);
+typedef efi_status_t efi_query_variable_info_t(u32 attr,
+ u64 *storage_space,
+ u64 *remaining_space,
+ u64 *max_variable_size);
+typedef efi_status_t efi_update_capsule_t(efi_capsule_header_t **capsules,
+ unsigned long count,
+ unsigned long sg_list);
+typedef efi_status_t efi_query_capsule_caps_t(efi_capsule_header_t **capsules,
+ unsigned long count,
+ u64 *max_size,
+ int *reset_type);
+typedef efi_status_t efi_query_variable_store_t(u32 attributes,
+ unsigned long size,
+ bool nonblocking);
+
+typedef union {
+ struct {
+ efi_table_hdr_t hdr;
+ efi_get_time_t __efiapi *get_time;
+ efi_set_time_t __efiapi *set_time;
+ efi_get_wakeup_time_t __efiapi *get_wakeup_time;
+ efi_set_wakeup_time_t __efiapi *set_wakeup_time;
+ efi_set_virtual_address_map_t __efiapi *set_virtual_address_map;
+ void *convert_pointer;
+ efi_get_variable_t __efiapi *get_variable;
+ efi_get_next_variable_t __efiapi *get_next_variable;
+ efi_set_variable_t __efiapi *set_variable;
+ efi_get_next_high_mono_count_t __efiapi *get_next_high_mono_count;
+ efi_reset_system_t __efiapi *reset_system;
+ efi_update_capsule_t __efiapi *update_capsule;
+ efi_query_capsule_caps_t __efiapi *query_capsule_caps;
+ efi_query_variable_info_t __efiapi *query_variable_info;
+ };
+ efi_runtime_services_32_t mixed_mode;
+} efi_runtime_services_t;
+
+#define EFI_SYSTEM_TABLE_SIGNATURE ((u64)0x5453595320494249ULL)
+
+#define EFI_2_30_SYSTEM_TABLE_REVISION ((2 << 16) | (30))
+#define EFI_2_20_SYSTEM_TABLE_REVISION ((2 << 16) | (20))
+#define EFI_2_10_SYSTEM_TABLE_REVISION ((2 << 16) | (10))
+#define EFI_2_00_SYSTEM_TABLE_REVISION ((2 << 16) | (00))
+#define EFI_1_10_SYSTEM_TABLE_REVISION ((1 << 16) | (10))
+#define EFI_1_02_SYSTEM_TABLE_REVISION ((1 << 16) | (02))
+
+typedef struct {
+ efi_table_hdr_t hdr;
+ u64 fw_vendor; /* physical addr of CHAR16 vendor string */
+ u32 fw_revision;
+ u32 __pad1;
+ u64 con_in_handle;
+ u64 con_in;
+ u64 con_out_handle;
+ u64 con_out;
+ u64 stderr_handle;
+ u64 stderr;
+ u64 runtime;
+ u64 boottime;
+ u32 nr_tables;
+ u32 __pad2;
+ u64 tables;
+} efi_system_table_64_t;
+
+typedef struct {
+ efi_table_hdr_t hdr;
+ u32 fw_vendor; /* physical addr of CHAR16 vendor string */
+ u32 fw_revision;
+ u32 con_in_handle;
+ u32 con_in;
+ u32 con_out_handle;
+ u32 con_out;
+ u32 stderr_handle;
+ u32 stderr;
+ u32 runtime;
+ u32 boottime;
+ u32 nr_tables;
+ u32 tables;
+} efi_system_table_32_t;
+
+typedef union efi_simple_text_input_protocol efi_simple_text_input_protocol_t;
+typedef union efi_simple_text_output_protocol efi_simple_text_output_protocol_t;
+
+typedef union {
+ struct {
+ efi_table_hdr_t hdr;
+ unsigned long fw_vendor; /* physical addr of CHAR16 vendor string */
+ u32 fw_revision;
+ unsigned long con_in_handle;
+ efi_simple_text_input_protocol_t *con_in;
+ unsigned long con_out_handle;
+ efi_simple_text_output_protocol_t *con_out;
+ unsigned long stderr_handle;
+ unsigned long stderr;
+ efi_runtime_services_t *runtime;
+ efi_boot_services_t *boottime;
+ unsigned long nr_tables;
+ unsigned long tables;
+ };
+ efi_system_table_32_t mixed_mode;
+} efi_system_table_t;
+
+struct efi_boot_memmap {
+ efi_memory_desc_t **map;
+ unsigned long *map_size;
+ unsigned long *desc_size;
+ u32 *desc_ver;
+ unsigned long *key_ptr;
+ unsigned long *buff_size;
+};
+
+#define efi_bs_call(func, ...) \
+ efi_system_table->boottime->func(__VA_ARGS__)
+
+#endif /* __LINUX_UEFI_H */
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 01/17] x86 UEFI: Copy code from Linux
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 01/17] x86 UEFI: Copy code from Linux Zixuan Wang
@ 2021-09-20 14:33 ` Paolo Bonzini
2021-09-21 3:58 ` Zixuan Wang
2021-09-21 16:33 ` Andrew Jones
1 sibling, 1 reply; 47+ messages in thread
From: Paolo Bonzini @ 2021-09-20 14:33 UTC (permalink / raw)
To: Zixuan Wang, kvm, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
On 27/08/21 05:12, Zixuan Wang wrote:
> +
> +/*
> + * The UEFI spec and EDK2 reference implementation both define EFI_GUID as
> + * struct { u32 a; u16; b; u16 c; u8 d[8]; }; and so the implied alignment
> + * is 32 bits not 8 bits like our guid_t. In some cases (i.e., on 32-bit ARM),
> + * this means that firmware services invoked by the kernel may assume that
> + * efi_guid_t* arguments are 32-bit aligned, and use memory accessors that
> + * do not tolerate misalignment. So let's set the minimum alignment to 32 bits.
Here you're not doing that though.
Paolo
> + * Note that the UEFI spec as well as some comments in the EDK2 code base
> + * suggest that EFI_GUID should be 64-bit aligned, but this appears to be
> + * a mistake, given that no code seems to exist that actually enforces that
> + * or relies on it.
> + */
> +typedef struct {
> + u8 b[16];
> +} guid_t;
> +typedef guid_t efi_guid_t;
> +
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 01/17] x86 UEFI: Copy code from Linux
2021-09-20 14:33 ` Paolo Bonzini
@ 2021-09-21 3:58 ` Zixuan Wang
2021-09-21 6:37 ` Varad Gautam
0 siblings, 1 reply; 47+ messages in thread
From: Zixuan Wang @ 2021-09-21 3:58 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Zixuan Wang, kvm, drjones, Marc Orr, Hyunwook (Wooky) Baek,
tmroeder, erdemaktas, rientjes, seanjc, brijesh.singh,
Thomas.Lendacky, varad.gautam, jroedel, bp
On Mon, Sep 20, 2021 at 6:26 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 27/08/21 05:12, Zixuan Wang wrote:
> > +
> > +/*
> > + * The UEFI spec and EDK2 reference implementation both define EFI_GUID as
> > + * struct { u32 a; u16; b; u16 c; u8 d[8]; }; and so the implied alignment
> > + * is 32 bits not 8 bits like our guid_t. In some cases (i.e., on 32-bit ARM),
> > + * this means that firmware services invoked by the kernel may assume that
> > + * efi_guid_t* arguments are 32-bit aligned, and use memory accessors that
> > + * do not tolerate misalignment. So let's set the minimum alignment to 32 bits.
>
> Here you're not doing that though.
>
> Paolo
Hi Paolo,
Indeed, I checked the original Linux code [1] and it has the alignment.
This patch is from Varad's patch set [2]. I can update this code in
the next version if Varad is OK with it.
I just finished my Google internship and lost my access to the Google
email, so I'm replying with my personal email.
[1] https://elixir.bootlin.com/linux/v5.14/source/include/linux/efi.h#L73
[2] https://lore.kernel.org/kvm/20210819113400.26516-1-varad.gautam@suse.com/
Best regards,
Zixuan
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 01/17] x86 UEFI: Copy code from Linux
2021-09-21 3:58 ` Zixuan Wang
@ 2021-09-21 6:37 ` Varad Gautam
0 siblings, 0 replies; 47+ messages in thread
From: Varad Gautam @ 2021-09-21 6:37 UTC (permalink / raw)
To: Zixuan Wang, Paolo Bonzini
Cc: Zixuan Wang, kvm, drjones, Marc Orr, Hyunwook (Wooky) Baek,
tmroeder, erdemaktas, rientjes, seanjc, brijesh.singh,
Thomas.Lendacky, jroedel, bp
Hi Zixuan,
On 9/21/21 5:58 AM, Zixuan Wang wrote:
> On Mon, Sep 20, 2021 at 6:26 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 27/08/21 05:12, Zixuan Wang wrote:
>>> +
>>> +/*
>>> + * The UEFI spec and EDK2 reference implementation both define EFI_GUID as
>>> + * struct { u32 a; u16; b; u16 c; u8 d[8]; }; and so the implied alignment
>>> + * is 32 bits not 8 bits like our guid_t. In some cases (i.e., on 32-bit ARM),
>>> + * this means that firmware services invoked by the kernel may assume that
>>> + * efi_guid_t* arguments are 32-bit aligned, and use memory accessors that
>>> + * do not tolerate misalignment. So let's set the minimum alignment to 32 bits.
>>
>> Here you're not doing that though.
>>
>> Paolo
>
> Hi Paolo,
>
> Indeed, I checked the original Linux code [1] and it has the alignment.
>
> This patch is from Varad's patch set [2]. I can update this code in
> the next version if Varad is OK with it.
>
Please feel free to modify the patch in the next revision. The alignment
indeed does not make sense here.
Thanks,
Varad
> I just finished my Google internship and lost my access to the Google
> email, so I'm replying with my personal email.
>
> [1] https://elixir.bootlin.com/linux/v5.14/source/include/linux/efi.h#L73
> [2] https://lore.kernel.org/kvm/20210819113400.26516-1-varad.gautam@suse.com/
>
> Best regards,
> Zixuan
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [kvm-unit-tests PATCH v2 01/17] x86 UEFI: Copy code from Linux
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 01/17] x86 UEFI: Copy code from Linux Zixuan Wang
2021-09-20 14:33 ` Paolo Bonzini
@ 2021-09-21 16:33 ` Andrew Jones
2021-09-22 20:10 ` Zixuan Wang
1 sibling, 1 reply; 47+ messages in thread
From: Andrew Jones @ 2021-09-21 16:33 UTC (permalink / raw)
To: Zixuan Wang
Cc: kvm, pbonzini, marcorr, baekhw, tmroeder, erdemaktas, rientjes,
seanjc, brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel,
bp
On Fri, Aug 27, 2021 at 03:12:06AM +0000, Zixuan Wang wrote:
> From: Varad Gautam <varad.gautam@suse.com>
>
> Copy UEFI-related definitions from Linux, so the follow-up commits can
> develop UEFI function calls based on these definitions, without relying
> on GNU-EFI library.
>
> Signed-off-by: Varad Gautam <varad.gautam@suse.com>
> ---
> lib/linux/uefi.h | 518 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 518 insertions(+)
> create mode 100644 lib/linux/uefi.h
>
> diff --git a/lib/linux/uefi.h b/lib/linux/uefi.h
> new file mode 100644
> index 0000000..567cddc
> --- /dev/null
> +++ b/lib/linux/uefi.h
Any reason to rename this to uefi.h even though it's efi.h in Linux?
Usually I'd suggest we take the whole file from Linux (but that would
be a mess for this one, so no) or that we only take what we need, when
we need it, rather than dumping a bunch of stuff up front which may or
may not be needed. Skimming through though, it looks like we'll likely
need most the stuff brought over. So I guess I'm OK with this approach.
Thanks,
drew
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [kvm-unit-tests PATCH v2 01/17] x86 UEFI: Copy code from Linux
2021-09-21 16:33 ` Andrew Jones
@ 2021-09-22 20:10 ` Zixuan Wang
0 siblings, 0 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-09-22 20:10 UTC (permalink / raw)
To: Andrew Jones
Cc: Zixuan Wang, kvm, Paolo Bonzini, Marc Orr, Hyunwook (Wooky) Baek,
Tom Roeder, erdemaktas, rientjes, seanjc, brijesh.singh,
Thomas.Lendacky, varad.gautam, jroedel, bp
On Tue, Sep 21, 2021 at 9:33 AM Andrew Jones <drjones@redhat.com> wrote:
>
> On Fri, Aug 27, 2021 at 03:12:06AM +0000, Zixuan Wang wrote:
> > From: Varad Gautam <varad.gautam@suse.com>
> >
> > Copy UEFI-related definitions from Linux, so the follow-up commits can
> > develop UEFI function calls based on these definitions, without relying
> > on GNU-EFI library.
> >
> > Signed-off-by: Varad Gautam <varad.gautam@suse.com>
> > ---
> > lib/linux/uefi.h | 518 +++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 518 insertions(+)
> > create mode 100644 lib/linux/uefi.h
> >
> > diff --git a/lib/linux/uefi.h b/lib/linux/uefi.h
> > new file mode 100644
> > index 0000000..567cddc
> > --- /dev/null
> > +++ b/lib/linux/uefi.h
>
> Any reason to rename this to uefi.h even though it's efi.h in Linux?
This file is from Varad's patch set [1]. I can rename the file to
efi.h in the next version if Varad is OK with it.
[1] https://lore.kernel.org/kvm/20210819113400.26516-1-varad.gautam@suse.com/
> Usually I'd suggest we take the whole file from Linux (but that would
> be a mess for this one, so no) or that we only take what we need, when
> we need it, rather than dumping a bunch of stuff up front which may or
> may not be needed. Skimming through though, it looks like we'll likely
> need most the stuff brought over. So I guess I'm OK with this approach.
>
> Thanks,
> drew
Thank you for the detailed explanation! Another reason to keep this a
separate commit is to preserve Varad's full authorship of these 2
commits. We already re-organized Varad's 6 patches into these 2, and
do not want to further reduce Varad's authorships.
Best regards,
Zixuan
^ permalink raw reply [flat|nested] 47+ messages in thread
* [kvm-unit-tests PATCH v2 02/17] x86 UEFI: Implement UEFI function calls
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 01/17] x86 UEFI: Copy code from Linux Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-09-21 16:43 ` Andrew Jones
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 03/17] x86 UEFI: Copy code from GNU-EFI Zixuan Wang
` (15 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
From: Varad Gautam <varad.gautam@suse.com>
This commit implements helper functions that call UEFI services and
assist the boot up process.
Signed-off-by: Varad Gautam <varad.gautam@suse.com>
---
lib/efi.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
create mode 100644 lib/efi.c
diff --git a/lib/efi.c b/lib/efi.c
new file mode 100644
index 0000000..9711354
--- /dev/null
+++ b/lib/efi.c
@@ -0,0 +1,58 @@
+#include <linux/uefi.h>
+
+unsigned long __efiapi efi_main(efi_handle_t handle, efi_system_table_t *sys_tab);
+efi_system_table_t *efi_system_table = NULL;
+
+static void efi_free_pool(void *ptr)
+{
+ efi_bs_call(free_pool, ptr);
+}
+
+static efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
+{
+ efi_memory_desc_t *m = NULL;
+ efi_status_t status;
+ unsigned long key = 0, map_size = 0, desc_size = 0;
+
+ status = efi_bs_call(get_memory_map, &map_size,
+ NULL, &key, &desc_size, NULL);
+ if (status != EFI_BUFFER_TOO_SMALL || map_size == 0)
+ goto out;
+
+ /* Pad map_size with additional descriptors so we don't need to
+ * retry. */
+ map_size += 4 * desc_size;
+ *map->buff_size = map_size;
+ status = efi_bs_call(allocate_pool, EFI_LOADER_DATA,
+ map_size, (void **)&m);
+ if (status != EFI_SUCCESS)
+ goto out;
+
+ /* Get the map. */
+ status = efi_bs_call(get_memory_map, &map_size,
+ m, &key, &desc_size, NULL);
+ if (status != EFI_SUCCESS) {
+ efi_free_pool(m);
+ goto out;
+ }
+
+ *map->desc_size = desc_size;
+ *map->map_size = map_size;
+ *map->key_ptr = key;
+out:
+ *map->map = m;
+ return status;
+}
+
+static efi_status_t efi_exit_boot_services(void *handle,
+ struct efi_boot_memmap *map)
+{
+ return efi_bs_call(exit_boot_services, handle, *map->key_ptr);
+}
+
+unsigned long __efiapi efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
+{
+ efi_system_table = sys_tab;
+
+ return 0;
+}
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 02/17] x86 UEFI: Implement UEFI function calls
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 02/17] x86 UEFI: Implement UEFI function calls Zixuan Wang
@ 2021-09-21 16:43 ` Andrew Jones
2021-09-22 20:17 ` Zixuan Wang
0 siblings, 1 reply; 47+ messages in thread
From: Andrew Jones @ 2021-09-21 16:43 UTC (permalink / raw)
To: Zixuan Wang
Cc: kvm, pbonzini, marcorr, baekhw, tmroeder, erdemaktas, rientjes,
seanjc, brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel,
bp
On Fri, Aug 27, 2021 at 03:12:07AM +0000, Zixuan Wang wrote:
> From: Varad Gautam <varad.gautam@suse.com>
>
> This commit implements helper functions that call UEFI services and
> assist the boot up process.
>
> Signed-off-by: Varad Gautam <varad.gautam@suse.com>
> ---
> lib/efi.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 58 insertions(+)
> create mode 100644 lib/efi.c
>
> diff --git a/lib/efi.c b/lib/efi.c
> new file mode 100644
> index 0000000..9711354
> --- /dev/null
> +++ b/lib/efi.c
> @@ -0,0 +1,58 @@
> +#include <linux/uefi.h>
Please add at least an SPDX header.
> +
> +unsigned long __efiapi efi_main(efi_handle_t handle, efi_system_table_t *sys_tab);
> +efi_system_table_t *efi_system_table = NULL;
> +
> +static void efi_free_pool(void *ptr)
> +{
> + efi_bs_call(free_pool, ptr);
> +}
> +
> +static efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
> +{
> + efi_memory_desc_t *m = NULL;
> + efi_status_t status;
> + unsigned long key = 0, map_size = 0, desc_size = 0;
> +
> + status = efi_bs_call(get_memory_map, &map_size,
> + NULL, &key, &desc_size, NULL);
> + if (status != EFI_BUFFER_TOO_SMALL || map_size == 0)
> + goto out;
> +
> + /* Pad map_size with additional descriptors so we don't need to
> + * retry. */
nit: please use Linux comment style
> + map_size += 4 * desc_size;
> + *map->buff_size = map_size;
> + status = efi_bs_call(allocate_pool, EFI_LOADER_DATA,
> + map_size, (void **)&m);
> + if (status != EFI_SUCCESS)
> + goto out;
> +
> + /* Get the map. */
> + status = efi_bs_call(get_memory_map, &map_size,
> + m, &key, &desc_size, NULL);
> + if (status != EFI_SUCCESS) {
> + efi_free_pool(m);
> + goto out;
> + }
> +
> + *map->desc_size = desc_size;
> + *map->map_size = map_size;
> + *map->key_ptr = key;
> +out:
> + *map->map = m;
> + return status;
> +}
> +
> +static efi_status_t efi_exit_boot_services(void *handle,
> + struct efi_boot_memmap *map)
> +{
> + return efi_bs_call(exit_boot_services, handle, *map->key_ptr);
> +}
> +
> +unsigned long __efiapi efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> +{
> + efi_system_table = sys_tab;
> +
> + return 0;
> +}
> --
> 2.33.0.259.gc128427fd7-goog
>
Otherwise
Reviewed-by: Andrew Jones <drjones@redhat.com>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 02/17] x86 UEFI: Implement UEFI function calls
2021-09-21 16:43 ` Andrew Jones
@ 2021-09-22 20:17 ` Zixuan Wang
0 siblings, 0 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-09-22 20:17 UTC (permalink / raw)
To: Andrew Jones
Cc: Zixuan Wang, kvm, Paolo Bonzini, Marc Orr, Hyunwook (Wooky) Baek,
Tom Roeder, erdemaktas, rientjes, seanjc, brijesh.singh,
Thomas.Lendacky, varad.gautam, jroedel, bp
On Tue, Sep 21, 2021 at 9:44 AM Andrew Jones <drjones@redhat.com> wrote:
>
> On Fri, Aug 27, 2021 at 03:12:07AM +0000, Zixuan Wang wrote:
> > From: Varad Gautam <varad.gautam@suse.com>
> >
> > This commit implements helper functions that call UEFI services and
> > assist the boot up process.
> >
> > Signed-off-by: Varad Gautam <varad.gautam@suse.com>
> > ---
> > lib/efi.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 58 insertions(+)
> > create mode 100644 lib/efi.c
> >
> > diff --git a/lib/efi.c b/lib/efi.c
> > new file mode 100644
> > index 0000000..9711354
> > --- /dev/null
> > +++ b/lib/efi.c
> > @@ -0,0 +1,58 @@
> > +#include <linux/uefi.h>
>
> Please add at least an SPDX header.
Got it, I will add one in the next version.
> > + status = efi_bs_call(get_memory_map, &map_size,
> > + NULL, &key, &desc_size, NULL);
> > + if (status != EFI_BUFFER_TOO_SMALL || map_size == 0)
> > + goto out;
> > +
> > + /* Pad map_size with additional descriptors so we don't need to
> > + * retry. */
>
> nit: please use Linux comment style
Got it! I will update the comment style in the next version
> > + return 0;
> > +}
> > --
> > 2.33.0.259.gc128427fd7-goog
> >
>
> Otherwise
>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
>
Thank you!
Best regards,
Zixuan
^ permalink raw reply [flat|nested] 47+ messages in thread
* [kvm-unit-tests PATCH v2 03/17] x86 UEFI: Copy code from GNU-EFI
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 01/17] x86 UEFI: Copy code from Linux Zixuan Wang
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 02/17] x86 UEFI: Implement UEFI function calls Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-10-04 12:44 ` Andrew Jones
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 04/17] x86 UEFI: Boot from UEFI Zixuan Wang
` (14 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
To build x86 test cases with UEFI, we need to borrow some source
code from GNU-EFI, which includes the initialization code and linker
scripts. This commit only copies the source code, without any
modification. These source code files are not used by KVM-Unit-Tests
in this commit.
The following source code is copied from GNU-EFI:
1. x86/efi/elf_x86_64_efi.lds
2. x86/efi/reloc_x86_64.c
3. x86/efi/crt0-efi-x86_64.S
We put these EFI-related files under a new dir `x86/efi` because:
1. EFI-related code is easy to find
2. EFI-related code is separated from the original code in `x86/`
3. EFI-related code can still reuse the Makefile and test case code
in its parent dir `x86/`
GNU-EFI repo and version:
GIT URL: https://git.code.sf.net/p/gnu-efi/code
Commit ID: 4fe83e102674
Website: https://sourceforge.net/p/gnu-efi/code/ci/4fe83e/tree/
Co-developed-by: Varad Gautam <varad.gautam@suse.com>
Signed-off-by: Varad Gautam <varad.gautam@suse.com>
Signed-off-by: Zixuan Wang <zixuanwang@google.com>
---
x86/efi/README.md | 25 ++++++++++
x86/efi/crt0-efi-x86_64.S | 79 +++++++++++++++++++++++++++++
x86/efi/elf_x86_64_efi.lds | 77 ++++++++++++++++++++++++++++
x86/efi/reloc_x86_64.c | 100 +++++++++++++++++++++++++++++++++++++
4 files changed, 281 insertions(+)
create mode 100644 x86/efi/README.md
create mode 100644 x86/efi/crt0-efi-x86_64.S
create mode 100644 x86/efi/elf_x86_64_efi.lds
create mode 100644 x86/efi/reloc_x86_64.c
diff --git a/x86/efi/README.md b/x86/efi/README.md
new file mode 100644
index 0000000..256ef8c
--- /dev/null
+++ b/x86/efi/README.md
@@ -0,0 +1,25 @@
+# EFI Startup Code and Linker Script
+
+This dir contains source code and linker script copied from
+[GNU-EFI](https://sourceforge.net/projects/gnu-efi/):
+ - crt0-efi-x86_64.S: startup code of an EFI application
+ - elf_x86_64_efi.lds: linker script to build an EFI application
+ - reloc_x86_64.c: position independent x86_64 ELF shared object relocator
+
+EFI application binaries should be relocatable as UEFI loads binaries to dynamic
+runtime addresses. To build such relocatable binaries, GNU-EFI utilizes the
+above-mentioned files in its build process:
+
+ 1. build an ELF shared object and link it using linker script
+ `elf_x86_64_efi.lds` to organize the sections in a way UEFI recognizes
+ 2. link the shared object with self-relocator `reloc_x86_64.c` that applies
+ dynamic relocations that may be present in the shared object
+ 3. link the entry point code `crt0-efi-x86_64.S` that invokes self-relocator
+ and then jumps to EFI application's `efi_main()` function
+ 4. convert the shared object to an EFI binary
+
+More details can be found in `GNU-EFI/README.gnuefi`, section "Building
+Relocatable Binaries".
+
+KVM-Unit-Tests follows a similar build process, but does not link with GNU-EFI
+library.
diff --git a/x86/efi/crt0-efi-x86_64.S b/x86/efi/crt0-efi-x86_64.S
new file mode 100644
index 0000000..eaf1656
--- /dev/null
+++ b/x86/efi/crt0-efi-x86_64.S
@@ -0,0 +1,79 @@
+/* The following code is copied from GNU-EFI/gnuefi/crt0-efi-x86_64.S
+
+ crt0-efi-x86_64.S - x86_64 EFI startup code.
+ Copyright (C) 1999 Hewlett-Packard Co.
+ Contributed by David Mosberger <davidm@hpl.hp.com>.
+ Copyright (C) 2005 Intel Co.
+ Contributed by Fenghua Yu <fenghua.yu@intel.com>.
+
+ All rights reserved.
+
+ Redistribution and use in source and binary forms, with or without
+ modification, are permitted provided that the following conditions
+ are met:
+
+ * Redistributions of source code must retain the above copyright
+ notice, this list of conditions and the following disclaimer.
+ * Redistributions in binary form must reproduce the above
+ copyright notice, this list of conditions and the following
+ disclaimer in the documentation and/or other materials
+ provided with the distribution.
+ * Neither the name of Hewlett-Packard Co. nor the names of its
+ contributors may be used to endorse or promote products derived
+ from this software without specific prior written permission.
+
+ THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS
+ BE LIABLE FOR ANYDIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
+ OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
+ TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ SUCH DAMAGE.
+*/
+ .text
+ .align 4
+
+ .globl _start
+_start:
+ subq $8, %rsp
+ pushq %rcx
+ pushq %rdx
+
+0:
+ lea ImageBase(%rip), %rdi
+ lea _DYNAMIC(%rip), %rsi
+
+ popq %rcx
+ popq %rdx
+ pushq %rcx
+ pushq %rdx
+ call _relocate
+
+ popq %rdi
+ popq %rsi
+
+ call efi_main
+ addq $8, %rsp
+
+.exit:
+ ret
+
+ // hand-craft a dummy .reloc section so EFI knows it's a relocatable executable:
+
+ .data
+dummy: .long 0
+
+#define IMAGE_REL_ABSOLUTE 0
+ .section .reloc, "a"
+label1:
+ .long dummy-label1 // Page RVA
+ .long 12 // Block Size (2*4+2*2), must be aligned by 32 Bits
+ .word (IMAGE_REL_ABSOLUTE<<12) + 0 // reloc for dummy
+ .word (IMAGE_REL_ABSOLUTE<<12) + 0 // reloc for dummy
+
diff --git a/x86/efi/elf_x86_64_efi.lds b/x86/efi/elf_x86_64_efi.lds
new file mode 100644
index 0000000..5eae376
--- /dev/null
+++ b/x86/efi/elf_x86_64_efi.lds
@@ -0,0 +1,77 @@
+/* Copied from GNU-EFI/gnuefi/elf_x86_64_efi.lds, licensed under GNU GPL */
+/* Same as elf_x86_64_fbsd_efi.lds, except for OUTPUT_FORMAT below - KEEP IN SYNC */
+OUTPUT_FORMAT("elf64-x86-64", "elf64-x86-64", "elf64-x86-64")
+OUTPUT_ARCH(i386:x86-64)
+ENTRY(_start)
+SECTIONS
+{
+ . = 0;
+ ImageBase = .;
+ /* .hash and/or .gnu.hash MUST come first! */
+ .hash : { *(.hash) }
+ .gnu.hash : { *(.gnu.hash) }
+ . = ALIGN(4096);
+ .eh_frame :
+ {
+ *(.eh_frame)
+ }
+ . = ALIGN(4096);
+ .text :
+ {
+ _text = .;
+ *(.text)
+ *(.text.*)
+ *(.gnu.linkonce.t.*)
+ . = ALIGN(16);
+ }
+ _etext = .;
+ _text_size = . - _text;
+ . = ALIGN(4096);
+ .reloc :
+ {
+ *(.reloc)
+ }
+ . = ALIGN(4096);
+ .data :
+ {
+ _data = .;
+ *(.rodata*)
+ *(.got.plt)
+ *(.got)
+ *(.data*)
+ *(.sdata)
+ /* the EFI loader doesn't seem to like a .bss section, so we stick
+ it all into .data: */
+ *(.sbss)
+ *(.scommon)
+ *(.dynbss)
+ *(.bss)
+ *(COMMON)
+ *(.rel.local)
+ }
+ .note.gnu.build-id : { *(.note.gnu.build-id) }
+
+ _edata = .;
+ _data_size = . - _etext;
+ . = ALIGN(4096);
+ .dynamic : { *(.dynamic) }
+ . = ALIGN(4096);
+ .rela :
+ {
+ *(.rela.data*)
+ *(.rela.got)
+ *(.rela.stab)
+ }
+ . = ALIGN(4096);
+ .dynsym : { *(.dynsym) }
+ . = ALIGN(4096);
+ .dynstr : { *(.dynstr) }
+ . = ALIGN(4096);
+ .ignored.reloc :
+ {
+ *(.rela.reloc)
+ *(.eh_frame)
+ *(.note.GNU-stack)
+ }
+ .comment 0 : { *(.comment) }
+}
diff --git a/x86/efi/reloc_x86_64.c b/x86/efi/reloc_x86_64.c
new file mode 100644
index 0000000..d13b53e
--- /dev/null
+++ b/x86/efi/reloc_x86_64.c
@@ -0,0 +1,100 @@
+/* This file is copied from GNU-EFI/gnuefi/reloc_x86_64.c
+
+ reloc_x86_64.c - position independent x86_64 ELF shared object relocator
+ Copyright (C) 1999 Hewlett-Packard Co.
+ Contributed by David Mosberger <davidm@hpl.hp.com>.
+ Copyright (C) 2005 Intel Co.
+ Contributed by Fenghua Yu <fenghua.yu@intel.com>.
+
+ All rights reserved.
+
+ Redistribution and use in source and binary forms, with or without
+ modification, are permitted provided that the following conditions
+ are met:
+
+ * Redistributions of source code must retain the above copyright
+ notice, this list of conditions and the following disclaimer.
+ * Redistributions in binary form must reproduce the above
+ copyright notice, this list of conditions and the following
+ disclaimer in the documentation and/or other materials
+ provided with the distribution.
+ * Neither the name of Hewlett-Packard Co. nor the names of its
+ contributors may be used to endorse or promote products derived
+ from this software without specific prior written permission.
+
+ THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS
+ BE LIABLE FOR ANYDIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
+ OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
+ TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ SUCH DAMAGE.
+*/
+
+#include <efi.h>
+#include <efilib.h>
+
+#include <elf.h>
+
+EFI_STATUS _relocate (long ldbase, Elf64_Dyn *dyn,
+ EFI_HANDLE image EFI_UNUSED,
+ EFI_SYSTEM_TABLE *systab EFI_UNUSED)
+{
+ long relsz = 0, relent = 0;
+ Elf64_Rel *rel = 0;
+ unsigned long *addr;
+ int i;
+
+ for (i = 0; dyn[i].d_tag != DT_NULL; ++i) {
+ switch (dyn[i].d_tag) {
+ case DT_RELA:
+ rel = (Elf64_Rel*)
+ ((unsigned long)dyn[i].d_un.d_ptr
+ + ldbase);
+ break;
+
+ case DT_RELASZ:
+ relsz = dyn[i].d_un.d_val;
+ break;
+
+ case DT_RELAENT:
+ relent = dyn[i].d_un.d_val;
+ break;
+
+ default:
+ break;
+ }
+ }
+
+ if (!rel && relent == 0)
+ return EFI_SUCCESS;
+
+ if (!rel || relent == 0)
+ return EFI_LOAD_ERROR;
+
+ while (relsz > 0) {
+ /* apply the relocs */
+ switch (ELF64_R_TYPE (rel->r_info)) {
+ case R_X86_64_NONE:
+ break;
+
+ case R_X86_64_RELATIVE:
+ addr = (unsigned long *)
+ (ldbase + rel->r_offset);
+ *addr += ldbase;
+ break;
+
+ default:
+ break;
+ }
+ rel = (Elf64_Rel*) ((char *) rel + relent);
+ relsz -= relent;
+ }
+ return EFI_SUCCESS;
+}
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 03/17] x86 UEFI: Copy code from GNU-EFI
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 03/17] x86 UEFI: Copy code from GNU-EFI Zixuan Wang
@ 2021-10-04 12:44 ` Andrew Jones
2021-10-04 22:09 ` Zixuan Wang
0 siblings, 1 reply; 47+ messages in thread
From: Andrew Jones @ 2021-10-04 12:44 UTC (permalink / raw)
To: Zixuan Wang
Cc: kvm, pbonzini, marcorr, baekhw, tmroeder, erdemaktas, rientjes,
seanjc, brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel,
bp
On Fri, Aug 27, 2021 at 03:12:08AM +0000, Zixuan Wang wrote:
> To build x86 test cases with UEFI, we need to borrow some source
> code from GNU-EFI, which includes the initialization code and linker
> scripts. This commit only copies the source code, without any
> modification. These source code files are not used by KVM-Unit-Tests
> in this commit.
>
> The following source code is copied from GNU-EFI:
> 1. x86/efi/elf_x86_64_efi.lds
> 2. x86/efi/reloc_x86_64.c
> 3. x86/efi/crt0-efi-x86_64.S
>
> We put these EFI-related files under a new dir `x86/efi` because:
> 1. EFI-related code is easy to find
> 2. EFI-related code is separated from the original code in `x86/`
> 3. EFI-related code can still reuse the Makefile and test case code
> in its parent dir `x86/`
>
> GNU-EFI repo and version:
> GIT URL: https://git.code.sf.net/p/gnu-efi/code
> Commit ID: 4fe83e102674
> Website: https://sourceforge.net/p/gnu-efi/code/ci/4fe83e/tree/
>
> Co-developed-by: Varad Gautam <varad.gautam@suse.com>
> Signed-off-by: Varad Gautam <varad.gautam@suse.com>
> Signed-off-by: Zixuan Wang <zixuanwang@google.com>
> ---
> x86/efi/README.md | 25 ++++++++++
> x86/efi/crt0-efi-x86_64.S | 79 +++++++++++++++++++++++++++++
> x86/efi/elf_x86_64_efi.lds | 77 ++++++++++++++++++++++++++++
> x86/efi/reloc_x86_64.c | 100 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 281 insertions(+)
> create mode 100644 x86/efi/README.md
> create mode 100644 x86/efi/crt0-efi-x86_64.S
> create mode 100644 x86/efi/elf_x86_64_efi.lds
> create mode 100644 x86/efi/reloc_x86_64.c
>
> diff --git a/x86/efi/README.md b/x86/efi/README.md
> new file mode 100644
> index 0000000..256ef8c
> --- /dev/null
> +++ b/x86/efi/README.md
> @@ -0,0 +1,25 @@
> +# EFI Startup Code and Linker Script
> +
> +This dir contains source code and linker script copied from
> +[GNU-EFI](https://sourceforge.net/projects/gnu-efi/):
> + - crt0-efi-x86_64.S: startup code of an EFI application
> + - elf_x86_64_efi.lds: linker script to build an EFI application
> + - reloc_x86_64.c: position independent x86_64 ELF shared object relocator
> +
> +EFI application binaries should be relocatable as UEFI loads binaries to dynamic
> +runtime addresses. To build such relocatable binaries, GNU-EFI utilizes the
> +above-mentioned files in its build process:
> +
> + 1. build an ELF shared object and link it using linker script
> + `elf_x86_64_efi.lds` to organize the sections in a way UEFI recognizes
> + 2. link the shared object with self-relocator `reloc_x86_64.c` that applies
> + dynamic relocations that may be present in the shared object
> + 3. link the entry point code `crt0-efi-x86_64.S` that invokes self-relocator
> + and then jumps to EFI application's `efi_main()` function
> + 4. convert the shared object to an EFI binary
> +
> +More details can be found in `GNU-EFI/README.gnuefi`, section "Building
> +Relocatable Binaries".
> +
> +KVM-Unit-Tests follows a similar build process, but does not link with GNU-EFI
> +library.
So, for AArch64, I also want to drop the gnu-efi dependency of my original
PoC. My second PoC, which I haven't finished, took things a bit further
than this does, though. I was integrating a PE/COFF header and linker
script changes directly into the kvm-unit-tests code rather than copying
these files over.
Thanks,
drew
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [kvm-unit-tests PATCH v2 03/17] x86 UEFI: Copy code from GNU-EFI
2021-10-04 12:44 ` Andrew Jones
@ 2021-10-04 22:09 ` Zixuan Wang
2021-10-05 5:58 ` Andrew Jones
0 siblings, 1 reply; 47+ messages in thread
From: Zixuan Wang @ 2021-10-04 22:09 UTC (permalink / raw)
To: Andrew Jones
Cc: kvm, Paolo Bonzini, Marc Orr, Hyunwook (Wooky) Baek, Tom Roeder,
erdemaktas, rientjes, seanjc, brijesh.singh, Thomas.Lendacky,
varad.gautam, jroedel, bp
On Mon, Oct 4, 2021 at 5:46 AM Andrew Jones <drjones@redhat.com> wrote:
>
> On Fri, Aug 27, 2021 at 03:12:08AM +0000, Zixuan Wang wrote:
> > +More details can be found in `GNU-EFI/README.gnuefi`, section "Building
> > +Relocatable Binaries".
> > +
> > +KVM-Unit-Tests follows a similar build process, but does not link with GNU-EFI
> > +library.
>
> So, for AArch64, I also want to drop the gnu-efi dependency of my original
> PoC. My second PoC, which I haven't finished, took things a bit further
> than this does, though. I was integrating a PE/COFF header and linker
> script changes directly into the kvm-unit-tests code rather than copying
> these files over.
>
> Thanks,
> drew
>
This approach sounds really interesting. Is there a public repo for
this new PoC?
I think the self-relocation code is the most important one in this
patch. If we can avoid or rewrite the self-relocation process, then we
can pretty much avoid copying GNU-EFI files.
Best regards,
Zixuan
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [kvm-unit-tests PATCH v2 03/17] x86 UEFI: Copy code from GNU-EFI
2021-10-04 22:09 ` Zixuan Wang
@ 2021-10-05 5:58 ` Andrew Jones
0 siblings, 0 replies; 47+ messages in thread
From: Andrew Jones @ 2021-10-05 5:58 UTC (permalink / raw)
To: Zixuan Wang
Cc: kvm, Paolo Bonzini, Marc Orr, Hyunwook (Wooky) Baek, Tom Roeder,
erdemaktas, rientjes, seanjc, brijesh.singh, Thomas.Lendacky,
varad.gautam, jroedel, bp
On Mon, Oct 04, 2021 at 03:09:23PM -0700, Zixuan Wang wrote:
> On Mon, Oct 4, 2021 at 5:46 AM Andrew Jones <drjones@redhat.com> wrote:
> >
> > On Fri, Aug 27, 2021 at 03:12:08AM +0000, Zixuan Wang wrote:
> > > +More details can be found in `GNU-EFI/README.gnuefi`, section "Building
> > > +Relocatable Binaries".
> > > +
> > > +KVM-Unit-Tests follows a similar build process, but does not link with GNU-EFI
> > > +library.
> >
> > So, for AArch64, I also want to drop the gnu-efi dependency of my original
> > PoC. My second PoC, which I haven't finished, took things a bit further
> > than this does, though. I was integrating a PE/COFF header and linker
> > script changes directly into the kvm-unit-tests code rather than copying
> > these files over.
> >
> > Thanks,
> > drew
> >
>
> This approach sounds really interesting. Is there a public repo for
> this new PoC?
Not yet, but reviewing this series has inspired me to reprioritize that
work. I'll try to get it dusted off, improved, and published somewhere.
>
> I think the self-relocation code is the most important one in this
> patch. If we can avoid or rewrite the self-relocation process, then we
> can pretty much avoid copying GNU-EFI files.
Yup. AArch64 already has a simple relocator (only does R_AARCH64_RELATIVE)
in cstart64.S::start. I also wrote a R_PPC_RELATIVE relocator for ppc in
C, see powerpc/reloc64.c. But, if the gnu-efi one is better for x86, then
I don't see much problem in importing it.
Thanks,
drew
^ permalink raw reply [flat|nested] 47+ messages in thread
* [kvm-unit-tests PATCH v2 04/17] x86 UEFI: Boot from UEFI
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
` (2 preceding siblings ...)
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 03/17] x86 UEFI: Copy code from GNU-EFI Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-10-04 12:55 ` Andrew Jones
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 05/17] x86 UEFI: Load IDT after UEFI boot up Zixuan Wang
` (13 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
This commit provides initial support for x86 test cases to boot from
UEFI:
1. UEFI compiler flags are added to Makefile
2. A new TARGET_EFI macro is added to turn on/off UEFI startup code
3. Previous Multiboot setup code is refactored and updated for
supporting UEFI, including the following changes:
1. x86/efi/crt0-efi-x86_64.S: provides entry point and jumps to
setup code in lib/efi.c.
2. lib/efi.c: performs UEFI setup, calls arch-related setup
functions, then jumps to test case main() function
3. lib/x86/setup.c: provides arch-related setup under UEFI
To build test cases for UEFI, please first install the GNU-EFI library.
Check x86/efi/README.md for more details.
This commit is tested by a simple test calling report() and
report_summayr(). This commit does not include such a test to avoid
unnecessary files added into git history. To build and run this test in
UEFI (assuming file name is x86/dummy.c):
./configure --target-efi
make x86/dummy.efi
./x86/efi/run ./x86/dummy.efi
To use the default Multiboot instead of UEFI:
./configure
make x86/dummy.flat
./x86/run ./x86/dummy.flat
Some x86 test cases require additional fixes to work in UEFI, e.g.,
converting to position independent code (PIC), setting up page tables,
etc. This commit does not provide these fixes, so compiling and running
UEFI test cases other than x86/dummy.c may trigger compiler errors or
QEMU crashes. These test cases will be fixed by the follow-up commits in
this series.
The following code is ported from github.com/rhdrjones/kvm-unit-tests
- ./configure: 'target-efi'-related code
See original code:
- Repo: https://github.com/rhdrjones/kvm-unit-tests
- Branch: target-efi
Co-developed-by: Varad Gautam <varad.gautam@suse.com>
Signed-off-by: Varad Gautam <varad.gautam@suse.com>
Signed-off-by: Zixuan Wang <zixuanwang@google.com>
---
.gitignore | 3 ++
Makefile | 29 +++++++++++++++++-
README.md | 6 ++++
configure | 6 ++++
lib/efi.c | 38 ++++++++++++++++++++----
lib/efi.h | 17 +++++++++++
lib/linux/uefi.h | 10 +++++--
lib/x86/asm/setup.h | 11 +++++++
lib/x86/setup.c | 15 ++++++++++
x86/Makefile.common | 67 +++++++++++++++++++++++++++++++-----------
x86/Makefile.i386 | 5 ++--
x86/Makefile.x86_64 | 54 ++++++++++++++++++++++------------
x86/efi/README.md | 40 ++++++++++++++++++++++++-
x86/efi/reloc_x86_64.c | 9 ++----
x86/efi/run | 63 +++++++++++++++++++++++++++++++++++++++
x86/run | 16 ++++++++--
16 files changed, 333 insertions(+), 56 deletions(-)
create mode 100644 lib/efi.h
create mode 100644 lib/x86/asm/setup.h
create mode 100755 x86/efi/run
diff --git a/.gitignore b/.gitignore
index b3cf2cb..dca6d29 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,7 +3,9 @@ tags
*.a
*.d
*.o
+*.so
*.flat
+*.efi
*.elf
.pc
patches
@@ -24,3 +26,4 @@ cscope.*
/api/dirty-log-perf
/s390x/*.bin
/s390x/snippets/*/*.gbin
+/efi-tests/*
diff --git a/Makefile b/Makefile
index f7b9f28..c4a3905 100644
--- a/Makefile
+++ b/Makefile
@@ -38,6 +38,29 @@ LIBFDT_archive = $(LIBFDT_objdir)/libfdt.a
OBJDIRS += $(LIBFDT_objdir)
+# EFI App
+ifeq ($(TARGET_EFI),y)
+ifeq ($(ARCH_NAME),x86_64)
+EFI_ARCH = x86_64
+else
+$(error Cannot build $(ARCH_NAME) tests as EFI apps)
+endif
+EFI_CFLAGS := -DTARGET_EFI
+# The following CFLAGS and LDFLAGS come from:
+# - GNU-EFI/Makefile.defaults
+# - GNU-EFI/apps/Makefile
+# Function calls must include the number of arguments passed to the functions
+# More details: https://wiki.osdev.org/GNU-EFI
+EFI_CFLAGS += -maccumulate-outgoing-args
+# GCC defines wchar to be 32 bits, but EFI expects 16 bits
+EFI_CFLAGS += -fshort-wchar
+# EFI applications use PIC as they are loaded to dynamic addresses, not a fixed
+# starting address
+EFI_CFLAGS += -fPIC
+# Create shared library
+EFI_LDFLAGS := -Bsymbolic -shared -nostdlib
+endif
+
#include architecture specific make rules
include $(SRCDIR)/$(TEST_DIR)/Makefile
@@ -62,7 +85,11 @@ COMMON_CFLAGS += $(fno_stack_protector)
COMMON_CFLAGS += $(fno_stack_protector_all)
COMMON_CFLAGS += $(wno_frame_address)
COMMON_CFLAGS += $(if $(U32_LONG_FMT),-D__U32_LONG_FMT__,)
+ifeq ($(TARGET_EFI),y)
+COMMON_CFLAGS += $(EFI_CFLAGS)
+else
COMMON_CFLAGS += $(fno_pic) $(no_pie)
+endif
COMMON_CFLAGS += $(wclobbered)
COMMON_CFLAGS += $(wunused_but_set_parameter)
@@ -113,7 +140,7 @@ clean: arch_clean libfdt_clean
distclean: clean
$(RM) lib/asm lib/config.h config.mak $(TEST_DIR)-run msr.out cscope.* build-head
- $(RM) -r tests logs logs.old
+ $(RM) -r tests logs logs.old efi-tests
cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
cscope:
diff --git a/README.md b/README.md
index b498aaf..6edacfe 100644
--- a/README.md
+++ b/README.md
@@ -17,6 +17,8 @@ in this directory. Test images are created in ./ARCH/\*.flat
NOTE: GCC cross-compiler is required for [build on macOS](README.macOS.md).
+To build with UEFI, check [build and run with UEFI](./x86/efi/README.md).
+
## Standalone tests
The tests can be built as standalone. To create and use standalone tests do:
@@ -54,6 +56,10 @@ ACCEL=name environment variable:
ACCEL=kvm ./x86-run ./x86/msr.flat
+## Running the tests with UEFI
+
+Check [build and run with UEFI](./x86/efi/README.md).
+
# Tests configuration file
The test case may need specific runtime configurations, for
diff --git a/configure b/configure
index 1d4d855..b6c09b3 100755
--- a/configure
+++ b/configure
@@ -28,6 +28,7 @@ erratatxt="$srcdir/errata.txt"
host_key_document=
page_size=
earlycon=
+target_efi=
usage() {
cat <<-EOF
@@ -69,6 +70,7 @@ usage() {
pl011,mmio32,ADDR
Specify a PL011 compatible UART at address ADDR. Supported
register stride is 32 bit only.
+ --target-efi Boot and run from UEFI
EOF
exit 1
}
@@ -133,6 +135,9 @@ while [[ "$1" = -* ]]; do
--earlycon)
earlycon="$arg"
;;
+ --target-efi)
+ target_efi=y
+ ;;
--help)
usage
;;
@@ -341,6 +346,7 @@ U32_LONG_FMT=$u32_long
WA_DIVIDE=$wa_divide
GENPROTIMG=${GENPROTIMG-genprotimg}
HOST_KEY_DOCUMENT=$host_key_document
+TARGET_EFI=$target_efi
EOF
if [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then
echo "TARGET=$target" >> config.mak
diff --git a/lib/efi.c b/lib/efi.c
index 9711354..99307db 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -1,6 +1,24 @@
+/*
+ * efi_main() function to set up and run test cases in EFI
+ *
+ * Copyright (c) 2021, SUSE, Varad Gautam <varad.gautam@suse.com>
+ * Copyright (c) 2021, Google Inc, Zixuan Wang <zixuanwang@google.com>
+ *
+ * SPDX-License-Identifier: LGPL-2.0-or-later
+ */
+
#include <linux/uefi.h>
+#include <libcflat.h>
+#include <asm/setup.h>
+#include "efi.h"
+
+/* From lib/argv.c */
+extern int __argc, __envc;
+extern char *__argv[100];
+extern char *__environ[200];
+
+extern int main(int argc, char **argv, char **envp);
-unsigned long __efiapi efi_main(efi_handle_t handle, efi_system_table_t *sys_tab);
efi_system_table_t *efi_system_table = NULL;
static void efi_free_pool(void *ptr)
@@ -8,7 +26,7 @@ static void efi_free_pool(void *ptr)
efi_bs_call(free_pool, ptr);
}
-static efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
+efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
{
efi_memory_desc_t *m = NULL;
efi_status_t status;
@@ -44,15 +62,23 @@ out:
return status;
}
-static efi_status_t efi_exit_boot_services(void *handle,
- struct efi_boot_memmap *map)
+efi_status_t efi_exit_boot_services(void *handle, struct efi_boot_memmap *map)
{
return efi_bs_call(exit_boot_services, handle, *map->key_ptr);
}
-unsigned long __efiapi efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
+efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
{
+ int ret;
+
efi_system_table = sys_tab;
- return 0;
+ setup_efi();
+ ret = main(__argc, __argv, __environ);
+
+ /* Shutdown the guest VM */
+ efi_rs_call(reset_system, EFI_RESET_SHUTDOWN, ret, 0, NULL);
+
+ /* Unreachable */
+ return EFI_UNSUPPORTED;
}
diff --git a/lib/efi.h b/lib/efi.h
new file mode 100644
index 0000000..60cdb6f
--- /dev/null
+++ b/lib/efi.h
@@ -0,0 +1,17 @@
+#ifndef _EFI_H_
+#define _EFI_H_
+
+/*
+ * EFI-related functions in KVM-Unit-Tests. This file's name "efi.h" is in
+ * conflict with GNU-EFI library's "efi.h", but KVM-Unit-Tests does not include
+ * GNU-EFI headers or links against GNU-EFI.
+ */
+#include "linux/uefi.h"
+#include <elf.h>
+
+efi_status_t _relocate(long ldbase, Elf64_Dyn *dyn, efi_handle_t handle, efi_system_table_t *sys_tab);
+efi_status_t efi_get_memory_map(struct efi_boot_memmap *map);
+efi_status_t efi_exit_boot_services(void *handle, struct efi_boot_memmap *map);
+efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab);
+
+#endif /* _EFI_H_ */
diff --git a/lib/linux/uefi.h b/lib/linux/uefi.h
index 567cddc..9adc7ab 100644
--- a/lib/linux/uefi.h
+++ b/lib/linux/uefi.h
@@ -4,6 +4,12 @@
#ifndef __LINUX_UEFI_H
#define __LINUX_UEFI_H
+#include "libcflat.h"
+
+#ifndef __packed
+# define __packed __attribute__((__packed__))
+#endif
+
#define BITS_PER_LONG 64
#define EFI_SUCCESS 0
@@ -512,7 +518,7 @@ struct efi_boot_memmap {
unsigned long *buff_size;
};
-#define efi_bs_call(func, ...) \
- efi_system_table->boottime->func(__VA_ARGS__)
+#define efi_bs_call(func, ...) efi_system_table->boottime->func(__VA_ARGS__)
+#define efi_rs_call(func, ...) efi_system_table->runtime->func(__VA_ARGS__)
#endif /* __LINUX_UEFI_H */
diff --git a/lib/x86/asm/setup.h b/lib/x86/asm/setup.h
new file mode 100644
index 0000000..eb1cf73
--- /dev/null
+++ b/lib/x86/asm/setup.h
@@ -0,0 +1,11 @@
+#ifndef _X86_ASM_SETUP_H_
+#define _X86_ASM_SETUP_H_
+
+#ifdef TARGET_EFI
+#include "x86/apic.h"
+#include "x86/smp.h"
+
+void setup_efi(void);
+#endif /* TARGET_EFI */
+
+#endif /* _X86_ASM_SETUP_H_ */
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index 7befe09..efb5ecd 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -2,6 +2,7 @@
* Initialize machine setup information
*
* Copyright (C) 2017, Red Hat Inc, Andrew Jones <drjones@redhat.com>
+ * Copyright (C) 2021, Google Inc, Zixuan Wang <zixuanwang@google.com>
*
* This work is licensed under the terms of the GNU LGPL, version 2.
*/
@@ -9,6 +10,7 @@
#include "fwcfg.h"
#include "alloc_phys.h"
#include "argv.h"
+#include "asm/setup.h"
extern char edata;
@@ -118,6 +120,19 @@ void setup_multiboot(struct mbi_bootinfo *bi)
initrd_size = mods->end - mods->start;
}
+#ifdef TARGET_EFI
+
+void setup_efi(void)
+{
+ reset_apic();
+ mask_pic_interrupts();
+ enable_apic();
+ enable_x2apic();
+ smp_init();
+}
+
+#endif /* TARGET_EFI */
+
void setup_libcflat(void)
{
if (initrd) {
diff --git a/x86/Makefile.common b/x86/Makefile.common
index 52bb7aa..4859bf3 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -22,6 +22,11 @@ cflatobjs += lib/x86/acpi.o
cflatobjs += lib/x86/stack.o
cflatobjs += lib/x86/fault_test.o
cflatobjs += lib/x86/delay.o
+ifeq ($(TARGET_EFI),y)
+cflatobjs += lib/x86/setup.o
+cflatobjs += lib/efi.o
+cflatobjs += x86/efi/reloc_x86_64.o
+endif
OBJDIRS += lib/x86
@@ -37,10 +42,25 @@ COMMON_CFLAGS += -O1
# stack.o relies on frame pointers.
KEEP_FRAME_POINTER := y
-# We want to keep intermediate file: %.elf and %.o
+FLATLIBS = lib/libcflat.a
+
+ifeq ($(TARGET_EFI),y)
+.PRECIOUS: %.efi %.so
+
+%.so: %.o $(FLATLIBS) $(SRCDIR)/x86/efi/elf_x86_64_efi.lds $(cstart.o)
+ $(LD) -T $(SRCDIR)/x86/efi/elf_x86_64_efi.lds $(EFI_LDFLAGS) -o $@ \
+ $(filter %.o, $^) $(FLATLIBS)
+ @chmod a-x $@
+
+%.efi: %.so
+ $(OBJCOPY) \
+ -j .text -j .sdata -j .data -j .dynamic -j .dynsym -j .rel \
+ -j .rela -j .reloc -S --target=$(FORMAT) $< $@
+ @chmod a-x $@
+else
+# We want to keep intermediate file: %.elf and %.o
.PRECIOUS: %.elf %.o
-FLATLIBS = lib/libcflat.a
%.elf: %.o $(FLATLIBS) $(SRCDIR)/x86/flat.lds $(cstart.o)
$(CC) $(CFLAGS) -nostdlib -o $@ -Wl,-T,$(SRCDIR)/x86/flat.lds \
$(filter %.o, $^) $(FLATLIBS)
@@ -49,18 +69,29 @@ FLATLIBS = lib/libcflat.a
%.flat: %.elf
$(OBJCOPY) -O elf32-i386 $^ $@
@chmod a-x $@
+endif
-tests-common = $(TEST_DIR)/vmexit.flat $(TEST_DIR)/tsc.flat \
- $(TEST_DIR)/smptest.flat \
- $(TEST_DIR)/realmode.flat $(TEST_DIR)/msr.flat \
- $(TEST_DIR)/hypercall.flat $(TEST_DIR)/sieve.flat \
- $(TEST_DIR)/kvmclock_test.flat $(TEST_DIR)/eventinj.flat \
- $(TEST_DIR)/s3.flat $(TEST_DIR)/pmu.flat $(TEST_DIR)/setjmp.flat \
- $(TEST_DIR)/tsc_adjust.flat $(TEST_DIR)/asyncpf.flat \
- $(TEST_DIR)/init.flat $(TEST_DIR)/smap.flat \
- $(TEST_DIR)/hyperv_synic.flat $(TEST_DIR)/hyperv_stimer.flat \
- $(TEST_DIR)/hyperv_connections.flat \
- $(TEST_DIR)/umip.flat $(TEST_DIR)/tsx-ctrl.flat
+tests-common = $(TEST_DIR)/vmexit.$(exe) $(TEST_DIR)/tsc.$(exe) \
+ $(TEST_DIR)/smptest.$(exe) \
+ $(TEST_DIR)/msr.$(exe) \
+ $(TEST_DIR)/hypercall.$(exe) $(TEST_DIR)/sieve.$(exe) \
+ $(TEST_DIR)/kvmclock_test.$(exe) \
+ $(TEST_DIR)/s3.$(exe) $(TEST_DIR)/pmu.$(exe) $(TEST_DIR)/setjmp.$(exe) \
+ $(TEST_DIR)/tsc_adjust.$(exe) $(TEST_DIR)/asyncpf.$(exe) \
+ $(TEST_DIR)/init.$(exe) \
+ $(TEST_DIR)/hyperv_synic.$(exe) $(TEST_DIR)/hyperv_stimer.$(exe) \
+ $(TEST_DIR)/hyperv_connections.$(exe) \
+ $(TEST_DIR)/tsx-ctrl.$(exe)
+
+# The following test cases are disabled when building EFI tests because they
+# use absolute addresses in their inline assembly code, which cannot compile
+# with the '-fPIC' flag
+ifneq ($(TARGET_EFI),y)
+tests-common += $(TEST_DIR)/eventinj.$(exe) \
+ $(TEST_DIR)/smap.$(exe) \
+ $(TEST_DIR)/realmode.$(exe) \
+ $(TEST_DIR)/umip.$(exe)
+endif
test_cases: $(tests-common) $(tests)
@@ -72,14 +103,16 @@ $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o
$(TEST_DIR)/realmode.o: bits = $(if $(call cc-option,-m16,""),16,32)
-$(TEST_DIR)/kvmclock_test.elf: $(TEST_DIR)/kvmclock.o
+$(TEST_DIR)/kvmclock_test.$(bin): $(TEST_DIR)/kvmclock.o
-$(TEST_DIR)/hyperv_synic.elf: $(TEST_DIR)/hyperv.o
+$(TEST_DIR)/hyperv_synic.$(bin): $(TEST_DIR)/hyperv.o
-$(TEST_DIR)/hyperv_stimer.elf: $(TEST_DIR)/hyperv.o
+$(TEST_DIR)/hyperv_stimer.$(bin): $(TEST_DIR)/hyperv.o
-$(TEST_DIR)/hyperv_connections.elf: $(TEST_DIR)/hyperv.o
+$(TEST_DIR)/hyperv_connections.$(bin): $(TEST_DIR)/hyperv.o
arch_clean:
$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
$(TEST_DIR)/.*.d lib/x86/.*.d \
+ $(TEST_DIR)/efi/*.o $(TEST_DIR)/efi/.*.d \
+ $(TEST_DIR)/*.so $(TEST_DIR)/*.efi
diff --git a/x86/Makefile.i386 b/x86/Makefile.i386
index 960e274..340c561 100644
--- a/x86/Makefile.i386
+++ b/x86/Makefile.i386
@@ -1,11 +1,12 @@
cstart.o = $(TEST_DIR)/cstart.o
bits = 32
ldarch = elf32-i386
+exe = flat
COMMON_CFLAGS += -mno-sse -mno-sse2
cflatobjs += lib/x86/setjmp32.o lib/ldiv32.o
-tests = $(TEST_DIR)/taskswitch.flat $(TEST_DIR)/taskswitch2.flat \
- $(TEST_DIR)/cmpxchg8b.flat $(TEST_DIR)/la57.flat
+tests = $(TEST_DIR)/taskswitch.$(exe) $(TEST_DIR)/taskswitch2.$(exe) \
+ $(TEST_DIR)/cmpxchg8b.$(exe) $(TEST_DIR)/la57.$(exe)
include $(SRCDIR)/$(TEST_DIR)/Makefile.common
diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
index 8134952..a5f8923 100644
--- a/x86/Makefile.x86_64
+++ b/x86/Makefile.x86_64
@@ -1,6 +1,15 @@
cstart.o = $(TEST_DIR)/cstart64.o
bits = 64
ldarch = elf64-x86-64
+ifeq ($(TARGET_EFI),y)
+exe = efi
+bin = so
+FORMAT = efi-app-x86_64
+cstart.o = x86/efi/crt0-efi-x86_64.o
+else
+exe = flat
+bin = elf
+endif
fcf_protection_full := $(call cc-option, -fcf-protection=full,)
COMMON_CFLAGS += -mno-red-zone -mno-sse -mno-sse2 $(fcf_protection_full)
@@ -9,29 +18,36 @@ cflatobjs += lib/x86/setjmp64.o
cflatobjs += lib/x86/intel-iommu.o
cflatobjs += lib/x86/usermode.o
-tests = $(TEST_DIR)/access.flat $(TEST_DIR)/apic.flat \
- $(TEST_DIR)/emulator.flat $(TEST_DIR)/idt_test.flat \
- $(TEST_DIR)/xsave.flat $(TEST_DIR)/rmap_chain.flat \
- $(TEST_DIR)/pcid.flat $(TEST_DIR)/debug.flat \
- $(TEST_DIR)/ioapic.flat $(TEST_DIR)/memory.flat \
- $(TEST_DIR)/pku.flat $(TEST_DIR)/hyperv_clock.flat
-tests += $(TEST_DIR)/syscall.flat
-tests += $(TEST_DIR)/svm.flat
-tests += $(TEST_DIR)/vmx.flat
-tests += $(TEST_DIR)/tscdeadline_latency.flat
-tests += $(TEST_DIR)/intel-iommu.flat
-tests += $(TEST_DIR)/vmware_backdoors.flat
-tests += $(TEST_DIR)/rdpru.flat
-tests += $(TEST_DIR)/pks.flat
-tests += $(TEST_DIR)/pmu_lbr.flat
+tests = $(TEST_DIR)/apic.$(exe) \
+ $(TEST_DIR)/idt_test.$(exe) \
+ $(TEST_DIR)/xsave.$(exe) $(TEST_DIR)/rmap_chain.$(exe) \
+ $(TEST_DIR)/pcid.$(exe) $(TEST_DIR)/debug.$(exe) \
+ $(TEST_DIR)/ioapic.$(exe) $(TEST_DIR)/memory.$(exe) \
+ $(TEST_DIR)/pku.$(exe) $(TEST_DIR)/hyperv_clock.$(exe)
+tests += $(TEST_DIR)/syscall.$(exe)
+tests += $(TEST_DIR)/tscdeadline_latency.$(exe)
+tests += $(TEST_DIR)/intel-iommu.$(exe)
+tests += $(TEST_DIR)/rdpru.$(exe)
+tests += $(TEST_DIR)/pks.$(exe)
+tests += $(TEST_DIR)/pmu_lbr.$(exe)
+# The following test cases are disabled when building EFI tests because they
+# use absolute addresses in their inline assembly code, which cannot compile
+# with the '-fPIC' flag
+ifneq ($(TARGET_EFI),y)
+tests += $(TEST_DIR)/access.$(exe)
+tests += $(TEST_DIR)/emulator.$(exe)
+tests += $(TEST_DIR)/svm.$(exe)
+tests += $(TEST_DIR)/vmx.$(exe)
+tests += $(TEST_DIR)/vmware_backdoors.$(exe)
ifneq ($(fcf_protection_full),)
-tests += $(TEST_DIR)/cet.flat
+tests += $(TEST_DIR)/cet.$(exe)
+endif
endif
include $(SRCDIR)/$(TEST_DIR)/Makefile.common
-$(TEST_DIR)/hyperv_clock.elf: $(TEST_DIR)/hyperv_clock.o
+$(TEST_DIR)/hyperv_clock.$(bin): $(TEST_DIR)/hyperv_clock.o
-$(TEST_DIR)/vmx.elf: $(TEST_DIR)/vmx_tests.o
-$(TEST_DIR)/svm.elf: $(TEST_DIR)/svm_tests.o
+$(TEST_DIR)/vmx.$(bin): $(TEST_DIR)/vmx_tests.o
+$(TEST_DIR)/svm.$(bin): $(TEST_DIR)/svm_tests.o
diff --git a/x86/efi/README.md b/x86/efi/README.md
index 256ef8c..befe9cc 100644
--- a/x86/efi/README.md
+++ b/x86/efi/README.md
@@ -1,4 +1,38 @@
-# EFI Startup Code and Linker Script
+# Build KVM-Unit-Tests and run under UEFI
+
+## Introduction
+
+This dir provides code to build KVM-Unit-Tests test cases and run them under
+QEMU and UEFI.
+
+### Install dependencies
+
+The following dependencies should be installed:
+
+- [UEFI firmware](https://github.com/tianocore/edk2): to run test cases in QEMU
+
+### Build
+
+To build:
+
+ ./configure --target-efi
+ make
+
+### Run test cases with UEFI
+
+To run a test case with UEFI:
+
+ ./x86/efi/run ./x86/dummy.efi
+
+By default the runner script loads the UEFI firmware `/usr/share/ovmf/OVMF.fd`;
+please install UEFI firmware to this path, or specify the correct path through
+the env variable `EFI_UEFI`:
+
+ EFI_UEFI=/path/to/OVMF.fd ./x86/efi/run ./x86/dummy.efi
+
+## Code structure
+
+### Code from GNU-EFI
This dir contains source code and linker script copied from
[GNU-EFI](https://sourceforge.net/projects/gnu-efi/):
@@ -23,3 +57,7 @@ Relocatable Binaries".
KVM-Unit-Tests follows a similar build process, but does not link with GNU-EFI
library.
+### Startup code for KVM-Unit-Tests in UEFI
+
+This dir also contains KVM-Unit-Tests startup code in UEFI:
+ - efistart64.S: startup code for KVM-Unit-Tests in UEFI
diff --git a/x86/efi/reloc_x86_64.c b/x86/efi/reloc_x86_64.c
index d13b53e..511ef82 100644
--- a/x86/efi/reloc_x86_64.c
+++ b/x86/efi/reloc_x86_64.c
@@ -37,14 +37,11 @@
SUCH DAMAGE.
*/
-#include <efi.h>
-#include <efilib.h>
-
+#include "linux/uefi.h"
+#include "efi.h"
#include <elf.h>
-EFI_STATUS _relocate (long ldbase, Elf64_Dyn *dyn,
- EFI_HANDLE image EFI_UNUSED,
- EFI_SYSTEM_TABLE *systab EFI_UNUSED)
+efi_status_t _relocate(long ldbase, Elf64_Dyn *dyn, efi_handle_t handle, efi_system_table_t *sys_tab)
{
long relsz = 0, relent = 0;
Elf64_Rel *rel = 0;
diff --git a/x86/efi/run b/x86/efi/run
new file mode 100755
index 0000000..72ad4a9
--- /dev/null
+++ b/x86/efi/run
@@ -0,0 +1,63 @@
+#!/bin/bash
+
+set -e
+
+if [ $# -eq 0 ]; then
+ echo "Usage $0 TEST_CASE [QEMU_ARGS]"
+ exit 2
+fi
+
+if [ ! -f config.mak ]; then
+ echo "run './configure --target-efi && make' first. See ./configure -h"
+ exit 2
+fi
+source config.mak
+
+: "${EFI_SRC:=$(realpath "$(dirname "$0")/../")}"
+: "${EFI_UEFI:=/usr/share/ovmf/OVMF.fd}"
+: "${EFI_TEST:=efi-tests}"
+: "${EFI_SMP:=1}"
+: "${EFI_CASE:=$(basename $1 .efi)}"
+
+if [ ! -f "$EFI_UEFI" ]; then
+ echo "UEFI firmware not found: $EFI_UEFI"
+ echo "Please install the UEFI firmware to this path"
+ echo "Or specify the correct path with the env variable EFI_UEFI"
+ exit 2
+fi
+
+# Remove the TEST_CASE from $@
+shift 1
+
+# Prepare EFI boot file system
+# - Copy .efi file to host dir $EFI_TEST/$EFI_CASE/
+# This host dir will be loaded by QEMU as a FAT32 image
+# - Make UEFI startup script that runs the .efi on boot
+mkdir -p "$EFI_TEST/$EFI_CASE/"
+cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_TEST/$EFI_CASE/"
+
+pushd "$EFI_TEST/$EFI_CASE" || exit 2
+# 'startup.nsh' is the default script executed by UEFI on boot
+# Use this script to run the test binary automatically
+cat << EOF >startup.nsh
+@echo -off
+fs0:
+"$EFI_CASE.efi"
+EOF
+popd || exit 2
+
+# Run test case with 256MiB QEMU memory. QEMU default memory size is 128MiB.
+# After UEFI boot up and we call `LibMemoryMap()`, the largest consecutive
+# memory region is ~42MiB. Although this is sufficient for many test cases to
+# run in UEFI, some test cases, e.g. `x86/pmu.c`, require more free memory. A
+# simple fix is to increase the QEMU default memory size to 256MiB so that
+# UEFI's largest allocatable memory region is large enough.
+EFI_RUN=y \
+"$TEST_DIR/run" \
+ -drive file="$EFI_UEFI",format=raw,if=pflash \
+ -drive file.dir="$EFI_TEST/$EFI_CASE/",file.driver=vvfat,file.rw=on,format=raw \
+ -net none \
+ -nographic \
+ -smp "$EFI_SMP" \
+ -m 256 \
+ "$@"
diff --git a/x86/run b/x86/run
index 8b2425f..4eba2b9 100755
--- a/x86/run
+++ b/x86/run
@@ -38,7 +38,19 @@ else
fi
command="${qemu} --no-reboot -nodefaults $pc_testdev -vnc none -serial stdio $pci_testdev"
-command+=" -machine accel=$ACCEL -kernel"
+command+=" -machine accel=$ACCEL"
+if ! [ "$EFI_RUN" ]; then
+ command+=" -kernel"
+fi
command="$(timeout_cmd) $command"
-run_qemu ${command} "$@"
+if [ "$EFI_RUN" ]; then
+ # Set ENVIRON_DEFAULT=n to remove '-initrd' flag for QEMU (see
+ # 'scripts/arch-run.bash' for more details). This is because when using
+ # UEFI, the test case binaries are passed to QEMU through the disk
+ # image, not through the '-kernel' flag. And QEMU reports an error if it
+ # gets '-initrd' without a '-kernel'
+ ENVIRON_DEFAULT=n run_qemu ${command} "$@"
+else
+ run_qemu ${command} "$@"
+fi
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 04/17] x86 UEFI: Boot from UEFI
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 04/17] x86 UEFI: Boot from UEFI Zixuan Wang
@ 2021-10-04 12:55 ` Andrew Jones
2021-10-04 21:30 ` Zixuan Wang
0 siblings, 1 reply; 47+ messages in thread
From: Andrew Jones @ 2021-10-04 12:55 UTC (permalink / raw)
To: Zixuan Wang
Cc: kvm, pbonzini, marcorr, baekhw, tmroeder, erdemaktas, rientjes,
seanjc, brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel,
bp
On Fri, Aug 27, 2021 at 03:12:09AM +0000, Zixuan Wang wrote:
> This commit provides initial support for x86 test cases to boot from
> UEFI:
>
> 1. UEFI compiler flags are added to Makefile
> 2. A new TARGET_EFI macro is added to turn on/off UEFI startup code
> 3. Previous Multiboot setup code is refactored and updated for
> supporting UEFI, including the following changes:
> 1. x86/efi/crt0-efi-x86_64.S: provides entry point and jumps to
> setup code in lib/efi.c.
> 2. lib/efi.c: performs UEFI setup, calls arch-related setup
> functions, then jumps to test case main() function
> 3. lib/x86/setup.c: provides arch-related setup under UEFI
>
> To build test cases for UEFI, please first install the GNU-EFI library.
> Check x86/efi/README.md for more details.
>
> This commit is tested by a simple test calling report() and
> report_summayr(). This commit does not include such a test to avoid
> unnecessary files added into git history. To build and run this test in
> UEFI (assuming file name is x86/dummy.c):
>
> ./configure --target-efi
> make x86/dummy.efi
> ./x86/efi/run ./x86/dummy.efi
>
> To use the default Multiboot instead of UEFI:
>
> ./configure
> make x86/dummy.flat
> ./x86/run ./x86/dummy.flat
>
> Some x86 test cases require additional fixes to work in UEFI, e.g.,
> converting to position independent code (PIC), setting up page tables,
> etc. This commit does not provide these fixes, so compiling and running
> UEFI test cases other than x86/dummy.c may trigger compiler errors or
> QEMU crashes. These test cases will be fixed by the follow-up commits in
> this series.
>
> The following code is ported from github.com/rhdrjones/kvm-unit-tests
> - ./configure: 'target-efi'-related code
>
> See original code:
> - Repo: https://github.com/rhdrjones/kvm-unit-tests
> - Branch: target-efi
>
> Co-developed-by: Varad Gautam <varad.gautam@suse.com>
> Signed-off-by: Varad Gautam <varad.gautam@suse.com>
> Signed-off-by: Zixuan Wang <zixuanwang@google.com>
> ---
> .gitignore | 3 ++
> Makefile | 29 +++++++++++++++++-
> README.md | 6 ++++
> configure | 6 ++++
> lib/efi.c | 38 ++++++++++++++++++++----
> lib/efi.h | 17 +++++++++++
> lib/linux/uefi.h | 10 +++++--
> lib/x86/asm/setup.h | 11 +++++++
> lib/x86/setup.c | 15 ++++++++++
> x86/Makefile.common | 67 +++++++++++++++++++++++++++++++-----------
> x86/Makefile.i386 | 5 ++--
> x86/Makefile.x86_64 | 54 ++++++++++++++++++++++------------
> x86/efi/README.md | 40 ++++++++++++++++++++++++-
> x86/efi/reloc_x86_64.c | 9 ++----
> x86/efi/run | 63 +++++++++++++++++++++++++++++++++++++++
> x86/run | 16 ++++++++--
> 16 files changed, 333 insertions(+), 56 deletions(-)
> create mode 100644 lib/efi.h
> create mode 100644 lib/x86/asm/setup.h
> create mode 100755 x86/efi/run
>
> diff --git a/.gitignore b/.gitignore
> index b3cf2cb..dca6d29 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -3,7 +3,9 @@ tags
> *.a
> *.d
> *.o
> +*.so
> *.flat
> +*.efi
> *.elf
> .pc
> patches
> @@ -24,3 +26,4 @@ cscope.*
> /api/dirty-log-perf
> /s390x/*.bin
> /s390x/snippets/*/*.gbin
> +/efi-tests/*
> diff --git a/Makefile b/Makefile
> index f7b9f28..c4a3905 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -38,6 +38,29 @@ LIBFDT_archive = $(LIBFDT_objdir)/libfdt.a
>
> OBJDIRS += $(LIBFDT_objdir)
>
> +# EFI App
> +ifeq ($(TARGET_EFI),y)
> +ifeq ($(ARCH_NAME),x86_64)
> +EFI_ARCH = x86_64
> +else
> +$(error Cannot build $(ARCH_NAME) tests as EFI apps)
> +endif
> +EFI_CFLAGS := -DTARGET_EFI
> +# The following CFLAGS and LDFLAGS come from:
> +# - GNU-EFI/Makefile.defaults
> +# - GNU-EFI/apps/Makefile
> +# Function calls must include the number of arguments passed to the functions
> +# More details: https://wiki.osdev.org/GNU-EFI
> +EFI_CFLAGS += -maccumulate-outgoing-args
> +# GCC defines wchar to be 32 bits, but EFI expects 16 bits
> +EFI_CFLAGS += -fshort-wchar
> +# EFI applications use PIC as they are loaded to dynamic addresses, not a fixed
> +# starting address
> +EFI_CFLAGS += -fPIC
> +# Create shared library
> +EFI_LDFLAGS := -Bsymbolic -shared -nostdlib
> +endif
> +
> #include architecture specific make rules
> include $(SRCDIR)/$(TEST_DIR)/Makefile
>
> @@ -62,7 +85,11 @@ COMMON_CFLAGS += $(fno_stack_protector)
> COMMON_CFLAGS += $(fno_stack_protector_all)
> COMMON_CFLAGS += $(wno_frame_address)
> COMMON_CFLAGS += $(if $(U32_LONG_FMT),-D__U32_LONG_FMT__,)
> +ifeq ($(TARGET_EFI),y)
> +COMMON_CFLAGS += $(EFI_CFLAGS)
> +else
> COMMON_CFLAGS += $(fno_pic) $(no_pie)
> +endif
> COMMON_CFLAGS += $(wclobbered)
> COMMON_CFLAGS += $(wunused_but_set_parameter)
>
> @@ -113,7 +140,7 @@ clean: arch_clean libfdt_clean
>
> distclean: clean
> $(RM) lib/asm lib/config.h config.mak $(TEST_DIR)-run msr.out cscope.* build-head
> - $(RM) -r tests logs logs.old
> + $(RM) -r tests logs logs.old efi-tests
>
> cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
> cscope:
> diff --git a/README.md b/README.md
> index b498aaf..6edacfe 100644
> --- a/README.md
> +++ b/README.md
> @@ -17,6 +17,8 @@ in this directory. Test images are created in ./ARCH/\*.flat
>
> NOTE: GCC cross-compiler is required for [build on macOS](README.macOS.md).
>
> +To build with UEFI, check [build and run with UEFI](./x86/efi/README.md).
> +
> ## Standalone tests
>
> The tests can be built as standalone. To create and use standalone tests do:
> @@ -54,6 +56,10 @@ ACCEL=name environment variable:
>
> ACCEL=kvm ./x86-run ./x86/msr.flat
>
> +## Running the tests with UEFI
> +
> +Check [build and run with UEFI](./x86/efi/README.md).
> +
> # Tests configuration file
>
> The test case may need specific runtime configurations, for
> diff --git a/configure b/configure
> index 1d4d855..b6c09b3 100755
> --- a/configure
> +++ b/configure
> @@ -28,6 +28,7 @@ erratatxt="$srcdir/errata.txt"
> host_key_document=
> page_size=
> earlycon=
> +target_efi=
>
> usage() {
> cat <<-EOF
> @@ -69,6 +70,7 @@ usage() {
> pl011,mmio32,ADDR
> Specify a PL011 compatible UART at address ADDR. Supported
> register stride is 32 bit only.
> + --target-efi Boot and run from UEFI
> EOF
> exit 1
> }
> @@ -133,6 +135,9 @@ while [[ "$1" = -* ]]; do
> --earlycon)
> earlycon="$arg"
> ;;
> + --target-efi)
> + target_efi=y
> + ;;
> --help)
> usage
> ;;
> @@ -341,6 +346,7 @@ U32_LONG_FMT=$u32_long
> WA_DIVIDE=$wa_divide
> GENPROTIMG=${GENPROTIMG-genprotimg}
> HOST_KEY_DOCUMENT=$host_key_document
> +TARGET_EFI=$target_efi
> EOF
> if [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then
> echo "TARGET=$target" >> config.mak
> diff --git a/lib/efi.c b/lib/efi.c
> index 9711354..99307db 100644
> --- a/lib/efi.c
> +++ b/lib/efi.c
> @@ -1,6 +1,24 @@
> +/*
> + * efi_main() function to set up and run test cases in EFI
> + *
> + * Copyright (c) 2021, SUSE, Varad Gautam <varad.gautam@suse.com>
> + * Copyright (c) 2021, Google Inc, Zixuan Wang <zixuanwang@google.com>
> + *
> + * SPDX-License-Identifier: LGPL-2.0-or-later
> + */
> +
> #include <linux/uefi.h>
> +#include <libcflat.h>
> +#include <asm/setup.h>
> +#include "efi.h"
> +
> +/* From lib/argv.c */
> +extern int __argc, __envc;
> +extern char *__argv[100];
> +extern char *__environ[200];
> +
> +extern int main(int argc, char **argv, char **envp);
>
> -unsigned long __efiapi efi_main(efi_handle_t handle, efi_system_table_t *sys_tab);
> efi_system_table_t *efi_system_table = NULL;
>
> static void efi_free_pool(void *ptr)
> @@ -8,7 +26,7 @@ static void efi_free_pool(void *ptr)
> efi_bs_call(free_pool, ptr);
> }
>
> -static efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
> +efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
> {
> efi_memory_desc_t *m = NULL;
> efi_status_t status;
> @@ -44,15 +62,23 @@ out:
> return status;
> }
>
> -static efi_status_t efi_exit_boot_services(void *handle,
> - struct efi_boot_memmap *map)
> +efi_status_t efi_exit_boot_services(void *handle, struct efi_boot_memmap *map)
> {
> return efi_bs_call(exit_boot_services, handle, *map->key_ptr);
> }
>
> -unsigned long __efiapi efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> +efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> {
> + int ret;
> +
> efi_system_table = sys_tab;
>
> - return 0;
> + setup_efi();
> + ret = main(__argc, __argv, __environ);
In my new AArch64 PoC I call an 'efi_setup' from 'start' in cstart64.S
and then when that returns 'main' is called like normal.
> +
> + /* Shutdown the guest VM */
> + efi_rs_call(reset_system, EFI_RESET_SHUTDOWN, ret, 0, NULL);
> +
> + /* Unreachable */
> + return EFI_UNSUPPORTED;
> }
> diff --git a/lib/efi.h b/lib/efi.h
> new file mode 100644
> index 0000000..60cdb6f
> --- /dev/null
> +++ b/lib/efi.h
> @@ -0,0 +1,17 @@
> +#ifndef _EFI_H_
> +#define _EFI_H_
> +
> +/*
> + * EFI-related functions in KVM-Unit-Tests. This file's name "efi.h" is in
> + * conflict with GNU-EFI library's "efi.h", but KVM-Unit-Tests does not include
> + * GNU-EFI headers or links against GNU-EFI.
> + */
> +#include "linux/uefi.h"
> +#include <elf.h>
> +
> +efi_status_t _relocate(long ldbase, Elf64_Dyn *dyn, efi_handle_t handle, efi_system_table_t *sys_tab);
> +efi_status_t efi_get_memory_map(struct efi_boot_memmap *map);
> +efi_status_t efi_exit_boot_services(void *handle, struct efi_boot_memmap *map);
> +efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab);
> +
> +#endif /* _EFI_H_ */
> diff --git a/lib/linux/uefi.h b/lib/linux/uefi.h
> index 567cddc..9adc7ab 100644
> --- a/lib/linux/uefi.h
> +++ b/lib/linux/uefi.h
> @@ -4,6 +4,12 @@
> #ifndef __LINUX_UEFI_H
> #define __LINUX_UEFI_H
>
> +#include "libcflat.h"
> +
> +#ifndef __packed
> +# define __packed __attribute__((__packed__))
> +#endif
> +
> #define BITS_PER_LONG 64
>
> #define EFI_SUCCESS 0
> @@ -512,7 +518,7 @@ struct efi_boot_memmap {
> unsigned long *buff_size;
> };
>
> -#define efi_bs_call(func, ...) \
> - efi_system_table->boottime->func(__VA_ARGS__)
> +#define efi_bs_call(func, ...) efi_system_table->boottime->func(__VA_ARGS__)
> +#define efi_rs_call(func, ...) efi_system_table->runtime->func(__VA_ARGS__)
>
> #endif /* __LINUX_UEFI_H */
> diff --git a/lib/x86/asm/setup.h b/lib/x86/asm/setup.h
> new file mode 100644
> index 0000000..eb1cf73
> --- /dev/null
> +++ b/lib/x86/asm/setup.h
> @@ -0,0 +1,11 @@
> +#ifndef _X86_ASM_SETUP_H_
> +#define _X86_ASM_SETUP_H_
> +
> +#ifdef TARGET_EFI
> +#include "x86/apic.h"
> +#include "x86/smp.h"
> +
> +void setup_efi(void);
> +#endif /* TARGET_EFI */
> +
> +#endif /* _X86_ASM_SETUP_H_ */
> diff --git a/lib/x86/setup.c b/lib/x86/setup.c
> index 7befe09..efb5ecd 100644
> --- a/lib/x86/setup.c
> +++ b/lib/x86/setup.c
> @@ -2,6 +2,7 @@
> * Initialize machine setup information
> *
> * Copyright (C) 2017, Red Hat Inc, Andrew Jones <drjones@redhat.com>
> + * Copyright (C) 2021, Google Inc, Zixuan Wang <zixuanwang@google.com>
> *
> * This work is licensed under the terms of the GNU LGPL, version 2.
> */
> @@ -9,6 +10,7 @@
> #include "fwcfg.h"
> #include "alloc_phys.h"
> #include "argv.h"
> +#include "asm/setup.h"
>
> extern char edata;
>
> @@ -118,6 +120,19 @@ void setup_multiboot(struct mbi_bootinfo *bi)
> initrd_size = mods->end - mods->start;
> }
>
> +#ifdef TARGET_EFI
> +
> +void setup_efi(void)
> +{
> + reset_apic();
> + mask_pic_interrupts();
> + enable_apic();
> + enable_x2apic();
> + smp_init();
> +}
> +
> +#endif /* TARGET_EFI */
> +
> void setup_libcflat(void)
> {
> if (initrd) {
> diff --git a/x86/Makefile.common b/x86/Makefile.common
> index 52bb7aa..4859bf3 100644
> --- a/x86/Makefile.common
> +++ b/x86/Makefile.common
> @@ -22,6 +22,11 @@ cflatobjs += lib/x86/acpi.o
> cflatobjs += lib/x86/stack.o
> cflatobjs += lib/x86/fault_test.o
> cflatobjs += lib/x86/delay.o
> +ifeq ($(TARGET_EFI),y)
> +cflatobjs += lib/x86/setup.o
> +cflatobjs += lib/efi.o
> +cflatobjs += x86/efi/reloc_x86_64.o
> +endif
>
> OBJDIRS += lib/x86
>
> @@ -37,10 +42,25 @@ COMMON_CFLAGS += -O1
> # stack.o relies on frame pointers.
> KEEP_FRAME_POINTER := y
>
> -# We want to keep intermediate file: %.elf and %.o
> +FLATLIBS = lib/libcflat.a
> +
> +ifeq ($(TARGET_EFI),y)
> +.PRECIOUS: %.efi %.so
> +
> +%.so: %.o $(FLATLIBS) $(SRCDIR)/x86/efi/elf_x86_64_efi.lds $(cstart.o)
> + $(LD) -T $(SRCDIR)/x86/efi/elf_x86_64_efi.lds $(EFI_LDFLAGS) -o $@ \
> + $(filter %.o, $^) $(FLATLIBS)
> + @chmod a-x $@
> +
> +%.efi: %.so
> + $(OBJCOPY) \
> + -j .text -j .sdata -j .data -j .dynamic -j .dynsym -j .rel \
> + -j .rela -j .reloc -S --target=$(FORMAT) $< $@
> + @chmod a-x $@
> +else
> +# We want to keep intermediate file: %.elf and %.o
> .PRECIOUS: %.elf %.o
>
> -FLATLIBS = lib/libcflat.a
> %.elf: %.o $(FLATLIBS) $(SRCDIR)/x86/flat.lds $(cstart.o)
> $(CC) $(CFLAGS) -nostdlib -o $@ -Wl,-T,$(SRCDIR)/x86/flat.lds \
> $(filter %.o, $^) $(FLATLIBS)
> @@ -49,18 +69,29 @@ FLATLIBS = lib/libcflat.a
> %.flat: %.elf
> $(OBJCOPY) -O elf32-i386 $^ $@
> @chmod a-x $@
> +endif
>
> -tests-common = $(TEST_DIR)/vmexit.flat $(TEST_DIR)/tsc.flat \
> - $(TEST_DIR)/smptest.flat \
> - $(TEST_DIR)/realmode.flat $(TEST_DIR)/msr.flat \
> - $(TEST_DIR)/hypercall.flat $(TEST_DIR)/sieve.flat \
> - $(TEST_DIR)/kvmclock_test.flat $(TEST_DIR)/eventinj.flat \
> - $(TEST_DIR)/s3.flat $(TEST_DIR)/pmu.flat $(TEST_DIR)/setjmp.flat \
> - $(TEST_DIR)/tsc_adjust.flat $(TEST_DIR)/asyncpf.flat \
> - $(TEST_DIR)/init.flat $(TEST_DIR)/smap.flat \
> - $(TEST_DIR)/hyperv_synic.flat $(TEST_DIR)/hyperv_stimer.flat \
> - $(TEST_DIR)/hyperv_connections.flat \
> - $(TEST_DIR)/umip.flat $(TEST_DIR)/tsx-ctrl.flat
> +tests-common = $(TEST_DIR)/vmexit.$(exe) $(TEST_DIR)/tsc.$(exe) \
> + $(TEST_DIR)/smptest.$(exe) \
> + $(TEST_DIR)/msr.$(exe) \
> + $(TEST_DIR)/hypercall.$(exe) $(TEST_DIR)/sieve.$(exe) \
> + $(TEST_DIR)/kvmclock_test.$(exe) \
> + $(TEST_DIR)/s3.$(exe) $(TEST_DIR)/pmu.$(exe) $(TEST_DIR)/setjmp.$(exe) \
> + $(TEST_DIR)/tsc_adjust.$(exe) $(TEST_DIR)/asyncpf.$(exe) \
> + $(TEST_DIR)/init.$(exe) \
> + $(TEST_DIR)/hyperv_synic.$(exe) $(TEST_DIR)/hyperv_stimer.$(exe) \
> + $(TEST_DIR)/hyperv_connections.$(exe) \
> + $(TEST_DIR)/tsx-ctrl.$(exe)
> +
> +# The following test cases are disabled when building EFI tests because they
> +# use absolute addresses in their inline assembly code, which cannot compile
> +# with the '-fPIC' flag
> +ifneq ($(TARGET_EFI),y)
> +tests-common += $(TEST_DIR)/eventinj.$(exe) \
> + $(TEST_DIR)/smap.$(exe) \
> + $(TEST_DIR)/realmode.$(exe) \
> + $(TEST_DIR)/umip.$(exe)
> +endif
>
> test_cases: $(tests-common) $(tests)
>
> @@ -72,14 +103,16 @@ $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o
>
> $(TEST_DIR)/realmode.o: bits = $(if $(call cc-option,-m16,""),16,32)
>
> -$(TEST_DIR)/kvmclock_test.elf: $(TEST_DIR)/kvmclock.o
> +$(TEST_DIR)/kvmclock_test.$(bin): $(TEST_DIR)/kvmclock.o
>
> -$(TEST_DIR)/hyperv_synic.elf: $(TEST_DIR)/hyperv.o
> +$(TEST_DIR)/hyperv_synic.$(bin): $(TEST_DIR)/hyperv.o
>
> -$(TEST_DIR)/hyperv_stimer.elf: $(TEST_DIR)/hyperv.o
> +$(TEST_DIR)/hyperv_stimer.$(bin): $(TEST_DIR)/hyperv.o
>
> -$(TEST_DIR)/hyperv_connections.elf: $(TEST_DIR)/hyperv.o
> +$(TEST_DIR)/hyperv_connections.$(bin): $(TEST_DIR)/hyperv.o
>
> arch_clean:
> $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
> $(TEST_DIR)/.*.d lib/x86/.*.d \
> + $(TEST_DIR)/efi/*.o $(TEST_DIR)/efi/.*.d \
> + $(TEST_DIR)/*.so $(TEST_DIR)/*.efi
> diff --git a/x86/Makefile.i386 b/x86/Makefile.i386
> index 960e274..340c561 100644
> --- a/x86/Makefile.i386
> +++ b/x86/Makefile.i386
> @@ -1,11 +1,12 @@
> cstart.o = $(TEST_DIR)/cstart.o
> bits = 32
> ldarch = elf32-i386
> +exe = flat
> COMMON_CFLAGS += -mno-sse -mno-sse2
>
> cflatobjs += lib/x86/setjmp32.o lib/ldiv32.o
>
> -tests = $(TEST_DIR)/taskswitch.flat $(TEST_DIR)/taskswitch2.flat \
> - $(TEST_DIR)/cmpxchg8b.flat $(TEST_DIR)/la57.flat
> +tests = $(TEST_DIR)/taskswitch.$(exe) $(TEST_DIR)/taskswitch2.$(exe) \
> + $(TEST_DIR)/cmpxchg8b.$(exe) $(TEST_DIR)/la57.$(exe)
>
> include $(SRCDIR)/$(TEST_DIR)/Makefile.common
> diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
> index 8134952..a5f8923 100644
> --- a/x86/Makefile.x86_64
> +++ b/x86/Makefile.x86_64
> @@ -1,6 +1,15 @@
> cstart.o = $(TEST_DIR)/cstart64.o
> bits = 64
> ldarch = elf64-x86-64
> +ifeq ($(TARGET_EFI),y)
> +exe = efi
> +bin = so
> +FORMAT = efi-app-x86_64
> +cstart.o = x86/efi/crt0-efi-x86_64.o
> +else
> +exe = flat
> +bin = elf
> +endif
>
> fcf_protection_full := $(call cc-option, -fcf-protection=full,)
> COMMON_CFLAGS += -mno-red-zone -mno-sse -mno-sse2 $(fcf_protection_full)
> @@ -9,29 +18,36 @@ cflatobjs += lib/x86/setjmp64.o
> cflatobjs += lib/x86/intel-iommu.o
> cflatobjs += lib/x86/usermode.o
>
> -tests = $(TEST_DIR)/access.flat $(TEST_DIR)/apic.flat \
> - $(TEST_DIR)/emulator.flat $(TEST_DIR)/idt_test.flat \
> - $(TEST_DIR)/xsave.flat $(TEST_DIR)/rmap_chain.flat \
> - $(TEST_DIR)/pcid.flat $(TEST_DIR)/debug.flat \
> - $(TEST_DIR)/ioapic.flat $(TEST_DIR)/memory.flat \
> - $(TEST_DIR)/pku.flat $(TEST_DIR)/hyperv_clock.flat
> -tests += $(TEST_DIR)/syscall.flat
> -tests += $(TEST_DIR)/svm.flat
> -tests += $(TEST_DIR)/vmx.flat
> -tests += $(TEST_DIR)/tscdeadline_latency.flat
> -tests += $(TEST_DIR)/intel-iommu.flat
> -tests += $(TEST_DIR)/vmware_backdoors.flat
> -tests += $(TEST_DIR)/rdpru.flat
> -tests += $(TEST_DIR)/pks.flat
> -tests += $(TEST_DIR)/pmu_lbr.flat
> +tests = $(TEST_DIR)/apic.$(exe) \
> + $(TEST_DIR)/idt_test.$(exe) \
> + $(TEST_DIR)/xsave.$(exe) $(TEST_DIR)/rmap_chain.$(exe) \
> + $(TEST_DIR)/pcid.$(exe) $(TEST_DIR)/debug.$(exe) \
> + $(TEST_DIR)/ioapic.$(exe) $(TEST_DIR)/memory.$(exe) \
> + $(TEST_DIR)/pku.$(exe) $(TEST_DIR)/hyperv_clock.$(exe)
> +tests += $(TEST_DIR)/syscall.$(exe)
> +tests += $(TEST_DIR)/tscdeadline_latency.$(exe)
> +tests += $(TEST_DIR)/intel-iommu.$(exe)
> +tests += $(TEST_DIR)/rdpru.$(exe)
> +tests += $(TEST_DIR)/pks.$(exe)
> +tests += $(TEST_DIR)/pmu_lbr.$(exe)
>
> +# The following test cases are disabled when building EFI tests because they
> +# use absolute addresses in their inline assembly code, which cannot compile
> +# with the '-fPIC' flag
> +ifneq ($(TARGET_EFI),y)
> +tests += $(TEST_DIR)/access.$(exe)
> +tests += $(TEST_DIR)/emulator.$(exe)
> +tests += $(TEST_DIR)/svm.$(exe)
> +tests += $(TEST_DIR)/vmx.$(exe)
> +tests += $(TEST_DIR)/vmware_backdoors.$(exe)
> ifneq ($(fcf_protection_full),)
> -tests += $(TEST_DIR)/cet.flat
> +tests += $(TEST_DIR)/cet.$(exe)
> +endif
> endif
>
> include $(SRCDIR)/$(TEST_DIR)/Makefile.common
>
> -$(TEST_DIR)/hyperv_clock.elf: $(TEST_DIR)/hyperv_clock.o
> +$(TEST_DIR)/hyperv_clock.$(bin): $(TEST_DIR)/hyperv_clock.o
>
> -$(TEST_DIR)/vmx.elf: $(TEST_DIR)/vmx_tests.o
> -$(TEST_DIR)/svm.elf: $(TEST_DIR)/svm_tests.o
> +$(TEST_DIR)/vmx.$(bin): $(TEST_DIR)/vmx_tests.o
> +$(TEST_DIR)/svm.$(bin): $(TEST_DIR)/svm_tests.o
> diff --git a/x86/efi/README.md b/x86/efi/README.md
> index 256ef8c..befe9cc 100644
> --- a/x86/efi/README.md
> +++ b/x86/efi/README.md
> @@ -1,4 +1,38 @@
> -# EFI Startup Code and Linker Script
> +# Build KVM-Unit-Tests and run under UEFI
> +
> +## Introduction
> +
> +This dir provides code to build KVM-Unit-Tests test cases and run them under
> +QEMU and UEFI.
> +
> +### Install dependencies
> +
> +The following dependencies should be installed:
> +
> +- [UEFI firmware](https://github.com/tianocore/edk2): to run test cases in QEMU
> +
> +### Build
> +
> +To build:
> +
> + ./configure --target-efi
> + make
> +
> +### Run test cases with UEFI
> +
> +To run a test case with UEFI:
> +
> + ./x86/efi/run ./x86/dummy.efi
> +
> +By default the runner script loads the UEFI firmware `/usr/share/ovmf/OVMF.fd`;
> +please install UEFI firmware to this path, or specify the correct path through
> +the env variable `EFI_UEFI`:
> +
> + EFI_UEFI=/path/to/OVMF.fd ./x86/efi/run ./x86/dummy.efi
> +
> +## Code structure
> +
> +### Code from GNU-EFI
>
> This dir contains source code and linker script copied from
> [GNU-EFI](https://sourceforge.net/projects/gnu-efi/):
> @@ -23,3 +57,7 @@ Relocatable Binaries".
>
> KVM-Unit-Tests follows a similar build process, but does not link with GNU-EFI
> library.
> +### Startup code for KVM-Unit-Tests in UEFI
> +
> +This dir also contains KVM-Unit-Tests startup code in UEFI:
> + - efistart64.S: startup code for KVM-Unit-Tests in UEFI
> diff --git a/x86/efi/reloc_x86_64.c b/x86/efi/reloc_x86_64.c
> index d13b53e..511ef82 100644
> --- a/x86/efi/reloc_x86_64.c
> +++ b/x86/efi/reloc_x86_64.c
> @@ -37,14 +37,11 @@
> SUCH DAMAGE.
> */
>
> -#include <efi.h>
> -#include <efilib.h>
> -
> +#include "linux/uefi.h"
> +#include "efi.h"
> #include <elf.h>
>
> -EFI_STATUS _relocate (long ldbase, Elf64_Dyn *dyn,
> - EFI_HANDLE image EFI_UNUSED,
> - EFI_SYSTEM_TABLE *systab EFI_UNUSED)
> +efi_status_t _relocate(long ldbase, Elf64_Dyn *dyn, efi_handle_t handle, efi_system_table_t *sys_tab)
I think these conversions should be done as separate patches after the
import patches.
> {
> long relsz = 0, relent = 0;
> Elf64_Rel *rel = 0;
> diff --git a/x86/efi/run b/x86/efi/run
> new file mode 100755
> index 0000000..72ad4a9
> --- /dev/null
> +++ b/x86/efi/run
> @@ -0,0 +1,63 @@
> +#!/bin/bash
> +
> +set -e
> +
> +if [ $# -eq 0 ]; then
> + echo "Usage $0 TEST_CASE [QEMU_ARGS]"
> + exit 2
> +fi
> +
> +if [ ! -f config.mak ]; then
> + echo "run './configure --target-efi && make' first. See ./configure -h"
> + exit 2
> +fi
> +source config.mak
> +
> +: "${EFI_SRC:=$(realpath "$(dirname "$0")/../")}"
> +: "${EFI_UEFI:=/usr/share/ovmf/OVMF.fd}"
> +: "${EFI_TEST:=efi-tests}"
> +: "${EFI_SMP:=1}"
> +: "${EFI_CASE:=$(basename $1 .efi)}"
> +
> +if [ ! -f "$EFI_UEFI" ]; then
> + echo "UEFI firmware not found: $EFI_UEFI"
> + echo "Please install the UEFI firmware to this path"
> + echo "Or specify the correct path with the env variable EFI_UEFI"
> + exit 2
> +fi
> +
> +# Remove the TEST_CASE from $@
> +shift 1
> +
> +# Prepare EFI boot file system
> +# - Copy .efi file to host dir $EFI_TEST/$EFI_CASE/
> +# This host dir will be loaded by QEMU as a FAT32 image
> +# - Make UEFI startup script that runs the .efi on boot
> +mkdir -p "$EFI_TEST/$EFI_CASE/"
> +cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_TEST/$EFI_CASE/"
> +
> +pushd "$EFI_TEST/$EFI_CASE" || exit 2
> +# 'startup.nsh' is the default script executed by UEFI on boot
> +# Use this script to run the test binary automatically
> +cat << EOF >startup.nsh
> +@echo -off
> +fs0:
> +"$EFI_CASE.efi"
> +EOF
> +popd || exit 2
> +
> +# Run test case with 256MiB QEMU memory. QEMU default memory size is 128MiB.
> +# After UEFI boot up and we call `LibMemoryMap()`, the largest consecutive
> +# memory region is ~42MiB. Although this is sufficient for many test cases to
> +# run in UEFI, some test cases, e.g. `x86/pmu.c`, require more free memory. A
> +# simple fix is to increase the QEMU default memory size to 256MiB so that
> +# UEFI's largest allocatable memory region is large enough.
> +EFI_RUN=y \
> +"$TEST_DIR/run" \
> + -drive file="$EFI_UEFI",format=raw,if=pflash \
Also need the 'readonly' property on this drive.
> + -drive file.dir="$EFI_TEST/$EFI_CASE/",file.driver=vvfat,file.rw=on,format=raw \
> + -net none \
> + -nographic \
> + -smp "$EFI_SMP" \
> + -m 256 \
> + "$@"
> diff --git a/x86/run b/x86/run
> index 8b2425f..4eba2b9 100755
> --- a/x86/run
> +++ b/x86/run
> @@ -38,7 +38,19 @@ else
> fi
>
> command="${qemu} --no-reboot -nodefaults $pc_testdev -vnc none -serial stdio $pci_testdev"
> -command+=" -machine accel=$ACCEL -kernel"
> +command+=" -machine accel=$ACCEL"
> +if ! [ "$EFI_RUN" ]; then
> + command+=" -kernel"
> +fi
> command="$(timeout_cmd) $command"
>
> -run_qemu ${command} "$@"
> +if [ "$EFI_RUN" ]; then
> + # Set ENVIRON_DEFAULT=n to remove '-initrd' flag for QEMU (see
> + # 'scripts/arch-run.bash' for more details). This is because when using
> + # UEFI, the test case binaries are passed to QEMU through the disk
> + # image, not through the '-kernel' flag. And QEMU reports an error if it
> + # gets '-initrd' without a '-kernel'
> + ENVIRON_DEFAULT=n run_qemu ${command} "$@"
> +else
> + run_qemu ${command} "$@"
> +fi
> --
> 2.33.0.259.gc128427fd7-goog
>
Thanks,
drew
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 04/17] x86 UEFI: Boot from UEFI
2021-10-04 12:55 ` Andrew Jones
@ 2021-10-04 21:30 ` Zixuan Wang
0 siblings, 0 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-10-04 21:30 UTC (permalink / raw)
To: Andrew Jones
Cc: Zixuan Wang, kvm, Paolo Bonzini, Marc Orr, Hyunwook (Wooky) Baek,
Tom Roeder, erdemaktas, rientjes, seanjc, brijesh.singh,
Thomas.Lendacky, varad.gautam, jroedel, bp
On Mon, Oct 4, 2021 at 5:58 AM Andrew Jones <drjones@redhat.com> wrote:
>
> On Fri, Aug 27, 2021 at 03:12:09AM +0000, Zixuan Wang wrote:
> > This commit provides initial support for x86 test cases to boot from
> > -unsigned long __efiapi efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> > +efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> > {
> > + int ret;
> > +
> > efi_system_table = sys_tab;
> >
> > - return 0;
> > + setup_efi();
> > + ret = main(__argc, __argv, __environ);
>
> In my new AArch64 PoC I call an 'efi_setup' from 'start' in cstart64.S
> and then when that returns 'main' is called like normal.
>
x86 is a bit tricky: the cstart64.S 'start' code is compiled to run in
real mode, but C functions are compiled for long mode. From my
experience, calling a long mode function from real mode can crash the
guest VM. So we developed a new start-up assembly that directly starts
from long mode.
I remember Varad's original patchset [1] calls the EFI setup function
from cstart64.S, which introduces several ifdefs to the file.
[1] https://lore.kernel.org/kvm/20210819113400.26516-1-varad.gautam@suse.com/
> > -#include <efi.h>
> > -#include <efilib.h>
> > -
> > +#include "linux/uefi.h"
> > +#include "efi.h"
> > #include <elf.h>
> >
> > -EFI_STATUS _relocate (long ldbase, Elf64_Dyn *dyn,
> > - EFI_HANDLE image EFI_UNUSED,
> > - EFI_SYSTEM_TABLE *systab EFI_UNUSED)
> > +efi_status_t _relocate(long ldbase, Elf64_Dyn *dyn, efi_handle_t handle, efi_system_table_t *sys_tab)
>
> I think these conversions should be done as separate patches after the
> import patches.
>
Got it! I will add this to the next version.
> > +EFI_RUN=y \
> > +"$TEST_DIR/run" \
> > + -drive file="$EFI_UEFI",format=raw,if=pflash \
>
> Also need the 'readonly' property on this drive.
>
I didn't notice this. I will add it to the next version. Thanks for
pointing it out.
> > --
> > 2.33.0.259.gc128427fd7-goog
> >
>
> Thanks,
> drew
>
Best regards,
Zixuan
^ permalink raw reply [flat|nested] 47+ messages in thread
* [kvm-unit-tests PATCH v2 05/17] x86 UEFI: Load IDT after UEFI boot up
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
` (3 preceding siblings ...)
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 04/17] x86 UEFI: Boot from UEFI Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 06/17] x86 UEFI: Load GDT and TSS " Zixuan Wang
` (12 subsequent siblings)
17 siblings, 0 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
Interrupt descriptor table (IDT) is used by x86 arch to describe the
interrupt handlers. UEFI already setup an IDT before running the test
binaries, but this IDT is not compatible with the existing
KVM-Unit-Tests test cases, e.g., x86/msr.c triggers a #GP fault when
using UEFI IDT. This is because test cases setup new interrupt handlers
and register them into KVM-Unit-Tests' IDT, but it takes no effect if we
do not load KVM-Unit-Tests' IDT.
This commit fixes this issue:
1. Port the IDT definition from cstart64.S to efistart64.S
2. Update IDT descriptor with runtime IDT address and load it
In this commit, the x86/msr.c can run in UEFI and generates same output
as the default Seabios version.
Co-developed-by: Varad Gautam <varad.gautam@suse.com>
Signed-off-by: Varad Gautam <varad.gautam@suse.com>
Signed-off-by: Zixuan Wang <zixuanwang@google.com>
---
lib/x86/setup.c | 6 ++++++
x86/Makefile.x86_64 | 2 +-
x86/efi/README.md | 4 ++--
x86/efi/efistart64.S | 34 ++++++++++++++++++++++++++++++++++
x86/efi/elf_x86_64_efi.lds | 6 +++++-
5 files changed, 48 insertions(+), 4 deletions(-)
create mode 100644 x86/efi/efistart64.S
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index efb5ecd..5e369ea 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -10,6 +10,7 @@
#include "fwcfg.h"
#include "alloc_phys.h"
#include "argv.h"
+#include "x86/desc.h"
#include "asm/setup.h"
extern char edata;
@@ -122,9 +123,14 @@ void setup_multiboot(struct mbi_bootinfo *bi)
#ifdef TARGET_EFI
+/* From x86/efi/efistart64.S */
+extern void load_idt(void);
+
void setup_efi(void)
{
reset_apic();
+ setup_idt();
+ load_idt();
mask_pic_interrupts();
enable_apic();
enable_x2apic();
diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
index a5f8923..aa23b22 100644
--- a/x86/Makefile.x86_64
+++ b/x86/Makefile.x86_64
@@ -5,7 +5,7 @@ ifeq ($(TARGET_EFI),y)
exe = efi
bin = so
FORMAT = efi-app-x86_64
-cstart.o = x86/efi/crt0-efi-x86_64.o
+cstart.o = $(TEST_DIR)/efi/efistart64.o
else
exe = flat
bin = elf
diff --git a/x86/efi/README.md b/x86/efi/README.md
index befe9cc..360f9fe 100644
--- a/x86/efi/README.md
+++ b/x86/efi/README.md
@@ -22,13 +22,13 @@ To build:
To run a test case with UEFI:
- ./x86/efi/run ./x86/dummy.efi
+ ./x86/efi/run ./x86/msr.efi
By default the runner script loads the UEFI firmware `/usr/share/ovmf/OVMF.fd`;
please install UEFI firmware to this path, or specify the correct path through
the env variable `EFI_UEFI`:
- EFI_UEFI=/path/to/OVMF.fd ./x86/efi/run ./x86/dummy.efi
+ EFI_UEFI=/path/to/OVMF.fd ./x86/efi/run ./x86/msr.efi
## Code structure
diff --git a/x86/efi/efistart64.S b/x86/efi/efistart64.S
new file mode 100644
index 0000000..ec607da
--- /dev/null
+++ b/x86/efi/efistart64.S
@@ -0,0 +1,34 @@
+/* Startup code and pre-defined data structures */
+
+#include "crt0-efi-x86_64.S"
+
+.globl boot_idt
+.globl idt_descr
+
+.data
+
+boot_idt:
+ .rept 256
+ .quad 0
+ .quad 0
+ .endr
+end_boot_idt:
+
+idt_descr:
+ .word end_boot_idt - boot_idt - 1
+ .quad 0 /* To be filled with runtime addr of boot_idt(%rip) */
+
+.section .init
+.code64
+.text
+
+.globl load_idt
+load_idt:
+ /* Set IDT runtime address */
+ lea boot_idt(%rip), %rax
+ mov %rax, idt_descr+2(%rip)
+
+ /* Load IDT */
+ lidtq idt_descr(%rip)
+
+ retq
diff --git a/x86/efi/elf_x86_64_efi.lds b/x86/efi/elf_x86_64_efi.lds
index 5eae376..3d92c86 100644
--- a/x86/efi/elf_x86_64_efi.lds
+++ b/x86/efi/elf_x86_64_efi.lds
@@ -1,4 +1,4 @@
-/* Copied from GNU-EFI/gnuefi/elf_x86_64_efi.lds, licensed under GNU GPL */
+/* Developed based on GNU-EFI/gnuefi/elf_x86_64_efi.lds, licensed under GNU GPL */
/* Same as elf_x86_64_fbsd_efi.lds, except for OUTPUT_FORMAT below - KEEP IN SYNC */
OUTPUT_FORMAT("elf64-x86-64", "elf64-x86-64", "elf64-x86-64")
OUTPUT_ARCH(i386:x86-64)
@@ -38,6 +38,10 @@ SECTIONS
*(.rodata*)
*(.got.plt)
*(.got)
+ /* Expected by lib/x86/desc.c to store exception_table */
+ exception_table_start = .;
+ *(.data.ex)
+ exception_table_end = .;
*(.data*)
*(.sdata)
/* the EFI loader doesn't seem to like a .bss section, so we stick
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* [kvm-unit-tests PATCH v2 06/17] x86 UEFI: Load GDT and TSS after UEFI boot up
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
` (4 preceding siblings ...)
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 05/17] x86 UEFI: Load IDT after UEFI boot up Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-09-20 15:40 ` Paolo Bonzini
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 07/17] x86 UEFI: Set up memory allocator Zixuan Wang
` (11 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
Global Descriptor Table (GDT) defines x86 memory areas, e.g. memory area
for data or code. UEFI has a different GDT compared to KVM-Unit-Tests,
e.g., in UEFI, 3rd GDT entry defines a code segment, while in
KVM-Unit-Tests, 3rd GDT entry is data segment.
Without loading KVM-Unit-Tests GDT, the UEFI GDT is used and is
incompatible with KVM-Unit-Tests. This causes QEMU to silently crash
when a test case changes segments.
This commit fixes this issue by loading KVM-Unit-Tests GDT to replace
UEFI GDT. And since Task State Segment (TSS) is tightly coupled with
GDT, this commit also loads TSS on boot-up.
The GDT and TSS loading function is originally written in assembly code,
see cstart64.S load_tss function. This commit provides a similar C
function setup_gdt_tss() which is more readable and easier to modify.
In this commit, x86/debug.c can run in UEFI and pass all sub-tests.
Co-developed-by: Varad Gautam <varad.gautam@suse.com>
Signed-off-by: Varad Gautam <varad.gautam@suse.com>
Signed-off-by: Zixuan Wang <zixuanwang@google.com>
---
lib/x86/setup.c | 44 ++++++++++++++++++++++
x86/efi/efistart64.S | 88 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 132 insertions(+)
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index 5e369ea..0a065fe 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -125,10 +125,54 @@ void setup_multiboot(struct mbi_bootinfo *bi)
/* From x86/efi/efistart64.S */
extern void load_idt(void);
+extern void load_gdt_tss(size_t tss_offset);
+extern phys_addr_t tss_descr;
+extern phys_addr_t ring0stacktop;
+extern gdt_entry_t gdt64[];
+extern size_t ring0stacksize;
+
+static void setup_gdt_tss(void)
+{
+ gdt_entry_t *tss_lo, *tss_hi;
+ tss64_t *curr_tss;
+ phys_addr_t curr_tss_addr;
+ u32 id;
+ size_t tss_offset;
+ size_t pre_tss_entries;
+
+ /* Get APIC ID, see also x86/cstart64.S:load_tss */
+ id = apic_id();
+
+ /* Get number of GDT entries before TSS-related GDT entry */
+ pre_tss_entries = (size_t)((u8 *)&(tss_descr) - (u8 *)gdt64) / sizeof(gdt_entry_t);
+
+ /* Each TSS descriptor takes up 2 GDT entries */
+ tss_offset = (pre_tss_entries + id * 2) * sizeof(gdt_entry_t);
+ tss_lo = &(gdt64[pre_tss_entries + id * 2 + 0]);
+ tss_hi = &(gdt64[pre_tss_entries + id * 2 + 1]);
+
+ /* Runtime address of current TSS */
+ curr_tss_addr = (((phys_addr_t)&tss) + (phys_addr_t)(id * sizeof(tss64_t)));
+
+ /* Use runtime address for ring0stacktop, see also x86/cstart64.S:tss */
+ curr_tss = (tss64_t *)curr_tss_addr;
+ curr_tss->rsp0 = (u64)((u8*)&ring0stacktop - id * ring0stacksize);
+
+ /* Update TSS descriptors */
+ tss_lo->limit_low = sizeof(tss64_t);
+ tss_lo->base_low = (u16)(curr_tss_addr & 0xffff);
+ tss_lo->base_middle = (u8)((curr_tss_addr >> 16) & 0xff);
+ tss_lo->base_high = (u8)((curr_tss_addr >> 24) & 0xff);
+ tss_hi->limit_low = (u16)((curr_tss_addr >> 32) & 0xffff);
+ tss_hi->base_low = (u16)((curr_tss_addr >> 48) & 0xffff);
+
+ load_gdt_tss(tss_offset);
+}
void setup_efi(void)
{
reset_apic();
+ setup_gdt_tss();
setup_idt();
load_idt();
mask_pic_interrupts();
diff --git a/x86/efi/efistart64.S b/x86/efi/efistart64.S
index ec607da..a14bd46 100644
--- a/x86/efi/efistart64.S
+++ b/x86/efi/efistart64.S
@@ -1,9 +1,24 @@
/* Startup code and pre-defined data structures */
+#include "apic-defs.h"
+#include "asm-generic/page.h"
#include "crt0-efi-x86_64.S"
.globl boot_idt
.globl idt_descr
+.globl tss_descr
+.globl gdt64_desc
+.globl ring0stacksize
+
+max_cpus = MAX_TEST_CPUS
+ring0stacksize = PAGE_SIZE
+
+.bss
+
+.globl ring0stacktop
+ . = . + ring0stacksize * max_cpus
+ .align 16
+ring0stacktop:
.data
@@ -18,6 +33,48 @@ idt_descr:
.word end_boot_idt - boot_idt - 1
.quad 0 /* To be filled with runtime addr of boot_idt(%rip) */
+gdt64_desc:
+ .word gdt64_end - gdt64 - 1
+ .quad 0 /* To be filled with runtime addr of gdt64(%rip) */
+
+.globl gdt64
+gdt64:
+ .quad 0
+ .quad 0x00af9b000000ffff /* 64-bit code segment */
+ .quad 0x00cf93000000ffff /* 32/64-bit data segment */
+ .quad 0x00af1b000000ffff /* 64-bit code segment, not present */
+ .quad 0x00cf9b000000ffff /* 32-bit code segment */
+ .quad 0x008f9b000000FFFF /* 16-bit code segment */
+ .quad 0x008f93000000FFFF /* 16-bit data segment */
+ .quad 0x00cffb000000ffff /* 32-bit code segment (user) */
+ .quad 0x00cff3000000ffff /* 32/64-bit data segment (user) */
+ .quad 0x00affb000000ffff /* 64-bit code segment (user) */
+
+ .quad 0 /* 6 spare selectors */
+ .quad 0
+ .quad 0
+ .quad 0
+ .quad 0
+ .quad 0
+
+tss_descr:
+ .rept max_cpus
+ .quad 0x000089000000ffff /* 64-bit avail tss */
+ .quad 0 /* tss high addr */
+ .endr
+gdt64_end:
+
+.globl tss
+tss:
+ .rept max_cpus
+ .long 0
+ .quad 0
+ .quad 0, 0
+ .quad 0, 0, 0, 0, 0, 0, 0, 0
+ .long 0, 0, 0
+ .endr
+tss_end:
+
.section .init
.code64
.text
@@ -32,3 +89,34 @@ load_idt:
lidtq idt_descr(%rip)
retq
+
+.globl load_gdt_tss
+load_gdt_tss:
+ /* Set GDT runtime address */
+ lea gdt64(%rip), %rax
+ mov %rax, gdt64_desc+2(%rip)
+
+ /* Load GDT */
+ lgdt gdt64_desc(%rip)
+
+ /* Load TSS */
+ mov %rdi, %rax
+ ltr %ax
+
+ /* Update data segments */
+ mov $0x10, %ax /* 3rd entry in gdt64: 32/64-bit data segment */
+ mov %ax, %ds
+ mov %ax, %es
+ mov %ax, %fs
+ mov %ax, %gs
+ mov %ax, %ss
+
+ /*
+ * Update the code segment by putting it on the stack before the return
+ * address, then doing a far return: this will use the new code segment
+ * along with the address.
+ */
+ popq %rdi
+ pushq $0x08 /* 2nd entry in gdt64: 64-bit code segment */
+ pushq %rdi
+ lretq
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 06/17] x86 UEFI: Load GDT and TSS after UEFI boot up
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 06/17] x86 UEFI: Load GDT and TSS " Zixuan Wang
@ 2021-09-20 15:40 ` Paolo Bonzini
2021-09-21 4:15 ` Zixuan Wang
0 siblings, 1 reply; 47+ messages in thread
From: Paolo Bonzini @ 2021-09-20 15:40 UTC (permalink / raw)
To: Zixuan Wang, kvm, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
On 27/08/21 05:12, Zixuan Wang wrote:
> +.globl gdt64
> +gdt64:
> + .quad 0
> + .quad 0x00af9b000000ffff /* 64-bit code segment */
> + .quad 0x00cf93000000ffff /* 32/64-bit data segment */
> + .quad 0x00af1b000000ffff /* 64-bit code segment, not present */
> + .quad 0x00cf9b000000ffff /* 32-bit code segment */
> + .quad 0x008f9b000000FFFF /* 16-bit code segment */
> + .quad 0x008f93000000FFFF /* 16-bit data segment */
> + .quad 0x00cffb000000ffff /* 32-bit code segment (user) */
> + .quad 0x00cff3000000ffff /* 32/64-bit data segment (user) */
> + .quad 0x00affb000000ffff /* 64-bit code segment (user) */
> +
> + .quad 0 /* 6 spare selectors */
> + .quad 0
> + .quad 0
> + .quad 0
> + .quad 0
> + .quad 0
> +
> +tss_descr:
> + .rept max_cpus
> + .quad 0x000089000000ffff /* 64-bit avail tss */
> + .quad 0 /* tss high addr */
> + .endr
> +gdt64_end:
>
> +.globl tss
> +tss:
> + .rept max_cpus
> + .long 0
> + .quad 0
> + .quad 0, 0
> + .quad 0, 0, 0, 0, 0, 0, 0, 0
> + .long 0, 0, 0
> + .endr
> +tss_end:
> +
Please place the IDT (from the previous patch), GDT and TSS in a common
source file for both UEFI and multiboot. It could in fact be
lib/x86/desc.c even.
Duplicating the descriptors is fine, since they're only referred from
the startup code.
Paolo
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [kvm-unit-tests PATCH v2 06/17] x86 UEFI: Load GDT and TSS after UEFI boot up
2021-09-20 15:40 ` Paolo Bonzini
@ 2021-09-21 4:15 ` Zixuan Wang
0 siblings, 0 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-09-21 4:15 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Zixuan Wang, kvm, drjones, Marc Orr, Hyunwook (Wooky) Baek,
tmroeder, erdemaktas, rientjes, seanjc, brijesh.singh,
Thomas.Lendacky, varad.gautam, jroedel, bp
On Mon, Sep 20, 2021 at 6:26 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 27/08/21 05:12, Zixuan Wang wrote:
> > +.globl gdt64
> > +gdt64:
> > + .quad 0
> > + .quad 0x00af9b000000ffff /* 64-bit code segment */
> > + .quad 0x00cf93000000ffff /* 32/64-bit data segment */
> > + .quad 0x00af1b000000ffff /* 64-bit code segment, not present */
> > + .quad 0x00cf9b000000ffff /* 32-bit code segment */
> > + .quad 0x008f9b000000FFFF /* 16-bit code segment */
> > + .quad 0x008f93000000FFFF /* 16-bit data segment */
> > + .quad 0x00cffb000000ffff /* 32-bit code segment (user) */
> > + .quad 0x00cff3000000ffff /* 32/64-bit data segment (user) */
> > + .quad 0x00affb000000ffff /* 64-bit code segment (user) */
> > +
> > + .quad 0 /* 6 spare selectors */
> > + .quad 0
> > + .quad 0
> > + .quad 0
> > + .quad 0
> > + .quad 0
> > +
> > +tss_descr:
> > + .rept max_cpus
> > + .quad 0x000089000000ffff /* 64-bit avail tss */
> > + .quad 0 /* tss high addr */
> > + .endr
> > +gdt64_end:
> >
> > +.globl tss
> > +tss:
> > + .rept max_cpus
> > + .long 0
> > + .quad 0
> > + .quad 0, 0
> > + .quad 0, 0, 0, 0, 0, 0, 0, 0
> > + .long 0, 0, 0
> > + .endr
> > +tss_end:
> > +
>
> Please place the IDT (from the previous patch), GDT and TSS in a common
> source file for both UEFI and multiboot. It could in fact be
> lib/x86/desc.c even.
>
> Duplicating the descriptors is fine, since they're only referred from
> the startup code.
>
> Paolo
>
I will make this update in the next version.
I think placing descriptors in lib/x86/desc.c is also a good idea. In
this way, compilers can automatically generate position-independent
code (PIC) for data structures written in C, and the following
descriptor address setup code can be removed.
> + /* Set GDT runtime address */
> + lea gdt64(%rip), %rax
> + mov %rax, gdt64_desc+2(%rip)
Best regards,
Zixuan
^ permalink raw reply [flat|nested] 47+ messages in thread
* [kvm-unit-tests PATCH v2 07/17] x86 UEFI: Set up memory allocator
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
` (5 preceding siblings ...)
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 06/17] x86 UEFI: Load GDT and TSS " Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-10-04 13:06 ` Andrew Jones
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 08/17] x86 UEFI: Set up RSDP after UEFI boot up Zixuan Wang
` (10 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
KVM-Unit-Tests library implements a memory allocator which requires
two arguments to set up (See `lib/alloc_phys.c:phys_alloc_init()` for
more details):
1. A base (start) physical address
2. Size of available memory for allocation
To get this memory info, we scan all the memory regions returned by
`LibMemoryMap()`, find out the largest free memory region and use it for
memory allocation.
After retrieving this memory info, we call `ExitBootServices` so that
KVM-Unit-Tests has full control of the machine, and UEFI will not touch
the memory after this point.
Starting from this commit, `x86/hypercall.c` test case can run in UEFI
and generates the same output as in Seabios.
Co-developed-by: Varad Gautam <varad.gautam@suse.com>
Signed-off-by: Varad Gautam <varad.gautam@suse.com>
Signed-off-by: Zixuan Wang <zixuanwang@google.com>
---
lib/efi.c | 28 +++++++++++++---
lib/efi.h | 2 +-
lib/x86/asm/setup.h | 16 +++++++++-
lib/x86/setup.c | 78 ++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 116 insertions(+), 8 deletions(-)
diff --git a/lib/efi.c b/lib/efi.c
index 99307db..b7a69d3 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -31,9 +31,10 @@ efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
efi_memory_desc_t *m = NULL;
efi_status_t status;
unsigned long key = 0, map_size = 0, desc_size = 0;
+ u32 desc_ver;
status = efi_bs_call(get_memory_map, &map_size,
- NULL, &key, &desc_size, NULL);
+ NULL, &key, &desc_size, &desc_ver);
if (status != EFI_BUFFER_TOO_SMALL || map_size == 0)
goto out;
@@ -48,12 +49,13 @@ efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
/* Get the map. */
status = efi_bs_call(get_memory_map, &map_size,
- m, &key, &desc_size, NULL);
+ m, &key, &desc_size, &desc_ver);
if (status != EFI_SUCCESS) {
efi_free_pool(m);
goto out;
}
+ *map->desc_ver = desc_ver;
*map->desc_size = desc_size;
*map->map_size = map_size;
*map->key_ptr = key;
@@ -62,18 +64,34 @@ out:
return status;
}
-efi_status_t efi_exit_boot_services(void *handle, struct efi_boot_memmap *map)
+efi_status_t efi_exit_boot_services(void *handle, unsigned long mapkey)
{
- return efi_bs_call(exit_boot_services, handle, *map->key_ptr);
+ return efi_bs_call(exit_boot_services, handle, mapkey);
}
efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
{
int ret;
+ unsigned long mapkey = 0;
+ efi_status_t status;
+ efi_bootinfo_t efi_bootinfo;
efi_system_table = sys_tab;
- setup_efi();
+ setup_efi_bootinfo(&efi_bootinfo);
+ status = setup_efi_pre_boot(&mapkey, &efi_bootinfo);
+ if (status != EFI_SUCCESS) {
+ printf("Failed to set up before ExitBootServices, exiting.\n");
+ return status;
+ }
+
+ status = efi_exit_boot_services(handle, mapkey);
+ if (status != EFI_SUCCESS) {
+ printf("Failed to exit boot services\n");
+ return status;
+ }
+
+ setup_efi(&efi_bootinfo);
ret = main(__argc, __argv, __environ);
/* Shutdown the guest VM */
diff --git a/lib/efi.h b/lib/efi.h
index 60cdb6f..2d3772c 100644
--- a/lib/efi.h
+++ b/lib/efi.h
@@ -11,7 +11,7 @@
efi_status_t _relocate(long ldbase, Elf64_Dyn *dyn, efi_handle_t handle, efi_system_table_t *sys_tab);
efi_status_t efi_get_memory_map(struct efi_boot_memmap *map);
-efi_status_t efi_exit_boot_services(void *handle, struct efi_boot_memmap *map);
+efi_status_t efi_exit_boot_services(void *handle, unsigned long mapkey);
efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab);
#endif /* _EFI_H_ */
diff --git a/lib/x86/asm/setup.h b/lib/x86/asm/setup.h
index eb1cf73..8ff31ef 100644
--- a/lib/x86/asm/setup.h
+++ b/lib/x86/asm/setup.h
@@ -4,8 +4,22 @@
#ifdef TARGET_EFI
#include "x86/apic.h"
#include "x86/smp.h"
+#include "efi.h"
-void setup_efi(void);
+/*
+ * efi_bootinfo_t: stores EFI-related machine info retrieved by
+ * setup_efi_pre_boot(), and is then used by setup_efi(). setup_efi() cannot
+ * retrieve this info as it is called after ExitBootServices and thus some EFI
+ * resources are not available.
+ */
+typedef struct {
+ phys_addr_t free_mem_start;
+ phys_addr_t free_mem_size;
+} efi_bootinfo_t;
+
+void setup_efi_bootinfo(efi_bootinfo_t *efi_bootinfo);
+void setup_efi(efi_bootinfo_t *efi_bootinfo);
+efi_status_t setup_efi_pre_boot(unsigned long *mapkey, efi_bootinfo_t *efi_bootinfo);
#endif /* TARGET_EFI */
#endif /* _X86_ASM_SETUP_H_ */
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index 0a065fe..a49e0d4 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -131,6 +131,81 @@ extern phys_addr_t ring0stacktop;
extern gdt_entry_t gdt64[];
extern size_t ring0stacksize;
+void setup_efi_bootinfo(efi_bootinfo_t *efi_bootinfo)
+{
+ efi_bootinfo->free_mem_size = 0;
+ efi_bootinfo->free_mem_start = 0;
+}
+
+static efi_status_t setup_pre_boot_memory(unsigned long *mapkey, efi_bootinfo_t *efi_bootinfo)
+{
+ int i;
+ unsigned long free_mem_total_pages;
+ efi_status_t status;
+ struct efi_boot_memmap map;
+ efi_memory_desc_t *buffer, *d;
+ unsigned long map_size, desc_size, buff_size;
+ u32 desc_ver;
+
+ map.map = &buffer;
+ map.map_size = &map_size;
+ map.desc_size = &desc_size;
+ map.desc_ver = &desc_ver;
+ map.buff_size = &buff_size;
+ map.key_ptr = mapkey;
+
+ status = efi_get_memory_map(&map);
+ if (status != EFI_SUCCESS) {
+ return status;
+ }
+
+ /*
+ * The 'buffer' contains multiple descriptors that describe memory
+ * regions maintained by UEFI. This code records the largest free
+ * EFI_CONVENTIONAL_MEMORY region which will be used to set up the
+ * memory allocator, so that the memory allocator can work in the
+ * largest free continuous memory region.
+ */
+ free_mem_total_pages = 0;
+ for (i = 0; i < map_size; i += desc_size) {
+ d = (efi_memory_desc_t *)(&((u8 *)buffer)[i]);
+ if (d->type == EFI_CONVENTIONAL_MEMORY) {
+ if (free_mem_total_pages < d->num_pages) {
+ free_mem_total_pages = d->num_pages;
+ efi_bootinfo->free_mem_size = free_mem_total_pages << EFI_PAGE_SHIFT;
+ efi_bootinfo->free_mem_start = d->phys_addr;
+ }
+ }
+ }
+
+ if (efi_bootinfo->free_mem_size == 0) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ return EFI_SUCCESS;
+}
+
+efi_status_t setup_efi_pre_boot(unsigned long *mapkey, efi_bootinfo_t *efi_bootinfo)
+{
+ efi_status_t status;
+
+ status = setup_pre_boot_memory(mapkey, efi_bootinfo);
+ if (status != EFI_SUCCESS) {
+ printf("setup_pre_boot_memory() failed: ");
+ switch (status) {
+ case EFI_OUT_OF_RESOURCES:
+ printf("No free memory region\n");
+ break;
+ default:
+ printf("Unknown error\n");
+ break;
+ }
+ return status;
+ }
+
+ return EFI_SUCCESS;
+}
+
static void setup_gdt_tss(void)
{
gdt_entry_t *tss_lo, *tss_hi;
@@ -169,7 +244,7 @@ static void setup_gdt_tss(void)
load_gdt_tss(tss_offset);
}
-void setup_efi(void)
+void setup_efi(efi_bootinfo_t *efi_bootinfo)
{
reset_apic();
setup_gdt_tss();
@@ -179,6 +254,7 @@ void setup_efi(void)
enable_apic();
enable_x2apic();
smp_init();
+ phys_alloc_init(efi_bootinfo->free_mem_start, efi_bootinfo->free_mem_size);
}
#endif /* TARGET_EFI */
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 07/17] x86 UEFI: Set up memory allocator
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 07/17] x86 UEFI: Set up memory allocator Zixuan Wang
@ 2021-10-04 13:06 ` Andrew Jones
2021-10-04 21:43 ` Zixuan Wang
0 siblings, 1 reply; 47+ messages in thread
From: Andrew Jones @ 2021-10-04 13:06 UTC (permalink / raw)
To: Zixuan Wang
Cc: kvm, pbonzini, marcorr, baekhw, tmroeder, erdemaktas, rientjes,
seanjc, brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel,
bp
On Fri, Aug 27, 2021 at 03:12:12AM +0000, Zixuan Wang wrote:
> KVM-Unit-Tests library implements a memory allocator which requires
> two arguments to set up (See `lib/alloc_phys.c:phys_alloc_init()` for
> more details):
> 1. A base (start) physical address
> 2. Size of available memory for allocation
>
> To get this memory info, we scan all the memory regions returned by
> `LibMemoryMap()`, find out the largest free memory region and use it for
> memory allocation.
>
> After retrieving this memory info, we call `ExitBootServices` so that
> KVM-Unit-Tests has full control of the machine, and UEFI will not touch
> the memory after this point.
>
> Starting from this commit, `x86/hypercall.c` test case can run in UEFI
> and generates the same output as in Seabios.
>
> Co-developed-by: Varad Gautam <varad.gautam@suse.com>
> Signed-off-by: Varad Gautam <varad.gautam@suse.com>
> Signed-off-by: Zixuan Wang <zixuanwang@google.com>
> ---
> lib/efi.c | 28 +++++++++++++---
> lib/efi.h | 2 +-
> lib/x86/asm/setup.h | 16 +++++++++-
> lib/x86/setup.c | 78 ++++++++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 116 insertions(+), 8 deletions(-)
>
> diff --git a/lib/efi.c b/lib/efi.c
> index 99307db..b7a69d3 100644
> --- a/lib/efi.c
> +++ b/lib/efi.c
> @@ -31,9 +31,10 @@ efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
> efi_memory_desc_t *m = NULL;
> efi_status_t status;
> unsigned long key = 0, map_size = 0, desc_size = 0;
> + u32 desc_ver;
>
> status = efi_bs_call(get_memory_map, &map_size,
> - NULL, &key, &desc_size, NULL);
> + NULL, &key, &desc_size, &desc_ver);
> if (status != EFI_BUFFER_TOO_SMALL || map_size == 0)
> goto out;
>
> @@ -48,12 +49,13 @@ efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
>
> /* Get the map. */
> status = efi_bs_call(get_memory_map, &map_size,
> - m, &key, &desc_size, NULL);
> + m, &key, &desc_size, &desc_ver);
> if (status != EFI_SUCCESS) {
> efi_free_pool(m);
> goto out;
> }
>
> + *map->desc_ver = desc_ver;
> *map->desc_size = desc_size;
> *map->map_size = map_size;
> *map->key_ptr = key;
> @@ -62,18 +64,34 @@ out:
> return status;
> }
>
> -efi_status_t efi_exit_boot_services(void *handle, struct efi_boot_memmap *map)
> +efi_status_t efi_exit_boot_services(void *handle, unsigned long mapkey)
> {
> - return efi_bs_call(exit_boot_services, handle, *map->key_ptr);
> + return efi_bs_call(exit_boot_services, handle, mapkey);
> }
>
> efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> {
> int ret;
> + unsigned long mapkey = 0;
> + efi_status_t status;
> + efi_bootinfo_t efi_bootinfo;
>
> efi_system_table = sys_tab;
>
> - setup_efi();
> + setup_efi_bootinfo(&efi_bootinfo);
> + status = setup_efi_pre_boot(&mapkey, &efi_bootinfo);
> + if (status != EFI_SUCCESS) {
> + printf("Failed to set up before ExitBootServices, exiting.\n");
> + return status;
> + }
> +
> + status = efi_exit_boot_services(handle, mapkey);
> + if (status != EFI_SUCCESS) {
> + printf("Failed to exit boot services\n");
> + return status;
> + }
> +
> + setup_efi(&efi_bootinfo);
> ret = main(__argc, __argv, __environ);
>
> /* Shutdown the guest VM */
> diff --git a/lib/efi.h b/lib/efi.h
> index 60cdb6f..2d3772c 100644
> --- a/lib/efi.h
> +++ b/lib/efi.h
> @@ -11,7 +11,7 @@
>
> efi_status_t _relocate(long ldbase, Elf64_Dyn *dyn, efi_handle_t handle, efi_system_table_t *sys_tab);
> efi_status_t efi_get_memory_map(struct efi_boot_memmap *map);
> -efi_status_t efi_exit_boot_services(void *handle, struct efi_boot_memmap *map);
> +efi_status_t efi_exit_boot_services(void *handle, unsigned long mapkey);
> efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab);
>
> #endif /* _EFI_H_ */
> diff --git a/lib/x86/asm/setup.h b/lib/x86/asm/setup.h
> index eb1cf73..8ff31ef 100644
> --- a/lib/x86/asm/setup.h
> +++ b/lib/x86/asm/setup.h
> @@ -4,8 +4,22 @@
> #ifdef TARGET_EFI
> #include "x86/apic.h"
> #include "x86/smp.h"
> +#include "efi.h"
>
> -void setup_efi(void);
> +/*
> + * efi_bootinfo_t: stores EFI-related machine info retrieved by
> + * setup_efi_pre_boot(), and is then used by setup_efi(). setup_efi() cannot
> + * retrieve this info as it is called after ExitBootServices and thus some EFI
> + * resources are not available.
> + */
> +typedef struct {
> + phys_addr_t free_mem_start;
> + phys_addr_t free_mem_size;
> +} efi_bootinfo_t;
> +
> +void setup_efi_bootinfo(efi_bootinfo_t *efi_bootinfo);
> +void setup_efi(efi_bootinfo_t *efi_bootinfo);
> +efi_status_t setup_efi_pre_boot(unsigned long *mapkey, efi_bootinfo_t *efi_bootinfo);
> #endif /* TARGET_EFI */
>
> #endif /* _X86_ASM_SETUP_H_ */
> diff --git a/lib/x86/setup.c b/lib/x86/setup.c
> index 0a065fe..a49e0d4 100644
> --- a/lib/x86/setup.c
> +++ b/lib/x86/setup.c
> @@ -131,6 +131,81 @@ extern phys_addr_t ring0stacktop;
> extern gdt_entry_t gdt64[];
> extern size_t ring0stacksize;
>
> +void setup_efi_bootinfo(efi_bootinfo_t *efi_bootinfo)
> +{
> + efi_bootinfo->free_mem_size = 0;
> + efi_bootinfo->free_mem_start = 0;
> +}
> +
> +static efi_status_t setup_pre_boot_memory(unsigned long *mapkey, efi_bootinfo_t *efi_bootinfo)
> +{
> + int i;
> + unsigned long free_mem_total_pages;
> + efi_status_t status;
> + struct efi_boot_memmap map;
> + efi_memory_desc_t *buffer, *d;
> + unsigned long map_size, desc_size, buff_size;
> + u32 desc_ver;
> +
> + map.map = &buffer;
> + map.map_size = &map_size;
> + map.desc_size = &desc_size;
> + map.desc_ver = &desc_ver;
> + map.buff_size = &buff_size;
> + map.key_ptr = mapkey;
> +
> + status = efi_get_memory_map(&map);
> + if (status != EFI_SUCCESS) {
> + return status;
> + }
> +
> + /*
> + * The 'buffer' contains multiple descriptors that describe memory
> + * regions maintained by UEFI. This code records the largest free
> + * EFI_CONVENTIONAL_MEMORY region which will be used to set up the
> + * memory allocator, so that the memory allocator can work in the
> + * largest free continuous memory region.
> + */
> + free_mem_total_pages = 0;
> + for (i = 0; i < map_size; i += desc_size) {
> + d = (efi_memory_desc_t *)(&((u8 *)buffer)[i]);
> + if (d->type == EFI_CONVENTIONAL_MEMORY) {
> + if (free_mem_total_pages < d->num_pages) {
> + free_mem_total_pages = d->num_pages;
> + efi_bootinfo->free_mem_size = free_mem_total_pages << EFI_PAGE_SHIFT;
> + efi_bootinfo->free_mem_start = d->phys_addr;
> + }
> + }
> + }
> +
> + if (efi_bootinfo->free_mem_size == 0) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> +efi_status_t setup_efi_pre_boot(unsigned long *mapkey, efi_bootinfo_t *efi_bootinfo)
> +{
> + efi_status_t status;
> +
> + status = setup_pre_boot_memory(mapkey, efi_bootinfo);
> + if (status != EFI_SUCCESS) {
> + printf("setup_pre_boot_memory() failed: ");
> + switch (status) {
> + case EFI_OUT_OF_RESOURCES:
> + printf("No free memory region\n");
> + break;
> + default:
> + printf("Unknown error\n");
> + break;
> + }
> + return status;
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> static void setup_gdt_tss(void)
> {
> gdt_entry_t *tss_lo, *tss_hi;
> @@ -169,7 +244,7 @@ static void setup_gdt_tss(void)
> load_gdt_tss(tss_offset);
> }
>
> -void setup_efi(void)
> +void setup_efi(efi_bootinfo_t *efi_bootinfo)
> {
> reset_apic();
> setup_gdt_tss();
> @@ -179,6 +254,7 @@ void setup_efi(void)
> enable_apic();
> enable_x2apic();
> smp_init();
> + phys_alloc_init(efi_bootinfo->free_mem_start, efi_bootinfo->free_mem_size);
> }
>
> #endif /* TARGET_EFI */
> --
> 2.33.0.259.gc128427fd7-goog
>
How about just getting the memory map (efi_boot_memmap) and then exiting
boot services in arch-neutral code and then have arch-specific code decide
what to do with the memory map?
Thanks,
drew
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 07/17] x86 UEFI: Set up memory allocator
2021-10-04 13:06 ` Andrew Jones
@ 2021-10-04 21:43 ` Zixuan Wang
2021-10-05 6:05 ` Andrew Jones
0 siblings, 1 reply; 47+ messages in thread
From: Zixuan Wang @ 2021-10-04 21:43 UTC (permalink / raw)
To: Andrew Jones
Cc: kvm, Paolo Bonzini, Marc Orr, Hyunwook (Wooky) Baek, Tom Roeder,
erdemaktas, rientjes, seanjc, brijesh.singh, Thomas.Lendacky,
varad.gautam, jroedel, bp
On Mon, Oct 4, 2021 at 6:08 AM Andrew Jones <drjones@redhat.com> wrote:
>
> On Fri, Aug 27, 2021 at 03:12:12AM +0000, Zixuan Wang wrote:
> > KVM-Unit-Tests library implements a memory allocator which requires
> > two arguments to set up (See `lib/alloc_phys.c:phys_alloc_init()` for
> > more details):
> >
> > #endif /* TARGET_EFI */
> > --
> > 2.33.0.259.gc128427fd7-goog
> >
>
> How about just getting the memory map (efi_boot_memmap) and then exiting
> boot services in arch-neutral code and then have arch-specific code decide
> what to do with the memory map?
>
> Thanks,
> drew
>
I see, I will try to refactor the code in the next version:
1. Defines an arch-neutral data structure to store the memory map
2. Calls an arch-neutral function to get the memory map
3. Exits UEFI boot services
4. Calls an arch-specific function to process the memory map
Best regards,
Zixuan
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [kvm-unit-tests PATCH v2 07/17] x86 UEFI: Set up memory allocator
2021-10-04 21:43 ` Zixuan Wang
@ 2021-10-05 6:05 ` Andrew Jones
0 siblings, 0 replies; 47+ messages in thread
From: Andrew Jones @ 2021-10-05 6:05 UTC (permalink / raw)
To: Zixuan Wang
Cc: kvm, Paolo Bonzini, Marc Orr, Hyunwook (Wooky) Baek, Tom Roeder,
erdemaktas, rientjes, seanjc, brijesh.singh, Thomas.Lendacky,
varad.gautam, jroedel, bp
On Mon, Oct 04, 2021 at 02:43:52PM -0700, Zixuan Wang wrote:
> On Mon, Oct 4, 2021 at 6:08 AM Andrew Jones <drjones@redhat.com> wrote:
> >
> > On Fri, Aug 27, 2021 at 03:12:12AM +0000, Zixuan Wang wrote:
> > > KVM-Unit-Tests library implements a memory allocator which requires
> > > two arguments to set up (See `lib/alloc_phys.c:phys_alloc_init()` for
> > > more details):
> > >
> > > #endif /* TARGET_EFI */
> > > --
> > > 2.33.0.259.gc128427fd7-goog
> > >
> >
> > How about just getting the memory map (efi_boot_memmap) and then exiting
> > boot services in arch-neutral code and then have arch-specific code decide
> > what to do with the memory map?
> >
> > Thanks,
> > drew
> >
>
> I see, I will try to refactor the code in the next version:
>
> 1. Defines an arch-neutral data structure to store the memory map
> 2. Calls an arch-neutral function to get the memory map
So, in my first PoC, I was using the AArch64 mem_region structure,
but in hindsight, an arch-neutral structure already exists that
I should have used from the AArch64 code. That arch-neutral
structure is simply the EFI structure (efi_boot_memmap).
> 3. Exits UEFI boot services
> 4. Calls an arch-specific function to process the memory map
Sounds good.
Thanks,
drew
^ permalink raw reply [flat|nested] 47+ messages in thread
* [kvm-unit-tests PATCH v2 08/17] x86 UEFI: Set up RSDP after UEFI boot up
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
` (6 preceding siblings ...)
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 07/17] x86 UEFI: Set up memory allocator Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-10-04 13:21 ` Andrew Jones
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 09/17] x86 UEFI: Set up page tables Zixuan Wang
` (9 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
Root system description pointer (RSDP) is a data structure used in the
ACPI programming interface. In BIOS, RSDP is located within a
predefined memory area, so a program can scan the memory area and find
RSDP. But in UEFI, RSDP may not appear in that memory area, instead, a
program should find it in the EFI system table.
This commit provides RSDP set up code in UEFI:
1. Read RSDP from EFI system table
2. Pass RSDP pointer to find_acpi_table_attr() function
From this commit, the `x86/s3.c` test can run in UEFI and generates
similar output as in Seabios, note that:
1. In its output, memory addresses are different than Seabios's, this
is because EFI application starts from a dynamic runtime address,
not a fixed predefined memory address
2. There is a short delay (~5 secs) after the test case prints "PM1a
event registers" line. This test case sleeps for a few seconds
and then wakes up, so give it a few seconds to run.
Signed-off-by: Zixuan Wang <zixuanwang@google.com>
---
lib/efi.c | 15 +++++++++++++++
lib/efi.h | 1 +
lib/linux/uefi.h | 15 +++++++++++++++
lib/x86/acpi.c | 38 +++++++++++++++++++++++++++++++-------
lib/x86/acpi.h | 11 +++++++++++
lib/x86/asm/setup.h | 2 ++
lib/x86/setup.c | 13 +++++++++++++
7 files changed, 88 insertions(+), 7 deletions(-)
diff --git a/lib/efi.c b/lib/efi.c
index b7a69d3..a0d4476 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -69,6 +69,21 @@ efi_status_t efi_exit_boot_services(void *handle, unsigned long mapkey)
return efi_bs_call(exit_boot_services, handle, mapkey);
}
+efi_status_t efi_get_system_config_table(efi_guid_t table_guid, void **table)
+{
+ size_t i;
+ efi_config_table_t *tables;
+
+ tables = (efi_config_table_t *)efi_system_table->tables;
+ for (i = 0; i < efi_system_table->nr_tables; i++) {
+ if (!memcmp(&table_guid, &tables[i].guid, sizeof(efi_guid_t))) {
+ *table = tables[i].table;
+ return EFI_SUCCESS;
+ }
+ }
+ return EFI_NOT_FOUND;
+}
+
efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
{
int ret;
diff --git a/lib/efi.h b/lib/efi.h
index 2d3772c..dbb8159 100644
--- a/lib/efi.h
+++ b/lib/efi.h
@@ -12,6 +12,7 @@
efi_status_t _relocate(long ldbase, Elf64_Dyn *dyn, efi_handle_t handle, efi_system_table_t *sys_tab);
efi_status_t efi_get_memory_map(struct efi_boot_memmap *map);
efi_status_t efi_exit_boot_services(void *handle, unsigned long mapkey);
+efi_status_t efi_get_system_config_table(efi_guid_t table_guid, void **table);
efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab);
#endif /* _EFI_H_ */
diff --git a/lib/linux/uefi.h b/lib/linux/uefi.h
index 9adc7ab..d1d599f 100644
--- a/lib/linux/uefi.h
+++ b/lib/linux/uefi.h
@@ -58,6 +58,21 @@ typedef guid_t efi_guid_t;
(b) & 0xff, ((b) >> 8) & 0xff, \
(c) & 0xff, ((c) >> 8) & 0xff, d } }
+#define ACPI_TABLE_GUID EFI_GUID(0xeb9d2d30, 0x2d88, 0x11d3, 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
+
+typedef struct {
+ efi_guid_t guid;
+ u32 table;
+} efi_config_table_32_t;
+
+typedef union {
+ struct {
+ efi_guid_t guid;
+ void *table;
+ };
+ efi_config_table_32_t mixed_mode;
+} efi_config_table_t;
+
/*
* Generic EFI table header
*/
diff --git a/lib/x86/acpi.c b/lib/x86/acpi.c
index 4373106..0f75d79 100644
--- a/lib/x86/acpi.c
+++ b/lib/x86/acpi.c
@@ -1,9 +1,37 @@
#include "libcflat.h"
#include "acpi.h"
+#ifdef TARGET_EFI
+struct rsdp_descriptor *efi_rsdp = NULL;
+
+void setup_efi_rsdp(struct rsdp_descriptor *rsdp) {
+ efi_rsdp = rsdp;
+}
+
+static struct rsdp_descriptor *get_rsdp(void) {
+ if (efi_rsdp == NULL) {
+ printf("Can't find RSDP from UEFI, maybe setup_efi_rsdp() was not called\n");
+ }
+ return efi_rsdp;
+}
+#else
+static struct rsdp_descriptor *get_rsdp(void) {
+ struct rsdp_descriptor *rsdp;
+ unsigned long addr;
+ for(addr = 0xf0000; addr < 0x100000; addr += 16) {
+ rsdp = (void*)addr;
+ if (rsdp->signature == RSDP_SIGNATURE_8BYTE)
+ break;
+ }
+ if (addr == 0x100000) {
+ return NULL;
+ }
+ return rsdp;
+}
+#endif /* TARGET_EFI */
+
void* find_acpi_table_addr(u32 sig)
{
- unsigned long addr;
struct rsdp_descriptor *rsdp;
struct rsdt_descriptor_rev1 *rsdt;
void *end;
@@ -19,12 +47,8 @@ void* find_acpi_table_addr(u32 sig)
return (void*)(ulong)fadt->firmware_ctrl;
}
- for(addr = 0xf0000; addr < 0x100000; addr += 16) {
- rsdp = (void*)addr;
- if (rsdp->signature == 0x2052545020445352LL)
- break;
- }
- if (addr == 0x100000) {
+ rsdp = get_rsdp();
+ if (rsdp == NULL) {
printf("Can't find RSDP\n");
return 0;
}
diff --git a/lib/x86/acpi.h b/lib/x86/acpi.h
index 1b80374..db8ee56 100644
--- a/lib/x86/acpi.h
+++ b/lib/x86/acpi.h
@@ -11,6 +11,13 @@
#define FACP_SIGNATURE ACPI_SIGNATURE('F','A','C','P')
#define FACS_SIGNATURE ACPI_SIGNATURE('F','A','C','S')
+
+#define ACPI_SIGNATURE_8BYTE(c1, c2, c3, c4, c5, c6, c7, c8) \
+ ((uint64_t)(ACPI_SIGNATURE(c1, c2, c3, c4))) | \
+ ((uint64_t)(ACPI_SIGNATURE(c5, c6, c7, c8)) << 32)
+
+#define RSDP_SIGNATURE_8BYTE (ACPI_SIGNATURE_8BYTE('R', 'S', 'D', ' ', 'P', 'T', 'R', ' '))
+
struct rsdp_descriptor { /* Root System Descriptor Pointer */
u64 signature; /* ACPI signature, contains "RSD PTR " */
u8 checksum; /* To make sum of struct == 0 */
@@ -101,4 +108,8 @@ struct facs_descriptor_rev1
void* find_acpi_table_addr(u32 sig);
+#ifdef TARGET_EFI
+void setup_efi_rsdp(struct rsdp_descriptor *rsdp);
+#endif /* TARGET_EFI */
+
#endif
diff --git a/lib/x86/asm/setup.h b/lib/x86/asm/setup.h
index 8ff31ef..40fd963 100644
--- a/lib/x86/asm/setup.h
+++ b/lib/x86/asm/setup.h
@@ -2,6 +2,7 @@
#define _X86_ASM_SETUP_H_
#ifdef TARGET_EFI
+#include "x86/acpi.h"
#include "x86/apic.h"
#include "x86/smp.h"
#include "efi.h"
@@ -15,6 +16,7 @@
typedef struct {
phys_addr_t free_mem_start;
phys_addr_t free_mem_size;
+ struct rsdp_descriptor *rsdp;
} efi_bootinfo_t;
void setup_efi_bootinfo(efi_bootinfo_t *efi_bootinfo);
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index a49e0d4..1ddfb8c 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -135,6 +135,7 @@ void setup_efi_bootinfo(efi_bootinfo_t *efi_bootinfo)
{
efi_bootinfo->free_mem_size = 0;
efi_bootinfo->free_mem_start = 0;
+ efi_bootinfo->rsdp = NULL;
}
static efi_status_t setup_pre_boot_memory(unsigned long *mapkey, efi_bootinfo_t *efi_bootinfo)
@@ -185,6 +186,11 @@ static efi_status_t setup_pre_boot_memory(unsigned long *mapkey, efi_bootinfo_t
return EFI_SUCCESS;
}
+static efi_status_t setup_pre_boot_rsdp(efi_bootinfo_t *efi_bootinfo)
+{
+ return efi_get_system_config_table(ACPI_TABLE_GUID, (void **)&efi_bootinfo->rsdp);
+}
+
efi_status_t setup_efi_pre_boot(unsigned long *mapkey, efi_bootinfo_t *efi_bootinfo)
{
efi_status_t status;
@@ -203,6 +209,12 @@ efi_status_t setup_efi_pre_boot(unsigned long *mapkey, efi_bootinfo_t *efi_booti
return status;
}
+ status = setup_pre_boot_rsdp(efi_bootinfo);
+ if (status != EFI_SUCCESS) {
+ printf("Cannot find RSDP in EFI system table\n");
+ return status;
+ }
+
return EFI_SUCCESS;
}
@@ -255,6 +267,7 @@ void setup_efi(efi_bootinfo_t *efi_bootinfo)
enable_x2apic();
smp_init();
phys_alloc_init(efi_bootinfo->free_mem_start, efi_bootinfo->free_mem_size);
+ setup_efi_rsdp(efi_bootinfo->rsdp);
}
#endif /* TARGET_EFI */
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 08/17] x86 UEFI: Set up RSDP after UEFI boot up
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 08/17] x86 UEFI: Set up RSDP after UEFI boot up Zixuan Wang
@ 2021-10-04 13:21 ` Andrew Jones
2021-10-04 21:58 ` Zixuan Wang
0 siblings, 1 reply; 47+ messages in thread
From: Andrew Jones @ 2021-10-04 13:21 UTC (permalink / raw)
To: Zixuan Wang
Cc: kvm, pbonzini, marcorr, baekhw, tmroeder, erdemaktas, rientjes,
seanjc, brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel,
bp
On Fri, Aug 27, 2021 at 03:12:13AM +0000, Zixuan Wang wrote:
> Root system description pointer (RSDP) is a data structure used in the
> ACPI programming interface. In BIOS, RSDP is located within a
> predefined memory area, so a program can scan the memory area and find
> RSDP. But in UEFI, RSDP may not appear in that memory area, instead, a
> program should find it in the EFI system table.
>
> This commit provides RSDP set up code in UEFI:
> 1. Read RSDP from EFI system table
> 2. Pass RSDP pointer to find_acpi_table_attr() function
>
> From this commit, the `x86/s3.c` test can run in UEFI and generates
> similar output as in Seabios, note that:
> 1. In its output, memory addresses are different than Seabios's, this
> is because EFI application starts from a dynamic runtime address,
> not a fixed predefined memory address
> 2. There is a short delay (~5 secs) after the test case prints "PM1a
> event registers" line. This test case sleeps for a few seconds
> and then wakes up, so give it a few seconds to run.
>
> Signed-off-by: Zixuan Wang <zixuanwang@google.com>
> ---
> lib/efi.c | 15 +++++++++++++++
> lib/efi.h | 1 +
> lib/linux/uefi.h | 15 +++++++++++++++
> lib/x86/acpi.c | 38 +++++++++++++++++++++++++++++++-------
> lib/x86/acpi.h | 11 +++++++++++
> lib/x86/asm/setup.h | 2 ++
> lib/x86/setup.c | 13 +++++++++++++
> 7 files changed, 88 insertions(+), 7 deletions(-)
>
> diff --git a/lib/efi.c b/lib/efi.c
> index b7a69d3..a0d4476 100644
> --- a/lib/efi.c
> +++ b/lib/efi.c
> @@ -69,6 +69,21 @@ efi_status_t efi_exit_boot_services(void *handle, unsigned long mapkey)
> return efi_bs_call(exit_boot_services, handle, mapkey);
> }
>
> +efi_status_t efi_get_system_config_table(efi_guid_t table_guid, void **table)
> +{
> + size_t i;
> + efi_config_table_t *tables;
> +
> + tables = (efi_config_table_t *)efi_system_table->tables;
> + for (i = 0; i < efi_system_table->nr_tables; i++) {
> + if (!memcmp(&table_guid, &tables[i].guid, sizeof(efi_guid_t))) {
> + *table = tables[i].table;
> + return EFI_SUCCESS;
> + }
> + }
> + return EFI_NOT_FOUND;
> +}
> +
> efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> {
> int ret;
> diff --git a/lib/efi.h b/lib/efi.h
> index 2d3772c..dbb8159 100644
> --- a/lib/efi.h
> +++ b/lib/efi.h
> @@ -12,6 +12,7 @@
> efi_status_t _relocate(long ldbase, Elf64_Dyn *dyn, efi_handle_t handle, efi_system_table_t *sys_tab);
> efi_status_t efi_get_memory_map(struct efi_boot_memmap *map);
> efi_status_t efi_exit_boot_services(void *handle, unsigned long mapkey);
> +efi_status_t efi_get_system_config_table(efi_guid_t table_guid, void **table);
> efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab);
>
> #endif /* _EFI_H_ */
> diff --git a/lib/linux/uefi.h b/lib/linux/uefi.h
> index 9adc7ab..d1d599f 100644
> --- a/lib/linux/uefi.h
> +++ b/lib/linux/uefi.h
> @@ -58,6 +58,21 @@ typedef guid_t efi_guid_t;
> (b) & 0xff, ((b) >> 8) & 0xff, \
> (c) & 0xff, ((c) >> 8) & 0xff, d } }
>
> +#define ACPI_TABLE_GUID EFI_GUID(0xeb9d2d30, 0x2d88, 0x11d3, 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
> +
> +typedef struct {
> + efi_guid_t guid;
> + u32 table;
> +} efi_config_table_32_t;
> +
> +typedef union {
> + struct {
> + efi_guid_t guid;
> + void *table;
> + };
> + efi_config_table_32_t mixed_mode;
Can't we drop all the mixed_mode stuff? Or do we really want to support
32-bit UEFI kvm-unit-tests?
> +} efi_config_table_t;
> +
> /*
> * Generic EFI table header
> */
> diff --git a/lib/x86/acpi.c b/lib/x86/acpi.c
> index 4373106..0f75d79 100644
> --- a/lib/x86/acpi.c
> +++ b/lib/x86/acpi.c
> @@ -1,9 +1,37 @@
> #include "libcflat.h"
> #include "acpi.h"
>
> +#ifdef TARGET_EFI
> +struct rsdp_descriptor *efi_rsdp = NULL;
> +
> +void setup_efi_rsdp(struct rsdp_descriptor *rsdp) {
> + efi_rsdp = rsdp;
> +}
{ on its own line please
> +
> +static struct rsdp_descriptor *get_rsdp(void) {
> + if (efi_rsdp == NULL) {
> + printf("Can't find RSDP from UEFI, maybe setup_efi_rsdp() was not called\n");
> + }
> + return efi_rsdp;
> +}
> +#else
> +static struct rsdp_descriptor *get_rsdp(void) {
> + struct rsdp_descriptor *rsdp;
> + unsigned long addr;
> + for(addr = 0xf0000; addr < 0x100000; addr += 16) {
> + rsdp = (void*)addr;
> + if (rsdp->signature == RSDP_SIGNATURE_8BYTE)
> + break;
> + }
When moving code please take the opportunity to clean up its style.
> + if (addr == 0x100000) {
> + return NULL;
> + }
> + return rsdp;
> +}
> +#endif /* TARGET_EFI */
> +
> void* find_acpi_table_addr(u32 sig)
> {
> - unsigned long addr;
> struct rsdp_descriptor *rsdp;
> struct rsdt_descriptor_rev1 *rsdt;
> void *end;
> @@ -19,12 +47,8 @@ void* find_acpi_table_addr(u32 sig)
> return (void*)(ulong)fadt->firmware_ctrl;
> }
>
> - for(addr = 0xf0000; addr < 0x100000; addr += 16) {
> - rsdp = (void*)addr;
> - if (rsdp->signature == 0x2052545020445352LL)
> - break;
> - }
> - if (addr == 0x100000) {
> + rsdp = get_rsdp();
> + if (rsdp == NULL) {
> printf("Can't find RSDP\n");
> return 0;
> }
> diff --git a/lib/x86/acpi.h b/lib/x86/acpi.h
> index 1b80374..db8ee56 100644
> --- a/lib/x86/acpi.h
> +++ b/lib/x86/acpi.h
> @@ -11,6 +11,13 @@
> #define FACP_SIGNATURE ACPI_SIGNATURE('F','A','C','P')
> #define FACS_SIGNATURE ACPI_SIGNATURE('F','A','C','S')
>
> +
> +#define ACPI_SIGNATURE_8BYTE(c1, c2, c3, c4, c5, c6, c7, c8) \
> + ((uint64_t)(ACPI_SIGNATURE(c1, c2, c3, c4))) | \
> + ((uint64_t)(ACPI_SIGNATURE(c5, c6, c7, c8)) << 32)
> +
> +#define RSDP_SIGNATURE_8BYTE (ACPI_SIGNATURE_8BYTE('R', 'S', 'D', ' ', 'P', 'T', 'R', ' '))
> +
> struct rsdp_descriptor { /* Root System Descriptor Pointer */
> u64 signature; /* ACPI signature, contains "RSD PTR " */
> u8 checksum; /* To make sum of struct == 0 */
> @@ -101,4 +108,8 @@ struct facs_descriptor_rev1
>
> void* find_acpi_table_addr(u32 sig);
>
> +#ifdef TARGET_EFI
> +void setup_efi_rsdp(struct rsdp_descriptor *rsdp);
> +#endif /* TARGET_EFI */
Unnecessary ifdef.
> +
> #endif
> diff --git a/lib/x86/asm/setup.h b/lib/x86/asm/setup.h
> index 8ff31ef..40fd963 100644
> --- a/lib/x86/asm/setup.h
> +++ b/lib/x86/asm/setup.h
> @@ -2,6 +2,7 @@
> #define _X86_ASM_SETUP_H_
>
> #ifdef TARGET_EFI
> +#include "x86/acpi.h"
> #include "x86/apic.h"
> #include "x86/smp.h"
> #include "efi.h"
> @@ -15,6 +16,7 @@
> typedef struct {
> phys_addr_t free_mem_start;
> phys_addr_t free_mem_size;
> + struct rsdp_descriptor *rsdp;
> } efi_bootinfo_t;
>
> void setup_efi_bootinfo(efi_bootinfo_t *efi_bootinfo);
> diff --git a/lib/x86/setup.c b/lib/x86/setup.c
> index a49e0d4..1ddfb8c 100644
> --- a/lib/x86/setup.c
> +++ b/lib/x86/setup.c
> @@ -135,6 +135,7 @@ void setup_efi_bootinfo(efi_bootinfo_t *efi_bootinfo)
> {
> efi_bootinfo->free_mem_size = 0;
> efi_bootinfo->free_mem_start = 0;
> + efi_bootinfo->rsdp = NULL;
> }
>
> static efi_status_t setup_pre_boot_memory(unsigned long *mapkey, efi_bootinfo_t *efi_bootinfo)
> @@ -185,6 +186,11 @@ static efi_status_t setup_pre_boot_memory(unsigned long *mapkey, efi_bootinfo_t
> return EFI_SUCCESS;
> }
>
> +static efi_status_t setup_pre_boot_rsdp(efi_bootinfo_t *efi_bootinfo)
> +{
> + return efi_get_system_config_table(ACPI_TABLE_GUID, (void **)&efi_bootinfo->rsdp);
> +}
> +
> efi_status_t setup_efi_pre_boot(unsigned long *mapkey, efi_bootinfo_t *efi_bootinfo)
> {
> efi_status_t status;
> @@ -203,6 +209,12 @@ efi_status_t setup_efi_pre_boot(unsigned long *mapkey, efi_bootinfo_t *efi_booti
> return status;
> }
>
> + status = setup_pre_boot_rsdp(efi_bootinfo);
> + if (status != EFI_SUCCESS) {
> + printf("Cannot find RSDP in EFI system table\n");
> + return status;
> + }
> +
> return EFI_SUCCESS;
> }
>
> @@ -255,6 +267,7 @@ void setup_efi(efi_bootinfo_t *efi_bootinfo)
> enable_x2apic();
> smp_init();
> phys_alloc_init(efi_bootinfo->free_mem_start, efi_bootinfo->free_mem_size);
> + setup_efi_rsdp(efi_bootinfo->rsdp);
What memory region is this table in? We should make sure it's reserved or
copy the table out to somewhere that is reserved.
> }
>
> #endif /* TARGET_EFI */
> --
> 2.33.0.259.gc128427fd7-goog
>
I'd much prefer we avoid too much of this split setup where we have a bit
of setup in a common efi lib and then an x86 specific part that populates
an x86 specific info structure before exiting boot services and then more
x86 specific setup that uses that later...
Can't we do almost everything in lib/efi.c and only call out once into an
arch_efi_setup function after exiting boot services?
Thanks,
drew
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 08/17] x86 UEFI: Set up RSDP after UEFI boot up
2021-10-04 13:21 ` Andrew Jones
@ 2021-10-04 21:58 ` Zixuan Wang
0 siblings, 0 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-10-04 21:58 UTC (permalink / raw)
To: Andrew Jones
Cc: kvm, Paolo Bonzini, Marc Orr, Hyunwook (Wooky) Baek, Tom Roeder,
erdemaktas, rientjes, seanjc, brijesh.singh, Thomas.Lendacky,
varad.gautam, jroedel, bp
On Mon, Oct 4, 2021 at 6:23 AM Andrew Jones <drjones@redhat.com> wrote:
>
> On Fri, Aug 27, 2021 at 03:12:13AM +0000, Zixuan Wang wrote:
> > Root system description pointer (RSDP) is a data structure used in the
> > ACPI programming interface. In BIOS, RSDP is located within a
> > predefined memory area, so a program can scan the memory area and find
> > RSDP. But in UEFI, RSDP may not appear in that memory area, instead, a
> > program should find it in the EFI system table.
> >
> > +typedef union {
> > + struct {
> > + efi_guid_t guid;
> > + void *table;
> > + };
> > + efi_config_table_32_t mixed_mode;
>
> Can't we drop all the mixed_mode stuff? Or do we really want to support
> 32-bit UEFI kvm-unit-tests?
We are currently not considering the 32-bit UEFI support. I will drop
the mixed_mode code in the next version.
> > +void setup_efi_rsdp(struct rsdp_descriptor *rsdp) {
> > + efi_rsdp = rsdp;
> > +}
>
> { on its own line please
Got it! I will fix it in the next version.
> > +
> > +static struct rsdp_descriptor *get_rsdp(void) {
> > + if (efi_rsdp == NULL) {
> > + printf("Can't find RSDP from UEFI, maybe setup_efi_rsdp() was not called\n");
> > + }
> > + return efi_rsdp;
> > +}
> > +#else
> > +static struct rsdp_descriptor *get_rsdp(void) {
> > + struct rsdp_descriptor *rsdp;
> > + unsigned long addr;
> > + for(addr = 0xf0000; addr < 0x100000; addr += 16) {
> > + rsdp = (void*)addr;
> > + if (rsdp->signature == RSDP_SIGNATURE_8BYTE)
> > + break;
> > + }
>
> When moving code please take the opportunity to clean up its style.
Got it! I will fix this in the next version.
> > +#ifdef TARGET_EFI
> > +void setup_efi_rsdp(struct rsdp_descriptor *rsdp);
> > +#endif /* TARGET_EFI */
>
> Unnecessary ifdef.
I previously thought it's safer to use ifdef here, as the function is
only defined in the UEFI kvm-unit-tests.
I will drop it in the next version. Dropping it does not affect the
compilation of Multiboot kvm-unit-tests.
> > @@ -255,6 +267,7 @@ void setup_efi(efi_bootinfo_t *efi_bootinfo)
> > enable_x2apic();
> > smp_init();
> > phys_alloc_init(efi_bootinfo->free_mem_start, efi_bootinfo->free_mem_size);
> > + setup_efi_rsdp(efi_bootinfo->rsdp);
>
> What memory region is this table in? We should make sure it's reserved or
> copy the table out to somewhere that is reserved.
I will double-check it in the next version. I will add a comment to
this line if it's already in reserved memory. Otherwise, I will copy
it to a reserved location.
> > }
> >
> > #endif /* TARGET_EFI */
> > --
> > 2.33.0.259.gc128427fd7-goog
> >
>
> I'd much prefer we avoid too much of this split setup where we have a bit
> of setup in a common efi lib and then an x86 specific part that populates
> an x86 specific info structure before exiting boot services and then more
> x86 specific setup that uses that later...
>
> Can't we do almost everything in lib/efi.c and only call out once into an
> arch_efi_setup function after exiting boot services?
>
> Thanks,
> drew
>
Indeed, I will refactor this part in the next version. I will move the
arch-neutral functions to lib/efi.c and pass an EFI system info data
structure to the arch-specific function for additional setup.
Best regards,
Zixuan
^ permalink raw reply [flat|nested] 47+ messages in thread
* [kvm-unit-tests PATCH v2 09/17] x86 UEFI: Set up page tables
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
` (7 preceding siblings ...)
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 08/17] x86 UEFI: Set up RSDP after UEFI boot up Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-09-20 15:43 ` Paolo Bonzini
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 10/17] x86 UEFI: Convert x86 test cases to PIC Zixuan Wang
` (8 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
UEFI sets up page tables before executing EFI application binaries.
These page tables do not allow user space code to access kernel space
memory. But `x86/syscall.c` test case places a user space function
`syscall_tf_user32` inside kernel space memory. When using UEFI page
tables, fetching this kernel memory from user space triggers a #PF
fault, which is not expected by this test case.
KVM-Unit-Tests defines page tables that allow such behavior. So the
solution to this problem is to load KVM-Unit-Tests' page tables:
1. Copy the page table definition from `x86/cstart64.S`
2. Update page table entries with runtime memory addresses
3. Update CR3 register with the new page table root address
Since this commit, `x86/syscall.c` can run in UEFI and generate same
output as in Seabios, using the following command:
./x86/efi/run ./x86/syscall.efi --cpu Opteron_G1,vendor=AuthenticAMD
Signed-off-by: Zixuan Wang <zixuanwang@google.com>
---
lib/x86/asm/page.h | 1 +
lib/x86/asm/setup.h | 3 +++
lib/x86/setup.c | 57 ++++++++++++++++++++++++++++++++++++++++++++
x86/efi/efistart64.S | 21 ++++++++++++++++
4 files changed, 82 insertions(+)
diff --git a/lib/x86/asm/page.h b/lib/x86/asm/page.h
index fc14160..f6f740b 100644
--- a/lib/x86/asm/page.h
+++ b/lib/x86/asm/page.h
@@ -31,6 +31,7 @@ typedef unsigned long pgd_t;
#define PT_ACCESSED_MASK (1ull << 5)
#define PT_DIRTY_MASK (1ull << 6)
#define PT_PAGE_SIZE_MASK (1ull << 7)
+#define PT_GLOBAL_MASK (1ull << 8)
#define PT64_NX_MASK (1ull << 63)
#define PT_ADDR_MASK GENMASK_ULL(51, 12)
diff --git a/lib/x86/asm/setup.h b/lib/x86/asm/setup.h
index 40fd963..16bad0f 100644
--- a/lib/x86/asm/setup.h
+++ b/lib/x86/asm/setup.h
@@ -4,7 +4,9 @@
#ifdef TARGET_EFI
#include "x86/acpi.h"
#include "x86/apic.h"
+#include "x86/processor.h"
#include "x86/smp.h"
+#include "asm/page.h"
#include "efi.h"
/*
@@ -22,6 +24,7 @@ typedef struct {
void setup_efi_bootinfo(efi_bootinfo_t *efi_bootinfo);
void setup_efi(efi_bootinfo_t *efi_bootinfo);
efi_status_t setup_efi_pre_boot(unsigned long *mapkey, efi_bootinfo_t *efi_bootinfo);
+void setup_5level_page_table(void);
#endif /* TARGET_EFI */
#endif /* _X86_ASM_SETUP_H_ */
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index 1ddfb8c..03598fe 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -218,6 +218,62 @@ efi_status_t setup_efi_pre_boot(unsigned long *mapkey, efi_bootinfo_t *efi_booti
return EFI_SUCCESS;
}
+/* Defined in cstart64.S or efistart64.S */
+extern phys_addr_t ptl5;
+extern phys_addr_t ptl4;
+extern phys_addr_t ptl3;
+extern phys_addr_t ptl2;
+
+static void setup_page_table(void)
+{
+ pgd_t *curr_pt;
+ phys_addr_t flags;
+ int i;
+
+ /* Set default flags */
+ flags = PT_PRESENT_MASK | PT_WRITABLE_MASK | PT_USER_MASK;
+
+ /* Level 5 */
+ curr_pt = (pgd_t *)&ptl5;
+ curr_pt[0] = ((phys_addr_t)&ptl4) | flags;
+ /* Level 4 */
+ curr_pt = (pgd_t *)&ptl4;
+ curr_pt[0] = ((phys_addr_t)&ptl3) | flags;
+ /* Level 3 */
+ curr_pt = (pgd_t *)&ptl3;
+ for (i = 0; i < 4; i++) {
+ curr_pt[i] = (((phys_addr_t)&ptl2) + i * PAGE_SIZE) | flags;
+ }
+ /* Level 2 */
+ curr_pt = (pgd_t *)&ptl2;
+ flags |= PT_ACCESSED_MASK | PT_DIRTY_MASK | PT_PAGE_SIZE_MASK | PT_GLOBAL_MASK;
+ for (i = 0; i < 4 * 512; i++) {
+ curr_pt[i] = ((phys_addr_t)(i << 21)) | flags;
+ }
+
+ /* Load 4-level page table */
+ write_cr3((ulong)&ptl4);
+}
+
+void setup_5level_page_table(void)
+{
+ /* Check if 5-level page table is already enabled */
+ if (read_cr4() & X86_CR4_LA57) {
+ return;
+ }
+
+ /* Disable CR4.PCIDE */
+ write_cr4(read_cr4() & ~(X86_CR4_PCIDE));
+ /* Disable CR0.PG */
+ write_cr0(read_cr0() & ~(X86_CR0_PG));
+
+ /* Load new page table */
+ write_cr3((ulong)&ptl5);
+
+ /* Enable CR4.LA57 */
+ write_cr4(read_cr4() | X86_CR4_LA57);
+}
+
static void setup_gdt_tss(void)
{
gdt_entry_t *tss_lo, *tss_hi;
@@ -268,6 +324,7 @@ void setup_efi(efi_bootinfo_t *efi_bootinfo)
smp_init();
phys_alloc_init(efi_bootinfo->free_mem_start, efi_bootinfo->free_mem_size);
setup_efi_rsdp(efi_bootinfo->rsdp);
+ setup_page_table();
}
#endif /* TARGET_EFI */
diff --git a/x86/efi/efistart64.S b/x86/efi/efistart64.S
index a14bd46..86c3760 100644
--- a/x86/efi/efistart64.S
+++ b/x86/efi/efistart64.S
@@ -22,6 +22,27 @@ ring0stacktop:
.data
+.align PAGE_SIZE
+.globl ptl2
+ptl2:
+ . = . + 4 * PAGE_SIZE
+.align PAGE_SIZE
+
+.globl ptl3
+ptl3:
+ . = . + PAGE_SIZE
+.align PAGE_SIZE
+
+.globl ptl4
+ptl4:
+ . = . + PAGE_SIZE
+.align PAGE_SIZE
+
+.globl ptl5
+ptl5:
+ . = . + PAGE_SIZE
+.align PAGE_SIZE
+
boot_idt:
.rept 256
.quad 0
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 09/17] x86 UEFI: Set up page tables
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 09/17] x86 UEFI: Set up page tables Zixuan Wang
@ 2021-09-20 15:43 ` Paolo Bonzini
2021-09-21 4:31 ` Zixuan Wang
0 siblings, 1 reply; 47+ messages in thread
From: Paolo Bonzini @ 2021-09-20 15:43 UTC (permalink / raw)
To: Zixuan Wang, kvm, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
On 27/08/21 05:12, Zixuan Wang wrote:
> +static void setup_page_table(void)
It would also be nice if cstart64.S reused setup_page_table, but unlike
GDT/IDT/TSS I guess it's not super-necessary.
Paolo
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [kvm-unit-tests PATCH v2 09/17] x86 UEFI: Set up page tables
2021-09-20 15:43 ` Paolo Bonzini
@ 2021-09-21 4:31 ` Zixuan Wang
0 siblings, 0 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-09-21 4:31 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Zixuan Wang, kvm, drjones, Marc Orr, Hyunwook (Wooky) Baek,
tmroeder, erdemaktas, rientjes, seanjc, brijesh.singh,
Thomas.Lendacky, varad.gautam, jroedel, bp
On Mon, Sep 20, 2021 at 6:26 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 27/08/21 05:12, Zixuan Wang wrote:
> > +static void setup_page_table(void)
>
> It would also be nice if cstart64.S reused setup_page_table, but unlike
> GDT/IDT/TSS I guess it's not super-necessary.
>
> Paolo
>
I can update this in the next version. I can also move the page table
definitions to a C file, maybe lib/x86/desc.c or lib/x86/vm.c? I'm not
sure which file is better.
Best regards,
Zixuan
^ permalink raw reply [flat|nested] 47+ messages in thread
* [kvm-unit-tests PATCH v2 10/17] x86 UEFI: Convert x86 test cases to PIC
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
` (8 preceding siblings ...)
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 09/17] x86 UEFI: Set up page tables Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 11/17] x86 AMD SEV: Initial support Zixuan Wang
` (7 subsequent siblings)
17 siblings, 0 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
UEFI loads EFI applications to dynamic runtime addresses, so it requires
all applications to be compiled as PIC (position independent code). PIC
does not allow the usage of compile time absolute address.
This commit converts multiple x86 test cases to PIC so they can compile
and run in UEFI:
- x86/cet.efi
- x86/emulator.c: x86/emulator.c depends on lib/x86/usermode.c. But
usermode.c contains non-PIC inline assembly code. This commit converts
lib/x86/usermode.c and x86/emulator.c to PIC, so x86/emulator.c can
compile and run in UEFI.
- x86/vmware_backdoors.c: it depends on lib/x86/usermode.c and now works
without modifications
- x86/eventinj.c
- x86/smap.c
- x86/access.c
- x86/umip.c
Signed-off-by: Zixuan Wang <zixuanwang@google.com>
---
lib/x86/usermode.c | 3 ++-
x86/Makefile.common | 10 +++++-----
x86/Makefile.x86_64 | 7 ++++---
x86/access.c | 9 +++++----
x86/cet.c | 8 +++++---
x86/emulator.c | 5 +++--
x86/eventinj.c | 6 ++++--
x86/smap.c | 8 ++++----
x86/umip.c | 10 +++++++---
9 files changed, 39 insertions(+), 27 deletions(-)
diff --git a/lib/x86/usermode.c b/lib/x86/usermode.c
index f032523..c550545 100644
--- a/lib/x86/usermode.c
+++ b/lib/x86/usermode.c
@@ -58,7 +58,8 @@ uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
"pushq %[user_stack_top]\n\t"
"pushfq\n\t"
"pushq %[user_cs]\n\t"
- "pushq $user_mode\n\t"
+ "lea user_mode(%%rip), %%rdx\n\t"
+ "pushq %%rdx\n\t"
"iretq\n"
"user_mode:\n\t"
diff --git a/x86/Makefile.common b/x86/Makefile.common
index 4859bf3..959379c 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -81,16 +81,16 @@ tests-common = $(TEST_DIR)/vmexit.$(exe) $(TEST_DIR)/tsc.$(exe) \
$(TEST_DIR)/init.$(exe) \
$(TEST_DIR)/hyperv_synic.$(exe) $(TEST_DIR)/hyperv_stimer.$(exe) \
$(TEST_DIR)/hyperv_connections.$(exe) \
- $(TEST_DIR)/tsx-ctrl.$(exe)
+ $(TEST_DIR)/tsx-ctrl.$(exe) \
+ $(TEST_DIR)/eventinj.$(exe) \
+ $(TEST_DIR)/umip.$(exe)
# The following test cases are disabled when building EFI tests because they
# use absolute addresses in their inline assembly code, which cannot compile
# with the '-fPIC' flag
ifneq ($(TARGET_EFI),y)
-tests-common += $(TEST_DIR)/eventinj.$(exe) \
- $(TEST_DIR)/smap.$(exe) \
- $(TEST_DIR)/realmode.$(exe) \
- $(TEST_DIR)/umip.$(exe)
+tests-common += $(TEST_DIR)/smap.$(exe) \
+ $(TEST_DIR)/realmode.$(exe)
endif
test_cases: $(tests-common) $(tests)
diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
index aa23b22..7e8a57a 100644
--- a/x86/Makefile.x86_64
+++ b/x86/Makefile.x86_64
@@ -30,20 +30,21 @@ tests += $(TEST_DIR)/intel-iommu.$(exe)
tests += $(TEST_DIR)/rdpru.$(exe)
tests += $(TEST_DIR)/pks.$(exe)
tests += $(TEST_DIR)/pmu_lbr.$(exe)
+tests += $(TEST_DIR)/emulator.$(exe)
+tests += $(TEST_DIR)/vmware_backdoors.$(exe)
# The following test cases are disabled when building EFI tests because they
# use absolute addresses in their inline assembly code, which cannot compile
# with the '-fPIC' flag
ifneq ($(TARGET_EFI),y)
tests += $(TEST_DIR)/access.$(exe)
-tests += $(TEST_DIR)/emulator.$(exe)
tests += $(TEST_DIR)/svm.$(exe)
tests += $(TEST_DIR)/vmx.$(exe)
-tests += $(TEST_DIR)/vmware_backdoors.$(exe)
+endif
+
ifneq ($(fcf_protection_full),)
tests += $(TEST_DIR)/cet.$(exe)
endif
-endif
include $(SRCDIR)/$(TEST_DIR)/Makefile.common
diff --git a/x86/access.c b/x86/access.c
index 4725bbd..8d620a7 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -700,7 +700,7 @@ static int ac_test_do_access(ac_test_t *at)
if (F(AC_ACCESS_TWICE)) {
asm volatile (
- "mov $fixed2, %%rsi \n\t"
+ "lea fixed2(%%rip), %%rsi \n\t"
"mov (%[addr]), %[reg] \n\t"
"fixed2:"
: [reg]"=r"(r), [fault]"=a"(fault), "=b"(e)
@@ -710,7 +710,7 @@ static int ac_test_do_access(ac_test_t *at)
fault = 0;
}
- asm volatile ("mov $fixed1, %%rsi \n\t"
+ asm volatile ("lea fixed1(%%rip), %%rsi \n\t"
"mov %%rsp, %%rdx \n\t"
"cmp $0, %[user] \n\t"
"jz do_access \n\t"
@@ -719,7 +719,8 @@ static int ac_test_do_access(ac_test_t *at)
"pushq %[user_stack_top] \n\t"
"pushfq \n\t"
"pushq %[user_cs] \n\t"
- "pushq $do_access \n\t"
+ "lea do_access(%%rip), %%r8\n\t"
+ "pushq %%r8\n\t"
"iretq \n"
"do_access: \n\t"
"cmp $0, %[fetch] \n\t"
@@ -744,7 +745,7 @@ static int ac_test_do_access(ac_test_t *at)
[user_cs]"i"(USER_CS),
[user_stack_top]"r"(user_stack + sizeof user_stack),
[kernel_entry_vector]"i"(0x20)
- : "rsi");
+ : "rsi", "r8");
asm volatile (".section .text.pf \n\t"
"page_fault: \n\t"
diff --git a/x86/cet.c b/x86/cet.c
index a21577a..a4b79cb 100644
--- a/x86/cet.c
+++ b/x86/cet.c
@@ -52,7 +52,7 @@ static u64 cet_ibt_func(void)
printf("No endbr64 instruction at jmp target, this triggers #CP...\n");
asm volatile ("movq $2, %rcx\n"
"dec %rcx\n"
- "leaq 2f, %rax\n"
+ "leaq 2f(%rip), %rax\n"
"jmp *%rax \n"
"2:\n"
"dec %rcx\n");
@@ -67,7 +67,8 @@ void test_func(void) {
"pushq %[user_stack_top]\n\t"
"pushfq\n\t"
"pushq %[user_cs]\n\t"
- "pushq $user_mode\n\t"
+ "lea user_mode(%%rip), %%rax\n\t"
+ "pushq %%rax\n\t"
"iretq\n"
"user_mode:\n\t"
@@ -77,7 +78,8 @@ void test_func(void) {
[user_ds]"i"(USER_DS),
[user_cs]"i"(USER_CS),
[user_stack_top]"r"(user_stack +
- sizeof(user_stack)));
+ sizeof(user_stack))
+ : "rax");
}
#define SAVE_REGS() \
diff --git a/x86/emulator.c b/x86/emulator.c
index 9fda1a0..4d2de24 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -262,12 +262,13 @@ static void test_pop(void *mem)
asm volatile("mov %%rsp, %[tmp] \n\t"
"mov %[stack_top], %%rsp \n\t"
- "push $1f \n\t"
+ "lea 1f(%%rip), %%rax \n\t"
+ "push %%rax \n\t"
"ret \n\t"
"2: jmp 2b \n\t"
"1: mov %[tmp], %%rsp"
: [tmp]"=&r"(tmp) : [stack_top]"r"(stack_top)
- : "memory");
+ : "memory", "rax");
report(1, "ret");
stack_top[-1] = 0x778899;
diff --git a/x86/eventinj.c b/x86/eventinj.c
index 46593c9..0cd68e8 100644
--- a/x86/eventinj.c
+++ b/x86/eventinj.c
@@ -155,9 +155,11 @@ asm("do_iret:"
"pushf"W" \n\t"
"mov %cs, %ecx \n\t"
"push"W" %"R "cx \n\t"
- "push"W" $2f \n\t"
+ "lea"W" 2f(%"R "ip), %"R "bx \n\t"
+ "push"W" %"R "bx \n\t"
- "cmpb $0, no_test_device\n\t" // see if need to flush
+ "mov no_test_device(%"R "ip), %bl \n\t"
+ "cmpb $0, %bl\n\t" // see if need to flush
"jnz 1f\n\t"
"outl %eax, $0xe4 \n\t" // flush page
"1: \n\t"
diff --git a/x86/smap.c b/x86/smap.c
index ac2c8d5..b3ee16f 100644
--- a/x86/smap.c
+++ b/x86/smap.c
@@ -161,10 +161,10 @@ int main(int ac, char **av)
test = -1;
asm("or $(" xstr(USER_BASE) "), %"R "sp \n"
"push $44 \n "
- "decl test\n"
+ "decl test(%"R "ip)\n"
"and $~(" xstr(USER_BASE) "), %"R "sp \n"
"pop %"R "ax\n"
- "movl %eax, test");
+ "movl %eax, test(%"R "ip)");
report(pf_count == 0 && test == 44,
"write to user stack with AC=1");
@@ -173,10 +173,10 @@ int main(int ac, char **av)
test = -1;
asm("or $(" xstr(USER_BASE) "), %"R "sp \n"
"push $45 \n "
- "decl test\n"
+ "decl test(%"R "ip)\n"
"and $~(" xstr(USER_BASE) "), %"R "sp \n"
"pop %"R "ax\n"
- "movl %eax, test");
+ "movl %eax, test(%"R "ip)");
report(pf_count == 1 && test == 45 && save == -1,
"write to user stack with AC=0");
diff --git a/x86/umip.c b/x86/umip.c
index c5700b3..8b4e798 100644
--- a/x86/umip.c
+++ b/x86/umip.c
@@ -23,7 +23,10 @@ static void gp_handler(struct ex_regs *regs)
#define GP_ASM(stmt, in, clobber) \
asm volatile ( \
- "mov" W " $1f, %[expected_rip]\n\t" \
+ "push" W " %%" R "ax\n\t" \
+ "lea 1f(%%" R "ip), %%" R "ax\n\t" \
+ "mov %%" R "ax, %[expected_rip]\n\t" \
+ "pop" W " %%" R "ax\n\t" \
"movl $2f-1f, %[skip_count]\n\t" \
"1: " stmt "\n\t" \
"2: " \
@@ -130,7 +133,8 @@ static int do_ring3(void (*fn)(const char *), const char *arg)
"push" W " %%" R "dx \n\t"
"pushf" W "\n\t"
"push" W " %[user_cs] \n\t"
- "push" W " $1f \n\t"
+ "lea 1f(%%" R "ip), %%" R "dx \n\t"
+ "push" W " %%" R "dx \n\t"
"iret" W "\n"
"1: \n\t"
"push %%" R "cx\n\t" /* save kernel SP */
@@ -144,7 +148,7 @@ static int do_ring3(void (*fn)(const char *), const char *arg)
#endif
"pop %%" R "cx\n\t"
- "mov $1f, %%" R "dx\n\t"
+ "lea 1f(%%" R "ip), %%" R "dx\n\t"
"int %[kernel_entry_vector]\n\t"
".section .text.entry \n\t"
"kernel_entry: \n\t"
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* [kvm-unit-tests PATCH v2 11/17] x86 AMD SEV: Initial support
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
` (9 preceding siblings ...)
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 10/17] x86 UEFI: Convert x86 test cases to PIC Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-08-27 14:51 ` Tom Lendacky
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 12/17] x86 AMD SEV: Page table with c-bit Zixuan Wang
` (6 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
AMD Secure Encrypted Virtualization (SEV) is a hardware accelerated
memory encryption feature that protects guest VMs from host attacks.
This commit provides set up code and a test case for AMD SEV. The set up
code checks if SEV is supported and enabled, and then sets SEV c-bit for
each page table entry.
Co-developed-by: Hyunwook (Wooky) Baek <baekhw@google.com>
Signed-off-by: Hyunwook (Wooky) Baek <baekhw@google.com>
Signed-off-by: Zixuan Wang <zixuanwang@google.com>
---
lib/x86/amd_sev.c | 77 +++++++++++++++++++++++++++++++++++++++++++++
lib/x86/amd_sev.h | 45 ++++++++++++++++++++++++++
lib/x86/asm/setup.h | 1 +
lib/x86/setup.c | 15 +++++++++
x86/Makefile.common | 1 +
x86/Makefile.x86_64 | 3 ++
x86/amd_sev.c | 64 +++++++++++++++++++++++++++++++++++++
7 files changed, 206 insertions(+)
create mode 100644 lib/x86/amd_sev.c
create mode 100644 lib/x86/amd_sev.h
create mode 100644 x86/amd_sev.c
diff --git a/lib/x86/amd_sev.c b/lib/x86/amd_sev.c
new file mode 100644
index 0000000..5498ed6
--- /dev/null
+++ b/lib/x86/amd_sev.c
@@ -0,0 +1,77 @@
+/*
+ * AMD SEV support in KVM-Unit-Tests
+ *
+ * Copyright (c) 2021, Google Inc
+ *
+ * Authors:
+ * Zixuan Wang <zixuanwang@google.com>
+ *
+ * SPDX-License-Identifier: LGPL-2.0-or-later
+ */
+
+#include "amd_sev.h"
+#include "x86/processor.h"
+
+static unsigned long long amd_sev_c_bit_pos;
+
+bool amd_sev_enabled(void)
+{
+ struct cpuid cpuid_out;
+ static bool sev_enabled;
+ static bool initialized = false;
+
+ /* Check CPUID and MSR for SEV status and store it for future function calls. */
+ if (!initialized) {
+ sev_enabled = false;
+ initialized = true;
+
+ /* Test if we can query SEV features */
+ cpuid_out = cpuid(CPUID_FN_LARGEST_EXT_FUNC_NUM);
+ if (cpuid_out.a < CPUID_FN_ENCRYPT_MEM_CAPAB) {
+ return sev_enabled;
+ }
+
+ /* Test if SEV is supported */
+ cpuid_out = cpuid(CPUID_FN_ENCRYPT_MEM_CAPAB);
+ if (!(cpuid_out.a & SEV_SUPPORT_MASK)) {
+ return sev_enabled;
+ }
+
+ /* Test if SEV is enabled */
+ if (!(rdmsr(MSR_SEV_STATUS) & SEV_ENABLED_MASK)) {
+ return sev_enabled;
+ }
+
+ sev_enabled = true;
+ }
+
+ return sev_enabled;
+}
+
+efi_status_t setup_amd_sev(void)
+{
+ struct cpuid cpuid_out;
+
+ if (!amd_sev_enabled()) {
+ return EFI_UNSUPPORTED;
+ }
+
+ /*
+ * Extract C-Bit position from ebx[5:0]
+ * AMD64 Architecture Programmer's Manual Volume 3
+ * - Section " Function 8000_001Fh - Encrypted Memory Capabilities"
+ */
+ cpuid_out = cpuid(CPUID_FN_ENCRYPT_MEM_CAPAB);
+ amd_sev_c_bit_pos = (unsigned long long)(cpuid_out.b & 0x3f);
+
+ return EFI_SUCCESS;
+}
+
+unsigned long long get_amd_sev_c_bit_mask(void)
+{
+ if (amd_sev_enabled()) {
+ return 1ull << amd_sev_c_bit_pos;
+ } else {
+ return 0;
+ }
+}
diff --git a/lib/x86/amd_sev.h b/lib/x86/amd_sev.h
new file mode 100644
index 0000000..516d500
--- /dev/null
+++ b/lib/x86/amd_sev.h
@@ -0,0 +1,45 @@
+/*
+ * AMD SEV support in KVM-Unit-Tests
+ *
+ * Copyright (c) 2021, Google Inc
+ *
+ * Authors:
+ * Zixuan Wang <zixuanwang@google.com>
+ *
+ * SPDX-License-Identifier: LGPL-2.0-or-later
+ */
+
+#ifndef _X86_AMD_SEV_H_
+#define _X86_AMD_SEV_H_
+
+#ifdef TARGET_EFI
+
+#include "libcflat.h"
+#include "desc.h"
+#include "asm/page.h"
+#include "efi.h"
+
+/*
+ * AMD Programmer's Manual Volume 3
+ * - Section "Function 8000_0000h - Maximum Extended Function Number and Vendor String"
+ * - Section "Function 8000_001Fh - Encrypted Memory Capabilities"
+ */
+#define CPUID_FN_LARGEST_EXT_FUNC_NUM 0x80000000
+#define CPUID_FN_ENCRYPT_MEM_CAPAB 0x8000001f
+#define SEV_SUPPORT_MASK 0b10
+
+/*
+ * AMD Programmer's Manual Volume 2
+ * - Section "SEV_STATUS MSR"
+ */
+#define MSR_SEV_STATUS 0xc0010131
+#define SEV_ENABLED_MASK 0b1
+
+bool amd_sev_enabled(void);
+efi_status_t setup_amd_sev(void);
+
+unsigned long long get_amd_sev_c_bit_mask(void);
+
+#endif /* TARGET_EFI */
+
+#endif /* _X86_AMD_SEV_H_ */
diff --git a/lib/x86/asm/setup.h b/lib/x86/asm/setup.h
index 16bad0f..d494528 100644
--- a/lib/x86/asm/setup.h
+++ b/lib/x86/asm/setup.h
@@ -8,6 +8,7 @@
#include "x86/smp.h"
#include "asm/page.h"
#include "efi.h"
+#include "x86/amd_sev.h"
/*
* efi_bootinfo_t: stores EFI-related machine info retrieved by
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index 03598fe..bdda337 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -215,6 +215,18 @@ efi_status_t setup_efi_pre_boot(unsigned long *mapkey, efi_bootinfo_t *efi_booti
return status;
}
+ status = setup_amd_sev();
+ if (status != EFI_SUCCESS) {
+ switch (status) {
+ case EFI_UNSUPPORTED:
+ /* Continue if AMD SEV is not supported */
+ break;
+ default:
+ printf("Set up AMD SEV failed\n");
+ return status;
+ }
+ }
+
return EFI_SUCCESS;
}
@@ -233,6 +245,9 @@ static void setup_page_table(void)
/* Set default flags */
flags = PT_PRESENT_MASK | PT_WRITABLE_MASK | PT_USER_MASK;
+ /* Set AMD SEV C-Bit for page table entries */
+ flags |= get_amd_sev_c_bit_mask();
+
/* Level 5 */
curr_pt = (pgd_t *)&ptl5;
curr_pt[0] = ((phys_addr_t)&ptl4) | flags;
diff --git a/x86/Makefile.common b/x86/Makefile.common
index 959379c..0913083 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -23,6 +23,7 @@ cflatobjs += lib/x86/stack.o
cflatobjs += lib/x86/fault_test.o
cflatobjs += lib/x86/delay.o
ifeq ($(TARGET_EFI),y)
+cflatobjs += lib/x86/amd_sev.o
cflatobjs += lib/x86/setup.o
cflatobjs += lib/efi.o
cflatobjs += x86/efi/reloc_x86_64.o
diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
index 7e8a57a..8304939 100644
--- a/x86/Makefile.x86_64
+++ b/x86/Makefile.x86_64
@@ -32,6 +32,9 @@ tests += $(TEST_DIR)/pks.$(exe)
tests += $(TEST_DIR)/pmu_lbr.$(exe)
tests += $(TEST_DIR)/emulator.$(exe)
tests += $(TEST_DIR)/vmware_backdoors.$(exe)
+ifeq ($(TARGET_EFI),y)
+tests += $(TEST_DIR)/amd_sev.$(exe)
+endif
# The following test cases are disabled when building EFI tests because they
# use absolute addresses in their inline assembly code, which cannot compile
diff --git a/x86/amd_sev.c b/x86/amd_sev.c
new file mode 100644
index 0000000..a07a48f
--- /dev/null
+++ b/x86/amd_sev.c
@@ -0,0 +1,64 @@
+/*
+ * AMD SEV test cases
+ *
+ * Copyright (c) 2021, Google Inc
+ *
+ * Authors:
+ * Hyunwook (Wooky) Baek <baekhw@google.com>
+ * Zixuan Wang <zixuanwang@google.com>
+ *
+ * SPDX-License-Identifier: LGPL-2.0-or-later
+ */
+
+#include "libcflat.h"
+#include "x86/processor.h"
+#include "x86/amd_sev.h"
+
+#define EXIT_SUCCESS 0
+#define EXIT_FAILURE 1
+
+static int test_sev_activation(void)
+{
+ struct cpuid cpuid_out;
+ u64 msr_out;
+
+ printf("SEV activation test is loaded.\n");
+
+ /* Tests if CPUID function to check SEV is implemented */
+ cpuid_out = cpuid(CPUID_FN_LARGEST_EXT_FUNC_NUM);
+ printf("CPUID Fn8000_0000[EAX]: 0x%08x\n", cpuid_out.a);
+ if (cpuid_out.a < CPUID_FN_ENCRYPT_MEM_CAPAB) {
+ printf("CPUID does not support FN%08x\n",
+ CPUID_FN_ENCRYPT_MEM_CAPAB);
+ return EXIT_FAILURE;
+ }
+
+ /* Tests if SEV is supported */
+ cpuid_out = cpuid(CPUID_FN_ENCRYPT_MEM_CAPAB);
+ printf("CPUID Fn8000_001F[EAX]: 0x%08x\n", cpuid_out.a);
+ printf("CPUID Fn8000_001F[EBX]: 0x%08x\n", cpuid_out.b);
+ if (!(cpuid_out.a & SEV_SUPPORT_MASK)) {
+ printf("SEV is not supported.\n");
+ return EXIT_FAILURE;
+ }
+ printf("SEV is supported\n");
+
+ /* Tests if SEV is enabled */
+ msr_out = rdmsr(MSR_SEV_STATUS);
+ printf("MSR C001_0131[EAX]: 0x%08lx\n", msr_out & 0xffffffff);
+ if (!(msr_out & SEV_ENABLED_MASK)) {
+ printf("SEV is not enabled.\n");
+ return EXIT_FAILURE;
+ }
+ printf("SEV is enabled\n");
+
+ return EXIT_SUCCESS;
+}
+
+int main(void)
+{
+ int rtn;
+ rtn = test_sev_activation();
+ report(rtn == EXIT_SUCCESS, "SEV activation test.");
+ return report_summary();
+}
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 11/17] x86 AMD SEV: Initial support
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 11/17] x86 AMD SEV: Initial support Zixuan Wang
@ 2021-08-27 14:51 ` Tom Lendacky
2021-08-31 19:36 ` Zixuan Wang
0 siblings, 1 reply; 47+ messages in thread
From: Tom Lendacky @ 2021-08-27 14:51 UTC (permalink / raw)
To: Zixuan Wang, kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, varad.gautam, jroedel, bp
On 8/26/21 10:12 PM, Zixuan Wang wrote:
> AMD Secure Encrypted Virtualization (SEV) is a hardware accelerated
> memory encryption feature that protects guest VMs from host attacks.
>
> This commit provides set up code and a test case for AMD SEV. The set up
> code checks if SEV is supported and enabled, and then sets SEV c-bit for
> each page table entry.
>
> Co-developed-by: Hyunwook (Wooky) Baek <baekhw@google.com>
> Signed-off-by: Hyunwook (Wooky) Baek <baekhw@google.com>
> Signed-off-by: Zixuan Wang <zixuanwang@google.com>
> ---
> lib/x86/amd_sev.c | 77 +++++++++++++++++++++++++++++++++++++++++++++
> lib/x86/amd_sev.h | 45 ++++++++++++++++++++++++++
> lib/x86/asm/setup.h | 1 +
> lib/x86/setup.c | 15 +++++++++
> x86/Makefile.common | 1 +
> x86/Makefile.x86_64 | 3 ++
> x86/amd_sev.c | 64 +++++++++++++++++++++++++++++++++++++
> 7 files changed, 206 insertions(+)
> create mode 100644 lib/x86/amd_sev.c
> create mode 100644 lib/x86/amd_sev.h
> create mode 100644 x86/amd_sev.c
>
> diff --git a/lib/x86/amd_sev.c b/lib/x86/amd_sev.c
> new file mode 100644
> index 0000000..5498ed6
> --- /dev/null
> +++ b/lib/x86/amd_sev.c
> @@ -0,0 +1,77 @@
> +/*
> + * AMD SEV support in KVM-Unit-Tests
> + *
> + * Copyright (c) 2021, Google Inc
> + *
> + * Authors:
> + * Zixuan Wang <zixuanwang@google.com>
> + *
> + * SPDX-License-Identifier: LGPL-2.0-or-later
> + */
> +
> +#include "amd_sev.h"
> +#include "x86/processor.h"
> +
> +static unsigned long long amd_sev_c_bit_pos;
This can be a unsigned short since this is just the bit position, not the
mask.
> +
> +bool amd_sev_enabled(void)
> +{
> + struct cpuid cpuid_out;
> + static bool sev_enabled;
> + static bool initialized = false;
> +
> + /* Check CPUID and MSR for SEV status and store it for future function calls. */
> + if (!initialized) {
> + sev_enabled = false;
> + initialized = true;
> +
> + /* Test if we can query SEV features */
> + cpuid_out = cpuid(CPUID_FN_LARGEST_EXT_FUNC_NUM);
> + if (cpuid_out.a < CPUID_FN_ENCRYPT_MEM_CAPAB) {
> + return sev_enabled;
> + }
> +
> + /* Test if SEV is supported */
> + cpuid_out = cpuid(CPUID_FN_ENCRYPT_MEM_CAPAB);
> + if (!(cpuid_out.a & SEV_SUPPORT_MASK)) {
> + return sev_enabled;
> + }
> +
> + /* Test if SEV is enabled */
> + if (!(rdmsr(MSR_SEV_STATUS) & SEV_ENABLED_MASK)) {
> + return sev_enabled;
> + }
> +
> + sev_enabled = true;
Maybe just make this a bit easier to read by doing:
if (rdmsr(MSR_SEV_STATUS & SEV_ENABLED_MASK)
sev_enabled = true;
No need to return early since you are at the end of the if statement. Just
my opinion, though, not a big deal.
Thanks,
Tom
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 11/17] x86 AMD SEV: Initial support
2021-08-27 14:51 ` Tom Lendacky
@ 2021-08-31 19:36 ` Zixuan Wang
0 siblings, 0 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-08-31 19:36 UTC (permalink / raw)
To: Tom Lendacky
Cc: kvm, pbonzini, drjones, marcorr, baekhw, tmroeder, erdemaktas,
rientjes, seanjc, brijesh.singh, varad.gautam, jroedel, bp
On Fri, Aug 27, 2021 at 7:51 AM Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> On 8/26/21 10:12 PM, Zixuan Wang wrote:
> > +++ b/lib/x86/amd_sev.c
> > @@ -0,0 +1,77 @@
> > +/*
> > + * AMD SEV support in KVM-Unit-Tests
> > + *
> > + * Copyright (c) 2021, Google Inc
> > + *
> > + * Authors:
> > + * Zixuan Wang <zixuanwang@google.com>
> > + *
> > + * SPDX-License-Identifier: LGPL-2.0-or-later
> > + */
> > +
> > +#include "amd_sev.h"
> > +#include "x86/processor.h"
> > +
> > +static unsigned long long amd_sev_c_bit_pos;
>
> This can be a unsigned short since this is just the bit position, not the
> mask.
>
I agree. I will update it in the next version.
> > +
> > +bool amd_sev_enabled(void)
> > +{
> > + struct cpuid cpuid_out;
> > + static bool sev_enabled;
> > + static bool initialized = false;
> > +
> > + /* Check CPUID and MSR for SEV status and store it for future function calls. */
> > + if (!initialized) {
> > + sev_enabled = false;
> > + initialized = true;
> > +
> > + /* Test if we can query SEV features */
> > + cpuid_out = cpuid(CPUID_FN_LARGEST_EXT_FUNC_NUM);
> > + if (cpuid_out.a < CPUID_FN_ENCRYPT_MEM_CAPAB) {
> > + return sev_enabled;
> > + }
> > +
> > + /* Test if SEV is supported */
> > + cpuid_out = cpuid(CPUID_FN_ENCRYPT_MEM_CAPAB);
> > + if (!(cpuid_out.a & SEV_SUPPORT_MASK)) {
> > + return sev_enabled;
> > + }
> > +
> > + /* Test if SEV is enabled */
> > + if (!(rdmsr(MSR_SEV_STATUS) & SEV_ENABLED_MASK)) {
> > + return sev_enabled;
> > + }
> > +
> > + sev_enabled = true;
>
> Maybe just make this a bit easier to read by doing:
>
> if (rdmsr(MSR_SEV_STATUS & SEV_ENABLED_MASK)
> sev_enabled = true;
>
> No need to return early since you are at the end of the if statement. Just
> my opinion, though, not a big deal.
>
> Thanks,
> Tom
>
I agree, I will update it in the next version. Thank you for the comments!
Best regards,
Zixuan
^ permalink raw reply [flat|nested] 47+ messages in thread
* [kvm-unit-tests PATCH v2 12/17] x86 AMD SEV: Page table with c-bit
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
` (10 preceding siblings ...)
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 11/17] x86 AMD SEV: Initial support Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 13/17] x86 AMD SEV-ES: Check SEV-ES status Zixuan Wang
` (5 subsequent siblings)
17 siblings, 0 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
AMD SEV introduces c-bit to page table entries. To work with AMD SEV:
1. c-bit should be set for new page table entries
2. address calculation should not use c-bit as part of address
Signed-off-by: Zixuan Wang <zixuanwang@google.com>
---
lib/x86/amd_sev.c | 10 ++++++++++
lib/x86/amd_sev.h | 1 +
lib/x86/asm/page.h | 27 ++++++++++++++++++++++++---
lib/x86/vm.c | 18 ++++++++++++++----
4 files changed, 49 insertions(+), 7 deletions(-)
diff --git a/lib/x86/amd_sev.c b/lib/x86/amd_sev.c
index 5498ed6..f5e3585 100644
--- a/lib/x86/amd_sev.c
+++ b/lib/x86/amd_sev.c
@@ -75,3 +75,13 @@ unsigned long long get_amd_sev_c_bit_mask(void)
return 0;
}
}
+
+unsigned long long get_amd_sev_addr_upperbound(void)
+{
+ if (amd_sev_enabled()) {
+ return amd_sev_c_bit_pos - 1;
+ } else {
+ /* Default memory upper bound */
+ return PT_ADDR_UPPER_BOUND_DEFAULT;
+ }
+}
diff --git a/lib/x86/amd_sev.h b/lib/x86/amd_sev.h
index 516d500..2780560 100644
--- a/lib/x86/amd_sev.h
+++ b/lib/x86/amd_sev.h
@@ -39,6 +39,7 @@ bool amd_sev_enabled(void);
efi_status_t setup_amd_sev(void);
unsigned long long get_amd_sev_c_bit_mask(void);
+unsigned long long get_amd_sev_addr_upperbound(void);
#endif /* TARGET_EFI */
diff --git a/lib/x86/asm/page.h b/lib/x86/asm/page.h
index f6f740b..c25bc66 100644
--- a/lib/x86/asm/page.h
+++ b/lib/x86/asm/page.h
@@ -25,6 +25,12 @@ typedef unsigned long pgd_t;
#define LARGE_PAGE_SIZE (1024 * PAGE_SIZE)
#endif
+#ifdef TARGET_EFI
+/* lib/x86/amd_sev.c */
+extern unsigned long long get_amd_sev_c_bit_mask(void);
+extern unsigned long long get_amd_sev_addr_upperbound(void);
+#endif /* TARGET_EFI */
+
#define PT_PRESENT_MASK (1ull << 0)
#define PT_WRITABLE_MASK (1ull << 1)
#define PT_USER_MASK (1ull << 2)
@@ -33,10 +39,25 @@ typedef unsigned long pgd_t;
#define PT_PAGE_SIZE_MASK (1ull << 7)
#define PT_GLOBAL_MASK (1ull << 8)
#define PT64_NX_MASK (1ull << 63)
-#define PT_ADDR_MASK GENMASK_ULL(51, 12)
-#define PDPTE64_PAGE_SIZE_MASK (1ull << 7)
-#define PDPTE64_RSVD_MASK GENMASK_ULL(51, cpuid_maxphyaddr())
+/*
+ * Without AMD SEV, the default address upper bound is 51 (i.e., pte bit 51 and
+ * lower bits are addresses). But with AMD SEV enabled, the upper bound is one
+ * bit lower than the c-bit position.
+ */
+#define PT_ADDR_UPPER_BOUND_DEFAULT (51)
+
+#ifdef TARGET_EFI
+#define PT_ADDR_UPPER_BOUND (get_amd_sev_addr_upperbound())
+#else
+#define PT_ADDR_UPPER_BOUND (PT_ADDR_UPPER_BOUND_DEFAULT)
+#endif /* TARGET_EFI */
+
+#define PT_ADDR_LOWER_BOUND (PAGE_SHIFT)
+#define PT_ADDR_MASK GENMASK_ULL(PT_ADDR_UPPER_BOUND, PT_ADDR_LOWER_BOUND)
+
+#define PDPTE64_PAGE_SIZE_MASK (1ull << 7)
+#define PDPTE64_RSVD_MASK GENMASK_ULL(PT_ADDR_UPPER_BOUND, cpuid_maxphyaddr())
#define PT_AD_MASK (PT_ACCESSED_MASK | PT_DIRTY_MASK)
diff --git a/lib/x86/vm.c b/lib/x86/vm.c
index 5cd2ee4..0ebc860 100644
--- a/lib/x86/vm.c
+++ b/lib/x86/vm.c
@@ -26,6 +26,9 @@ pteval_t *install_pte(pgd_t *cr3,
pt_page = 0;
memset(new_pt, 0, PAGE_SIZE);
pt[offset] = virt_to_phys(new_pt) | PT_PRESENT_MASK | PT_WRITABLE_MASK | pte_opt_mask;
+#ifdef TARGET_EFI
+ pt[offset] |= get_amd_sev_c_bit_mask();
+#endif /* TARGET_EFI */
}
pt = phys_to_virt(pt[offset] & PT_ADDR_MASK);
}
@@ -63,7 +66,7 @@ struct pte_search find_pte_level(pgd_t *cr3, void *virt,
if (r.level == lowest_level)
return r;
- pt = phys_to_virt(pte & 0xffffffffff000ull);
+ pt = phys_to_virt(pte & PT_ADDR_MASK);
}
}
@@ -94,13 +97,20 @@ pteval_t *get_pte_level(pgd_t *cr3, void *virt, int pte_level)
pteval_t *install_large_page(pgd_t *cr3, phys_addr_t phys, void *virt)
{
- return install_pte(cr3, 2, virt,
- phys | PT_PRESENT_MASK | PT_WRITABLE_MASK | pte_opt_mask | PT_PAGE_SIZE_MASK, 0);
+ phys_addr_t flags = PT_PRESENT_MASK | PT_WRITABLE_MASK | pte_opt_mask | PT_PAGE_SIZE_MASK;
+#ifdef TARGET_EFI
+ flags |= get_amd_sev_c_bit_mask();
+#endif /* TARGET_EFI */
+ return install_pte(cr3, 2, virt, phys | flags, 0);
}
pteval_t *install_page(pgd_t *cr3, phys_addr_t phys, void *virt)
{
- return install_pte(cr3, 1, virt, phys | PT_PRESENT_MASK | PT_WRITABLE_MASK | pte_opt_mask, 0);
+ phys_addr_t flags = PT_PRESENT_MASK | PT_WRITABLE_MASK | pte_opt_mask;
+#ifdef TARGET_EFI
+ flags |= get_amd_sev_c_bit_mask();
+#endif /* TARGET_EFI */
+ return install_pte(cr3, 1, virt, phys | flags, 0);
}
void install_pages(pgd_t *cr3, phys_addr_t phys, size_t len, void *virt)
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* [kvm-unit-tests PATCH v2 13/17] x86 AMD SEV-ES: Check SEV-ES status
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
` (11 preceding siblings ...)
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 12/17] x86 AMD SEV: Page table with c-bit Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-08-27 14:55 ` Tom Lendacky
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 14/17] x86 AMD SEV-ES: Load GDT with UEFI segments Zixuan Wang
` (4 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
This commit provides initial start up code for KVM-Unit-Tests to run in
an SEV-ES guest VM. This start up code checks if SEV-ES feature is
supported and enabled for the guest.
In this commit, KVM-Unit-Tests can pass the SEV-ES check and enter
setup_efi() function, but crashes in setup_gdt_tss(), which will be
fixed by follow-up commits.
Signed-off-by: Zixuan Wang <zixuanwang@google.com>
---
lib/x86/amd_sev.c | 24 ++++++++++++++++++++++++
lib/x86/amd_sev.h | 7 +++++--
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/lib/x86/amd_sev.c b/lib/x86/amd_sev.c
index f5e3585..8d4df8c 100644
--- a/lib/x86/amd_sev.c
+++ b/lib/x86/amd_sev.c
@@ -67,6 +67,30 @@ efi_status_t setup_amd_sev(void)
return EFI_SUCCESS;
}
+bool amd_sev_es_enabled(void)
+{
+ static bool sev_es_enabled;
+ static bool initialized = false;
+
+ if (!initialized) {
+ sev_es_enabled = false;
+ initialized = true;
+
+ if (!amd_sev_enabled()) {
+ return sev_es_enabled;
+ }
+
+ /* Test if SEV-ES is enabled */
+ if (!(rdmsr(MSR_SEV_STATUS) & SEV_ES_ENABLED_MASK)) {
+ return sev_es_enabled;
+ }
+
+ sev_es_enabled = true;
+ }
+
+ return sev_es_enabled;
+}
+
unsigned long long get_amd_sev_c_bit_mask(void)
{
if (amd_sev_enabled()) {
diff --git a/lib/x86/amd_sev.h b/lib/x86/amd_sev.h
index 2780560..b73a872 100644
--- a/lib/x86/amd_sev.h
+++ b/lib/x86/amd_sev.h
@@ -32,12 +32,15 @@
* AMD Programmer's Manual Volume 2
* - Section "SEV_STATUS MSR"
*/
-#define MSR_SEV_STATUS 0xc0010131
-#define SEV_ENABLED_MASK 0b1
+#define MSR_SEV_STATUS 0xc0010131
+#define SEV_ENABLED_MASK 0b1
+#define SEV_ES_ENABLED_MASK 0b10
bool amd_sev_enabled(void);
efi_status_t setup_amd_sev(void);
+bool amd_sev_es_enabled(void);
+
unsigned long long get_amd_sev_c_bit_mask(void);
unsigned long long get_amd_sev_addr_upperbound(void);
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 13/17] x86 AMD SEV-ES: Check SEV-ES status
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 13/17] x86 AMD SEV-ES: Check SEV-ES status Zixuan Wang
@ 2021-08-27 14:55 ` Tom Lendacky
2021-08-31 19:38 ` Zixuan Wang
0 siblings, 1 reply; 47+ messages in thread
From: Tom Lendacky @ 2021-08-27 14:55 UTC (permalink / raw)
To: Zixuan Wang, kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, varad.gautam, jroedel, bp
On 8/26/21 10:12 PM, Zixuan Wang wrote:
> This commit provides initial start up code for KVM-Unit-Tests to run in
> an SEV-ES guest VM. This start up code checks if SEV-ES feature is
> supported and enabled for the guest.
>
> In this commit, KVM-Unit-Tests can pass the SEV-ES check and enter
> setup_efi() function, but crashes in setup_gdt_tss(), which will be
> fixed by follow-up commits.
>
> Signed-off-by: Zixuan Wang <zixuanwang@google.com>
> ---
> lib/x86/amd_sev.c | 24 ++++++++++++++++++++++++
> lib/x86/amd_sev.h | 7 +++++--
> 2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/lib/x86/amd_sev.c b/lib/x86/amd_sev.c
> index f5e3585..8d4df8c 100644
> --- a/lib/x86/amd_sev.c
> +++ b/lib/x86/amd_sev.c
> @@ -67,6 +67,30 @@ efi_status_t setup_amd_sev(void)
> return EFI_SUCCESS;
> }
>
> +bool amd_sev_es_enabled(void)
> +{
> + static bool sev_es_enabled;
> + static bool initialized = false;
> +
> + if (!initialized) {
> + sev_es_enabled = false;
> + initialized = true;
> +
> + if (!amd_sev_enabled()) {
> + return sev_es_enabled;
> + }
> +
> + /* Test if SEV-ES is enabled */
> + if (!(rdmsr(MSR_SEV_STATUS) & SEV_ES_ENABLED_MASK)) {
> + return sev_es_enabled;
> + }
> +
> + sev_es_enabled = true;
Same comment here as previous for the amd_sev_enabled() function in
regards to readability.
Thanks,
Tom
> + }
> +
> + return sev_es_enabled;
> +}
> +
> unsigned long long get_amd_sev_c_bit_mask(void)
> {
> if (amd_sev_enabled()) {
> diff --git a/lib/x86/amd_sev.h b/lib/x86/amd_sev.h
> index 2780560..b73a872 100644
> --- a/lib/x86/amd_sev.h
> +++ b/lib/x86/amd_sev.h
> @@ -32,12 +32,15 @@
> * AMD Programmer's Manual Volume 2
> * - Section "SEV_STATUS MSR"
> */
> -#define MSR_SEV_STATUS 0xc0010131
> -#define SEV_ENABLED_MASK 0b1
> +#define MSR_SEV_STATUS 0xc0010131
> +#define SEV_ENABLED_MASK 0b1
> +#define SEV_ES_ENABLED_MASK 0b10
>
> bool amd_sev_enabled(void);
> efi_status_t setup_amd_sev(void);
>
> +bool amd_sev_es_enabled(void);
> +
> unsigned long long get_amd_sev_c_bit_mask(void);
> unsigned long long get_amd_sev_addr_upperbound(void);
>
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 13/17] x86 AMD SEV-ES: Check SEV-ES status
2021-08-27 14:55 ` Tom Lendacky
@ 2021-08-31 19:38 ` Zixuan Wang
0 siblings, 0 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-08-31 19:38 UTC (permalink / raw)
To: Tom Lendacky
Cc: kvm, pbonzini, drjones, marcorr, baekhw, tmroeder, erdemaktas,
rientjes, seanjc, brijesh.singh, varad.gautam, jroedel, bp
On Fri, Aug 27, 2021 at 7:56 AM Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> On 8/26/21 10:12 PM, Zixuan Wang wrote:
> > +bool amd_sev_es_enabled(void)
> > +{
> > + static bool sev_es_enabled;
> > + static bool initialized = false;
> > +
> > + if (!initialized) {
> > + sev_es_enabled = false;
> > + initialized = true;
> > +
> > + if (!amd_sev_enabled()) {
> > + return sev_es_enabled;
> > + }
> > +
> > + /* Test if SEV-ES is enabled */
> > + if (!(rdmsr(MSR_SEV_STATUS) & SEV_ES_ENABLED_MASK)) {
> > + return sev_es_enabled;
> > + }
> > +
> > + sev_es_enabled = true;
>
> Same comment here as previous for the amd_sev_enabled() function in
> regards to readability.
>
> Thanks,
> Tom
Got it, I will update it in the next version. Thank you for the suggestion!
Best regards,
Zixuan
^ permalink raw reply [flat|nested] 47+ messages in thread
* [kvm-unit-tests PATCH v2 14/17] x86 AMD SEV-ES: Load GDT with UEFI segments
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
` (12 preceding siblings ...)
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 13/17] x86 AMD SEV-ES: Check SEV-ES status Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-09-20 16:00 ` Paolo Bonzini
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 15/17] x86 AMD SEV-ES: Copy UEFI #VC IDT entry Zixuan Wang
` (3 subsequent siblings)
17 siblings, 1 reply; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
Before this commit, KVM-Unit-Tests set up process crashes when executing
'lgdt' instruction under SEV-ES. This is because lgdt triggers UEFI
procedures (e.g. UEFI #VC handler) that require UEFI's code and data
segments. But these segments are are not compatible with KVM-Unit-Tests
GDT:
UEFI uses 0x30 as code segment and 0x38 as data segment, but in
KVM-Unit-Tests' GDT, 0x30 is a data segment, and 0x38 is a code segment.
This discrepancy crashes the UEFI procedures and thus crashes the 'lgdt'
execution.
This commit fixes this issue by copying UEFI GDT's code and data
segments into KVM-Unit-Tests GDT, so that UEFI procedures (e.g. UEFI #VC
handler) can work.
In this commit, the guest VM passes setup_gdt_tss() but crashes in
load_idt(), which will be fixed by follow-up commits.
Signed-off-by: Zixuan Wang <zixuanwang@google.com>
---
lib/x86/amd_sev.c | 41 +++++++++++++++++++++++++++++++++++++++++
lib/x86/amd_sev.h | 1 +
lib/x86/setup.c | 4 ++++
3 files changed, 46 insertions(+)
diff --git a/lib/x86/amd_sev.c b/lib/x86/amd_sev.c
index 8d4df8c..c9fabc4 100644
--- a/lib/x86/amd_sev.c
+++ b/lib/x86/amd_sev.c
@@ -91,6 +91,47 @@ bool amd_sev_es_enabled(void)
return sev_es_enabled;
}
+static void copy_gdt_entry(gdt_entry_t *dst, gdt_entry_t *src, unsigned segment)
+{
+ unsigned index;
+
+ index = segment / sizeof(gdt_entry_t);
+ dst[index] = src[index];
+}
+
+/* Defined in x86/efi/efistart64.S */
+extern gdt_entry_t gdt64[];
+
+/*
+ * Copy UEFI's code and data segments to KVM-Unit-Tests GDT.
+ *
+ * This is because KVM-Unit-Tests reuses UEFI #VC handler that requires UEFI
+ * code and data segments to run. The UEFI #VC handler crashes the guest VM if
+ * these segments are not available. So we need to copy these two UEFI segments
+ * into KVM-Unit-Tests GDT.
+ *
+ * UEFI uses 0x30 as code segment and 0x38 as data segment. Fortunately, these
+ * segments can be safely overridden in KVM-Unit-Tests as they are used as
+ * protected mode and real mode segments (see x86/efi/efistart64.S for more
+ * details), which are not used in EFI set up process.
+ */
+void copy_uefi_segments(void)
+{
+ if (!amd_sev_es_enabled()) {
+ return;
+ }
+
+ /* GDT and GDTR in current UEFI */
+ gdt_entry_t *gdt_curr;
+ struct descriptor_table_ptr gdtr_curr;
+
+ /* Copy code and data segments from UEFI */
+ sgdt(&gdtr_curr);
+ gdt_curr = (gdt_entry_t *)gdtr_curr.base;
+ copy_gdt_entry(gdt64, gdt_curr, read_cs());
+ copy_gdt_entry(gdt64, gdt_curr, read_ds());
+}
+
unsigned long long get_amd_sev_c_bit_mask(void)
{
if (amd_sev_enabled()) {
diff --git a/lib/x86/amd_sev.h b/lib/x86/amd_sev.h
index b73a872..0b4ff8c 100644
--- a/lib/x86/amd_sev.h
+++ b/lib/x86/amd_sev.h
@@ -40,6 +40,7 @@ bool amd_sev_enabled(void);
efi_status_t setup_amd_sev(void);
bool amd_sev_es_enabled(void);
+void copy_uefi_segments(void);
unsigned long long get_amd_sev_c_bit_mask(void);
unsigned long long get_amd_sev_addr_upperbound(void);
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index bdda337..c6eb3e9 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -324,6 +324,10 @@ static void setup_gdt_tss(void)
tss_hi->limit_low = (u16)((curr_tss_addr >> 32) & 0xffff);
tss_hi->base_low = (u16)((curr_tss_addr >> 48) & 0xffff);
+ if (amd_sev_es_enabled()) {
+ copy_uefi_segments();
+ }
+
load_gdt_tss(tss_offset);
}
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 14/17] x86 AMD SEV-ES: Load GDT with UEFI segments
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 14/17] x86 AMD SEV-ES: Load GDT with UEFI segments Zixuan Wang
@ 2021-09-20 16:00 ` Paolo Bonzini
2021-09-21 4:41 ` Zixuan Wang
0 siblings, 1 reply; 47+ messages in thread
From: Paolo Bonzini @ 2021-09-20 16:00 UTC (permalink / raw)
To: Zixuan Wang, kvm, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
On 27/08/21 05:12, Zixuan Wang wrote:
> + *
> + * This is because KVM-Unit-Tests reuses UEFI #VC handler that requires UEFI
> + * code and data segments to run. The UEFI #VC handler crashes the guest VM if
> + * these segments are not available. So we need to copy these two UEFI segments
> + * into KVM-Unit-Tests GDT.
> + *
> + * UEFI uses 0x30 as code segment and 0x38 as data segment. Fortunately, these
> + * segments can be safely overridden in KVM-Unit-Tests as they are used as
> + * protected mode and real mode segments (see x86/efi/efistart64.S for more
> + * details), which are not used in EFI set up process.
Is 0x30/0x38 the same as kvm-unit-tests's 0x08/0x10? Can kvm-unit-tests
simply change its ring-0 64-bit CS/DS to 0x30 and 0x38 instead of 0x08
and 0x10? I can help with that too, since there would be some more
shuffling to keep similar descriptors together:
* 0x00 NULL descriptor NULL descriptor
* 0x08 intr_alt_stack TSS ring-0 code segment (32-bit)
* 0x10 (0x13) **unused** ring-3 code segment (64-bit)
* 0x18 ring-0 code segment (P=0) ring-0 code segment (64-bit, P=0)
* 0x20 ring-0 code segment (16-bit) same
* 0x28 ring-0 data segment (16-bit) same
* 0x30 ring-0 code segment (32-bit) ring-0 code segment (64-bit)
* 0x38 ring-0 data segment (32-bit) ring-0 data segment (32/64-bit)
* 0x40 (0x43) ring-3 code segment (32-bit) same
* 0x48 (0x4b) ring-3 data segment (32-bit) ring-3 data segment (32/64-bit)
* 0x50-0x78 free to use for test cases same
or:
old new
---- ----
0x00 0x00
0x20 0x08
0x48 0x10
0x18 0x18
0x28 0x20
0x30 0x28
0x08 0x30
0x10 0x38
0x38 0x40
0x40 0x48
Thanks,
Paolo
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [kvm-unit-tests PATCH v2 14/17] x86 AMD SEV-ES: Load GDT with UEFI segments
2021-09-20 16:00 ` Paolo Bonzini
@ 2021-09-21 4:41 ` Zixuan Wang
0 siblings, 0 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-09-21 4:41 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Zixuan Wang, kvm, drjones, Marc Orr, Hyunwook (Wooky) Baek,
tmroeder, erdemaktas, rientjes, seanjc, brijesh.singh,
Thomas.Lendacky, varad.gautam, jroedel, bp
On Mon, Sep 20, 2021 at 6:27 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 27/08/21 05:12, Zixuan Wang wrote:
> > + *
> > + * This is because KVM-Unit-Tests reuses UEFI #VC handler that requires UEFI
> > + * code and data segments to run. The UEFI #VC handler crashes the guest VM if
> > + * these segments are not available. So we need to copy these two UEFI segments
> > + * into KVM-Unit-Tests GDT.
> > + *
> > + * UEFI uses 0x30 as code segment and 0x38 as data segment. Fortunately, these
> > + * segments can be safely overridden in KVM-Unit-Tests as they are used as
> > + * protected mode and real mode segments (see x86/efi/efistart64.S for more
> > + * details), which are not used in EFI set up process.
>
> Is 0x30/0x38 the same as kvm-unit-tests's 0x08/0x10? Can kvm-unit-tests
> simply change its ring-0 64-bit CS/DS to 0x30 and 0x38 instead of 0x08
> and 0x10? I can help with that too, since there would be some more
> shuffling to keep similar descriptors together:
>
> * 0x00 NULL descriptor NULL descriptor
> * 0x08 intr_alt_stack TSS ring-0 code segment (32-bit)
> * 0x10 (0x13) **unused** ring-3 code segment (64-bit)
> * 0x18 ring-0 code segment (P=0) ring-0 code segment (64-bit, P=0)
> * 0x20 ring-0 code segment (16-bit) same
> * 0x28 ring-0 data segment (16-bit) same
> * 0x30 ring-0 code segment (32-bit) ring-0 code segment (64-bit)
> * 0x38 ring-0 data segment (32-bit) ring-0 data segment (32/64-bit)
> * 0x40 (0x43) ring-3 code segment (32-bit) same
> * 0x48 (0x4b) ring-3 data segment (32-bit) ring-3 data segment (32/64-bit)
> * 0x50-0x78 free to use for test cases same
>
> or:
>
> old new
> ---- ----
> 0x00 0x00
> 0x20 0x08
> 0x48 0x10
> 0x18 0x18
> 0x28 0x20
> 0x30 0x28
> 0x08 0x30
> 0x10 0x38
> 0x38 0x40
> 0x40 0x48
>
> Thanks,
>
> Paolo
>
Thank you for the detailed explanation! Updating KVM-unit-tests GDT is
one way to solve the problem, but we found a more straightforward
solution [1]:
We found it possible to update the 'code segment selector' field in
the #VC IDT entry and point it to the KVM-unit-tests code segment.
This allows the UEFI #VC handler to use KVM-unit-tests segments, and
we do not need to copy the UEFI segments.
I will update this into the next version.
[1] https://github.com/TheNetAdmin/KVM-Unit-Tests-dev-fork/commit/ab480fd0fbad813c2922526a0bccadf121cb9240
Best regards,
Zixuan
^ permalink raw reply [flat|nested] 47+ messages in thread
* [kvm-unit-tests PATCH v2 15/17] x86 AMD SEV-ES: Copy UEFI #VC IDT entry
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
` (13 preceding siblings ...)
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 14/17] x86 AMD SEV-ES: Load GDT with UEFI segments Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 16/17] x86 AMD SEV-ES: Set up GHCB page Zixuan Wang
` (2 subsequent siblings)
17 siblings, 0 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
AMD SEV-ES introduces a new #VC exception that handles the communication
between guest and host. UEFI already implements a #VC handler so there
is no need to re-implement it in KVM-Unit-Tests. To reuse this #VC
handler, this commit reads UEFI's IDT, copy the #VC IDT entry into
KVM-Unit-Tests' IDT.
Reusing UEFI #VC handler is a temporary workaround, and the long-term
solution is to implement a #VC handler in KVM-Unit-Tests so it does not
depend on specific UEFI's #VC handler. However, we still believe that
the current approach is good as an intermediate solution, because it
unlocks a lot of testing and we do not expect that testing to be
inherently tied to the UEFI's #VC handler.
In this commit, load_idt() can work and now guest crashes in
setup_page_table(), which will be fixed by follow-up commits.
Signed-off-by: Zixuan Wang <zixuanwang@google.com>
---
lib/x86/amd_sev.c | 25 +++++++++++++++++++++++++
lib/x86/amd_sev.h | 7 +++++++
lib/x86/setup.c | 12 ++++++++++++
3 files changed, 44 insertions(+)
diff --git a/lib/x86/amd_sev.c b/lib/x86/amd_sev.c
index c9fabc4..d1e43ae 100644
--- a/lib/x86/amd_sev.c
+++ b/lib/x86/amd_sev.c
@@ -91,6 +91,31 @@ bool amd_sev_es_enabled(void)
return sev_es_enabled;
}
+efi_status_t setup_amd_sev_es(void)
+{
+ struct descriptor_table_ptr idtr;
+ idt_entry_t *idt;
+
+ if (!amd_sev_es_enabled()) {
+ return EFI_UNSUPPORTED;
+ }
+
+ /*
+ * Copy UEFI's #VC IDT entry, so KVM-Unit-Tests can reuse it and does
+ * not have to re-implement a #VC handler.
+ *
+ * TODO: Reusing UEFI #VC handler is a temporary workaround to simplify
+ * the boot up process, the long-term solution is to implement a #VC
+ * handler in KVM-Unit-Tests and load it, so that KVM-Unit-Tests does
+ * not depend on specific UEFI #VC handler implementation.
+ */
+ sidt(&idtr);
+ idt = (idt_entry_t *)idtr.base;
+ boot_idt[SEV_ES_VC_HANDLER_VECTOR] = idt[SEV_ES_VC_HANDLER_VECTOR];
+
+ return EFI_SUCCESS;
+}
+
static void copy_gdt_entry(gdt_entry_t *dst, gdt_entry_t *src, unsigned segment)
{
unsigned index;
diff --git a/lib/x86/amd_sev.h b/lib/x86/amd_sev.h
index 0b4ff8c..aaa4806 100644
--- a/lib/x86/amd_sev.h
+++ b/lib/x86/amd_sev.h
@@ -39,7 +39,14 @@
bool amd_sev_enabled(void);
efi_status_t setup_amd_sev(void);
+/*
+ * AMD Programmer's Manual Volume 2
+ * - Section "#VC Exception"
+ */
+#define SEV_ES_VC_HANDLER_VECTOR 29
+
bool amd_sev_es_enabled(void);
+efi_status_t setup_amd_sev_es(void);
void copy_uefi_segments(void);
unsigned long long get_amd_sev_c_bit_mask(void);
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index c6eb3e9..9075a22 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -227,6 +227,18 @@ efi_status_t setup_efi_pre_boot(unsigned long *mapkey, efi_bootinfo_t *efi_booti
}
}
+ status = setup_amd_sev_es();
+ if (status != EFI_SUCCESS) {
+ switch (status) {
+ case EFI_UNSUPPORTED:
+ /* Continue if AMD SEV-ES is not supported */
+ break;
+ default:
+ printf("Set up AMD SEV-ES failed\n");
+ return status;
+ }
+ }
+
return EFI_SUCCESS;
}
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* [kvm-unit-tests PATCH v2 16/17] x86 AMD SEV-ES: Set up GHCB page
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
` (14 preceding siblings ...)
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 15/17] x86 AMD SEV-ES: Copy UEFI #VC IDT entry Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 17/17] x86 AMD SEV-ES: Add test cases Zixuan Wang
2021-10-04 13:27 ` [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Andrew Jones
17 siblings, 0 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
AMD SEV-ES introduces a GHCB page for guest/host communication. This
page should be unencrypted, i.e. its c-bit should be unset, otherwise
the guest VM may crash when #VC exception happens.
By default, KVM-Unit-Tests only sets up 2MiB pages, i.e. only Level 2
page table entries are provided. Unsetting GHCB Level 2 pte's c-bit
still crashes the guest VM. The solution is to unset only its Level 1
pte's c-bit.
This commit provides GHCB page set up code that:
1. finds GHCB Level 1 pte
2. if not found, installs corresponding Level 1 pages
3. unsets GHCB Level 1 pte's c-bit
In this commit, KVM-Unit-Tests can run in an SEV-ES VM and boot into
test cases' main().
Signed-off-by: Zixuan Wang <zixuanwang@google.com>
---
lib/x86/amd_sev.c | 37 +++++++++++++++++++++++++++++++++++++
lib/x86/amd_sev.h | 7 +++++++
lib/x86/setup.c | 4 ++++
3 files changed, 48 insertions(+)
diff --git a/lib/x86/amd_sev.c b/lib/x86/amd_sev.c
index d1e43ae..e004a7e 100644
--- a/lib/x86/amd_sev.c
+++ b/lib/x86/amd_sev.c
@@ -11,6 +11,7 @@
#include "amd_sev.h"
#include "x86/processor.h"
+#include "x86/vm.h"
static unsigned long long amd_sev_c_bit_pos;
@@ -116,6 +117,42 @@ efi_status_t setup_amd_sev_es(void)
return EFI_SUCCESS;
}
+void setup_ghcb_pte(pgd_t *page_table)
+{
+ /*
+ * SEV-ES guest uses GHCB page to communicate with the host. This page
+ * must be unencrypted, i.e. its c-bit should be unset. To do so, this
+ * function searches GHCB's L1 pte, creates corresponding L1 ptes if not
+ * found, and unsets the c-bit of GHCB's L1 pte.
+ */
+ phys_addr_t ghcb_addr, ghcb_base_addr;
+ pteval_t *pte;
+
+ /* Read the current GHCB page addr */
+ ghcb_addr = rdmsr(SEV_ES_GHCB_MSR_INDEX);
+
+ /* Search Level 1 page table entry for GHCB page */
+ pte = get_pte_level(page_table, (void *)ghcb_addr, 1);
+
+ /* Create Level 1 pte for GHCB page if not found */
+ if (pte == NULL) {
+ /* Find Level 2 page base address */
+ ghcb_base_addr = ghcb_addr & ~(LARGE_PAGE_SIZE - 1);
+ /* Install Level 1 ptes */
+ install_pages(page_table, ghcb_base_addr, LARGE_PAGE_SIZE, (void *)ghcb_base_addr);
+ /* Find Level 2 pte, set as 4KB pages */
+ pte = get_pte_level(page_table, (void *)ghcb_addr, 2);
+ assert(pte);
+ *pte &= ~(PT_PAGE_SIZE_MASK);
+ /* Find Level 1 GHCB pte */
+ pte = get_pte_level(page_table, (void *)ghcb_addr, 1);
+ assert(pte);
+ }
+
+ /* Unset c-bit in Level 1 GHCB pte */
+ *pte &= ~(get_amd_sev_c_bit_mask());
+}
+
static void copy_gdt_entry(gdt_entry_t *dst, gdt_entry_t *src, unsigned segment)
{
unsigned index;
diff --git a/lib/x86/amd_sev.h b/lib/x86/amd_sev.h
index aaa4806..81f5605 100644
--- a/lib/x86/amd_sev.h
+++ b/lib/x86/amd_sev.h
@@ -45,8 +45,15 @@ efi_status_t setup_amd_sev(void);
*/
#define SEV_ES_VC_HANDLER_VECTOR 29
+/*
+ * AMD Programmer's Manual Volume 2
+ * - Section "GHCB"
+ */
+#define SEV_ES_GHCB_MSR_INDEX 0xc0010130
+
bool amd_sev_es_enabled(void);
efi_status_t setup_amd_sev_es(void);
+void setup_ghcb_pte(pgd_t *page_table);
void copy_uefi_segments(void);
unsigned long long get_amd_sev_c_bit_mask(void);
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index 9075a22..0de3ec2 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -278,6 +278,10 @@ static void setup_page_table(void)
curr_pt[i] = ((phys_addr_t)(i << 21)) | flags;
}
+ if (amd_sev_es_enabled()) {
+ setup_ghcb_pte((pgd_t *)&ptl4);
+ }
+
/* Load 4-level page table */
write_cr3((ulong)&ptl4);
}
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* [kvm-unit-tests PATCH v2 17/17] x86 AMD SEV-ES: Add test cases
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
` (15 preceding siblings ...)
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 16/17] x86 AMD SEV-ES: Set up GHCB page Zixuan Wang
@ 2021-08-27 3:12 ` Zixuan Wang
2021-10-04 13:27 ` [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Andrew Jones
17 siblings, 0 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-08-27 3:12 UTC (permalink / raw)
To: kvm, pbonzini, drjones
Cc: marcorr, baekhw, tmroeder, erdemaktas, rientjes, seanjc,
brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel, bp
SEV-ES introduces #VC handler for guest/host communications, e.g.,
accessing MSR, executing CPUID. This commit provides test cases to check
if SEV-ES is enabled and if rdmsr/wrmsr are handled correctly in SEV-ES.
Signed-off-by: Zixuan Wang <zixuanwang@google.com>
---
x86/amd_sev.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/x86/amd_sev.c b/x86/amd_sev.c
index a07a48f..21a491c 100644
--- a/x86/amd_sev.c
+++ b/x86/amd_sev.c
@@ -13,6 +13,7 @@
#include "libcflat.h"
#include "x86/processor.h"
#include "x86/amd_sev.h"
+#include "msr.h"
#define EXIT_SUCCESS 0
#define EXIT_FAILURE 1
@@ -55,10 +56,39 @@ static int test_sev_activation(void)
return EXIT_SUCCESS;
}
+static int test_sev_es_activation(void)
+{
+ if (!(rdmsr(MSR_SEV_STATUS) & SEV_ES_ENABLED_MASK)) {
+ return EXIT_FAILURE;
+ }
+
+ return EXIT_SUCCESS;
+}
+
+static int test_sev_es_msr(void)
+{
+ /*
+ * With SEV-ES, rdmsr/wrmsr trigger #VC exception. If #VC is handled
+ * correctly, rdmsr/wrmsr should work like without SEV-ES and not crash
+ * the guest VM.
+ */
+ u64 val = 0x1234;
+ wrmsr(MSR_TSC_AUX, val);
+ if(val != rdmsr(MSR_TSC_AUX)) {
+ return EXIT_FAILURE;
+ }
+
+ return EXIT_SUCCESS;
+}
+
int main(void)
{
int rtn;
rtn = test_sev_activation();
report(rtn == EXIT_SUCCESS, "SEV activation test.");
+ rtn = test_sev_es_activation();
+ report(rtn == EXIT_SUCCESS, "SEV-ES activation test.");
+ rtn = test_sev_es_msr();
+ report(rtn == EXIT_SUCCESS, "SEV-ES MSR test.");
return report_summary();
}
--
2.33.0.259.gc128427fd7-goog
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support
2021-08-27 3:12 [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Zixuan Wang
` (16 preceding siblings ...)
2021-08-27 3:12 ` [kvm-unit-tests PATCH v2 17/17] x86 AMD SEV-ES: Add test cases Zixuan Wang
@ 2021-10-04 13:27 ` Andrew Jones
2021-10-04 20:54 ` Zixuan Wang
17 siblings, 1 reply; 47+ messages in thread
From: Andrew Jones @ 2021-10-04 13:27 UTC (permalink / raw)
To: zxwang42
Cc: kvm, pbonzini, marcorr, baekhw, tmroeder, erdemaktas, rientjes,
seanjc, brijesh.singh, Thomas.Lendacky, varad.gautam, jroedel,
bp
On Fri, Aug 27, 2021 at 03:12:05AM +0000, Zixuan Wang wrote:
> Hello,
>
> This patch series updates the x86_64 KVM-Unit-Tests to run under UEFI
> and culminates in enabling AMD SEV/SEV-ES. The patches are organized as
> three parts.
>
> The first part (patches 1-2) copies code from Varad's patch set [1]
> that builds EFI stubs without depending on GNU-EFI. Part 2 and 3 are
> built on top of this part.
>
> The second part (patches 3-10) enables the x86_64 test cases to run
> under UEFI. In particular, these patches allow the x86_64 test cases to
> be built as EFI executables and take full control of the guest VM. The
> efi_main() function sets up the KVM-Unit-Tests framework to run under
> UEFI and then launches the test cases' main() function. To date, we
> have 38/43 test cases running with UEFI using this approach.
>
> The third part of the series (patches 11-17) focuses on SEV. In
> particular, these patches introduce SEV/SEV-ES set up code into the EFI
> set up process, including checking if SEV is supported, setting c-bits
> for page table entries, and (notably) reusing the UEFI #VC handler so
> that the set up process does not need to re-implement it (a test case
> can always implement a new #VC handler and load it after set up is
> finished). Using this approach, we are able to launch the x86_64 test
> cases under SEV-ES and exercise KVM's VMGEXIT handler.
>
> Note, a previous feedback [3] indicated that long-term we'd like to
> instrument KVM-Unit-Tests with it's own #VC handler. However, we still
> believe that the current approach is good as an intermediate solution,
> because it unlocks a lot of testing and we do not expect that testing
> to be inherently tied to the UEFI's #VC handler. Rather, test cases
> should be tied to the underlying GHCB spec implemented by an
> arbitrary #VC handler.
>
> See the Part 1 to Part 3 summaries, below, for a high-level breakdown
> of how the patches are organized.
>
> Part 1 Summary:
> Commits 1-2 copy code from Varad's patch set [1] that implements
> EFI-related helper functions to replace the GNU-EFI library.
>
> Part 2 Summary:
> Commits 3-4 introduce support to build test cases with EFI support.
>
> Commits 5-9 set up KVM-Unit-Tests to run under UEFI. In doing so, these
> patches incrementally enable most existing x86_64 test cases to run
> under UEFI.
>
> Commit 10 fixes several test cases that fail to compile with EFI due
> to UEFI's position independent code (PIC) requirement.
>
> Part 3 Summary:
> Commits 11-12 introduce support for SEV by adding code to set the SEV
> c-bit in page table entries.
>
> Commits 13-16 introduce support for SEV-ES by reusing the UEFI #VC
> handler in KVM-Unit-Tests. They also fix GDT and IDT issues that occur
> when reusing UEFI functions in KVM-Unit-Tests.
>
> Commit 17 adds additional test cases for SEV-ES.
>
> Changes in V2:
> 1.Merge Varad's patch set [1] as the foundation of this V2 patch set.
> 2.Remove AMD SEV/SEV-ES config flags and macros (patches 11-17)
> 3.Drop one commit 'x86 UEFI: Move setjmp.h out of desc.h' because we do
> not link GNU-EFI library.
>
> Notes on authorships and attributions:
> The first two commits are from Varad's patch set [1], so they are
> tagged as 'From:' and 'Signed-off-by:' Varad. Commits 3-7 are from our
> V1 patch set [2], and since Varad implemented similar code [1], these
> commits are tagged as 'Co-developed-by:' and 'Signed-off-by:' Varad.
>
> Notes on patch sets merging strategy:
> We understand that the current merging strategy (reorganizing and
> squeezing Varad's patches into two) reduces Varad's authorships, and we
> hope the additional attribution tags make up for it. We see another
> approach which is to build our patch set on top of Varad's original
> patch set, but this creates some noise in the final patch set, e.g.,
> x86/cstart64.S is modified in Varad's part and later reverted in our
> part as we implement start up code in C. For the sake of the clarity of
> the code history, we believe the current approach is the best effort so
> far, and we are open to all kinds of opinions.
>
> [1] https://lore.kernel.org/kvm/20210819113400.26516-1-varad.gautam@suse.com/
> [2] https://lore.kernel.org/kvm/20210818000905.1111226-1-zixuanwang@google.com/
> [3] https://lore.kernel.org/kvm/YSA%2FsYhGgMU72tn+@google.com/
>
> Regards,
> Zixuan Wang
>
> Varad Gautam (2):
> x86 UEFI: Copy code from Linux
> x86 UEFI: Implement UEFI function calls
>
> Zixuan Wang (15):
> x86 UEFI: Copy code from GNU-EFI
> x86 UEFI: Boot from UEFI
> x86 UEFI: Load IDT after UEFI boot up
> x86 UEFI: Load GDT and TSS after UEFI boot up
> x86 UEFI: Set up memory allocator
> x86 UEFI: Set up RSDP after UEFI boot up
> x86 UEFI: Set up page tables
> x86 UEFI: Convert x86 test cases to PIC
> x86 AMD SEV: Initial support
> x86 AMD SEV: Page table with c-bit
> x86 AMD SEV-ES: Check SEV-ES status
> x86 AMD SEV-ES: Load GDT with UEFI segments
> x86 AMD SEV-ES: Copy UEFI #VC IDT entry
> x86 AMD SEV-ES: Set up GHCB page
> x86 AMD SEV-ES: Add test cases
>
> .gitignore | 3 +
> Makefile | 29 +-
> README.md | 6 +
> configure | 6 +
> lib/efi.c | 117 ++++++++
> lib/efi.h | 18 ++
> lib/linux/uefi.h | 539 +++++++++++++++++++++++++++++++++++++
> lib/x86/acpi.c | 38 ++-
> lib/x86/acpi.h | 11 +
> lib/x86/amd_sev.c | 214 +++++++++++++++
> lib/x86/amd_sev.h | 64 +++++
> lib/x86/asm/page.h | 28 +-
> lib/x86/asm/setup.h | 31 +++
> lib/x86/setup.c | 246 +++++++++++++++++
> lib/x86/usermode.c | 3 +-
> lib/x86/vm.c | 18 +-
> x86/Makefile.common | 68 +++--
> x86/Makefile.i386 | 5 +-
> x86/Makefile.x86_64 | 58 ++--
> x86/access.c | 9 +-
> x86/amd_sev.c | 94 +++++++
> x86/cet.c | 8 +-
> x86/efi/README.md | 63 +++++
> x86/efi/crt0-efi-x86_64.S | 79 ++++++
> x86/efi/efistart64.S | 143 ++++++++++
> x86/efi/elf_x86_64_efi.lds | 81 ++++++
> x86/efi/reloc_x86_64.c | 97 +++++++
> x86/efi/run | 63 +++++
> x86/emulator.c | 5 +-
> x86/eventinj.c | 6 +-
> x86/run | 16 +-
> x86/smap.c | 8 +-
> x86/umip.c | 10 +-
> 33 files changed, 2110 insertions(+), 74 deletions(-)
> create mode 100644 lib/efi.c
> create mode 100644 lib/efi.h
> create mode 100644 lib/linux/uefi.h
> create mode 100644 lib/x86/amd_sev.c
> create mode 100644 lib/x86/amd_sev.h
> create mode 100644 lib/x86/asm/setup.h
> create mode 100644 x86/amd_sev.c
> create mode 100644 x86/efi/README.md
> create mode 100644 x86/efi/crt0-efi-x86_64.S
> create mode 100644 x86/efi/efistart64.S
> create mode 100644 x86/efi/elf_x86_64_efi.lds
> create mode 100644 x86/efi/reloc_x86_64.c
> create mode 100755 x86/efi/run
>
> --
> 2.33.0.259.gc128427fd7-goog
>
Hi Zixuan,
If you still intend to work on this series, please send a new posting from
your personal mail address to avoid mail bounces on reviews.
Thanks,
drew
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support
2021-10-04 13:27 ` [kvm-unit-tests PATCH v2 00/17] x86_64 UEFI and AMD SEV/SEV-ES support Andrew Jones
@ 2021-10-04 20:54 ` Zixuan Wang
0 siblings, 0 replies; 47+ messages in thread
From: Zixuan Wang @ 2021-10-04 20:54 UTC (permalink / raw)
To: Andrew Jones
Cc: kvm, Paolo Bonzini, Marc Orr, Hyunwook (Wooky) Baek, Tom Roeder,
erdemaktas, rientjes, seanjc, brijesh.singh, Thomas.Lendacky,
varad.gautam, jroedel, bp
On Mon, Oct 4, 2021 at 6:27 AM Andrew Jones <drjones@redhat.com> wrote:
>
> On Fri, Aug 27, 2021 at 03:12:05AM +0000, Zixuan Wang wrote:
> > create mode 100644 x86/efi/reloc_x86_64.c
> > create mode 100755 x86/efi/run
> >
> > --
> > 2.33.0.259.gc128427fd7-goog
> >
>
> Hi Zixuan,
>
> If you still intend to work on this series, please send a new posting from
> your personal mail address to avoid mail bounces on reviews.
>
> Thanks,
> drew
>
Hi Andrew,
I just sent out the V3 patchset [1] (from my personal email) that
applies the comments up to the last weekend. I will apply the latest
review comments in the next version.
[1] https://lore.kernel.org/kvm/20211004204931.1537823-1-zxwang42@gmail.com
Best regards,
Zixuan
^ permalink raw reply [flat|nested] 47+ messages in thread