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: <ZJwEj710jhXoKHfm@vermeer>
Date:   Wed, 28 Jun 2023 11:59:43 +0200
From:   Samuel Ortiz <sameo@...osinc.com>
To:     "Hongren (Zenithal) Zheng" <i@...ithal.me>
Cc:     Conor Dooley <conor@...nel.org>, Evan Green <evan@...osinc.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        linux-riscv@...ts.infradead.org, linux@...osinc.com,
        Conor Dooley <conor.dooley@...rochip.com>,
        Andrew Jones <ajones@...tanamicro.com>,
        Heiko Stuebner <heiko.stuebner@...ll.eu>,
        Anup Patel <apatel@...tanamicro.com>,
        linux-kernel@...r.kernel.org, Guo Ren <guoren@...nel.org>,
        Atish Patra <atishp@...osinc.com>,
        Björn Töpel <bjorn@...osinc.com>,
        Jiatai He <jiatai2021@...as.ac.cn>
Subject: Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

On Wed, Jun 28, 2023 at 03:03:58AM +0800, Hongren (Zenithal) Zheng wrote:
> On Tue, Jun 27, 2023 at 07:48:15PM +0100, Conor Dooley wrote:
> > On Tue, Jun 27, 2023 at 11:14:30AM -0700, Evan Green wrote:
> > > On Tue, Jun 27, 2023 at 7:38 AM Samuel Ortiz <sameo@...osinc.com> wrote:
> > > >
> > > > From: "Hongren (Zenithal) Zheng" <i@...ithal.me>
> > > >
> > > > This patch parses Zb/Zk related string from DT and
> > 
> > %s/This patch//
> > 
> > > > output them in cpuinfo
> > > >
> > > > One thing worth noting is that if DT provides zk,
> > > > all zbkb, zbkc, zbkx and zkn, zkr, zkt would be enabled.
> > 
> > Please explain why this is okay.
> 
> From riscv scalar crypto spec, zk is a shorthand
> for zkn, zkr and zkt and zkn also includes zbkb, zbkc
> and zbkx.
> 
> > 
> > > > Note that zk is a valid extension name and the current
> > > > DT binding spec allows this.
> > > >
> > > > This patch also changes the logical id of
> > > > existing multi-letter extensions and adds a statement
> > > > that instead of logical id compatibility, the order
> > > > is needed.
> > 
> > Does it?
> 
> That is in the old version of this patch,
> should be removed now
> see https://lore.kernel.org/linux-riscv/YqY0aSngjI0Hc5d4@Sun/
> 
> > 
> > > > There currently lacks a mechanism to merge them when
> > > > producing cpuinfo. Namely if you provide a riscv,isa
> > > > "rv64imafdc_zk_zks", the cpuinfo output would be
> > > > "rv64imafdc_zbkb_zbkc_zbkx_zknd_zkne_zknh_zkr_zksed
> > > > _zksh_zkt"
> > 
> > I think this is fine.
> > 
> > Please re-wrap this all to 72 characters.
> > 
> > > >
> > > > Tested-by: Jiatai He <jiatai2021@...as.ac.cn>
> > > > Signed-off-by: Hongren (Zenithal) Zheng <i@...ithal.me>
> > 
> > This is missing your SoB Samuel.
> > 
> > > > ---
> > > >  arch/riscv/include/asm/hwcap.h | 11 +++++++++++
> > > >  arch/riscv/kernel/cpu.c        | 11 +++++++++++
> > > >  arch/riscv/kernel/cpufeature.c | 30 ++++++++++++++++++++++++++++++
> > > >  3 files changed, 52 insertions(+)
> > > >
> > > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > > > index f041bfa7f6a0..b80ca6e77088 100644
> > > > --- a/arch/riscv/include/asm/hwcap.h
> > > > +++ b/arch/riscv/include/asm/hwcap.h
> > > > @@ -53,6 +53,17 @@
> > > >  #define RISCV_ISA_EXT_ZICSR            40
> > > >  #define RISCV_ISA_EXT_ZIFENCEI         41
> > > >  #define RISCV_ISA_EXT_ZIHPM            42
> > > > +#define RISCV_ISA_EXT_ZBC              43
> > > > +#define RISCV_ISA_EXT_ZBKB             44
> > > > +#define RISCV_ISA_EXT_ZBKC             45
> > > > +#define RISCV_ISA_EXT_ZBKX             46
> > > > +#define RISCV_ISA_EXT_ZKND             47
> > > > +#define RISCV_ISA_EXT_ZKNE             48
> > > > +#define RISCV_ISA_EXT_ZKNH             49
> > > > +#define RISCV_ISA_EXT_ZKR              50
> > > > +#define RISCV_ISA_EXT_ZKSED            51
> > > > +#define RISCV_ISA_EXT_ZKSH             52
> > > > +#define RISCV_ISA_EXT_ZKT              53
> > > >
> > > >  #define RISCV_ISA_EXT_MAX              64
> > > >  #define RISCV_ISA_EXT_NAME_LEN_MAX     32
> > > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > > > index a2fc952318e9..10524322a4c0 100644
> > > > --- a/arch/riscv/kernel/cpu.c
> > > > +++ b/arch/riscv/kernel/cpu.c
> > > > @@ -215,7 +215,18 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
> > > >         __RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
> > > >         __RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA),
> > > >         __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> > > > +       __RISCV_ISA_EXT_DATA(zbc, RISCV_ISA_EXT_ZBC),
> > > > +       __RISCV_ISA_EXT_DATA(zbkb, RISCV_ISA_EXT_ZBKB),
> > > > +       __RISCV_ISA_EXT_DATA(zbkc, RISCV_ISA_EXT_ZBKC),
> > > > +       __RISCV_ISA_EXT_DATA(zbkx, RISCV_ISA_EXT_ZBKX),
> > > >         __RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
> > > > +       __RISCV_ISA_EXT_DATA(zknd, RISCV_ISA_EXT_ZKND),
> > > > +       __RISCV_ISA_EXT_DATA(zkne, RISCV_ISA_EXT_ZKNE),
> > > > +       __RISCV_ISA_EXT_DATA(zknh, RISCV_ISA_EXT_ZKNH),
> > > > +       __RISCV_ISA_EXT_DATA(zkr, RISCV_ISA_EXT_ZKR),
> > > > +       __RISCV_ISA_EXT_DATA(zksed, RISCV_ISA_EXT_ZKSED),
> > > > +       __RISCV_ISA_EXT_DATA(zksh, RISCV_ISA_EXT_ZKSH),
> > > > +       __RISCV_ISA_EXT_DATA(zkt, RISCV_ISA_EXT_ZKT),
> > > >         __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> > > >         __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> > > >         __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> > > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > > index bdcf460ea53d..447f853a5a4c 100644
> > > > --- a/arch/riscv/kernel/cpufeature.c
> > > > +++ b/arch/riscv/kernel/cpufeature.c
> > > > @@ -309,10 +309,40 @@ void __init riscv_fill_hwcap(void)
> > > >                                 SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> > > >                                 SET_ISA_EXT_MAP("zba", RISCV_ISA_EXT_ZBA);
> > > >                                 SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
> > > > +                               SET_ISA_EXT_MAP("zbc", RISCV_ISA_EXT_ZBC);
> > > >                                 SET_ISA_EXT_MAP("zbs", RISCV_ISA_EXT_ZBS);
> > > > +                               SET_ISA_EXT_MAP("zbkb", RISCV_ISA_EXT_ZBKB);
> > 
> > This order does not look correct, please add them in alphanumerical
> > order as the comment these SET_ISA_EXT_MAP()s requests. Ditto below.
> 
> Agreed. Seems that I did not worked carefully for this part.

Or it could be me when rebasing that patch. In any case, it will be
fixed with v2 of the patch.

Cheers,
Samuel.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ