| Message ID | 20231003211658.14327-1-shentey@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] hw/i386/acpi-build: Remove build-time assertion on PIIX/ICH9 reset registers being identical | expand |
The iteration in the subject should have been 1, not 3... Am 3. Oktober 2023 21:16:58 UTC schrieb Bernhard Beschow <shentey@gmail.com>: >Commit 6103451aeb74 ("hw/i386: Build-time assertion on pc/q35 reset register >being identical.") introduced a build-time check where the addresses of the >reset registers are expected to be equal. Back then the code to generate AML for >the reset register in the FADT was common. However, since commit 937d1b58714b >("pc: acpi: isolate FADT specific data into AcpiFadtData structure") the AML >gets generated for ICH9 only. There is no need any loger for the assertion, so >remove it. > >Signed-off-by: Bernhard Beschow <shentey@gmail.com> >--- > hw/i386/acpi-build.c | 5 ----- > 1 file changed, 5 deletions(-) > >diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >index 95199c8900..6fff1901f5 100644 >--- a/hw/i386/acpi-build.c >+++ b/hw/i386/acpi-build.c >@@ -56,7 +56,6 @@ > > /* Supported chipsets: */ > #include "hw/southbridge/ich9.h" >-#include "hw/southbridge/piix.h" > #include "hw/acpi/pcihp.h" > #include "hw/i386/fw_cfg.h" > #include "hw/i386/pc.h" >@@ -242,10 +241,6 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm) > pm->pcihp_io_len = > object_property_get_uint(obj, ACPI_PCIHP_IO_LEN_PROP, NULL); > >- /* The above need not be conditional on machine type because the reset port >- * happens to be the same on PIIX (pc) and ICH9 (q35). */ >- QEMU_BUILD_BUG_ON(ICH9_RST_CNT_IOPORT != PIIX_RCR_IOPORT); >- > /* Fill in optional s3/s4 related properties */ > o = object_property_get_qobject(obj, ACPI_PM_PROP_S3_DISABLED, NULL); > if (o) {
On 3/10/23 23:16, Bernhard Beschow wrote: > Commit 6103451aeb74 ("hw/i386: Build-time assertion on pc/q35 reset register > being identical.") introduced a build-time check where the addresses of the > reset registers are expected to be equal. Back then the code to generate AML for > the reset register in the FADT was common. However, since commit 937d1b58714b > ("pc: acpi: isolate FADT specific data into AcpiFadtData structure") the AML > gets generated for ICH9 only. There is no need any loger for the assertion, so "longer" > remove it. > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > --- > hw/i386/acpi-build.c | 5 ----- > 1 file changed, 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> On 04-Oct-2023, at 2:46 AM, Bernhard Beschow <shentey@gmail.com> wrote: > > Commit 6103451aeb74 ("hw/i386: Build-time assertion on pc/q35 reset register > being identical.") introduced a build-time check where the addresses of the > reset registers are expected to be equal. Back then the code to generate AML for > the reset register in the FADT was common. However, since commit 937d1b58714b > ("pc: acpi: isolate FADT specific data into AcpiFadtData structure") the AML > gets generated for ICH9 only. This isn’t quite true. See 3a3fcc75f92ab0d71ba (" pc: acpi: force FADT rev1 for 440fx based machine types”) where the fadt table size for i440fx is no longer *fadt but offsetof(typeof(*fadt), reset_register). The above commit simply makes sure we do not populate reset_register etc for i440fx since its not used anyway. > There is no need any loger for the assertion, so Typo ^^^^^ > remove it. > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> Other than the above, I agree with the change. So .. Reviewed-by: Ani Sinha <anisinha@redhat.com> > --- > hw/i386/acpi-build.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 95199c8900..6fff1901f5 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -56,7 +56,6 @@ > > /* Supported chipsets: */ > #include "hw/southbridge/ich9.h" > -#include "hw/southbridge/piix.h" > #include "hw/acpi/pcihp.h" > #include "hw/i386/fw_cfg.h" > #include "hw/i386/pc.h" > @@ -242,10 +241,6 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm) > pm->pcihp_io_len = > object_property_get_uint(obj, ACPI_PCIHP_IO_LEN_PROP, NULL); > > - /* The above need not be conditional on machine type because the reset port > - * happens to be the same on PIIX (pc) and ICH9 (q35). */ > - QEMU_BUILD_BUG_ON(ICH9_RST_CNT_IOPORT != PIIX_RCR_IOPORT); > - > /* Fill in optional s3/s4 related properties */ > o = object_property_get_qobject(obj, ACPI_PM_PROP_S3_DISABLED, NULL); > if (o) { > -- > 2.42.0 >
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 95199c8900..6fff1901f5 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -56,7 +56,6 @@ /* Supported chipsets: */ #include "hw/southbridge/ich9.h" -#include "hw/southbridge/piix.h" #include "hw/acpi/pcihp.h" #include "hw/i386/fw_cfg.h" #include "hw/i386/pc.h" @@ -242,10 +241,6 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm) pm->pcihp_io_len = object_property_get_uint(obj, ACPI_PCIHP_IO_LEN_PROP, NULL); - /* The above need not be conditional on machine type because the reset port - * happens to be the same on PIIX (pc) and ICH9 (q35). */ - QEMU_BUILD_BUG_ON(ICH9_RST_CNT_IOPORT != PIIX_RCR_IOPORT); - /* Fill in optional s3/s4 related properties */ o = object_property_get_qobject(obj, ACPI_PM_PROP_S3_DISABLED, NULL); if (o) {
Commit 6103451aeb74 ("hw/i386: Build-time assertion on pc/q35 reset register being identical.") introduced a build-time check where the addresses of the reset registers are expected to be equal. Back then the code to generate AML for the reset register in the FADT was common. However, since commit 937d1b58714b ("pc: acpi: isolate FADT specific data into AcpiFadtData structure") the AML gets generated for ICH9 only. There is no need any loger for the assertion, so remove it. Signed-off-by: Bernhard Beschow <shentey@gmail.com> --- hw/i386/acpi-build.c | 5 ----- 1 file changed, 5 deletions(-)