[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <670ebca6e8aa0_3f14294cf@dwillia2-xfh.jf.intel.com.notmuch>
Date: Tue, 15 Oct 2024 12:04:07 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: Paolo Bonzini <pbonzini@...hat.com>, Dave Hansen <dave.hansen@...el.com>
CC: Kai Huang <kai.huang@...el.com>, <kirill.shutemov@...ux.intel.com>,
<tglx@...utronix.de>, <bp@...en8.de>, <peterz@...radead.org>,
<mingo@...hat.com>, <hpa@...or.com>, <dan.j.williams@...el.com>,
<seanjc@...gle.com>, <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
<kvm@...r.kernel.org>, <rick.p.edgecombe@...el.com>,
<isaku.yamahata@...el.com>, <adrian.hunter@...el.com>, <nik.borisov@...e.com>
Subject: Re: [PATCH v5 0/8] TDX host: metadata reading tweaks, bug fix and
info dump
Paolo Bonzini wrote:
> On Tue, Oct 15, 2024 at 5:30 PM Dave Hansen <dave.hansen@...el.com> 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@...hat.om>
>
> 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@dwillia2-xfh.jf.intel.com.notmuch
Powered by blists - more mailing lists