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