[v7] RISC-V Pointer Masking implementation

[PATCH v7 0/6] RISC-V Pointer Masking implementation

Posted by Alexey Baturo 3 years, 4 months ago

Hi folks,

Sorry it took me almost 3 month to provide the reply and fixes: it was a really busy EOY.
This series contains fixed @Alistair suggestion on enabling J-ext.

As for @Richard comments:
- Indeed I've missed appending review-by to the approved commits. Now I've restored them except for the fourth commit. @Richard could you please tell if you think it's still ok to commit it as is, or should I support masking mem ops for RVV first?
- These patches don't have any support for load/store masking for RVV and RVH extensions, so no support for special load/store for Hypervisor in particular.

If this patch series would be accepted, I think my further attention would be to:
- Support pm for memory operations for RVV
- Add proper csr and support pm for memory operations for Hypervisor mode
- Support address wrapping on unaligned accesses as @Richard mentioned previously

Thanks!

Alexey Baturo (5):
  [RISCV_PM] Add J-extension into RISC-V
  [RISCV_PM] Support CSRs required for RISC-V PM extension except for
    the ones required for hypervisor mode
  [RISCV_PM] Print new PM CSRs in QEMU logs
  [RISCV_PM] Support pointer masking for RISC-V for i/c/f/d/a types of
    instructions
  [RISCV_PM] Allow experimental J-ext to be turned on

Anatoly Parshintsev (1):
  [RISCV_PM] Implement address masking functions required for RISC-V
    Pointer Masking extension

 target/riscv/cpu.c                      |  30 +++
 target/riscv/cpu.h                      |  33 +++
 target/riscv/cpu_bits.h                 |  66 ++++++
 target/riscv/csr.c                      | 271 ++++++++++++++++++++++++
 target/riscv/insn_trans/trans_rva.c.inc |   3 +
 target/riscv/insn_trans/trans_rvd.c.inc |   2 +
 target/riscv/insn_trans/trans_rvf.c.inc |   2 +
 target/riscv/insn_trans/trans_rvi.c.inc |   2 +
 target/riscv/translate.c                |  44 ++++
 9 files changed, 453 insertions(+)

-- 
2.20.1

Re: [PATCH v7 0/6] RISC-V Pointer Masking implementation

Posted by Alistair Francis 3 years, 3 months ago

On Sun, Jan 10, 2021 at 10:54 AM Alexey Baturo <baturo.alexey@gmail.com> wrote:
>
> Hi folks,
>
> Sorry it took me almost 3 month to provide the reply and fixes: it was a really busy EOY.
> This series contains fixed @Alistair suggestion on enabling J-ext.
>
> As for @Richard comments:
> - Indeed I've missed appending review-by to the approved commits. Now I've restored them except for the fourth commit. @Richard could you please tell if you think it's still ok to commit it as is, or should I support masking mem ops for RVV first?
> - These patches don't have any support for load/store masking for RVV and RVH extensions, so no support for special load/store for Hypervisor in particular.
>
> If this patch series would be accepted, I think my further attention would be to:
> - Support pm for memory operations for RVV
> - Add proper csr and support pm for memory operations for Hypervisor mode
> - Support address wrapping on unaligned accesses as @Richard mentioned previously

Overall this looks fine.

Unfortunately it doesn't look like there is a release of the pointer
masking spec. Until there is a release (a draft release counts) we
can't accept it. We need a version to point to so that we can say "we
support v0.1 of the RISC-V pointer masking spec". Otherwise we are
chasing a moving target and users don't know what version we do/don't
support.

Do you know the current state of the spec?

Alistair

>
> Thanks!
>
> Alexey Baturo (5):
>   [RISCV_PM] Add J-extension into RISC-V
>   [RISCV_PM] Support CSRs required for RISC-V PM extension except for
>     the ones required for hypervisor mode
>   [RISCV_PM] Print new PM CSRs in QEMU logs
>   [RISCV_PM] Support pointer masking for RISC-V for i/c/f/d/a types of
>     instructions
>   [RISCV_PM] Allow experimental J-ext to be turned on
>
> Anatoly Parshintsev (1):
>   [RISCV_PM] Implement address masking functions required for RISC-V
>     Pointer Masking extension
>
>  target/riscv/cpu.c                      |  30 +++
>  target/riscv/cpu.h                      |  33 +++
>  target/riscv/cpu_bits.h                 |  66 ++++++
>  target/riscv/csr.c                      | 271 ++++++++++++++++++++++++
>  target/riscv/insn_trans/trans_rva.c.inc |   3 +
>  target/riscv/insn_trans/trans_rvd.c.inc |   2 +
>  target/riscv/insn_trans/trans_rvf.c.inc |   2 +
>  target/riscv/insn_trans/trans_rvi.c.inc |   2 +
>  target/riscv/translate.c                |  44 ++++
>  9 files changed, 453 insertions(+)
>
> --
> 2.20.1
>
>

Re: [PATCH v7 0/6] RISC-V Pointer Masking implementation

Posted by Alexey Baturo 3 years, 3 months ago

Hi Alistair,

Sorry for the late reply.

>Do you know the current state of the spec?
As far as I can tell, the spec for PointerMasking is quite frozen: it has
not been updated for quite some time, but some of the clarifications have
not been included in the actual spec.
I'll ask the J workgroup if they could do draft release v0.1, while I'll
address the fixed you proposed in this patch series.
Meanwhile could you please tell, if it's ok to push several series of
patches to claim the support of i.e. v0.1 or it's mandatory to push all the
patches at once?

Thanks!

ср, 3 февр. 2021 г. в 22:22, Alistair Francis <alistair23@gmail.com>:

> On Sun, Jan 10, 2021 at 10:54 AM Alexey Baturo <baturo.alexey@gmail.com>
> wrote:
> >
> > Hi folks,
> >
> > Sorry it took me almost 3 month to provide the reply and fixes: it was a
> really busy EOY.
> > This series contains fixed @Alistair suggestion on enabling J-ext.
> >
> > As for @Richard comments:
> > - Indeed I've missed appending review-by to the approved commits. Now
> I've restored them except for the fourth commit. @Richard could you please
> tell if you think it's still ok to commit it as is, or should I support
> masking mem ops for RVV first?
> > - These patches don't have any support for load/store masking for RVV
> and RVH extensions, so no support for special load/store for Hypervisor in
> particular.
> >
> > If this patch series would be accepted, I think my further attention
> would be to:
> > - Support pm for memory operations for RVV
> > - Add proper csr and support pm for memory operations for Hypervisor mode
> > - Support address wrapping on unaligned accesses as @Richard mentioned
> previously
>
> Overall this looks fine.
>
> Unfortunately it doesn't look like there is a release of the pointer
> masking spec. Until there is a release (a draft release counts) we
> can't accept it. We need a version to point to so that we can say "we
> support v0.1 of the RISC-V pointer masking spec". Otherwise we are
> chasing a moving target and users don't know what version we do/don't
> support.
>
> Do you know the current state of the spec?
>
> Alistair
>
> >
> > Thanks!
> >
> > Alexey Baturo (5):
> >   [RISCV_PM] Add J-extension into RISC-V
> >   [RISCV_PM] Support CSRs required for RISC-V PM extension except for
> >     the ones required for hypervisor mode
> >   [RISCV_PM] Print new PM CSRs in QEMU logs
> >   [RISCV_PM] Support pointer masking for RISC-V for i/c/f/d/a types of
> >     instructions
> >   [RISCV_PM] Allow experimental J-ext to be turned on
> >
> > Anatoly Parshintsev (1):
> >   [RISCV_PM] Implement address masking functions required for RISC-V
> >     Pointer Masking extension
> >
> >  target/riscv/cpu.c                      |  30 +++
> >  target/riscv/cpu.h                      |  33 +++
> >  target/riscv/cpu_bits.h                 |  66 ++++++
> >  target/riscv/csr.c                      | 271 ++++++++++++++++++++++++
> >  target/riscv/insn_trans/trans_rva.c.inc |   3 +
> >  target/riscv/insn_trans/trans_rvd.c.inc |   2 +
> >  target/riscv/insn_trans/trans_rvf.c.inc |   2 +
> >  target/riscv/insn_trans/trans_rvi.c.inc |   2 +
> >  target/riscv/translate.c                |  44 ++++
> >  9 files changed, 453 insertions(+)
> >
> > --
> > 2.20.1
> >
> >
>

Re: [PATCH v7 0/6] RISC-V Pointer Masking implementation

Posted by Alistair Francis 3 years, 3 months ago

On Mon, Feb 15, 2021 at 12:52 PM Alexey Baturo <baturo.alexey@gmail.com> wrote:
>
> Hi Alistair,
>
> Sorry for the late reply.
>
> >Do you know the current state of the spec?
> As far as I can tell, the spec for PointerMasking is quite frozen: it has not been updated for quite some time, but some of the clarifications have not been included in the actual spec.
> I'll ask the J workgroup if they could do draft release v0.1, while I'll address the fixed you proposed in this patch series.
> Meanwhile could you please tell, if it's ok to push several series of patches to claim the support of i.e. v0.1 or it's mandatory to push all the patches at once?

You can post multiple patches on the list, but I wouldn't claim to
support v0.1 until there is a v0.1 to claim.

For actual changes being merged we can only claim to support v0.1 when
there is a v0.1 and it is supported. We can push the patches in
stages, for example we can merge some tidy ups before merging the full
support, but we can't allow a user to enable support (or advertise it)
until it's fully supported.

Alistair

>
> Thanks!
>
> ср, 3 февр. 2021 г. в 22:22, Alistair Francis <alistair23@gmail.com>:
>>
>> On Sun, Jan 10, 2021 at 10:54 AM Alexey Baturo <baturo.alexey@gmail.com> wrote:
>> >
>> > Hi folks,
>> >
>> > Sorry it took me almost 3 month to provide the reply and fixes: it was a really busy EOY.
>> > This series contains fixed @Alistair suggestion on enabling J-ext.
>> >
>> > As for @Richard comments:
>> > - Indeed I've missed appending review-by to the approved commits. Now I've restored them except for the fourth commit. @Richard could you please tell if you think it's still ok to commit it as is, or should I support masking mem ops for RVV first?
>> > - These patches don't have any support for load/store masking for RVV and RVH extensions, so no support for special load/store for Hypervisor in particular.
>> >
>> > If this patch series would be accepted, I think my further attention would be to:
>> > - Support pm for memory operations for RVV
>> > - Add proper csr and support pm for memory operations for Hypervisor mode
>> > - Support address wrapping on unaligned accesses as @Richard mentioned previously
>>
>> Overall this looks fine.
>>
>> Unfortunately it doesn't look like there is a release of the pointer
>> masking spec. Until there is a release (a draft release counts) we
>> can't accept it. We need a version to point to so that we can say "we
>> support v0.1 of the RISC-V pointer masking spec". Otherwise we are
>> chasing a moving target and users don't know what version we do/don't
>> support.
>>
>> Do you know the current state of the spec?
>>
>> Alistair
>>
>> >
>> > Thanks!
>> >
>> > Alexey Baturo (5):
>> >   [RISCV_PM] Add J-extension into RISC-V
>> >   [RISCV_PM] Support CSRs required for RISC-V PM extension except for
>> >     the ones required for hypervisor mode
>> >   [RISCV_PM] Print new PM CSRs in QEMU logs
>> >   [RISCV_PM] Support pointer masking for RISC-V for i/c/f/d/a types of
>> >     instructions
>> >   [RISCV_PM] Allow experimental J-ext to be turned on
>> >
>> > Anatoly Parshintsev (1):
>> >   [RISCV_PM] Implement address masking functions required for RISC-V
>> >     Pointer Masking extension
>> >
>> >  target/riscv/cpu.c                      |  30 +++
>> >  target/riscv/cpu.h                      |  33 +++
>> >  target/riscv/cpu_bits.h                 |  66 ++++++
>> >  target/riscv/csr.c                      | 271 ++++++++++++++++++++++++
>> >  target/riscv/insn_trans/trans_rva.c.inc |   3 +
>> >  target/riscv/insn_trans/trans_rvd.c.inc |   2 +
>> >  target/riscv/insn_trans/trans_rvf.c.inc |   2 +
>> >  target/riscv/insn_trans/trans_rvi.c.inc |   2 +
>> >  target/riscv/translate.c                |  44 ++++
>> >  9 files changed, 453 insertions(+)
>> >
>> > --
>> > 2.20.1
>> >
>> >