Message ID | 1429017160-3583-1-git-send-email-kraxel@redhat.com |
---|---|
State | New |
Headers | show |
On 14/04/2015 15:12, Gerd Hoffmann wrote: > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > hw/pci-host/q35.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c > index 79bab15..9227489 100644 > --- a/hw/pci-host/q35.c > +++ b/hw/pci-host/q35.c > @@ -268,6 +268,20 @@ static void mch_update_smram(MCHPCIState *mch) > PCIDevice *pd = PCI_DEVICE(mch); > bool h_smrame = (pd->config[MCH_HOST_BRIDGE_ESMRAMC] & MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME); > > + /* implement SMRAM.D_LCK */ > + if (pd->config[MCH_HOST_BRIDGE_SMRAM] & MCH_HOST_BRIDGE_SMRAM_D_LCK) { > + pd->config[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_D_OPEN; > + > + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_D_OPEN; > + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_D_LCK; > + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_G_SMRAME; > + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_C_BASE_SEG_MASK; > + > + pd->wmask[MCH_HOST_BRIDGE_ESMRAMC] &= ~MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME; > + pd->wmask[MCH_HOST_BRIDGE_ESMRAMC] &= ~MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_MASK; > + pd->wmask[MCH_HOST_BRIDGE_ESMRAMC] &= ~MCH_HOST_BRIDGE_ESMRAMC_T_EN; > + } > + > memory_region_transaction_begin(); > > if (pd->config[MCH_HOST_BRIDGE_SMRAM] & SMRAM_D_OPEN) { > @@ -297,7 +311,6 @@ static void mch_write_config(PCIDevice *d, > { > MCHPCIState *mch = MCH_PCI_DEVICE(d); > > - /* XXX: implement SMRAM.D_LOCK */ > pci_default_write_config(d, address, val, len); > > if (ranges_overlap(address, len, MCH_HOST_BRIDGE_PAM0, > @@ -351,6 +364,8 @@ static void mch_reset(DeviceState *qdev) > MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT); > > d->config[MCH_HOST_BRIDGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_DEFAULT; > + d->wmask[MCH_HOST_BRIDGE_SMRAM] = 0xff; > + d->wmask[MCH_HOST_BRIDGE_ESMRAMC] = 0xff; S3, if I remember correctly, should not be able to reset D_LCK. Does this do the right thing? Paolo > > mch_update(mch); > } >
On Tue, Apr 14, 2015 at 03:12:39PM +0200, Gerd Hoffmann wrote: > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > hw/pci-host/q35.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c > index 79bab15..9227489 100644 > --- a/hw/pci-host/q35.c > +++ b/hw/pci-host/q35.c > @@ -268,6 +268,20 @@ static void mch_update_smram(MCHPCIState *mch) > PCIDevice *pd = PCI_DEVICE(mch); > bool h_smrame = (pd->config[MCH_HOST_BRIDGE_ESMRAMC] & MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME); > > + /* implement SMRAM.D_LCK */ > + if (pd->config[MCH_HOST_BRIDGE_SMRAM] & MCH_HOST_BRIDGE_SMRAM_D_LCK) { > + pd->config[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_D_OPEN; > + > + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_D_OPEN; > + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_D_LCK; > + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_G_SMRAME; > + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_C_BASE_SEG_MASK; > + > + pd->wmask[MCH_HOST_BRIDGE_ESMRAMC] &= ~MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME; > + pd->wmask[MCH_HOST_BRIDGE_ESMRAMC] &= ~MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_MASK; > + pd->wmask[MCH_HOST_BRIDGE_ESMRAMC] &= ~MCH_HOST_BRIDGE_ESMRAMC_T_EN; I'd prefer a single statement: pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~(MCH_HOST_BRIDGE_SMRAM_D_OPEN | MCH_HOST_BRIDGE_SMRAM_D_LCK | ... ) > + } > + > memory_region_transaction_begin(); > > if (pd->config[MCH_HOST_BRIDGE_SMRAM] & SMRAM_D_OPEN) { > @@ -297,7 +311,6 @@ static void mch_write_config(PCIDevice *d, > { > MCHPCIState *mch = MCH_PCI_DEVICE(d); > > - /* XXX: implement SMRAM.D_LOCK */ > pci_default_write_config(d, address, val, len); > > if (ranges_overlap(address, len, MCH_HOST_BRIDGE_PAM0, > @@ -351,6 +364,8 @@ static void mch_reset(DeviceState *qdev) > MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT); > > d->config[MCH_HOST_BRIDGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_DEFAULT; > + d->wmask[MCH_HOST_BRIDGE_SMRAM] = 0xff; Is this right? I see a bunch of reserved bits etc there. > + d->wmask[MCH_HOST_BRIDGE_ESMRAMC] = 0xff; Doesn't this mean we need to reset this register now? > > mch_update(mch); > } Don't you also need to clear D_LCK? > -- > 1.8.3.1
On Tue, Apr 14, 2015 at 05:41:14PM +0200, Michael S. Tsirkin wrote: > On Tue, Apr 14, 2015 at 03:12:39PM +0200, Gerd Hoffmann wrote: > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > > --- > > hw/pci-host/q35.c | 17 ++++++++++++++++- > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c > > index 79bab15..9227489 100644 > > --- a/hw/pci-host/q35.c > > +++ b/hw/pci-host/q35.c > > @@ -268,6 +268,20 @@ static void mch_update_smram(MCHPCIState *mch) > > PCIDevice *pd = PCI_DEVICE(mch); > > bool h_smrame = (pd->config[MCH_HOST_BRIDGE_ESMRAMC] & MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME); > > > > + /* implement SMRAM.D_LCK */ > > + if (pd->config[MCH_HOST_BRIDGE_SMRAM] & MCH_HOST_BRIDGE_SMRAM_D_LCK) { > > + pd->config[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_D_OPEN; > > + > > + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_D_OPEN; > > + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_D_LCK; > > + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_G_SMRAME; > > + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_C_BASE_SEG_MASK; > > + > > + pd->wmask[MCH_HOST_BRIDGE_ESMRAMC] &= ~MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME; > > + pd->wmask[MCH_HOST_BRIDGE_ESMRAMC] &= ~MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_MASK; > > + pd->wmask[MCH_HOST_BRIDGE_ESMRAMC] &= ~MCH_HOST_BRIDGE_ESMRAMC_T_EN; > > > I'd prefer a single statement: > pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= > ~(MCH_HOST_BRIDGE_SMRAM_D_OPEN | MCH_HOST_BRIDGE_SMRAM_D_LCK | ... ) > > > + } > > + > > memory_region_transaction_begin(); > > > > if (pd->config[MCH_HOST_BRIDGE_SMRAM] & SMRAM_D_OPEN) { > > @@ -297,7 +311,6 @@ static void mch_write_config(PCIDevice *d, > > { > > MCHPCIState *mch = MCH_PCI_DEVICE(d); > > > > - /* XXX: implement SMRAM.D_LOCK */ > > pci_default_write_config(d, address, val, len); > > > > if (ranges_overlap(address, len, MCH_HOST_BRIDGE_PAM0, > > @@ -351,6 +364,8 @@ static void mch_reset(DeviceState *qdev) > > MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT); > > > > d->config[MCH_HOST_BRIDGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_DEFAULT; > > + d->wmask[MCH_HOST_BRIDGE_SMRAM] = 0xff; > > Is this right? I see a bunch of reserved bits etc there. > > > > + d->wmask[MCH_HOST_BRIDGE_ESMRAMC] = 0xff; And this mask seems not to match the spec, either. > Doesn't this mean we need to reset this register now? > > > > > mch_update(mch); > > } > > Don't you also need to clear D_LCK? > > > -- > > 1.8.3.1
On Di, 2015-04-14 at 16:35 +0200, Paolo Bonzini wrote: > > On 14/04/2015 15:12, Gerd Hoffmann wrote: > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > > --- > > hw/pci-host/q35.c | 17 ++++++++++++++++- > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c > > index 79bab15..9227489 100644 > > --- a/hw/pci-host/q35.c > > +++ b/hw/pci-host/q35.c > > @@ -268,6 +268,20 @@ static void mch_update_smram(MCHPCIState *mch) > > PCIDevice *pd = PCI_DEVICE(mch); > > bool h_smrame = (pd->config[MCH_HOST_BRIDGE_ESMRAMC] & MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME); > > > > + /* implement SMRAM.D_LCK */ > > + if (pd->config[MCH_HOST_BRIDGE_SMRAM] & MCH_HOST_BRIDGE_SMRAM_D_LCK) { > > + pd->config[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_D_OPEN; > > + > > + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_D_OPEN; > > + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_D_LCK; > > + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_G_SMRAME; > > + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_C_BASE_SEG_MASK; > > + > > + pd->wmask[MCH_HOST_BRIDGE_ESMRAMC] &= ~MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME; > > + pd->wmask[MCH_HOST_BRIDGE_ESMRAMC] &= ~MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_MASK; > > + pd->wmask[MCH_HOST_BRIDGE_ESMRAMC] &= ~MCH_HOST_BRIDGE_ESMRAMC_T_EN; > > + } > > + > > memory_region_transaction_begin(); > > > > if (pd->config[MCH_HOST_BRIDGE_SMRAM] & SMRAM_D_OPEN) { > > @@ -297,7 +311,6 @@ static void mch_write_config(PCIDevice *d, > > { > > MCHPCIState *mch = MCH_PCI_DEVICE(d); > > > > - /* XXX: implement SMRAM.D_LOCK */ > > pci_default_write_config(d, address, val, len); > > > > if (ranges_overlap(address, len, MCH_HOST_BRIDGE_PAM0, > > @@ -351,6 +364,8 @@ static void mch_reset(DeviceState *qdev) > > MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT); > > > > d->config[MCH_HOST_BRIDGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_DEFAULT; > > + d->wmask[MCH_HOST_BRIDGE_SMRAM] = 0xff; > > + d->wmask[MCH_HOST_BRIDGE_ESMRAMC] = 0xff; > > S3, if I remember correctly, should not be able to reset D_LCK. The "A Tour Beyond BIOS Implementing S3 Resume with EDKII" white paper lists "Lock SMM. This must be done to maintain SMM integrity." as todo list item for the edk2 resume code path (page 18). So it seems to me it is the job of the firmware to re-lock smm after S3 (and before handing control back to the OS, obviously). > Does > this do the right thing? I think so ... cheers, Gerd
Hi, > > d->config[MCH_HOST_BRIDGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_DEFAULT; > > + d->wmask[MCH_HOST_BRIDGE_SMRAM] = 0xff; > > Is this right? I see a bunch of reserved bits etc there. Restores the state we had before the guest flipped the lock bit. Entriely possible that we should have a non-0xff wmask in the first place, I'll look into that, but it's unrelated to lock bit handling and thus something for another patch. > > + d->wmask[MCH_HOST_BRIDGE_ESMRAMC] = 0xff; > > Doesn't this mean we need to reset this register now? Again, this is something not related to the lock bit implementation, probably the patch adding esramc support should have added this too. I'll have a look, probably will cook up a incremental fix paolo can squash in then. > > > > mch_update(mch); > > } > > Don't you also need to clear D_LCK? Setting MCH_HOST_BRIDGE_SMRAM to MCH_HOST_BRIDGE_SMRAM_DEFAULT does that. Also see 2/2 with the test case which shows lock+unlock works correctly. cheers, Gerd
On Wed, Apr 15, 2015 at 04:12:00PM +0200, Gerd Hoffmann wrote: > Hi, > > > > d->config[MCH_HOST_BRIDGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_DEFAULT; > > > + d->wmask[MCH_HOST_BRIDGE_SMRAM] = 0xff; > > > > Is this right? I see a bunch of reserved bits etc there. > > Restores the state we had before the guest flipped the lock bit. > > Entriely possible that we should have a non-0xff wmask in the first > place, I'll look into that, but it's unrelated to lock bit handling and > thus something for another patch. > > > + d->wmask[MCH_HOST_BRIDGE_ESMRAMC] = 0xff; > > > > Doesn't this mean we need to reset this register now? > > Again, this is something not related to the lock bit implementation, > probably the patch adding esramc support should have added this too. > > I'll have a look, probably will cook up a incremental fix paolo can > squash in then. I'd prefer a complete patch to review. Let's just set the correct wmask values directly. > > > > > > > mch_update(mch); > > > } > > > > Don't you also need to clear D_LCK? > > Setting MCH_HOST_BRIDGE_SMRAM to MCH_HOST_BRIDGE_SMRAM_DEFAULT does > that. > > Also see 2/2 with the test case which shows lock+unlock works correctly. > > cheers, > Gerd >
On 15/04/2015 15:58, Gerd Hoffmann wrote: > The "A Tour Beyond BIOS Implementing S3 Resume with EDKII" white paper > lists "Lock SMM. This must be done to maintain SMM integrity." as todo > list item for the edk2 resume code path (page 18). > > So it seems to me it is the job of the firmware to re-lock smm after S3 > (and before handing control back to the OS, obviously). Right, however "D_LCK can be set to 1 via a normal configuration space write but can only be cleared by a Full Reset." A Full Reset is the PLTRST# pin, which is asserted by the south bridge during power-up and during CF9h reset. S3 doesn't seem to be included. My reading of the EDK2 whitepaper is that this may vary for other chipsets. Paolo
On 04/16/15 10:12, Paolo Bonzini wrote: > > > On 15/04/2015 15:58, Gerd Hoffmann wrote: >> The "A Tour Beyond BIOS Implementing S3 Resume with EDKII" white >> paper lists "Lock SMM. This must be done to maintain SMM integrity." >> as todo list item for the edk2 resume code path (page 18). let's mark this with (*) for the end of my email >> So it seems to me it is the job of the firmware to re-lock smm after >> S3 (and before handing control back to the OS, obviously). > > Right, however "D_LCK can be set to 1 via a normal configuration space > write but can only be cleared by a Full Reset." A Full Reset is the > PLTRST# pin, which is asserted by the south bridge during power-up and > during CF9h reset. S3 doesn't seem to be included. > > My reading of the EDK2 whitepaper is that this may vary for other > chipsets. Page 25, "PEI Instance", option "2)" applies: the S3 resume PEI module needs to be able to open SMRAM. From the OVMF whitepaper <http://people.redhat.com/~lersek/ovmf-whitepaper-c770f8c.txt>: > (b) In PEI, the S3 Resume PEIM (UefiCpuPkg/Universal/Acpi/S3Resume2Pei) > retrieves data from the LockBox. > > Presumably, S3Resume2Pei should be considered an "unprivileged PEIM", > and the SMRAM access should be layered as seen in DXE. Unfortunately, > edk2 does not implement all of the layers in PEI -- the code either > doesn't exist, or it is not open source: > > role | DXE: protocol/module | PEI: PPI/module > -------------+--------------------------------+------------------------------ > unprivileged | any | S3Resume2Pei.inf > driver | | > -------------+--------------------------------+------------------------------ > command | LIBRARY_CLASS = LockBoxLib | LIBRARY_CLASS = LockBoxLib > formatting | | > and response | SmmLockBoxDxeLib.inf | SmmLockBoxPeiLib.inf > parsing | | > -------------+--------------------------------+------------------------------ > privilege | EFI_SMM_COMMUNICATION_PROTOCOL | EFI_PEI_SMM_COMMUNICATION_PPI > separation | | > | PiSmmCore.inf | missing! > -------------+--------------------------------+------------------------------ > platform SMM | EFI_SMM_CONTROL2_PROTOCOL | PEI_SMM_CONTROL_PPI > and SMRAM | EFI_SMM_ACCESS2_PROTOCOL | PEI_SMM_ACCESS_PPI > access | | > | to be done in OVMF | to be done in OVMF > -------------+--------------------------------+------------------------------ > command | LIBRARY_CLASS = LockBoxLib | LIBRARY_CLASS = LockBoxLib > parsing and | | > response | SmmLockBoxSmmLib.inf | missing! > formatting | | > -------------+--------------------------------+------------------------------ > privileged | SmmLockBox.inf | missing! > LockBox | | > driver | | > > Alternatively, in the future OVMF might be able to provide a LockBoxLib > instance (an SmmLockBoxPeiLib substitute) for S3Resume2Pei that > accesses SMRAM directly, eliminating the need for deeper layers in the > stack (that is, EFI_PEI_SMM_COMMUNICATION_PPI and deeper). > > In fact, a "thin" EFI_PEI_SMM_COMMUNICATION_PPI implementation whose > sole Communicate() member invariably returns EFI_NOT_STARTED would > cause the current SmmLockBoxPeiLib library instance to directly perform > full-depth SMRAM access and LockBox search, obviating the "missing" > cells. (With reference to A Tour Beyond BIOS: Implementing S3 Resume > with EDK2, by Jiewen Yao and Vincent Zimmer, October 2014.) The code flow in edk2 corresponding to the last paragraph above is: S3RestoreConfig2() [UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c] RestoreLockBox() [MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c] SmmCommunicationPpi->Communicate() [1] InternalRestoreLockBoxFromSmram() [MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c] SmmAccess->Open() [2] [1] EFI_PEI_SMM_COMMUNICATION_PPI, to be implemented in OVMF; returns EFI_NOT_STARTED (see the reasoning above, in both whitepapers) [2] PEI_SMM_ACCESS_PPI, to be implemented in OVMF The PEI_SMM_ACCESS_PPI.Open() member, called on the last line above, runs on S3 resume, and it needs to be able to open up SMRAM. If the lock is still in place, that won't work. Additionally, the following language from page 18 in the Intel whitepaper, marked above as (*): o Lock SMM. This must be done to maintain SMM integrity. is not a TODO list item; it is a responsibility that the S3Resume2Pei module already covers. See S3ResumeExecuteBootScript() [UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c] SmmAccess->Close() [2] SmmAccess->Lock() [2] So, the hardware should clear the lock on S3 resume, then edk2 will open and close the SMRAM regions as appropriate, and it will also re-lock SMRAM, before (a) it executes the S3 boot script (which might contain opcodes from UEFI (ie. third party) drivers), (b) it transfers control to the OS's wakeup vector. (In our case the S3 boot script has only one entry (an informational no-op) anyway; the main assurance is that the OS's wakeup vector is reached with the SMRAM re-locked.) Thanks! Laszlo
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index 79bab15..9227489 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -268,6 +268,20 @@ static void mch_update_smram(MCHPCIState *mch) PCIDevice *pd = PCI_DEVICE(mch); bool h_smrame = (pd->config[MCH_HOST_BRIDGE_ESMRAMC] & MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME); + /* implement SMRAM.D_LCK */ + if (pd->config[MCH_HOST_BRIDGE_SMRAM] & MCH_HOST_BRIDGE_SMRAM_D_LCK) { + pd->config[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_D_OPEN; + + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_D_OPEN; + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_D_LCK; + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_G_SMRAME; + pd->wmask[MCH_HOST_BRIDGE_SMRAM] &= ~MCH_HOST_BRIDGE_SMRAM_C_BASE_SEG_MASK; + + pd->wmask[MCH_HOST_BRIDGE_ESMRAMC] &= ~MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME; + pd->wmask[MCH_HOST_BRIDGE_ESMRAMC] &= ~MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_MASK; + pd->wmask[MCH_HOST_BRIDGE_ESMRAMC] &= ~MCH_HOST_BRIDGE_ESMRAMC_T_EN; + } + memory_region_transaction_begin(); if (pd->config[MCH_HOST_BRIDGE_SMRAM] & SMRAM_D_OPEN) { @@ -297,7 +311,6 @@ static void mch_write_config(PCIDevice *d, { MCHPCIState *mch = MCH_PCI_DEVICE(d); - /* XXX: implement SMRAM.D_LOCK */ pci_default_write_config(d, address, val, len); if (ranges_overlap(address, len, MCH_HOST_BRIDGE_PAM0, @@ -351,6 +364,8 @@ static void mch_reset(DeviceState *qdev) MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT); d->config[MCH_HOST_BRIDGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_DEFAULT; + d->wmask[MCH_HOST_BRIDGE_SMRAM] = 0xff; + d->wmask[MCH_HOST_BRIDGE_ESMRAMC] = 0xff; mch_update(mch); }
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- hw/pci-host/q35.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)