* [PATCH 0/3] igvm: Supply MADT via IGVM parameter
@ 2025-12-11 8:15 Oliver Steffen
2025-12-11 8:15 ` [PATCH 1/3] hw/acpi: Make BIOS linker optional Oliver Steffen
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Oliver Steffen @ 2025-12-11 8:15 UTC (permalink / raw)
To: qemu-devel
Cc: Joerg Roedel, Marcel Apfelbaum, Gerd Hoffmann, Paolo Bonzini,
Stefano Garzarella, Marcelo Tosatti, kvm, Richard Henderson,
Zhao Liu, Eduardo Habkost, Ani Sinha, Michael S. Tsirkin,
Luigi Leonardi, Igor Mammedov, Oliver Steffen
When launching using an IGVM file, supply a copy of the MADT (part of the ACPI
tables) via an IGVM parameter (IGVM_VHY_MADT) to the guest, in addition to the
regular fw_cfg mechanism.
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; switching
over would allow removing specialized code paths for QEMU in Coconut.
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.
This series makes ACPI table building more generic by making the BIOS linker
optional. This allows the MADT to be generated outside of the ACPI build
context. A new function (acpi_build_madt_standalone()) is added for that. With
that, the IGVM MADT parameter field can be filled with the MADT data during
processing of the IGVM file.
Generating the MADT twice (IGVM processing and ACPI table building) seems
acceptable, since there is no infrastructure to obtain the MADT out of the ACPI
table memory area during IGVM processing.
[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
Based-On: 20251118122133.1695767-1-kraxel@redhat.com
Signed-off-by: Oliver Steffen <osteffen@redhat.com>
Oliver Steffen (3):
hw/acpi: Make BIOS linker optional
hw/acpi: Add standalone function to build MADT
igvm: Fill MADT IGVM parameter field
backends/igvm-cfg.c | 8 +++++++-
backends/igvm.c | 37 ++++++++++++++++++++++++++++++++++++-
hw/acpi/aml-build.c | 7 +++++--
hw/i386/acpi-build.c | 8 ++++++++
hw/i386/acpi-build.h | 2 ++
include/system/igvm-cfg.h | 4 ++--
include/system/igvm.h | 2 +-
target/i386/sev.c | 2 +-
8 files changed, 62 insertions(+), 8 deletions(-)
--
2.52.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/3] hw/acpi: Make BIOS linker optional 2025-12-11 8:15 [PATCH 0/3] igvm: Supply MADT via IGVM parameter Oliver Steffen @ 2025-12-11 8:15 ` Oliver Steffen 2025-12-11 8:15 ` [PATCH 2/3] hw/acpi: Add standalone function to build MADT Oliver Steffen 2025-12-11 8:15 ` [PATCH 3/3] igvm: Fill MADT IGVM parameter field Oliver Steffen 2 siblings, 0 replies; 7+ messages in thread From: Oliver Steffen @ 2025-12-11 8:15 UTC (permalink / raw) To: qemu-devel Cc: Joerg Roedel, Marcel Apfelbaum, Gerd Hoffmann, Paolo Bonzini, Stefano Garzarella, Marcelo Tosatti, kvm, Richard Henderson, Zhao Liu, Eduardo Habkost, Ani Sinha, Michael S. Tsirkin, Luigi Leonardi, Igor Mammedov, Oliver Steffen Make the BIOS linker optional in acpi_table_end(). This makes it possible to call for example acpi_build_madt() from outside the ACPI table builder context. Signed-off-by: Oliver Steffen <osteffen@redhat.com> --- hw/acpi/aml-build.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index 2d5826a8f1..ed86867ae3 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -1748,8 +1748,11 @@ 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); + } } void *acpi_data_push(GArray *table_data, unsigned size) -- 2.52.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] hw/acpi: Add standalone function to build MADT 2025-12-11 8:15 [PATCH 0/3] igvm: Supply MADT via IGVM parameter Oliver Steffen 2025-12-11 8:15 ` [PATCH 1/3] hw/acpi: Make BIOS linker optional Oliver Steffen @ 2025-12-11 8:15 ` Oliver Steffen 2025-12-11 8:15 ` [PATCH 3/3] igvm: Fill MADT IGVM parameter field Oliver Steffen 2 siblings, 0 replies; 7+ messages in thread From: Oliver Steffen @ 2025-12-11 8:15 UTC (permalink / raw) To: qemu-devel Cc: Joerg Roedel, Marcel Apfelbaum, Gerd Hoffmann, Paolo Bonzini, Stefano Garzarella, Marcelo Tosatti, kvm, Richard Henderson, Zhao Liu, Eduardo Habkost, Ani Sinha, Michael S. Tsirkin, Luigi Leonardi, Igor Mammedov, Oliver Steffen Add a fuction called `acpi_build_madt_standalone()` that builds a MADT without the rest of the ACPI table structure. Signed-off-by: Oliver Steffen <osteffen@redhat.com> --- hw/i386/acpi-build.c | 8 ++++++++ hw/i386/acpi-build.h | 2 ++ 2 files changed, 10 insertions(+) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 9446a9f862..e472876567 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -2249,3 +2249,11 @@ 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] igvm: Fill MADT IGVM parameter field 2025-12-11 8:15 [PATCH 0/3] igvm: Supply MADT via IGVM parameter Oliver Steffen 2025-12-11 8:15 ` [PATCH 1/3] hw/acpi: Make BIOS linker optional Oliver Steffen 2025-12-11 8:15 ` [PATCH 2/3] hw/acpi: Add standalone function to build MADT Oliver Steffen @ 2025-12-11 8:15 ` Oliver Steffen 2025-12-11 8:46 ` Stefano Garzarella 2 siblings, 1 reply; 7+ messages in thread From: Oliver Steffen @ 2025-12-11 8:15 UTC (permalink / raw) To: qemu-devel Cc: Joerg Roedel, Marcel Apfelbaum, Gerd Hoffmann, Paolo Bonzini, Stefano Garzarella, Marcelo Tosatti, kvm, Richard Henderson, Zhao Liu, Eduardo Habkost, Ani Sinha, Michael S. Tsirkin, Luigi Leonardi, Igor Mammedov, Oliver Steffen Use the new acpi_build_madt_standalone() function to fill the MADT parameter field. Signed-off-by: Oliver Steffen <osteffen@redhat.com> --- backends/igvm-cfg.c | 8 +++++++- backends/igvm.c | 37 ++++++++++++++++++++++++++++++++++++- include/system/igvm-cfg.h | 4 ++-- include/system/igvm.h | 2 +- target/i386/sev.c | 2 +- 5 files changed, 47 insertions(+), 6 deletions(-) diff --git a/backends/igvm-cfg.c b/backends/igvm-cfg.c index c1b45401f4..0a77f7b7a1 100644 --- a/backends/igvm-cfg.c +++ b/backends/igvm-cfg.c @@ -17,6 +17,7 @@ #include "qom/object_interfaces.h" #include "hw/qdev-core.h" #include "hw/boards.h" +#include "hw/i386/acpi-build.h" #include "trace.h" @@ -48,10 +49,15 @@ static void igvm_reset_hold(Object *obj, ResetType type) { MachineState *ms = MACHINE(qdev_get_machine()); IgvmCfg *igvm = IGVM_CFG(obj); + GArray *madt = NULL; trace_igvm_reset_hold(type); - qigvm_process_file(igvm, ms->cgs, false, &error_fatal); + madt = acpi_build_madt_standalone(ms); + + qigvm_process_file(igvm, ms->cgs, false, madt, &error_fatal); + + g_array_free(madt, true); } static void igvm_reset_exit(Object *obj, ResetType type) diff --git a/backends/igvm.c b/backends/igvm.c index a350c890cc..7e56b19b0a 100644 --- a/backends/igvm.c +++ b/backends/igvm.c @@ -93,6 +93,7 @@ typedef struct QIgvm { unsigned region_start_index; unsigned region_last_index; unsigned region_page_count; + GArray *madt; } QIgvm; static int qigvm_directive_page_data(QIgvm *ctx, const uint8_t *header_data, @@ -120,6 +121,8 @@ static int qigvm_directive_snp_id_block(QIgvm *ctx, const uint8_t *header_data, static int qigvm_initialization_guest_policy(QIgvm *ctx, const uint8_t *header_data, Error **errp); +static int qigvm_initialization_madt(QIgvm *ctx, + const uint8_t *header_data, Error **errp); struct QIGVMHandler { uint32_t type; @@ -148,6 +151,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_initialization_madt }, }; static int qigvm_handler(QIgvm *ctx, uint32_t type, Error **errp) @@ -764,6 +769,34 @@ static int qigvm_initialization_guest_policy(QIgvm *ctx, return 0; } +static int qigvm_initialization_madt(QIgvm *ctx, + const uint8_t *header_data, Error **errp) +{ + const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data; + QIgvmParameterData *param_entry; + + if (ctx->madt == NULL) { + return 0; + } + + /* Find the parameter area that should hold the device tree */ + QTAILQ_FOREACH(param_entry, &ctx->parameter_data, next) + { + if (param_entry->index == param->parameter_area_index) { + + if (ctx->madt->len > param_entry->size) { + error_setg( + errp, + "IGVM: MADT size exceeds parameter area defined in IGVM file"); + return -1; + } + memcpy(param_entry->data, ctx->madt->data, ctx->madt->len); + break; + } + } + return 0; +} + static int qigvm_supported_platform_compat_mask(QIgvm *ctx, Error **errp) { int32_t header_count; @@ -892,7 +925,7 @@ IgvmHandle qigvm_file_init(char *filename, Error **errp) } int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, - bool onlyVpContext, Error **errp) + bool onlyVpContext, GArray *madt, Error **errp) { int32_t header_count; QIgvmParameterData *parameter; @@ -915,6 +948,8 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, ctx.cgs = cgs; ctx.cgsc = cgs ? CONFIDENTIAL_GUEST_SUPPORT_GET_CLASS(cgs) : NULL; + ctx.madt = madt; + /* * Check that the IGVM file provides configuration for the current * platform diff --git a/include/system/igvm-cfg.h b/include/system/igvm-cfg.h index 7dc48677fd..1a04302beb 100644 --- a/include/system/igvm-cfg.h +++ b/include/system/igvm-cfg.h @@ -42,8 +42,8 @@ typedef struct IgvmCfgClass { * * Returns 0 for ok and -1 on error. */ - int (*process)(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, - bool onlyVpContext, Error **errp); + int (*process)(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, + bool onlyVpContext, GArray *madt, Error **errp); } IgvmCfgClass; diff --git a/include/system/igvm.h b/include/system/igvm.h index ec2538daa0..f2e580e4ee 100644 --- a/include/system/igvm.h +++ b/include/system/igvm.h @@ -18,7 +18,7 @@ IgvmHandle qigvm_file_init(char *filename, Error **errp); int qigvm_process_file(IgvmCfg *igvm, ConfidentialGuestSupport *cgs, - bool onlyVpContext, Error **errp); + bool onlyVpContext, GArray *madt, 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 fd2dada013..ffeb9f52a2 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -1892,7 +1892,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) == + ->process(x86machine->igvm, machine->cgs, true, NULL, errp) == -1) { return -1; } -- 2.52.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] igvm: Fill MADT IGVM parameter field 2025-12-11 8:15 ` [PATCH 3/3] igvm: Fill MADT IGVM parameter field Oliver Steffen @ 2025-12-11 8:46 ` Stefano Garzarella 2025-12-11 9:24 ` Oliver Steffen 0 siblings, 1 reply; 7+ messages in thread From: Stefano Garzarella @ 2025-12-11 8:46 UTC (permalink / raw) To: Oliver Steffen Cc: qemu-devel, Joerg Roedel, Marcel Apfelbaum, Gerd Hoffmann, Paolo Bonzini, Marcelo Tosatti, kvm, Richard Henderson, Zhao Liu, Eduardo Habkost, Ani Sinha, Michael S. Tsirkin, Luigi Leonardi, Igor Mammedov On Thu, Dec 11, 2025 at 09:15:17AM +0100, Oliver Steffen wrote: >Use the new acpi_build_madt_standalone() function to fill the MADT >parameter field. The cover letter will not usually be part of the git history, so IMO it is better to include also here the information that you have rightly written there, explaining why we are adding this change. > >Signed-off-by: Oliver Steffen <osteffen@redhat.com> >--- > backends/igvm-cfg.c | 8 +++++++- > backends/igvm.c | 37 ++++++++++++++++++++++++++++++++++++- > include/system/igvm-cfg.h | 4 ++-- > include/system/igvm.h | 2 +- > target/i386/sev.c | 2 +- > 5 files changed, 47 insertions(+), 6 deletions(-) > >diff --git a/backends/igvm-cfg.c b/backends/igvm-cfg.c >index c1b45401f4..0a77f7b7a1 100644 >--- a/backends/igvm-cfg.c >+++ b/backends/igvm-cfg.c >@@ -17,6 +17,7 @@ > #include "qom/object_interfaces.h" > #include "hw/qdev-core.h" > #include "hw/boards.h" >+#include "hw/i386/acpi-build.h" > > #include "trace.h" > >@@ -48,10 +49,15 @@ static void igvm_reset_hold(Object *obj, ResetType type) > { > MachineState *ms = MACHINE(qdev_get_machine()); > IgvmCfg *igvm = IGVM_CFG(obj); >+ GArray *madt = NULL; > > trace_igvm_reset_hold(type); > >- qigvm_process_file(igvm, ms->cgs, false, &error_fatal); >+ madt = acpi_build_madt_standalone(ms); >+ >+ qigvm_process_file(igvm, ms->cgs, false, madt, &error_fatal); >+ >+ g_array_free(madt, true); > } > > static void igvm_reset_exit(Object *obj, ResetType type) >diff --git a/backends/igvm.c b/backends/igvm.c >index a350c890cc..7e56b19b0a 100644 >--- a/backends/igvm.c >+++ b/backends/igvm.c >@@ -93,6 +93,7 @@ typedef struct QIgvm { > unsigned region_start_index; > unsigned region_last_index; > unsigned region_page_count; >+ GArray *madt; > } QIgvm; > > static int qigvm_directive_page_data(QIgvm *ctx, const uint8_t *header_data, >@@ -120,6 +121,8 @@ static int qigvm_directive_snp_id_block(QIgvm *ctx, const uint8_t *header_data, > static int qigvm_initialization_guest_policy(QIgvm *ctx, > const uint8_t *header_data, > Error **errp); >+static int qigvm_initialization_madt(QIgvm *ctx, >+ const uint8_t *header_data, Error **errp); > > struct QIGVMHandler { > uint32_t type; >@@ -148,6 +151,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_initialization_madt }, > }; > > static int qigvm_handler(QIgvm *ctx, uint32_t type, Error **errp) >@@ -764,6 +769,34 @@ static int qigvm_initialization_guest_policy(QIgvm *ctx, > return 0; > } > >+static int qigvm_initialization_madt(QIgvm *ctx, >+ const uint8_t *header_data, Error **errp) >+{ >+ const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data; >+ QIgvmParameterData *param_entry; >+ >+ if (ctx->madt == NULL) { >+ return 0; >+ } >+ >+ /* Find the parameter area that should hold the device tree */ >+ QTAILQ_FOREACH(param_entry, &ctx->parameter_data, next) >+ { >+ if (param_entry->index == param->parameter_area_index) { >+ >+ if (ctx->madt->len > param_entry->size) { >+ error_setg( >+ errp, >+ "IGVM: MADT size exceeds parameter area defined in IGVM file"); >+ return -1; >+ } >+ memcpy(param_entry->data, ctx->madt->data, ctx->madt->len); >+ break; >+ } >+ } >+ return 0; >+} >+ > static int qigvm_supported_platform_compat_mask(QIgvm *ctx, Error **errp) > { > int32_t header_count; >@@ -892,7 +925,7 @@ IgvmHandle qigvm_file_init(char *filename, Error **errp) > } > > int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, >- bool onlyVpContext, Error **errp) >+ bool onlyVpContext, GArray *madt, Error **errp) > { > int32_t header_count; > QIgvmParameterData *parameter; >@@ -915,6 +948,8 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, > ctx.cgs = cgs; > ctx.cgsc = cgs ? CONFIDENTIAL_GUEST_SUPPORT_GET_CLASS(cgs) : NULL; > >+ ctx.madt = madt; >+ > /* > * Check that the IGVM file provides configuration for the current > * platform >diff --git a/include/system/igvm-cfg.h b/include/system/igvm-cfg.h >index 7dc48677fd..1a04302beb 100644 >--- a/include/system/igvm-cfg.h >+++ b/include/system/igvm-cfg.h >@@ -42,8 +42,8 @@ typedef struct IgvmCfgClass { > * > * Returns 0 for ok and -1 on error. > */ Should we update the documentation of this function now that we have a new parameter, also explaining that it's optional. >- int (*process)(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, >- bool onlyVpContext, Error **errp); >+ int (*process)(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, >+ bool onlyVpContext, GArray *madt, Error **errp); > > } IgvmCfgClass; > >diff --git a/include/system/igvm.h b/include/system/igvm.h >index ec2538daa0..f2e580e4ee 100644 >--- a/include/system/igvm.h >+++ b/include/system/igvm.h >@@ -18,7 +18,7 @@ > > IgvmHandle qigvm_file_init(char *filename, Error **errp); > int qigvm_process_file(IgvmCfg *igvm, ConfidentialGuestSupport *cgs, >- bool onlyVpContext, Error **errp); >+ bool onlyVpContext, GArray *madt, 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 fd2dada013..ffeb9f52a2 100644 >--- a/target/i386/sev.c >+++ b/target/i386/sev.c >@@ -1892,7 +1892,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) == >+ ->process(x86machine->igvm, machine->cgs, true, NULL, errp) == Why here we don't need to pass it? Thanks, Stefano > -1) { > return -1; > } >-- >2.52.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] igvm: Fill MADT IGVM parameter field 2025-12-11 8:46 ` Stefano Garzarella @ 2025-12-11 9:24 ` Oliver Steffen 2025-12-11 10:11 ` Stefano Garzarella 0 siblings, 1 reply; 7+ messages in thread From: Oliver Steffen @ 2025-12-11 9:24 UTC (permalink / raw) To: Stefano Garzarella Cc: qemu-devel, Joerg Roedel, Marcel Apfelbaum, Gerd Hoffmann, Paolo Bonzini, Marcelo Tosatti, kvm, Richard Henderson, Zhao Liu, Eduardo Habkost, Ani Sinha, Michael S. Tsirkin, Luigi Leonardi, Igor Mammedov On Thu, Dec 11, 2025 at 9:46 AM Stefano Garzarella <sgarzare@redhat.com> wrote: > > On Thu, Dec 11, 2025 at 09:15:17AM +0100, Oliver Steffen wrote: > >Use the new acpi_build_madt_standalone() function to fill the MADT > >parameter field. > > The cover letter will not usually be part of the git history, so IMO it > is better to include also here the information that you have rightly > written there, explaining why we are adding this change. Will do. > > > >Signed-off-by: Oliver Steffen <osteffen@redhat.com> > >--- > > backends/igvm-cfg.c | 8 +++++++- > > backends/igvm.c | 37 ++++++++++++++++++++++++++++++++++++- > > include/system/igvm-cfg.h | 4 ++-- > > include/system/igvm.h | 2 +- > > target/i386/sev.c | 2 +- > > 5 files changed, 47 insertions(+), 6 deletions(-) > > > >diff --git a/backends/igvm-cfg.c b/backends/igvm-cfg.c > >index c1b45401f4..0a77f7b7a1 100644 > >--- a/backends/igvm-cfg.c > >+++ b/backends/igvm-cfg.c > >@@ -17,6 +17,7 @@ > > #include "qom/object_interfaces.h" > > #include "hw/qdev-core.h" > > #include "hw/boards.h" > >+#include "hw/i386/acpi-build.h" > > > > #include "trace.h" > > > >@@ -48,10 +49,15 @@ static void igvm_reset_hold(Object *obj, ResetType type) > > { > > MachineState *ms = MACHINE(qdev_get_machine()); > > IgvmCfg *igvm = IGVM_CFG(obj); > >+ GArray *madt = NULL; > > > > trace_igvm_reset_hold(type); > > > >- qigvm_process_file(igvm, ms->cgs, false, &error_fatal); > >+ madt = acpi_build_madt_standalone(ms); > >+ > >+ qigvm_process_file(igvm, ms->cgs, false, madt, &error_fatal); > >+ > >+ g_array_free(madt, true); > > } > > > > static void igvm_reset_exit(Object *obj, ResetType type) > >diff --git a/backends/igvm.c b/backends/igvm.c > >index a350c890cc..7e56b19b0a 100644 > >--- a/backends/igvm.c > >+++ b/backends/igvm.c > >@@ -93,6 +93,7 @@ typedef struct QIgvm { > > unsigned region_start_index; > > unsigned region_last_index; > > unsigned region_page_count; > >+ GArray *madt; > > } QIgvm; > > > > static int qigvm_directive_page_data(QIgvm *ctx, const uint8_t *header_data, > >@@ -120,6 +121,8 @@ static int qigvm_directive_snp_id_block(QIgvm *ctx, const uint8_t *header_data, > > static int qigvm_initialization_guest_policy(QIgvm *ctx, > > const uint8_t *header_data, > > Error **errp); > >+static int qigvm_initialization_madt(QIgvm *ctx, > >+ const uint8_t *header_data, Error **errp); > > > > struct QIGVMHandler { > > uint32_t type; > >@@ -148,6 +151,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_initialization_madt }, > > }; > > > > static int qigvm_handler(QIgvm *ctx, uint32_t type, Error **errp) > >@@ -764,6 +769,34 @@ static int qigvm_initialization_guest_policy(QIgvm *ctx, > > return 0; > > } > > > >+static int qigvm_initialization_madt(QIgvm *ctx, > >+ const uint8_t *header_data, Error **errp) > >+{ > >+ const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data; > >+ QIgvmParameterData *param_entry; > >+ > >+ if (ctx->madt == NULL) { > >+ return 0; > >+ } > >+ > >+ /* Find the parameter area that should hold the device tree */ > >+ QTAILQ_FOREACH(param_entry, &ctx->parameter_data, next) > >+ { > >+ if (param_entry->index == param->parameter_area_index) { > >+ > >+ if (ctx->madt->len > param_entry->size) { > >+ error_setg( > >+ errp, > >+ "IGVM: MADT size exceeds parameter area defined in IGVM file"); > >+ return -1; > >+ } > >+ memcpy(param_entry->data, ctx->madt->data, ctx->madt->len); > >+ break; > >+ } > >+ } > >+ return 0; > >+} > >+ > > static int qigvm_supported_platform_compat_mask(QIgvm *ctx, Error **errp) > > { > > int32_t header_count; > >@@ -892,7 +925,7 @@ IgvmHandle qigvm_file_init(char *filename, Error **errp) > > } > > > > int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, > >- bool onlyVpContext, Error **errp) > >+ bool onlyVpContext, GArray *madt, Error **errp) > > { > > int32_t header_count; > > QIgvmParameterData *parameter; > >@@ -915,6 +948,8 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, > > ctx.cgs = cgs; > > ctx.cgsc = cgs ? CONFIDENTIAL_GUEST_SUPPORT_GET_CLASS(cgs) : NULL; > > > >+ ctx.madt = madt; > >+ > > /* > > * Check that the IGVM file provides configuration for the current > > * platform > >diff --git a/include/system/igvm-cfg.h b/include/system/igvm-cfg.h > >index 7dc48677fd..1a04302beb 100644 > >--- a/include/system/igvm-cfg.h > >+++ b/include/system/igvm-cfg.h > >@@ -42,8 +42,8 @@ typedef struct IgvmCfgClass { > > * > > * Returns 0 for ok and -1 on error. > > */ > > Should we update the documentation of this function now that we have a > new parameter, also explaining that it's optional. > Will do. > >- int (*process)(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, > >- bool onlyVpContext, Error **errp); > >+ int (*process)(IgvmCfg *cfg, ConfidentialGuestSupport *cgs, > >+ bool onlyVpContext, GArray *madt, Error **errp); > > > > } IgvmCfgClass; > > > >diff --git a/include/system/igvm.h b/include/system/igvm.h > >index ec2538daa0..f2e580e4ee 100644 > >--- a/include/system/igvm.h > >+++ b/include/system/igvm.h > >@@ -18,7 +18,7 @@ > > > > IgvmHandle qigvm_file_init(char *filename, Error **errp); > > int qigvm_process_file(IgvmCfg *igvm, ConfidentialGuestSupport *cgs, > >- bool onlyVpContext, Error **errp); > >+ bool onlyVpContext, GArray *madt, 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 fd2dada013..ffeb9f52a2 100644 > >--- a/target/i386/sev.c > >+++ b/target/i386/sev.c > >@@ -1892,7 +1892,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) == > >+ ->process(x86machine->igvm, machine->cgs, true, NULL, errp) == > > Why here we don't need to pass it? Here we only read the IGVM to figure out the initial vcpu configuration (the `onlyVpContext` parameter is true) to initialize kvm, The actual IGVM processing is done later. Should I mention in the comment above why madt is NULL here ? > > Thanks, > Stefano > > > -1) { > > return -1; > > } > >-- > >2.52.0 > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] igvm: Fill MADT IGVM parameter field 2025-12-11 9:24 ` Oliver Steffen @ 2025-12-11 10:11 ` Stefano Garzarella 0 siblings, 0 replies; 7+ messages in thread From: Stefano Garzarella @ 2025-12-11 10:11 UTC (permalink / raw) To: Oliver Steffen Cc: qemu-devel, Joerg Roedel, Marcel Apfelbaum, Gerd Hoffmann, Paolo Bonzini, Marcelo Tosatti, kvm, Richard Henderson, Zhao Liu, Eduardo Habkost, Ani Sinha, Michael S. Tsirkin, Luigi Leonardi, Igor Mammedov On Thu, Dec 11, 2025 at 10:24:35AM +0100, Oliver Steffen wrote: >On Thu, Dec 11, 2025 at 9:46 AM Stefano Garzarella <sgarzare@redhat.com> wrote: >> On Thu, Dec 11, 2025 at 09:15:17AM +0100, Oliver Steffen wrote: [...] >> >diff --git a/target/i386/sev.c b/target/i386/sev.c >> >index fd2dada013..ffeb9f52a2 100644 >> >--- a/target/i386/sev.c >> >+++ b/target/i386/sev.c >> >@@ -1892,7 +1892,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) == >> >+ ->process(x86machine->igvm, machine->cgs, true, NULL, errp) == >> >> Why here we don't need to pass it? > >Here we only read the IGVM to figure out the initial vcpu configuration >(the `onlyVpContext` parameter is true) to initialize kvm, >The actual IGVM processing is done later. okay, I see, thanks! >Should I mention in the comment above why madt is NULL here ? Yes, please :-) Stefano ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-12-11 10:12 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-12-11 8:15 [PATCH 0/3] igvm: Supply MADT via IGVM parameter Oliver Steffen 2025-12-11 8:15 ` [PATCH 1/3] hw/acpi: Make BIOS linker optional Oliver Steffen 2025-12-11 8:15 ` [PATCH 2/3] hw/acpi: Add standalone function to build MADT Oliver Steffen 2025-12-11 8:15 ` [PATCH 3/3] igvm: Fill MADT IGVM parameter field Oliver Steffen 2025-12-11 8:46 ` Stefano Garzarella 2025-12-11 9:24 ` Oliver Steffen 2025-12-11 10:11 ` Stefano Garzarella
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).