Change in coreboot[master]: soc/intel/common/fast_spi: Add Lockdown of extended BIOS region
Srinidhi N Kaushik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48068 )
Change subject: soc/intel/common/fast_spi: Add Lockdown of extended BIOS region ......................................................................
soc/intel/common/fast_spi: Add Lockdown of extended BIOS region
This change adds support to Lock down the configuration of extended BIOS region. This is done as part of fast_spi_lockdown_cfg() so that it is consistent with the other lockdown.
Change includes: 1. New helper function fast_spi_lock_ext_bios_cfg() added that will basically set EXT_BIOS_LOCK.
BUG=b:171534504
Signed-off-by: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Change-Id: I730fc12a9c5ca8bb4a1f946cad45944dda8e0518 --- M src/soc/intel/common/block/fast_spi/fast_spi.c M src/soc/intel/common/block/fast_spi/fast_spi_def.h M src/soc/intel/common/block/include/intelblocks/fast_spi.h M src/soc/intel/common/pch/lockdown/lockdown.c 4 files changed, 21 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/48068/1
diff --git a/src/soc/intel/common/block/fast_spi/fast_spi.c b/src/soc/intel/common/block/fast_spi/fast_spi.c index e4d23a6..d4e6230 100644 --- a/src/soc/intel/common/block/fast_spi/fast_spi.c +++ b/src/soc/intel/common/block/fast_spi/fast_spi.c @@ -64,7 +64,7 @@ /* * Set FAST_SPIBAR BIOS Control register based on input bit field. */ -static void fast_spi_set_bios_control_reg(uint8_t bios_cntl_bit) +static void fast_spi_set_bios_control_reg(uint32_t bios_cntl_bit) { #if defined(__SIMPLE_DEVICE__) pci_devfn_t dev = PCH_DEV_SPI; @@ -74,9 +74,9 @@ uint8_t bc_cntl;
assert((bios_cntl_bit & (bios_cntl_bit - 1)) == 0); - bc_cntl = pci_read_config8(dev, SPIBAR_BIOS_CONTROL); + bc_cntl = pci_read_config32(dev, SPIBAR_BIOS_CONTROL); bc_cntl |= bios_cntl_bit; - pci_write_config8(dev, SPIBAR_BIOS_CONTROL, bc_cntl); + pci_write_config32(dev, SPIBAR_BIOS_CONTROL, bc_cntl); }
/* @@ -108,6 +108,16 @@ }
/* + * Set FAST_SPIBAR BIOS Control EXT BIOS LE bit. + */ +void fast_spi_set_ext_bios_lock_enable(void) +{ + fast_spi_set_bios_control_reg(SPIBAR_BIOS_CONTROL_EXT_BIOS_LOCK_ENABLE); + + fast_spi_read_post_write(SPIBAR_BIOS_CONTROL); +} + +/* * Set FAST_SPIBAR BIOS Control EISS bit. */ void fast_spi_set_eiss(void) diff --git a/src/soc/intel/common/block/fast_spi/fast_spi_def.h b/src/soc/intel/common/block/fast_spi/fast_spi_def.h index f1c0f1e..9784797 100644 --- a/src/soc/intel/common/block/fast_spi/fast_spi_def.h +++ b/src/soc/intel/common/block/fast_spi/fast_spi_def.h @@ -14,6 +14,7 @@ /* Bit definitions for BIOS_CONTROL */ #define SPIBAR_BIOS_CONTROL_WPD (1 << 0) #define SPIBAR_BIOS_CONTROL_LOCK_ENABLE (1 << 1) +#define SPIBAR_BIOS_CONTROL_EXT_BIOS_LOCK_ENABLE (1 << 28) #define SPIBAR_BIOS_CONTROL_CACHE_DISABLE (1 << 2) #define SPIBAR_BIOS_CONTROL_PREFETCH_ENABLE (1 << 3) #define SPIBAR_BIOS_CONTROL_EISS (1 << 5) diff --git a/src/soc/intel/common/block/include/intelblocks/fast_spi.h b/src/soc/intel/common/block/include/intelblocks/fast_spi.h index 58bf25b..729690a 100644 --- a/src/soc/intel/common/block/include/intelblocks/fast_spi.h +++ b/src/soc/intel/common/block/include/intelblocks/fast_spi.h @@ -23,6 +23,10 @@ */ void fast_spi_set_lock_enable(void); /* + * Set FAST_SPIBAR BIOS Control Ext Bios LE bit. + */ +void fast_spi_set_ext_bios_lock_enable(void); +/* * Set FAST_SPIBAR BIOS Control EISS bit. */ void fast_spi_set_eiss(void); diff --git a/src/soc/intel/common/pch/lockdown/lockdown.c b/src/soc/intel/common/pch/lockdown/lockdown.c index 906f8b0..b10306e 100644 --- a/src/soc/intel/common/pch/lockdown/lockdown.c +++ b/src/soc/intel/common/pch/lockdown/lockdown.c @@ -63,6 +63,9 @@
/* BIOS Lock */ fast_spi_set_lock_enable(); + + /* EXT BIOS Lock */ + fast_spi_set_ext_bios_lock_enable(); } }
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48068 )
Change subject: soc/intel/common/fast_spi: Add Lockdown of extended BIOS region ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/48068/2/src/soc/intel/common/block/... File src/soc/intel/common/block/fast_spi/fast_spi.c:
https://review.coreboot.org/c/coreboot/+/48068/2/src/soc/intel/common/block/... PS2, Line 74: uint8_t uint32_t
https://review.coreboot.org/c/coreboot/+/48068/2/src/soc/intel/common/block/... PS2, Line 77: bc_cntl = pci_read_config32(dev, SPIBAR_BIOS_CONTROL); : bc_cntl |= bios_cntl_bit; : pci_write_config32(dev, SPIBAR_BIOS_CONTROL, bc_cntl); This can be changed to pci_or_config32, right?
https://review.coreboot.org/c/coreboot/+/48068/2/src/soc/intel/common/block/... PS2, Line 97: fast_spi_read_post_write(SPIBAR_BIOS_CONTROL); Not for this change, but I think we should move the call to fast_spi_read_post_write() inside fast_spi_set_bios_control_reg().
Hello build bot (Jenkins), Furquan Shaikh, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48068
to look at the new patch set (#3).
Change subject: soc/intel/common/fast_spi: Add Lockdown of extended BIOS region ......................................................................
soc/intel/common/fast_spi: Add Lockdown of extended BIOS region
This change adds support to Lock down the configuration of extended BIOS region. This is done as part of fast_spi_lockdown_cfg() so that it is consistent with the other lockdown.
Change includes: 1. New helper function fast_spi_lock_ext_bios_cfg() added that will basically set EXT_BIOS_LOCK.
BUG=b:171534504
Signed-off-by: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Change-Id: I730fc12a9c5ca8bb4a1f946cad45944dda8e0518 --- M src/soc/intel/common/block/fast_spi/fast_spi.c M src/soc/intel/common/block/fast_spi/fast_spi_def.h M src/soc/intel/common/block/include/intelblocks/fast_spi.h M src/soc/intel/common/pch/lockdown/lockdown.c 4 files changed, 22 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/48068/3
Srinidhi N Kaushik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48068 )
Change subject: soc/intel/common/fast_spi: Add Lockdown of extended BIOS region ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/48068/2/src/soc/intel/common/block/... File src/soc/intel/common/block/fast_spi/fast_spi.c:
https://review.coreboot.org/c/coreboot/+/48068/2/src/soc/intel/common/block/... PS2, Line 74: uint8_t
uint32_t
Done
https://review.coreboot.org/c/coreboot/+/48068/2/src/soc/intel/common/block/... PS2, Line 77: bc_cntl = pci_read_config32(dev, SPIBAR_BIOS_CONTROL); : bc_cntl |= bios_cntl_bit; : pci_write_config32(dev, SPIBAR_BIOS_CONTROL, bc_cntl);
This can be changed to pci_or_config32, right?
Just to be consistent with other Reg writes, I left it as it is and just changed 8 to 32.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48068 )
Change subject: soc/intel/common/fast_spi: Add Lockdown of extended BIOS region ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48068/5/src/soc/intel/common/block/... File src/soc/intel/common/block/fast_spi/fast_spi.c:
https://review.coreboot.org/c/coreboot/+/48068/5/src/soc/intel/common/block/... PS5, Line 113: { if (!CONFIG(FAST_SPI_SUPPORTS_EXT_BIOS_WINDOW)) return;
Hello build bot (Jenkins), Furquan Shaikh, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48068
to look at the new patch set (#6).
Change subject: soc/intel/common/fast_spi: Add Lockdown of extended BIOS region ......................................................................
soc/intel/common/fast_spi: Add Lockdown of extended BIOS region
This change adds support to Lock down the configuration of extended BIOS region. This is done as part of fast_spi_lockdown_cfg() so that it is consistent with the other lockdown.
Change includes: 1. New helper function fast_spi_lock_ext_bios_cfg() added that will basically set EXT_BIOS_LOCK.
BUG=b:171534504
Signed-off-by: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Change-Id: I730fc12a9c5ca8bb4a1f946cad45944dda8e0518 --- M src/soc/intel/common/block/fast_spi/fast_spi.c M src/soc/intel/common/block/fast_spi/fast_spi_def.h M src/soc/intel/common/block/include/intelblocks/fast_spi.h M src/soc/intel/common/pch/lockdown/lockdown.c 4 files changed, 25 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/48068/6
Srinidhi N Kaushik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48068 )
Change subject: soc/intel/common/fast_spi: Add Lockdown of extended BIOS region ......................................................................
Patch Set 6: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/48068/5/src/soc/intel/common/block/... File src/soc/intel/common/block/fast_spi/fast_spi.c:
https://review.coreboot.org/c/coreboot/+/48068/5/src/soc/intel/common/block/... PS5, Line 113: {
if (!CONFIG(FAST_SPI_SUPPORTS_EXT_BIOS_WINDOW)) […]
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48068 )
Change subject: soc/intel/common/fast_spi: Add Lockdown of extended BIOS region ......................................................................
Patch Set 6: Code-Review+2
Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48068 )
Change subject: soc/intel/common/fast_spi: Add Lockdown of extended BIOS region ......................................................................
soc/intel/common/fast_spi: Add Lockdown of extended BIOS region
This change adds support to Lock down the configuration of extended BIOS region. This is done as part of fast_spi_lockdown_cfg() so that it is consistent with the other lockdown.
Change includes: 1. New helper function fast_spi_lock_ext_bios_cfg() added that will basically set EXT_BIOS_LOCK.
BUG=b:171534504
Signed-off-by: Srinidhi N Kaushik srinidhi.n.kaushik@intel.com Change-Id: I730fc12a9c5ca8bb4a1f946cad45944dda8e0518 Reviewed-on: https://review.coreboot.org/c/coreboot/+/48068 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/common/block/fast_spi/fast_spi.c M src/soc/intel/common/block/fast_spi/fast_spi_def.h M src/soc/intel/common/block/include/intelblocks/fast_spi.h M src/soc/intel/common/pch/lockdown/lockdown.c 4 files changed, 25 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Srinidhi N Kaushik: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/common/block/fast_spi/fast_spi.c b/src/soc/intel/common/block/fast_spi/fast_spi.c index d6e8f53..7859b53 100644 --- a/src/soc/intel/common/block/fast_spi/fast_spi.c +++ b/src/soc/intel/common/block/fast_spi/fast_spi.c @@ -63,19 +63,19 @@ /* * Set FAST_SPIBAR BIOS Control register based on input bit field. */ -static void fast_spi_set_bios_control_reg(uint8_t bios_cntl_bit) +static void fast_spi_set_bios_control_reg(uint32_t bios_cntl_bit) { #if defined(__SIMPLE_DEVICE__) pci_devfn_t dev = PCH_DEV_SPI; #else struct device *dev = PCH_DEV_SPI; #endif - uint8_t bc_cntl; + uint32_t bc_cntl;
assert((bios_cntl_bit & (bios_cntl_bit - 1)) == 0); - bc_cntl = pci_read_config8(dev, SPIBAR_BIOS_CONTROL); + bc_cntl = pci_read_config32(dev, SPIBAR_BIOS_CONTROL); bc_cntl |= bios_cntl_bit; - pci_write_config8(dev, SPIBAR_BIOS_CONTROL, bc_cntl); + pci_write_config32(dev, SPIBAR_BIOS_CONTROL, bc_cntl); }
/* @@ -107,6 +107,19 @@ }
/* + * Set FAST_SPIBAR BIOS Control EXT BIOS LE bit. + */ +void fast_spi_set_ext_bios_lock_enable(void) +{ + if (!CONFIG(FAST_SPI_SUPPORTS_EXT_BIOS_WINDOW)) + return; + + fast_spi_set_bios_control_reg(SPIBAR_BIOS_CONTROL_EXT_BIOS_LOCK_ENABLE); + + fast_spi_read_post_write(SPIBAR_BIOS_CONTROL); +} + +/* * Set FAST_SPIBAR BIOS Control EISS bit. */ void fast_spi_set_eiss(void) diff --git a/src/soc/intel/common/block/fast_spi/fast_spi_def.h b/src/soc/intel/common/block/fast_spi/fast_spi_def.h index 883c3ce..ce4eb2d 100644 --- a/src/soc/intel/common/block/fast_spi/fast_spi_def.h +++ b/src/soc/intel/common/block/fast_spi/fast_spi_def.h @@ -14,6 +14,7 @@ /* Bit definitions for BIOS_CONTROL */ #define SPIBAR_BIOS_CONTROL_WPD (1 << 0) #define SPIBAR_BIOS_CONTROL_LOCK_ENABLE (1 << 1) +#define SPIBAR_BIOS_CONTROL_EXT_BIOS_LOCK_ENABLE (1 << 28) #define SPIBAR_BIOS_CONTROL_CACHE_DISABLE (1 << 2) #define SPIBAR_BIOS_CONTROL_PREFETCH_ENABLE (1 << 3) #define SPIBAR_BIOS_CONTROL_EISS (1 << 5) diff --git a/src/soc/intel/common/block/include/intelblocks/fast_spi.h b/src/soc/intel/common/block/include/intelblocks/fast_spi.h index e3ffa6a..e742a27 100644 --- a/src/soc/intel/common/block/include/intelblocks/fast_spi.h +++ b/src/soc/intel/common/block/include/intelblocks/fast_spi.h @@ -23,6 +23,10 @@ */ void fast_spi_set_lock_enable(void); /* + * Set FAST_SPIBAR BIOS Control Ext Bios LE bit. + */ +void fast_spi_set_ext_bios_lock_enable(void); +/* * Set FAST_SPIBAR BIOS Control EISS bit. */ void fast_spi_set_eiss(void); diff --git a/src/soc/intel/common/pch/lockdown/lockdown.c b/src/soc/intel/common/pch/lockdown/lockdown.c index 906f8b0..b10306e 100644 --- a/src/soc/intel/common/pch/lockdown/lockdown.c +++ b/src/soc/intel/common/pch/lockdown/lockdown.c @@ -63,6 +63,9 @@
/* BIOS Lock */ fast_spi_set_lock_enable(); + + /* EXT BIOS Lock */ + fast_spi_set_ext_bios_lock_enable(); } }
-
Furquan Shaikh (Code Review) -
Srinidhi N Kaushik (Code Review)