search

openhwgroup / cvw

CORE-V Wally is a configurable RISC-V Processor associated with RISC-V System-on-Chip Design textbook. Contains a 5-stage pipeline, support for A, B, C, D, F, M and Q extensions, and optional caches, BP, FPU, VM/MMU, AHB, RAMs, and peripherals.
Other
238 stars 175 forks source link

Issue with zknde32 K extension regression #828

Closed stineje closed 3 months ago

stineje commented 3 months ago

There seems to be an error. The shamt on the sbox does not convert correctly when its look at the state from a byte. The original code is on L42 of zknde32.sv

assign shamt = {bs, 3'b0};

The following will fix this error correctly

assign shamt = {3'b0, bs} << 3;

This will handle the shamt and allow the index select part to allow the conversion to be multiplied by x8. I am currently doing the PR for the JTAG so cannot put in at the moment. I am just putting this item in once I get the JTAG in or someone else can add it. Running regression-wally with this changer gives 0 errors.

davidharrishmc commented 3 months ago

The zknde tests pass quests. What tool is flagging an error?

On Fri, Jun 7, 2024 at 11:35 PM James E. Stine @.***> wrote:

There seems to be an error. The shamt on the sbox does not convert correctly when its look at the state from a byte. The original code is on L42 of zknde32.sv

assign shamt = {bs, 3'b0};

The following will fix this error correctly

assign shamt = {3'b0, bs} << 3;

This will handle the shamt and allow the index select part to allow the conversion to be multiplied by x8. I am currently doing the PR for the JTAG so cannot put in at the moment. I am just putting this item in once I get the JTAG in or someone else can add it. Running regression-wally with this changer gives 0 errors.

— Reply to this email directly, view it on GitHub https://github.com/openhwgroup/cvw/issues/828, or unsubscribe https://github.com/notifications/unsubscribe-auth/AR4AA34HD2RSIA6XEZLWVILZGIRTLAVCNFSM6AAAAABI7K4YEWVHI2DSMVQWIX3LMV43ASLTON2WKOZSGM2DCMJZGY4TMMA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

stineje commented 3 months ago

Mine does not and I verified it with a fresh test and pull . Can post shortly.

Sent from my iPhone

On Jun 7, 2024, at 16:49, David Harris @.***> wrote:



The zknde tests pass quests. What tool is flagging an error?

On Fri, Jun 7, 2024 at 11:35 PM James E. Stine @.***> wrote:

There seems to be an error. The shamt on the sbox does not convert correctly when its look at the state from a byte. The original code is on L42 of zknde32.sv

assign shamt = {bs, 3'b0};

The following will fix this error correctly

assign shamt = {3'b0, bs} << 3;

This will handle the shamt and allow the index select part to allow the conversion to be multiplied by x8. I am currently doing the PR for the JTAG so cannot put in at the moment. I am just putting this item in once I get the JTAG in or someone else can add it. Running regression-wally with this changer gives 0 errors.

— Reply to this email directly, view it on GitHub https://github.com/openhwgroup/cvw/issues/828, or unsubscribe https://github.com/notifications/unsubscribe-auth/AR4AA34HD2RSIA6XEZLWVILZGIRTLAVCNFSM6AAAAABI7K4YEWVHI2DSMVQWIX3LMV43ASLTON2WKOZSGM2DCMJZGY4TMMA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

— Reply to this email directly, view it on GitHubhttps://github.com/openhwgroup/cvw/issues/828#issuecomment-2155618138, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AFSIHPH33YIWFIVP247SEE3ZGITF3AVCNFSM6AAAAABI7K4YEWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJVGYYTQMJTHA. You are receiving this because you authored the thread.Message ID: @.***>

stineje commented 3 months ago

I verified with Kelvin and he helped me figure it out. I believe Rose also saw the error. We are using the following hash: 5dfde808f003bf0eb94a5627b5c1a0a40128d347

wsim --sim questa rv32gc arch32zkne > /home/jstine/cvw/sim/questa/logs/rv32gc_arch32zkne.log: Failures detected in output Check /home/jstine/cvw/sim/questa/logs/rv32gc_arch32zkne.log wsim --sim questa rv32gc arch32zknd > /home/jstine/cvw/sim/questa/logs/rv32gc_arch32zknd.log: Failures detected in output Check /home/jstine/cvw/sim/questa/logs/rv32gc_arch32zknd.log

I just tried again and get the same error. I sam confused why you have no error.

Loading work.cacheway(fast__1)

Error on test rv32i_m/K/src/aes32esi-01.S result 3: adr = 8000411c sim (D$) d069e102 signature = de69

e102

** Note: $stop : /home/jstine/cvw/testbench/testbench.sv(918)

Error on test rv32i_m/K/src/aes32dsi-01.S result 3: adr = 8000411c sim (D$) e169e102 signature = f869 e102 # ** Note: $stop : /home/jstine/cvw/testbench/testbench.sv(918)

I will try again to validate

On Jun 7, 2024, at 4:57 PM, Stine, James @.***> wrote:

Mine does not and I verified it with a fresh test and pull . Can post shortly.

Sent from my iPhone

On Jun 7, 2024, at 16:49, David Harris @.***> wrote:

 The zknde tests pass quests. What tool is flagging an error?

On Fri, Jun 7, 2024 at 11:35 PM James E. Stine @.***> wrote:

There seems to be an error. The shamt on the sbox does not convert correctly when its look at the state from a byte. The original code is on L42 of zknde32.sv

assign shamt = {bs, 3'b0};

The following will fix this error correctly

assign shamt = {3'b0, bs} << 3;

This will handle the shamt and allow the index select part to allow the conversion to be multiplied by x8. I am currently doing the PR for the JTAG so cannot put in at the moment. I am just putting this item in once I get the JTAG in or someone else can add it. Running regression-wally with this changer gives 0 errors.

— Reply to this email directly, view it on GitHub https://github.com/openhwgroup/cvw/issues/828, or unsubscribe https://github.com/notifications/unsubscribe-auth/AR4AA34HD2RSIA6XEZLWVILZGIRTLAVCNFSM6AAAAABI7K4YEWVHI2DSMVQWIX3LMV43ASLTON2WKOZSGM2DCMJZGY4TMMA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

stineje commented 3 months ago

I tried again and got the same error (5dfde808f003bf0eb94a5627b5c1a0a40128d347). Maybe it’s something I did but I think the error seems valid.

lint-wally | tee /home/jstine/cvw/sim/verilator/logs/all_lints.log: Success wsim --sim questa rv32gc arch32zknd > /home/jstine/cvw/sim/questa/logs/rv32gc_arch32zknd.log: Failures detected in output Check /home/jstine/cvw/sim/questa/logs/rv32gc_arch32zknd.log wsim --sim questa rv32gc wally32periph > /home/jstine/cvw/sim/questa/logs/rv32gc_wally32periph.log: Success wsim --sim questa rv32gc arch32zbkc > /home/jstine/cvw/sim/questa/logs/rv32gc_arch32zbkc.log: Success wsim --sim questa rv32gc arch32zkne > /home/jstine/cvw/sim/questa/logs/rv32gc_arch32zkne.log: Failures detected in output Check /home/jstine/cvw/sim/questa/logs/rv32gc_arch32zkne.log

On Jun 7, 2024, at 5:23 PM, Stine, James @.***> wrote:

I verified with Kelvin and he helped me figure it out. I believe Rose also saw the error. We are using the following hash: 5dfde808f003bf0eb94a5627b5c1a0a40128d347

wsim --sim questa rv32gc arch32zkne > /home/jstine/cvw/sim/questa/logs/rv32gc_arch32zkne.log: Failures detected in output Check /home/jstine/cvw/sim/questa/logs/rv32gc_arch32zkne.log wsim --sim questa rv32gc arch32zknd > /home/jstine/cvw/sim/questa/logs/rv32gc_arch32zknd.log: Failures detected in output Check /home/jstine/cvw/sim/questa/logs/rv32gc_arch32zknd.log

I just tried again and get the same error. I sam confused why you have no error.

Loading work.cacheway(fast__1)

Error on test rv32i_m/K/src/aes32esi-01.S result 3: adr = 8000411c sim (D$) d069e102 signature = de69

e102

** Note: $stop : /home/jstine/cvw/testbench/testbench.sv(918)

Error on test rv32i_m/K/src/aes32dsi-01.S result 3: adr = 8000411c sim (D$) e169e102 signature = f869 e102 # ** Note: $stop : /home/jstine/cvw/testbench/testbench.sv(918)

I will try again to validate

On Jun 7, 2024, at 4:57 PM, Stine, James @.***> wrote:

Mine does not and I verified it with a fresh test and pull . Can post shortly.

Sent from my iPhone

On Jun 7, 2024, at 16:49, David Harris @.***> wrote:

 The zknde tests pass quests. What tool is flagging an error?

On Fri, Jun 7, 2024 at 11:35 PM James E. Stine @.***> wrote:

There seems to be an error. The shamt on the sbox does not convert correctly when its look at the state from a byte. The original code is on L42 of zknde32.sv

assign shamt = {bs, 3'b0};

The following will fix this error correctly

assign shamt = {3'b0, bs} << 3;

This will handle the shamt and allow the index select part to allow the conversion to be multiplied by x8. I am currently doing the PR for the JTAG so cannot put in at the moment. I am just putting this item in once I get the JTAG in or someone else can add it. Running regression-wally with this changer gives 0 errors.

— Reply to this email directly, view it on GitHub https://github.com/openhwgroup/cvw/issues/828, or unsubscribe https://github.com/notifications/unsubscribe-auth/AR4AA34HD2RSIA6XEZLWVILZGIRTLAVCNFSM6AAAAABI7K4YEWVHI2DSMVQWIX3LMV43ASLTON2WKOZSGM2DCMJZGY4TMMA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

davidharrishmc commented 3 months ago

It's working for me. See below for versions.

I also don't see how your proposed fix changes anything. bs is 2 bits. shamt is 5 bits.
assign shamt = {bs, 3'b0}; // shamt = bs * 8 (convert bytes to bits) is a perfectly correct way to put bs in the top 2 bits of the 5-bit bus.

I presume you've pulled the latest version?

harris@chips:~/cvw$ wsim --sim questa  rv32gc arch32zkne
...
# rv32i_m/K/src/aes32esi-01.S succeeded.  Brilliant!!!
# rv32i_m/K/src/aes32esmi-01.S succeeded.  Brilliant!!!
# SUCCESS! All tests ran without failures.
# ** Note: $stop    : /home/harris/cvw/testbench/testbench.sv(455)
#    Time: 71820 ns  Iteration: 1  Instance: /testbench
# Break at /home/harris/cvw/testbench/testbench.sv line 455
# Stopped at /home/harris/cvw/testbench/testbench.sv line 455
# End time: 23:44:49 on Jun 07,2024, Elapsed time: 0:00:01
# Errors: 0, Warnings: 0
harris@chips:~/cvw$ git log
commit 5dfde808f003bf0eb94a5627b5c1a0a40128d347 (HEAD -> dev, upstream/main)
harris@chips:~/cvw$ which vsim
/cad/mentor/questa_sim-2023.4/questasim/bin/vsim
stineje commented 3 months ago

I agree with you, but I tried on 3 more machines here and got the same error. Yes, the fix is the same but something is happening here. I even tried regenerating the tool chain on one machine and still got the same error. This does not make sense as the nightly regression doesn’t show anything. I will look at it Monday - working on getting JTAG all cleared and tested.

I still think it’s worthy having this as an issue until we can figure out what is going on. My guess is something with Questa.

On Jun 8, 2024, at 1:50 AM, David Harris @.***> wrote:

It's working for me. See below for versions.

I also don't see how your proposed fix changes anything. bs is 2 bits. shamt is 5 bits. assign shamt = {bs, 3'b0}; // shamt = bs * 8 (convert bytes to bits) is a perfectly correct way to put bs in the top 2 bits of the 5-bit bus.

I presume you've pulled the latest version?

@.***:~/cvw$ wsim --sim questa rv32gc arch32zkne ...

rv32i_m/K/src/aes32esi-01.S succeeded. Brilliant!!!

rv32i_m/K/src/aes32esmi-01.S succeeded. Brilliant!!!

SUCCESS! All tests ran without failures.

** Note: $stop : /home/harris/cvw/testbench/testbench.sv(455)

Time: 71820 ns Iteration: 1 Instance: /testbench

Break at /home/harris/cvw/testbench/testbench.sv line 455

Stopped at /home/harris/cvw/testbench/testbench.sv line 455

End time: 23:44:49 on Jun 07,2024, Elapsed time: 0:00:01

Errors: 0, Warnings: 0

@.:~/cvw$ git log commit 5dfde808f003bf0eb94a5627b5c1a0a40128d347 (HEAD -> dev, upstream/main) @.:~/cvw$ which vsim /cad/mentor/questa_sim-2023.4/questasim/bin/vsim — Reply to this email directly, view it on GitHub https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fopenhwgroup%2Fcvw%2Fissues%2F828%23issuecomment-2155842121&data=05%7C02%7Cjames.stine%40okstate.edu%7Caf0a8f13266549604b7f08dc87873c5e%7C2a69c91de8494e34a230cdf8b27e1964%7C0%7C0%7C638534262117835355%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=12hfm5uID18YQBxV8G3mJS8w1pXQT0urB1hmmEFDff8%3D&reserved=0, or unsubscribe https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAFSIHPASDXXBWCOSCQ4KF5DZGKSR5AVCNFSM6AAAAABI7K4YEWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJVHA2DEMJSGE&data=05%7C02%7Cjames.stine%40okstate.edu%7Caf0a8f13266549604b7f08dc87873c5e%7C2a69c91de8494e34a230cdf8b27e1964%7C0%7C0%7C638534262117846754%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=8%2BMAJyVigmiwPgQZRMi6mky0N9bxl0ZlCx%2Frr2qNKpU%3D&reserved=0. You are receiving this because you authored the thread.

stineje commented 3 months ago

Issue is fixed in Quests 2023.3_2 and onward. No reference in the release information what was fixed that pertained to this problem. Nevertheless, the problem is fixed in the current release.