Re: [PATCH v4 2/2] riscv: accept the atomic extensions this kernel was built with
From: Conor Dooley
Date: Fri Nov 06 2026 - 08:09:04 EST
On Thu, Nov 05, 2026 at 02:19:41PM +0200, Vladimir Kondratiev wrote:
> On 11/4/26 23:55, Conor Dooley wrote:
> > On Wed, Nov 04, 2026 at 01:23:52PM +0200, Vladimir Kondratiev wrote:
> >> On 11/3/26 23:41, Conor Dooley wrote:
> >>> That got me thinking though, I feel like this breaks some of the
> >>> extension parsing logic in other ways.
> >>>
> >>> If you putting zalrsc and zaamo in your devicetree, we never do any
> >>> backwards processing to set the a extension (we don't do this for any
> >>> extensions by design, we only populate "child" extensions based on their
> >>> "parent"). If there's some userspace thing that expects to see a in
> >>> cpufeature or w/e it'd break. There shouldn't be since a has always been
> >>> supported but who knows. You need custom userspace anyway so I honestly
> >>> don't give that much of a shit about this going wrong.
> >>>
> >>> What does "worry" me is hwprobe. Currently you've not modified hwprobe
> >>> at all to correctly state that you do not support
> >>> RISCV_HWPROBE_BASE_BEHAVIOR_IMA. (and brings up the question of whether
> >>> you support RISCV_HWPROBE_BASE_BEHAVIOR_IMA_FD etc!)
> >>>
> >>> Maybe, again you don't care cos of your custom userspace, but this needs
> >>> to be holistic if it is being done at all.
> >>> Personally, I don't want to lower the minimum requirements - the people
> >>> designing this stuff to run linux should have checked to see what our
> >>> requirements were (and have always been...).
> >>
> >> I will drop this patch and keep advertising "a" because formally it is
> >> true - all "a" including amo instructions do work but rather slow. Also,
> >> not worth to complicate logic here for (so far) one very special SoC.
> >
> > By that logic, do not complicate the kernel at all with this zalrsc-only
> > stuff and drop patch 1 too. Please do this properly or do not do it at
> > all. If we are introducing the ability to configure the kernel so that
> > it runs on systems with only zalrsc, people should be able to run the
> > kernel on such systems without a) failing to boot or b) incorrectly
> > reporting to userspace what the hardware is capable of doing.
>
> I am reading again risc-v spec, and it says here:
> https://docs.riscv.org/reference/isa/v20260120/priv/machine.html
>
> "M-mode can also be used to implement features that are too difficult or
> expensive to implement in hardware directly."
>
> I read it like the feature implemented in the M-mode trap considered
> "implemented".
>
> Thus in case of AMO emulated in M-mode, it should be considered
> "implemented" however slow. Indeed AMO instructions are executed
> correctly from the S/U modes standpoint. Program compiled with these
> instruction will not crash. It will run quite slow but correctly. So
> proper wording would be "slow ZAAMO" and not "unsupported ZAAMO"
>
> Is my interpretation of the spec correct?
>
> Also, you accepted patch set introducing this p8700 SoC with "A" marked
> as "implemented", here:
> https://lore.kernel.org/all/20261103-agent-lagging-d7fe1c9334f1@spud/
>
> I will follow this direction, keeping "A" as "implemented" and offering
> choice to use faster atomics implementation for such system
I think you completely missed my point. I have no problem with declaring
that this platforms supports any extension that is emulated in firmware.
My point is that if we are adding Kconfig options that ostensibly
mean that the kernel can run on hardware that with a minimum of im + zalrsc,
then the kernel should actually run and run correctly on those systems.
The option isn't
CONFIG_SPECIAL_SNOWFLAKE_MIPS_OPTIMISATION_COS_IT_DOESNT_MEET_MIN_REQUIREMENTS,
and if it is, my opinion is that performance improvements for things
that don't meet our minimum requirements should stay out of tree (as
really should any soc-specific tuning options that can't be enabled
without penalising general hardware). It's not as if this is this device in
particular being picked on, my own employer has stuff out of tree of that
nature FWIW. In my opinion, as I have said, patch 1 is only acceptable if
it actually lowers the minimum requirements from ima to im_zalrsc. It
really should not be too hard to extend hwprobe to do the right thing,
or to deal with bpf by either disabling it when the new Kconfig option is
disabled or by modifying the bpf code to reject the instructions that
are not present.
Cheers,
Conor.
Attachment:
signature.asc
Description: PGP signature