Re: [PATCH 3/9] backends/confidential-guest-support: Add functions to support IGVM

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

On Tue, Feb 27, 2024 at 02:50:09PM +0000, Roy Hopkins wrote:
> In preparation for supporting the processing of IGVM files to configure
> guests, this adds a set of functions to ConfidentialGuestSupport
> allowing configuration of secure virtual machines that can be
> implemented for each supported isolation platform type such as Intel TDX
> or AMD SEV-SNP. These functions will be called by IGVM processing code
> in subsequent patches.
> 
> This commit provides a default implementation of the functions that
> either perform no action or generate a warning or error when they are
> called. Targets that support ConfidentalGuestSupport should override
> these implementations.
> 
> Signed-off-by: Roy Hopkins <roy.hopkins@suse.com>
> ---
>  backends/confidential-guest-support.c     | 26 ++++++++
>  include/exec/confidential-guest-support.h | 76 +++++++++++++++++++++++
>  2 files changed, 102 insertions(+)
> 
> diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
> index da436fb736..42628be8d7 100644
> --- a/backends/confidential-guest-support.c
> +++ b/backends/confidential-guest-support.c
> @@ -14,6 +14,7 @@
>  #include "qemu/osdep.h"
>  
>  #include "exec/confidential-guest-support.h"
> +#include "qemu/error-report.h"
>  
>  OBJECT_DEFINE_ABSTRACT_TYPE(ConfidentialGuestSupport,
>                              confidential_guest_support,
> @@ -45,8 +46,33 @@ static void confidential_guest_support_class_init(ObjectClass *oc, void *data)
>  #endif
>  }
>  
> +static int check_support(ConfidentialGuestPlatformType platform,
> +                         uint16_t platform_version, uint8_t highest_vtl,
> +                         uint64_t shared_gpa_boundary)
> +{
> +    /* Default: no support. */
> +    return 0;
> +}
> +
> +static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
> +                                 ConfidentialGuestPageType memory_type,
> +                                 uint16_t cpu_index)
> +{
> +    warn_report("Confidential guest memory not supported");
> +    return -1;
> +}
> +
> +static int get_mem_map_entry(int index, ConfidentialGuestMemoryMapEntry *entry)
> +{
> +    return 1;
> +}

IIUC, all these can reports errors, and as such I think
they should have an 'Error **errp' parameter, so we can
report precise errors in these methods, rather than
less useful generic errors in the caller.

The above 'warn_report' ought to be an error too, since
it is returning an failure code (-1)

> +
>  static void confidential_guest_support_init(Object *obj)
>  {
> +    ConfidentialGuestSupport *cgs = CONFIDENTIAL_GUEST_SUPPORT(obj);
> +    cgs->check_support = check_support;
> +    cgs->set_guest_state = set_guest_state;
> +    cgs->get_mem_map_entry = get_mem_map_entry;
>  }
>  
>  static void confidential_guest_support_finalize(Object *obj)
> diff --git a/include/exec/confidential-guest-support.h b/include/exec/confidential-guest-support.h
> index b08ad8de4d..c43a1a32f1 100644
> --- a/include/exec/confidential-guest-support.h
> +++ b/include/exec/confidential-guest-support.h
> @@ -21,10 +21,46 @@
>  #ifndef CONFIG_USER_ONLY
>  
>  #include "qom/object.h"
> +#include "exec/hwaddr.h"
> +
> +#if defined(CONFIG_IGVM)
> +#include "igvm/igvm.h"
> +#endif
>  
>  #define TYPE_CONFIDENTIAL_GUEST_SUPPORT "confidential-guest-support"
>  OBJECT_DECLARE_SIMPLE_TYPE(ConfidentialGuestSupport, CONFIDENTIAL_GUEST_SUPPORT)
>  
> +typedef enum ConfidentialGuestPlatformType {
> +    CGS_PLATFORM_SEV,
> +    CGS_PLATFORM_SEV_ES,
> +    CGS_PLATFORM_SEV_SNP,
> +    CGS_PLATFORM_TDX,

QEMU only has support for SEV & SEV_ES today. We should leave the
others until we actually get an impl of SEV-SNP/TDX in QEMU that
supports those platforms.

> +} ConfidentialGuestPlatformType;
> +
> +typedef enum ConfidentialGuestMemoryType {
> +    CGS_MEM_RAM,
> +    CGS_MEM_RESERVED,
> +    CGS_MEM_ACPI,
> +    CGS_MEM_NVS,
> +    CGS_MEM_UNUSABLE,
> +} ConfidentialGuestMemoryType;
> +
> +typedef struct ConfidentialGuestMemoryMapEntry {
> +    uint64_t gpa;
> +    uint64_t size;
> +    ConfidentialGuestMemoryType type;
> +} ConfidentialGuestMemoryMapEntry;
> +
> +typedef enum ConfidentialGuestPageType {
> +    CGS_PAGE_TYPE_NORMAL,
> +    CGS_PAGE_TYPE_VMSA,
> +    CGS_PAGE_TYPE_ZERO,
> +    CGS_PAGE_TYPE_UNMEASURED,
> +    CGS_PAGE_TYPE_SECRETS,
> +    CGS_PAGE_TYPE_CPUID,
> +    CGS_PAGE_TYPE_REQUIRED_MEMORY,
> +} ConfidentialGuestPageType;
> +
>  struct ConfidentialGuestSupport {
>      Object parent;
>  
> @@ -60,6 +96,46 @@ struct ConfidentialGuestSupport {
>       */
>      char *igvm_filename;
>  #endif
> +
> +    /*
> +     * The following virtual methods need to be implemented by systems that
> +     * support confidential guests that can be configured with IGVM and are
> +     * used during processing of the IGVM file with process_igvm().
> +     */
> +
> +    /*
> +     * Check for to see if this confidential guest supports a particular
> +     * platform or configuration
> +     */
> +    int (*check_support)(ConfidentialGuestPlatformType platform,
> +                         uint16_t platform_version, uint8_t highest_vtl,
> +                         uint64_t shared_gpa_boundary);
> +
> +    /*
> +     * Configure part of the state of a guest for a particular set of data, page
> +     * type and gpa. This can be used for example to pre-populate and measure
> +     * guest memory contents, define private ranges or set the initial CPU state
> +     * for one or more CPUs.
> +     *
> +     * If memory_type is CGS_PAGE_TYPE_VMSA then ptr points to the initial CPU
> +     * context for a virtual CPU. The format of the data depends on the type of
> +     * confidential virtual machine. For example, for SEV-ES ptr will point to a
> +     * vmcb_save_area structure that should be copied into guest memory at the
> +     * address specified in gpa. The cpu_index parameter contains the index of
> +     * the CPU the VMSA applies to.
> +     */
> +    int (*set_guest_state)(hwaddr gpa, uint8_t *ptr, uint64_t len,
> +                                 ConfidentialGuestPageType memory_type,
> +                                 uint16_t cpu_index);
> +
> +    /*
> +     * Iterate the system memory map, getting the entry with the given index
> +     * that can be populated into guest memory.
> +     *
> +     * Returns 1 if the index is out of range.
> +     */
> +    int (*get_mem_map_entry)(int index,
> +                              ConfidentialGuestMemoryMapEntry *entry);
>  };
>  
>  typedef struct ConfidentialGuestSupportClass {
> -- 
> 2.43.0
> 
> 

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 :|