Discussion:
[flashrom] Libflashrom development and integration with fwupd
a***@3mdeb.com
2018-10-24 10:57:44 UTC
Permalink
Hello,

We would like to contribute to the flashrom project by adding missing
libflashrom
features like queries, proper flashrom_shutdown etc.

Until now, I have implemented and tested missing libflashrom functions
from
query section [https://www.flashrom.org/Libflashrom] (based on patches
from
Łukasz Dmitrowski [https://patchwork.coreboot.org/patch/4316/]). There
are 2
ways to get required information (e.g. supported programmers):

const char **flashrom_supported_programmers(void); //#1
int flashrom_supported_programmers(const char **supported_programmers);
//#2

I suppose that returning function status (#2) is not necessary with
query
functions, so I'll stick to the first approach.

What we want to accomplish is remote firmware updates for brand new
platforms
straight from the factory. To do so, libflashrom will be integrated with
fwupd
which would control all required lib resources. I have read the
conversation in
the https://www.mail-archive.com/***@flashrom.org/msg13518.html
from which
it appears that process callbacks for the progress bar is still high
priority
task. Are there any new ideas / hints that we need to know for further
development?

Best regards,
Artur Raglis
Richard Hughes
2018-10-24 11:33:00 UTC
Permalink
Post by a***@3mdeb.com
I suppose that returning function status (#2) is not necessary with
query functions, so I'll stick to the first approach.
I think it's also a really good idea to have a "context" so that there
isn't any global state, i.e. some pointer returned by fl_init() that
can be used in all operations. Having global state in a library isn't
awesome -- and having a context lets you do memory management in a
sane way.
Post by a***@3mdeb.com
Are there any new ideas / hints that we need to know for further development?
I don't think so; fwupd mainly cares about:

1) enumerating devices, which at the moment is controlled using a
whitelist on the machine DMI data:
https://github.com/hughsie/fwupd/blob/master/plugins/flashrom/flashrom.quirk
2) flashing a blob of memory on an abstract device, so we'd need to be
able to chose the flashrom programmer and not just flash the firmware
on some random device
3) verification of the flash process, e.g. just reading back what we
wrote and checking the blob is the same
4) progress reporting, with an update at least every 5 seconds
5) sane error reporting, e.g. providing a ASCII/UTF-8 string rather
than just setting a tty to write to.

Lastly, it's awesome you're driving this work forward. I'm happy to
test any WIP branch with fwupd, or do some sort of high-level API
review if that helps.

Richard.
Nico Huber
2018-10-24 20:54:41 UTC
Permalink
Hello Artur,
Post by a***@3mdeb.com
We would like to contribute to the flashrom project by adding missing
libflashrom
features like queries, proper flashrom_shutdown etc.
sounds great. ;)
Post by a***@3mdeb.com
Until now, I have implemented and tested missing libflashrom functions from
query section [https://www.flashrom.org/Libflashrom] (based on patches from
Łukasz Dmitrowski [https://patchwork.coreboot.org/patch/4316/]). There
are 2
const char **flashrom_supported_programmers(void); //#1
int flashrom_supported_programmers(const char **supported_programmers);
//#2
I suppose that returning function status (#2) is not necessary with query
functions, so I'll stick to the first approach.
Please just push patches early to Gerrit (see [1]). Makes it easier to
discuss interface design.
Post by a***@3mdeb.com
What we want to accomplish is remote firmware updates for brand new
platforms
straight from the factory. To do so, libflashrom will be integrated with
fwupd
which would control all required lib resources. I have read the
conversation in
from which
it appears that process callbacks for the progress bar is still high
priority
task.
Well, it's a task. Nothing has high priority here as nobody is directly
paid for flashrom work. I would be happy to review patches for that,
too (if they implement what I suggested back then, I hope; not sure,
if not). :)
Post by a***@3mdeb.com
Are there any new ideas / hints that we need to know for further
development?
Mostly, what Richard already mentioned: The lack of context pointers
in parts of the API. Basically what I wrote here [2]. Regarding the
(then three) different kinds of context, the current flags mechanism
(enum flashrom_flag) should be replaced with per context options that
one can set (everything that can be set on the command line currently
would need some abstraction). Maybe not a pair of ID|bool but instead
a pair of ID|void* and the actual programmer (or chip) implementation
could decide how that is interpreted based on the ID.

Unification of error codes is another concern. Would make it easier
to provide error strings. Though, the current log callback mechanism
already provides a lot of flexibility (e.g. you could capture all the
level `error` messages during execution of one procedure and show them
if something went wrong).

Layout handling is another thing that needs improvement but that also
requires internal restructuring. But... maybe we should start with the
API and implement it later.

Nico

[1] https://coreboot.org/Git
[2] https://www.mail-archive.com/***@flashrom.org/msg13532.html
Loading...