[PATCH v2 1/3] RISC-V: Minimal support of scalar crypto extension

[Jan Beulich]

On 03.11.2021 09:33, 陈嘉炜 wrote:
> "Jan Beulich" <jbeulich@suse.com>写道：
> > On 03.11.2021 02:54, jiawei wrote:
> > > Fix subextension order set by Nelson's suggestion.
> > 
> > Isn't this rather the revision log than the description of the
> > change?
> 
> Hi Jan, here is the log of discussion:

Right, which is why I was suspecting that what looks to be the description
of this patch is in fact the revision log. (The former ought to go into
git, while the latter shouldn't.)

Jan

> >>On Tue, Nov 2, 2021 at 8:23 PM 陈嘉炜 <jiawei@iscas.ac.cn> wrote:
> 
> >> Hi Nelson,
> 
> >> Thank you for the check, if we undo with this patch, here comes another unexpected results same as you mentioned:
> 
> >> riscv64-unknown-elf-as -misa-spec=20191213 -march=rv64i_zbb_zknd empty.s -o empty.o
> >> Assembler messages:
> >> Error: rv64i_zicsr_zifencei_zbb_zknd: prefixed ISA extension `zknd' is not in expected order.  It must come before `zbb'
> 
> >> I think that zb* should goes before zk*, so I change the order at first patch to make 'zb' before 'zk'. Sorry for that I missed zicsr， 
> >> zifencei and don't check it.
> 
> >> What should the right order if we have a string both contain 'zb', 'zk' even 'zp', 'zv' like  -march=rv64i_zicsr_zifencei_zbb_zknd?
> 
> >> You can add the entry for k in the riscv_supported_std_ext, for example,
> 
> >> @@ -1127,6 +1127,7 @@ static struct riscv_supported_ext
> >> riscv_supported_std_ext[] =
>    {"c",                ISA_SPEC_CLASS_20190608,        2, 0, 0 },
>    {"c",                ISA_SPEC_CLASS_2P2,             2, 0, 0 },
>    {"b",                ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION,
> RISCV_UNKNOWN_VERSION, 0 },
> +  {"k",         ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION,
> RISCV_UNKNOWN_VERSION, 0 },
>    {"j",                ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION,
> RISCV_UNKNOWN_VERSION, 0 },
>    {"t",                ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION,
> RISCV_UNKNOWN_VERSION, 0 },
>    {"p",                ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION,
> RISCV_UNKNOWN_VERSION, 0 },
> >>
> >> The standard extensions must be added in canonical order in the
> >> riscv_supported_std_ext table, since I used to use this table to build
> >> riscv_ext_order.  So you could find a proper place to add the entry
> >> for k extension.
> 
> >> "Nelson Chu" <nelson.chu@sifive.com>写道：
> >> > Hi jiawei,
> >> >
> >> > On Tue, Nov 2, 2021 at 5:44 PM jiawei <jiawei@iscas.ac.cn> wrote:
> >> > >
> >> > > ---
> >> > >  bfd/elfxx-riscv.c | 2 +-
> >> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> > >
> >> > > diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> >> > > index cdb4fa0996a..0d8fc755b5c 100644
> >> > > --- a/bfd/elfxx-riscv.c
> >> > > +++ b/bfd/elfxx-riscv.c
> >> > > @@ -1333,7 +1333,7 @@ riscv_compare_subsets (const char *subset1, const char *subset2)
> >> > >           order1 = riscv_ext_order[(*++subset1 - 'a')];
> >> > >           order2 = riscv_ext_order[(*++subset2 - 'a')];
> >> > >           if (order1 != order2)
> >> > > -           return order1 - order2;
> >> > > +           return order2 - order1;
> >> > >         }
> >> > >        return strcasecmp (++subset1, ++subset2);
> >> > >      }
> >> > > --
> >> > > 2.25.1
> >> > >
> >> >
> >> > After I apply this change, I get the unexpected results as follows,
> >> >
> >> > nelson@LAPTOP-QFSGI1F2:~/test$
> >> > ~/binutils-dev/build-linux64-upstream/build-install/bin/riscv64-unknown-linux-gnu-as
> >> > -misa-spec=20191213 -march=rv64i_zicsr_zifencei_zbb empty.s -o empty.o
> >> > Assembler messages:
> >> > Error: rv64i_zicsr_zifencei_zbb: prefixed ISA extension `zbb' is not
> >> > in expected order.  It must come before `zifencei'
> >> >
> >> > BTW, the expected result is as follows, before applying this patch,
> >> >
> >> > nelson@LAPTOP-QFSGI1F2:~/test$
> >> > ~/binutils-dev/build-linux64-upstream/build-install/bin/riscv64-unknown-linux-gnu-as
> >> > -misa-spec=20191213 -march=rv64i_zicsr_zifencei_zbb empty.s -o empty.o
> >> > nelson@LAPTOP-QFSGI1F2:~/test$
> >> > ~/binutils-dev/build-linux64-upstream/build-install/bin/riscv64-unknown-linux-gnu-readelf
> >> > -A empty.o                  Attribute Section: riscv
> >> > File Attributes
> >> >   Tag_RISCV_arch: "rv64i2p1_zicsr2p0_zifencei2p0_zbb1p0"
> >> >
> >> > Could I ask what issues or cases you want to resolve originally, so
> >> > that you need to change this?
> >> >
> >> > Thanks
> >> > Nelson
> >> </jiawei@iscas.ac.cn>
> 
> > > @@ -1146,6 +1161,19 @@ static struct riscv_supported_ext riscv_supported_std_z_ext[] =
> > >    {"zba",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
> > >    {"zbc",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
> > >    {"zbs",               ISA_SPEC_CLASS_DRAFT,           1, 0,  0 },
> > > +  {"zbkb",		ISA_SPEC_CLASS_DRAFT,		1, 0,  0 },
> > 
> > I seem to vaguely recall that sorting is supposed to be alphabetical,
> > which the insertion point here still would violate.
> > 
> > Jan
> 
> Since 'zbk*' is sub-extension of Scalar Cryptography extension(k extension), so I added 'zbk*' behind 'zb*', I'm not sure if we need to change it into alphabetical strictly.
> </jiawei@iscas.ac.cn>
>