On 02.10.2007 19:01, Stefan Reinauer wrote:
- ron minnich rminnich@gmail.com [071002 18:38]:
My questions are:
- I do not think this is portable beyond linux. Is that an issue?
it's not, but it is not really an issue at present.
Maybe the code should be #ifdef'ed __linux__ with an #else case just printing a warning a la "Not supported by this OS yet."
I thought that was in the structure of flashrom. Now that I look, it seems like we lost it!
Flashrom never did any such cleanup. I was about to implement it when we renamed the tool but then I stumbled upon this:
printf("OK, only ENABLING flash write, but NOT FLASHING.\n");
So obviously at some point there was a sense in leaving the system in such a state. But I want to propose that we drop this behavior and instead try to always leave the machine in the state we entered it. Especially when not flashing.
While one might want to mess with an unprotected flash on purpose, for 99% of the cases this is just opening another security issue. That one % that theoretically might use this as a feature is welcome to improve the flashrom utility instead of running it for flash unprotection before running another utility.
Agreed.
I propose this at the end of flashrom: board_flash_disable(lb_vendor, lb_part); chipset_flash_disable(chipset);
yepp. Agreed.
Disagree. We still have to store the previous state to be able to revert to it.
but we'll need to change some things to make this all work. We need a penable struct * to use for the disable; no point in searching each time we touch a chip. or not?
To achieve this
struct board_pciid_enable *board in board_enable.c:board_flash_enable()
and
enables[i] in chipset_enable.c:chipset_flash_enable()
should be globally available.
Different proposal: If a particular enable function has been run successfully, it should store a pointer to the corresponding cleanup function and a pointer to the data which has to be restored.
At least the restore information has to be available somewhere. Otherwise the exercise is totally pointless since we can't know the state before the enable function was run.
Carl-Daniel