Re: [PATCH v1 1/8] x86/virt/tdx: Stop treating tdx_global_metadata.h as auto-generated
Chao Gao <[email protected]>
| Newsgroups | dev.linux.lists.linux-coco,org.kernel.vger.kvm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
>> Today the header that holds these structures is generated by an >> out-of-tree script from a JSON file listing all of the TDX module's >> metadata. That made it trivial to add a new field, but everything >> else suffered for it: >> >> - The header is opaque to anyone who doesn't have the script and >> the JSON file handy, and the "Automatically generated" tag tells >> reviewers their edits will be clobbered. >> - The script ships outside the tree, so reproducing changes >> requires fetching it from a mailing list link. >> - The structures are short and stable; the script's value over >> a hand-edited header is small. > >Well, and the big one: after we started doing this, the "ABI Definitions >for Intel® TDX" was declared to be not an ABI. So, even if we code to >the JSON, there's no guarantee the JSON will be stable. > ><sigh> > >Honestly that's what matters. The script and all the other fluff is just >noise. Thanks for this. So the key point is that auto-generating rests on a premise: the JSON is the ABI and stays stable. Then regenerating gives you something you can trust without re-checking. That premise isn't true now, so we should stop auto-generating. One thing I'd like to confirm: is the concern that the JSON could be wrong about what modules actually implement, or just that there's no commitment to keep the JSON stable? or both? > >> Signed-off-by: Chao Gao <[email protected]> > >So, I'm curious: What made you feel OK to sign-off on this? I'm not >judging. I'm open-minded on this. All I know is *I* wasn't ready to >sign-off on this. > >I really want to know what your thought process was. The idea and the implementation looks good to me, and I thought they were ready for on-list review. As the person who modified and posted the series, my reading of submitting-patches.rst is that I need to sign off _any_ patches I posted.