Re: [PATCH v4 02/15] qapi: expand docs for SEV commands

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 5/27/25 4:26 AM, Markus Armbruster wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 
>> From: Daniel P. Berrangé <berrange@redhat.com>
>>
>> This gives some more context about the behaviour of the commands in
>> unsupported guest configuration or platform scenarios.
>>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>   qapi/misc-target.json | 43 ++++++++++++++++++++++++++++++++++++-------
>>   1 file changed, 36 insertions(+), 7 deletions(-)
>>
>> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
>> index c5f9f6be7e1..6b857efc1cc 100644
>> --- a/qapi/misc-target.json
>> +++ b/qapi/misc-target.json
>> @@ -110,7 +110,11 @@
>>   ##
>>   # @query-sev:
>>   #
>> -# Returns information about SEV
>> +# Returns information about SEV/SEV-ES/SEV-SNP.
>> +#
>> +# If unavailable due to an incompatible configuration the
>> +# returned @enabled field will be set to 'false' and the
>> +# state of all other fields is undefined.
>>   #
>>   # Returns: @SevInfo
>>   #
>> @@ -141,7 +145,16 @@
>>   ##
>>   # @query-sev-launch-measure:
>>   #
>> -# Query the SEV guest launch information.
>> +# Query the SEV/SEV-ES guest launch information.
>> +#
>> +# This is only valid on x86 machines configured with KVM and the
>> +# 'sev-guest' confidential virtualization object. The launch
> 
> Humor me, please: separate sentences with two spaces for consistency.
> 
>> +# measurement for SEV-SNP guests is only available within
>> +# the guest.
>> +#
>> +# This will return an error if the launch measurement is
>> +# unavailable, either due to an invalid guest configuration
>> +# or if the guest has not reached the required SEV state.
>>   #
>>   # Returns: The @SevLaunchMeasureInfo for the guest
>>   #
>> @@ -185,8 +198,9 @@
>>   ##
>>   # @query-sev-capabilities:
>>   #
>> -# This command is used to get the SEV capabilities, and is supported
>> -# on AMD X86 platforms only.
>> +# This command is used to get the SEV capabilities, and is only
>> +# supported on AMD X86 platforms with KVM enabled. If SEV is not
>> +# available on the platform an error will be returned.
>>   #
>>   # Returns: SevCapability objects.
>>   #
>> @@ -205,7 +219,15 @@
>>   ##
>>   # @sev-inject-launch-secret:
>>   #
>> -# This command injects a secret blob into memory of SEV guest.
>> +# This command injects a secret blob into memory of a SEV/SEV-ES guest.
>> +#
>> +# This is only valid on x86 machines configured with KVM and the
>> +# 'sev-guest' confidential virtualization object. SEV-SNP guests
>> +# do not support launch secret injection
> 
> Missing period at the end of sentence.
> 
>> +#
>> +# This will return an error if launch secret injection is not possible,
>> +# either due to an invalid guest configuration, or if the guest has not
>> +# reached the required SEV state.
> 
> Slightly long lines.  docs/devel/qapi-code-gen.rst:
> 
>      For legibility, wrap text paragraphs so every line is at most 70
>      characters long.
> 
>>   #
>>   # @packet-header: the launch secret packet header encoded in base64
>>   #
>> @@ -236,8 +258,15 @@
>>   ##
>>   # @query-sev-attestation-report:
>>   #
>> -# This command is used to get the SEV attestation report, and is
>> -# supported on AMD X86 platforms only.
>> +# This command is used to get the SEV attestation report.
>> +#
>> +# This is only valid on x86 machines configured with KVM and the
>> +# 'sev-guest' confidential virtualization object. The attestation
>> +# report for SEV-SNP guests is only available within the guest.
>> +#
>> +# This will return an error if the attestation report is
>> +# unavailable, either due to an invalid guest configuration
>> +# or if the guest has not reached the required SEV state.
>>   #
>>   # @mnonce: a random 16 bytes value encoded in base64 (it will be
>>   #     included in report)
> 

All good for me.
The only question that crossed my mind when you asked for those changes 
previously was: "Why does QAPI has it's own style, and not simply 
following the QEMU official style?"

In the end, you choose which rules apply to this subsystem, and I have 
no strong opinion on whether it should be 70, 72 or 80 characters on the 
line, or if we prefer tabs to spaces (to make some analogy). I just 
think it's surprising to have a different coding style only here for 
arbitrary reasons.