| Message ID | 20240328155439.58719-10-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/i386/pc: Decouple ISA vs PCI-based machines | expand |
On Thu, 28 Mar 2024, Philippe Mathieu-Daudé wrote: > acpi_setup() caller knows about the machine state, so pass > it as argument to avoid a qdev_get_machine() call. > > We already resolved X86_MACHINE(pcms) as 'x86ms' so use the > latter. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > hw/i386/acpi-build.h | 3 ++- > hw/i386/acpi-build.c | 5 ++--- > hw/i386/pc.c | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h > index 0dce155c8c..31de5bddbd 100644 > --- a/hw/i386/acpi-build.h > +++ b/hw/i386/acpi-build.h > @@ -2,6 +2,7 @@ > #ifndef HW_I386_ACPI_BUILD_H > #define HW_I386_ACPI_BUILD_H > #include "hw/acpi/acpi-defs.h" > +#include "hw/i386/pc.h" > > extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; > > @@ -9,7 +10,7 @@ extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; > #define ACPI_PCIHP_SEJ_BASE 0x8 > #define ACPI_PCIHP_BNMR_BASE 0x10 > > -void acpi_setup(void); > +void acpi_setup(PCMachineState *pcms); This is changed to PcPciMachineState * in a following patch so can't you already introduce it here to avoid some churn? Regards, BALATON Zoltan > Object *acpi_get_i386_pci_host(void); > > #endif > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 6e8e32e5d2..e702d5e9d2 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -2749,9 +2749,8 @@ static const VMStateDescription vmstate_acpi_build = { > }, > }; > > -void acpi_setup(void) > +void acpi_setup(PCMachineState *pcms) > { > - PCMachineState *pcms = PC_MACHINE(qdev_get_machine()); > X86MachineState *x86ms = X86_MACHINE(pcms); > AcpiBuildTables tables; > AcpiBuildState *build_state; > @@ -2771,7 +2770,7 @@ void acpi_setup(void) > return; > } > > - if (!x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) { > + if (!x86_machine_is_acpi_enabled(x86ms)) { > ACPI_BUILD_DPRINTF("ACPI disabled. Bailing out.\n"); > return; > } > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 6d87d1d4c2..dfc0247bb6 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -601,7 +601,7 @@ void pc_machine_done(Notifier *notifier, void *data) > /* set the number of CPUs */ > x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus); > > - acpi_setup(); > + acpi_setup(pcms); > if (x86ms->fw_cfg) { > fw_cfg_build_smbios(pcms, x86ms->fw_cfg, pcms->smbios_entry_point_type); > fw_cfg_build_feature_control(MACHINE(pcms), x86ms->fw_cfg); >
On 28/3/24 19:45, BALATON Zoltan wrote: > On Thu, 28 Mar 2024, Philippe Mathieu-Daudé wrote: >> acpi_setup() caller knows about the machine state, so pass >> it as argument to avoid a qdev_get_machine() call. >> >> We already resolved X86_MACHINE(pcms) as 'x86ms' so use the >> latter. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> hw/i386/acpi-build.h | 3 ++- >> hw/i386/acpi-build.c | 5 ++--- >> hw/i386/pc.c | 2 +- >> 3 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h >> index 0dce155c8c..31de5bddbd 100644 >> --- a/hw/i386/acpi-build.h >> +++ b/hw/i386/acpi-build.h >> @@ -2,6 +2,7 @@ >> #ifndef HW_I386_ACPI_BUILD_H >> #define HW_I386_ACPI_BUILD_H >> #include "hw/acpi/acpi-defs.h" >> +#include "hw/i386/pc.h" >> >> extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; >> >> @@ -9,7 +10,7 @@ extern const struct AcpiGenericAddress >> x86_nvdimm_acpi_dsmio; >> #define ACPI_PCIHP_SEJ_BASE 0x8 >> #define ACPI_PCIHP_BNMR_BASE 0x10 >> >> -void acpi_setup(void); >> +void acpi_setup(PCMachineState *pcms); > > This is changed to PcPciMachineState * in a following patch so can't you > already introduce it here to avoid some churn? Unfortunately not, because we'd need to use: PcPciMachineState *ppms = PC_PCI_MACHINE(pcms); which would trigger an assertion at this point. > > Regards, > BALATON Zoltan
Am 28. März 2024 15:54:17 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>: >acpi_setup() caller knows about the machine state, so pass >it as argument to avoid a qdev_get_machine() call. > >We already resolved X86_MACHINE(pcms) as 'x86ms' so use the >latter. > >Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> This patch looks like good material on its own. Reviewed-by: Bernhard Beschow <shentey@gmail.com> >--- > hw/i386/acpi-build.h | 3 ++- > hw/i386/acpi-build.c | 5 ++--- > hw/i386/pc.c | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) > >diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h >index 0dce155c8c..31de5bddbd 100644 >--- a/hw/i386/acpi-build.h >+++ b/hw/i386/acpi-build.h >@@ -2,6 +2,7 @@ > #ifndef HW_I386_ACPI_BUILD_H > #define HW_I386_ACPI_BUILD_H > #include "hw/acpi/acpi-defs.h" >+#include "hw/i386/pc.h" > > extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; > >@@ -9,7 +10,7 @@ extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; > #define ACPI_PCIHP_SEJ_BASE 0x8 > #define ACPI_PCIHP_BNMR_BASE 0x10 > >-void acpi_setup(void); >+void acpi_setup(PCMachineState *pcms); > Object *acpi_get_i386_pci_host(void); > > #endif >diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >index 6e8e32e5d2..e702d5e9d2 100644 >--- a/hw/i386/acpi-build.c >+++ b/hw/i386/acpi-build.c >@@ -2749,9 +2749,8 @@ static const VMStateDescription vmstate_acpi_build = { > }, > }; > >-void acpi_setup(void) >+void acpi_setup(PCMachineState *pcms) > { >- PCMachineState *pcms = PC_MACHINE(qdev_get_machine()); > X86MachineState *x86ms = X86_MACHINE(pcms); > AcpiBuildTables tables; > AcpiBuildState *build_state; >@@ -2771,7 +2770,7 @@ void acpi_setup(void) > return; > } > >- if (!x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) { >+ if (!x86_machine_is_acpi_enabled(x86ms)) { > ACPI_BUILD_DPRINTF("ACPI disabled. Bailing out.\n"); > return; > } >diff --git a/hw/i386/pc.c b/hw/i386/pc.c >index 6d87d1d4c2..dfc0247bb6 100644 >--- a/hw/i386/pc.c >+++ b/hw/i386/pc.c >@@ -601,7 +601,7 @@ void pc_machine_done(Notifier *notifier, void *data) > /* set the number of CPUs */ > x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus); > >- acpi_setup(); >+ acpi_setup(pcms); > if (x86ms->fw_cfg) { > fw_cfg_build_smbios(pcms, x86ms->fw_cfg, pcms->smbios_entry_point_type); > fw_cfg_build_feature_control(MACHINE(pcms), x86ms->fw_cfg);
diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h index 0dce155c8c..31de5bddbd 100644 --- a/hw/i386/acpi-build.h +++ b/hw/i386/acpi-build.h @@ -2,6 +2,7 @@ #ifndef HW_I386_ACPI_BUILD_H #define HW_I386_ACPI_BUILD_H #include "hw/acpi/acpi-defs.h" +#include "hw/i386/pc.h" extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; @@ -9,7 +10,7 @@ extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; #define ACPI_PCIHP_SEJ_BASE 0x8 #define ACPI_PCIHP_BNMR_BASE 0x10 -void acpi_setup(void); +void acpi_setup(PCMachineState *pcms); Object *acpi_get_i386_pci_host(void); #endif diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 6e8e32e5d2..e702d5e9d2 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -2749,9 +2749,8 @@ static const VMStateDescription vmstate_acpi_build = { }, }; -void acpi_setup(void) +void acpi_setup(PCMachineState *pcms) { - PCMachineState *pcms = PC_MACHINE(qdev_get_machine()); X86MachineState *x86ms = X86_MACHINE(pcms); AcpiBuildTables tables; AcpiBuildState *build_state; @@ -2771,7 +2770,7 @@ void acpi_setup(void) return; } - if (!x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) { + if (!x86_machine_is_acpi_enabled(x86ms)) { ACPI_BUILD_DPRINTF("ACPI disabled. Bailing out.\n"); return; } diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 6d87d1d4c2..dfc0247bb6 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -601,7 +601,7 @@ void pc_machine_done(Notifier *notifier, void *data) /* set the number of CPUs */ x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus); - acpi_setup(); + acpi_setup(pcms); if (x86ms->fw_cfg) { fw_cfg_build_smbios(pcms, x86ms->fw_cfg, pcms->smbios_entry_point_type); fw_cfg_build_feature_control(MACHINE(pcms), x86ms->fw_cfg);
acpi_setup() caller knows about the machine state, so pass it as argument to avoid a qdev_get_machine() call. We already resolved X86_MACHINE(pcms) as 'x86ms' so use the latter. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/i386/acpi-build.h | 3 ++- hw/i386/acpi-build.c | 5 ++--- hw/i386/pc.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)