Xilinx / systemctlm-cosim-demo

QEMU libsystemctlm-soc co-simulation demos.
https://xilinx-wiki.atlassian.net/wiki/spaces/A/pages/862421112/Co-simulation
Other
117 stars 46 forks source link

Getting Started Documentation #15

Closed Mluckydwyer closed 2 years ago

Mluckydwyer commented 3 years ago

This PR contains documentation about getting started with Xilinx Co-Simulation with QEMU and SystemCTLM with the included demo. It includes a step-by-step guide for cloning and building the required additional tools (SystemC, Buildroot, and QEMU), running the demo, and sample output a user can expect to see upon completion. This fills a gap in setting up a basic environment that can be further built upon by new users. This also includes .dtsi and .dts files for describing the hardware described by the demo application for QEMU. They were adapted from the official Xilinx Device Tree Files to work with the test hardware provided in the demo. The README has also been updated to point new users in the direction of the getting started guide and the LMAC demo documentation.

Mluckydwyer commented 3 years ago

@rc-matthew-l-weber @edgarigl We would love some feedback on these additions to the documentation of the demos. Thank you in advance.

edgarigl commented 3 years ago

@rc-matthew-l-weber @edgarigl We would love some feedback on these additions to the documentation of the demos. Thank you in advance.

Thanks for working on this! I see that you've already gotten some comments from Matthew. His comments all make sense so I'll wait for a new version of the PR!

Best regards, Edgar

Mluckydwyer commented 3 years ago

Hi, thanks for this piece of work! Excellent start! I agree with the comments given by @rc-matthew-l-weber and some comments from me are found below.

git commit comments: squash patch 'Minor revisions to README formatting' into 'Updated README to reflect changes in additional documentation' for obtaining only 1 commit of those 2.

Also in the git commit messages add a summary line and signed-off-by line, similar to the current top of master:

libsystemctlm-soc: Bump version

Bump libsystemctlm-soc and update the Makefile as necessary.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Max size of the lines in the message should also be 80 chars (in case it is not possible it will probably also be ok). The signed-of-by line can be obtained by providing the -s switch when creating the commit: git commit -s, alternatively it can be added manually.

Great job else!

Thanks!

Best regards, Francisco

Apologies for the non-standard commits. We will fix those when updating with the other suggestions. Thank you for the suggestions.

ctomkins1 commented 3 years ago

Hi Xilinx team,

Apologies for the lack of communication this past summer, our team had quite the busy summer with internships and other summer activities. The documentation we created last semester should be updated based on your comments and feedback. We are hoping to get this merged in sometime this week if possible. If there are any further updates, please let us know and we can update them to your standards. Thank you!

Mluckydwyer commented 3 years ago

The documentation should now have all of those requested changes added except for the diagram mentioned by @rc-matthew-l-weber:

Then add a ditta/dot diagram(whatever github is most happiest with) that shows how all the parts of the demo come together. Below that you can then note what tools and purposes each part has.

We are working on adding that as well and formalizing the commit.

Mluckydwyer commented 2 years ago

I have now made all of the additions requested and verified them on a bare Ubuntu 18.04 Docker container (docker run -it ubuntu:18.04 bash).

@rc-matthew-l-weber You mentioned being able to assist with the frequency governer part of the Buildroot configuration. Is that still the case?

I am also having some issues squashing all of the commits into one and signing them. It is my understanding though that squash can be done with the merge of the PR, correct?

franciscoIglesias commented 2 years ago

Hi Matt,

I made some minor final changes (see below), tested the commands and applied the PR (see commit c7dc0450)!

Changes:

Thank you for the hard work!

Best regards, Francisco Iglesias

Mluckydwyer commented 2 years ago

Hello @franciscoIglesias Thank you for your assistance, we really appreciate it. Apologies on the styling, that's slipped my mind. That's is great to hear though. Hopefully this can help wrap up this PR that's has been awhile in the making. @rc-matthew-l-weber were you able to make those CPU Frequency Governor changes?

franciscoIglesias commented 2 years ago

Hi Matt,

No worries and thanks again for the contribution! :)

About the CPU frequency governor printks: I added in the wget for the xilinx_zynq_defconfig along with the buildroot config update to use that defconfig in the guide (instead of using the default for the arch Arm it is better to use the kernel defconfig targeting the zynq). The zynq defconfig has selected the CPU Frequency Governor to userspace already (fixing the printk problem). Basically, I think we can close this PR and in case and you would like to add/change something we can always open a new PR.

Best regards, Francisco Iglesias

Mluckydwyer commented 2 years ago

Hello Francisco,

That sounds great. Thank you again for all of your assistance. Those solutions sound warranted and like you mentioned, we can also open another PR if changes need to be made.

Best, Matthew Dwyer

edgarigl commented 2 years ago

Merged.