[prev in list] [next in list] [prev in thread] [next in thread]
List: flashrom
Subject: Re: [flashrom] Ideas for implementing support for 2nd status register
From: Hatim Kanchwala <hatim () hatimak ! me>
Date: 2016-05-31 10:37:23
Message-ID: 574D6693.6050002 () hatimak ! me
[Download RAW message or body]
On Sunday 29 May 2016 02:40 PM, David Hendricks wrote:
> Hi Hatim,
> Interesting approach. It seems to work well for pretty printing, though
> I am curious how this will translate into ranges. Do you have an example
> for translating status_register_layout structs to a range of bytes
> protected, for example 0x000000-0x1fffff?
Hi,
I have been researching and developing ideas on how to go about block
protection. A lot of this is based on Chromium OS' implementation. Here
are the two models I have.
Before diving into the models themselves, here's idea that both models
share. We define a struct range (just like Chromium OS) and then define
an array struct range range[32]. We use the BP bitfield as indexes into
the array - so a combination of BP4=0, BP3=0, BP2=1, BP1=0, BP0=1 gives
a bitfield 0x05 and corresponding range is given by range[0x05]={start,
len}. (32 because we have seen a maximum of 5 block protect bits so that
is 2^5 = 32 combinations.) The obvious advantage is that status_to_range
operations will happen in O(1) time. IMO, the use cases of reading the
status register and translating BP bitfields into range are more than
vice-versa. So IMHO having an array of ranges indexed by bitfield will
certainly be more helpful. You will see that the two approaches differ
in the implementation of this idea.
1. Block Protection Table
The driving idea behind this model is to have a representation of block
protection table in flashrom very similar to what is given in datasheets
(just like Chromium OS currently has). But if we are to use array of
ranges indexed by bitfield as per above idea, then we need to process
this representation first. I came up with the following algorithm for
translating the table into the range array -
translate_table_to_range(table):
proc = []
done = []
for row in table
proc.append(row)
while proc is not empty
tmp = proc.pop(0)
for i = 0:5
if tmp.bp[i] is X
new = tmp
new.bp.replace(i, 0)
proc.append(new)
new.bp.replace(i, 1)
proc.append(new)
break
if i is 5
done.append(tmp)
for row in done
index = row.bp4 << 4 | row.bp3 << 3 | row.bp1 << 1 |row.bp0
range[index] = row.range
These were the few questions came up immediately -
- When do we call such a function in the code?
Flashrom will always probe before performing any operation. So once
the chip is found, we fetch the BP table from the struct flashchip, call
this function, store the array of ranges, and then perform all future
operations for that run using the array.
- Do we call it for all the chips?
IMO, calling this for all chips is a bad idea and the above answer
makes this redundant.
- How to handle corner cases?
This needs to be dealt with still.
2. Array of Range
This model is very simple compared to #1 - instead of any such
processing, we simply represent the BP table in the form of an array of
ranges in the first place. This might make representing the table in
flashrom more cumbersome.
Attached is a prototype of the two models (both implementations are
combined in the same file). I have extensively annotated the code to
better convey the models defined above. The output is available here
http://paste.flashrom.org/view.php?id=2921. Chromium implementation
defines two separate structs from CMP=0 and CMP=1. This case can be
handled automatically.
IMO, #2 is better than #1 and we should go forward with #2. The
additional processing step that needs to be taken in #1 before array of
ranges is available is what made me favour #2. I would like to hear your
take on all this, particularly use cases (I could come up with only one
or two, so please tell me more). There's a lot of comments in the code,
so please go through those as well.
I am also wondering if it is possible to get the best of both #1 and #2
- is there a way to pre-process the BP Table into the array of ranges
representation at the time of compilation itself for all chips? This
would eliminate the additional step at runtime, and would make
representation in the code easier.
Personally, I had a lot of fun implementing #1 mainly because it was a
direct materialization of the algorithm and data structures theory I had
in my course. I had started out developing my own data structures but
then stopped mid-way and realized I should be looking for standard
solutions. Then I found sys/queue.h. So, that was a nice learning
experience!
Looking forward to your comments and feedback. Thanks for your time and
patience.
Bye,
Hatim
["test.c" (text/x-csrc)]
#include <stdio.h>
#include <string.h> /* memcpy */
#include <stdlib.h> /* malloc */
#include <sys/queue.h> /* SLIST, singly linked lists (man 3 queue) */
#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
#define MAX_BP_COMBINATIONS (0x01 << 5)
/* This will come from struct flashchip total_size */
#define FLASHCHIP_SIZE 1024 /* kB */
/* Feel free to skip reading throught the following two macros */
#define BP_CHECK_X(n, m) \
wp_n_new = malloc(sizeof(struct wp)); \
memcpy(wp_n_new, wp_n_tmp, sizeof(struct wp)); \
wp_n_new->bp##n = (m); \
SLIST_INSERT_HEAD(&wp_head_proc, wp_n_new, entries)
#define BP_CHECK(n) \
if (wp_n_tmp->bp##n == X) { \
BP_CHECK_X(n, 0); \
BP_CHECK_X(n, 1); \
j = 1; \
}
enum bit_state {
X = -1,
OFF = 0,
ON = 1
};
static struct range {
unsigned int start;
unsigned int len; /* in kB */
} **range;
/* Declare a global (scope limited to the file; returned
on calling wp_init) range array to hold processed BP table
*/
/* TODO : More apt to rename to struct wp_config or wp_table or something.
Should be a part of aonther struct which is a part of struct flashchip.
Should hold a pointer to the BP table (8 bytes for pointer vs 896 bytes
for array, although pointer will increase complexity but chips with
same density are very likely to have similar config).
struct flashchip {
...
struct statusreg {
struct status_register_layout *layout[MAX_STATUS_REGISTERS];
...
} statusreg;
struct wp {
struct wp_config {
struct range range;
enum bit_state bp0;
...
} **wp_config;
struct range (*bp_to_range)(struct flashchip *flash ...);
int (*rangeto_bp)(struct flashchip *flash ...);
...
} wp;
...
}
Note that size of wp_config[] should be 32. It doesn't matter if fewer
entries are made because allocation initializes with zeroes by default
and BP bitfield 0x00 corresponds to NONE protection (that is how flashrom
disables block protection as of now!)
*/
struct wp {
struct range range;
/* This order is deliberately arranged so that while initializing,
if a chip doesn't have BP4, then simply drop it, like - { { 0x0f0000, 8 }, 1, 0, \
1, 0 } For chips with TB bit and (say) 3 BP bits (BP0, BP1, BP2), TB works just \
like BP3 (of course BP4 can be dropped like just explained above). Same goes for \
SEC bit.
*/
enum bit_state bp0;
enum bit_state bp1;
enum bit_state bp2;
enum bit_state bp3;
enum bit_state bp4;
/* This is required by the SLIST macros */
SLIST_ENTRY(wp) entries;
};
/* Transform the BP table (struct wp) corresponding to flashchip into
a range array (struct range []) indexed by the BP bitfield
(e.g. range[0x01] = {0x0f0000, 64}). The range array is static and
global, reference to it is returned.
TODO : First parameter should be struct flashchip *flash
*/
struct range **wp_init(struct wp wp_ranges[]) {
if (range == NULL) {
range = malloc(MAX_BP_COMBINATIONS * sizeof(struct range));
}
int j;
struct wp *wp_n_tmp, *wp_n_new;
/* Initialize singly linked lists */
SLIST_HEAD(wp_head_proc, wp) wp_head_proc = SLIST_HEAD_INITIALIZER(wp_head_proc);
SLIST_HEAD(wp_head_done, wp) wp_head_done = SLIST_HEAD_INITIALIZER(wp_head_done);
SLIST_INIT(&wp_head_proc);
SLIST_INIT(&wp_head_done);
/* Add entries to proc list - to be processed */
for (int i = 0; i < 32; i++) {
SLIST_INSERT_HEAD(&wp_head_proc, &wp_ranges[i], entries);
}
/* Resolve all dont-care combinations - [X, 0, 1] becomes [[0, 0, 1], [1, 0, 1]] */
while (!SLIST_EMPTY(&wp_head_proc)) {
wp_n_tmp = SLIST_FIRST(&wp_head_proc);
SLIST_REMOVE_HEAD(&wp_head_proc, entries);
j = 0;
/* TODO : In all the if conditions, need to also check if corresponding BP bit
even exists for the flashchip
(Please refer to the macro above to make sense of comment in context. Please
pardon this obfuscation, the macro is defined only so that I would have
to type lesser!)
*/
BP_CHECK(4)
else BP_CHECK(3)
else BP_CHECK(2)
else BP_CHECK(1)
else BP_CHECK(0)
if (j == 0) {
SLIST_INSERT_HEAD(&wp_head_done, wp_n_tmp, entries);
}
}
/* Populate the range array */
SLIST_FOREACH(wp_n_tmp, &wp_head_done, entries) {
*(range + (wp_n_tmp->bp4 << 4 | wp_n_tmp->bp3 << 3 | wp_n_tmp->bp2 << 2 | \
wp_n_tmp->bp1 << 1 | wp_n_tmp->bp0)) = &wp_n_tmp->range; }
return range;
}
/* CMP is Complement Protect, flips range, i.e., protection will now work
for complemented range.
TODO : Ideally this should take only struct flashchip *flash as parameter.
There should be a routine to look up the status register layout whether this
chip has a CMP bit, then read it and continue.
*/
struct range *bp_to_range(unsigned int bp_bitfield, int CMP) {
if (CMP) {
struct range *cmp_range = malloc(sizeof(struct range));
memcpy(cmp_range, *(range + bp_bitfield), sizeof(struct range));
if (cmp_range->start) {
cmp_range->start = 0;
} else {
cmp_range->start = cmp_range->len * 1024;
}
/* In place of FLASHCHIPS_SIZE, size will come from flash->total_size */
cmp_range->len = FLASHCHIP_SIZE - cmp_range->len;
return cmp_range;
} else {
return *(range + bp_bitfield);
}
}
/* TODO : Like the previous function, this should also take struct flashchip *flash
as argument along with start and len. The same CMP bit discussion above applies \
here as well.
*/
int range_to_bp(unsigned int start, unsigned int len, int CMP) {
for (unsigned int bp = 0; bp < 32; bp++)
if (start == bp_to_range(bp, CMP)->start && len == bp_to_range(bp, CMP)->len)
return bp;
return -1;
}
/* W25Q80BL, W25Q80DV, W25Q80DL, W25Q80BW, W25Q80EW,
GD25LQ80B, GD25VQ80C, GD25Q80C
*/
struct wp w25q_gd25_ranges[32] = {
/* { struct range }, BP0, BP1, BP2, BP3, BP4
If a chip were to not have BP4 or BP4 and BP3,
then simply dropping them of would do, like -
{ { 0x0f0000, 64 }, 1, 0, 0 },
*/
{ { 0x000000, 0 }, 0, 0, 0, X, X },
{ { 0x0f0000, 64 }, 1, 0, 0, 0, 0 },
{ { 0x0e0000, 128 }, 0, 1, 0, 0, 0 },
{ { 0x0c0000, 256 }, 1, 1, 0, 0, 0 },
{ { 0x080000, 512 }, 0, 0, 1, 0, 0 },
{ { 0x000000, 64 }, 1, 0, 0, 1, 0 },
{ { 0x000000, 128 }, 0, 1, 0, 1, 0 },
{ { 0x000000, 256 }, 1, 1, 0, 1, 0 },
{ { 0x000000, 512 }, 0, 0, 1, 1, 0 },
{ { 0x000000, 1024 }, 1, 0, 1, X, 0 },
{ { 0x000000, 1024 }, X, 1, 1, X, X },
{ { 0x0ff000, 4 }, 1, 0, 0, 0, 1 },
{ { 0x0fe000, 8 }, 0, 1, 0, 0, 1 },
{ { 0x0fc000, 16 }, 1, 1, 0, 0, 1 },
{ { 0x0f8000, 32 }, X, 0, 1, 0, 1 },
{ { 0x000000, 4 }, 1, 0, 0, 1, 1 },
{ { 0x000000, 8 }, 0, 1, 0, 1, 1 },
{ { 0x000000, 16 }, 1, 1, 0, 1, 1 },
{ { 0x000000, 32 }, X, 0, 1, 1, 1 },
};
/*
This representation seems more difficult to represent that the corresponding
one just above, but it makes the code far easier than its counterpart.
Some entries are skipped (those aren't holes!) because default initialization
to zeroes is what we want anyway for those cases.
struct range w25q_gd25_range[32] = {
[0x00] = {0x000000, 0},
[0x01] = {0x0f0000, 64},
[0x02] = {0x0e0000, 128},
[0x03] = {0x0c0000, 256},
[0x04] = {0x080000, 512},
[0x11] = {0x0ff000, 4},
[0x12] = {0x0fe000, 8},
[0x13] = {0x0fc000, 16},
[0x14] = {0x0f8000, 32},
[0x15] = {0x0f8000, 32},
[0x05] = {0x000000, 1024},
[0x06] = {0x000000, 1024},
[0x07] = {0x000000, 1024},
[0x0d] = {0x000000, 1024},
[0x0e] = {0x000000, 1024},
[0x0f] = {0x000000, 1024},
[0x16] = {0x000000, 1024},
[0x17] = {0x000000, 1024},
[0x1e] = {0x000000, 1024},
[0x1f] = {0x000000, 1024},
[0x09] = {0x000000, 64},
[0x0a] = {0x000000, 128},
[0x0b] = {0x000000, 256},
[0x0c] = {0x000000, 512},
[0x19] = {0x000000, 4},
[0x1a] = {0x000000, 8},
[0x1b] = {0x000000, 16},
[0x1c] = {0x000000, 32},
[0x1d] = {0x000000, 32},
};
*/
/* EN25QH32 */
struct range en25_range_array[32] = {
{ 0x000000, 0 },
{ 0x03f000, 64 },
{ 0x3e0000, 128 },
{ 0x3c0000, 256 },
{ 0x380000, 512 },
{ 0x300000, 1024 },
{ 0x200000, 2 * 1024 },
{ 0x000000, 4 * 1024 },
{ 0x000000, 0 },
{ 0x000000, 64 },
{ 0x000000, 128 },
{ 0x000000, 256 },
{ 0x000000, 512 },
{ 0x000000, 1024 },
{ 0x000000, 2 * 1024 },
{ 0x000000, 4 * 1024 },
/* Repeat what is above because this
chip doesn't have a BP4 so BP4 is X
*/
{ 0x000000, 0 },
{ 0x03f000, 64 },
{ 0x3e0000, 128 },
{ 0x3c0000, 256 },
{ 0x380000, 512 },
{ 0x300000, 1024 },
{ 0x200000, 2 * 1024 },
{ 0x000000, 4 * 1024 },
{ 0x000000, 0 },
{ 0x000000, 64 },
{ 0x000000, 128 },
{ 0x000000, 256 },
{ 0x000000, 512 },
{ 0x000000, 1024 },
{ 0x000000, 2 * 1024 },
{ 0x000000, 4 * 1024 }
};
int main(int argc, const char **argv) {
wp_init(w25q_gd25_ranges);
printf("CMP=0\n");
for (int i = 0; i < 32; ++i)
printf("range[0x%02x]={0x%06x, %4dkB}\n", i, bp_to_range(i, 0)->start, \
bp_to_range(i, 0)->len); printf("\n");
printf("CMP=1\n");
for (int i = 0; i < 32; ++i)
printf("range[0x%02x]={0x%06x, %4dkB}\n", i, bp_to_range(i, 1)->start, \
bp_to_range(i, 1)->len);
unsigned int start = 0x000000, len = 1020, CMP = 1;
printf("{0x%06x, %4dkB} : bp=0x%02x\n", start, len, range_to_bp(start, len, CMP));
printf("\n");
*range = en25_range_array;
for (int i = 0; i < 32; ++i)
printf("range[0x%02x]={0x%06x, %4dkB}\n", i, bp_to_range(i, 0)->start, \
bp_to_range(i, 0)->len);
return 0;
}
_______________________________________________
flashrom mailing list
flashrom@flashrom.org
https://www.flashrom.org/mailman/listinfo/flashrom
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic