Skip to content
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

Re-align with OpenTitan #43

Merged
merged 1 commit into from
Feb 25, 2021
Merged

Re-align with OpenTitan #43

merged 1 commit into from
Feb 25, 2021

Conversation

vogelpi
Copy link
Collaborator

@vogelpi vogelpi commented Feb 23, 2021

Note: This PR needs #41 and #42 to go in first. In addition, it supersedes #40.

This PR re-aligns the binaries and configuration with OpenTitan (commit 1b00157). The relevant changes are:

  • Increase spiflash frame size, adjust wait times accordingly. This resolves React to changes in spi and spiflash #34.
  • Update aes_serial binary and relevant paths in OpenTitan.
  • Update FPGA bitstream (by default OpenTitan now uses domain-oriented masking, short DOM).
  • Increase ADC gain and adjust number of samples for DOM.
  • Update the sample traces figure as the AES traces look pretty different when using DOM compared to Canright masking or unmasked implementations.

@vogelpi
Copy link
Collaborator Author

vogelpi commented Feb 23, 2021

FYI @vrozic.

@vogelpi
Copy link
Collaborator Author

vogelpi commented Feb 23, 2021

Update, #41 and #42 got merged, I've rebased this PR on top of master.

@alphan
Copy link
Contributor

alphan commented Feb 24, 2021

Thanks @vogelpi! Can you apply the following to keep things in sync:

diff --git a/cw/cw305/waverunner.py b/cw/cw305/waverunner.py
index 784927e..961beeb 100644
--- a/cw/cw305/waverunner.py
+++ b/cw/cw305/waverunner.py
@@ -69,7 +69,7 @@ class WaveRunner:
         """
         self._ip_addr = ip_addr
         self.seq_num_waves = 1000
-        self._num_samples = 180
+        self._num_samples = 740
         self._instr = vxi11.Instrument(self._ip_addr)
         self._populate_device_info()
         self._print_device_info()
@@ -134,8 +134,8 @@ class WaveRunner:
         commands = [
             # DC coupling, 1 Mohm.
             "C3:CPL D1M",
-            "C3:VDIV 40MV",
-            "C3:OFST 140MV",
+            "C3:VDIV 35MV",
+            "C3:OFST 105MV",
         ]
         self._write(";".join(commands))
         self._write("vbs 'app.Acquisition.C3.BandwidthLimit = \"200MHz\"'")
@@ -168,12 +168,12 @@ class WaveRunner:
 
     def _configure_timebase(self):
         commands = [
-            "TDIV 200NS",
+            "TDIV 800NS",
             # Trigger delay: Trigger is centered by default. Move to the left to
             # include the samples that we are interested in.
             # Note: This number is tuned to align WaveRunner traces with ChipWhisperer
             # traces.
-            "TRDL -960NS",
+            "TRDL -4950NS",
         ]
         self._write(";".join(commands))
 
@@ -189,7 +189,7 @@ class WaveRunner:
             # NP: number of points, self._num_samples.
             # FP: first point (without decimation).
             # SN: All sequences: 0
-            f"WFSU SP,10,NP,{self._num_samples},FP,0,SN,0",
+            f"WFSU SP,10,NP,{self._num_samples},FP,10,SN,0",
             # Data format: with DEF9 header, bytes (8-bit signed integers), binary encoding.
             # TODO: byte vs. word.
             "CFMT DEF9,BYTE,BIN",

Also, was this bitstream generated after lowRISC/opentitan#5279?

@vogelpi vogelpi force-pushed the realign-with-ot branch 2 times, most recently from bb717f4 to 1bbb759 Compare February 25, 2021 13:39
@vogelpi
Copy link
Collaborator Author

vogelpi commented Feb 25, 2021

Thanks @alphan for having a look. I've updated waverunner.py accordingly (+ made it executable like the other scripts).

Yes, the bitstream does include the identification via USR_ACCESS register.

Copy link
Contributor

@alphan alphan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @vogelpi! I tried fpga.get_fpga_buildtime() with the new bitstream but it still doesn't return the correct value. I reopened newaetech/chipwhisperer#306 to ask NewAE if there was anything else that needs to be done to enable this. We can debug further after hearing from them.

This commit re-aligns the binaries and configuration with OpenTitan (commit
ea96a354f870338f1d0e3646a6c922fda8a71b27). The relevant changes are:
- Increase spiflash frame size, adjust wait times accordingly. This
  resolves lowRISC#34.
- Update aes_serial binary and relevant paths in OpenTitan.
- Update FPGA bitstream (by default OpenTitan now uses domain-oriented
  masking, short DOM, also the bitstream now has a timestamp embedded that
  can be read in the USR_ACCESS register).
- Adjust capture settings for DOM: gain, number of samples, alignment, etc.
- Update the sample traces figure as the AES traces look pretty different
  when using DOM compared to Canright masking or unmasked implementations.

Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
@vogelpi
Copy link
Collaborator Author

vogelpi commented Feb 25, 2021

Thanks @alphan for approving the PR and for trying out the buildtime feature. I updated and tested the bitstream and binaries once more just to be sure. I can confirm that the current bitstream has the following timestamp embedded (from the Vivado log):

Overwriting "TIMESTAMP" with "C92B0B41" for option USR_ACCESS
TIMESTAMP = Thu Feb 25 16:45:01 2021

I am now going ahead and merge this to unblock other tasks.

@vogelpi vogelpi merged commit 10a42b5 into lowRISC:master Feb 25, 2021
@vogelpi vogelpi deleted the realign-with-ot branch March 18, 2021 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

React to changes in spi and spiflash
2 participants