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

VCU118 FPGA Updates + FireMarshal on Prototypes #849

Merged
merged 15 commits into from
Apr 14, 2021
Merged

Conversation

abejgonzalez
Copy link
Contributor

@abejgonzalez abejgonzalez commented Apr 1, 2021

Related issue: #822 firesim/FireMarshal#190

Type of change: other enhancement

Impact: software change + other

Release Notes
Enlarge the payload from 20MiB to 30MiB for the SDCard boot (for larger FireMarshal images). Use DefaultClockFrequencyKey to determine the FPGA frequency instead of another key. Add documentation for using FireMarshal for the prototype platform.

TODO:

@abejgonzalez abejgonzalez self-assigned this Apr 2, 2021
@abejgonzalez
Copy link
Contributor Author

Note: The doc build passes locally. The doc error seems to happen on other PRs so this isn't an issue with these docs.

@colinschmidt
Copy link
Contributor

CI only started failing the doc check after this PR: #846
It seems to fail on dev specifically but not on the PR branches

@abejgonzalez abejgonzalez requested a review from jerryz123 April 7, 2021 01:44
@jerryz123
Copy link
Contributor

Chisel changes LGTM. Someone who has done bringup should review the doc changes.

@abejgonzalez
Copy link
Contributor Author

Ping @alonamid @colinschmidt .

@alonamid
Copy link
Contributor

Could you please add commas in the docs page?

@abejgonzalez
Copy link
Contributor Author

I won't re-run CI unless necessary since it passed before the grammar changes.

Copy link
Contributor

@colinschmidt colinschmidt left a comment

Choose a reason for hiding this comment

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

LGTM. Just some minor docs changes

Building Linux with FireMarshal
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Since the prototype does not have a block device, we build Linux with the rootfs built into the binary (otherwise known as "initramfs" or "nodisk" version of Linux).
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't something we should change now but can't the SDCard act as a block device? Would that make things easier or faster?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assume it probably can, but I don't know how to do that (nor do I think it will make much of a difference). Having it as a block device would change things in a couple ways:

  1. Have to figure out how to get it to work with Linux / SPI interface. Create new partitions? Have a block device driver? Etc.
  2. Could probably minimize the binary size. Thus making loading faster if we change the bootrom.

docs/Prototyping/VCU118.rst Outdated Show resolved Hide resolved

.. code-block:: shell

echo "board-dir : 'boards/prototype'" > $PATH_TO_FIREMARSHAL/marshal-config.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be an append? Or do we really want to overwrite?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file doesn't exist unless the user makes it. So this is assuming we create it from scratch.

docs/Prototyping/VCU118.rst Outdated Show resolved Hide resolved
docs/Prototyping/VCU118.rst Outdated Show resolved Hide resolved
// A sector is 512 bytes, so ((1 << 11) * 512) = 1 MiB
#define PAYLOAD_SIZE (16 << 11)
// Total payload in B
#define PAYLOAD_SIZE_B (30 << 20) // default: 30MiB
Copy link
Contributor

Choose a reason for hiding this comment

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

How much extra room is there here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Linux binary with FireMarshal is around 20-25MiB so 5MiB room.

@@ -168,9 +170,11 @@ static int copy(void)
int rc = 0;

dputs("CMD18");

kprintf("LOADING 0x%xB PAYLOAD\r\n", PAYLOAD_SIZE_B);
kprintf("LOADING ");
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need both of these messages? The second loading seems redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a nice spinner that comes up next to the loading so I just left this. Technically I can put the spinner next to the kprintf that I added, but I figured this extra print wasn't a bottleneck so just leave it.

fpga/src/main/resources/vcu118/sdboot/sd.c Show resolved Hide resolved

sudo gdisk /dev/sdc

2. The VCU118 bootrom assumes that the Linux binary to load into memory will be located on sector 34 of the SDCard.
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be possible to give prompt examples? (..code-block:: shell ....)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that is a bit excessive and unnecessary. These instructions although are setup for gdisk are also a bit abstract to allow others to port to other disk programs. If people get stuck on this step we can add more details if need be (I presume this will be the easiest step with the FireMarshal setup being the trickiest).

Copy link
Contributor

@alonamid alonamid left a comment

Choose a reason for hiding this comment

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

LGTM pending commas

@abejgonzalez abejgonzalez merged commit 0edb2fd into dev Apr 14, 2021
@abejgonzalez abejgonzalez deleted the vcu118-update branch May 3, 2021 20:29
@DecodeTheEncoded
Copy link
Contributor

@abejgonzalez Hello, I want to run my rv32imafc chipyard configuration on vcu118, and test it with baremetal software. Since chipyard currently does not support boot from flash, I therefore want to compile a simple rv32 helloworld program, create helloword.bin using objcopy, and put it in specific sectors of sdcard following instructions in chipyard doc. My confusion is that do I need FireMarshal to generate the baremetal workload? I read somewhere in FireMarshal that it also supports build baremetal workloads. I wonder if my method will work. And Is there some extra considerations for this? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants