mch_realize has a large section that deals with initializing the SMRAM-specific MemoryRegions. Currently we do an early return from the realize function if mch->has_smm_ranges is false, but this has the potential for bugs if somebody adds new code at the end of the function that isn't SMM-specific. Pull the MR init code out into its own function, so we can do the smm-ranges specific handling in the realize function in a more obvious way.
This commit shouldn't change behaviour at all. Signed-off-by: Peter Maydell <[email protected]> --- hw/pci-host/q35.c | 91 ++++++++++++++++++++++++----------------------- 1 file changed, 47 insertions(+), 44 deletions(-) diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index 47e726c41d..e42b84b978 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -572,49 +572,9 @@ static void mch_reset(DeviceState *qdev) mch_update(mch); } -static void mch_realize(PCIDevice *d, Error **errp) +static void mch_init_smram_regions(MCHPCIState *mch) { - int i; - MCHPCIState *mch = MCH_PCI_DEVICE(d); - - if (mch->ext_tseg_mbytes > MCH_HOST_BRIDGE_EXT_TSEG_MBYTES_MAX) { - error_setg(errp, "invalid extended-tseg-mbytes value: %" PRIu16, - mch->ext_tseg_mbytes); - return; - } - - /* setup pci memory mapping */ - pc_pci_as_mapping_init(mch->system_memory, mch->pci_address_space); - - /* PAM */ - init_pam(&mch->pam_regions[0], OBJECT(mch), mch->ram_memory, - mch->system_memory, mch->pci_address_space, - PAM_BIOS_BASE, PAM_BIOS_SIZE); - for (i = 0; i < ARRAY_SIZE(mch->pam_regions) - 1; ++i) { - init_pam(&mch->pam_regions[i + 1], OBJECT(mch), mch->ram_memory, - mch->system_memory, mch->pci_address_space, - PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE); - } - - /* - * This memory region looks like it's SMM specific, but it is not. - * It's an alias that makes the pci_address_space appear in system - * memory at the SMRAM_C_BASE address. The alias is enabled when the - * CPU should not see SMRAM, and *disabled* when the low SMRAM should be - * visible. So for non-SMM configs we need to create the alias, and - * leave it permanently enabled. - */ - memory_region_init_alias(&mch->smram_region, OBJECT(mch), "smram-region", - mch->pci_address_space, MCH_HOST_BRIDGE_SMRAM_C_BASE, - MCH_HOST_BRIDGE_SMRAM_C_SIZE); - memory_region_add_subregion_overlap(mch->system_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE, - &mch->smram_region, 1); - memory_region_set_enabled(&mch->smram_region, true); - - if (!mch->has_smm_ranges) { - return; - } - + /* Initialize all the SMRAM specific MemoryRegions */ memory_region_init_alias(&mch->open_high_smram, OBJECT(mch), "smram-open-high", mch->ram_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE, MCH_HOST_BRIDGE_SMRAM_C_SIZE); @@ -670,9 +630,52 @@ static void mch_realize(PCIDevice *d, Error **errp) memory_region_set_enabled(&mch->smbase_window, false); memory_region_add_subregion(&mch->smram, MCH_HOST_BRIDGE_SMBASE_ADDR, &mch->smbase_window); +} - object_property_add_const_link(qdev_get_machine(), "smram", - OBJECT(&mch->smram)); +static void mch_realize(PCIDevice *d, Error **errp) +{ + int i; + MCHPCIState *mch = MCH_PCI_DEVICE(d); + + if (mch->ext_tseg_mbytes > MCH_HOST_BRIDGE_EXT_TSEG_MBYTES_MAX) { + error_setg(errp, "invalid extended-tseg-mbytes value: %" PRIu16, + mch->ext_tseg_mbytes); + return; + } + + /* setup pci memory mapping */ + pc_pci_as_mapping_init(mch->system_memory, mch->pci_address_space); + + /* PAM */ + init_pam(&mch->pam_regions[0], OBJECT(mch), mch->ram_memory, + mch->system_memory, mch->pci_address_space, + PAM_BIOS_BASE, PAM_BIOS_SIZE); + for (i = 0; i < ARRAY_SIZE(mch->pam_regions) - 1; ++i) { + init_pam(&mch->pam_regions[i + 1], OBJECT(mch), mch->ram_memory, + mch->system_memory, mch->pci_address_space, + PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE); + } + + /* + * This memory region looks like it's SMM specific, but it is not. + * It's an alias that makes the pci_address_space appear in system + * memory at the SMRAM_C_BASE address. The alias is enabled when the + * CPU should not see SMRAM, and *disabled* when the low SMRAM should be + * visible. So for non-SMM configs we need to create the alias, and + * leave it permanently enabled. + */ + memory_region_init_alias(&mch->smram_region, OBJECT(mch), "smram-region", + mch->pci_address_space, MCH_HOST_BRIDGE_SMRAM_C_BASE, + MCH_HOST_BRIDGE_SMRAM_C_SIZE); + memory_region_add_subregion_overlap(mch->system_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE, + &mch->smram_region, 1); + memory_region_set_enabled(&mch->smram_region, true); + + if (mch->has_smm_ranges) { + mch_init_smram_regions(mch); + object_property_add_const_link(qdev_get_machine(), "smram", + OBJECT(&mch->smram)); + } } static const Property mch_props[] = { -- 2.43.0
