[FPGA] Automatically dump mmi #1618

tjaychen · 2020-02-25T22:29:48Z

If brom sizes are changed in the future, this should be automatically handled.

Signed-off-by: Timothy Chen timothytim@google.com

rpenugo · 2020-02-27T20:23:29Z

LGTM

imphil

Thanks for this script @tjaychen and sorry for the delay in review. I have a couple suggestions, please have a look.

hw/top_earlgrey/util/vivado_hook_write_bitstream_pre.tcl

imphil · 2020-03-16T18:03:34Z

hw/top_earlgrey/util/vivado_hook_write_bitstream_pre.tcl

+puts $fileout "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
+puts $fileout "<MemInfo Version=\"1\" Minor=\"0\">"
+puts $fileout "  <Processor Endianness=\"Little\" InstPath=\"dummy\">"
+puts $fileout "  <AddressSpace Name=\"brom\" Begin=\"0\" End=\"$space\">"


Suggested change

puts $fileout " <AddressSpace Name=\"brom\" Begin=\"0\" End=\"$space\">"

puts $fileout " <AddressSpace Name=\"rom\" Begin=\"0\" End=\"$space\">"

(For consistency within our code base.)

hw/top_earlgrey/util/vivado_hook_write_bitstream_pre.tcl

util/fpga/splice_nexysvideo.sh

tjaychen · 2020-06-24T05:48:50Z

my i completely forgot about this PR.
@imphil do you still think this is worth doing? Or maybe we did something better already in the interim?

imphil · 2020-06-26T13:52:51Z

I'd say it's up to you. If we want to keep the splice flow (which we didn't use much in the past, re-synthesizing the whole bitstream seems to be less hassle), I'd appreciate if we can get this PR in. If not, then we can probably close it and remove the splicing if it breaks at one point.

tjaychen · 2020-11-19T02:36:47Z

actually maybe this will be useful at some point...

moidx · 2021-03-23T06:24:44Z

The current .tcl changes generate the following MMI map:

<?xml version="1.0" encoding="UTF-8"?>
<MemInfo Version="1" Minor="0">
  <Processor Endianness="Little" InstPath="dummy">
  <AddressSpace Name="brom" Begin="0" End="16895">
      <BusBlock>
        <BitLane MemType="RAMB32" Placement="X4Y18">
          <DataWidth MSB="8" LSB="0"/>
          <AddressRange Begin="0" End="4095"/>
          <Parity ON="false" NumBits="0"/>
        </BitLane>
        <BitLane MemType="RAMB32" Placement="X4Y19">
          <DataWidth MSB="17" LSB="9"/>
          <AddressRange Begin="0" End="4095"/>
          <Parity ON="false" NumBits="0"/>
        </BitLane>
        <BitLane MemType="RAMB32" Placement="X3Y14">
          <DataWidth MSB="26" LSB="18"/>
          <AddressRange Begin="0" End="4095"/>
          <Parity ON="false" NumBits="0"/>
        </BitLane>
        <BitLane MemType="RAMB32" Placement="X3Y15">
          <DataWidth MSB="32" LSB="27"/>
          <AddressRange Begin="0" End="4095"/>
          <Parity ON="false" NumBits="0"/>
        </BitLane>
      </BusBlock>
    </AddressSpace>
  </Processor>
<Config>
  <Option Name="Part" Val="xc7a200tsbg484-1"/>
</Config>
</MemInfo>

The DataWidth values seem to match the bram_slice_{begin,end} parameters.

Unfortunately updatemem throws the following error:

/tools/xilinx/Vivado/2020.1/bin/rdiArgs.sh: line 286: 1374147 Segmentation fault      (core dumped) "$RDI_PROG" "$@"

I am using Vivado v2020.1 SW build 2902540.

moidx · 2021-03-23T17:25:32Z

Alternative options are being considered in #5751

vogelpi · 2021-09-08T21:42:34Z

Hey guys, this PR is pretty old but I think it's actually very useful. Therefore, I've rebased it and extended it to re-enable FPGA ROM splicing on boards other than the NexysVideo and more importantly if also boot ROM scrambling is enabled. I've sucessfully tested that on the CW310.

If you think it's better to create a new PR I can do that.

vogelpi · 2021-09-08T21:43:29Z

FYI @tjaychen

Previously, only the first two BRAM instances were checked, there were only placement rules for the first four banks (with scrambling there are five banks) and the placement seemed to be rather arbitrary (the chosen sites were not available on some FPGAs). With this commit, BRAM sites are chosen that are available on most FPGAs and the placement/checking is also enabled on boards other than the NexysVideo. Note this commit doesn't touch the splicing which currently doesn't work with the scrambling enabled. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

This commit causes Vivado to output an MMI file that can be used for boot ROM splicing. Signed-off-by: Timothy Chen <timothytim@google.com>

imphil

Thanks, LGTM with a couple clarifications around naming. I'd prefer if you can squash the commits together that are fixups for previous commits.

imphil · 2021-09-10T13:22:53Z

util/fpga/splice_nexysvideo.sh

-
-util/fpga/addr4x.py -i "${TARGET}.brammem" -o "${TARGET}.mem"
+# Create the Vivado image for splicing.
+hw/ip/rom_ctrl/util/gen_vivado_mem_image.py "${TARGET}.32.vmem" "${TARGET}_vivado.mem"


mem and vmem files are effectively different variations of the same file format; the file extension in itself isn't really describing it. (vmem files as we use them are also given a .mem suffix from time to time). Also, Vivado uses different variations of the file format in different places, with updatemem having its own requirements. (https://www.xilinx.com/support/documentation/sw_manuals/xilinx2020_2/ug898-vivado-embedded-design.pdf#page=165 for the beautiful details).

To reduce confusion, would you mind renaming this file to something like "${TARGET}.updatemem.mem" (also keeping the use of a dot for further qualifying the file, as we do for .scr. etc.), and update the comment in the gen_vivado_mem_image.py to better describe this?

Thanks @imphil for providing some more insight here. I've added some more details + the ref to gen_vivado_mem_image.py and renamed the files as suggested.

imphil · 2021-09-10T13:26:36Z

util/fpga/splice_rom.sh

+./meson_init.sh
+ninja -C "${OBJ_DIR}" "${TARGET_EXPORT}"


Please avoid doing the software build in here, and expect the software to be built already.

This sounds fair. I've adjusted also the documentation accordingly.

imphil · 2021-09-10T13:29:11Z

util/fpga/splice_rom.sh

+TARGET_EXPORT="${TARGET_PREFIX}/boot_rom_export_fpga_${TARGET_BOARD}"
+TARGET="${BIN_DIR}/${TARGET_PREFIX}/boot_rom_fpga_${TARGET_BOARD}"
+
+FPGA_BUILD_DIR=build/lowrisc_systems_chip_${TARGET_TOP}_${TARGET_BOARD}_0.1/synth-vivado


None of these directory paths are meant to be stable, only the paths in build-bin are. Unfortunately, we don't have a build system that moves the bitstream there itself, we do this manually in CI.

So I'm fine with keeping things like that for now, but we'll need to change this if we want to use this script in CI (which is helpful to reduce CI times and test different ROMs)

I agree with you. The reason for not changing it now was mainly that I didn't want to change too much stuff at once. The main change to this file is the change of its name. But yes, we should change this in a follow up.

Previously, a combination of srec_cat and a custom Python script was was used to generate the Vivado ROM image from a .bin file. The new script directly reads the .vmem file to produce the Vivado .mem file usable for FPGA ROM splicing. But more importantly, this script is also compatible with word widths different to 32 bits. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

It turns out slices declared in the MMI need to be divisble by 8. Otherwise the updatemem tool doing the splicing segfaults. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

This script no longer just works for the NexysVideo FPGA board, but it can work for all FPGA platforms instead. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

These placement constraints are no longer needed as the MMI file is now generated automatically. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

vogelpi · 2021-09-10T14:22:25Z

Thanks for your review @imphil ! I've implemented the suggested changes and retested this locally. I will merge this once CI passes.

tjaychen · 2021-09-10T17:43:37Z

hw/top_earlgrey/data/placement.xdc

-set_property LOC RAMB36_X0Y12 [get_cells -hierarchical -filter { NAME =~ "*u_rom_ctrl*u_rom*rdata_o_reg_2" && PRIMITIVE_TYPE =~ BMEM.*.* }]
-set_property LOC RAMB36_X0Y13 [get_cells -hierarchical -filter { NAME =~ "*u_rom_ctrl*u_rom*rdata_o_reg_3" && PRIMITIVE_TYPE =~ BMEM.*.* }]
-set_property LOC RAMB36_X0Y14 [get_cells -hierarchical -filter { NAME =~ "*u_rom_ctrl*u_rom*rdata_o_reg_4" && PRIMITIVE_TYPE =~ BMEM.*.* }]
-


sorry i'm a bit late to the party, but could someone quickly explain for a minute why we don't need the boot rom placement anymore? I thought we needed to find these in order to know where to splice.

The thing is: the nice script you wrote some time ago actually extracts the location of the BRAMs in the design. If the script is called late enough (meaning after place and route) these locations are indeed known and there is no point in setting a placement constraint. All that is needed is to verify that the ROM is indeed implemented using BRAM such that your script can extract the locations. This check is performed as part of the opt_design post hook.

The advantage of not specifying the locations is that the tool is free to place the ROM where suitable. On the CW310, I noted that the ROM logic was usually placed pretty far away from the ROM BRAMs. Also, without LOC constraints there won't be issues e.g. because on a different FPGA, the specified locations do not exist.

tjaychen requested review from rpenugo and imphil February 25, 2020 22:29

rpenugo approved these changes Feb 27, 2020

View reviewed changes

tjaychen force-pushed the rom_mmi branch 3 times, most recently from 263bb3b to 7f08d72 Compare March 4, 2020 01:34

tjaychen changed the title ~~[draft] Try to automatically dump mmi~~ [FPGA] Automatically dump mmi Mar 4, 2020

tjaychen marked this pull request as ready for review March 4, 2020 01:34

tjaychen requested a review from sjgitty as a code owner March 4, 2020 01:34

tjaychen requested a review from rpenugo March 4, 2020 01:34

tjaychen mentioned this pull request Mar 4, 2020

[ROM] Double ROM size to 16KB #1608

Merged

imphil suggested changes Mar 16, 2020

View reviewed changes

tjaychen closed this Nov 13, 2020

tjaychen deleted the rom_mmi branch November 13, 2020 23:21

tjaychen restored the rom_mmi branch November 19, 2020 02:36

tjaychen reopened this Nov 19, 2020

moidx force-pushed the rom_mmi branch from 7f08d72 to 84f2082 Compare March 23, 2021 06:15

moidx force-pushed the rom_mmi branch from 84f2082 to c1ce30f Compare March 23, 2021 06:30

moidx linked an issue Mar 23, 2021 that may be closed by this pull request

[fpga] Look into ways to make ROM loadable on FPGA #5751

Closed

moidx removed a link to an issue Mar 23, 2021

[fpga] Look into ways to make ROM loadable on FPGA #5751

Closed

imphil mentioned this pull request Aug 30, 2021

[fpga] Support memory scrambling in boot ROM splicing script #7988

Closed

vogelpi force-pushed the rom_mmi branch from c1ce30f to 7b1673f Compare September 8, 2021 21:38

tjaychen requested a review from asb as a code owner September 8, 2021 21:38

vogelpi removed the request for review from asb September 8, 2021 21:38

vogelpi requested review from rswarbrick and removed request for sjgitty and rpenugo September 8, 2021 21:38

vogelpi requested review from moidx and imphil September 8, 2021 21:42

vogelpi force-pushed the rom_mmi branch 4 times, most recently from a37f9eb to 0a3c673 Compare September 9, 2021 15:09

vogelpi and others added 2 commits September 10, 2021 12:42

[fpga] Automatically dump MMI for boot ROM splicing flow

eb1600f

This commit causes Vivado to output an MMI file that can be used for boot ROM splicing. Signed-off-by: Timothy Chen <timothytim@google.com>

vogelpi force-pushed the rom_mmi branch from 0a3c673 to 8d90198 Compare September 10, 2021 10:48

imphil approved these changes Sep 10, 2021

View reviewed changes

vogelpi added 5 commits September 10, 2021 16:00

[fpga] Enable ROM splicing for scrambled boot ROMs

2dab468

It turns out slices declared in the MMI need to be divisble by 8. Otherwise the updatemem tool doing the splicing segfaults. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

[fpga] Rename the ROM splicing script and update its documentation

0b37d37

This script no longer just works for the NexysVideo FPGA board, but it can work for all FPGA platforms instead. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

[fpga, doc] Update FPGA reference manual

951e31d

Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

[fpga] Remove placement constraints for Boot ROM

ec7fbab

These placement constraints are no longer needed as the MMI file is now generated automatically. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

vogelpi force-pushed the rom_mmi branch from 8d90198 to ec7fbab Compare September 10, 2021 14:13

vogelpi merged commit 52e5b21 into lowRISC:master Sep 10, 2021

tjaychen commented Sep 10, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FPGA] Automatically dump mmi #1618

[FPGA] Automatically dump mmi #1618

tjaychen commented Feb 25, 2020 •

edited

Loading

rpenugo commented Feb 27, 2020

imphil left a comment

imphil Mar 16, 2020

tjaychen commented Jun 24, 2020

imphil commented Jun 26, 2020

tjaychen commented Nov 19, 2020

moidx commented Mar 23, 2021 •

edited

Loading

moidx commented Mar 23, 2021

vogelpi commented Sep 8, 2021

vogelpi commented Sep 8, 2021

imphil left a comment

imphil Sep 10, 2021

vogelpi Sep 10, 2021

imphil Sep 10, 2021

vogelpi Sep 10, 2021

imphil Sep 10, 2021

vogelpi Sep 10, 2021

vogelpi commented Sep 10, 2021

tjaychen Sep 10, 2021

vogelpi Sep 10, 2021

vogelpi Sep 10, 2021

	puts $fileout " <AddressSpace Name=\"brom\" Begin=\"0\" End=\"$space\">"
	puts $fileout " <AddressSpace Name=\"rom\" Begin=\"0\" End=\"$space\">"

		./meson_init.sh
		ninja -C "${OBJ_DIR}" "${TARGET_EXPORT}"

[FPGA] Automatically dump mmi #1618

[FPGA] Automatically dump mmi #1618

Conversation

tjaychen commented Feb 25, 2020 • edited Loading

rpenugo commented Feb 27, 2020

imphil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjaychen commented Jun 24, 2020

imphil commented Jun 26, 2020

tjaychen commented Nov 19, 2020

moidx commented Mar 23, 2021 • edited Loading

moidx commented Mar 23, 2021

vogelpi commented Sep 8, 2021

vogelpi commented Sep 8, 2021

imphil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vogelpi commented Sep 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjaychen commented Feb 25, 2020 •

edited

Loading

moidx commented Mar 23, 2021 •

edited

Loading