redsees
Junior Member level 1
Hello World,
A while ago, I posted a thread here asking about something in my Simplified DES Encryption Algorithm Verilog design.
Everyone looked at the code said that it was a hardware design written by a software programmer, and that was I using a bad practices in my code.
That was my software-like Verilog code:
After getting a few advices from some of the experts in this forum. I decided to redesign the whole algorithm taking everything you said into consideration.
I ended up with the following code.
Of course, I'm not asking for a complete code review or any tracing. My question is what do you think about the new design? Is it still a hardware design written by a software programmer or it is a bit better?
Also, if anyone has any additional advices about the design, I'll be happy to read it.
Much Regards
A while ago, I posted a thread here asking about something in my Simplified DES Encryption Algorithm Verilog design.
Everyone looked at the code said that it was a hardware design written by a software programmer, and that was I using a bad practices in my code.
That was my software-like Verilog code:
Code Verilog - [expand] 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 /* Simplified Data Encryption Standard (S-DES) 8-bit 2-round block cipher encryption and decryption algorithm using 10-bit key. */ module SDES(input en, input[0:9] key, input[0:7] plaintext, input encrypt, output[0:7] ciphertext); reg[0:7] tempB, tempD, IP_out, sw_out, iip_out, feistel_out; assign ciphertext = (en)?(iip_out):(8'b0); // Generate Key1 and Key2 task GenerateKeys; input[0:9] Key; output[0:7] Key1, Key2; reg[0:9] p10_out, tempA, tempC; begin // P10 Permutations p10_out = {Key[2], Key[4], Key[1], Key[6], Key[3], Key[9], Key[0], Key[8], Key[7], Key[5]}; // Left Rotation by 1 bit tempA = {p10_out[1:4], p10_out[0], p10_out[6:9], p10_out[5]}; // P8 Permutations Key1 = {tempA[5], tempA[2], tempA[6], tempA[3], tempA[7], tempA[4], tempA[9], tempA[8]}; // Left Rotation by 3 bits tempC = {p10_out[3:4], p10_out[0:2], p10_out[8:9], p10_out[5:7]}; // P8 Permutations Key2 = {tempC[5], tempC[2], tempC[6], tempC[3], tempC[7], tempC[4], tempC[9], tempC[8]}; end endtask // Feistel Function task Feistel; input[0:7] inp_block, key; output[0:7] out_block; reg[0:3] first_chunk, second_chunk, xor_fout, xor_f1, xor_f2, p4_in, p4_out; reg[0:7] EP_out, xor_out; reg[0:1] s0_out, s1_out; begin first_chunk = inp_block[0:3]; second_chunk = inp_block[4:7]; EP_out = {second_chunk[3], second_chunk[0], second_chunk[1], second_chunk[2], second_chunk[1], second_chunk[2], second_chunk[3], second_chunk[0]}; xor_out = EP_out ^ key; xor_f1 = xor_out[0:3]; xor_f2 = xor_out[4:7]; S0_Box(xor_f1, s0_out); S1_Box(xor_f2, s1_out); p4_in = {s0_out, s1_out}; p4_out = {p4_in[1],p4_in[3],p4_in[2],p4_in[0]}; xor_fout = p4_out ^ first_chunk; out_block = {xor_fout, second_chunk}; end endtask // S0 Box task S0_Box; input[0:3] inp_bits; output[0:1] out_bits; begin case(inp_bits) 4'b0000: out_bits = 2'b01; 4'b0001: out_bits = 2'b11; 4'b0010: out_bits = 2'b00; 4'b0011: out_bits = 2'b10; 4'b0100: out_bits = 2'b11; 4'b0101: out_bits = 2'b01; 4'b0110: out_bits = 2'b10; 4'b0111: out_bits = 2'b00; 4'b1000: out_bits = 2'b00; 4'b1001: out_bits = 2'b11; 4'b1010: out_bits = 2'b10; 4'b1011: out_bits = 2'b01; 4'b1100: out_bits = 2'b01; 4'b1101: out_bits = 2'b11; 4'b1110: out_bits = 2'b11; 4'b1111: out_bits = 2'b10; endcase end endtask // S1 Box task S1_Box; input[0:3] inp_bits; output[0:1] out_bits; begin case(inp_bits) 4'b0000: out_bits = 2'b00; 4'b0001: out_bits = 2'b10; 4'b0010: out_bits = 2'b01; 4'b0011: out_bits = 2'b00; 4'b0100: out_bits = 2'b10; 4'b0101: out_bits = 2'b01; 4'b0110: out_bits = 2'b11; 4'b0111: out_bits = 2'b11; 4'b1000: out_bits = 2'b11; 4'b1001: out_bits = 2'b10; 4'b1010: out_bits = 2'b00; 4'b1011: out_bits = 2'b01; 4'b1100: out_bits = 2'b01; 4'b1101: out_bits = 2'b00; 4'b1110: out_bits = 2'b00; 4'b1111: out_bits = 2'b11; endcase end endtask //always@(plaintext or key or encrypt) always@(en) begin if(en) begin // Generate Key1 and Key2 GenerateKeys(key, tempB, tempD); // Initial Permutation IP_out = {plaintext[1], plaintext[5], plaintext[2], plaintext[0], plaintext[3], plaintext[7], plaintext[4], plaintext[6]}; // First Round if(encrypt) Feistel(IP_out, tempB, feistel_out); else Feistel(IP_out, tempD, feistel_out); // Swapping sw_out = {feistel_out[4:7], feistel_out[0:3]}; // Second Round if(encrypt) Feistel(sw_out, tempD, feistel_out); else Feistel(sw_out, tempB, feistel_out); // Inverse Initial Permutation iip_out = {feistel_out[3], feistel_out[0], feistel_out[2], feistel_out[4], feistel_out[6], feistel_out[1], feistel_out[7], feistel_out[5]}; end end endmodule
After getting a few advices from some of the experts in this forum. I decided to redesign the whole algorithm taking everything you said into consideration.
I ended up with the following code.
Code Verilog - [expand] 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 /* Simplified Data Encryption Standard (S-DES) 8-bit 2-round block cipher symmetric-key algorithm using 10-bit key. Encryption/Decryption routine takes total of 7 clock cycles */ module SDES( en, // Enable Bit CLK, // Clock signal Key, // 10-bit Key plaintext, // 8-bit data block encrypt, // Encryption flag, 1 for encryption, 0 for decryption ciphertext // 8-bit data block ); input en; input encrypt; input CLK; input [0:7] plaintext; input [0:9] Key; output [0:7] ciphertext; // Internal signals reg [0:7] sw_out, iip_out, xor_f1, xor_f2, p4_in, s0_out, s1_out; reg [0:2] cnt = 3'b000; reg [0:7] Key1, Key2; assign ciphertext = (en)?(iip_out):(8'b0); always@(cnt) begin if(cnt == 3'b110) cnt = 3'b000; end always@(posedge CLK) begin Key1 = {Key[0], Key[6], Key[8], Key[3], Key[7], Key[2], Key[9], Key[5]}; Key2 = {Key[7], Key[2], Key[5], Key[4], Key[9], Key[1], Key[8], Key[0]}; end // Feistel Function Round One always@(posedge CLK) begin if(cnt < 3'b011) begin if(encrypt) begin xor_f1 <= { plaintext[6] ^ Key1[0], plaintext[3] ^ Key1[1], plaintext[7] ^ Key1[2], plaintext[4] ^ Key1[3] }; xor_f2 <= { plaintext[7] ^ Key1[4], plaintext[4] ^ Key1[5], plaintext[6] ^ Key1[6], plaintext[3] ^ Key1[7] }; end else begin xor_f1 <= { plaintext[6] ^ Key2[0], plaintext[3] ^ Key2[1], plaintext[7] ^ Key2[2], plaintext[4] ^ Key2[3] }; xor_f2 <= { plaintext[7] ^ Key2[4], plaintext[4] ^ Key2[5], plaintext[6] ^ Key2[6], plaintext[3] ^ Key2[7] }; end p4_in <= {s0_out, s1_out}; cnt <= cnt + 1'b1; end end // Feistel Function Round Two always@(posedge CLK) begin if(cnt == 3'b011) begin if(encrypt) begin xor_f1 <= { p4_in[0] ^ plaintext[0] ^ Key2[0], p4_in[1] ^ plaintext[1] ^ Key2[1], p4_in[3] ^ plaintext[5] ^ Key2[2], p4_in[2] ^ plaintext[2] ^ Key2[3] }; xor_f2 <= { p4_in[3] ^ plaintext[5] ^ Key2[4], p4_in[2] ^ plaintext[2] ^ Key2[5], p4_in[0] ^ plaintext[0] ^ Key2[6], p4_in[1] ^ plaintext[1] ^ Key2[7] }; end else begin xor_f1 <= { p4_in[0] ^ plaintext[0] ^ Key1[0], p4_in[1] ^ plaintext[1] ^ Key1[1], p4_in[3] ^ plaintext[5] ^ Key1[2], p4_in[2] ^ plaintext[2] ^ Key1[3] }; xor_f2 <= { p4_in[3] ^ plaintext[5] ^ Key1[4], p4_in[2] ^ plaintext[2] ^ Key1[5], p4_in[0] ^ plaintext[0] ^ Key1[6], p4_in[1] ^ plaintext[1] ^ Key1[7] }; end p4_in <= {s0_out, s1_out}; iip_out <= { p4_in[0] ^ plaintext[6], p4_in[1] ^ plaintext[3], p4_in[2] ^ plaintext[4], p4_in[1] ^ plaintext[1], p4_in[2] ^ plaintext[2], p4_in[3] ^ plaintext[7], p4_in[0] ^ plaintext[0], p4_in[3] ^ plaintext[5] }; cnt <= cnt + 1'b1; end end // S0 Box always@(xor_f1) begin case(xor_f1) 4'b0000: s0_out = 2'b01; 4'b0001: s0_out = 2'b11; 4'b0010: s0_out = 2'b00; 4'b0011: s0_out = 2'b10; 4'b0100: s0_out = 2'b11; 4'b0101: s0_out = 2'b01; 4'b0110: s0_out = 2'b10; 4'b0111: s0_out = 2'b00; 4'b1000: s0_out = 2'b00; 4'b1001: s0_out = 2'b11; 4'b1010: s0_out = 2'b10; 4'b1011: s0_out = 2'b01; 4'b1100: s0_out = 2'b01; 4'b1101: s0_out = 2'b11; 4'b1110: s0_out = 2'b11; 4'b1111: s0_out = 2'b10; endcase end // S1 Box always@(xor_f2) begin case(xor_f2) 4'b0000: s1_out = 2'b00; 4'b0001: s1_out = 2'b10; 4'b0010: s1_out = 2'b01; 4'b0011: s1_out = 2'b00; 4'b0100: s1_out = 2'b10; 4'b0101: s1_out = 2'b01; 4'b0110: s1_out = 2'b11; 4'b0111: s1_out = 2'b11; 4'b1000: s1_out = 2'b11; 4'b1001: s1_out = 2'b10; 4'b1010: s1_out = 2'b00; 4'b1011: s1_out = 2'b01; 4'b1100: s1_out = 2'b01; 4'b1101: s1_out = 2'b00; 4'b1110: s1_out = 2'b00; 4'b1111: s1_out = 2'b11; endcase end endmodule
Of course, I'm not asking for a complete code review or any tracing. My question is what do you think about the new design? Is it still a hardware design written by a software programmer or it is a bit better?
Also, if anyone has any additional advices about the design, I'll be happy to read it.
Much Regards