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

Add Baremetal IDE support #1534

Merged
merged 18 commits into from
Jan 7, 2024
Merged

Add Baremetal IDE support #1534

merged 18 commits into from
Jan 7, 2024

Conversation

T-K-233
Copy link
Member

@T-K-233 T-K-233 commented Jun 27, 2023

This PR adds an integrated environment for compiling baremetal code for both simulation and post-silicon chips.

Different from the simple test code from /tests folder, this environment also includes the complete initialization procedure of the memory layout, global, stack, and frame pointers, and other C run-time requirements.

With additional Microsoft Visual Studio Code plugins, users can launch interactive JTAG GDB debug sessions.

Additionally, this PR provides an example chip config that is generated from the GUI interface, which can both run in simulation and on Arty FPGA (FPGA support added in another PR).

Related PRs / Issues:
#194
#1046

Type of change:

  • Bug fix
  • New feature
  • Other enhancement

Impact:

  • RTL change
  • Software change (RISC-V software)
  • Build system change
  • Other

Contributor Checklist:

  • Did you set main as the base branch?
  • Is this PR's title suitable for inclusion in the changelog and have you added a changelog:<topic> label?
  • Did you state the type-of-change/impact?
  • Did you delete any extraneous prints/debugging code?
  • Did you mark the PR with a changelog: label?
  • (If applicable) Did you update the conda .conda-lock.yml file if you updated the conda requirements file?
  • (If applicable) Did you add documentation for the feature?
  • (If applicable) Did you add a test demonstrating the PR?
  • (If applicable) Did you mark the PR as Please Backport?

* Chip Config generated by the GUI
*/
class ExampleChipConfig extends Config(
// ==================================
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be merged into ChipLikeRocketConfig?

Copy link
Member Author

Choose a reason for hiding this comment

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

Where is the ChipLikeRocketConfig defined...?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actualy I forgot, hasn't been merged yet, after #1484 I change ChipLikeQuadRocket to ChipLikeRocket

Copy link
Contributor

Choose a reason for hiding this comment

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

There should be only 1 basic ChipConfig ... this should be merged into ChipLikeRocketConfig above. Is there anything here that should not be merged into that?

Copy link
Member Author

Choose a reason for hiding this comment

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

The ExampleChipConfig is generated by the GUI interface. It lists every Config fragment in the chip explicitly within this class.

The idea is that this gives the user a clearer view of what components are included inside a chip, what Config fragment can be swapped with what else etc., and therefore can be used as a good teaching material for getting started tutorials (tutorials coming soon).

Listing Config fragments out in a single class also helps resolve the "what is overridden by what" problem. For example, for a tapeout chip, instead of overriding WithXXXPunchthrough with WithXXXIOBinder, the user will replace and switch between these two fragments for simulation vs. tapeout.

I'll try to merge this to the ChipLikeRocketConfig in future PR, but since this ExampleChipConfig is subject to user change (GUI will auto-generate the ChipConfigs.scala based on user configuration), and the scope can expand beyond using rocket cores, I suggest giving the class a more versatile name.

Copy link
Contributor

Choose a reason for hiding this comment

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

At one point, AbstractConfig did not exist, and every single example config included every single relevant config fragment, as you describe. I fought for this model for quite some time, until it became untenable to maintain so many 50-line configs.

My perspective on "tapeout configs" is that they should be as explicit as possible, and will often require careful hand-edits/bespoke-configs. With that in mind, removing AbstractConfig from the ChipLike configs is a move I would support.

However, I'm not sure that using GUI-configs as "tapeout configs" is the right direction, given that the GUI will only be able to support a smaller design space than what the full system is capable of.

I'd also prefer to consolidate any "ChipLike" example config into the ChipLikeRocketConfig, as currently there is CI/regressions around that Config that test many features, including a regression that builds and tests the FPGA bringup harness.

A proposal based on the above points:

  • Expand ChipLikeRocketConfig to not override AbstractConfig.
  • Have GUI dump configs into a separate file. Since I expect users will often want to edit ChipConfigs.scala by hand, this also prevents edit conflicts. One idea... user specifies a name in the GUI... GUI dumps the file into configs/GUI_<user_config_name>.scala.
  • Make GUI flexible enough to support generating the exact same design (and exact same config text) as the ChipLikeRocketConfig.

@jerryz123
Copy link
Contributor

Is the GUI going to be part of this PR?

@T-K-233
Copy link
Member Author

T-K-233 commented Jul 2, 2023

Is the GUI going to be part of this PR?

It's on a separate branch of this submodule. There's still some work to do before it can be published.

@jerryz123
Copy link
Contributor

Will GUI be part of submodule? Is it possible to included it upstream?

@T-K-233 T-K-233 marked this pull request as ready for review September 3, 2023 16:24
* Chip Config generated by the GUI
*/
class ExampleChipConfig extends Config(
// ==================================
Copy link
Contributor

Choose a reason for hiding this comment

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

There should be only 1 basic ChipConfig ... this should be merged into ChipLikeRocketConfig above. Is there anything here that should not be merged into that?

new chipyard.iobinders.WithAXI4MMIOPunchthrough ++
new chipyard.iobinders.WithTLMemPunchthrough ++
new chipyard.iobinders.WithL2FBusAXI4Punchthrough ++
new chipyard.iobinders.WithBlockDeviceIOPunchthrough ++
Copy link
Contributor

Choose a reason for hiding this comment

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

The "Punchthrough" IOBinders should not be included in any "Chip" configs, since they are not intended to be taped outl.

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea is to make the default example a simulation-only setting. These Config fragments are directly extracted from chipyard.config.AbstractConfig.

// ==================================
// External memory section
new testchipip.WithSerialTLClientIdBits(4) ++ // support up to 1 << 4 simultaneous requests from serialTL port
new testchipip.WithSerialTLWidth(32) ++ // fatten the serialTL interface to improve testing performance
Copy link
Contributor

Choose a reason for hiding this comment

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

This is wider than what we usually build (1-4 bits)

// Debug settings
new chipyard.config.WithJTAGDTMKey(idcodeVersion = 2, partNum = 0x000, manufId = 0x489, debugIdleCycles = 5) ++
new freechips.rocketchip.subsystem.WithNBreakpoints(2) ++
new freechips.rocketchip.subsystem.WithNBreakpoints(2) ++
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicate

@@ -29,7 +29,7 @@ import chipyard.{ExtTLMem}
* @param hang the power-on reset vector, i.e. the program counter will be set to this value on reset
* @param contentFileName the path to the BootROM image
*/
class WithBootROM(address: BigInt = 0x10000, size: Int = 0x10000, hang: BigInt = 0x10040) extends Config((site, here, up) => {
class WithBootROM(address: BigInt = 0x10000, size: Int = 0x10000, hang: BigInt = 0x10000) extends Config((site, here, up) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is changed, then we need to change the testchipip bootrom as well. Does this PR have a corresponding testchipip PR that changes that?

Also, what is the motivation for this change?

Copy link
Contributor

@jerryz123 jerryz123 left a comment

Choose a reason for hiding this comment

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

I think this needs to be split into 3 PRs... I see three orthogonal changes here

  • BootROM changes... we can change this, but we'd have to check that all the tests pass. It looks like there are some failures related to this currently, so this may take some time to debug. Moving these to a separate PR would prevent them from blocking the other changes.
  • GUI-generated Configs. We ought to have a longer discussion on how these configs are generated+presented, so its clear what the differences are between the included configs, the GUI-generated configs, and the example Chip config. The GUI should be added to chipyard, not a submodule, since we would need to keep the GUI up-to-date with the SoC configuration.
  • IDE support. I think there's a strong argument for inlining this into Chipyard, rather than maintaining a separate submodule. Docs and regression tests should also be included with this.

@T-K-233
Copy link
Member Author

T-K-233 commented Sep 8, 2023

I think this needs to be split into 3 PRs... I see three orthogonal changes here

  • BootROM changes... we can change this, but we'd have to check that all the tests pass. It looks like there are some failures related to this currently, so this may take some time to debug. Moving these to a separate PR would prevent them from blocking the other changes.
  • GUI-generated Configs. We ought to have a longer discussion on how these configs are generated+presented, so its clear what the differences are between the included configs, the GUI-generated configs, and the example Chip config. The GUI should be added to chipyard, not a submodule, since we would need to keep the GUI up-to-date with the SoC configuration.
  • IDE support. I think there's a strong argument for inlining this into Chipyard, rather than maintaining a separate submodule. Docs and regression tests should also be included with this.

I see. I'll make the BootROM change a separate PR.

As for the IDE support, my design idea for the IDE is that it will be a standalone environment for firmware development, with the folder workspace being the working directory. To create multiple projects, user would clone multiple instance of the IDE repository. I'm not sure if inlining into Chipyard main repo would be optimal, since this would create a huge overhead for repo cloning and maintenance for those who just want to work on firmware/software development.

@jerryz123
Copy link
Contributor

As for the IDE support, my design idea for the IDE is that it will be a standalone environment for firmware development, with the folder workspace being the working directory. To create multiple projects, user would clone multiple instance of the IDE repository. I'm not sure if inlining into Chipyard main repo would be optimal, since this would create a huge overhead for repo cloning and maintenance for those who just want to work on firmware/software development.

Ok. That sounds reasonable to me. In that case, I think we should add regressions to chipyard that test that the default IDE configurations match the default chipyard configuration. Tests should check out the IDE, compile some simple hello-world that may access some devices (UART/GPIOs/SPI), and run them on the chipyard chip configuration.

@T-K-233 T-K-233 self-assigned this Sep 27, 2023
@T-K-233 T-K-233 requested a review from jerryz123 January 7, 2024 01:58
@T-K-233
Copy link
Member Author

T-K-233 commented Jan 7, 2024

I think we can add regression tests at another PR.

I would like to add the submodule to Chipyard first to get the Lab 1 flow working for now.

@jerryz123 jerryz123 merged commit ba56000 into main Jan 7, 2024
33 checks passed
@jerryz123 jerryz123 deleted the baremetal-ide branch January 7, 2024 02:23
@jerryz123
Copy link
Contributor

I've merged this to unblock lab development.
Please add a documentation link from the Chipyard docs at your earliest convenience.

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.

2 participants