Make the ACPI table checksum calculation function (in core.c) public so it can be reused in other parts of the ACPI code. Signed-off-by: Oliver Steffen --- hw/acpi/core.c | 5 ++++- include/hw/acpi/acpi.h | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/hw/acpi/core.c b/hw/acpi/core.c index d9979b0da9..6b65e587f2 100644 --- a/hw/acpi/core.c +++ b/hw/acpi/core.c @@ -83,7 +83,10 @@ bool acpi_builtin(void) return true; } -static int acpi_checksum(const uint8_t *data, int len) +/* Calculate the ACPI checksum value so that if used in the corresponding + * header field, the ACPI checksum verification will be successful. + */ +int acpi_checksum(const uint8_t *data, int len) { int sum, i; sum = 0; diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h index 4b8ee094c4..b036116dfb 100644 --- a/include/hw/acpi/acpi.h +++ b/include/hw/acpi/acpi.h @@ -203,4 +203,7 @@ struct AcpiSlicOem { }; int acpi_get_slic_oem(AcpiSlicOem *oem); +/* core.c */ +int acpi_checksum(const uint8_t *data, int len); + #endif /* QEMU_HW_ACPI_H */ -- 2.52.0 Make the BIOS linker optional in acpi_table_end() and calculate the ACPI table checksum directly if no linker is provided. This makes it possible to call for example acpi_build_madt() from outside the ACPI table builder context. Signed-off-by: Oliver Steffen --- hw/acpi/aml-build.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index dad4cfcc7d..ea1c415b21 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -22,6 +22,7 @@ #include "qemu/osdep.h" #include #include "hw/acpi/aml-build.h" +#include "hw/acpi/acpi.h" #include "qemu/bswap.h" #include "qemu/bitops.h" #include "system/numa.h" @@ -1741,6 +1742,7 @@ void acpi_table_end(BIOSLinker *linker, AcpiTable *desc) uint32_t table_len = desc->array->len - desc->table_offset; uint32_t table_len_le = cpu_to_le32(table_len); gchar *len_ptr = &desc->array->data[desc->table_offset + 4]; + uint8_t *table; /* patch "Length" field that has been reserved by acpi_table_begin() * to the actual length, i.e. accumulated table length from @@ -1748,8 +1750,14 @@ void acpi_table_end(BIOSLinker *linker, AcpiTable *desc) */ memcpy(len_ptr, &table_len_le, sizeof table_len_le); - bios_linker_loader_add_checksum(linker, ACPI_BUILD_TABLE_FILE, - desc->table_offset, table_len, desc->table_offset + checksum_offset); + if (linker != NULL) { + bios_linker_loader_add_checksum(linker, ACPI_BUILD_TABLE_FILE, + desc->table_offset, table_len, + desc->table_offset + checksum_offset); + } else { + table = (uint8_t *) &desc->array->data[desc->table_offset]; + table[checksum_offset] = acpi_checksum(table, table_len); + } } void *acpi_data_push(GArray *table_data, unsigned size) -- 2.52.0 Add a function called `acpi_build_madt_standalone()` that builds a MADT without the rest of the ACPI table structure. Signed-off-by: Oliver Steffen --- hw/i386/acpi-build.c | 9 +++++++++ hw/i386/acpi-build.h | 2 ++ 2 files changed, 11 insertions(+) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 9446a9f862..19c62362e3 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -2249,3 +2249,12 @@ void acpi_setup(void) */ acpi_build_tables_cleanup(&tables, false); } + +GArray *acpi_build_madt_standalone(MachineState *machine) +{ + X86MachineState *x86ms = X86_MACHINE(machine); + GArray *table = g_array_new(false, true, 1); + acpi_build_madt(table, NULL, x86ms, x86ms->oem_id, + x86ms->oem_table_id); + return table; +} diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h index 8ba3c33e48..00e19bbe5e 100644 --- a/hw/i386/acpi-build.h +++ b/hw/i386/acpi-build.h @@ -8,4 +8,6 @@ extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; void acpi_setup(void); Object *acpi_get_i386_pci_host(void); +GArray *acpi_build_madt_standalone(MachineState *machine); + #endif -- 2.52.0 Move QIgvm and QIgvmParameter struct definitions from the source file into an IGVM internal header file to allow implementing architecture specific IGVM code in other places, for example target/i386/igvm.c. Signed-off-by: Oliver Steffen --- backends/igvm.c | 37 ------------------------------- include/system/igvm-internal.h | 40 ++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 37 deletions(-) diff --git a/backends/igvm.c b/backends/igvm.c index 4cf7b57234..3a3832dc2d 100644 --- a/backends/igvm.c +++ b/backends/igvm.c @@ -25,12 +25,6 @@ #include #include -typedef struct QIgvmParameterData { - QTAILQ_ENTRY(QIgvmParameterData) next; - uint8_t *data; - uint32_t size; - uint32_t index; -} QIgvmParameterData; /* * Some directives are specific to particular confidential computing platforms. @@ -66,37 +60,6 @@ struct QEMU_PACKED sev_id_authentication { #define IGVM_SEV_ID_BLOCK_VERSION 1 -/* - * QIgvm contains the information required during processing - * of a single IGVM file. - */ -typedef struct QIgvm { - IgvmHandle file; - ConfidentialGuestSupport *cgs; - ConfidentialGuestSupportClass *cgsc; - uint32_t compatibility_mask; - unsigned current_header_index; - QTAILQ_HEAD(, QIgvmParameterData) parameter_data; - IgvmPlatformType platform_type; - - /* - * SEV-SNP platforms can contain an ID block and authentication - * that should be verified by the guest. - */ - struct sev_id_block *id_block; - struct sev_id_authentication *id_auth; - - /* Define the guest policy for SEV guests */ - uint64_t sev_policy; - - /* These variables keep track of contiguous page regions */ - IGVM_VHS_PAGE_DATA region_prev_page_data; - uint64_t region_start; - unsigned region_start_index; - unsigned region_last_index; - unsigned region_page_count; -} QIgvm; - static int qigvm_directive_page_data(QIgvm *ctx, const uint8_t *header_data, Error **errp); static int qigvm_directive_vp_context(QIgvm *ctx, const uint8_t *header_data, diff --git a/include/system/igvm-internal.h b/include/system/igvm-internal.h index 171cec8d0f..9c8e887d6f 100644 --- a/include/system/igvm-internal.h +++ b/include/system/igvm-internal.h @@ -9,10 +9,12 @@ #ifndef QEMU_IGVM_INTERNAL_H #define QEMU_IGVM_INTERNAL_H +#include "qemu/queue.h" #include "qemu/typedefs.h" #include "qom/object.h" #include "hw/core/resettable.h" +#include "system/confidential-guest-support.h" #include struct IgvmCfg { @@ -28,6 +30,44 @@ struct IgvmCfg { ResettableState reset_state; }; +typedef struct QIgvmParameterData { + QTAILQ_ENTRY(QIgvmParameterData) next; + uint8_t *data; + uint32_t size; + uint32_t index; +} QIgvmParameterData; + +/* + * QIgvm contains the information required during processing of a single IGVM + * file. + */ +typedef struct QIgvm { + IgvmHandle file; + ConfidentialGuestSupport *cgs; + ConfidentialGuestSupportClass *cgsc; + uint32_t compatibility_mask; + unsigned current_header_index; + QTAILQ_HEAD(, QIgvmParameterData) parameter_data; + IgvmPlatformType platform_type; + + /* + * SEV-SNP platforms can contain an ID block and authentication + * that should be verified by the guest. + */ + struct sev_id_block *id_block; + struct sev_id_authentication *id_auth; + + /* Define the guest policy for SEV guests */ + uint64_t sev_policy; + + /* These variables keep track of contiguous page regions */ + IGVM_VHS_PAGE_DATA region_prev_page_data; + uint64_t region_start; + unsigned region_start_index; + unsigned region_last_index; + unsigned region_page_count; +} QIgvm; + IgvmHandle qigvm_file_init(char *filename, Error **errp); #endif -- 2.52.0 Move repeating code for finding the parameter entries in the IGVM backend out of the parameter handlers into a common function. A warning message is emitted in case a no parameter entry can be found for a given index. Reviewed-by: Luigi Leonardi Signed-off-by: Oliver Steffen --- backends/igvm.c | 143 ++++++++++++++++++--------------- include/system/igvm-internal.h | 3 + 2 files changed, 80 insertions(+), 66 deletions(-) diff --git a/backends/igvm.c b/backends/igvm.c index 3a3832dc2d..ea3f9d6b00 100644 --- a/backends/igvm.c +++ b/backends/igvm.c @@ -12,6 +12,7 @@ #include "qemu/osdep.h" #include "qapi/error.h" +#include "qemu/error-report.h" #include "qemu/target-info-qapi.h" #include "system/igvm.h" #include "system/igvm-cfg.h" @@ -60,6 +61,20 @@ struct QEMU_PACKED sev_id_authentication { #define IGVM_SEV_ID_BLOCK_VERSION 1 +QIgvmParameterData* +qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index) +{ + QIgvmParameterData *param_entry; + QTAILQ_FOREACH(param_entry, &igvm->parameter_data, next) + { + if (param_entry->index == parameter_area_index) { + return param_entry; + } + } + warn_report("IGVM: No parameter area for index %u", parameter_area_index); + return NULL; +} + static int qigvm_directive_page_data(QIgvm *ctx, const uint8_t *header_data, Error **errp); static int qigvm_directive_vp_context(QIgvm *ctx, const uint8_t *header_data, @@ -534,58 +549,54 @@ static int qigvm_directive_memory_map(QIgvm *ctx, const uint8_t *header_data, } /* Find the parameter area that should hold the memory map */ - QTAILQ_FOREACH(param_entry, &ctx->parameter_data, next) - { - if (param_entry->index == param->parameter_area_index) { - max_entry_count = - param_entry->size / sizeof(IGVM_VHS_MEMORY_MAP_ENTRY); - mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)param_entry->data; - - retval = get_mem_map_entry(entry, &cgmm_entry, errp); - while (retval == 0) { - if (entry >= max_entry_count) { - error_setg( - errp, - "IGVM: guest memory map size exceeds parameter area defined in IGVM file"); - return -1; - } - mm_entry[entry].starting_gpa_page_number = cgmm_entry.gpa >> 12; - mm_entry[entry].number_of_pages = cgmm_entry.size >> 12; - - switch (cgmm_entry.type) { - case CGS_MEM_RAM: - mm_entry[entry].entry_type = - IGVM_MEMORY_MAP_ENTRY_TYPE_MEMORY; - break; - case CGS_MEM_RESERVED: - mm_entry[entry].entry_type = - IGVM_MEMORY_MAP_ENTRY_TYPE_PLATFORM_RESERVED; - break; - case CGS_MEM_ACPI: - mm_entry[entry].entry_type = - IGVM_MEMORY_MAP_ENTRY_TYPE_PLATFORM_RESERVED; - break; - case CGS_MEM_NVS: - mm_entry[entry].entry_type = - IGVM_MEMORY_MAP_ENTRY_TYPE_PERSISTENT; - break; - case CGS_MEM_UNUSABLE: - mm_entry[entry].entry_type = - IGVM_MEMORY_MAP_ENTRY_TYPE_PLATFORM_RESERVED; - break; - } - retval = get_mem_map_entry(++entry, &cgmm_entry, errp); - } - if (retval < 0) { - return retval; - } - /* The entries need to be sorted */ - qsort(mm_entry, entry, sizeof(IGVM_VHS_MEMORY_MAP_ENTRY), - qigvm_cmp_mm_entry); + param_entry = qigvm_find_param_entry(ctx, param->parameter_area_index); + if (param_entry == NULL) { + return 0; + } + + max_entry_count = param_entry->size / sizeof(IGVM_VHS_MEMORY_MAP_ENTRY); + mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)param_entry->data; + + retval = get_mem_map_entry(entry, &cgmm_entry, errp); + while (retval == 0) { + if (entry >= max_entry_count) { + error_setg( + errp, + "IGVM: guest memory map size exceeds parameter area defined " + "in IGVM file"); + return -1; + } + mm_entry[entry].starting_gpa_page_number = cgmm_entry.gpa >> 12; + mm_entry[entry].number_of_pages = cgmm_entry.size >> 12; + switch (cgmm_entry.type) { + case CGS_MEM_RAM: + mm_entry[entry].entry_type = IGVM_MEMORY_MAP_ENTRY_TYPE_MEMORY; + break; + case CGS_MEM_RESERVED: + mm_entry[entry].entry_type = + IGVM_MEMORY_MAP_ENTRY_TYPE_PLATFORM_RESERVED; + break; + case CGS_MEM_ACPI: + mm_entry[entry].entry_type = + IGVM_MEMORY_MAP_ENTRY_TYPE_PLATFORM_RESERVED; + break; + case CGS_MEM_NVS: + mm_entry[entry].entry_type = IGVM_MEMORY_MAP_ENTRY_TYPE_PERSISTENT; + break; + case CGS_MEM_UNUSABLE: + mm_entry[entry].entry_type = + IGVM_MEMORY_MAP_ENTRY_TYPE_PLATFORM_RESERVED; break; } + retval = get_mem_map_entry(++entry, &cgmm_entry, errp); + } + if (retval < 0) { + return retval; } + /* The entries need to be sorted */ + qsort(mm_entry, entry, sizeof(IGVM_VHS_MEMORY_MAP_ENTRY), + qigvm_cmp_mm_entry); return 0; } @@ -597,18 +608,18 @@ static int qigvm_directive_vp_count(QIgvm *ctx, const uint8_t *header_data, uint32_t *vp_count; CPUState *cpu; - QTAILQ_FOREACH(param_entry, &ctx->parameter_data, next) + param_entry = qigvm_find_param_entry(ctx, param->parameter_area_index); + if (param_entry == NULL) { + return 0; + } + + vp_count = (uint32_t *)(param_entry->data + param->byte_offset); + *vp_count = 0; + CPU_FOREACH(cpu) { - if (param_entry->index == param->parameter_area_index) { - vp_count = (uint32_t *)(param_entry->data + param->byte_offset); - *vp_count = 0; - CPU_FOREACH(cpu) - { - (*vp_count)++; - } - break; - } + (*vp_count)++; } + return 0; } @@ -620,15 +631,15 @@ static int qigvm_directive_environment_info(QIgvm *ctx, QIgvmParameterData *param_entry; IgvmEnvironmentInfo *environmental_state; - QTAILQ_FOREACH(param_entry, &ctx->parameter_data, next) - { - if (param_entry->index == param->parameter_area_index) { - environmental_state = - (IgvmEnvironmentInfo *)(param_entry->data + param->byte_offset); - environmental_state->memory_is_shared = 1; - break; - } + param_entry = qigvm_find_param_entry(ctx, param->parameter_area_index); + if (param_entry == NULL) { + return 0; } + + environmental_state = + (IgvmEnvironmentInfo *)(param_entry->data + param->byte_offset); + environmental_state->memory_is_shared = 1; + return 0; } diff --git a/include/system/igvm-internal.h b/include/system/igvm-internal.h index 9c8e887d6f..019f95e866 100644 --- a/include/system/igvm-internal.h +++ b/include/system/igvm-internal.h @@ -70,4 +70,7 @@ typedef struct QIgvm { IgvmHandle qigvm_file_init(char *filename, Error **errp); +QIgvmParameterData* +qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index); + #endif -- 2.52.0 Use qigvm_find_param_entry() also in qigvm_parameter_insert(). This changes behavior: Processing now stops after the first parameter entry found. That is OK, because we expect only one matching entry anyway. Reviewed-by: Luigi Leonardi Signed-off-by: Oliver Steffen --- backends/igvm.c | 50 ++++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/backends/igvm.c b/backends/igvm.c index ea3f9d6b00..ffd1c325b6 100644 --- a/backends/igvm.c +++ b/backends/igvm.c @@ -476,31 +476,31 @@ static int qigvm_directive_parameter_insert(QIgvm *ctx, return 0; } - QTAILQ_FOREACH(param_entry, &ctx->parameter_data, next) - { - if (param_entry->index == param->parameter_area_index) { - region = qigvm_prepare_memory(ctx, param->gpa, param_entry->size, - ctx->current_header_index, errp); - if (!region) { - return -1; - } - memcpy(region, param_entry->data, param_entry->size); - g_free(param_entry->data); - param_entry->data = NULL; - - /* - * If a confidential guest support object is provided then use it to - * set the guest state. - */ - if (ctx->cgs) { - result = ctx->cgsc->set_guest_state(param->gpa, region, - param_entry->size, - CGS_PAGE_TYPE_UNMEASURED, 0, - errp); - if (result < 0) { - return -1; - } - } + param_entry = qigvm_find_param_entry(ctx, param->parameter_area_index); + if (param_entry == NULL) { + return 0; + } + + region = qigvm_prepare_memory(ctx, param->gpa, param_entry->size, + ctx->current_header_index, errp); + if (!region) { + return -1; + } + memcpy(region, param_entry->data, param_entry->size); + g_free(param_entry->data); + param_entry->data = NULL; + + /* + * If a confidential guest support object is provided then use it to + * set the guest state. + */ + if (ctx->cgs) { + result = ctx->cgsc->set_guest_state(param->gpa, region, + param_entry->size, + CGS_PAGE_TYPE_UNMEASURED, 0, + errp); + if (result < 0) { + return -1; } } return 0; -- 2.52.0 Pass the full MachineState to the IGVM backend during file processing, instead of just the ConfidentialGuestSupport struct (which is a member of the MachineState). This replaces the cgs parameter of qigvm_process_file() with the machine state to make it available in the IGVM processing context. We will use it later to generate MADT data there to pass to the guest as IGVM parameter. Reviewed-by: Luigi Leonardi Signed-off-by: Oliver Steffen --- backends/igvm-cfg.c | 2 +- backends/igvm.c | 28 ++++++++++++++++------------ include/system/igvm-cfg.h | 3 ++- include/system/igvm-internal.h | 3 ++- include/system/igvm.h | 5 +++-- target/i386/sev.c | 3 +-- 6 files changed, 25 insertions(+), 19 deletions(-) diff --git a/backends/igvm-cfg.c b/backends/igvm-cfg.c index f236b523df..64589ca34f 100644 --- a/backends/igvm-cfg.c +++ b/backends/igvm-cfg.c @@ -52,7 +52,7 @@ static void igvm_reset_hold(Object *obj, ResetType type) trace_igvm_reset_hold(type); - qigvm_process_file(igvm, ms->cgs, false, &error_fatal); + qigvm_process_file(igvm, ms, false, &error_fatal); } static void igvm_reset_exit(Object *obj, ResetType type) diff --git a/backends/igvm.c b/backends/igvm.c index ffd1c325b6..3e7c0ea41d 100644 --- a/backends/igvm.c +++ b/backends/igvm.c @@ -202,7 +202,8 @@ static void *qigvm_prepare_memory(QIgvm *ctx, uint64_t addr, uint64_t size, g_autofree char *region_name = g_strdup_printf("igvm.%X", region_identifier); igvm_pages = g_new0(MemoryRegion, 1); - if (ctx->cgs && ctx->cgs->require_guest_memfd) { + if (ctx->machine_state->cgs && + ctx->machine_state->cgs->require_guest_memfd) { if (!memory_region_init_ram_guest_memfd(igvm_pages, NULL, region_name, size, errp)) { return NULL; @@ -322,7 +323,7 @@ static int qigvm_process_mem_region(QIgvm *ctx, unsigned start_index, * If a confidential guest support object is provided then use it to set the * guest state. */ - if (ctx->cgs) { + if (ctx->machine_state->cgs) { cgs_page_type = qigvm_type_to_cgs_type(page_type, flags->unmeasured, zero); if (cgs_page_type < 0) { @@ -424,7 +425,7 @@ static int qigvm_directive_vp_context(QIgvm *ctx, const uint8_t *header_data, data = (uint8_t *)igvm_get_buffer(ctx->file, data_handle); - if (ctx->cgs) { + if (ctx->machine_state->cgs) { result = ctx->cgsc->set_guest_state( vp_context->gpa, data, igvm_get_buffer_size(ctx->file, data_handle), CGS_PAGE_TYPE_VMSA, vp_context->vp_index, errp); @@ -494,7 +495,7 @@ static int qigvm_directive_parameter_insert(QIgvm *ctx, * If a confidential guest support object is provided then use it to * set the guest state. */ - if (ctx->cgs) { + if (ctx->machine_state->cgs) { result = ctx->cgsc->set_guest_state(param->gpa, region, param_entry->size, CGS_PAGE_TYPE_UNMEASURED, 0, @@ -535,7 +536,7 @@ static int qigvm_directive_memory_map(QIgvm *ctx, const uint8_t *header_data, ConfidentialGuestMemoryMapEntry cgmm_entry; int retval = 0; - if (ctx->cgs && ctx->cgsc->get_mem_map_entry) { + if (ctx->machine_state->cgs && ctx->cgsc->get_mem_map_entry) { get_mem_map_entry = ctx->cgsc->get_mem_map_entry; } else if (target_arch() == SYS_EMU_TARGET_X86_64) { @@ -661,7 +662,7 @@ static int qigvm_directive_required_memory(QIgvm *ctx, if (!region) { return -1; } - if (ctx->cgs) { + if (ctx->machine_state->cgs) { result = ctx->cgsc->set_guest_state(mem->gpa, region, mem->number_of_bytes, CGS_PAGE_TYPE_REQUIRED_MEMORY, 0, errp); @@ -779,14 +780,14 @@ static int qigvm_supported_platform_compat_mask(QIgvm *ctx, Error **errp) sizeof( IGVM_VHS_VARIABLE_HEADER)); if ((platform->platform_type == IGVM_PLATFORM_TYPE_SEV_ES) && - ctx->cgs) { + ctx->machine_state->cgs) { if (ctx->cgsc->check_support( CGS_PLATFORM_SEV_ES, platform->platform_version, platform->highest_vtl, platform->shared_gpa_boundary)) { compatibility_mask_sev_es = platform->compatibility_mask; } } else if ((platform->platform_type == IGVM_PLATFORM_TYPE_SEV) && - ctx->cgs) { + ctx->machine_state->cgs) { if (ctx->cgsc->check_support( CGS_PLATFORM_SEV, platform->platform_version, platform->highest_vtl, platform->shared_gpa_boundary)) { @@ -794,7 +795,7 @@ static int qigvm_supported_platform_compat_mask(QIgvm *ctx, Error **errp) } } else if ((platform->platform_type == IGVM_PLATFORM_TYPE_SEV_SNP) && - ctx->cgs) { + ctx->machine_state->cgs) { if (ctx->cgsc->check_support( CGS_PLATFORM_SEV_SNP, platform->platform_version, platform->highest_vtl, platform->shared_gpa_boundary)) { @@ -867,7 +868,7 @@ IgvmHandle qigvm_file_init(char *filename, Error **errp) return igvm; } -int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, +int qigvm_process_file(IgvmCfg *cfg, MachineState *machine_state, bool onlyVpContext, Error **errp) { int32_t header_count; @@ -883,13 +884,16 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, ctx.file = cfg->file; trace_igvm_process_file(cfg->file, onlyVpContext); + ctx.machine_state = machine_state; + /* * The ConfidentialGuestSupport object is optional and allows a confidential * guest platform to perform extra processing, such as page measurement, on * IGVM directives. */ - ctx.cgs = cgs; - ctx.cgsc = cgs ? CONFIDENTIAL_GUEST_SUPPORT_GET_CLASS(cgs) : NULL; + ctx.cgsc = machine_state->cgs ? + CONFIDENTIAL_GUEST_SUPPORT_GET_CLASS(machine_state->cgs) : + NULL; /* * Check that the IGVM file provides configuration for the current diff --git a/include/system/igvm-cfg.h b/include/system/igvm-cfg.h index 6c07f30840..e06d611f74 100644 --- a/include/system/igvm-cfg.h +++ b/include/system/igvm-cfg.h @@ -12,6 +12,7 @@ #ifndef QEMU_IGVM_CFG_H #define QEMU_IGVM_CFG_H +#include "hw/core/boards.h" #include "qemu/typedefs.h" #include "qom/object.h" @@ -27,7 +28,7 @@ typedef struct IgvmCfgClass { * * Returns 0 for ok and -1 on error. */ - int (*process)(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, + int (*process)(IgvmCfg *cfg, MachineState *machine_state, bool onlyVpContext, Error **errp); } IgvmCfgClass; diff --git a/include/system/igvm-internal.h b/include/system/igvm-internal.h index 019f95e866..1d36519ab0 100644 --- a/include/system/igvm-internal.h +++ b/include/system/igvm-internal.h @@ -12,6 +12,7 @@ #include "qemu/queue.h" #include "qemu/typedefs.h" #include "qom/object.h" +#include "hw/core/boards.h" #include "hw/core/resettable.h" #include "system/confidential-guest-support.h" @@ -43,7 +44,7 @@ typedef struct QIgvmParameterData { */ typedef struct QIgvm { IgvmHandle file; - ConfidentialGuestSupport *cgs; + MachineState *machine_state; ConfidentialGuestSupportClass *cgsc; uint32_t compatibility_mask; unsigned current_header_index; diff --git a/include/system/igvm.h b/include/system/igvm.h index 8355e54e95..5573a6111a 100644 --- a/include/system/igvm.h +++ b/include/system/igvm.h @@ -12,12 +12,13 @@ #ifndef BACKENDS_IGVM_H #define BACKENDS_IGVM_H +#include "hw/core/boards.h" #include "qemu/typedefs.h" #include "system/confidential-guest-support.h" #include "qapi/error.h" -int qigvm_process_file(IgvmCfg *igvm, ConfidentialGuestSupport *cgs, - bool onlyVpContext, Error **errp); +int qigvm_process_file(IgvmCfg *igvm, MachineState *machine_state, + bool onlyVpContext, Error **errp); /* x86 native */ int qigvm_x86_get_mem_map_entry(int index, diff --git a/target/i386/sev.c b/target/i386/sev.c index 1d70f96ec1..6f86dd710b 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -1891,8 +1891,7 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp) */ if (x86machine->igvm) { if (IGVM_CFG_GET_CLASS(x86machine->igvm) - ->process(x86machine->igvm, machine->cgs, true, errp) == - -1) { + ->process(x86machine->igvm, machine, true, errp) == -1) { return -1; } /* -- 2.52.0 Change meson script to only include the IGVM stubs file if the IGVM feature is enabled. It is used to handle architecture specific differences within the IGVM backend, not to provide stubs of the backend itself. Signed-off-by: Oliver Steffen --- stubs/meson.build | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/stubs/meson.build b/stubs/meson.build index 2b5fd8a88a..8a07059500 100644 --- a/stubs/meson.build +++ b/stubs/meson.build @@ -71,7 +71,9 @@ if have_system stub_ss.add(files('dump.c')) stub_ss.add(files('cmos.c')) stub_ss.add(files('fw_cfg.c')) - stub_ss.add(files('igvm.c')) + if igvm.found() + stub_ss.add(files('igvm.c')) + endif stub_ss.add(files('target-get-monitor-def.c')) stub_ss.add(files('target-monitor-defs.c')) stub_ss.add(files('win32-kbd-hook.c')) -- 2.52.0 Use the new acpi_build_madt_standalone() function to fill the MADT parameter field. The IGVM parameter can be consumed by Coconut SVSM [1], instead of relying on the fw_cfg interface, which has caused problems before due to unexpected access [2,3]. Using IGVM parameters is the default way for Coconut SVSM across hypervisors; switching over would allow removing specialized code paths for QEMU in Coconut. Coconut SVSM needs to know the SMP configuration, but does not look at any other ACPI data, nor does it interact with the PCI bus settings. Since the MADT is static and not linked with other ACPI tables, it can be supplied stand-alone like this. Generating the MADT twice (during ACPI table building and IGVM processing) seems acceptable, since there is no infrastructure to obtain the MADT out of the ACPI table memory area. In any case OVMF, which runs after SVSM has already been initialized, will continue reading all ACPI tables via fw_cfg and provide fixed up ACPI data to the OS as before without any changes. The IGVM parameter handler is implemented for the i386 machine target and stubbed for all others. [1] https://github.com/coconut-svsm/svsm/pull/858 [2] https://gitlab.com/qemu-project/qemu/-/issues/2882 [3] https://github.com/coconut-svsm/svsm/issues/646 Signed-off-by: Oliver Steffen --- backends/igvm.c | 2 ++ include/system/igvm-internal.h | 5 +++++ stubs/igvm.c | 6 ++++++ target/i386/igvm.c | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+) diff --git a/backends/igvm.c b/backends/igvm.c index 3e7c0ea41d..b01a19ba46 100644 --- a/backends/igvm.c +++ b/backends/igvm.c @@ -128,6 +128,8 @@ static struct QIGVMHandler handlers[] = { qigvm_directive_snp_id_block }, { IGVM_VHT_GUEST_POLICY, IGVM_HEADER_SECTION_INITIALIZATION, qigvm_initialization_guest_policy }, + { IGVM_VHT_MADT, IGVM_HEADER_SECTION_DIRECTIVE, + qigvm_directive_madt }, }; static int qigvm_handler(QIgvm *ctx, uint32_t type, Error **errp) diff --git a/include/system/igvm-internal.h b/include/system/igvm-internal.h index 1d36519ab0..38004bd908 100644 --- a/include/system/igvm-internal.h +++ b/include/system/igvm-internal.h @@ -74,4 +74,9 @@ IgvmHandle qigvm_file_init(char *filename, Error **errp); QIgvmParameterData* qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index); +/* + * IGVM parameter handlers + */ +int qigvm_directive_madt(QIgvm *ctx, const uint8_t *header_data, Error **errp); + #endif diff --git a/stubs/igvm.c b/stubs/igvm.c index 17cd1e903e..47d5130d9d 100644 --- a/stubs/igvm.c +++ b/stubs/igvm.c @@ -12,6 +12,7 @@ #include "qemu/osdep.h" #include "system/igvm.h" +#include "system/igvm-internal.h" int qigvm_x86_get_mem_map_entry(int index, ConfidentialGuestMemoryMapEntry *entry, @@ -24,3 +25,8 @@ int qigvm_x86_set_vp_context(void *data, int index, Error **errp) { return -1; } + +int qigvm_directive_madt(QIgvm *ctx, const uint8_t *header_data, Error **errp) +{ + return -1; +} diff --git a/target/i386/igvm.c b/target/i386/igvm.c index 457c253b03..f41b498b89 100644 --- a/target/i386/igvm.c +++ b/target/i386/igvm.c @@ -13,7 +13,9 @@ #include "cpu.h" #include "hw/i386/e820_memory_layout.h" +#include "hw/i386/acpi-build.h" #include "system/igvm.h" +#include "system/igvm-internal.h" struct IgvmNativeVpContextX64 { uint64_t rax; @@ -178,3 +180,33 @@ void qigvm_x86_bsp_reset(CPUX86State *env) qigvm_x86_load_context(bsp_context, env); } + +/* + * Process MADT IGVM parameter + */ +int qigvm_directive_madt(QIgvm *ctx, const uint8_t *header_data, Error **errp) +{ + const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data; + QIgvmParameterData *param_entry; + int result = 0; + + /* Find the parameter area that should hold the MADT data */ + param_entry = qigvm_find_param_entry(ctx, param->parameter_area_index); + if (param_entry == NULL) { + return 0; + } + + GArray *madt = acpi_build_madt_standalone(ctx->machine_state); + + if (madt->len <= param_entry->size) { + memcpy(param_entry->data, madt->data, madt->len); + } else { + error_setg( + errp, + "IGVM: MADT size exceeds parameter area defined in IGVM file"); + result = -1; + } + + g_array_free(madt, true); + return result; +} -- 2.52.0