ifdtool: Enable GbE/PDR/EC region access only if they exist

Instead of assuming GbE/PDR/EC regions may exist or not, check if there
is a valid region defined in the descriptor and set the region access
permissions based on that.

The net effect change is to enable the use of the PDR region on the
sarien platform, which also uses the GbE and EC regions.

This results in the following example changes:

mb/google/sarien (GbE, PDR, EC)
.      DESC BIOS ME GbE PDR EC
-BIOS  r    rw      rw      r
-------------------------------
+BIOS  r    rw      rw  rw  r

mb/google/eve: (no GbE, no PDR, no EC)
.      DESC BIOS ME GbE PDR EC
-BIOS  r    rw      rw      r
-ME    r         rw r
-GbE   r            rw
-EC    r                    rw
-------------------------------
+BIOS  r    rw
+ME    r         rw
+GbE
+EC

BUG=b:134703987

Change-Id: I7aeffc8f8194638c6012340b43aea8f8460d268a
Signed-off-by: Duncan Laurie <dlaurie@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/33273
Reviewed-by: Furquan Shaikh <furquan@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This commit is contained in:
Duncan Laurie 2019-06-06 13:39:26 -07:00 committed by Duncan Laurie
parent c145e54f69
commit 7775d67218
2 changed files with 78 additions and 30 deletions

View File

@ -925,15 +925,24 @@ static void set_chipdensity(const char *filename, char *image, int size,
write_image(filename, image, size); write_image(filename, image, size);
} }
static int check_region(const frba_t *frba, unsigned int region_type)
{
region_t region;
if (!frba)
return 0;
region = get_region(frba, region_type);
return !!((region.base < region.limit) && (region.size > 0));
}
static void lock_descriptor(const char *filename, char *image, int size) static void lock_descriptor(const char *filename, char *image, int size)
{ {
int wr_shift, rd_shift; int wr_shift, rd_shift;
fmba_t *fmba = find_fmba(image, size); fmba_t *fmba = find_fmba(image, size);
const frba_t *frba = find_frba(image, size);
if (!fmba) if (!fmba)
exit(EXIT_FAILURE); exit(EXIT_FAILURE);
/* TODO: Dynamically take Platform Data Region and GbE Region
* into regard.
*/
if (ifd_version >= IFD_VERSION_2) { if (ifd_version >= IFD_VERSION_2) {
wr_shift = FLMSTR_WR_SHIFT_V2; wr_shift = FLMSTR_WR_SHIFT_V2;
@ -969,36 +978,66 @@ static void lock_descriptor(const char *filename, char *image, int size)
case PLATFORM_CNL: case PLATFORM_CNL:
case PLATFORM_ICL: case PLATFORM_ICL:
case PLATFORM_SKLKBL: case PLATFORM_SKLKBL:
/* CPU/BIOS can read descriptor, BIOS, EC and GbE. */ /* CPU/BIOS can read descriptor and BIOS. */
fmba->flmstr1 |= 0x10b << rd_shift; fmba->flmstr1 |= (1 << REGION_DESC) << rd_shift;
/* CPU/BIOS can write BIOS and Gbe. */ fmba->flmstr1 |= (1 << REGION_BIOS) << rd_shift;
fmba->flmstr1 |= 0xa << wr_shift; /* CPU/BIOS can write BIOS. */
/* ME can read descriptor, ME and GbE. */ fmba->flmstr1 |= (1 << REGION_BIOS) << wr_shift;
fmba->flmstr2 |= 0xd << rd_shift; /* ME can read descriptor and ME. */
fmba->flmstr2 |= (1 << REGION_DESC) << rd_shift;
fmba->flmstr2 |= (1 << REGION_ME) << rd_shift;
/* ME can write ME. */ /* ME can write ME. */
fmba->flmstr2 |= 0x4 << wr_shift; fmba->flmstr2 |= (1 << REGION_ME) << wr_shift;
/* GbE can read GbE and descriptor. */ if (check_region(frba, REGION_GBE)) {
fmba->flmstr3 |= 0x9 << rd_shift; /* BIOS can read/write GbE. */
/* GbE can write GbE. */ fmba->flmstr1 |= (1 << REGION_GBE) << rd_shift;
fmba->flmstr3 |= 0x8 << wr_shift; fmba->flmstr1 |= (1 << REGION_GBE) << wr_shift;
/* EC can read EC and descriptor. */ /* ME can read GbE. */
fmba->flmstr5 |= 0x101 << rd_shift; fmba->flmstr2 |= (1 << REGION_GBE) << rd_shift;
/* EC can write EC region. */ /* GbE can read descriptor and read/write GbE.. */
fmba->flmstr5 |= 0x100 << wr_shift; fmba->flmstr3 |= (1 << REGION_DESC) << rd_shift;
fmba->flmstr3 |= (1 << REGION_GBE) << rd_shift;
fmba->flmstr3 |= (1 << REGION_GBE) << wr_shift;
}
if (check_region(frba, REGION_PDR)) {
/* BIOS can read/write PDR. */
fmba->flmstr1 |= (1 << REGION_PDR) << rd_shift;
fmba->flmstr1 |= (1 << REGION_PDR) << wr_shift;
}
if (check_region(frba, REGION_EC)) {
/* BIOS can read EC. */
fmba->flmstr1 |= (1 << REGION_EC) << rd_shift;
/* EC can read descriptor and read/write EC. */
fmba->flmstr5 |= (1 << REGION_DESC) << rd_shift;
fmba->flmstr5 |= (1 << REGION_EC) << rd_shift;
fmba->flmstr5 |= (1 << REGION_EC) << wr_shift;
}
break; break;
default: default:
/* CPU/BIOS can read descriptor, BIOS, and GbE. */ /* CPU/BIOS can read descriptor and BIOS. */
fmba->flmstr1 |= 0xb << rd_shift; fmba->flmstr1 |= (1 << REGION_DESC) << rd_shift;
/* CPU/BIOS can write BIOS and GbE. */ fmba->flmstr1 |= (1 << REGION_BIOS) << rd_shift;
fmba->flmstr1 |= 0xa << wr_shift; /* CPU/BIOS can write BIOS. */
/* ME can read descriptor, ME, and GbE. */ fmba->flmstr1 |= (1 << REGION_BIOS) << wr_shift;
fmba->flmstr2 |= 0xd << rd_shift; /* ME can read descriptor and ME. */
/* ME can write ME and GbE. */ fmba->flmstr2 |= (1 << REGION_DESC) << rd_shift;
fmba->flmstr2 |= 0xc << wr_shift; fmba->flmstr2 |= (1 << REGION_ME) << rd_shift;
/* GbE can write only GbE. */ /* ME can write ME. */
fmba->flmstr3 |= 0x8 << rd_shift; fmba->flmstr2 |= (1 << REGION_ME) << wr_shift;
/* GbE can read only GbE. */ if (check_region(frba, REGION_GBE)) {
fmba->flmstr3 |= 0x8 << wr_shift; /* BIOS can read GbE. */
fmba->flmstr1 |= (1 << REGION_GBE) << rd_shift;
/* BIOS can write GbE. */
fmba->flmstr1 |= (1 << REGION_GBE) << wr_shift;
/* ME can read GbE. */
fmba->flmstr2 |= (1 << REGION_GBE) << rd_shift;
/* ME can write GbE. */
fmba->flmstr2 |= (1 << REGION_GBE) << wr_shift;
/* GbE can write GbE. */
fmba->flmstr3 |= (1 << REGION_GBE) << rd_shift;
/* GbE can read GbE. */
fmba->flmstr3 |= (1 << REGION_GBE) << wr_shift;
}
break; break;
} }

View File

@ -94,6 +94,15 @@ typedef struct {
#define MAX_REGIONS 9 #define MAX_REGIONS 9
#define MAX_REGIONS_OLD 5 #define MAX_REGIONS_OLD 5
enum flash_regions {
REGION_DESC,
REGION_BIOS,
REGION_ME,
REGION_GBE,
REGION_PDR,
REGION_EC = 8,
};
typedef struct { typedef struct {
uint32_t flreg[MAX_REGIONS]; uint32_t flreg[MAX_REGIONS];
} __attribute__((packed)) frba_t; } __attribute__((packed)) frba_t;