Re: [PATCH v4 0/2] riscv: support CPUs with Zalrsc but not Zaamo

[Vladimir Kondratiev <Vladimir.Kondratiev@mobileye.com>]

>> RISC-V split the A extension into Zaamo (AMO*.w/AMO*.d) and Zalrsc
>> (LR/SC). Linux has so far required the whole A extension.
>>
>> MIPS P8700 [1] implements only Zalrsc. AMO instructions are emulated by a
>> trap into SBI. I measured typical userspace atomics on this CPU: an
>> LR/SC atomic takes about 40 cycles, an AMO atomic about 600. The same
>> ratio applies to kernel refcounts, bitops, xchg and futexes, so the
>> kernel needs to emit LR/SC when Zaamo is not implemented.
> 
> In RISC-V land, things that are implemented by M-mode traps are
> generally considered "implemented" in the context of supervisor mode,
> just implemented to have low performance.  I don't agree with that
> decision, but that's really an ISA thing and not something we can do
> that much about it Linux.
> 
> So I think the first thing you need to decide here is how you're going
> to be treating the atomics: are they really unimplemented, or are they
> implemented but slow?  Right now this is kind of a mixed bag: you're
> sort of treated them as unimplemented in the kernel (or at least parts
> of the kernel), but then implemented in userspace.
> 
> If you're going to treat the AMOs as "implemented but slow", then I
> think it's best to see some actual performance numbers for a real
> application.  Sure 600 cycles is slow for an atomic, but I was recently
> playing around with some much larger systems with ~300 cycle AMOs and
> couldn't get any meaningful benchmarks to show up over there -- those
> were OOO cores blocked on bus resources, so a completely different
> problem, but just saying they're 600 cycles so we can't use them isn't
> really good enough -- particularly when the alternative is 40 cycles,
> which is also super slow, so it's not like you can be doing all that
> much with atomics on these cores even with the "fast" sequences.  You're
> probably also going to need some way to tell userspace about this
> "implemented but slow" behavior, similar to how we describe misaligned
> accesses (which is all a bit of a messy retrofit, so maybe don't copy it
> exactly).
> 
> If you're going to treat the AMOs as "unimplemented", then we need some
> way to deal with userspace.  Right now we have A as part of all the base
> behaviors we expose to userspace.  We used to have the non-A userspace
> support, but we decided to drop it when upstreaming the port because it
> didn't seem useful to anyone.  There was a design sketched out here with
> a VDSO call for cmpxchg() that looked similar to some of the non-A Arm
> systems, but as far as I know nobody's implemented that in the kernel or
> userspace so it'd need to be proved out.

Thanks for very clear description.

Risc-v spec says "M-mode can also be used to implement features that are
too difficult or expensive to implement in hardware directly."
here: https://docs.riscv.org/reference/isa/v20260120/priv/machine.html
This means I should treat the AMOs as "implemented but slow". So I will
keep advertising "a" to the user space and rephrase comments.
>> Mobileye is going to use the "eyeq7h" platform, built on this CPU, for
>> ADAS and autonomous driving. Kernel atomics are on the scheduling,
>> memory-management and driver paths of that product, so the emulated
>> AMO cost is not acceptable there.
> 
> Is that announced anywhere?  All I can find is an EyeQ 6 on the website.

Not yet, it is still in development. With the eyeq7h, we're trying to
upstream early. This is why patches appears before official platform
announcement.
> Also: are you planning on continuing with these non-A systems?  Those
> 40-cycle atomics are going to burn you at some point, if the plan for
> the next-generation systems is just to go implement A then maybe it's
> best to keep this out of tree.  We already have way too much
> fragmentation in RISC-V land as it is, we don't need any more.

That's the hardware I have. In the automotive industry projects are
moving slowly and stays for a long time. This SoC going to be used in
safety critical applications so I'd better have it in-tree.

That 40 cycles timing for LR/SC is because reservation monitors
implemented in the point of coherency, i.e. L2

Correct me if I'm wrong, I think there's 3 options that can be used
regarding atomics:

1) full "A" support (strictly say, + ziccrse but it is not formally
required by today's kernel, kind of super-set of 2)
2) zalrsc + ziccrse
3) zaamo + zacas

1) is what is supported now; I am adding support for 2) while leaving
option for 3)

>> This series keeps today's default: both extensions on, -march still
>> contains "a", generated code is unchanged. Turning Zaamo off is a
>> NONPORTABLE choice. The resulting kernel matches one -march, which
>> userspace has to use as well, so a runtime alternative would not
>> produce a portable image.
> 
> It's really not non-portable, though: the Zalrsc code runs fine on
> systems that support Zaamo, it just may be a performance issue.  I'm not
> sure anyone's actually measured that, but I think it's sane to just
> assume that native AMOs are going to out-performance LR/SC on _some_
> systems.  It's still just a performance thing, though...

That's correct, dropping NONPORTABLE

Next series will add the BPF Jit fix, and patch to reject kprobe/uprobe
inside the LR/SC block

Thanks, Vladimir