On Fri, 2024-03-01 at 16:37 +0000, Daniel P. Berrangé wrote:
> 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.hopk...@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)
Yes, that makes sense. I've made a comprehensive rework of the error handling in
the patch series which addresses this and your suggestions in the other patches
regarding error handling. I'll submit these as a V2.
>
> > +
> > 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.
Ok. I'll remove the currently unsupported 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
