Re: [PATCH v3 31/49] i386/sev: Update query-sev QAPI format to handle SEV-SNP
Michael Roth writes:
> Most of the current 'query-sev' command is relevant to both legacy
> SEV/SEV-ES guests and SEV-SNP guests, with 2 exceptions:
>
> - 'policy' is a 64-bit field for SEV-SNP, not 32-bit, and
> the meaning of the bit positions has changed
> - 'handle' is not relevant to SEV-SNP
>
> To address this, this patch adds a new 'sev-type' field that can be
> used as a discriminator to select between SEV and SEV-SNP-specific
> fields/formats without breaking compatibility for existing management
> tools (so long as management tools that add support for launching
> SEV-SNP guest update their handling of query-sev appropriately).
>
> The corresponding HMP command has also been fixed up similarly.
>
> Signed-off-by: Michael Roth
> ---
> qapi/misc-target.json | 71 ++-
> target/i386/sev.c | 50 --
> target/i386/sev.h | 3 ++
> 3 files changed, 94 insertions(+), 30 deletions(-)
>
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index 4e0a6492a9..daceb85d95 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -47,6 +47,49 @@
> 'send-update', 'receive-update' ],
>'if': 'TARGET_I386' }
>
> +##
> +# @SevGuestType:
> +#
> +# An enumeration indicating the type of SEV guest being run.
> +#
> +# @sev: The guest is a legacy SEV or SEV-ES guest.
Single space after ':', please.
Recommend a blank line between argument descriptions.
> +# @sev-snp: The guest is an SEV-SNP guest.
> +#
> +# Since: 6.2
The type is since 9.1, but its members become results of query-sev,
where they are since 2.12. See also my reply to Daniel's question on
PATCH 21.
> +##
> +{ 'enum': 'SevGuestType',
> + 'data': [ 'sev', 'sev-snp' ],
> + 'if': 'TARGET_I386' }
> +
> +##
> +# @SevGuestInfo:
> +#
> +# Information specific to legacy SEV/SEV-ES guests.
> +#
> +# @policy: SEV policy value
I know you're just moving existing documentation. Still: this is rather
sparse. Where would I find what numbers to pass for @policy?
> +#
> +# @handle: SEV firmware handle
> +#
> +# Since: 2.12
> +##
> +{ 'struct': 'SevGuestInfo',
> + 'data': { 'policy': 'uint32',
> +'handle': 'uint32' },
> + 'if': 'TARGET_I386' }
> +
> +##
> +# @SevSnpGuestInfo:
> +#
> +# Information specific to SEV-SNP guests.
> +#
> +# @snp-policy: SEV-SNP policy value
Same question.
> +#
> +# Since: 6.2
9.1
> +##
> +{ 'struct': 'SevSnpGuestInfo',
> + 'data': { 'snp-policy': 'uint64' },
> + 'if': 'TARGET_I386' }
> +
> ##
> # @SevInfo:
> #
> @@ -60,25 +103,25 @@
> #
> # @build-id: SEV FW build id
> #
> -# @policy: SEV policy value
> -#
> # @state: SEV guest state
> #
> -# @handle: SEV firmware handle
> +# @sev-type: Type of SEV guest being run
> #
> # Since: 2.12
> ##
> -{ 'struct': 'SevInfo',
> -'data': { 'enabled': 'bool',
> - 'api-major': 'uint8',
> - 'api-minor' : 'uint8',
> - 'build-id' : 'uint8',
> - 'policy' : 'uint32',
> - 'state' : 'SevState',
> - 'handle' : 'uint32'
> -},
> - 'if': 'TARGET_I386'
> -}
> +{ 'union': 'SevInfo',
> + 'base': { 'enabled': 'bool',
> +'api-major': 'uint8',
> +'api-minor' : 'uint8',
> +'build-id' : 'uint8',
> +'state' : 'SevState',
> +'sev-type' : 'SevGuestType' },
> + 'discriminator': 'sev-type',
> + 'data': {
> + 'sev': 'SevGuestInfo',
> + 'sev-snp': 'SevSnpGuestInfo' },
> + 'if': 'TARGET_I386' }
> +
>
> ##
> # @query-sev:
[...]
Re: [PATCH v3 31/49] i386/sev: Update query-sev QAPI format to handle SEV-SNP
On Wed, Mar 20, 2024 at 12:10:04PM +, Daniel P. Berrangé wrote:
> On Wed, Mar 20, 2024 at 03:39:27AM -0500, Michael Roth wrote:
> > Most of the current 'query-sev' command is relevant to both legacy
> > SEV/SEV-ES guests and SEV-SNP guests, with 2 exceptions:
> >
> > - 'policy' is a 64-bit field for SEV-SNP, not 32-bit, and
> > the meaning of the bit positions has changed
> > - 'handle' is not relevant to SEV-SNP
> >
> > To address this, this patch adds a new 'sev-type' field that can be
> > used as a discriminator to select between SEV and SEV-SNP-specific
> > fields/formats without breaking compatibility for existing management
> > tools (so long as management tools that add support for launching
> > SEV-SNP guest update their handling of query-sev appropriately).
> >
> > The corresponding HMP command has also been fixed up similarly.
> >
> > Signed-off-by: Michael Roth
> > ---
> > qapi/misc-target.json | 71 ++-
> > target/i386/sev.c | 50 --
> > target/i386/sev.h | 3 ++
> > 3 files changed, 94 insertions(+), 30 deletions(-)
> >
> > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > index 4e0a6492a9..daceb85d95 100644
> > --- a/qapi/misc-target.json
> > +++ b/qapi/misc-target.json
> > @@ -47,6 +47,49 @@
> > 'send-update', 'receive-update' ],
> >'if': 'TARGET_I386' }
> >
> > +##
> > +# @SevGuestType:
> > +#
> > +# An enumeration indicating the type of SEV guest being run.
> > +#
> > +# @sev: The guest is a legacy SEV or SEV-ES guest.
> > +# @sev-snp: The guest is an SEV-SNP guest.
> > +#
> > +# Since: 6.2
>
> Now 9.1 at the earliest.
>
> > +##
> > +{ 'enum': 'SevGuestType',
> > + 'data': [ 'sev', 'sev-snp' ],
> > + 'if': 'TARGET_I386' }
> > +
> > +##
> > +# @SevGuestInfo:
> > +#
> > +# Information specific to legacy SEV/SEV-ES guests.
> > +#
> > +# @policy: SEV policy value
> > +#
> > +# @handle: SEV firmware handle
> > +#
> > +# Since: 2.12
> > +##
> > +{ 'struct': 'SevGuestInfo',
> > + 'data': { 'policy': 'uint32',
> > +'handle': 'uint32' },
> > + 'if': 'TARGET_I386' }
> > +
> > +##
> > +# @SevSnpGuestInfo:
> > +#
> > +# Information specific to SEV-SNP guests.
> > +#
> > +# @snp-policy: SEV-SNP policy value
> > +#
> > +# Since: 6.2
> > +##
> > +{ 'struct': 'SevSnpGuestInfo',
> > + 'data': { 'snp-policy': 'uint64' },
> > + 'if': 'TARGET_I386' }
>
> IMHO it can just be called 'policy' still, since
> it is implicitly within a 'Snp' specific type.
>
>
> > +
> > ##
> > # @SevInfo:
> > #
> > @@ -60,25 +103,25 @@
> > #
> > # @build-id: SEV FW build id
> > #
> > -# @policy: SEV policy value
> > -#
> > # @state: SEV guest state
> > #
> > -# @handle: SEV firmware handle
> > +# @sev-type: Type of SEV guest being run
> > #
> > # Since: 2.12
> > ##
> > -{ 'struct': 'SevInfo',
> > -'data': { 'enabled': 'bool',
> > - 'api-major': 'uint8',
> > - 'api-minor' : 'uint8',
> > - 'build-id' : 'uint8',
> > - 'policy' : 'uint32',
> > - 'state' : 'SevState',
> > - 'handle' : 'uint32'
> > -},
> > - 'if': 'TARGET_I386'
> > -}
> > +{ 'union': 'SevInfo',
> > + 'base': { 'enabled': 'bool',
> > +'api-major': 'uint8',
> > +'api-minor' : 'uint8',
> > +'build-id' : 'uint8',
> > +'state' : 'SevState',
> > +'sev-type' : 'SevGuestType' },
> > + 'discriminator': 'sev-type',
> > + 'data': {
> > + 'sev': 'SevGuestInfo',
> > + 'sev-snp': 'SevSnpGuestInfo' },
> > + 'if': 'TARGET_I386' }
> > +
> >
> > ##
> > # @query-sev:
> > diff --git a/target/i386/sev.c b/target/i386/sev.c
> > index 43e6c0172f..b03d70a3d1 100644
> > --- a/target/i386/sev.c
> > +++ b/target/i386/sev.c
> > @@ -353,25 +353,27 @@ static SevInfo *sev_get_info(void)
> > {
> > SevInfo *info;
> > SevCommonState *sev_common =
> > SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
> > -SevGuestState *sev_guest =
> > -(SevGuestState *)object_dynamic_cast(OBJECT(sev_common),
> > - TYPE_SEV_GUEST);
> >
> > info = g_new0(SevInfo, 1);
> > info->enabled = sev_enabled();
> >
> > if (info->enabled) {
> > -if (sev_guest) {
> > -info->handle = sev_guest->handle;
> > -}
> > info->api_major = sev_common->api_major;
> > info->api_minor = sev_common->api_minor;
> > info->build_id = sev_common->build_id;
> > info->state = sev_common->state;
> > -/* we only report the lower 32-bits of policy for SNP, ok for
> > now... */
> > -info->policy =
> > -(uint32_t)object_property_get_uint(OBJECT(sev_common),
> > - "policy", NULL);
> > +
> > +if (sev_snp_enabled()) {
> > +info->sev_type = SEV_GUEST_TYPE_SEV_SNP;
> > +info->u.sev_s
Re: [PATCH v3 31/49] i386/sev: Update query-sev QAPI format to handle SEV-SNP
On Wed, Mar 20, 2024 at 03:39:27AM -0500, Michael Roth wrote:
> Most of the current 'query-sev' command is relevant to both legacy
> SEV/SEV-ES guests and SEV-SNP guests, with 2 exceptions:
>
> - 'policy' is a 64-bit field for SEV-SNP, not 32-bit, and
> the meaning of the bit positions has changed
> - 'handle' is not relevant to SEV-SNP
>
> To address this, this patch adds a new 'sev-type' field that can be
> used as a discriminator to select between SEV and SEV-SNP-specific
> fields/formats without breaking compatibility for existing management
> tools (so long as management tools that add support for launching
> SEV-SNP guest update their handling of query-sev appropriately).
>
> The corresponding HMP command has also been fixed up similarly.
>
> Signed-off-by: Michael Roth
> ---
> qapi/misc-target.json | 71 ++-
> target/i386/sev.c | 50 --
> target/i386/sev.h | 3 ++
> 3 files changed, 94 insertions(+), 30 deletions(-)
>
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index 4e0a6492a9..daceb85d95 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -47,6 +47,49 @@
> 'send-update', 'receive-update' ],
>'if': 'TARGET_I386' }
>
> +##
> +# @SevGuestType:
> +#
> +# An enumeration indicating the type of SEV guest being run.
> +#
> +# @sev: The guest is a legacy SEV or SEV-ES guest.
> +# @sev-snp: The guest is an SEV-SNP guest.
> +#
> +# Since: 6.2
Now 9.1 at the earliest.
> +##
> +{ 'enum': 'SevGuestType',
> + 'data': [ 'sev', 'sev-snp' ],
> + 'if': 'TARGET_I386' }
> +
> +##
> +# @SevGuestInfo:
> +#
> +# Information specific to legacy SEV/SEV-ES guests.
> +#
> +# @policy: SEV policy value
> +#
> +# @handle: SEV firmware handle
> +#
> +# Since: 2.12
> +##
> +{ 'struct': 'SevGuestInfo',
> + 'data': { 'policy': 'uint32',
> +'handle': 'uint32' },
> + 'if': 'TARGET_I386' }
> +
> +##
> +# @SevSnpGuestInfo:
> +#
> +# Information specific to SEV-SNP guests.
> +#
> +# @snp-policy: SEV-SNP policy value
> +#
> +# Since: 6.2
> +##
> +{ 'struct': 'SevSnpGuestInfo',
> + 'data': { 'snp-policy': 'uint64' },
> + 'if': 'TARGET_I386' }
IMHO it can just be called 'policy' still, since
it is implicitly within a 'Snp' specific type.
> +
> ##
> # @SevInfo:
> #
> @@ -60,25 +103,25 @@
> #
> # @build-id: SEV FW build id
> #
> -# @policy: SEV policy value
> -#
> # @state: SEV guest state
> #
> -# @handle: SEV firmware handle
> +# @sev-type: Type of SEV guest being run
> #
> # Since: 2.12
> ##
> -{ 'struct': 'SevInfo',
> -'data': { 'enabled': 'bool',
> - 'api-major': 'uint8',
> - 'api-minor' : 'uint8',
> - 'build-id' : 'uint8',
> - 'policy' : 'uint32',
> - 'state' : 'SevState',
> - 'handle' : 'uint32'
> -},
> - 'if': 'TARGET_I386'
> -}
> +{ 'union': 'SevInfo',
> + 'base': { 'enabled': 'bool',
> +'api-major': 'uint8',
> +'api-minor' : 'uint8',
> +'build-id' : 'uint8',
> +'state' : 'SevState',
> +'sev-type' : 'SevGuestType' },
> + 'discriminator': 'sev-type',
> + 'data': {
> + 'sev': 'SevGuestInfo',
> + 'sev-snp': 'SevSnpGuestInfo' },
> + 'if': 'TARGET_I386' }
> +
>
> ##
> # @query-sev:
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 43e6c0172f..b03d70a3d1 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -353,25 +353,27 @@ static SevInfo *sev_get_info(void)
> {
> SevInfo *info;
> SevCommonState *sev_common =
> SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
> -SevGuestState *sev_guest =
> -(SevGuestState *)object_dynamic_cast(OBJECT(sev_common),
> - TYPE_SEV_GUEST);
>
> info = g_new0(SevInfo, 1);
> info->enabled = sev_enabled();
>
> if (info->enabled) {
> -if (sev_guest) {
> -info->handle = sev_guest->handle;
> -}
> info->api_major = sev_common->api_major;
> info->api_minor = sev_common->api_minor;
> info->build_id = sev_common->build_id;
> info->state = sev_common->state;
> -/* we only report the lower 32-bits of policy for SNP, ok for now...
> */
> -info->policy =
> -(uint32_t)object_property_get_uint(OBJECT(sev_common),
> - "policy", NULL);
> +
> +if (sev_snp_enabled()) {
> +info->sev_type = SEV_GUEST_TYPE_SEV_SNP;
> +info->u.sev_snp.snp_policy =
> +object_property_get_uint(OBJECT(sev_common), "policy", NULL);
> +} else {
> +info->sev_type = SEV_GUEST_TYPE_SEV;
> +info->u.sev.handle = SEV_GUEST(sev_common)->handle;
> +info->u.sev.policy =
> +(uint32_t)object_property_get_uint(OBJECT(sev_common),
> +
[PATCH v3 31/49] i386/sev: Update query-sev QAPI format to handle SEV-SNP
Most of the current 'query-sev' command is relevant to both legacy
SEV/SEV-ES guests and SEV-SNP guests, with 2 exceptions:
- 'policy' is a 64-bit field for SEV-SNP, not 32-bit, and
the meaning of the bit positions has changed
- 'handle' is not relevant to SEV-SNP
To address this, this patch adds a new 'sev-type' field that can be
used as a discriminator to select between SEV and SEV-SNP-specific
fields/formats without breaking compatibility for existing management
tools (so long as management tools that add support for launching
SEV-SNP guest update their handling of query-sev appropriately).
The corresponding HMP command has also been fixed up similarly.
Signed-off-by: Michael Roth
---
qapi/misc-target.json | 71 ++-
target/i386/sev.c | 50 --
target/i386/sev.h | 3 ++
3 files changed, 94 insertions(+), 30 deletions(-)
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 4e0a6492a9..daceb85d95 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -47,6 +47,49 @@
'send-update', 'receive-update' ],
'if': 'TARGET_I386' }
+##
+# @SevGuestType:
+#
+# An enumeration indicating the type of SEV guest being run.
+#
+# @sev: The guest is a legacy SEV or SEV-ES guest.
+# @sev-snp: The guest is an SEV-SNP guest.
+#
+# Since: 6.2
+##
+{ 'enum': 'SevGuestType',
+ 'data': [ 'sev', 'sev-snp' ],
+ 'if': 'TARGET_I386' }
+
+##
+# @SevGuestInfo:
+#
+# Information specific to legacy SEV/SEV-ES guests.
+#
+# @policy: SEV policy value
+#
+# @handle: SEV firmware handle
+#
+# Since: 2.12
+##
+{ 'struct': 'SevGuestInfo',
+ 'data': { 'policy': 'uint32',
+'handle': 'uint32' },
+ 'if': 'TARGET_I386' }
+
+##
+# @SevSnpGuestInfo:
+#
+# Information specific to SEV-SNP guests.
+#
+# @snp-policy: SEV-SNP policy value
+#
+# Since: 6.2
+##
+{ 'struct': 'SevSnpGuestInfo',
+ 'data': { 'snp-policy': 'uint64' },
+ 'if': 'TARGET_I386' }
+
##
# @SevInfo:
#
@@ -60,25 +103,25 @@
#
# @build-id: SEV FW build id
#
-# @policy: SEV policy value
-#
# @state: SEV guest state
#
-# @handle: SEV firmware handle
+# @sev-type: Type of SEV guest being run
#
# Since: 2.12
##
-{ 'struct': 'SevInfo',
-'data': { 'enabled': 'bool',
- 'api-major': 'uint8',
- 'api-minor' : 'uint8',
- 'build-id' : 'uint8',
- 'policy' : 'uint32',
- 'state' : 'SevState',
- 'handle' : 'uint32'
-},
- 'if': 'TARGET_I386'
-}
+{ 'union': 'SevInfo',
+ 'base': { 'enabled': 'bool',
+'api-major': 'uint8',
+'api-minor' : 'uint8',
+'build-id' : 'uint8',
+'state' : 'SevState',
+'sev-type' : 'SevGuestType' },
+ 'discriminator': 'sev-type',
+ 'data': {
+ 'sev': 'SevGuestInfo',
+ 'sev-snp': 'SevSnpGuestInfo' },
+ 'if': 'TARGET_I386' }
+
##
# @query-sev:
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 43e6c0172f..b03d70a3d1 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -353,25 +353,27 @@ static SevInfo *sev_get_info(void)
{
SevInfo *info;
SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
-SevGuestState *sev_guest =
-(SevGuestState *)object_dynamic_cast(OBJECT(sev_common),
- TYPE_SEV_GUEST);
info = g_new0(SevInfo, 1);
info->enabled = sev_enabled();
if (info->enabled) {
-if (sev_guest) {
-info->handle = sev_guest->handle;
-}
info->api_major = sev_common->api_major;
info->api_minor = sev_common->api_minor;
info->build_id = sev_common->build_id;
info->state = sev_common->state;
-/* we only report the lower 32-bits of policy for SNP, ok for now... */
-info->policy =
-(uint32_t)object_property_get_uint(OBJECT(sev_common),
- "policy", NULL);
+
+if (sev_snp_enabled()) {
+info->sev_type = SEV_GUEST_TYPE_SEV_SNP;
+info->u.sev_snp.snp_policy =
+object_property_get_uint(OBJECT(sev_common), "policy", NULL);
+} else {
+info->sev_type = SEV_GUEST_TYPE_SEV;
+info->u.sev.handle = SEV_GUEST(sev_common)->handle;
+info->u.sev.policy =
+(uint32_t)object_property_get_uint(OBJECT(sev_common),
+ "policy", NULL);
+}
}
return info;
@@ -394,20 +396,36 @@ void hmp_info_sev(Monitor *mon, const QDict *qdict)
{
SevInfo *info = sev_get_info();
-if (info && info->enabled) {
-monitor_printf(mon, "handle: %d\n", info->handle);
+if (!info || !info->enabled) {
+monitor_printf(mon, "SEV is not enabled\n");
+goto out;
+}
+
+if (sev_snp_enabled()) {
monitor_printf(mon, "state: %s\n", SevState_str(info->state));
