[PATCH v2 0/3] igvm: Supply MADT via IGVM parameter

[PATCH v2 0/3] igvm: Supply MADT via IGVM parameter

[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

v2:
- Provide more context in the message of the main commit
- Document the madt parameter of IgvmCfgClass::process()
- Document why no MADT data is provided the the process call in sev.c

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 |  5 ++++-
 include/system/igvm.h     |  2 +-
 target/i386/sev.c         |  5 +++--
 8 files changed, 66 insertions(+), 8 deletions(-)

-- 
2.52.0

[PATCH v2 1/3] hw/acpi: Make BIOS linker optional

[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

[PATCH v2 2/3] hw/acpi: Add standalone function to build MADT

[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

[PATCH v2 3/3] igvm: Fill MADT IGVM parameter field

[Oliver Steffen]

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; 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.

Generating the MADT twice (here and during ACPI table building)
seems acceptable, since there is no infrastructure to obtain the MADT
out of the ACPI table memory area.

[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 <osteffen@redhat.com>
---
 backends/igvm-cfg.c       |  8 +++++++-
 backends/igvm.c           | 37 ++++++++++++++++++++++++++++++++++++-
 include/system/igvm-cfg.h |  5 ++++-
 include/system/igvm.h     |  2 +-
 target/i386/sev.c         |  5 +++--
 5 files changed, 51 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..d5138f745c 100644
--- a/include/system/igvm-cfg.h
+++ b/include/system/igvm-cfg.h
@@ -40,10 +40,13 @@ typedef struct IgvmCfgClass {
      * in the IGVM file will be processed, allowing information about the
      * CPU state to be determined before processing the entire file.
      *
+     * @madt: Optional ACPI MADT data to pass to the guest via the IGVM_VHT_MADT
+     *       parameter. Only relevant if onlyVpContext is false.
+     *
      * Returns 0 for ok and -1 on error.
      */
     int (*process)(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
-                   bool onlyVpContext, Error **errp);
+                   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..55fd20510a 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -1888,11 +1888,12 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
          * we need to pre-process it here to extract sev_features in order to
          * provide it to KVM_SEV_INIT2. Each cgs_* function that is called by
          * the IGVM processor detects this pre-process by observing the state
-         * as SEV_STATE_UNINIT.
+         * as SEV_STATE_UNINIT. The IGVM file is only read here, no MADT data
+         * needs to be supplied.
          */
         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

Re: [PATCH v2 3/3] igvm: Fill MADT IGVM parameter field

[Gerd Hoffmann]

  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.

> +    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?

>  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.

Long-term I'd like to also get rid of the onlyVpContext parameter.
That cleanup is something for another patch series though.

take care,
  Gerd

Re: [PATCH v2 3/3] igvm: Fill MADT IGVM parameter field

[Luigi Leonardi]

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.
>
>> +    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

>
>>  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.

Luigi

Re: [PATCH v2 3/3] igvm: Fill MADT IGVM parameter field

[Oliver Steffen]

On Thu, Dec 11, 2025 at 12:30 PM Luigi Leonardi <leonardi@redhat.com> 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
>

Re: [PATCH v2 3/3] igvm: Fill MADT IGVM parameter field

[Gerd Hoffmann]

  Hi,

> > >  (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?

Good point.  That clearly speaks for (b).  There already is
MachineState->fdt, filled in by machine code.  We can let machine code
store the madt in MachineState too, and the have the igvm code simply
pick it up from there.

take care,
  Gerd

Re: [PATCH v2 0/3] igvm: Supply MADT via IGVM parameter

[Igor Mammedov]

On Thu, 11 Dec 2025 11:31:33 +0100
Oliver Steffen <osteffen@redhat.com> wrote:

> 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.


looking at #2 and #3, it seems that root cause is still unknown,
one should be able read tables multiple times from fw_cg.
(so there is a but in QEMU or guest doesn't load ACPI tables correctly).

Also seeing that regenerating tables every time helps,
it hints that PCI subsystem is not configured when tables read 1st time.
Why that causes guest kernel errors is still unclear.

Main conditions to get acpi blob representing is that they should be read
after guest firmware enumerated/configured PCI subsystem and
firmware should use BIOSlinker workflow to load/postprocess
tables otherwise all bets are off (even if this series works for now,
it's subject to break without notice since user doesn't follow proper
procedures for reading/processing ACPI blob).
Hence I dislike this approach.

Alternatively, instead of ACPI tables one can use FW_CFG_MAX_CPUS
like old SeaBIOS used to do if all you need is APIC IDs.
Limitation of this approach is that one can't use sparse APIC ID
configs.
Benefit is that no QEMU change is required whatsoever.

If you still wish to proceed with standalone MADT approach,
please add to justification exact root cause of what corrupts
ACPI tables blob later on. With that, It would be easier to decide if
standalone MADT is an acceptable hack. 

> [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
> 
> v2:
> - Provide more context in the message of the main commit
> - Document the madt parameter of IgvmCfgClass::process()
> - Document why no MADT data is provided the the process call in sev.c
> 
> 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 |  5 ++++-
>  include/system/igvm.h     |  2 +-
>  target/i386/sev.c         |  5 +++--
>  8 files changed, 66 insertions(+), 8 deletions(-)
> 

Re: [PATCH v2 0/3] igvm: Supply MADT via IGVM parameter

[Gerd Hoffmann]

  Hi,

> Also seeing that regenerating tables every time helps,
> it hints that PCI subsystem is not configured when tables read 1st time.

This is the case.

> Main conditions to get acpi blob representing is that they should be read
> after guest firmware enumerated/configured PCI subsystem and
> firmware should use BIOSlinker workflow to load/postprocess
> tables otherwise all bets are off (even if this series works for now,
> it's subject to break without notice since user doesn't follow proper
> procedures for reading/processing ACPI blob).
> Hence I dislike this approach.

Well, the use case which needs this is a confidential VM with SVSM.  So
the SVSM firmware is the first thing which boots, sets up SVSM and the
services it provides (vTPM, in the future UEFI variable store), then
goes boot OVMF firmware which handles PCI initialization etc.

SVSM needs to know the SMP configuration, but it does not even look at
the PCI bus.

The MADT is static:  Nothing in there changes if the guest changes
hardware registers (such as pci bridge windows), nothing requires the
bios linker for cross-table references.  Given that I fail to see how
this can possibly break in the future.

> Alternatively, instead of ACPI tables one can use FW_CFG_MAX_CPUS
> like old SeaBIOS used to do if all you need is APIC IDs.
> Limitation of this approach is that one can't use sparse APIC ID
> configs.

Thats why the idea is to just use the MADT table for SMP discovery.
The APIC IDs are in there, and it also removes qemu-specific code from
SVSM.

take care,
  Gerd