-
Notifications
You must be signed in to change notification settings - Fork 705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding support for Scalar Crypto Extension (Bitmanip instructions for Cryptography, Zbkb) #2653
base: master
Are you sure you want to change the base?
Conversation
core/alu.sv
Outdated
PACK: result_o = (CVA6Cfg.IS_XLEN32) ? ({fu_data_i.operand_b[15:0], fu_data_i.operand_a[15:0]}) : ({fu_data_i.operand_b[31:0], fu_data_i.operand_a[31:0]}); | ||
PACK_H: result_o = (CVA6Cfg.IS_XLEN32) ? ({16'b0, fu_data_i.operand_b[7:0], fu_data_i.operand_a[7:0]}) : ({48'b0, fu_data_i.operand_b[7:0], fu_data_i.operand_a[7:0]}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[verible-verilog-format] reported by reviewdog 🐶
PACK: result_o = (CVA6Cfg.IS_XLEN32) ? ({fu_data_i.operand_b[15:0], fu_data_i.operand_a[15:0]}) : ({fu_data_i.operand_b[31:0], fu_data_i.operand_a[31:0]}); | |
PACK_H: result_o = (CVA6Cfg.IS_XLEN32) ? ({16'b0, fu_data_i.operand_b[7:0], fu_data_i.operand_a[7:0]}) : ({48'b0, fu_data_i.operand_b[7:0], fu_data_i.operand_a[7:0]}); | |
PACK: | |
result_o = (CVA6Cfg.IS_XLEN32) ? ({fu_data_i.operand_b[15:0], fu_data_i.operand_a[15:0]}) : ({fu_data_i.operand_b[31:0], fu_data_i.operand_a[31:0]}); | |
PACK_H: | |
result_o = (CVA6Cfg.IS_XLEN32) ? ({16'b0, fu_data_i.operand_b[7:0], fu_data_i.operand_a[7:0]}) : ({48'b0, fu_data_i.operand_b[7:0], fu_data_i.operand_a[7:0]}); |
core/alu.sv
Outdated
BREV8: result_o = brev8_reversed; | ||
default: ; | ||
endcase | ||
if (fu_data_i.operation == PACK_W && CVA6Cfg.IS_XLEN64) result_o = {{32{fu_data_i.operand_b[15]}}, {fu_data_i.operand_b[15:0]}, {fu_data_i.operand_a[15:0]}}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[verible-verilog-format] reported by reviewdog 🐶
if (fu_data_i.operation == PACK_W && CVA6Cfg.IS_XLEN64) result_o = {{32{fu_data_i.operand_b[15]}}, {fu_data_i.operand_b[15:0]}, {fu_data_i.operand_a[15:0]}}; | |
if (fu_data_i.operation == PACK_W && CVA6Cfg.IS_XLEN64) | |
result_o = { | |
{32{fu_data_i.operand_b[15]}}, {fu_data_i.operand_b[15:0]}, {fu_data_i.operand_a[15:0]} | |
}; |
core/decoder.sv
Outdated
{7'b000_0100, 3'b100} : if (CVA6Cfg.ZKN) instruction_o.op = ariane_pkg::PACK; else illegal_instr_bm = 1'b1; //pack | ||
{7'b000_0100, 3'b111} : if (CVA6Cfg.ZKN) instruction_o.op = ariane_pkg::PACK_H; else illegal_instr_bm = 1'b1; //packh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[verible-verilog-format] reported by reviewdog 🐶
{7'b000_0100, 3'b100} : if (CVA6Cfg.ZKN) instruction_o.op = ariane_pkg::PACK; else illegal_instr_bm = 1'b1; //pack | |
{7'b000_0100, 3'b111} : if (CVA6Cfg.ZKN) instruction_o.op = ariane_pkg::PACK_H; else illegal_instr_bm = 1'b1; //packh | |
{ | |
7'b000_0100, 3'b100 | |
} : | |
if (CVA6Cfg.ZKN) instruction_o.op = ariane_pkg::PACK; | |
else illegal_instr_bm = 1'b1; //pack | |
{ | |
7'b000_0100, 3'b111 | |
} : | |
if (CVA6Cfg.ZKN) instruction_o.op = ariane_pkg::PACK_H; | |
else illegal_instr_bm = 1'b1; //packh |
core/decoder.sv
Outdated
{7'b011_0000, 3'b001}: instruction_o.op = ariane_pkg::ROLW; // rolw | ||
{7'b011_0000, 3'b101}: instruction_o.op = ariane_pkg::RORW; // rorw |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[verible-verilog-format] reported by reviewdog 🐶
{7'b011_0000, 3'b001}: instruction_o.op = ariane_pkg::ROLW; // rolw | |
{7'b011_0000, 3'b101}: instruction_o.op = ariane_pkg::RORW; // rorw | |
{7'b011_0000, 3'b001} : instruction_o.op = ariane_pkg::ROLW; // rolw | |
{7'b011_0000, 3'b101} : instruction_o.op = ariane_pkg::RORW; // rorw |
core/decoder.sv
Outdated
@@ -851,6 +854,8 @@ module decoder | |||
// Bitwise Shifting | |||
{7'b011_0000, 3'b001}: instruction_o.op = ariane_pkg::ROLW; // rolw | |||
{7'b011_0000, 3'b101}: instruction_o.op = ariane_pkg::RORW; // rorw | |||
// Pack_W | |||
{7'b000_0100, 3'b100}: if (CVA6Cfg.ZKN) instruction_o.op = ariane_pkg::PACK_W; else illegal_instr_bm = 1'b1; //packw |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[verible-verilog-format] reported by reviewdog 🐶
{7'b000_0100, 3'b100}: if (CVA6Cfg.ZKN) instruction_o.op = ariane_pkg::PACK_W; else illegal_instr_bm = 1'b1; //packw | |
{ | |
7'b000_0100, 3'b100 | |
} : | |
if (CVA6Cfg.ZKN) instruction_o.op = ariane_pkg::PACK_W; | |
else illegal_instr_bm = 1'b1; //packw |
Great! To solve the Verible format issues, I suggest to use the command reported in CONTRIBUTING.md. |
❌ failed run, report available here. |
First comments:
|
Thanks @JeanRochCoulon . Sure, we will disable it for |
core/alu.sv
Outdated
result_o = { | ||
{32{fu_data_i.operand_b[15]}}, {fu_data_i.operand_b[15:0]}, {fu_data_i.operand_a[15:0]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[verible-verilog-format] reported by reviewdog 🐶
result_o = { | |
{32{fu_data_i.operand_b[15]}}, {fu_data_i.operand_b[15:0]}, {fu_data_i.operand_a[15:0]} | |
result_o = { | |
{32{fu_data_i.operand_b[15]}}, {fu_data_i.operand_b[15:0]}, {fu_data_i.operand_a[15:0]} |
core/decoder.sv
Outdated
// Packing | ||
{ | ||
7'b000_0100, 3'b100 | ||
} : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[verible-verilog-format] reported by reviewdog 🐶
} : | |
} : |
core/decoder.sv
Outdated
} : | ||
if (CVA6Cfg.ZKN) instruction_o.op = ariane_pkg::PACK_H; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[verible-verilog-format] reported by reviewdog 🐶
} : | |
if (CVA6Cfg.ZKN) instruction_o.op = ariane_pkg::PACK_H; | |
} : | |
if (CVA6Cfg.ZKN) instruction_o.op = ariane_pkg::PACK_H; |
core/decoder.sv
Outdated
@@ -849,8 +860,14 @@ module decoder | |||
// Unsigned word Op's | |||
{7'b000_0100, 3'b000}: instruction_o.op = ariane_pkg::ADDUW; // add.uw |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[verible-verilog-format] reported by reviewdog 🐶
{7'b000_0100, 3'b000}: instruction_o.op = ariane_pkg::ADDUW; // add.uw | |
{7'b000_0100, 3'b000} : instruction_o.op = ariane_pkg::ADDUW; // add.uw |
core/decoder.sv
Outdated
} : | ||
if (CVA6Cfg.ZKN) instruction_o.op = ariane_pkg::PACK_W; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[verible-verilog-format] reported by reviewdog 🐶
} : | |
if (CVA6Cfg.ZKN) instruction_o.op = ariane_pkg::PACK_W; | |
} : | |
if (CVA6Cfg.ZKN) instruction_o.op = ariane_pkg::PACK_W; |
❌ failed run, report available here. |
@@ -879,14 +879,14 @@ def load_config(args, cwd): | |||
args.isa = "rv64gch_zba_zbb_zbs_zbc" | |||
elif base in ("cv64a6_imafdc_sv39", "cv64a6_imafdc_sv39_hpdcache", "cv64a6_imafdc_sv39_wb"): | |||
args.mabi = "lp64d" | |||
args.isa = "rv64gc_zba_zbb_zbs_zbc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cv64a6_imafdc_sv39_wb does not support zbkb, this has to be fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we want to enable this extension for the basic 64-bit config as well, which is cv64a6_imafdc_sv39
@munailwaqar lets only enable it for cv64a6_imafdc_sv39.
elif base == "cv32a60x": | ||
args.mabi = "ilp32" | ||
args.isa = "rv32imc_zba_zbb_zbs_zbc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
60x and 65x do not support zbkb
ae24f11
to
cf404bf
Compare
Currently enabling Zbkb in 2 configs only:
|
❌ failed run, report available here. |
core/decoder.sv
Outdated
{ | ||
7'b000_0100, 3'b100 | ||
} : begin | ||
if (instr.instr[24:20] == 5'b00000) instruction_o.op = ariane_pkg::ZEXTH; // Zero Extend Op RV64 encoding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[verible-verilog-format] reported by reviewdog 🐶
if (instr.instr[24:20] == 5'b00000) instruction_o.op = ariane_pkg::ZEXTH; // Zero Extend Op RV64 encoding | |
if (instr.instr[24:20] == 5'b00000) | |
instruction_o.op = ariane_pkg::ZEXTH; // Zero Extend Op RV64 encoding |
❌ failed run, report available here. |
✔️ successful run, report available here. |
1 similar comment
✔️ successful run, report available here. |
Hi @JeanRochCoulon, can you please approve this? |
@@ -24,7 +24,7 @@ package cva6_config_pkg; | |||
localparam CVA6ConfigZcmpExtEn = 0; | |||
localparam CVA6ConfigAExtEn = 1; | |||
localparam CVA6ConfigHExtEn = 0; // always disabled | |||
localparam CVA6ConfigBExtEn = 0; | |||
localparam CVA6ConfigBExtEn = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This localparam can be removed, because not used
Hello @munailwaqar |
✔️ successful run, report available here. |
@JeanRochCoulon Added the relevant arch tests to the testlist but I also had to enable the Zbkb extension in cv64a6_imafdc_sv39_hpdcache config as the CI runs checks for this config too using the same testlist. |
✔️ successful run, report available here. |
Hi @JeanRochCoulon can you please review this again? Thanks. |
Hello @munailwaqar |
@JeanRochCoulon Yes. zip and unzip instructions are only for 32bit architectures according to the documentation. |
I cannot rebase because I have no right on your branch, can you ? |
Yes ill do it.
…On Tue, 17 Dec 2024, 9:20 pm JeanRochCoulon, ***@***.***> wrote:
I cannot rebase because I have no right on your branch, can you ?
—
Reply to this email directly, view it on GitHub
<#2653 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AXNX42OWES2CPVEEJURGTAL2GBFNDAVCNFSM6AAAAABTCKXVQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNBYHEZDOOJXGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
9166ab7
to
7f8a34e
Compare
@JeanRochCoulon I have rebased the branch. |
❌ failed run, report available here. |
LGTM. I am ready to merge. |
Introduction
This PR adds support for Zbkb extension in the CVA6 core. It also adds the documentation for this extension. These changes have been tested with self-written single instruction tests and with the riscv-arch-tests. This PR will be followed by other PRs that will add complete support for the Zkn - NIST Algorithm Suite extension.
Implementation
Zbkb Extension:
Added support for the Zbkb instruction set. It essentially expands the Zbb extension with additional instructions useful in cryptography. These instructions are pack, packh, packw, brev8, unzip and zip.
Modifications
Documentation and Reference
The official RISC-V Cryptography Extensions Volume I was followed to ensure alignment with ratification. The relevant documentation for the Zbkb instruction was also added.
Verification
Assembly Tests:
The instructions were tested and verified with the K module of both 32 bit and 64 bit versions of the riscv-arch-tests to ensure proper functionality. These tests check for ISA compliance, edge cases and use assertions to ensure expected behavior. The tests include: