mirror of
https://review.coreboot.org/flashrom.git
synced 2025-04-28 07:23:43 +02:00
ichspi.c: Simplify ich9_handle_{frap,pr}() to work with logical rep
Simplify ich9_handle_frap() to do the translation to the logical representation of the ich_access_protection enum in one place and work from there. This removes some unnecessary branch complexity and the possibility of out of bounds array accesses. Change-Id: I1eda067c44a84d662713475d13902c85534a59fe Signed-off-by: Edward O'Callaghan <quasisec@google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/65189 Reviewed-by: Sam McNally <sammc@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Nikolai Artemiev <nartemiev@google.com>
This commit is contained in:
parent
6f610a8391
commit
f56a0322f4
45
ichspi.c
45
ichspi.c
@ -1679,12 +1679,11 @@ static const enum ich_access_protection access_perms_to_protection[] = {
|
|||||||
LOCKED, WRITE_PROT, READ_PROT, NO_PROT
|
LOCKED, WRITE_PROT, READ_PROT, NO_PROT
|
||||||
};
|
};
|
||||||
static const char *const access_names[] = {
|
static const char *const access_names[] = {
|
||||||
"locked", "read-only", "write-only", "read-write"
|
"read-write", "write-only", "read-only", "locked"
|
||||||
};
|
};
|
||||||
|
|
||||||
static enum ich_access_protection ich9_handle_frap(uint32_t frap, unsigned int i)
|
static enum ich_access_protection ich9_handle_frap(uint32_t frap, unsigned int i)
|
||||||
{
|
{
|
||||||
const int rwperms_unknown = ARRAY_SIZE(access_names);
|
|
||||||
static const char *const region_names[] = {
|
static const char *const region_names[] = {
|
||||||
"Flash Descriptor", "BIOS", "Management Engine",
|
"Flash Descriptor", "BIOS", "Management Engine",
|
||||||
"Gigabit Ethernet", "Platform Data", "Device Expansion",
|
"Gigabit Ethernet", "Platform Data", "Device Expansion",
|
||||||
@ -1693,23 +1692,13 @@ static enum ich_access_protection ich9_handle_frap(uint32_t frap, unsigned int i
|
|||||||
const char *const region_name = i < ARRAY_SIZE(region_names) ? region_names[i] : "unknown";
|
const char *const region_name = i < ARRAY_SIZE(region_names) ? region_names[i] : "unknown";
|
||||||
|
|
||||||
uint32_t base, limit;
|
uint32_t base, limit;
|
||||||
int rwperms;
|
unsigned int rwperms_idx;
|
||||||
|
enum ich_access_protection rwperms;
|
||||||
const int offset = i < 12
|
const int offset = i < 12
|
||||||
? ICH9_REG_FREG0 + i * 4
|
? ICH9_REG_FREG0 + i * 4
|
||||||
: APL_REG_FREG12 + (i - 12) * 4;
|
: APL_REG_FREG12 + (i - 12) * 4;
|
||||||
uint32_t freg = mmio_readl(ich_spibar + offset);
|
uint32_t freg = mmio_readl(ich_spibar + offset);
|
||||||
|
|
||||||
if (i < 8) {
|
|
||||||
rwperms = (((ICH_BRWA(frap) >> i) & 1) << 1) |
|
|
||||||
(((ICH_BRRA(frap) >> i) & 1) << 0);
|
|
||||||
} else {
|
|
||||||
/* Datasheets don't define any access bits for regions > 7. We
|
|
||||||
can't rely on the actual descriptor settings either as there
|
|
||||||
are several overrides for them (those by other masters are
|
|
||||||
not even readable by us, *shrug*). */
|
|
||||||
rwperms = rwperms_unknown;
|
|
||||||
}
|
|
||||||
|
|
||||||
base = ICH_FREG_BASE(freg);
|
base = ICH_FREG_BASE(freg);
|
||||||
limit = ICH_FREG_LIMIT(freg);
|
limit = ICH_FREG_LIMIT(freg);
|
||||||
if (base > limit || (freg == 0 && i > 0)) {
|
if (base > limit || (freg == 0 && i > 0)) {
|
||||||
@ -1719,20 +1708,24 @@ static enum ich_access_protection ich9_handle_frap(uint32_t frap, unsigned int i
|
|||||||
return NO_PROT;
|
return NO_PROT;
|
||||||
}
|
}
|
||||||
msg_pdbg("0x%02X: 0x%08x ", offset, freg);
|
msg_pdbg("0x%02X: 0x%08x ", offset, freg);
|
||||||
if (rwperms == 0x3) {
|
|
||||||
msg_pdbg("FREG%u: %s region (0x%08x-0x%08x) is %s.\n", i,
|
if (i < 8) {
|
||||||
region_name, base, limit, access_names[rwperms]);
|
rwperms_idx = (((ICH_BRWA(frap) >> i) & 1) << 1) |
|
||||||
return NO_PROT;
|
(((ICH_BRRA(frap) >> i) & 1) << 0);
|
||||||
}
|
rwperms = access_perms_to_protection[rwperms_idx];
|
||||||
if (rwperms == rwperms_unknown) {
|
} else {
|
||||||
|
/* Datasheets don't define any access bits for regions > 7. We
|
||||||
|
can't rely on the actual descriptor settings either as there
|
||||||
|
are several overrides for them (those by other masters are
|
||||||
|
not even readable by us, *shrug*). */
|
||||||
msg_pdbg("FREG%u: %s region (0x%08x-0x%08x) has unknown permissions.\n",
|
msg_pdbg("FREG%u: %s region (0x%08x-0x%08x) has unknown permissions.\n",
|
||||||
i, region_name, base, limit);
|
i, region_name, base, limit);
|
||||||
return NO_PROT;
|
return NO_PROT;
|
||||||
}
|
}
|
||||||
|
|
||||||
msg_pinfo("FREG%u: %s region (0x%08x-0x%08x) is %s.\n", i,
|
msg_pinfo("FREG%u: %s region (0x%08x-0x%08x) is %s.\n", i,
|
||||||
region_name, base, limit, access_names[rwperms]);
|
region_name, base, limit, access_names[rwperms]);
|
||||||
return access_perms_to_protection[rwperms];
|
|
||||||
|
return rwperms;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* In contrast to FRAP and the master section of the descriptor the bits
|
/* In contrast to FRAP and the master section of the descriptor the bits
|
||||||
@ -1748,14 +1741,15 @@ static enum ich_access_protection ich9_handle_pr(const size_t reg_pr0, unsigned
|
|||||||
{
|
{
|
||||||
uint8_t off = reg_pr0 + (i * 4);
|
uint8_t off = reg_pr0 + (i * 4);
|
||||||
uint32_t pr = mmio_readl(ich_spibar + off);
|
uint32_t pr = mmio_readl(ich_spibar + off);
|
||||||
unsigned int rwperms = ICH_PR_PERMS(pr);
|
unsigned int rwperms_idx = ICH_PR_PERMS(pr);
|
||||||
|
enum ich_access_protection rwperms = access_perms_to_protection[rwperms_idx];
|
||||||
|
|
||||||
/* From 5 on we have GPR registers and start from 0 again. */
|
/* From 5 on we have GPR registers and start from 0 again. */
|
||||||
const char *const prefix = i >= 5 ? "G" : "";
|
const char *const prefix = i >= 5 ? "G" : "";
|
||||||
if (i >= 5)
|
if (i >= 5)
|
||||||
i -= 5;
|
i -= 5;
|
||||||
|
|
||||||
if (rwperms == 0x3) {
|
if (rwperms == NO_PROT) {
|
||||||
msg_pdbg2("0x%02X: 0x%08x (%sPR%u is unused)\n", off, pr, prefix, i);
|
msg_pdbg2("0x%02X: 0x%08x (%sPR%u is unused)\n", off, pr, prefix, i);
|
||||||
return NO_PROT;
|
return NO_PROT;
|
||||||
}
|
}
|
||||||
@ -1763,7 +1757,8 @@ static enum ich_access_protection ich9_handle_pr(const size_t reg_pr0, unsigned
|
|||||||
msg_pdbg("0x%02X: 0x%08x ", off, pr);
|
msg_pdbg("0x%02X: 0x%08x ", off, pr);
|
||||||
msg_pwarn("%sPR%u: Warning: 0x%08x-0x%08x is %s.\n", prefix, i, ICH_FREG_BASE(pr),
|
msg_pwarn("%sPR%u: Warning: 0x%08x-0x%08x is %s.\n", prefix, i, ICH_FREG_BASE(pr),
|
||||||
ICH_FREG_LIMIT(pr), access_names[rwperms]);
|
ICH_FREG_LIMIT(pr), access_names[rwperms]);
|
||||||
return access_perms_to_protection[rwperms];
|
|
||||||
|
return rwperms;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Set/Clear the read and write protection enable bits of PR register @i
|
/* Set/Clear the read and write protection enable bits of PR register @i
|
||||||
|
Loading…
x
Reference in New Issue
Block a user