linux-kernel - Re: [PATCH v3 0/6] Provide a fraemework for RISC-V ISA extensions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHBxVyF65jC_wvxcD6bueqpCY8-Kbahu1yxsSoBmO1s15dGkSQ@mail.gmail.com>
Date:   Tue, 15 Feb 2022 11:06:24 -0800
From:   Atish Kumar Patra <atishp@...osinc.com>
To:     Jisheng Zhang <jszhang@...nel.org>
Cc:     "linux-kernel@...r.kernel.org List" <linux-kernel@...r.kernel.org>,
        Albert Ou <aou@...s.berkeley.edu>,
        Atish Patra <atishp@...shpatra.org>,
        Anup Patel <anup@...infault.org>,
        Damien Le Moal <damien.lemoal@....com>,
        devicetree <devicetree@...r.kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        linux-riscv <linux-riscv@...ts.infradead.org>,
        Palmer Dabbelt <palmer@...belt.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Rob Herring <robh+dt@...nel.org>
Subject: Re: [PATCH v3 0/6] Provide a fraemework for RISC-V ISA extensions

On Tue, Feb 15, 2022 at 8:04 AM Jisheng Zhang <jszhang@...nel.org> wrote:
>
> On Tue, Feb 15, 2022 at 01:02:05AM -0800, Atish Patra wrote:
> > This series implements a generic framework to parse multi-letter ISA
> > extensions. This series is based on Tsukasa's v3 isa extension improvement
> > series[1]. I have fixed few bugs and improved comments from that series
> > (PATCH1-3). I have not used PATCH 4 from that series as we are not using
> > ISA extension versioning as of now. We can add that later if required.
> >
> > PATCH 4 allows the probing of multi-letter extensions via a macro.
> > It continues to use the common isa extensions between all the harts.
> > Thus hetergenous hart systems will only see the common ISA extensions.
> >
> > PATCH 6 improves the /proc/cpuinfo interface for the available ISA extensions
> > via /proc/cpuinfo.
> >
> > Here is the example output of /proc/cpuinfo:
> > (with debug patches in Qemu and Linux kernel)
> >
> > / # cat /proc/cpuinfo
> > processor     : 0
> > hart          : 0
> > isa           : rv64imafdcsu
> > isa-ext               : sstc,sscofpmf
> > mmu           : sv48
> >
> > processor     : 1
> > hart          : 1
> > isa           : rv64imafdcsu
> > isa-ext               : sstc,sscofpmf
> > mmu           : sv48
> >
> > processor     : 2
> > hart          : 2
> > isa           : rv64imafdcsu
> > isa-ext               : sstc,sscofpmf
> > mmu           : sv48
> >
> > processor     : 3
> > hart          : 3
> > isa           : rv64imafdcsu
> > isa-ext               : sstc,sscofpmf
> > mmu           : sv48
> >
> > Anybody adding support for any new multi-letter extensions should add an
> > entry to the riscv_isa_ext_id and the isa extension array.
> > E.g. The patch[2] adds the support for various ISA extensions.
>
> Hi Atish,
>
> Thanks for this series. I'm thinking cpu features VS ISA extenstions.
> I'm converting the sv48 to static key:
> https://lore.kernel.org/linux-riscv/20220125165036.987-1-jszhang@kernel.org/
>
> Previously, I thought the SV48 as a cpu feature, and there will be
> more and more cpu features, so I implemented an unified static key
> mechanism for CPU features. But after reading this series, I think
> I may need to rebase(even reimplement) the above patch to your series.
> But I'm a bit confused by CPU features VS ISA extenstions now:
>
> 1. Is cpu feature  == ISA extension?
>
> 2. Is SV48 considered as ISA extension?
> If yes, now SV48 or not is determined during runtime, but current ISA
> extensions seem parsed from DT. So how to support those ISA extensions
> which can be determined during runtime?
>
> Could you please share your thought?
>

Here are my two cents:

I think the cpu feature is a superset of the ISA extension.
cpu feature != ISA extension.

While all ISA extensions are cpu features, all CPU features may not be
an ISA extension.
e.g. sv48 is not a ISA extension but F/D are (used to set the
cpu_hwcap_fpu static key)

Moreover, not all cpu feature/ISA extension requires a static key.
e.g SSTC extension will require a static key because the check has to
happen in the hot path.
However, sscofpmf extension don't need a static key as the check
happens only one time during boot.

We should keep these two separate but a common static framework would
be very useful.

Here is the flow that I have in my mind.
1. All ISA extensions will be parsed through riscv,isa DT property
2. Any supported/enabled extension will be set in riscv_isa bitmap
3. Any extension requiring a static key will invoke the cpus_set_cap.

cpus_set_cap will be invoked from a different code path that uses a
static key for a specific ISA
extension or a CPU feature.

The only problem I see here is that we have to set a bit in both
cpu_hwcaps & riscv_isa bitmap.
We also have to define the value of that bit for any extension
requiring a static key twice as well.

I think that should be okay. But I would like to hear what everybody
else thinks as well.

> Thanks