[Qemu-devel] [PATCH] acpi: switch to a dummy SSDT

[Qemu-devel] [PATCH] acpi: switch to a dummy SSDT

[Michael S. Tsirkin]

We prefer not changing table sizes depending on parameters,
that's why we create a dummy table rather than just drop
the MCFG table.

However, a table named "QEMU" could be put to a better
use than just a stub, e.g. we could use it to pass
some QEMU specific info to guests.

Replace with an SSDT and pad with the Nop opcode
to preserve the length.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/acpi-build.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 73519ab..d0afb31e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2419,6 +2419,8 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
 {
     AcpiTableMcfg *mcfg;
     const char *sig;
+    const char *oem;
+    const char *oem_id;
     int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
 
     mcfg = acpi_data_push(table_data, len);
@@ -2431,16 +2433,21 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
     /* MCFG is used for ECAM which can be enabled or disabled by guest.
      * To avoid table size changes (which create migration issues),
      * always create the table even if there are no allocations,
-     * but set the signature to a reserved value in this case.
-     * ACPI spec requires OSPMs to ignore such tables.
+     * but fill it with Noop values.
+     * OSPMs ignore such tables.
      */
     if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
-        /* Reserved signature: ignored by OSPM */
-        sig = "QEMU";
+        sig = "SSDT";
+        oem = "QEMU ";
+        oem_id = "NOOP";
+        /* 0xa3 - NoopOp */
+        memset(&mcfg->reserved, 0xa3, len - offsetof(AcpiTableMcfg, reserved));
     } else {
         sig = "MCFG";
+        oem = NULL;
+        oem_id = NULL;
     }
-    build_header(linker, table_data, (void *)mcfg, sig, len, 1, NULL, NULL);
+    build_header(linker, table_data, (void *)mcfg, sig, len, 1, oem, oem_id);
 }
 
 /*
-- 
MST

Re: [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT

[Igor Mammedov]

On Tue, 9 Jan 2018 15:58:11 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> We prefer not changing table sizes depending on parameters,
> that's why we create a dummy table rather than just drop
> the MCFG table.
> 
> However, a table named "QEMU" could be put to a better
> use than just a stub, e.g. we could use it to pass
> some QEMU specific info to guests.
> 
> Replace with an SSDT and pad with the Nop opcode
> to preserve the length.
Not sure it's useful, do you have patches on top of that, which will use this?


> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/i386/acpi-build.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 73519ab..d0afb31e 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2419,6 +2419,8 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
>  {
>      AcpiTableMcfg *mcfg;
>      const char *sig;
> +    const char *oem;
> +    const char *oem_id;
>      int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
>  
>      mcfg = acpi_data_push(table_data, len);
> @@ -2431,16 +2433,21 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
>      /* MCFG is used for ECAM which can be enabled or disabled by guest.
>       * To avoid table size changes (which create migration issues),
>       * always create the table even if there are no allocations,
> -     * but set the signature to a reserved value in this case.
> -     * ACPI spec requires OSPMs to ignore such tables.
> +     * but fill it with Noop values.
> +     * OSPMs ignore such tables.
>       */
>      if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
> -        /* Reserved signature: ignored by OSPM */
> -        sig = "QEMU";
> +        sig = "SSDT";
> +        oem = "QEMU ";
> +        oem_id = "NOOP";
> +        /* 0xa3 - NoopOp */
> +        memset(&mcfg->reserved, 0xa3, len - offsetof(AcpiTableMcfg, reserved));
>      } else {
>          sig = "MCFG";
> +        oem = NULL;
> +        oem_id = NULL;
>      }
> -    build_header(linker, table_data, (void *)mcfg, sig, len, 1, NULL, NULL);
> +    build_header(linker, table_data, (void *)mcfg, sig, len, 1, oem, oem_id);
>  }
>  
>  /*

Re: [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT

[Michael S. Tsirkin]

On Wed, Jan 10, 2018 at 12:25:34PM +0100, Igor Mammedov wrote:
> On Tue, 9 Jan 2018 15:58:11 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > We prefer not changing table sizes depending on parameters,
> > that's why we create a dummy table rather than just drop
> > the MCFG table.
> > 
> > However, a table named "QEMU" could be put to a better
> > use than just a stub, e.g. we could use it to pass
> > some QEMU specific info to guests.
> > 
> > Replace with an SSDT and pad with the Nop opcode
> > to preserve the length.
> Not sure it's useful, do you have patches on top of that, which will use this?

Yes but not me - Stefan (Cc'd) wants to add a custom table.

> 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  hw/i386/acpi-build.c | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 73519ab..d0afb31e 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -2419,6 +2419,8 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
> >  {
> >      AcpiTableMcfg *mcfg;
> >      const char *sig;
> > +    const char *oem;
> > +    const char *oem_id;
> >      int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
> >  
> >      mcfg = acpi_data_push(table_data, len);
> > @@ -2431,16 +2433,21 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
> >      /* MCFG is used for ECAM which can be enabled or disabled by guest.
> >       * To avoid table size changes (which create migration issues),
> >       * always create the table even if there are no allocations,
> > -     * but set the signature to a reserved value in this case.
> > -     * ACPI spec requires OSPMs to ignore such tables.
> > +     * but fill it with Noop values.
> > +     * OSPMs ignore such tables.
> >       */
> >      if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
> > -        /* Reserved signature: ignored by OSPM */
> > -        sig = "QEMU";
> > +        sig = "SSDT";
> > +        oem = "QEMU ";
> > +        oem_id = "NOOP";
> > +        /* 0xa3 - NoopOp */
> > +        memset(&mcfg->reserved, 0xa3, len - offsetof(AcpiTableMcfg, reserved));
> >      } else {
> >          sig = "MCFG";
> > +        oem = NULL;
> > +        oem_id = NULL;
> >      }
> > -    build_header(linker, table_data, (void *)mcfg, sig, len, 1, NULL, NULL);
> > +    build_header(linker, table_data, (void *)mcfg, sig, len, 1, oem, oem_id);
> >  }
> >  
> >  /*

Re: [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT

[Igor Mammedov]

On Wed, 10 Jan 2018 13:29:09 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Jan 10, 2018 at 12:25:34PM +0100, Igor Mammedov wrote:
> > On Tue, 9 Jan 2018 15:58:11 +0200
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > 
> > > We prefer not changing table sizes depending on parameters,
> > > that's why we create a dummy table rather than just drop
> > > the MCFG table.
> > > 
> > > However, a table named "QEMU" could be put to a better
> > > use than just a stub, e.g. we could use it to pass
> > > some QEMU specific info to guests.
> > > 
> > > Replace with an SSDT and pad with the Nop opcode
> > > to preserve the length.
> > Not sure it's useful, do you have patches on top of that, which will use this?
> 
> Yes but not me - Stefan (Cc'd) wants to add a custom table.
Re-purposing dummy table that might become not dummy doesn't look like good approach.
What if later the guest enables MCFG and reboots, DUMMY SSDT would be replaced by MCFG? 

It'd be cleaner if Stefan would just use his own dedicated SSDT.

> > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > >  hw/i386/acpi-build.c | 17 ++++++++++++-----
> > >  1 file changed, 12 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index 73519ab..d0afb31e 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -2419,6 +2419,8 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
> > >  {
> > >      AcpiTableMcfg *mcfg;
> > >      const char *sig;
> > > +    const char *oem;
> > > +    const char *oem_id;
> > >      int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
> > >  
> > >      mcfg = acpi_data_push(table_data, len);
> > > @@ -2431,16 +2433,21 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
> > >      /* MCFG is used for ECAM which can be enabled or disabled by guest.
> > >       * To avoid table size changes (which create migration issues),
> > >       * always create the table even if there are no allocations,
> > > -     * but set the signature to a reserved value in this case.
> > > -     * ACPI spec requires OSPMs to ignore such tables.
> > > +     * but fill it with Noop values.
> > > +     * OSPMs ignore such tables.
> > >       */
> > >      if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
> > > -        /* Reserved signature: ignored by OSPM */
> > > -        sig = "QEMU";
> > > +        sig = "SSDT";
> > > +        oem = "QEMU ";
> > > +        oem_id = "NOOP";
> > > +        /* 0xa3 - NoopOp */
> > > +        memset(&mcfg->reserved, 0xa3, len - offsetof(AcpiTableMcfg, reserved));
> > >      } else {
> > >          sig = "MCFG";
> > > +        oem = NULL;
> > > +        oem_id = NULL;
> > >      }
> > > -    build_header(linker, table_data, (void *)mcfg, sig, len, 1, NULL, NULL);
> > > +    build_header(linker, table_data, (void *)mcfg, sig, len, 1, oem, oem_id);
> > >  }
> > >  
> > >  /*

Re: [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT

[Michael S. Tsirkin]

On Wed, Jan 10, 2018 at 12:41:52PM +0100, Igor Mammedov wrote:
> On Wed, 10 Jan 2018 13:29:09 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Jan 10, 2018 at 12:25:34PM +0100, Igor Mammedov wrote:
> > > On Tue, 9 Jan 2018 15:58:11 +0200
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > 
> > > > We prefer not changing table sizes depending on parameters,
> > > > that's why we create a dummy table rather than just drop
> > > > the MCFG table.
> > > > 
> > > > However, a table named "QEMU" could be put to a better
> > > > use than just a stub, e.g. we could use it to pass
> > > > some QEMU specific info to guests.
> > > > 
> > > > Replace with an SSDT and pad with the Nop opcode
> > > > to preserve the length.
> > > Not sure it's useful, do you have patches on top of that, which will use this?
> > 
> > Yes but not me - Stefan (Cc'd) wants to add a custom table.
> Re-purposing dummy table that might become not dummy doesn't look like good approach.
> What if later the guest enables MCFG and reboots, DUMMY SSDT would be replaced by MCFG? 

So what's the issue?  Why is this worse than a dummy QEMU table?
There's actually no re-purposing here at all. Either we create
an MCFG or an SSDT. We never re-purpose.

> It'd be cleaner if Stefan would just use his own dedicated SSDT.

SSDT doesn't always work. A data table is needed so e.g. seabios
can process it easily.

> > > 
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > ---
> > > >  hw/i386/acpi-build.c | 17 ++++++++++++-----
> > > >  1 file changed, 12 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > > index 73519ab..d0afb31e 100644
> > > > --- a/hw/i386/acpi-build.c
> > > > +++ b/hw/i386/acpi-build.c
> > > > @@ -2419,6 +2419,8 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
> > > >  {
> > > >      AcpiTableMcfg *mcfg;
> > > >      const char *sig;
> > > > +    const char *oem;
> > > > +    const char *oem_id;
> > > >      int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
> > > >  
> > > >      mcfg = acpi_data_push(table_data, len);
> > > > @@ -2431,16 +2433,21 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
> > > >      /* MCFG is used for ECAM which can be enabled or disabled by guest.
> > > >       * To avoid table size changes (which create migration issues),
> > > >       * always create the table even if there are no allocations,
> > > > -     * but set the signature to a reserved value in this case.
> > > > -     * ACPI spec requires OSPMs to ignore such tables.
> > > > +     * but fill it with Noop values.
> > > > +     * OSPMs ignore such tables.
> > > >       */
> > > >      if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
> > > > -        /* Reserved signature: ignored by OSPM */
> > > > -        sig = "QEMU";
> > > > +        sig = "SSDT";
> > > > +        oem = "QEMU ";
> > > > +        oem_id = "NOOP";
> > > > +        /* 0xa3 - NoopOp */
> > > > +        memset(&mcfg->reserved, 0xa3, len - offsetof(AcpiTableMcfg, reserved));
> > > >      } else {
> > > >          sig = "MCFG";
> > > > +        oem = NULL;
> > > > +        oem_id = NULL;
> > > >      }
> > > > -    build_header(linker, table_data, (void *)mcfg, sig, len, 1, NULL, NULL);
> > > > +    build_header(linker, table_data, (void *)mcfg, sig, len, 1, oem, oem_id);
> > > >  }
> > > >  
> > > >  /*

Re: [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT

[Igor Mammedov]

On Wed, 10 Jan 2018 13:55:37 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Jan 10, 2018 at 12:41:52PM +0100, Igor Mammedov wrote:
> > On Wed, 10 Jan 2018 13:29:09 +0200
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >   
> > > On Wed, Jan 10, 2018 at 12:25:34PM +0100, Igor Mammedov wrote:  
> > > > On Tue, 9 Jan 2018 15:58:11 +0200
> > > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > >   
> > > > > We prefer not changing table sizes depending on parameters,
> > > > > that's why we create a dummy table rather than just drop
> > > > > the MCFG table.
> > > > > 
> > > > > However, a table named "QEMU" could be put to a better
> > > > > use than just a stub, e.g. we could use it to pass
> > > > > some QEMU specific info to guests.
> > > > > 
> > > > > Replace with an SSDT and pad with the Nop opcode
> > > > > to preserve the length.  
> > > > Not sure it's useful, do you have patches on top of that, which will use this?  
> > > 
> > > Yes but not me - Stefan (Cc'd) wants to add a custom table.  
> > Re-purposing dummy table that might become not dummy doesn't look like good approach.
> > What if later the guest enables MCFG and reboots, DUMMY SSDT would be replaced by MCFG?   
> 
> So what's the issue?  Why is this worse than a dummy QEMU table?
> There's actually no re-purposing here at all. Either we create
> an MCFG or an SSDT. We never re-purpose.
I think, I've misread commit message, so it's table name 'QEMU'
that's going to be reused.

Having several tables with name QEMU shouldn't be an issue,
adding oem/oem_id should be sufficient to distinguish them
for parsing side (if there is any).

As for change from ignored table to always parsed SSDT table,
having ignored one seems better.
So I'd drop SSDT+memset and keep oem[id] parts in this patch.
 
> > It'd be cleaner if Stefan would just use his own dedicated SSDT.  
> 
> SSDT doesn't always work. A data table is needed so e.g. seabios
> can process it easily.
> 
> > > >   
> > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > > ---
> > > > >  hw/i386/acpi-build.c | 17 ++++++++++++-----
> > > > >  1 file changed, 12 insertions(+), 5 deletions(-)
> > > > > 
> > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > > > index 73519ab..d0afb31e 100644
> > > > > --- a/hw/i386/acpi-build.c
> > > > > +++ b/hw/i386/acpi-build.c
> > > > > @@ -2419,6 +2419,8 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
> > > > >  {
> > > > >      AcpiTableMcfg *mcfg;
> > > > >      const char *sig;
> > > > > +    const char *oem;
> > > > > +    const char *oem_id;
> > > > >      int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
> > > > >  
> > > > >      mcfg = acpi_data_push(table_data, len);
> > > > > @@ -2431,16 +2433,21 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
> > > > >      /* MCFG is used for ECAM which can be enabled or disabled by guest.
> > > > >       * To avoid table size changes (which create migration issues),
> > > > >       * always create the table even if there are no allocations,
> > > > > -     * but set the signature to a reserved value in this case.
> > > > > -     * ACPI spec requires OSPMs to ignore such tables.
> > > > > +     * but fill it with Noop values.
> > > > > +     * OSPMs ignore such tables.
> > > > >       */
> > > > >      if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
> > > > > -        /* Reserved signature: ignored by OSPM */
> > > > > -        sig = "QEMU";
> > > > > +        sig = "SSDT";
> > > > > +        oem = "QEMU ";
> > > > > +        oem_id = "NOOP";
> > > > > +        /* 0xa3 - NoopOp */
> > > > > +        memset(&mcfg->reserved, 0xa3, len - offsetof(AcpiTableMcfg, reserved));
> > > > >      } else {
> > > > >          sig = "MCFG";
> > > > > +        oem = NULL;
> > > > > +        oem_id = NULL;
> > > > >      }
> > > > > -    build_header(linker, table_data, (void *)mcfg, sig, len, 1, NULL, NULL);
> > > > > +    build_header(linker, table_data, (void *)mcfg, sig, len, 1, oem, oem_id);
> > > > >  }
> > > > >  
> > > > >  /*  

Re: [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT

[Michael S. Tsirkin]

On Wed, Jan 10, 2018 at 01:31:09PM +0100, Igor Mammedov wrote:
> On Wed, 10 Jan 2018 13:55:37 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Jan 10, 2018 at 12:41:52PM +0100, Igor Mammedov wrote:
> > > On Wed, 10 Jan 2018 13:29:09 +0200
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > >   
> > > > On Wed, Jan 10, 2018 at 12:25:34PM +0100, Igor Mammedov wrote:  
> > > > > On Tue, 9 Jan 2018 15:58:11 +0200
> > > > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > >   
> > > > > > We prefer not changing table sizes depending on parameters,
> > > > > > that's why we create a dummy table rather than just drop
> > > > > > the MCFG table.
> > > > > > 
> > > > > > However, a table named "QEMU" could be put to a better
> > > > > > use than just a stub, e.g. we could use it to pass
> > > > > > some QEMU specific info to guests.
> > > > > > 
> > > > > > Replace with an SSDT and pad with the Nop opcode
> > > > > > to preserve the length.  
> > > > > Not sure it's useful, do you have patches on top of that, which will use this?  
> > > > 
> > > > Yes but not me - Stefan (Cc'd) wants to add a custom table.  
> > > Re-purposing dummy table that might become not dummy doesn't look like good approach.
> > > What if later the guest enables MCFG and reboots, DUMMY SSDT would be replaced by MCFG?   
> > 
> > So what's the issue?  Why is this worse than a dummy QEMU table?
> > There's actually no re-purposing here at all. Either we create
> > an MCFG or an SSDT. We never re-purpose.
> I think, I've misread commit message, so it's table name 'QEMU'
> that's going to be reused.
> 
> Having several tables with name QEMU shouldn't be an issue,
> adding oem/oem_id should be sufficient to distinguish them
> for parsing side (if there is any).

It seems more work for bios to scan all such
tables and not just the first one. I might be wrong here.

> As for change from ignored table to always parsed SSDT table,
> having ignored one seems better.
> So I'd drop SSDT+memset and keep oem[id] parts in this patch.

I'll wait until Stefan posts his patches using this, then we
can see what does look better.


> > > It'd be cleaner if Stefan would just use his own dedicated SSDT.  
> > 
> > SSDT doesn't always work. A data table is needed so e.g. seabios
> > can process it easily.
> > 
> > > > >   
> > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > > > ---
> > > > > >  hw/i386/acpi-build.c | 17 ++++++++++++-----
> > > > > >  1 file changed, 12 insertions(+), 5 deletions(-)
> > > > > > 
> > > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > > > > index 73519ab..d0afb31e 100644
> > > > > > --- a/hw/i386/acpi-build.c
> > > > > > +++ b/hw/i386/acpi-build.c
> > > > > > @@ -2419,6 +2419,8 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
> > > > > >  {
> > > > > >      AcpiTableMcfg *mcfg;
> > > > > >      const char *sig;
> > > > > > +    const char *oem;
> > > > > > +    const char *oem_id;
> > > > > >      int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
> > > > > >  
> > > > > >      mcfg = acpi_data_push(table_data, len);
> > > > > > @@ -2431,16 +2433,21 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
> > > > > >      /* MCFG is used for ECAM which can be enabled or disabled by guest.
> > > > > >       * To avoid table size changes (which create migration issues),
> > > > > >       * always create the table even if there are no allocations,
> > > > > > -     * but set the signature to a reserved value in this case.
> > > > > > -     * ACPI spec requires OSPMs to ignore such tables.
> > > > > > +     * but fill it with Noop values.
> > > > > > +     * OSPMs ignore such tables.
> > > > > >       */
> > > > > >      if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
> > > > > > -        /* Reserved signature: ignored by OSPM */
> > > > > > -        sig = "QEMU";
> > > > > > +        sig = "SSDT";
> > > > > > +        oem = "QEMU ";
> > > > > > +        oem_id = "NOOP";
> > > > > > +        /* 0xa3 - NoopOp */
> > > > > > +        memset(&mcfg->reserved, 0xa3, len - offsetof(AcpiTableMcfg, reserved));
> > > > > >      } else {
> > > > > >          sig = "MCFG";
> > > > > > +        oem = NULL;
> > > > > > +        oem_id = NULL;
> > > > > >      }
> > > > > > -    build_header(linker, table_data, (void *)mcfg, sig, len, 1, NULL, NULL);
> > > > > > +    build_header(linker, table_data, (void *)mcfg, sig, len, 1, oem, oem_id);
> > > > > >  }
> > > > > >  
> > > > > >  /*  

Re: [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT

[Stefan Berger]

On 01/10/2018 06:29 AM, Michael S. Tsirkin wrote:
> On Wed, Jan 10, 2018 at 12:25:34PM +0100, Igor Mammedov wrote:
>> On Tue, 9 Jan 2018 15:58:11 +0200
>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>
>>> We prefer not changing table sizes depending on parameters,
>>> that's why we create a dummy table rather than just drop
>>> the MCFG table.
>>>
>>> However, a table named "QEMU" could be put to a better
>>> use than just a stub, e.g. we could use it to pass
>>> some QEMU specific info to guests.
>>>
>>> Replace with an SSDT and pad with the Nop opcode
>>> to preserve the length.
>> Not sure it's useful, do you have patches on top of that, which will use this?
> Yes but not me - Stefan (Cc'd) wants to add a custom table.

With the virtual device at a well known address I don't need the custom 
table anymore...

    Stefan
>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>> ---
>>>   hw/i386/acpi-build.c | 17 ++++++++++++-----
>>>   1 file changed, 12 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>> index 73519ab..d0afb31e 100644
>>> --- a/hw/i386/acpi-build.c
>>> +++ b/hw/i386/acpi-build.c
>>> @@ -2419,6 +2419,8 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
>>>   {
>>>       AcpiTableMcfg *mcfg;
>>>       const char *sig;
>>> +    const char *oem;
>>> +    const char *oem_id;
>>>       int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
>>>   
>>>       mcfg = acpi_data_push(table_data, len);
>>> @@ -2431,16 +2433,21 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
>>>       /* MCFG is used for ECAM which can be enabled or disabled by guest.
>>>        * To avoid table size changes (which create migration issues),
>>>        * always create the table even if there are no allocations,
>>> -     * but set the signature to a reserved value in this case.
>>> -     * ACPI spec requires OSPMs to ignore such tables.
>>> +     * but fill it with Noop values.
>>> +     * OSPMs ignore such tables.
>>>        */
>>>       if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
>>> -        /* Reserved signature: ignored by OSPM */
>>> -        sig = "QEMU";
>>> +        sig = "SSDT";
>>> +        oem = "QEMU ";
>>> +        oem_id = "NOOP";
>>> +        /* 0xa3 - NoopOp */
>>> +        memset(&mcfg->reserved, 0xa3, len - offsetof(AcpiTableMcfg, reserved));
>>>       } else {
>>>           sig = "MCFG";
>>> +        oem = NULL;
>>> +        oem_id = NULL;
>>>       }
>>> -    build_header(linker, table_data, (void *)mcfg, sig, len, 1, NULL, NULL);
>>> +    build_header(linker, table_data, (void *)mcfg, sig, len, 1, oem, oem_id);
>>>   }
>>>   
>>>   /*