New subject: Using libflashrom from fwupd

17 Aug 2017


      Hi Richard, David,
On 16.08.2017 23:02, David Hendricks wrote:
...
On Tue, Aug 15, 2017 at 4:55 AM, Richard Hughes hughsient@gmail.com wrote:
...
Hi all,
I'm the maintainer of fwupd, which is a daemon for doing firmware
updates in Linux. Using fwupd about 200,000 people update firmware
every month. At the moment I have an out-of-tree patch to use the
flashrom CLI for updating coreboot on Purism laptops, which exec's a
flashrom binary with the correct arguments and then screen scrapes the
output. This is less than ideal. I saw the appearance of a libflashrom
in the staging branch and got very excited. I'm assuming the long term
goal here is to install a shared with stable API/ABI library and a
pkg-config file. If so, please keep reading.
I'm only using it as a static library, kept reading anyway :) The cur-
rent API is to be considered unstable. We'd like to have it stabilized
before the next release (whenever that may happen), but it's not a top
priority.
...
...
One thing that is really important for fwupd is progress completion;
as flashing the firmware is an inherently scary thing to do users do
like to see a progress bar move from 0% to 100% in a gradual way as
it's very re-assuring. This is even more important for the GUI, where
people that just see a spinner don't know if the process is going to
take 10 seconds, or 10 minutes.
Agreed! Especially since a lot of users have large chips, slow programmers,
or sometimes both. Simple read operations can take a worryingly long time
on some common hardware these days.
...
Would it be possible to have a process callback for
flashrom_image_read(), flashrom_image_write() and
flashrom_image_verify()? It'd need to have some kind of callback
userdata too, for instance:
You already helped to improve our API: I forgot that one might need a
userdata pointer for the log callback. Will fix that...
...
...
int flashrom_image_verify(struct flashrom_flashctx *, const void
*buffer, size_t buffer_len, flashrom_progress_callback callback_fn,
void *callback_user_data);
and also:
typedef void(flashrom_progress_callback)(uint64 completed, unit64
total, void *user_data)
I guess this would suffice for read and verify, which are simple,
continuous operations. Write, however, is comprised of one, two or three
steps (depending on options and the data): Read (needed to know which
bytes have to change), (comparing to the data to be written) write (if
necessary) and optionally verify. So there is no straight forward way
to tell during read, for instance, if we are at 100% after the read or
not. We could compare the data early, but that might result in an awk-
ward implementation. My idea (not yet convinced if it's any good) would
be to add a `stage` parameter to the callback:
enum flashrom_progress_stage {
        FLASHROM_PROGRESS_READ,
        FLASHROM_PROGRESS_WRITE,
        FLASHROM_PROGRESS_VERIFY,
    };
...
...
The short answer is yes. Nico also briefly mentioned an idea on IRC to use
a mechanism similar to flashrom_set_log_callback() to avoid complicating
function signatures for the read, write, and verify functions.
Correct, I would prefer a method to set the current callback and private
pointer over adding more parameters. Though, we could have another set
of functions ofc, with more parameters.
Richard, please let us know, what you'd prefer. After all, the interface
should be designed to be most usable (and not driven by internals). e.g.
do you expect the callback or the userdata pointer to change between
calls?
...
...
I'm not familiar at all with flashrom internals, but could try to
produce a patch if required -- although to propagate the progress
through operations would mean patching a lot of chip->read() callbacks
which probably requires a more experienced hand. Ideas welcome,
thanks.
I'd be glad to help implementing it. But let's discuss the interface
first.
Nico

Re: [flashrom] Using libflashrom from fwupd