Re: [PATCH v6 00/10] TDX host: metadata reading tweaks, bug fix and info dump

[Huang, Kai]

On 29/10/2024 6:59 am, Paolo Bonzini wrote:
> On 10/28/24 13:41, Kai Huang wrote:
>
> > v5 -> v6:
> >   - Change to use a script [*] to auto-generate metadata reading code.
> >
> >    - https://lore.kernel.org/kvm/f25673ea-08c5-474b- 
> > a841-095656820b67@xxxxxxxxx/
> >    - https://lore.kernel.org/kvm/ 
> > CABgObfYXUxqQV_FoxKjC8U3t5DnyM45nz5DpTxYZv2x_uFK_Kw@xxxxxxxxxxxxxx/
> >
> >     Per Dave, this patchset doesn't contain a patch to add the script
> >     to the kernel tree but append it in this cover letter in order to
> >     minimize the review effort.
>
> I think Dave did want to check it in, but not tie it to the build (so 
> that you don't need to have global_metadata.json).
> > You can add an eleventh patch (or a v7 just for patch 3) that adds 
it in
> scripts/.  Maybe also add a
>
> print("/* Generated from global_metadata.json by scripts/ 
> tdx_parse_metadata.py */", file=f);
>
> line to the script, for both hfile and cfile?

Sure I can do.  But IIUC Dave wanted to keep this series simple and we 
can start with appending the script to the coverletter (we had as short 
chat internally).

Hi Dave, what's your preference?

> >   - Change to use auto-generated code to read TDX module version,
> >     supported features and CMRs in one patch, and made that from and
> >     signed by Paolo.
> >   - Couple of new patches due to using the auto-generated code
> >   - Remove the "reading metadata" part (due to they are auto-generated
> >     in one patch now) from the consumer patches.
>
> >      print(file=file)
> >      for f in fields:
> >          fname = f["Field Name"]
> >          field_id = f["Base FIELD_ID (Hex)"]
> >          num_fields = int(f["Num Fields"])
> >          num_elements = int(f["Num Elements"])
> >          struct_member = fname.lower()
> >          indent = "\t"
> >          if num_fields > 1:
> >              if fname == "CMR_BASE" or fname == "CMR_SIZE":
> >                  limit = "sysinfo_cmr->num_cmrs"
> >              elif fname == "CPUID_CONFIG_LEAVES" or fname == 
> > "CPUID_CONFIG_VALUES":
> >                  limit = "sysinfo_td_conf->num_cpuid_config"
>
> Thanks Intel for not telling the whole story in the "Num Fields" value 
> of global_metadata.json. :)

Yeah I feel bad about this too.  I asked whether we can use the value 
reported in "Num Fields" as the limit to loop, but I was told we should 
use the value reported in "NUM_CMRS". :-(