[PATCH 02/11] qapi/misc-target: Group SEV QAPI definitions

Philippe Mathieu-Daudé posted 11 patches 4 years, 8 months ago
There is a newer version of this series
[PATCH 02/11] qapi/misc-target: Group SEV QAPI definitions
Posted by Philippe Mathieu-Daudé 4 years, 8 months ago
There is already a section with various SEV commands / types,
so move the SEV guest attestation together.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 qapi/misc-target.json | 75 +++++++++++++++++++++----------------------
 1 file changed, 37 insertions(+), 38 deletions(-)

diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 5573dcf8f08..1b81f7017d4 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -219,6 +219,43 @@
   'data': { 'packet-header': 'str', 'secret': 'str', '*gpa': 'uint64' },
   'if': 'defined(TARGET_I386)' }
 
+##
+# @SevAttestationReport:
+#
+# The struct describes attestation report for a Secure Encrypted Virtualization
+# feature.
+#
+# @data:  guest attestation report (base64 encoded)
+#
+#
+# Since: 6.1
+##
+{ 'struct': 'SevAttestationReport',
+  'data': { 'data': 'str'},
+  'if': 'defined(TARGET_I386)' }
+
+##
+# @query-sev-attestation-report:
+#
+# This command is used to get the SEV attestation report, and is supported on AMD
+# X86 platforms only.
+#
+# @mnonce: a random 16 bytes value encoded in base64 (it will be included in report)
+#
+# Returns: SevAttestationReport objects.
+#
+# Since: 6.1
+#
+# Example:
+#
+# -> { "execute" : "query-sev-attestation-report", "arguments": { "mnonce": "aaaaaaa" } }
+# <- { "return" : { "data": "aaaaaaaabbbddddd"} }
+#
+##
+{ 'command': 'query-sev-attestation-report', 'data': { 'mnonce': 'str' },
+  'returns': 'SevAttestationReport',
+  'if': 'defined(TARGET_I386)' }
+
 ##
 # @dump-skeys:
 #
@@ -285,41 +322,3 @@
 ##
 { 'command': 'query-gic-capabilities', 'returns': ['GICCapability'],
   'if': 'defined(TARGET_ARM)' }
-
-
-##
-# @SevAttestationReport:
-#
-# The struct describes attestation report for a Secure Encrypted Virtualization
-# feature.
-#
-# @data:  guest attestation report (base64 encoded)
-#
-#
-# Since: 6.1
-##
-{ 'struct': 'SevAttestationReport',
-  'data': { 'data': 'str'},
-  'if': 'defined(TARGET_I386)' }
-
-##
-# @query-sev-attestation-report:
-#
-# This command is used to get the SEV attestation report, and is supported on AMD
-# X86 platforms only.
-#
-# @mnonce: a random 16 bytes value encoded in base64 (it will be included in report)
-#
-# Returns: SevAttestationReport objects.
-#
-# Since: 6.1
-#
-# Example:
-#
-# -> { "execute" : "query-sev-attestation-report", "arguments": { "mnonce": "aaaaaaa" } }
-# <- { "return" : { "data": "aaaaaaaabbbddddd"} }
-#
-##
-{ 'command': 'query-sev-attestation-report', 'data': { 'mnonce': 'str' },
-  'returns': 'SevAttestationReport',
-  'if': 'defined(TARGET_I386)' }
-- 
2.31.1

Re: [PATCH 02/11] qapi/misc-target: Group SEV QAPI definitions
Posted by Markus Armbruster 4 years, 8 months ago
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> There is already a section with various SEV commands / types,
> so move the SEV guest attestation together.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  qapi/misc-target.json | 75 +++++++++++++++++++++----------------------
>  1 file changed, 37 insertions(+), 38 deletions(-)
>
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index 5573dcf8f08..1b81f7017d4 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -219,6 +219,43 @@
>    'data': { 'packet-header': 'str', 'secret': 'str', '*gpa': 'uint64' },
>    'if': 'defined(TARGET_I386)' }
>  
> +##
> +# @SevAttestationReport:
> +#
> +# The struct describes attestation report for a Secure Encrypted Virtualization
> +# feature.
> +#
> +# @data:  guest attestation report (base64 encoded)
> +#
> +#
> +# Since: 6.1
> +##
> +{ 'struct': 'SevAttestationReport',
> +  'data': { 'data': 'str'},
> +  'if': 'defined(TARGET_I386)' }
> +
> +##
> +# @query-sev-attestation-report:
> +#
> +# This command is used to get the SEV attestation report, and is supported on AMD
> +# X86 platforms only.
> +#
> +# @mnonce: a random 16 bytes value encoded in base64 (it will be included in report)
> +#
> +# Returns: SevAttestationReport objects.
> +#
> +# Since: 6.1
> +#
> +# Example:
> +#
> +# -> { "execute" : "query-sev-attestation-report", "arguments": { "mnonce": "aaaaaaa" } }
> +# <- { "return" : { "data": "aaaaaaaabbbddddd"} }
> +#
> +##
> +{ 'command': 'query-sev-attestation-report', 'data': { 'mnonce': 'str' },
> +  'returns': 'SevAttestationReport',
> +  'if': 'defined(TARGET_I386)' }
> +
>  ##
>  # @dump-skeys:
>  #

Just code motion, so

Acked-by: Markus Armbruster <armbru@redhat.com>

Opportunity to wrap the long doc comment lines.  Should be kept under 70
or so.

[...]


Re: [PATCH 02/11] qapi/misc-target: Group SEV QAPI definitions
Posted by Philippe Mathieu-Daudé 4 years, 8 months ago
On 6/10/21 11:39 AM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> There is already a section with various SEV commands / types,
>> so move the SEV guest attestation together.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  qapi/misc-target.json | 75 +++++++++++++++++++++----------------------
>>  1 file changed, 37 insertions(+), 38 deletions(-)
>>
>> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
>> index 5573dcf8f08..1b81f7017d4 100644
>> --- a/qapi/misc-target.json
>> +++ b/qapi/misc-target.json
>> @@ -219,6 +219,43 @@
>>    'data': { 'packet-header': 'str', 'secret': 'str', '*gpa': 'uint64' },
>>    'if': 'defined(TARGET_I386)' }
>>  
>> +##
>> +# @SevAttestationReport:
>> +#
>> +# The struct describes attestation report for a Secure Encrypted Virtualization
>> +# feature.
>> +#
>> +# @data:  guest attestation report (base64 encoded)
>> +#
>> +#
>> +# Since: 6.1
>> +##
>> +{ 'struct': 'SevAttestationReport',
>> +  'data': { 'data': 'str'},
>> +  'if': 'defined(TARGET_I386)' }
>> +
>> +##
>> +# @query-sev-attestation-report:
>> +#
>> +# This command is used to get the SEV attestation report, and is supported on AMD
>> +# X86 platforms only.
>> +#
>> +# @mnonce: a random 16 bytes value encoded in base64 (it will be included in report)
>> +#
>> +# Returns: SevAttestationReport objects.
>> +#
>> +# Since: 6.1
>> +#
>> +# Example:
>> +#
>> +# -> { "execute" : "query-sev-attestation-report", "arguments": { "mnonce": "aaaaaaa" } }
>> +# <- { "return" : { "data": "aaaaaaaabbbddddd"} }
>> +#
>> +##
>> +{ 'command': 'query-sev-attestation-report', 'data': { 'mnonce': 'str' },
>> +  'returns': 'SevAttestationReport',
>> +  'if': 'defined(TARGET_I386)' }
>> +
>>  ##
>>  # @dump-skeys:
>>  #
> 
> Just code motion, so
> 
> Acked-by: Markus Armbruster <armbru@redhat.com>
> 
> Opportunity to wrap the long doc comment lines.  Should be kept under 70
> or so.

Hmm is that a QAPI specific requirement? It is not enforced by
checkpatch, and still in the 80-90 grey area:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg763806.html

(I can do if respin required, but I'd rather have this catch earlier,
not at code movement).

Thanks for the A-b!


Re: [PATCH 02/11] qapi/misc-target: Group SEV QAPI definitions
Posted by Markus Armbruster 4 years, 8 months ago
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 6/10/21 11:39 AM, Markus Armbruster wrote:
>> Just code motion, so
>> 
>> Acked-by: Markus Armbruster <armbru@redhat.com>
>> 
>> Opportunity to wrap the long doc comment lines.  Should be kept under 70
>> or so.
>
> Hmm is that a QAPI specific requirement? It is not enforced by
> checkpatch, and still in the 80-90 grey area:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg763806.html

Consider why we limit line length at all: it's for *legibility*.  Humans
tend to have trouble following long lines with our eyes (I sure do).
Typographic manuals suggest to limit columns to roughly 60 characters
for exactly that reason[1].  Four levels of indentation plus 60
characters of actual text yields 76.

We add a grey area to provide for the occasional case where deeper
indentation pushes code of reasonable width beyond the "white" area, and
breaking these lines would be less legible than making use of the grey
area.

The lines I referred to are long for no good reason.  Wrapping them will
improve legibility.

> (I can do if respin required, but I'd rather have this catch earlier,
> not at code movement).

Before, during, after, or even not at all *clank*[2], your choice.

> Thanks for the A-b!


[1] https://en.wikipedia.org/wiki/Column_(typography)#Typographic_style

[2] Sad sound of a can being kicked down the road