On 16/10/2024 8:04 am, Dan Williams wrote:
Paolo Bonzini wrote:
On Tue, Oct 15, 2024 at 5:30 PM Dave Hansen <dave.hansen@xxxxxxxxx> wrote:
I'm having one of those "I hate this all" moments. Look at what we say
in the code:
* See the "global_metadata.json" in the "TDX 1.5 ABI definitions".
Basically step one in verifying that this is all right is: Hey, humans,
please go parse a machine-readable format. That's insanity. If Intel
wants to publish JSON as the canonical source of truth, that's fine.
It's great, actually. But let's stop playing human JSON parser and make
the computers do it for us, OK?
Let's just generate the code. Basically, as long as the generated C is
marginally readable, I'm OK with it. The most important things are:
1. Adding a field is dirt simple
2. Using the generated C is simple
In 99% of the cases, nobody ends up having to ever look at the generated
code.
Take a look at the attached python program and generated C file. I
think they qualify. We can check the script into tools/scripts/ and it
can get re-run when new json comes out or when a new field is needed.
You'd could call the generated code like this:
Ok, so let's move this thing forward. Here is a more polished script
and the output. Untested beyond compilation.
Kai, feel free to include it in v6 with my
Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxx>
I made an attempt at adding array support and using it with the CMR
information; just to see if Intel is actually trying to make
global_metadata.json accurate. The original code has
for (i = 0; i < sysinfo_cmr->num_cmrs; i++) {
READ_SYS_INFO(CMR_BASE + i, cmr_base[i]);
READ_SYS_INFO(CMR_SIZE + i, cmr_size[i]);
}
The generated code instead always tries to read 32 fields and returns
non-zero from get_tdx_sys_info_cmr if they are missing. If it fails to
read the fields above NUM_CMRS, just remove that part of the tdx.py
script and make sure that a comment in the code shames the TDX ABI
documentation adequately. :)
Thanks for doing this Paolo, I regret not pushing harder [1] / polishing
up the bash+jq script I threw together to do the same.
I took a look at your script and the autogenerated code and it looks good
to me.
Feel free to add my Reviewed-by on a patch that adds that collateral to
the tools/ directory.
[1]: http://lore.kernel.org/66b19beaadd28_4fc729410@xxxxxxxxxxxxxxxxxxxxxxxxx.notmuch
Hi Dave/Paolo/Dan,
I'll go with this for the next version.
Thanks for all your feedback and the code you shared. I appreciate.
