Re: [flashrom] [PATCH 5/7] warn if SMM BIOS Write Protection is detected in BIOS_CNTL
On 5/23/11 4:23 PM, Stefan Tauner wrote:
Signed-off-by: Stefan Tauner<stefan.tauner@student.tuwien.ac.at> --- chipset_enable.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/chipset_enable.c b/chipset_enable.c index 83b49ad..339c6bb 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -264,8 +264,18 @@ static int enable_flash_ich(struct pci_dev *dev, const char *name, (old& (1<< 0)) ? "en" : "dis"); msg_pdbg("BIOS_CNTL is 0x%x\n", old);
- new = old | 1; + /* + * Quote from the 6 Series datasheet: + * "5: SMM BIOS Write Protect Disable (SMM_BWP) + * 1 = BIOS region SMM protection is enabled. + * The BIOS Region is not writable unless all processors are in SMM." + * In earlier chipsets this bit is reserved. */ + if (old& (5<< 1)) { + msg_pinfo("WARNING: BIOS region SMM protection is enabled!\n"); + return -1; You might still be successful doing the write, in case the SMM handler does not enforce the protection, so maybe you should just print a warning but not return here?
+ }
+ new = old | 1; if (new == old) return 0;
On Tue, 24 May 2011 18:15:52 -0700 Stefan Reinauer <stefan.reinauer@coreboot.org> wrote:
+ if (old& (5<< 1)) { + msg_pinfo("WARNING: BIOS region SMM protection is enabled!\n"); + return -1; You might still be successful doing the write, in case the SMM handler does not enforce the protection, so maybe you should just print a warning but not return here?
ok also: 1<<5 of course! thanks joshua for reporting this -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
On Tue, 24 May 2011 18:15:52 -0700 Stefan Reinauer <stefan.reinauer@coreboot.org> wrote:
On 5/23/11 4:23 PM, Stefan Tauner wrote:
Signed-off-by: Stefan Tauner<stefan.tauner@student.tuwien.ac.at> --- chipset_enable.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/chipset_enable.c b/chipset_enable.c index 83b49ad..339c6bb 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -264,8 +264,18 @@ static int enable_flash_ich(struct pci_dev *dev, const char *name, (old& (1<< 0)) ? "en" : "dis"); msg_pdbg("BIOS_CNTL is 0x%x\n", old);
- new = old | 1; + /* + * Quote from the 6 Series datasheet: + * "5: SMM BIOS Write Protect Disable (SMM_BWP) + * 1 = BIOS region SMM protection is enabled. + * The BIOS Region is not writable unless all processors are in SMM." + * In earlier chipsets this bit is reserved. */ + if (old& (5<< 1)) { + msg_pinfo("WARNING: BIOS region SMM protection is enabled!\n"); + return -1; You might still be successful doing the write, in case the SMM handler does not enforce the protection, so maybe you should just print a warning but not return here?
in chromium-os you are trying to unset that bit[1], but according to the data sheet this is impossible - it is R/W LO (read/write lock once). and you degraded the warning to dbg level... certainly not suited for upstream, but maybe desirable for chromium(?). have you tested this on a board where SMM_BWP is really set to 1? we may wanna try to write it anyway, but it would be far more interesting if it really works on some chipsets :) 1: http://git.chromium.org/gitweb/?p=chromiumos/third_party/flashrom.git;a=comm... -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
* Stefan Tauner <stefan.tauner@student.tuwien.ac.at> [111103 12:04]:
- new = old | 1; + /* + * Quote from the 6 Series datasheet: + * "5: SMM BIOS Write Protect Disable (SMM_BWP) + * 1 = BIOS region SMM protection is enabled. + * The BIOS Region is not writable unless all processors are in SMM." + * In earlier chipsets this bit is reserved. */ + if (old& (5<< 1)) { + msg_pinfo("WARNING: BIOS region SMM protection is enabled!\n"); + return -1; You might still be successful doing the write, in case the SMM handler does not enforce the protection, so maybe you should just print a warning but not return here?
in chromium-os you are trying to unset that bit[1], but according to the data sheet this is impossible - it is R/W LO (read/write lock once).
Maybe there is some confusion about R/W LO. The bit can be locked, but that does not mean it is locked automatically by writing / clearing it. Once it is locked, the lock can not be undone except by a chipset reset.
and you degraded the warning to dbg level... certainly not suited for upstream, but maybe desirable for chromium(?).
Yes, that was done on purpose, because the lock bit that prevents the bit from being cleared also produces a warning.
have you tested this on a board where SMM_BWP is really set to 1? we may wanna try to write it anyway, but it would be far more interesting if it really works on some chipsets :)
Yes, this was implemented to fix a problem I was seeing, and it solved the problem. :-) Stefan
On Thu, 3 Nov 2011 19:08:37 +0100 Stefan Reinauer <stefan.reinauer@coreboot.org> wrote:
* Stefan Tauner <stefan.tauner@student.tuwien.ac.at> [111103 12:04]:
- new = old | 1; + /* + * Quote from the 6 Series datasheet: + * "5: SMM BIOS Write Protect Disable (SMM_BWP) + * 1 = BIOS region SMM protection is enabled. + * The BIOS Region is not writable unless all processors are in SMM." + * In earlier chipsets this bit is reserved. */ + if (old& (5<< 1)) { + msg_pinfo("WARNING: BIOS region SMM protection is enabled!\n"); + return -1; You might still be successful doing the write, in case the SMM handler does not enforce the protection, so maybe you should just print a warning but not return here?
in chromium-os you are trying to unset that bit[1], but according to the data sheet this is impossible - it is R/W LO (read/write lock once).
Maybe there is some confusion about R/W LO. The bit can be locked, but that does not mean it is locked automatically by writing / clearing it. Once it is locked, the lock can not be undone except by a chipset reset.
hm. quote 6 series datasheet: "R/WLO Read/Write, Lock-Once. A register bit with this attribute can be written to the non-locked value multiple times, but to the locked value only once. After the locked value has been written, the bit becomes read only."
and you degraded the warning to dbg level... certainly not suited for upstream, but maybe desirable for chromium(?).
Yes, that was done on purpose, because the lock bit that prevents the bit from being cleared also produces a warning.
and that is? BLE? i do not interpret the public datasheet like that, but...
have you tested this on a board where SMM_BWP is really set to 1? we may wanna try to write it anyway, but it would be far more interesting if it really works on some chipsets :)
Yes, this was implemented to fix a problem I was seeing, and it solved the problem. :-)
hm ok, we should get this upstream then. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
On Fri, 4 Nov 2011 00:38:34 +0100 Stefan Tauner <stefan.tauner@student.tuwien.ac.at> wrote:
On Thu, 3 Nov 2011 19:08:37 +0100 Stefan Reinauer <stefan.reinauer@coreboot.org> wrote:
* Stefan Tauner <stefan.tauner@student.tuwien.ac.at> [111103 12:04]:
- new = old | 1; + /* + * Quote from the 6 Series datasheet: + * "5: SMM BIOS Write Protect Disable (SMM_BWP) + * 1 = BIOS region SMM protection is enabled. + * The BIOS Region is not writable unless all processors are in SMM." + * In earlier chipsets this bit is reserved. */ + if (old& (5<< 1)) { + msg_pinfo("WARNING: BIOS region SMM protection is enabled!\n"); + return -1; You might still be successful doing the write, in case the SMM handler does not enforce the protection, so maybe you should just print a warning but not return here?
in chromium-os you are trying to unset that bit[1], but according to the data sheet this is impossible - it is R/W LO (read/write lock once).
Maybe there is some confusion about R/W LO. The bit can be locked, but that does not mean it is locked automatically by writing / clearing it. Once it is locked, the lock can not be undone except by a chipset reset.
hm. quote 6 series datasheet: "R/WLO Read/Write, Lock-Once. A register bit with this attribute can be written to the non-locked value multiple times, but to the locked value only once. After the locked value has been written, the bit becomes read only."
and you degraded the warning to dbg level... certainly not suited for upstream, but maybe desirable for chromium(?).
Yes, that was done on purpose, because the lock bit that prevents the bit from being cleared also produces a warning.
and that is? BLE? i do not interpret the public datasheet like that, but...
have you tested this on a board where SMM_BWP is really set to 1? we may wanna try to write it anyway, but it would be far more interesting if it really works on some chipsets :)
Yes, this was implemented to fix a problem I was seeing, and it solved the problem. :-)
hm ok, we should get this upstream then.
but i would have hoped for an answer to my question before that... :) anyway, i have now committed something similar to what is already in chromiumos in r1582. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
participants (2)
-
Stefan Reinauer -
Stefan Tauner