Re: [PATCH v2 01/10] meson: Add optional dependency on IGVM library

[Roy Hopkins <roy.hopkins@suse.com>]

On Tue, 2024-04-16 at 15:13 +0100, Daniel P. Berrangé wrote:
> On Wed, Apr 03, 2024 at 12:11:32PM +0100, Roy Hopkins wrote:
> > The IGVM library allows Independent Guest Virtual Machine files to be
> > parsed and processed. IGVM files are used to configure guest memory
> > layout, initial processor state and other configuration pertaining to
> > secure virtual machines.
> 
> Looking at the generated header file for the IGVM library, I see
> some quite bad namespace pollution. eg igvm_defs.h has:
> 
> typedef uint64_t u64_le;
> typedef uint32_t u32_le;
> 
> #define UINT32_FLAGS_VALUE(x) *((uint32_t*)&(x))
> 
> #define MAKE_INVALID_IMPL(x, y) x##y
> #define MAKE_INVALID(x, y) MAKE_INVALID_IMPL(x, y)
> #define INVALID MAKE_INVALID(INVALID_, __COUNTER__)
> 
> enum IgvmPageDataType {
>   NORMAL = 0,
>   SECRETS = 1,
>   CPUID_DATA = 2,
>   CPUID_XF = 3,
> };
> 
> enum IgvmPlatformType {
>   NATIVE = 0,
>   VSM_ISOLATION = 1,
>   SEV_SNP = 2,
>   TDX = 3,
> };
> 
> 
> 
> enum IgvmVariableHeaderType {
>   INVALID = 0,
>   ...
> }
> 
> There are soo many more examples in igvm_defs.h that I won't
> list them all here.
> 
> 
> These are all way too generic as names to be exposing in library
> header file. We may be lucky right now that these definitions
> don't clash with anything else defined in the compilation namespace
> of the consuming application, but that's a bad bet to make long
> term.
> 
> IMHO this really needs fixing before there's any use of this igvm
> library, since fixing it will be a backwards-incompatible change.
> 
> Essentially everything in the header needs to have an 'IGVM/Igvm/igvm'
> prefix on it.
> 
> With regards,
> Daniel

Daniel,

I've just submitted a patch for the IGVM C library to fix this, ensuring
everything is now uniquely identified with an IGVM_/Igvm_ prefix and fixing some
of the other issues in the generated header file:
https://github.com/microsoft/igvm/pull/52

I'll update this with the changes once merged.

Thanks,
Roy