On Mon, Oct 31, 2022 at 01:45:29PM +0100, Igor Mammedov wrote: > On Fri, 28 Oct 2022 12:34:18 +0200 > Bernhard Beschow <[email protected]> wrote: > > > The is_piix4 attribute is set once in one location and read once in > > another. Doing both in one location allows for removing the attribute > > altogether. > > we also test for piix4 in acpi_get_pm_info(), > Perhaps we should move is_piix4 to AcpiPmInfo > and reuse it in build_dsdt()? > Also should use is_piix4 as argument to build_iqcr_method() > to make its call places self documenting. But that's another patch.
Well I picked this up for pull req since i had to make a decision before freeze. But sure, a cleanup like this might even be acceptable before rc1 I think. Bernhard? > > > Signed-off-by: Bernhard Beschow <[email protected]> > > Reviewed-by: Philippe Mathieu-Daudé <[email protected]> > > Message-Id: <[email protected]> > > --- > > hw/i386/acpi-build.c | 20 ++++++-------------- > > 1 file changed, 6 insertions(+), 14 deletions(-) > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index 1ebf14b899..73d8a59737 100644 > > --- a/hw/i386/acpi-build.c > > +++ b/hw/i386/acpi-build.c > > @@ -112,7 +112,6 @@ typedef struct AcpiPmInfo { > > } AcpiPmInfo; > > > > typedef struct AcpiMiscInfo { > > - bool is_piix4; > > bool has_hpet; > > #ifdef CONFIG_TPM > > TPMVersion tpm_version; > > @@ -281,17 +280,6 @@ static void acpi_get_pm_info(MachineState *machine, > > AcpiPmInfo *pm) > > > > static void acpi_get_misc_info(AcpiMiscInfo *info) > > { > > - Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM); > > - Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE); > > - assert(!!piix != !!lpc); > > - > > - if (piix) { > > - info->is_piix4 = true; > > - } > > - if (lpc) { > > - info->is_piix4 = false; > > - } > > ack for this hunk > > > info->has_hpet = hpet_find(); > > #ifdef CONFIG_TPM > > info->tpm_version = tpm_get_version(tpm_find()); > > @@ -1334,6 +1322,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > AcpiPmInfo *pm, AcpiMiscInfo *misc, > > Range *pci_hole, Range *pci_hole64, MachineState *machine) > > { > > + Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM); > > + Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE); > > CrsRangeEntry *entry; > > Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs; > > CrsRangeSet crs_range_set; > > @@ -1354,11 +1344,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = x86ms->oem_id, > > .oem_table_id = x86ms->oem_table_id }; > > > > + assert(!!piix != !!lpc); > > + > > acpi_table_begin(&table, table_data); > > dsdt = init_aml_allocator(); > > > > build_dbg_aml(dsdt); > > - if (misc->is_piix4) { > > + if (piix) { > > sb_scope = aml_scope("_SB"); > > dev = aml_device("PCI0"); > > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); > > @@ -1371,7 +1363,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base); > > } > > build_piix4_pci0_int(dsdt); > > - } else { > > + } else if (lpc) { > > sb_scope = aml_scope("_SB"); > > dev = aml_device("PCI0"); > > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
