On Thu, Dec 11, 2025 at 12:30 PM Luigi Leonardi <[email protected]> wrote:
>
> Hi,
>
> On Thu, Dec 11, 2025 at 12:15:59PM +0100, Gerd Hoffmann wrote:
> > Hi,
> >
> >> +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 */
> >
> >cut+paste error in the comment.
Will do.
> >
> >> + QTAILQ_FOREACH(param_entry, &ctx->parameter_data, next)
> >> + {
> >> + if (param_entry->index == param->parameter_area_index) {
> >
> >Hmm, that is a pattern repeated a number of times already in the igvm
> >code. Should we factor that out into a helper function?
>
> +1
Will do.
>
> >
> >> 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)
> >
> >I'd like to see less parameters for this function, not more.
> >
> >I think sensible options here are:
> >
> > (a) store the madt pointer in IgvmCfg, or
> > (b) pass MachineState instead of ConfidentialGuestSupport, so
> > we can use the MachineState here to generate the madt.
> >
> >Luigi, any opinion? I think device tree support will need access to
> >MachineState too, and I think both madt and dt should take the same
> >approach here.
>
> I have a slight preference over MachineState as it's more generic and we
> don't need to add more fields in IgvmCfg for new features.
>
Passing in MachineState would be easy, but do we really want to add machine
related logic (building of ACPI tables, and later maybe device trees)
into the igvm backend?
> Luigi
>
