In mch_write_config() we return early if has_smm_ranges is false. This is slightly bug-prone because it leaves the door open to somebody later adding non-SMM-specific code at the bottom of the function.
This case isn't as bad as the one in realize, because the function is a lot shorter. But putting the handling of the three SMM specific ranges into an if() rather than having an early return seems better. Signed-off-by: Peter Maydell <[email protected]> --- hw/pci-host/q35.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index e42b84b978..f4556ad03a 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -485,22 +485,20 @@ static void mch_write_config(PCIDevice *d, mch_update_pciexbar(mch); } - if (!mch->has_smm_ranges) { - return; - } + if (mch->has_smm_ranges) { + if (ranges_overlap(address, len, MCH_HOST_BRIDGE_SMRAM, + MCH_HOST_BRIDGE_SMRAM_SIZE)) { + mch_update_smram(mch); + } - if (ranges_overlap(address, len, MCH_HOST_BRIDGE_SMRAM, - MCH_HOST_BRIDGE_SMRAM_SIZE)) { - mch_update_smram(mch); - } + if (ranges_overlap(address, len, MCH_HOST_BRIDGE_EXT_TSEG_MBYTES, + MCH_HOST_BRIDGE_EXT_TSEG_MBYTES_SIZE)) { + mch_update_ext_tseg_mbytes(mch); + } - if (ranges_overlap(address, len, MCH_HOST_BRIDGE_EXT_TSEG_MBYTES, - MCH_HOST_BRIDGE_EXT_TSEG_MBYTES_SIZE)) { - mch_update_ext_tseg_mbytes(mch); - } - - if (ranges_overlap(address, len, MCH_HOST_BRIDGE_F_SMBASE, 1)) { - mch_update_smbase_smram(mch); + if (ranges_overlap(address, len, MCH_HOST_BRIDGE_F_SMBASE, 1)) { + mch_update_smbase_smram(mch); + } } } -- 2.43.0
