@samitouri / QOSamiQemu / commits / a90fcf0080

hw/pci-host/q35.c: Factor out creation of SMRAM MRs

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 <peter.maydell@linaro.org> Reviewed-by: Michael Tokarev <mjt@tls.msk.ru> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Message-ID: <20260708121011.1653365-3-peter.maydell@linaro.org>

Peter Maydell committed Jul 8, 2026 at 13:10 UTC a90fcf0080da2bd372f0a79dc243d8d2ede1b244
1 file changed +47 -44
hw/pci-host/q35.c
+47 -44
@@ -572,49 +572,9 @@ static void mch_reset(DeviceState *qdev)
572 mch_update(mch);
573 }
574
575 -static void mch_realize(PCIDevice *d, Error **errp)
575 +static void mch_init_smram_regions(MCHPCIState *mch)
576 {
577 - int i;
578 - MCHPCIState *mch = MCH_PCI_DEVICE(d);
579 -
580 - if (mch->ext_tseg_mbytes > MCH_HOST_BRIDGE_EXT_TSEG_MBYTES_MAX) {
581 - error_setg(errp, "invalid extended-tseg-mbytes value: %" PRIu16,
582 - mch->ext_tseg_mbytes);
583 - return;
584 - }
585 -
586 - /* setup pci memory mapping */
587 - pc_pci_as_mapping_init(mch->system_memory, mch->pci_address_space);
588 -
589 - /* PAM */
590 - init_pam(&mch->pam_regions[0], OBJECT(mch), mch->ram_memory,
591 - mch->system_memory, mch->pci_address_space,
592 - PAM_BIOS_BASE, PAM_BIOS_SIZE);
593 - for (i = 0; i < ARRAY_SIZE(mch->pam_regions) - 1; ++i) {
594 - init_pam(&mch->pam_regions[i + 1], OBJECT(mch), mch->ram_memory,
595 - mch->system_memory, mch->pci_address_space,
596 - PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE);
597 - }
598 -
599 - /*
600 - * This memory region looks like it's SMM specific, but it is not.
601 - * It's an alias that makes the pci_address_space appear in system
602 - * memory at the SMRAM_C_BASE address. The alias is enabled when the
603 - * CPU should not see SMRAM, and *disabled* when the low SMRAM should be
604 - * visible. So for non-SMM configs we need to create the alias, and
605 - * leave it permanently enabled.
606 - */
607 - memory_region_init_alias(&mch->smram_region, OBJECT(mch), "smram-region",
608 - mch->pci_address_space, MCH_HOST_BRIDGE_SMRAM_C_BASE,
609 - MCH_HOST_BRIDGE_SMRAM_C_SIZE);
610 - memory_region_add_subregion_overlap(mch->system_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
611 - &mch->smram_region, 1);
612 - memory_region_set_enabled(&mch->smram_region, true);
613 -
614 - if (!mch->has_smm_ranges) {
615 - return;
616 - }
617 -
577 + /* Initialize all the SMRAM specific MemoryRegions */
578 memory_region_init_alias(&mch->open_high_smram, OBJECT(mch), "smram-open-high",
579 mch->ram_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
580 MCH_HOST_BRIDGE_SMRAM_C_SIZE);
@@ -670,9 +630,52 @@ static void mch_realize(PCIDevice *d, Error **errp)
630 memory_region_set_enabled(&mch->smbase_window, false);
631 memory_region_add_subregion(&mch->smram, MCH_HOST_BRIDGE_SMBASE_ADDR,
632 &mch->smbase_window);
633 +}
634 +
635 +static void mch_realize(PCIDevice *d, Error **errp)
636 +{
637 + int i;
638 + MCHPCIState *mch = MCH_PCI_DEVICE(d);
639 +
640 + if (mch->ext_tseg_mbytes > MCH_HOST_BRIDGE_EXT_TSEG_MBYTES_MAX) {
641 + error_setg(errp, "invalid extended-tseg-mbytes value: %" PRIu16,
642 + mch->ext_tseg_mbytes);
643 + return;
644 + }
645 +
646 + /* setup pci memory mapping */
647 + pc_pci_as_mapping_init(mch->system_memory, mch->pci_address_space);
648 +
649 + /* PAM */
650 + init_pam(&mch->pam_regions[0], OBJECT(mch), mch->ram_memory,
651 + mch->system_memory, mch->pci_address_space,
652 + PAM_BIOS_BASE, PAM_BIOS_SIZE);
653 + for (i = 0; i < ARRAY_SIZE(mch->pam_regions) - 1; ++i) {
654 + init_pam(&mch->pam_regions[i + 1], OBJECT(mch), mch->ram_memory,
655 + mch->system_memory, mch->pci_address_space,
656 + PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE);
657 + }
658
674 - object_property_add_const_link(qdev_get_machine(), "smram",
675 - OBJECT(&mch->smram));
659 + /*
660 + * This memory region looks like it's SMM specific, but it is not.
661 + * It's an alias that makes the pci_address_space appear in system
662 + * memory at the SMRAM_C_BASE address. The alias is enabled when the
663 + * CPU should not see SMRAM, and *disabled* when the low SMRAM should be
664 + * visible. So for non-SMM configs we need to create the alias, and
665 + * leave it permanently enabled.
666 + */
667 + memory_region_init_alias(&mch->smram_region, OBJECT(mch), "smram-region",
668 + mch->pci_address_space, MCH_HOST_BRIDGE_SMRAM_C_BASE,
669 + MCH_HOST_BRIDGE_SMRAM_C_SIZE);
670 + memory_region_add_subregion_overlap(mch->system_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
671 + &mch->smram_region, 1);
672 + memory_region_set_enabled(&mch->smram_region, true);
673 +
674 + if (mch->has_smm_ranges) {
675 + mch_init_smram_regions(mch);
676 + object_property_add_const_link(qdev_get_machine(), "smram",
677 + OBJECT(&mch->smram));
678 + }
679 }
680
681 static const Property mch_props[] = {