| Message ID | 20250522190542.588267-3-pierrick.bouvier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: remove all TARGET_* conditionals from the schema | expand |
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)
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. "Undefined" makes my old C scars hurt. What about "unspecified"? > # > # Returns: @SevInfo > # [...]
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.
Pierrick Bouvier <pierrick.bouvier@linaro.org> writes: > On 5/27/25 4:26 AM, Markus Armbruster wrote: [...] > 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?" Fair question! It's down to the difference between code and documentation text. Humans tend to have trouble following long lines with their eyes (I sure do). Typographic manuals suggest to limit columns to roughly 60 characters for exactly that reason[*]. For code, four levels of indentation plus 60 characters of actual text yields 76. However, code lines can be awkward to break, and going over 80 can be less bad than an awkward line break. Use your judgement. Documentation text, however, tends to be indented much less: 6-10 characters of indentation plus 60 of actual text yields 66-70. When I reflowed the entire QAPI schema documentation to stay within that limit (commit a937b6aa739), not a single line break was awkward. > 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. I hope you understand my reasons better now :) [*] https://en.wikipedia.org/wiki/Column_(typography)#Typographic_style
On 5/27/25 11:01 PM, Markus Armbruster wrote: > Pierrick Bouvier <pierrick.bouvier@linaro.org> writes: > >> On 5/27/25 4:26 AM, Markus Armbruster wrote: > > [...] > >> 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?" > > Fair question! It's down to the difference between code and > documentation text. > > Humans tend to have trouble following long lines with their eyes (I sure > do). Typographic manuals suggest to limit columns to roughly 60 > characters for exactly that reason[*]. > > For code, four levels of indentation plus 60 characters of actual text > yields 76. However, code lines can be awkward to break, and going over > 80 can be less bad than an awkward line break. Use your judgement. > > Documentation text, however, tends to be indented much less: 6-10 > characters of indentation plus 60 of actual text yields 66-70. When I > reflowed the entire QAPI schema documentation to stay within that limit > (commit a937b6aa739), not a single line break was awkward. > >> 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. > > I hope you understand my reasons better now :) > > > [*] https://en.wikipedia.org/wiki/Column_(typography)#Typographic_style > Thanks for giving the insight on this. I think the (arbitrary) 80 columns for code is coming from punch cards era. Overall, whether it's 60, 72 or 80, it looks good for human eye.
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 +# 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 +# +# 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. # # @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)