David Hendricks has uploaded this change for review. ( https://review.coreboot.org/20570
Change subject: ichspi: Check if region is included before disabling writes
......................................................................
ichspi: Check if region is included before disabling writes
If the user only includes regions to write which are not restricted,
then they should not need to specify ich_spi_force=yes.
Change-Id: I8d5156954b88dbb3058194ab3f351aaf88ea6d8f
Signed-off-by: David Hendricks <dhendricks(a)fb.com>
---
M ichspi.c
1 file changed, 25 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/70/20570/1
diff --git a/ichspi.c b/ichspi.c
index f26a7c0..6f1faae 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -32,6 +32,7 @@
#include "hwaccess.h"
#include "spi.h"
#include "ich_descriptors.h"
+#include "layout.h"
/* Sunrise Point */
@@ -1817,8 +1818,30 @@
msg_pdbg("BRRA 0x%02x\n", ICH_BRRA(tmp));
/* Handle FREGx and FRAP registers */
- for (i = 0; i < num_freg; i++)
- ich_spi_rw_restricted |= ich9_handle_frap(tmp, i);
+ for (i = 0; i < num_freg; i++) {
+ int offset;
+ uint32_t freg, base, limit;
+
+ offset = ICH9_REG_FREG0 + i * 4;
+ freg = mmio_readl(ich_spibar + offset);
+ base = ICH_FREG_BASE(freg);
+ limit = ICH_FREG_LIMIT(freg);
+
+ if (range_is_included(base, limit)) {
+ int is_locked = ich9_handle_frap(tmp, i);
+
+ msg_pspew("Region %07x:%07x is included and ", base, limit);
+ if (is_locked) {
+ msg_pspew("locked.\n");
+ ich_spi_rw_restricted = 1;
+ } else {
+ msg_pspew("not locked.\n");
+ }
+ } else {
+ msg_pspew("Region %07x:%07x is not included, ignoring "
+ "access permission.\n", base, limit);
+ }
+ }
if (ich_spi_rw_restricted)
msg_pwarn("Not all flash regions are freely accessible by flashrom. This is "
"most likely\ndue to an active ME. Please see "
--
To view, visit https://review.coreboot.org/20570
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8d5156954b88dbb3058194ab3f351aaf88ea6d8f
Gerrit-Change-Number: 20570
Gerrit-PatchSet: 1
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
David Hendricks has uploaded this change for review. ( https://review.coreboot.org/20569
Change subject: layout: Add a function to check if numeric range is included
......................................................................
layout: Add a function to check if numeric range is included
This adds a helper function to iterate thru ranges to check if a
specified numeric range is marked as included in the layout.
If no include -i arguments were used to specify included regions,
then all any range will return true.
Change-Id: I096410ea957fe204affa67734e7589a6e4424a26
Signed-off-by: David Hendricks <dhendricks(a)fb.com>
---
M layout.c
M layout.h
2 files changed, 26 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/69/20569/1
diff --git a/layout.c b/layout.c
index 9bf0b03..a35a72c 100644
--- a/layout.c
+++ b/layout.c
@@ -225,3 +225,27 @@
return ret;
}
+
+/* returns 1 if any offset within [base, base+len) is included in layout */
+int range_is_included(chipoff_t start, chipoff_t end)
+{
+ int i;
+
+ if (num_include_args == 0)
+ return 1;
+
+ for (i = 0; i < layout.num_entries; i++) {
+ struct romentry entry = layout.entries[i];
+
+ if (!entry.included)
+ continue;
+ if (end < entry.start)
+ continue;
+ if (start > entry.end)
+ continue;
+
+ return 1;
+ }
+
+ return 0;
+}
diff --git a/layout.h b/layout.h
index c93d754..6fc4e2e 100644
--- a/layout.h
+++ b/layout.h
@@ -66,4 +66,6 @@
int process_include_args(struct flashrom_layout *);
+int range_is_included(chipoff_t start, chipoff_t end);
+
#endif /* !__LAYOUT_H__ */
--
To view, visit https://review.coreboot.org/20569
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: newchange
Gerrit-Change-Id: I096410ea957fe204affa67734e7589a6e4424a26
Gerrit-Change-Number: 20569
Gerrit-PatchSet: 1
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
David Hendricks has posted comments on this change. ( https://review.coreboot.org/18962 )
Change subject: ichspi: Add support for Intel Skylake
......................................................................
Patch Set 9:
Let's go with a comment, then. `17-16` still isn't obvious since the reader still needs to look at how it's used in the code and find an older datasheet to infer what is happening.
As an example:
/* HSFC and HSFS 16-bit registers are combined into the 32-bit BIOS_HSFSTS_CTL register in the Sunrise Point datasheet, however we still treat them separately in order to re-use code. */
This also has the advantage of being searchable, in case somebody wants to grep for BIOS_HSFSTS_CTL to see how it's used.
--
To view, visit https://review.coreboot.org/18962
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f4565a3c39f5fe3aec4fc8863605cebed1ad4ee
Gerrit-Change-Number: 18962
Gerrit-PatchSet: 9
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at>
Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Fri, 14 Jul 2017 04:22:11 +0000
Gerrit-HasComments: No
Nico Huber has posted comments on this change. ( https://review.coreboot.org/18962 )
Change subject: ichspi: Add support for Intel Skylake
......................................................................
Patch Set 9:
> Ah, right... That's something I fixed in the CrOS tree a while
> back: https://chromium-review.googlesource.com/c/448165
Hmmm, that change made sense for the weird cros code (where the
100series support was a copy-paste of the old code), but here we
share 90% of the code paths and I'd rather keep it like that.
>
> Maybe we can just add a comment for now so it's obvious to
> forgetful people reviewing the code? Or we can follow-up with a
> patch to match the datasheet conventions.
I'd prefer a comment. Or maybe make it more obvious in the code
itself, like writing `17 - 16` instead of `1`. How about that?
--
To view, visit https://review.coreboot.org/18962
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f4565a3c39f5fe3aec4fc8863605cebed1ad4ee
Gerrit-Change-Number: 18962
Gerrit-PatchSet: 9
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at>
Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 13 Jul 2017 23:23:43 +0000
Gerrit-HasComments: No
David Hendricks has posted comments on this change. ( https://review.coreboot.org/18962 )
Change subject: ichspi: Add support for Intel Skylake
......................................................................
Patch Set 9: Code-Review+2
Ah, right... That's something I fixed in the CrOS tree a while back: https://chromium-review.googlesource.com/c/448165
Maybe we can just add a comment for now so it's obvious to forgetful people reviewing the code? Or we can follow-up with a patch to match the datasheet conventions.
--
To view, visit https://review.coreboot.org/18962
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f4565a3c39f5fe3aec4fc8863605cebed1ad4ee
Gerrit-Change-Number: 18962
Gerrit-PatchSet: 9
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at>
Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 13 Jul 2017 21:09:30 +0000
Gerrit-HasComments: No