überSpark: Add x86_32 hardware üobject runtime library

Referencing discussions from the task below:

This task will add the x86_32 hardware üobject runtime library implementation. Specifically it will bring in src/libxmhfhw contents into src-nextgen/uobjrtl/hw/generic/x86_32/intel/ following the üobject runtime libraries documentation within the contributors guide.

Look at src-nextgen/uobjrtl/crt and src-nextgen/uobjrtl/crypto for comparison

Linked PRs:

Merges:

I have some questions about the overall naming scheme and defining CASM modules in the manifest:

Should I remove previous platform specifiers from the function names? i.e. renaming the function
xmhf_baseplatform_arch_x86_pci_type1_read” to something like “uberspark_uobjrtl_hw__generic_x86_32_intel__pci_type1_read

Are CASM modules defined the same way as C modules in the manifest?
i.e. CASM_FUNCDEF(void, xmhfhw_cpu_reloaddsregs, ...) from the file cpu_reloaddsregs.cS -->

{
   "module-path" : "src/generic/x86_32/intel/cpu_reloaddsregs.cS",
   "module-funcdecls" :  [
     { "funcname" : "uberspark_uobjrtl_hw__generic_x86_32_intel__reloaddsregs" },
   ]
}

Lastly, should I keep each source file separate, or can I combine files with similar functions (e.g. xmhf_cpu_*.c files)?

Thanks!

Sounds good. A grep or perl one-liner should do the trick here.

Yes, your understanding is correct.

I would like for us to keep it separate for now since the CASM extractor plugin currently only supports one function per file. There is perhaps a better re-organization (e.g., group as sysmem, vtd, pci functions) that we can re-visit in the future.

So I organized the namespace, made the manifest, and renamed all the functions, but I can’t compile it yet as there are a lot of defines from the hwm library. Will we have to work on converting the hwm defines into the next-gen framework now?

My work so far is in my fork of uberspark:

Thanks!

Thanks Ethan!

Compiling the library will require addition of a casm bridge to support compilation of .cS files. I will add that as a new task that we can handle separately.

Also, did you get a chance to add basic comment structure for each of the .cS file functions per the uobject runtime library documentation in the contributors guide?

Once you add the function comments and update the documentation, we can merge this changeset via a PR, close this task, and start a new task to handle the hardware model migration.

Thanks!

Yeah, I added placeholder documentation comments for each function and I tried to merge in the existing documentation on functions that had it.

I’m still unsure about the functionality of some of the files, but I’m sure that I’ll be able to update their documentation once I learn more about them through the task of converting xmhf.

Let me know if anything else should be done to the library before we merge.

Thanks!

Sounds great. Can you please submit a PR on github uberspark/uberspark:develop ?
Thanks!

Yep, just submitted a PR:

Hey Ethan,

As I was reviewing this PR for a merge, I noticed that the documentation for the hw üobj runtime library was not tied into the reference guide.

Can you please take a look at docs/contrib-guide/uobrtl.rst sub-section “Adding a new üobj runtime library” to see how you can hook this up?

Also look into docs/reference/uobjrtl/intro.rst to add the top-level link to the hw library and docs/reference/uobjrtl/crt.rst to see how the doxygen documentation for the functions are brought in for the crt üobjrtl.

I can merge once you have had a chance to plug this in.

Thanks!

Will do. I’ll start on this ASAP and update the thread with any last questions.

I just finished updating the doxygen comments and added the hw library files to conf.py.

Is reference/uobjrtl/hw.rst supposed to be auto-generated, or do I manually create the file and add documentation using the doxygen directives?

Thanks!

This has to be manually created. I suppose we need to update docs/contrib-guide/uobjrtl.rst to indicate this.

Gotcha, I can create the file later today. Would you like for me to update the docs/contrib-guide/uobjrtl.rst to indicate this as well?

That would be fantastic! Thanks @yeeb

Linked PR to OP. Also noticed that the PR is not current with develop:HEAD

Can you do a rebase and force push to get the PR current with develop:HEAD ? See https://github.com/edx/edx-platform/wiki/How-to-Rebase-a-Pull-Request

Thanks!

Hey Amit,
I just rebased the PR.

It seems like we may need to do something about the documentation for CASM functions, as doxygen doesn’t recognize them at the moment.

Also, there is a minor issue that I noticed while building the docs and I can’t seem to figure out the reason. Here’s part of the build log:

../docs/macros.rst:4: WARNING: Definition list ends without a blank line; unexpected unindent.
<breathe>:1: WARNING: Unexpected indentation.
<breathe>:1: WARNING: Unexpected indentation.
<breathe>:1: WARNING: Unexpected indentation.
/home/docker/uberspark/docs/nextgen-toolkit/reference/uobjrtl/hw.rst:48: WARNING: Inline emphasis start-string without end-string.
/home/docker/uberspark/docs/nextgen-toolkit/reference/uobjrtl/hw.rst:51: WARNING: Inline emphasis start-string without end-string.
<breathe>:1: WARNING: Unexpected indentation.
/home/docker/uberspark/docs/nextgen-toolkit/reference/uobjrtl/hw.rst:54: WARNING: Inline emphasis start-string without end-string.
<breathe>:1: WARNING: Unexpected indentation.
<breathe>:1: WARNING: Unexpected indentation.
<breathe>:1: WARNING: Unexpected indentation.
/home/docker/uberspark/docs/nextgen-toolkit/reference/uobjrtl/hw.rst:48: WARNING: Inline emphasis start-string without end-string.
/home/docker/uberspark/docs/nextgen-toolkit/reference/uobjrtl/hw.rst:51: WARNING: Inline emphasis start-string without end-string.
<breathe>:1: WARNING: Unexpected indentation.

It doesn’t seem to be a big issue since the docs build fine anyway, but the lines that it warns about are completely empty…

I wrote a short python script to automatically generate the reference/uobjrtl/hw.rst file by reading the manifest, and was wondering if you think that’s a worthwhile feature to integrate into the doc building process.

Thanks!

It seems like we may need to do something about the documentation for CASM functions, as doxygen doesn’t recognize them at the moment.

You will need to change breathe_doxygen_config_options within docs/conf.py to add .cS files to the list of files that doxygen recognizes. Check https://www.doxygen.nl/manual/config.html#cfg_extension_mapping

Also, there is a minor issue that I noticed while building the docs and I can’t seem to figure out the reason. Here’s part of the build log:

There might be something wrong with your python script text encoding (e.g., spaces, tabs and line-feeds). When I replaced the auto-generated hw.rst with a manual one with just a single function, it seems to work without the errors.

I wrote a short python script to automatically generate the reference/uobjrtl/hw.rst file by reading the manifest, and was wondering if you think that’s a worthwhile feature to integrate into the doc building process.

This sounds great. You can perhaps stick it into src-nextgen/tools/docgen and add a README to describe how to use the tool.

Turns out it was an issue with the formatting of the doxygen comments in a couple of functions. I didn’t realize that the comments used reStructuredText formatting, so there were a couple of rogue asterisks and inconsistent indentation that messed with it (I wish the error messages were a little more clear lol).

Cool, I added the script and updated the contribution guide as well.

I added the .cS extension to the conf.py, and configured the extension mapping, but it still doesn’t recognize the CASM functions. Not sure what else I need to add to parse the CASM_FUNCDEFs.

Also, it looks the download page for doxygen used in the dockerfile disappeared (404 errors). I modified it to use a sourceforge mirror for the same version, and was wondering if you want me to add the new dockerfile to the PR.

Turns out it was an issue with the formatting of the doxygen comments in a couple of functions. I didn’t realize that the comments used reStructuredText formatting, so there were a couple of rogue asterisks and inconsistent indentation that messed with it (I wish the error messages were a little more clear lol).

Oh man. Nice find! We should probably update docs/contrib-guide/uobjrtl.rst to mention this explicitly.

added the .cS extension to the conf.py , and configured the extension mapping, but it still doesn’t recognize the CASM functions. Not sure what else I need to add to parse the CASM_FUNCDEF s.

You likely need to add the FILE_PATTERNS doxygen command in addition as it seems connected to the extension mapping option. See https://www.doxygen.nl/manual/config.html#cfg_file_patterns

Also, it looks the download page for doxygen used in the dockerfile disappeared (404 errors). I modified it to use a sourceforge mirror for the same version, and was wondering if you want me to add the new dockerfile to the PR.

Ahh bummer! They released a new version 1.8.20 and I guess they removed the previous tarball. In any case, we may want to directly use their github releases page: https://github.com/doxygen/doxygen/releases

Would be great if you can change the dockerfile to use the doxygen github repo above and bump it up to v1.8.20 in the process.

Thanks Ethan!