[libvirt] [PATCH v5 02/10] qemu: introduce SEV feature in hypervisor capabilities

Brijesh Singh posted 10 patches 7 years, 1 month ago
There is a newer version of this series
[libvirt] [PATCH v5 02/10] qemu: introduce SEV feature in hypervisor capabilities
Posted by Brijesh Singh 7 years, 1 month ago
Extend hypervisor capabilities to include sev feature. When available,
hypervisor supports launching an encrypted VM on AMD platform. The
sev feature tag provides additional details like platform diffie-hellman
key and certificate chain which can be used by the guest owner to
establish a cryptographic session with the SEV firmware to negotiate
keys used for attestation or to provide secret during launch.

Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 docs/formatdomaincaps.html.in  | 40 ++++++++++++++++++++++++++++++++++++++++
 docs/schemas/domaincaps.rng    | 20 ++++++++++++++++++++
 src/conf/domain_capabilities.c | 20 ++++++++++++++++++++
 src/conf/domain_capabilities.h |  1 +
 src/qemu/qemu_capabilities.c   |  2 ++
 5 files changed, 83 insertions(+)

diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index 6bfcaf6..f383141 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -417,6 +417,12 @@
         &lt;value&gt;3&lt;/value&gt;
       &lt;/enum&gt;
     &lt;/gic&gt;
+    &lt;sev&gt;
+      &lt;pdh&gt; &lt;/pdh&gt;
+      &lt;cert-chain&gt; &lt;/cert-chain&gt;
+      &lt;cbitpos&gt; &lt;/cbitpos&gt;
+      &lt;reduced-phys-bits&gt; &lt;/reduced-phys-bits&gt;
+    &lt;/sev&gt;
   &lt;/features&gt;
 &lt;/domainCapabilities&gt;
 </pre>
@@ -441,5 +447,39 @@
       <code>gic</code> element.</dd>
     </dl>
 
+    <h4><a id="elementsSEV">SEV capabilities</a></h4>
+
+    <p>AMD Secure Encrypted Virtualization (SEV) capabilities are exposed under
+    the <code>sev</code> element.
+    SEV is an extension to the AMD-V architecture which supports running
+    virtual machines (VMs) under the control of a hypervisor. When supported,
+    guest owner can create a VM whose memory contents will be transparently
+    encrypted with a key unique to that VM.
+
+    For more details on SEV feature see:
+      <a href="https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf">
+        SEV API spec</a> and <a href="http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf">
+        SEV White Paper</a>
+
+    </p>
+
+    <dl>
+      <dt><code>pdh</code></dt>
+      <dd>Platform diffie-hellman key, which can be exported to remote entities
+      which wish to establish a secure transport context with the SEV platform
+      in order to transmit data securely. The key is encoded in base64</dd>
+      <dt><code>cert-chain</code></dt>
+      <dd> Platform certificate chain -- which includes platform endorsement key
+      (PEK), owners certificate authory (OCA) and chip endorsement key (CEK).
+      The certificate chain is encoded in base64.</dd>
+      <dt><code>cbitpos</code></dt>
+      <dd>When memory encryption is enabled, one of the physical address bit
+      (aka the C-bit) is utilized to mark if a memory page is protected. The
+      C-bit position is Hypervisor dependent.</dd>
+      <dt><code>reduced-phys-bits</code></dt>
+      <dd>When memory encryption is enabled, we loose certain bits in physical
+      address space. The number of bits we loose is hypervisor dependent.</dd>
+    </dl>
+
   </body>
 </html>
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index 3905318..53b33eb 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -173,6 +173,9 @@
     <element name='features'>
       <interleave>
         <ref name='gic'/>
+        <optional>
+        <ref name='sev'/>
+        </optional>
       </interleave>
     </element>
   </define>
@@ -184,6 +187,23 @@
     </element>
   </define>
 
+  <define name='sev'>
+    <element name='sev'>
+      <element name='pdh'>
+        <data type='string'/>
+      </element>
+      <element name='cert-chain'>
+        <data type='string'/>
+      </element>
+      <element name='cbitpos'>
+        <data type='unsignedInt'/>
+      </element>
+      <element name='reduced-phys-bits'>
+        <data type='unsignedInt'/>
+      </element>
+    </element>
+  </define>
+
   <define name='value'>
     <zeroOrMore>
       <element name='value'>
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index f7d9be5..082065f 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -549,6 +549,25 @@ virDomainCapsFeatureGICFormat(virBufferPtr buf,
     FORMAT_EPILOGUE(gic);
 }
 
+static void
+virDomainCapsFeatureSEVFormat(virBufferPtr buf,
+                              virSEVCapabilityPtr const sev)
+{
+    if (!sev)
+        return;
+
+    virBufferAddLit(buf, "<sev supported='yes'>\n");
+    virBufferAdjustIndent(buf, 2);
+    virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos);
+    virBufferAsprintf(buf, "<reduced-phys-bits>%d</reduced-phys-bits>\n",
+                      sev->reduced_phys_bits);
+    virBufferAsprintf(buf, "<pdh>%s</pdh>\n", sev->pdh);
+    virBufferAsprintf(buf, "<cert-chain>%s</cert-chain>\n",
+                      sev->cert_chain);
+    virBufferAdjustIndent(buf, -2);
+    virBufferAddLit(buf, "</sev>\n");
+}
+
 
 char *
 virDomainCapsFormat(virDomainCapsPtr const caps)
@@ -587,6 +606,7 @@ virDomainCapsFormat(virDomainCapsPtr const caps)
     virBufferAdjustIndent(&buf, 2);
 
     virDomainCapsFeatureGICFormat(&buf, &caps->gic);
+    virDomainCapsFeatureSEVFormat(&buf, caps->sev);
 
     virBufferAdjustIndent(&buf, -2);
     virBufferAddLit(&buf, "</features>\n");
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 72e9daf..2e8596c 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -170,6 +170,7 @@ struct _virDomainCaps {
     /* add new domain devices here */
 
     virDomainCapsFeatureGIC gic;
+    virSEVCapabilityPtr sev;
     /* add new domain features here */
 };
 
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 0f6e6fb..3fd4911 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5787,6 +5787,8 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps,
         virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev) < 0 ||
         virQEMUCapsFillDomainFeatureGICCaps(qemuCaps, domCaps) < 0)
         return -1;
+
+    domCaps->sev = qemuCaps->sevCapabilities;
     return 0;
 }
 
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 02/10] qemu: introduce SEV feature in hypervisor capabilities
Posted by John Ferlan 7 years, 1 month ago

On 04/02/2018 10:18 AM, Brijesh Singh wrote:
> Extend hypervisor capabilities to include sev feature. When available,
> hypervisor supports launching an encrypted VM on AMD platform. The
> sev feature tag provides additional details like platform diffie-hellman

Diffie-Hellman

right?

> key and certificate chain which can be used by the guest owner to
> establish a cryptographic session with the SEV firmware to negotiate
> keys used for attestation or to provide secret during launch.
> 
> Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  docs/formatdomaincaps.html.in  | 40 ++++++++++++++++++++++++++++++++++++++++
>  docs/schemas/domaincaps.rng    | 20 ++++++++++++++++++++
>  src/conf/domain_capabilities.c | 20 ++++++++++++++++++++
>  src/conf/domain_capabilities.h |  1 +
>  src/qemu/qemu_capabilities.c   |  2 ++
>  5 files changed, 83 insertions(+)
> 

I see this has Daniel's R-by, but I have a few notes and questions...

> diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
> index 6bfcaf6..f383141 100644
> --- a/docs/formatdomaincaps.html.in
> +++ b/docs/formatdomaincaps.html.in
> @@ -417,6 +417,12 @@
>          &lt;value&gt;3&lt;/value&gt;
>        &lt;/enum&gt;
>      &lt;/gic&gt;
> +    &lt;sev&gt;
> +      &lt;pdh&gt; &lt;/pdh&gt;
> +      &lt;cert-chain&gt; &lt;/cert-chain&gt;
> +      &lt;cbitpos&gt; &lt;/cbitpos&gt;
> +      &lt;reduced-phys-bits&gt; &lt;/reduced-phys-bits&gt;
> +    &lt;/sev&gt;

The example output should have some sort of example output and not an
empty space.

>    &lt;/features&gt;
>  &lt;/domainCapabilities&gt;
>  </pre>
> @@ -441,5 +447,39 @@
>        <code>gic</code> element.</dd>
>      </dl>
>  
> +    <h4><a id="elementsSEV">SEV capabilities</a></h4>
> +
> +    <p>AMD Secure Encrypted Virtualization (SEV) capabilities are exposed under
> +    the <code>sev</code> element.
> +    SEV is an extension to the AMD-V architecture which supports running
> +    virtual machines (VMs) under the control of a hypervisor. When supported,
> +    guest owner can create a VM whose memory contents will be transparently
> +    encrypted with a key unique to that VM.

I think it would be cleaner to add a </p> to after VM and then a new <p>
on the next line

> +
> +    For more details on SEV feature see:
> +      <a href="https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf">
> +        SEV API spec</a> and <a href="http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf">
> +        SEV White Paper</a>
> +
> +    </p>
> +
> +    <dl>
> +      <dt><code>pdh</code></dt>
> +      <dd>Platform diffie-hellman key, which can be exported to remote entities

Again, I think this should be Diffie-Hellman ?

And it's the public key right - so "A base64 encoded platform
Diffie-Hellman public key which..."

> +      which wish to establish a secure transport context with the SEV platform

"which wish" reads strange - how about "that desire"

> +      in order to transmit data securely. The key is encoded in base64</dd>

Add "A base64 encoded" up front makes the last sentence duplicitous.

> +      <dt><code>cert-chain</code></dt>
> +      <dd> Platform certificate chain -- which includes platform endorsement key
> +      (PEK), owners certificate authory (OCA) and chip endorsement key (CEK).
> +      The certificate chain is encoded in base64.</dd>

A base64 encoded platform certificate chain that includes the platform
endorsement key (PEK), owners certificate authority (OCD), and chip
endorsement key (CEK).

> +      <dt><code>cbitpos</code></dt>
> +      <dd>When memory encryption is enabled, one of the physical address bit

s/bit/bits

> +      (aka the C-bit) is utilized to mark if a memory page is protected. The
> +      C-bit position is Hypervisor dependent.</dd>
> +      <dt><code>reduced-phys-bits</code></dt>
> +      <dd>When memory encryption is enabled, we loose certain bits in physical
> +      address space. The number of bits we loose is hypervisor dependent.</dd>
> +    </dl>
> +

s/loose/lose

>    </body>
>  </html>
> diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
> index 3905318..53b33eb 100644
> --- a/docs/schemas/domaincaps.rng
> +++ b/docs/schemas/domaincaps.rng
> @@ -173,6 +173,9 @@
>      <element name='features'>
>        <interleave>
>          <ref name='gic'/>
> +        <optional>
> +        <ref name='sev'/>
> +        </optional>
>        </interleave>
>      </element>
>    </define>
> @@ -184,6 +187,23 @@
>      </element>
>    </define>
>  
> +  <define name='sev'>
> +    <element name='sev'>
> +      <element name='pdh'>
> +        <data type='string'/>
> +      </element>
> +      <element name='cert-chain'>
> +        <data type='string'/>
> +      </element>
> +      <element name='cbitpos'>
> +        <data type='unsignedInt'/>
> +      </element>
> +      <element name='reduced-phys-bits'>
> +        <data type='unsignedInt'/>
> +      </element>
> +    </element>
> +  </define>
> +

I want to make sure of recent preferences which aren't documented, but
sometimes draw attention of certain reviewers...

Do we want dashes or camelCase for new names?  e.g. "cert-chain" or
"certChain"? And likewise for reduced-phys-bits or reducedPhysBits?


>    <define name='value'>
>      <zeroOrMore>
>        <element name='value'>
> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
> index f7d9be5..082065f 100644
> --- a/src/conf/domain_capabilities.c
> +++ b/src/conf/domain_capabilities.c
> @@ -549,6 +549,25 @@ virDomainCapsFeatureGICFormat(virBufferPtr buf,
>      FORMAT_EPILOGUE(gic);
>  }
>  
> +static void
> +virDomainCapsFeatureSEVFormat(virBufferPtr buf,
> +                              virSEVCapabilityPtr const sev)
> +{
> +    if (!sev)
> +        return;
> +
> +    virBufferAddLit(buf, "<sev supported='yes'>\n");
> +    virBufferAdjustIndent(buf, 2);
> +    virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos);
> +    virBufferAsprintf(buf, "<reduced-phys-bits>%d</reduced-phys-bits>\n",
> +                      sev->reduced_phys_bits);
> +    virBufferAsprintf(buf, "<pdh>%s</pdh>\n", sev->pdh);
> +    virBufferAsprintf(buf, "<cert-chain>%s</cert-chain>\n",
> +                      sev->cert_chain);

When formatting strings, use virBufferEscapeString

Yes, I know they're base64 encoded, but not taking any chances...

Depending on whether there's an opinion w/r/t camelCase or not, I can
either fix up the nits or wait for a v6 with adjustments to the user
facing values.

John

> +    virBufferAdjustIndent(buf, -2);
> +    virBufferAddLit(buf, "</sev>\n");
> +}
> +
>  
>  char *
>  virDomainCapsFormat(virDomainCapsPtr const caps)
> @@ -587,6 +606,7 @@ virDomainCapsFormat(virDomainCapsPtr const caps)
>      virBufferAdjustIndent(&buf, 2);
>  
>      virDomainCapsFeatureGICFormat(&buf, &caps->gic);
> +    virDomainCapsFeatureSEVFormat(&buf, caps->sev);
>  
>      virBufferAdjustIndent(&buf, -2);
>      virBufferAddLit(&buf, "</features>\n");
> diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
> index 72e9daf..2e8596c 100644
> --- a/src/conf/domain_capabilities.h
> +++ b/src/conf/domain_capabilities.h
> @@ -170,6 +170,7 @@ struct _virDomainCaps {
>      /* add new domain devices here */
>  
>      virDomainCapsFeatureGIC gic;
> +    virSEVCapabilityPtr sev;
>      /* add new domain features here */
>  };
>  
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 0f6e6fb..3fd4911 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -5787,6 +5787,8 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps,
>          virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev) < 0 ||
>          virQEMUCapsFillDomainFeatureGICCaps(qemuCaps, domCaps) < 0)
>          return -1;
> +
> +    domCaps->sev = qemuCaps->sevCapabilities;
>      return 0;
>  }
>  
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 02/10] qemu: introduce SEV feature in hypervisor capabilities
Posted by Brijesh Singh 7 years, 1 month ago

On 04/02/2018 12:33 PM, John Ferlan wrote:
> 
> 
> On 04/02/2018 10:18 AM, Brijesh Singh wrote:
>> Extend hypervisor capabilities to include sev feature. When available,
>> hypervisor supports launching an encrypted VM on AMD platform. The
>> sev feature tag provides additional details like platform diffie-hellman
> 
> Diffie-Hellman
> 
> right?
> 

Yes, I will use camel case in next patch.


>> key and certificate chain which can be used by the guest owner to
>> establish a cryptographic session with the SEV firmware to negotiate
>> keys used for attestation or to provide secret during launch.
>>
>> Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>   docs/formatdomaincaps.html.in  | 40 ++++++++++++++++++++++++++++++++++++++++
>>   docs/schemas/domaincaps.rng    | 20 ++++++++++++++++++++
>>   src/conf/domain_capabilities.c | 20 ++++++++++++++++++++
>>   src/conf/domain_capabilities.h |  1 +
>>   src/qemu/qemu_capabilities.c   |  2 ++
>>   5 files changed, 83 insertions(+)
>>
> 
> I see this has Daniel's R-by, but I have a few notes and questions...
> 
>> diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
>> index 6bfcaf6..f383141 100644
>> --- a/docs/formatdomaincaps.html.in
>> +++ b/docs/formatdomaincaps.html.in
>> @@ -417,6 +417,12 @@
>>           &lt;value&gt;3&lt;/value&gt;
>>         &lt;/enum&gt;
>>       &lt;/gic&gt;
>> +    &lt;sev&gt;
>> +      &lt;pdh&gt; &lt;/pdh&gt;
>> +      &lt;cert-chain&gt; &lt;/cert-chain&gt;
>> +      &lt;cbitpos&gt; &lt;/cbitpos&gt;
>> +      &lt;reduced-phys-bits&gt; &lt;/reduced-phys-bits&gt;
>> +    &lt;/sev&gt;
> 
> The example output should have some sort of example output and not an
> empty space.
> 

Noted, I will fill in some random values.


>>     &lt;/features&gt;
>>   &lt;/domainCapabilities&gt;
>>   </pre>
>> @@ -441,5 +447,39 @@
>>         <code>gic</code> element.</dd>
>>       </dl>
>>   
>> +    <h4><a id="elementsSEV">SEV capabilities</a></h4>
>> +
>> +    <p>AMD Secure Encrypted Virtualization (SEV) capabilities are exposed under
>> +    the <code>sev</code> element.
>> +    SEV is an extension to the AMD-V architecture which supports running
>> +    virtual machines (VMs) under the control of a hypervisor. When supported,
>> +    guest owner can create a VM whose memory contents will be transparently
>> +    encrypted with a key unique to that VM.
> 
> I think it would be cleaner to add a </p> to after VM and then a new <p>
> on the next line
> 

Noted.


>> +
>> +    For more details on SEV feature see:
>> +      <a href="https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf">
>> +        SEV API spec</a> and <a href="http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf">
>> +        SEV White Paper</a>
>> +
>> +    </p>
>> +
>> +    <dl>
>> +      <dt><code>pdh</code></dt>
>> +      <dd>Platform diffie-hellman key, which can be exported to remote entities
> 
> Again, I think this should be Diffie-Hellman ?
> 
> And it's the public key right - so "A base64 encoded platform
> Diffie-Hellman public key which..."


Noted.


> 
>> +      which wish to establish a secure transport context with the SEV platform
> 
> "which wish" reads strange - how about "that desire"
> 
>> +      in order to transmit data securely. The key is encoded in base64</dd>
> 
> Add "A base64 encoded" up front makes the last sentence duplicitous.
> 
>> +      <dt><code>cert-chain</code></dt>
>> +      <dd> Platform certificate chain -- which includes platform endorsement key
>> +      (PEK), owners certificate authory (OCA) and chip endorsement key (CEK).
>> +      The certificate chain is encoded in base64.</dd>
> 
> A base64 encoded platform certificate chain that includes the platform
> endorsement key (PEK), owners certificate authority (OCD), and chip
> endorsement key (CEK).
> 


Noted, makes it much cleaner. thanks


>> +      <dt><code>cbitpos</code></dt>
>> +      <dd>When memory encryption is enabled, one of the physical address bit
> 
> s/bit/bits
> 

Noted.

>> +      (aka the C-bit) is utilized to mark if a memory page is protected. The
>> +      C-bit position is Hypervisor dependent.</dd>
>> +      <dt><code>reduced-phys-bits</code></dt>
>> +      <dd>When memory encryption is enabled, we loose certain bits in physical
>> +      address space. The number of bits we loose is hypervisor dependent.</dd>
>> +    </dl>
>> +
> 
> s/loose/lose
> 

Noted

>>     </body>
>>   </html>
>> diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
>> index 3905318..53b33eb 100644
>> --- a/docs/schemas/domaincaps.rng
>> +++ b/docs/schemas/domaincaps.rng
>> @@ -173,6 +173,9 @@
>>       <element name='features'>
>>         <interleave>
>>           <ref name='gic'/>
>> +        <optional>
>> +        <ref name='sev'/>
>> +        </optional>
>>         </interleave>
>>       </element>
>>     </define>
>> @@ -184,6 +187,23 @@
>>       </element>
>>     </define>
>>   
>> +  <define name='sev'>
>> +    <element name='sev'>
>> +      <element name='pdh'>
>> +        <data type='string'/>
>> +      </element>
>> +      <element name='cert-chain'>
>> +        <data type='string'/>
>> +      </element>
>> +      <element name='cbitpos'>
>> +        <data type='unsignedInt'/>
>> +      </element>
>> +      <element name='reduced-phys-bits'>
>> +        <data type='unsignedInt'/>
>> +      </element>
>> +    </element>
>> +  </define>
>> +
> 
> I want to make sure of recent preferences which aren't documented, but
> sometimes draw attention of certain reviewers...
> 
> Do we want dashes or camelCase for new names?  e.g. "cert-chain" or
> "certChain"? And likewise for reduced-phys-bits or reducedPhysBits?
> 

I have been trying keep the same naming convension as sev-guest object 
name (which uses dashes). I am flexible, if libvirt community prefers 
camelCase over the dashes then I am okay with it.


> 
>>     <define name='value'>
>>       <zeroOrMore>
>>         <element name='value'>
>> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
>> index f7d9be5..082065f 100644
>> --- a/src/conf/domain_capabilities.c
>> +++ b/src/conf/domain_capabilities.c
>> @@ -549,6 +549,25 @@ virDomainCapsFeatureGICFormat(virBufferPtr buf,
>>       FORMAT_EPILOGUE(gic);
>>   }
>>   
>> +static void
>> +virDomainCapsFeatureSEVFormat(virBufferPtr buf,
>> +                              virSEVCapabilityPtr const sev)
>> +{
>> +    if (!sev)
>> +        return;
>> +
>> +    virBufferAddLit(buf, "<sev supported='yes'>\n");
>> +    virBufferAdjustIndent(buf, 2);
>> +    virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos);
>> +    virBufferAsprintf(buf, "<reduced-phys-bits>%d</reduced-phys-bits>\n",
>> +                      sev->reduced_phys_bits);
>> +    virBufferAsprintf(buf, "<pdh>%s</pdh>\n", sev->pdh);
>> +    virBufferAsprintf(buf, "<cert-chain>%s</cert-chain>\n",
>> +                      sev->cert_chain);
> 
> When formatting strings, use virBufferEscapeString
> 
> Yes, I know they're base64 encoded, but not taking any chances...
> 
> Depending on whether there's an opinion w/r/t camelCase or not, I can
> either fix up the nits or wait for a v6 with adjustments to the user
> facing values.
> 

Noted.

-Brijesh

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 02/10] qemu: introduce SEV feature in hypervisor capabilities
Posted by John Ferlan 7 years, 1 month ago

On 04/02/2018 03:20 PM, Brijesh Singh wrote:
> 
> 
> On 04/02/2018 12:33 PM, John Ferlan wrote:
>>
>>
>> On 04/02/2018 10:18 AM, Brijesh Singh wrote:
>>> Extend hypervisor capabilities to include sev feature. When available,
>>> hypervisor supports launching an encrypted VM on AMD platform. The
>>> sev feature tag provides additional details like platform diffie-hellman
>>
>> Diffie-Hellman
>>
>> right?
>>
> 
> Yes, I will use camel case in next patch.
> 

Let's see what Daniel says about camel case before you go off and make a
lot of changes...

I can never quite remember which way the wind blows on this ;-)

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list