Re: [PATCH 2/2] target/i386/sev: Reduce system specific declarations

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Dec 18, 2024 at 04:59:13PM +0100, Philippe Mathieu-Daudé wrote:
> "system/confidential-guest-support.h" is not needed,
> remove it. Reorder #ifdef'ry to reduce declarations
> exposed on user emulation.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/i386/sev.h  | 29 ++++++++++++++++-------------
>  hw/i386/pc_sysfw.c |  2 +-
>  2 files changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/target/i386/sev.h b/target/i386/sev.h
> index 2664c0b1b6c..373669eaace 100644
> --- a/target/i386/sev.h
> +++ b/target/i386/sev.h
> @@ -18,7 +18,17 @@
>  #include CONFIG_DEVICES /* CONFIG_SEV */
>  #endif
>  
> -#include "system/confidential-guest-support.h"
> +#if !defined(CONFIG_SEV) || defined(CONFIG_USER_ONLY)
> +#define sev_enabled() 0
> +#define sev_es_enabled() 0
> +#define sev_snp_enabled() 0
> +#else
> +bool sev_enabled(void);
> +bool sev_es_enabled(void);
> +bool sev_snp_enabled(void);
> +#endif
> +
> +#if !defined(CONFIG_USER_ONLY)

I'm surprised any of this header file is relevant to
user mode. If something is mistakely calling sev_ functions
from user mode compiled code, I'd be inclined to fix the
caller such that its #include ".../sev.h" can be wrapped
by !CONFIG_USER_ONLY

>  
>  #define TYPE_SEV_COMMON "sev-common"
>  #define TYPE_SEV_GUEST "sev-guest"
> @@ -45,18 +55,6 @@ typedef struct SevKernelLoaderContext {
>      size_t cmdline_size;
>  } SevKernelLoaderContext;
>  
> -#ifdef CONFIG_SEV
> -bool sev_enabled(void);
> -bool sev_es_enabled(void);
> -bool sev_snp_enabled(void);
> -#else
> -#define sev_enabled() 0
> -#define sev_es_enabled() 0
> -#define sev_snp_enabled() 0
> -#endif
> -
> -uint32_t sev_get_cbit_position(void);
> -uint32_t sev_get_reduced_phys_bits(void);
>  bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp);
>  
>  int sev_encrypt_flash(hwaddr gpa, uint8_t *ptr, uint64_t len, Error **errp);
> @@ -68,4 +66,9 @@ void sev_es_set_reset_vector(CPUState *cpu);
>  
>  void pc_system_parse_sev_metadata(uint8_t *flash_ptr, size_t flash_size);
>  
> +#endif /* !CONFIG_USER_ONLY */
> +
> +uint32_t sev_get_cbit_position(void);
> +uint32_t sev_get_reduced_phys_bits(void);
> +
>  #endif
> diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
> index da7ed121292..1eeb58ab37f 100644
> --- a/hw/i386/pc_sysfw.c
> +++ b/hw/i386/pc_sysfw.c
> @@ -36,7 +36,7 @@
>  #include "hw/qdev-properties.h"
>  #include "hw/block/flash.h"
>  #include "system/kvm.h"
> -#include "sev.h"
> +#include "target/i386/sev.h"
>  
>  #define FLASH_SECTOR_SIZE 4096
>  
> -- 
> 2.45.2
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|