On 2/20/24 07:31, Takashi Kajinami wrote:
> This introduces the new "model" field in sev elements, returned by
> domain capabilities API, so that client can ensure SEV-ES is available
> in this hypervisor.
>
> Signed-off-by: Takashi Kajinami <kajinamit@xxxxxxxxxxxxxxx>
> ---
> docs/formatdomaincaps.rst | 5 ++
> src/conf/domain_capabilities.c | 2 +
> src/conf/domain_capabilities.h | 1 +
> src/conf/domain_conf.c | 7 +++
> src/conf/domain_conf.h | 8 ++++
> src/qemu/qemu_capabilities.c | 84 +++++++++++++++++++++++++---------
> 6 files changed, 85 insertions(+), 22 deletions(-)
>
> diff --git a/docs/formatdomaincaps.rst b/docs/formatdomaincaps.rst
> index ef752a0f3a..78a6f8736f 100644
> --- a/docs/formatdomaincaps.rst
> +++ b/docs/formatdomaincaps.rst
> @@ -753,6 +753,11 @@ in domain XML <formatdomain.html#launch-security>`__
> ``maxESGuests``
> The maximum number of SEV-ES guests that can be launched on the host. This
> value may be configurable in the firmware for some hosts.
> +``cpu0Id``
> + ID of CPU0, which is used to get the signed Chip Endorsement Key (CEK) of
> + the CPU of AMD system from AMD's Key Distribution Service (KDS).
> +``model``
> + Available SEV models.
>
> SGX capabilities
> ^^^^^^^^^^^^^^^^
> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
> index 68eb3c9797..26d9b0a21c 100644
> --- a/src/conf/domain_capabilities.c
> +++ b/src/conf/domain_capabilities.c
> @@ -654,6 +654,8 @@ virDomainCapsFeatureSEVFormat(virBuffer *buf,
> if (sev->cpu0_id != NULL)
> virBufferAsprintf(buf, "<cpu0Id>%s</cpu0Id>\n", sev->cpu0_id);
>
> + ENUM_PROCESS(sev, model, virDomainSevModelTypeToString);
> +
> virBufferAdjustIndent(buf, -2);
> virBufferAddLit(buf, "</sev>\n");
> }
> diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
> index fadc30cdd7..1a84ea6101 100644
> --- a/src/conf/domain_capabilities.h
> +++ b/src/conf/domain_capabilities.h
> @@ -213,6 +213,7 @@ struct _virSEVCapability {
> unsigned int reduced_phys_bits;
> unsigned int max_guests;
> unsigned int max_es_guests;
> + virDomainCapsEnum model;
> };
>
> typedef struct _virSGXSection virSGXSection;
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 3597959e33..cf0077d584 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1509,6 +1509,13 @@ VIR_ENUM_IMPL(virDomainLaunchSecurity,
> "s390-pv",
> );
>
> +VIR_ENUM_IMPL(virDomainSevModel,
> + VIR_DOMAIN_SEV_MODEL_LAST,
> + "",
> + "sev",
> + "sev-es",
> +);
> +
> typedef enum {
> VIR_DOMAIN_NET_VHOSTUSER_MODE_NONE,
> VIR_DOMAIN_NET_VHOSTUSER_MODE_CLIENT,
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index c7e5005b3b..a06fde1032 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2854,6 +2854,13 @@ typedef enum {
> VIR_DOMAIN_LAUNCH_SECURITY_LAST,
> } virDomainLaunchSecurity;
>
> +typedef enum {
> + VIR_DOMAIN_SEV_MODEL_NONE,
> + VIR_DOMAIN_SEV_MODEL_SEV,
> + VIR_DOMAIN_SEV_MODEL_SEV_ES,
There's SEV SNP now too.
> +
> + VIR_DOMAIN_SEV_MODEL_LAST,
> +} virDomainSevModel;
>
> struct _virDomainSEVDef {
> char *dh_cert;
> @@ -4237,6 +4244,7 @@ VIR_ENUM_DECL(virDomainCryptoType);
> VIR_ENUM_DECL(virDomainCryptoBackend);
> VIR_ENUM_DECL(virDomainShmemModel);
> VIR_ENUM_DECL(virDomainShmemRole);
> +VIR_ENUM_DECL(virDomainSevModel);
> VIR_ENUM_DECL(virDomainLaunchSecurity);
> /* from libvirt.h */
> VIR_ENUM_DECL(virDomainState);
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index e383d85920..d264c3128c 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -1881,6 +1881,7 @@ virQEMUCapsSEVInfoCopy(virSEVCapability **dst,
> tmp->reduced_phys_bits = src->reduced_phys_bits;
> tmp->max_guests = src->max_guests;
> tmp->max_es_guests = src->max_es_guests;
> + tmp->model = src->model;
>
> *dst = g_steal_pointer(&tmp);
> }
> @@ -3402,6 +3403,62 @@ virQEMUCapsGetSEVMaxGuests(virSEVCapability *caps)
> }
> }
>
> +
> +/*
> + * Check whether AMD Secure Encrypted Virtualization (x86) is enabled
> + */
> +static bool
> +virQEMUCapsKVMSupportsSecureGuestSEV(void)
> +{
> + g_autofree char *modValue = NULL;
> +
> + if (virFileReadValueString(&modValue, "/sys/module/kvm_amd/parameters/sev") < 0)
> + return false;
> +
> + if (modValue[0] != '1' && modValue[0] != 'Y' && modValue[0] != 'y')
> + return false;
> +
> + if (virFileExists(QEMU_DEV_SEV))
> + return true;
> +
> + return false;
> +}
> +
> +
> +/*
> + * Check whether AMD Secure Encrypted Virtualization-Encrypted State (x86) is enabled
> + */
> +static bool
> +virQEMUCapsKVMSupportsSecureGuestSEVES(void)
> +{
> + g_autofree char *modValue = NULL;
> +
> + if (virFileReadValueString(&modValue, "/sys/module/kvm_amd/parameters/sev_es") < 0)
> + return false;
> +
> + if (modValue[0] != '1' && modValue[0] != 'Y' && modValue[0] != 'y')
> + return false;
> +
> + if (virFileExists(QEMU_DEV_SEV))
> + return true;
> +
> + return false;
> +}
These two functions differ only in the path they query. Now, I'm not
sure the kernel module can have "Y" without /dev/sev existing.
Anyway, I think one of these function should be turned into
static bool
func(const char *path)
{
virFileReadValueString(&modValue, path);
if (modValue[0] != 'y' ...)
return false;
return true;
}
and then ...
> +
> +
> +static void
> +virQEMUCapsGetSEVModel(virSEVCapability *caps)
> +{
> + caps->model.report = true;
.. here we can have a static array of tuples:
const char *params[VIR_DOMAIN_SEV_MODEL_LAST] = {
[VIR_DOMAIN_SEV_MODEL_SEV] = "/sys/module/kvm_amd/parameters/sev",
[VIR_DOMAIN_SEV_MODEL_SEV_ES] = "/sys/module/kvm_amd/parameters/sev_es",
};
size_t i;
caps->model.report = true;
for (i = 0; i < VIR_DOMAIN_SEV_MODEL_LAST; i++) {
if (virQEMUCapsKVMSupportsSecureGuestSEVES(params[i]))
VIR_DOMAIN_CAPS_ENUM_SET(caps->model, i);
}
Or something among those lines.
Michal
