Rizwan Qureshi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30559
Change subject: drivers/spi: Add controller protection type ......................................................................
drivers/spi: Add controller protection type
Some SPI controllers support both READ and WRITE protection add a variable to teh protect API for the callers to specify the kind of protection that is requested. Also, update the callers and actual protect API implementation.
BUG=None BRANCH=None TEST=test that the mrc cache is protected as expected on soraka. Also tried if the read protection is applied correctly.
Change-Id: I093884c4768b08a378f21242ac82e430ac013d15 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/drivers/mrc_cache/mrc_cache.c M src/drivers/spi/spi_flash.c M src/include/spi-generic.h M src/include/spi_flash.h M src/soc/intel/broadwell/include/soc/spi.h M src/soc/intel/broadwell/spi.c M src/soc/intel/common/block/fast_spi/fast_spi_flash.c 7 files changed, 37 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/30559/1
diff --git a/src/drivers/mrc_cache/mrc_cache.c b/src/drivers/mrc_cache/mrc_cache.c index 4dd0dbc..20026b7 100644 --- a/src/drivers/mrc_cache/mrc_cache.c +++ b/src/drivers/mrc_cache/mrc_cache.c @@ -462,7 +462,8 @@ if (!IS_ENABLED(CONFIG_BOOT_DEVICE_SPI_FLASH)) return 0;
- return spi_flash_ctrlr_protect_region(boot_device_spi_flash(), r); + return spi_flash_ctrlr_protect_region(boot_device_spi_flash(), r, + WRITE_PROTECT); }
/* Protect mrc region with a Protected Range Register */ diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c index 57bdaf4..7874d25 100644 --- a/src/drivers/spi/spi_flash.c +++ b/src/drivers/spi/spi_flash.c @@ -573,7 +573,8 @@
int spi_flash_ctrlr_protect_region(const struct spi_flash *flash, - const struct region *region) + const struct region *region, + enum ctrlr_prot_type type) { const struct spi_ctrlr *ctrlr; struct region flash_region = { 0 }; @@ -592,7 +593,7 @@ return -1;
if (ctrlr->flash_protect) - return ctrlr->flash_protect(flash, region); + return ctrlr->flash_protect(flash, region, type);
return -1; } diff --git a/src/include/spi-generic.h b/src/include/spi-generic.h index e3e7f82..f071ef0 100644 --- a/src/include/spi-generic.h +++ b/src/include/spi-generic.h @@ -97,6 +97,11 @@
struct spi_flash;
+enum ctrlr_prot_type{ + READ_PROTECT = 1, + WRITE_PROTECT = 2, +}; + enum { /* Deduct the command length from the spi_crop_chunk() calculation for sizing a transaction. */ @@ -144,7 +149,8 @@ int (*flash_probe)(const struct spi_slave *slave, struct spi_flash *flash); int (*flash_protect)(const struct spi_flash *flash, - const struct region *region); + const struct region *region, + enum ctrlr_prot_type type); };
/*----------------------------------------------------------------------- diff --git a/src/include/spi_flash.h b/src/include/spi_flash.h index 09908eb..1498909 100644 --- a/src/include/spi_flash.h +++ b/src/include/spi_flash.h @@ -207,7 +207,8 @@ /* Protect a region of spi flash using its controller, if available. Returns * < 0 on error, else 0 on success. */ int spi_flash_ctrlr_protect_region(const struct spi_flash *flash, - const struct region *region); + const struct region *region, + enum ctrlr_prot_type type);
/* * This function is provided to support spi flash command-response transactions. diff --git a/src/soc/intel/broadwell/include/soc/spi.h b/src/soc/intel/broadwell/include/soc/spi.h index 588af17..5ad62fd 100644 --- a/src/soc/intel/broadwell/include/soc/spi.h +++ b/src/soc/intel/broadwell/include/soc/spi.h @@ -38,6 +38,7 @@ #define SPI_PRR_BASE_SHIFT 0 #define SPI_PRR_LIMIT_SHIFT 16 #define SPI_PRR_WPE (1 << 31) +#define SPI_PRR_RPE (1 << 15)
#define SPIBAR_PREOP 0x94 #define SPIBAR_OPTYPE 0x96 diff --git a/src/soc/intel/broadwell/spi.c b/src/soc/intel/broadwell/spi.c index 2317c9a..a027446 100644 --- a/src/soc/intel/broadwell/spi.c +++ b/src/soc/intel/broadwell/spi.c @@ -615,11 +615,13 @@
/* Use first empty Protected Range Register to cover region of flash */ static int spi_flash_protect(const struct spi_flash *flash, - const struct region *region) + const struct region *region, + enum ctrlr_prot_type type) { u32 start = region_offset(region); u32 end = start + region_sz(region) - 1; u32 reg; + u32 protect_mask; int prr;
/* Find first empty PRR */ @@ -637,12 +639,18 @@ reg = ((end >> SPI_PRR_SHIFT) & SPI_PRR_MASK); reg <<= SPI_PRR_LIMIT_SHIFT; reg |= ((start >> SPI_PRR_SHIFT) & SPI_PRR_MASK); - reg |= SPI_PRR_WPE; + + if (type & WRITE_PROTECT) + protect_mask |= SPI_PRR_WPE; + if (type & READ_PROTECT) + protect_mask |= SPI_PRR_RPE; + + reg |= protect_mask;
/* Set the PRR register and verify it is protected */ SPIBAR32(SPI_PRR(prr)) = reg; reg = SPIBAR32(SPI_PRR(prr)); - if (!(reg & SPI_PRR_WPE)) { + if (!(reg & protect_mask)) { printk(BIOS_ERR, "ERROR: Unable to set SPI PRR %d\n", prr); return -1; } diff --git a/src/soc/intel/common/block/fast_spi/fast_spi_flash.c b/src/soc/intel/common/block/fast_spi/fast_spi_flash.c index 4579b19..a56fe8b 100644 --- a/src/soc/intel/common/block/fast_spi/fast_spi_flash.c +++ b/src/soc/intel/common/block/fast_spi/fast_spi_flash.c @@ -367,11 +367,13 @@ * Protected Range (FPR) register if available. */ static int fast_spi_flash_protect(const struct spi_flash *flash, - const struct region *region) + const struct region *region, + enum ctrlr_prot_type type) { u32 start = region_offset(region); u32 end = start + region_sz(region) - 1; u32 reg; + u32 protect_mask = 0; int fpr; uintptr_t fpr_base; BOILERPLATE_CREATE_CTX(ctx); @@ -392,12 +394,18 @@ }
/* Set protected range base and limit */ - reg = SPI_FPR(start, end) | SPI_FPR_WPE; + + if (type & WRITE_PROTECT) + protect_mask |= SPI_FPR_WPE; + if (type & READ_PROTECT) + protect_mask |= SPI_FPR_RPE; + + reg = SPI_FPR(start, end) | protect_mask;
/* Set the FPR register and verify it is protected */ write32((void *)fpr_base, reg); reg = read32((void *)fpr_base); - if (!(reg & SPI_FPR_WPE)) { + if (!(reg & protect_mask)) { printk(BIOS_ERR, "ERROR: Unable to set SPI FPR %d\n", fpr); return -1; }
Hello Patrick Rudolph, Aaron Durbin, Arthur Heymans, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30559
to look at the new patch set (#7).
Change subject: drivers/spi: Add controller protection type ......................................................................
drivers/spi: Add controller protection type
Some SPI controllers support both READ and WRITE protection add a variable to the protect API for the callers to specify the kind of protection they want (Read/Write/Both). Also, update the callers and protect API implementation.
BUG=None BRANCH=None TEST=test that the mrc cache is protected as expected on soraka. Also tried if the read protection is applied correctly.
Change-Id: I093884c4768b08a378f21242ac82e430ac013d15 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/drivers/mrc_cache/mrc_cache.c M src/drivers/spi/spi_flash.c M src/include/spi-generic.h M src/include/spi_flash.h M src/soc/intel/broadwell/include/soc/spi.h M src/soc/intel/broadwell/spi.c M src/soc/intel/common/block/fast_spi/fast_spi_flash.c M src/southbridge/intel/common/spi.c 8 files changed, 82 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/30559/7
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30559 )
Change subject: drivers/spi: Add controller protection type ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/#/c/30559/7/src/southbridge/intel/common/spi.c File src/southbridge/intel/common/spi.c:
https://review.coreboot.org/#/c/30559/7/src/southbridge/intel/common/spi.c@1... PS7, Line 1043: if(IS_ENABLED(CONFIG_SOUTHBRIDGE_INTEL_I82801GX)) space required before the open parenthesis '('
https://review.coreboot.org/#/c/30559/7/src/southbridge/intel/common/spi.c@1... PS7, Line 1048: if(IS_ENABLED(CONFIG_SOUTHBRIDGE_INTEL_I82801GX)) space required before the open parenthesis '('
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30559 )
Change subject: drivers/spi: Add controller protection type ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/#/c/30559/6/src/southbridge/intel/common/spi.c File src/southbridge/intel/common/spi.c:
https://review.coreboot.org/#/c/30559/6/src/southbridge/intel/common/spi.c@2... PS6, Line 299: if (IS_ENABLED(CONFIG_SOUTHBRIDGE_INTEL_IBEXPEAK) || : IS_ENABLED(CONFIG_SOUTHBRIDGE_INTEL_BD82X6X) || : IS_ENABLED(CONFIG_SOUTHBRIDGE_INTEL_C216) || : IS_ENABLED(CONFIG_SOUTHBRIDGE_INTEL_I82801GX))
this is wrong. it's just INTEL_I82801GX that doesn't support it.
indeed, wrong. Fixed now.
Hello Patrick Rudolph, Aaron Durbin, Arthur Heymans, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30559
to look at the new patch set (#8).
Change subject: drivers/spi: Add controller protection type ......................................................................
drivers/spi: Add controller protection type
Some SPI controllers support both READ and WRITE protection add a variable to the protect API for the callers to specify the kind of protection they want (Read/Write/Both). Also, update the callers and protect API implementation.
BUG=None BRANCH=None TEST=test that the mrc cache is protected as expected on soraka. Also tried if the read protection is applied correctly.
Change-Id: I093884c4768b08a378f21242ac82e430ac013d15 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/drivers/mrc_cache/mrc_cache.c M src/drivers/spi/spi_flash.c M src/include/spi-generic.h M src/include/spi_flash.h M src/soc/intel/broadwell/include/soc/spi.h M src/soc/intel/broadwell/spi.c M src/soc/intel/common/block/fast_spi/fast_spi_flash.c M src/southbridge/intel/common/spi.c 8 files changed, 82 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/30559/8
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30559 )
Change subject: drivers/spi: Add controller protection type ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/#/c/30559/7/src/southbridge/intel/common/spi.c File src/southbridge/intel/common/spi.c:
https://review.coreboot.org/#/c/30559/7/src/southbridge/intel/common/spi.c@1... PS7, Line 1043: if(IS_ENABLED(CONFIG_SOUTHBRIDGE_INTEL_I82801GX))
space required before the open parenthesis '('
Done
https://review.coreboot.org/#/c/30559/7/src/southbridge/intel/common/spi.c@1... PS7, Line 1048: if(IS_ENABLED(CONFIG_SOUTHBRIDGE_INTEL_I82801GX))
space required before the open parenthesis '('
Done
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30559 )
Change subject: drivers/spi: Add controller protection type ......................................................................
Patch Set 8: Code-Review+1
(4 comments)
https://review.coreboot.org/#/c/30559/8/src/drivers/mrc_cache/mrc_cache.c File src/drivers/mrc_cache/mrc_cache.c:
https://review.coreboot.org/#/c/30559/8/src/drivers/mrc_cache/mrc_cache.c@46... PS8, Line 467: return spi_flash_ctrlr_protect_region(flash, r, WRITE_PROTECT); replace tab by space
https://review.coreboot.org/#/c/30559/8/src/drivers/spi/spi_flash.c File src/drivers/spi/spi_flash.c:
https://review.coreboot.org/#/c/30559/8/src/drivers/spi/spi_flash.c@576 PS8, Line 576: const struct region *region, align with (
https://review.coreboot.org/#/c/30559/8/src/include/spi_flash.h File src/include/spi_flash.h:
https://review.coreboot.org/#/c/30559/8/src/include/spi_flash.h@210 PS8, Line 210: const struct region *region, align with (
https://review.coreboot.org/#/c/30559/8/src/soc/intel/common/block/fast_spi/... File src/soc/intel/common/block/fast_spi/fast_spi_flash.c:
https://review.coreboot.org/#/c/30559/8/src/soc/intel/common/block/fast_spi/... PS8, Line 371: enum ctrlr_prot_type type) align with (
Hello Patrick Rudolph, Aaron Durbin, Arthur Heymans, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30559
to look at the new patch set (#9).
Change subject: drivers/spi: Add controller protection type ......................................................................
drivers/spi: Add controller protection type
Some SPI controllers support both READ and WRITE protection add a variable to the protect API for the callers to specify the kind of protection they want (Read/Write/Both). Also, update the callers and protect API implementation.
BUG=None BRANCH=None TEST=test that the mrc cache is protected as expected on soraka. Also tried if the read protection is applied correctly.
Change-Id: I093884c4768b08a378f21242ac82e430ac013d15 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/drivers/mrc_cache/mrc_cache.c M src/drivers/spi/spi_flash.c M src/include/spi-generic.h M src/include/spi_flash.h M src/soc/intel/broadwell/include/soc/spi.h M src/soc/intel/broadwell/spi.c M src/soc/intel/common/block/fast_spi/fast_spi_flash.c M src/southbridge/intel/common/spi.c 8 files changed, 82 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/30559/9
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30559 )
Change subject: drivers/spi: Add controller protection type ......................................................................
Patch Set 9: Code-Review+1
(2 comments)
Beside the small nits this looks good
https://review.coreboot.org/#/c/30559/9/src/include/spi_flash.h File src/include/spi_flash.h:
https://review.coreboot.org/#/c/30559/9/src/include/spi_flash.h@211 PS9, Line 211: enum ctrlr_prot_type type); you could make it const enum
https://review.coreboot.org/#/c/30559/9/src/soc/intel/broadwell/spi.c File src/soc/intel/broadwell/spi.c:
https://review.coreboot.org/#/c/30559/9/src/soc/intel/broadwell/spi.c@619 PS9, Line 619: enum ctrlr_prot_type type) alignment
Hello Patrick Rudolph, Aaron Durbin, Arthur Heymans, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30559
to look at the new patch set (#11).
Change subject: drivers/spi: Add controller protection type ......................................................................
drivers/spi: Add controller protection type
Some SPI controllers support both READ and WRITE protection add a variable to the protect API for the callers to specify the kind of protection they want (Read/Write/Both). Also, update the callers and protect API implementation.
BUG=None BRANCH=None TEST=test that the mrc cache is protected as expected on soraka. Also tried if the read protection is applied correctly.
Change-Id: I093884c4768b08a378f21242ac82e430ac013d15 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/drivers/mrc_cache/mrc_cache.c M src/drivers/spi/spi_flash.c M src/include/spi-generic.h M src/include/spi_flash.h M src/soc/intel/broadwell/include/soc/spi.h M src/soc/intel/broadwell/spi.c M src/soc/intel/common/block/fast_spi/fast_spi_flash.c M src/southbridge/intel/common/spi.c 8 files changed, 82 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/30559/11
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30559 )
Change subject: drivers/spi: Add controller protection type ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/#/c/30559/9/src/include/spi_flash.h File src/include/spi_flash.h:
https://review.coreboot.org/#/c/30559/9/src/include/spi_flash.h@211 PS9, Line 211: enum ctrlr_prot_type type);
you could make it const enum
Done
https://review.coreboot.org/#/c/30559/9/src/soc/intel/broadwell/spi.c File src/soc/intel/broadwell/spi.c:
https://review.coreboot.org/#/c/30559/9/src/soc/intel/broadwell/spi.c@619 PS9, Line 619: enum ctrlr_prot_type type)
alignment
Done
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30559 )
Change subject: drivers/spi: Add controller protection type ......................................................................
Patch Set 11: Code-Review+2
It would be great to have a separate documentation entry about this topic. It would fit into Intel / security section.
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30559 )
Change subject: drivers/spi: Add controller protection type ......................................................................
Patch Set 11:
Patch Set 11: Code-Review+2
It would be great to have a separate documentation entry about this topic. It would fit into Intel / security section.
Sure
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30559 )
Change subject: drivers/spi: Add controller protection type ......................................................................
drivers/spi: Add controller protection type
Some SPI controllers support both READ and WRITE protection add a variable to the protect API for the callers to specify the kind of protection they want (Read/Write/Both). Also, update the callers and protect API implementation.
BUG=None BRANCH=None TEST=test that the mrc cache is protected as expected on soraka. Also tried if the read protection is applied correctly.
Change-Id: I093884c4768b08a378f21242ac82e430ac013d15 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Reviewed-on: https://review.coreboot.org/c/30559 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Rudolph siro@das-labor.org --- M src/drivers/mrc_cache/mrc_cache.c M src/drivers/spi/spi_flash.c M src/include/spi-generic.h M src/include/spi_flash.h M src/soc/intel/broadwell/include/soc/spi.h M src/soc/intel/broadwell/spi.c M src/soc/intel/common/block/fast_spi/fast_spi_flash.c M src/southbridge/intel/common/spi.c 8 files changed, 82 insertions(+), 14 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Rudolph: Looks good to me, approved
diff --git a/src/drivers/mrc_cache/mrc_cache.c b/src/drivers/mrc_cache/mrc_cache.c index 4dd0dbc..84bbdb0 100644 --- a/src/drivers/mrc_cache/mrc_cache.c +++ b/src/drivers/mrc_cache/mrc_cache.c @@ -456,13 +456,15 @@ /* Apply protection to a range of flash */ static int nvm_protect(const struct region *r) { + const struct spi_flash *flash = boot_device_spi_flash(); + if (!IS_ENABLED(CONFIG_MRC_SETTINGS_PROTECT)) return 0;
if (!IS_ENABLED(CONFIG_BOOT_DEVICE_SPI_FLASH)) return 0;
- return spi_flash_ctrlr_protect_region(boot_device_spi_flash(), r); + return spi_flash_ctrlr_protect_region(flash, r, WRITE_PROTECT); }
/* Protect mrc region with a Protected Range Register */ diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c index f06de2a..204f607 100644 --- a/src/drivers/spi/spi_flash.c +++ b/src/drivers/spi/spi_flash.c @@ -572,7 +572,8 @@
int spi_flash_ctrlr_protect_region(const struct spi_flash *flash, - const struct region *region) + const struct region *region, + const enum ctrlr_prot_type type) { const struct spi_ctrlr *ctrlr; struct region flash_region = { 0 }; @@ -591,7 +592,7 @@ return -1;
if (ctrlr->flash_protect) - return ctrlr->flash_protect(flash, region); + return ctrlr->flash_protect(flash, region, type);
return -1; } diff --git a/src/include/spi-generic.h b/src/include/spi-generic.h index e3e7f82..c24aadd 100644 --- a/src/include/spi-generic.h +++ b/src/include/spi-generic.h @@ -97,6 +97,12 @@
struct spi_flash;
+enum ctrlr_prot_type { + READ_PROTECT = 1, + WRITE_PROTECT = 2, + READ_WRITE_PROTECT = 3, +}; + enum { /* Deduct the command length from the spi_crop_chunk() calculation for sizing a transaction. */ @@ -144,7 +150,8 @@ int (*flash_probe)(const struct spi_slave *slave, struct spi_flash *flash); int (*flash_protect)(const struct spi_flash *flash, - const struct region *region); + const struct region *region, + const enum ctrlr_prot_type type); };
/*----------------------------------------------------------------------- diff --git a/src/include/spi_flash.h b/src/include/spi_flash.h index 09908eb..936b0ab 100644 --- a/src/include/spi_flash.h +++ b/src/include/spi_flash.h @@ -207,7 +207,8 @@ /* Protect a region of spi flash using its controller, if available. Returns * < 0 on error, else 0 on success. */ int spi_flash_ctrlr_protect_region(const struct spi_flash *flash, - const struct region *region); + const struct region *region, + const enum ctrlr_prot_type type);
/* * This function is provided to support spi flash command-response transactions. diff --git a/src/soc/intel/broadwell/include/soc/spi.h b/src/soc/intel/broadwell/include/soc/spi.h index 588af17..5ad62fd 100644 --- a/src/soc/intel/broadwell/include/soc/spi.h +++ b/src/soc/intel/broadwell/include/soc/spi.h @@ -38,6 +38,7 @@ #define SPI_PRR_BASE_SHIFT 0 #define SPI_PRR_LIMIT_SHIFT 16 #define SPI_PRR_WPE (1 << 31) +#define SPI_PRR_RPE (1 << 15)
#define SPIBAR_PREOP 0x94 #define SPIBAR_OPTYPE 0x96 diff --git a/src/soc/intel/broadwell/spi.c b/src/soc/intel/broadwell/spi.c index 2317c9a..de3d061 100644 --- a/src/soc/intel/broadwell/spi.c +++ b/src/soc/intel/broadwell/spi.c @@ -615,11 +615,13 @@
/* Use first empty Protected Range Register to cover region of flash */ static int spi_flash_protect(const struct spi_flash *flash, - const struct region *region) + const struct region *region, + const enum ctrlr_prot_type type) { u32 start = region_offset(region); u32 end = start + region_sz(region) - 1; u32 reg; + u32 protect_mask = 0; int prr;
/* Find first empty PRR */ @@ -637,12 +639,28 @@ reg = ((end >> SPI_PRR_SHIFT) & SPI_PRR_MASK); reg <<= SPI_PRR_LIMIT_SHIFT; reg |= ((start >> SPI_PRR_SHIFT) & SPI_PRR_MASK); - reg |= SPI_PRR_WPE; + + switch (type) { + case WRITE_PROTECT: + protect_mask |= SPI_PRR_WPE; + break; + case READ_PROTECT: + protect_mask |= SPI_PRR_RPE; + break; + case READ_WRITE_PROTECT: + protect_mask |= (SPI_PRR_RPE | SPI_PRR_WPE); + break; + default: + printk(BIOS_ERR, "ERROR: Seeking invalid protection!\n"); + return -1; + } + + reg |= protect_mask;
/* Set the PRR register and verify it is protected */ SPIBAR32(SPI_PRR(prr)) = reg; reg = SPIBAR32(SPI_PRR(prr)); - if (!(reg & SPI_PRR_WPE)) { + if (!(reg & protect_mask)) { printk(BIOS_ERR, "ERROR: Unable to set SPI PRR %d\n", prr); return -1; } diff --git a/src/soc/intel/common/block/fast_spi/fast_spi_flash.c b/src/soc/intel/common/block/fast_spi/fast_spi_flash.c index 4579b19..65708a6 100644 --- a/src/soc/intel/common/block/fast_spi/fast_spi_flash.c +++ b/src/soc/intel/common/block/fast_spi/fast_spi_flash.c @@ -367,11 +367,13 @@ * Protected Range (FPR) register if available. */ static int fast_spi_flash_protect(const struct spi_flash *flash, - const struct region *region) + const struct region *region, + const enum ctrlr_prot_type type) { u32 start = region_offset(region); u32 end = start + region_sz(region) - 1; u32 reg; + u32 protect_mask = 0; int fpr; uintptr_t fpr_base; BOILERPLATE_CREATE_CTX(ctx); @@ -391,13 +393,28 @@ return -1; }
+ switch (type) { + case WRITE_PROTECT: + protect_mask |= SPI_FPR_WPE; + break; + case READ_PROTECT: + protect_mask |= SPI_FPR_RPE; + break; + case READ_WRITE_PROTECT: + protect_mask |= (SPI_FPR_RPE | SPI_FPR_WPE); + break; + default: + printk(BIOS_ERR, "ERROR: Seeking invalid protection!\n"); + return -1; + } + /* Set protected range base and limit */ - reg = SPI_FPR(start, end) | SPI_FPR_WPE; + reg = SPI_FPR(start, end) | protect_mask;
/* Set the FPR register and verify it is protected */ write32((void *)fpr_base, reg); reg = read32((void *)fpr_base); - if (!(reg & SPI_FPR_WPE)) { + if (!(reg & protect_mask)) { printk(BIOS_ERR, "ERROR: Unable to set SPI FPR %d\n", fpr); return -1; } diff --git a/src/southbridge/intel/common/spi.c b/src/southbridge/intel/common/spi.c index 9bc3414..4c59399 100644 --- a/src/southbridge/intel/common/spi.c +++ b/src/southbridge/intel/common/spi.c @@ -1010,12 +1010,14 @@ * Returns 0 on success, -1 on failure of programming fpr registers. */ static int spi_flash_protect(const struct spi_flash *flash, - const struct region *region) + const struct region *region, + const enum ctrlr_prot_type type) { ich_spi_controller *cntlr = car_get_var_ptr(&g_cntlr); u32 start = region_offset(region); u32 end = start + region_sz(region) - 1; u32 reg; + u32 protect_mask = 0; int fpr; uint32_t *fpr_base;
@@ -1033,13 +1035,32 @@ return -1; }
+ switch (type) { + case WRITE_PROTECT: + protect_mask |= SPI_FPR_WPE; + break; + case READ_PROTECT: + if (IS_ENABLED(CONFIG_SOUTHBRIDGE_INTEL_I82801GX)) + return -1; + protect_mask |= ICH9_SPI_FPR_RPE; + break; + case READ_WRITE_PROTECT: + if (IS_ENABLED(CONFIG_SOUTHBRIDGE_INTEL_I82801GX)) + return -1; + protect_mask |= (ICH9_SPI_FPR_RPE | SPI_FPR_WPE); + break; + default: + printk(BIOS_ERR, "ERROR: Seeking invalid protection!\n"); + return -1; + } + /* Set protected range base and limit */ - reg = spi_fpr(start, end) | SPI_FPR_WPE; + reg = spi_fpr(start, end) | protect_mask;
/* Set the FPR register and verify it is protected */ write32(&fpr_base[fpr], reg); reg = read32(&fpr_base[fpr]); - if (!(reg & SPI_FPR_WPE)) { + if (!(reg & protect_mask)) { printk(BIOS_ERR, "ERROR: Unable to set SPI FPR %d\n", fpr); return -1; }
-
build bot (Jenkins) (Code Review) -
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)
-
Rizwan Qureshi (Code Review)