Messages in this thread | | | Date | Tue, 8 Feb 2022 16:02:02 +0100 | From | Borislav Petkov <> | Subject | Re: [PATCH v9 33/43] x86/compressed: Add SEV-SNP feature detection/setup |
| |
On Tue, Feb 08, 2022 at 07:50:09AM -0600, Michael Roth wrote: > I'm assuming that would be considered 'non-static' since it would need > to be exported if sev-shared.c was compiled separately instead of > directly #include'd.
No, that one can lose the prefix too. We'll cross that bridge when we get to it.
> And then there's also these which are static helpers that are only used > within sev-shared.c: > > snp_cpuid_info_get_ptr()
So looking at that one - and it felt weird reading that code because it said "cpuid_info" but that's not an "info" - that should be:
struct snp_cpuid_table { u32 count; u32 __reserved1; u64 __reserved2; struct snp_cpuid_fn fn[SNP_CPUID_COUNT_MAX]; } __packed;
just call it what it is - a SNP CPUID *table*.
And then you can have
const struct snp_cpuid_table *cpuid_tbl = snp_cpuid_get_table();
and that makes it crystal clear what this does.
> snp_cpuid_calc_xsave_size() > snp_cpuid_get_validated_func() > > snp_cpuid_check_range()
You can merge that small function into its only call site and put a comment above the code:
/* Check function is within the supported CPUID leafs ranges */
> snp_cpuid_hv() > snp_cpuid_postprocess() > snp_cpuid() > > but in those cases it seems useful to keep them grouped under the > snp_cpuid_* prefix since they become ambiguous otherwise, and > just using cpuid_* as a prefix (or suffix/etc) makes it unclear > that they are only used for SNP and not for general CPUID handling. > Should we leave those as-is?
Yap, the rest make sense to denote to what functionality they belong to.
Thx.
-- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |