Attention is currently required from: Patrick Georgi, Martin Roth, Subrata Banik, Caveh Jalali, Stefan Reinauer, Tim Wawrzynczak, Sridhar Siricilla, Angel Pons, Alex Levin, Nick Vaccaro, YH Lin, Boris Mittelberg.
1 comment:
Patchset:
So, if this code change is considered a missed step in HW sequencing recommendation, then this could still be merged..correct?
A version of this change that takes the applicable context into account
(e.g. other waiting loops in this driver, implications when multiple
instances would reach this point in the code simultaneously) would be
accepted immediately.As for the failure that was noticed, it is when flashrom was being used in 2 modes
1. flashrom standalone
2. via futility which uses libflashrom implementationThe standalone flashrom already has a fix for the issue, i.e, lockfile mechanism.
libflashrom did not utilize/inherit the lockfile feature and did not acquire the lock. Which allowed other instances to run. This is already being fixed https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/+/3456667/3/libflashrom.cAFAIK, there is no such locking (yet) in this project. However, I fully
agree that file-based locking is the correct way. And it's interesting
to see that the problem is already solved for the fork. Makes me wonder
why Subrata asks how to solve such things here.Isn't the HW based locking is much better than file based locking
Absolutely not. HW based locking would require hardware designed for such
locking. Even if we would have that (which we don't), it would still need
such a mechanism in every single driver (not everybody uses flashrom only
for internal flashing on Intel hardware). That would mean more work to
implement, more effort to maintain, to achieve little. But doesn't need
any argument about it anyway, because we don't have the hardware ;)
You're just wasting our time here.
and how one can achieve file based locking even in low-level fw like coreboot spi driver or depthcharge ?
I don't know about depthcharge. But in coreboot there shouldn't be multiple
processes accessing the SPI controller. Even if there are, one could handle
it with a simple mutex because it's all one program.
The reason, we opted for file based locking is to have a short term approach and share a fix rather waiting here in loop in code review :-). we are looking for a long term fix inside flashrom to have a `_wait_before` executing run execution.
Can you tell me more about the reasoning? The code is more than a decade old,
I don't see how that's a short term approach.
Again, these are discussions for the mailing list. By constraining the
discussion to Gerrit, you create a bottleneck in the review process for
a project that has close to zero review resources. It hurts the project.
To view, visit change 61854. To unsubscribe, or for help writing mail filters, visit settings.