* [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT @ 2018-01-09 13:58 Michael S. Tsirkin 2018-01-10 11:25 ` Igor Mammedov 0 siblings, 1 reply; 8+ messages in thread From: Michael S. Tsirkin @ 2018-01-09 13:58 UTC (permalink / raw) To: qemu-devel Cc: Igor Mammedov, Paolo Bonzini, Richard Henderson, Eduardo Habkost 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT 2018-01-09 13:58 [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT Michael S. Tsirkin @ 2018-01-10 11:25 ` Igor Mammedov 2018-01-10 11:29 ` Michael S. Tsirkin 0 siblings, 1 reply; 8+ messages in thread From: Igor Mammedov @ 2018-01-10 11:25 UTC (permalink / raw) To: Michael S. Tsirkin Cc: qemu-devel, Paolo Bonzini, Richard Henderson, Eduardo Habkost 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); > } > > /* ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT 2018-01-10 11:25 ` Igor Mammedov @ 2018-01-10 11:29 ` Michael S. Tsirkin 2018-01-10 11:41 ` Igor Mammedov 2018-01-10 18:19 ` Stefan Berger 0 siblings, 2 replies; 8+ messages in thread From: Michael S. Tsirkin @ 2018-01-10 11:29 UTC (permalink / raw) To: Igor Mammedov Cc: qemu-devel, Paolo Bonzini, Richard Henderson, Eduardo Habkost, stefanb 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); > > } > > > > /* ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT 2018-01-10 11:29 ` Michael S. Tsirkin @ 2018-01-10 11:41 ` Igor Mammedov 2018-01-10 11:55 ` Michael S. Tsirkin 2018-01-10 18:19 ` Stefan Berger 1 sibling, 1 reply; 8+ messages in thread From: Igor Mammedov @ 2018-01-10 11:41 UTC (permalink / raw) To: Michael S. Tsirkin Cc: qemu-devel, Paolo Bonzini, Richard Henderson, Eduardo Habkost, stefanb 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); > > > } > > > > > > /* ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT 2018-01-10 11:41 ` Igor Mammedov @ 2018-01-10 11:55 ` Michael S. Tsirkin 2018-01-10 12:31 ` Igor Mammedov 0 siblings, 1 reply; 8+ messages in thread From: Michael S. Tsirkin @ 2018-01-10 11:55 UTC (permalink / raw) To: Igor Mammedov Cc: qemu-devel, Paolo Bonzini, Richard Henderson, Eduardo Habkost, stefanb 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); > > > > } > > > > > > > > /* ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT 2018-01-10 11:55 ` Michael S. Tsirkin @ 2018-01-10 12:31 ` Igor Mammedov 2018-01-10 13:46 ` Michael S. Tsirkin 0 siblings, 1 reply; 8+ messages in thread From: Igor Mammedov @ 2018-01-10 12:31 UTC (permalink / raw) To: Michael S. Tsirkin Cc: qemu-devel, Paolo Bonzini, Richard Henderson, Eduardo Habkost, stefanb 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); > > > > > } > > > > > > > > > > /* ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT 2018-01-10 12:31 ` Igor Mammedov @ 2018-01-10 13:46 ` Michael S. Tsirkin 0 siblings, 0 replies; 8+ messages in thread From: Michael S. Tsirkin @ 2018-01-10 13:46 UTC (permalink / raw) To: Igor Mammedov Cc: qemu-devel, Paolo Bonzini, Richard Henderson, Eduardo Habkost, stefanb 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); > > > > > > } > > > > > > > > > > > > /* ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT 2018-01-10 11:29 ` Michael S. Tsirkin 2018-01-10 11:41 ` Igor Mammedov @ 2018-01-10 18:19 ` Stefan Berger 1 sibling, 0 replies; 8+ messages in thread From: Stefan Berger @ 2018-01-10 18:19 UTC (permalink / raw) To: Michael S. Tsirkin, Igor Mammedov Cc: Paolo Bonzini, qemu-devel, Eduardo Habkost, Richard Henderson 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); >>> } >>> >>> /* ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-01-10 18:20 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-01-09 13:58 [Qemu-devel] [PATCH] acpi: switch to a dummy SSDT Michael S. Tsirkin 2018-01-10 11:25 ` Igor Mammedov 2018-01-10 11:29 ` Michael S. Tsirkin 2018-01-10 11:41 ` Igor Mammedov 2018-01-10 11:55 ` Michael S. Tsirkin 2018-01-10 12:31 ` Igor Mammedov 2018-01-10 13:46 ` Michael S. Tsirkin 2018-01-10 18:19 ` Stefan Berger
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).