On Thu, Dec 11, 2025 at 9:46 AM Stefano Garzarella <[email protected]> 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 <[email protected]> > >--- > > 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 > > >
