From: Peter Maydell <[email protected]> The MCHPCIState::smram_region looks like it ought to be SMM-specific, but it isn't, because its behaviour is "alias the PCI address space into system memory at the SMRAM_C_BASE offset", and it must be enabled for "hide SMRAM", and disabled for "show SMRAM". If the SMRAM regions are disabled, we want "hide SMRAM", so we need to initialize and place this MR. Do this in the minimal way, by moving the "bail out of realize if has_smm_ranges is false" check down below the initialization code.
This fixes a bug where disabling SMM causes the VGA screen to be blank during seabios output, until the OS graphics driver is initialized. This is most obvious for accelerators which have no SMM support (e.g. NVMM, HVF, WHPX) as there smm=off is the default, but you can also see it on KVM and TCG if you explicitly pass smm=off: qemu-system-x86_64 -machine q35,accel=kvm,smm=off The early return is bug-prone, so we can refactor the code to clean it up, but this is the minimal bug fix for backports, and is what Debian used to work around this: https://salsa.debian.org/qemu-team/qemu/-/commit/6e0766f0f897dc2b75ab87dd59da0d4639bb37ee Another proposed fix for this: https://patchew.org/QEMU/[email protected]/ also moves an early return in mch_update-smram() and tweaks mch_update_smram() accordingly. This shouldn't be necessary, because in the no-SMM case smram_region should always be enabled and we don't want to allow the guest to make it disabled. NetBSD bug: https://gnats.NetBSD.org/59721 Cc: [email protected] Fixes: b07bf7b7 ("q35: Introduce smm_ranges property for q35-pci-host") Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2608 Signed-off-by: Peter Maydell <[email protected]> Signed-off-by: Michael Tokarev <[email protected]> Reviewed-by: Michael S. Tsirkin <[email protected]> Signed-off-by: Michael S. Tsirkin <[email protected]> Message-ID: <[email protected]> --- hw/pci-host/q35.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index 4784b8f59b..47e726c41d 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -596,11 +596,14 @@ static void mch_realize(PCIDevice *d, Error **errp) PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE); } - if (!mch->has_smm_ranges) { - return; - } - - /* if *disabled* show SMRAM to all CPUs */ + /* + * 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); @@ -608,6 +611,10 @@ static void mch_realize(PCIDevice *d, Error **errp) &mch->smram_region, 1); memory_region_set_enabled(&mch->smram_region, true); + if (!mch->has_smm_ranges) { + return; + } + 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); -- MST
