| Message ID | 0da76c3956bf776a9bbb0e18a1813b8dc5e20bf8.1712141833.git.roy.hopkins@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,01/10] meson: Add optional dependency on IGVM library | expand |
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
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
diff --git a/backends/meson.build b/backends/meson.build index 8b2b111497..d550ac19f7 100644 --- a/backends/meson.build +++ b/backends/meson.build @@ -30,5 +30,8 @@ if have_vhost_user_crypto endif system_ss.add(when: gio, if_true: files('dbus-vmstate.c')) system_ss.add(when: 'CONFIG_SGX', if_true: files('hostmem-epc.c')) +if igvm.found() + system_ss.add(igvm) +endif subdir('tpm') diff --git a/meson.build b/meson.build index c9c3217ba4..f0b5a29ce7 100644 --- a/meson.build +++ b/meson.build @@ -1232,6 +1232,12 @@ if host_os == 'linux' and (have_system or have_tools) method: 'pkg-config', required: get_option('libudev')) endif +igvm = not_found +if not get_option('igvm').auto() or have_system + igvm = dependency('igvm', + method: 'pkg-config', + required: get_option('igvm')) +endif mpathlibs = [libudev] mpathpersist = not_found @@ -2320,6 +2326,7 @@ config_host_data.set('CONFIG_CFI', get_option('cfi')) config_host_data.set('CONFIG_SELINUX', selinux.found()) config_host_data.set('CONFIG_XEN_BACKEND', xen.found()) config_host_data.set('CONFIG_LIBDW', libdw.found()) +config_host_data.set('CONFIG_IGVM', igvm.found()) if xen.found() # protect from xen.version() having less than three components xen_version = xen.version().split('.') + ['0', '0'] @@ -4456,6 +4463,7 @@ summary_info += {'seccomp support': seccomp} summary_info += {'GlusterFS support': glusterfs} summary_info += {'hv-balloon support': hv_balloon} summary_info += {'TPM support': have_tpm} +summary_info += {'IGVM support': igvm} summary_info += {'libssh support': libssh} summary_info += {'lzo support': lzo} summary_info += {'snappy support': snappy} diff --git a/meson_options.txt b/meson_options.txt index 0a99a059ec..4eaba64f4b 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -109,6 +109,8 @@ option('dbus_display', type: 'feature', value: 'auto', description: '-display dbus support') option('tpm', type : 'feature', value : 'auto', description: 'TPM support') +option('igvm', type: 'feature', value: 'auto', + description: 'Independent Guest Virtual Machine (IGVM) file support') # Do not enable it by default even for Mingw32, because it doesn't # work on Wine. diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh index 680fa3f581..38a8183625 100644 --- a/scripts/meson-buildoptions.sh +++ b/scripts/meson-buildoptions.sh @@ -126,6 +126,7 @@ meson_options_help() { printf "%s\n" ' hv-balloon hv-balloon driver (requires Glib 2.68+ GTree API)' printf "%s\n" ' hvf HVF acceleration support' printf "%s\n" ' iconv Font glyph conversion support' + printf "%s\n" ' igvm IGVM file support' printf "%s\n" ' jack JACK sound support' printf "%s\n" ' keyring Linux keyring support' printf "%s\n" ' kvm KVM acceleration support' @@ -342,6 +343,8 @@ _meson_option_parse() { --iasl=*) quote_sh "-Diasl=$2" ;; --enable-iconv) printf "%s" -Diconv=enabled ;; --disable-iconv) printf "%s" -Diconv=disabled ;; + --enable-igvm) printf "%s" -Digvm=enabled ;; + --disable-igvm) printf "%s" -Digvm=disabled ;; --includedir=*) quote_sh "-Dincludedir=$2" ;; --enable-install-blobs) printf "%s" -Dinstall_blobs=true ;; --disable-install-blobs) printf "%s" -Dinstall_blobs=false ;;
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. This adds the --enable-igvm configure option, enabled by default, which attempts to locate and link against the IGVM library via pkgconfig and sets CONFIG_IGVM if found. The library is added to the system_ss target in backends/meson.build where the IGVM parsing will be performed by the ConfidentialGuestSupport object. Signed-off-by: Roy Hopkins <roy.hopkins@suse.com> --- backends/meson.build | 3 +++ meson.build | 8 ++++++++ meson_options.txt | 2 ++ scripts/meson-buildoptions.sh | 3 +++ 4 files changed, 16 insertions(+)