Libflashrom development and integration with fwupd

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/[e-mail]/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]

On Wed, 24 Oct 2018 at 11:57,  wrote:
> 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.
> 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]

Hello Artur,

On 10/24/18 12:57 PM, [e-mail] wrote:
> We would like to contribute to the flashrom project by adding missing
> libflashrom
> features like queries, proper flashrom_shutdown etc.

sounds great. ;)
> 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.

Please just push patches early to Gerrit (see [1]). Makes it easier to
discuss interface design.
> 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/[e-mail]/msg13518.html
> 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). :)
> 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/[e-mail]/msg13532.html