Re: MemoryFence()


Ankur Arora
 

On 2021-02-10 7:55 a.m., Laszlo Ersek wrote:
On 02/10/21 08:21, Ankur Arora wrote:

In general I would say that fences are -- except for one case -- strictly
greater in power than read_once/write_once. That one case is alignment
checks (which we do for instance in MmioRead32() and friends.)

Given that difference in power, I agree with the approach Paolo
recommended elsewhere that for CPU memory access syncrhonization we
start with Fences and then switch to lesser powered variants
as and when needed:

"However, these changes do not have to be done in a single step.
Starting with the rationalization of memory fences makes sense
because, in addition to enabling the annotation of code rather
than data, it is essentially a bugfix. "
Given the actual amount of MemoryFence() calls and "volatile" uses in
edk2, it's actually a pretty large undertaking to "fix" these issues. I
don't foresee a quick resolution here, especially if I'm supposed to
work on it alone (considering actual patches).
I've not seen any feedback from Mike or Liming in this thread yet, for
example, so I'm unsure about the MdePkg (BaseLib) maintainer buy-in.
This is just to state the context in which I interpret "first step" and
"essentially a bugfix". The issue is systemic in edk2, as every such
occasion as we've run into now has only added to the proliferation of
"volatile".
The next stable tag is coming up too. I recommend we focus on merging
the VCPU hotplug series. I don't intend to let this thread silently
peter out (like many similar threads must have, in the past); I hope to
do something about actual patches in the next development cycle. I do
admit it looks like a daunting task, considering the multiple
architectures and toolchains.
Yeah, also given that the bugs around this are subtle. And, it is
a non-trivial effort to test all the affected pieces of logic.

Just thinking out aloud -- maybe the thing to do might be to write
some validation code that compares before and after generated code to
flag cases when there are unexpected changes.

I'm not the most experienced with low level barriers and the like (except
for having used them once in a while) but happy to help any way I can.

(NB: we've not spent a single word on RISC-V yet -- AFAICS, there RISC-V
doesn't even have *any* implementation of MemoryFence(), yet. Refer to
commit 7601b251fd5c ("MdePkg/BaseLib: BaseLib for RISCV64 architecture",
2020-05-07), for example.)
"You touch it, you own it" and "learn as you go" should not be a
surprise to me in an open source project, but I admit I don't know if I
should even attempt patching UefiCpuPkg content (MdePkg and OvmfPkg look
more approachable). The risk of regressions runs high, reproducibility
is always a mess in multiprocessing code, and the code is also
privileged. I want it fixed everywhere, but I feel uncertain about the
apparent lack of commitment.
I have some hands-on multiprocessing background, but I've always made a
very conscious decision to stay away from lockless programming, and use
safe and *simple* POSIX Threads-based schemes (mutexes, condition
variables with *broadcasts* etc). The "network of IP blocks" that Ard
used for (correctly) describing modern computer architecture is way
below the programming abstraction level where I feel comfortable. Thus
I'm very much out of my comfort zone here. (My initial decision against
lockless programming goes back to when <http://www.1024cores.net/> was
published originally.)
Anyway: I have several messages in this thread starred for revising.
Ankur, is the picture clear enough for you to return to work on the v7
unplug series? The soft feature freeze is coming: 2021-02-22.
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning
Thanks for the heads up. Yeah, I think things are clear enough for me to
send out v7. I plan to send that out sometime early next week so there's
enough time to review before the soft freeze date.

Thanks
Ankur

Thanks
Laszlo

Join rfc@edk2.groups.io to automatically receive all group messages.