[PATCH] docs: Add measurement calculation details to amd-memory-encryption.txt

[PATCH] docs: Add measurement calculation details to amd-memory-encryption.txt

[Dov Murik]

Add a section explaining how the Guest Owner should calculate the
expected guest launch measurement for SEV and SEV-ES.

Also update the name and link to the SEV API Spec document.

Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
---
 docs/amd-memory-encryption.txt | 50 +++++++++++++++++++++++++++++++---
 1 file changed, 46 insertions(+), 4 deletions(-)

diff --git a/docs/amd-memory-encryption.txt b/docs/amd-memory-encryption.txt
index ffca382b5f..f97727482f 100644
--- a/docs/amd-memory-encryption.txt
+++ b/docs/amd-memory-encryption.txt
@@ -43,7 +43,7 @@ The guest policy is passed as plaintext. A hypervisor may choose to read it,
 but should not modify it (any modification of the policy bits will result
 in bad measurement). The guest policy is a 4-byte data structure containing
 several flags that restricts what can be done on a running SEV guest.
-See KM Spec section 3 and 6.2 for more details.
+See SEV API Spec [1] section 3 and 6.2 for more details.
 
 The guest policy can be provided via the 'policy' property (see below)
 
@@ -88,7 +88,7 @@ expects.
 LAUNCH_FINISH finalizes the guest launch and destroys the cryptographic
 context.
 
-See SEV KM API Spec [1] 'Launching a guest' usage flow (Appendix A) for the
+See SEV API Spec [1] 'Launching a guest' usage flow (Appendix A) for the
 complete flow chart.
 
 To launch a SEV guest
@@ -113,6 +113,45 @@ a SEV-ES guest:
  - Requires in-kernel irqchip - the burden is placed on the hypervisor to
    manage booting APs.
 
+Calculating expected guest launch measurement
+---------------------------------------------
+In order to verify the guest launch measurement, The Guest Owner must compute
+it in the exact same way as it is calculated by the AMD-SP.  SEV API Spec [1]
+section 6.5.1 describes the AMD-SP operations:
+
+    GCTX.LD is finalized, producing the hash digest of all plaintext data
+    imported into the guest.
+
+    The launch measurement is calculated as:
+
+    HMAC(0x04 || API_MAJOR || API_MINOR || BUILD || GCTX.POLICY || GCTX.LD || MNONCE; GCTX.TIK)
+
+    where "||" represents concatenation.
+
+The values of API_MAJOR, API_MINOR, BUILD, and GCTX.POLICY can be obtained
+from the 'query-sev' qmp command.
+
+The value of MNONCE is part of the response of 'query-sev-launch-measure': it
+is the last 16 bytes of the base64-decoded data field (see SEV API Spec [1]
+section 6.5.2 Table 52: LAUNCH_MEASURE Measurement Buffer).
+
+The value of GCTX.LD is SHA256(firmware_blob || kernel_hashes_blob || vmsas_blob),
+where:
+
+* firmware_blob is the content of the entire firmware flash file (for example,
+  OVMF.fd).
+* if kernel is used, and kernel-hashes=on, then kernel_hashes_blob is the
+  content of PaddedSevHashTable (including the zero padding), which itself
+  includes the hashes of kernel, initrd, and cmdline that are passed to the
+  guest.  The PaddedSevHashTable struct is defined in target/i386/sev.c .
+* if SEV-ES is enabled (policy & 0x4 != 0), vmsas_blob is the concatenation of
+  all VMSAs of the guest vcpus.  Each VMSA is 4096 bytes long; its content is
+  defined inside Linux kernel code as struct vmcb_save_area, or in AMD APM
+  Volume 2 [2] Table B-2: VMCB Layout, State Save Area.
+
+If kernel hashes are not used, or SEV-ES is disabled, use empty blobs for
+kernel_hashes_blob and vmsas_blob as needed.
+
 Debugging
 -----------
 Since the memory contents of a SEV guest are encrypted, hypervisor access to
@@ -134,8 +173,11 @@ References
 AMD Memory Encryption whitepaper:
 https://developer.amd.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf
 
-Secure Encrypted Virtualization Key Management:
-[1] http://developer.amd.com/wordpress/media/2017/11/55766_SEV-KM-API_Specification.pdf
+Secure Encrypted Virtualization API:
+[1] https://www.amd.com/system/files/TechDocs/55766_SEV-KM_API_Specification.pdf
+
+AMD64 Architecture Programmer's Manual Volume 2: System Programming
+[2] https://www.amd.com/system/files/TechDocs/24593.pdf
 
 KVM Forum slides:
 http://www.linux-kvm.org/images/7/74/02x08A-Thomas_Lendacky-AMDs_Virtualizatoin_Memory_Encryption_Technology.pdf

base-commit: a3607def89f9cd68c1b994e1030527df33aa91d0
-- 
2.25.1

Re: [PATCH] docs: Add measurement calculation details to amd-memory-encryption.txt

[Daniel P. Berrangé]

On Tue, Dec 14, 2021 at 01:59:10PM +0000, Dov Murik wrote:
> Add a section explaining how the Guest Owner should calculate the
> expected guest launch measurement for SEV and SEV-ES.
> 
> Also update the name and link to the SEV API Spec document.
> 
> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  docs/amd-memory-encryption.txt | 50 +++++++++++++++++++++++++++++++---
>  1 file changed, 46 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/amd-memory-encryption.txt b/docs/amd-memory-encryption.txt
> index ffca382b5f..f97727482f 100644
> --- a/docs/amd-memory-encryption.txt
> +++ b/docs/amd-memory-encryption.txt
> @@ -43,7 +43,7 @@ The guest policy is passed as plaintext. A hypervisor may choose to read it,
>  but should not modify it (any modification of the policy bits will result
>  in bad measurement). The guest policy is a 4-byte data structure containing
>  several flags that restricts what can be done on a running SEV guest.
> -See KM Spec section 3 and 6.2 for more details.
> +See SEV API Spec [1] section 3 and 6.2 for more details.
>  
>  The guest policy can be provided via the 'policy' property (see below)
>  
> @@ -88,7 +88,7 @@ expects.
>  LAUNCH_FINISH finalizes the guest launch and destroys the cryptographic
>  context.
>  
> -See SEV KM API Spec [1] 'Launching a guest' usage flow (Appendix A) for the
> +See SEV API Spec [1] 'Launching a guest' usage flow (Appendix A) for the
>  complete flow chart.
>  
>  To launch a SEV guest
> @@ -113,6 +113,45 @@ a SEV-ES guest:
>   - Requires in-kernel irqchip - the burden is placed on the hypervisor to
>     manage booting APs.
>  
> +Calculating expected guest launch measurement
> +---------------------------------------------
> +In order to verify the guest launch measurement, The Guest Owner must compute
> +it in the exact same way as it is calculated by the AMD-SP.  SEV API Spec [1]
> +section 6.5.1 describes the AMD-SP operations:
> +
> +    GCTX.LD is finalized, producing the hash digest of all plaintext data
> +    imported into the guest.
> +
> +    The launch measurement is calculated as:
> +
> +    HMAC(0x04 || API_MAJOR || API_MINOR || BUILD || GCTX.POLICY || GCTX.LD || MNONCE; GCTX.TIK)
> +
> +    where "||" represents concatenation.
> +
> +The values of API_MAJOR, API_MINOR, BUILD, and GCTX.POLICY can be obtained
> +from the 'query-sev' qmp command.
> +
> +The value of MNONCE is part of the response of 'query-sev-launch-measure': it
> +is the last 16 bytes of the base64-decoded data field (see SEV API Spec [1]
> +section 6.5.2 Table 52: LAUNCH_MEASURE Measurement Buffer).
> +
> +The value of GCTX.LD is SHA256(firmware_blob || kernel_hashes_blob || vmsas_blob),
> +where:
> +
> +* firmware_blob is the content of the entire firmware flash file (for example,
> +  OVMF.fd).

Lets add a caveat that the firmware flash should be built to be stateless
ie that it is not secure to attempt to measure a guest where the firmware
uses an NVRAM store.

> +* if kernel is used, and kernel-hashes=on, then kernel_hashes_blob is the
> +  content of PaddedSevHashTable (including the zero padding), which itself
> +  includes the hashes of kernel, initrd, and cmdline that are passed to the
> +  guest.  The PaddedSevHashTable struct is defined in target/i386/sev.c .
> +* if SEV-ES is enabled (policy & 0x4 != 0), vmsas_blob is the concatenation of
> +  all VMSAs of the guest vcpus.  Each VMSA is 4096 bytes long; its content is
> +  defined inside Linux kernel code as struct vmcb_save_area, or in AMD APM
> +  Volume 2 [2] Table B-2: VMCB Layout, State Save Area.

Is there any practical guidance we can give apps on the way the VMSAs
can be expected to be initialized ? eg can they assume essentially
all fields in vmcb_save_area are 0 initialized except for certain
ones ? Is initialization likely to vary at all across KVM or EDK2
vesions or something ?

> +
> +If kernel hashes are not used, or SEV-ES is disabled, use empty blobs for
> +kernel_hashes_blob and vmsas_blob as needed.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] docs: Add measurement calculation details to amd-memory-encryption.txt

[Dov Murik]

On 14/12/2021 20:39, Daniel P. Berrangé wrote:
> On Tue, Dec 14, 2021 at 01:59:10PM +0000, Dov Murik wrote:
>> Add a section explaining how the Guest Owner should calculate the
>> expected guest launch measurement for SEV and SEV-ES.
>>
>> Also update the name and link to the SEV API Spec document.
>>
>> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
>> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>  docs/amd-memory-encryption.txt | 50 +++++++++++++++++++++++++++++++---
>>  1 file changed, 46 insertions(+), 4 deletions(-)
>>
>> diff --git a/docs/amd-memory-encryption.txt b/docs/amd-memory-encryption.txt
>> index ffca382b5f..f97727482f 100644
>> --- a/docs/amd-memory-encryption.txt
>> +++ b/docs/amd-memory-encryption.txt
>> @@ -43,7 +43,7 @@ The guest policy is passed as plaintext. A hypervisor may choose to read it,
>>  but should not modify it (any modification of the policy bits will result
>>  in bad measurement). The guest policy is a 4-byte data structure containing
>>  several flags that restricts what can be done on a running SEV guest.
>> -See KM Spec section 3 and 6.2 for more details.
>> +See SEV API Spec [1] section 3 and 6.2 for more details.
>>  
>>  The guest policy can be provided via the 'policy' property (see below)
>>  
>> @@ -88,7 +88,7 @@ expects.
>>  LAUNCH_FINISH finalizes the guest launch and destroys the cryptographic
>>  context.
>>  
>> -See SEV KM API Spec [1] 'Launching a guest' usage flow (Appendix A) for the
>> +See SEV API Spec [1] 'Launching a guest' usage flow (Appendix A) for the
>>  complete flow chart.
>>  
>>  To launch a SEV guest
>> @@ -113,6 +113,45 @@ a SEV-ES guest:
>>   - Requires in-kernel irqchip - the burden is placed on the hypervisor to
>>     manage booting APs.
>>  
>> +Calculating expected guest launch measurement
>> +---------------------------------------------
>> +In order to verify the guest launch measurement, The Guest Owner must compute
>> +it in the exact same way as it is calculated by the AMD-SP.  SEV API Spec [1]
>> +section 6.5.1 describes the AMD-SP operations:
>> +
>> +    GCTX.LD is finalized, producing the hash digest of all plaintext data
>> +    imported into the guest.
>> +
>> +    The launch measurement is calculated as:
>> +
>> +    HMAC(0x04 || API_MAJOR || API_MINOR || BUILD || GCTX.POLICY || GCTX.LD || MNONCE; GCTX.TIK)
>> +
>> +    where "||" represents concatenation.
>> +
>> +The values of API_MAJOR, API_MINOR, BUILD, and GCTX.POLICY can be obtained
>> +from the 'query-sev' qmp command.
>> +
>> +The value of MNONCE is part of the response of 'query-sev-launch-measure': it
>> +is the last 16 bytes of the base64-decoded data field (see SEV API Spec [1]
>> +section 6.5.2 Table 52: LAUNCH_MEASURE Measurement Buffer).
>> +
>> +The value of GCTX.LD is SHA256(firmware_blob || kernel_hashes_blob || vmsas_blob),
>> +where:
>> +
>> +* firmware_blob is the content of the entire firmware flash file (for example,
>> +  OVMF.fd).
> 
> Lets add a caveat that the firmware flash should be built to be stateless
> ie that it is not secure to attempt to measure a guest where the firmware
> uses an NVRAM store.
> 

* firmware_blob is the content of the entire firmware flash file (for   
  example, OVMF.fd).  Note that you must build a stateless firmware file    
  which doesn't use an NVRAM store, because the NVRAM area is not
  measured, and therefore it is not secure to use a firmware which uses 
  state from an NVRAM store.



>> +* if kernel is used, and kernel-hashes=on, then kernel_hashes_blob is the
>> +  content of PaddedSevHashTable (including the zero padding), which itself
>> +  includes the hashes of kernel, initrd, and cmdline that are passed to the
>> +  guest.  The PaddedSevHashTable struct is defined in target/i386/sev.c .
>> +* if SEV-ES is enabled (policy & 0x4 != 0), vmsas_blob is the concatenation of
>> +  all VMSAs of the guest vcpus.  Each VMSA is 4096 bytes long; its content is
>> +  defined inside Linux kernel code as struct vmcb_save_area, or in AMD APM
>> +  Volume 2 [2] Table B-2: VMCB Layout, State Save Area.
> 
> Is there any practical guidance we can give apps on the way the VMSAs
> can be expected to be initialized ? eg can they assume essentially
> all fields in vmcb_save_area are 0 initialized except for certain
> ones ? Is initialization likely to vary at all across KVM or EDK2
> vesions or something ?

From my own experience, the VMSA of vcpu0 doesn't change; it is basically what QEMU
sets up in x86_cpu_reset() (which is mostly zeros but not all).  I don't know if it
may change in newer QEMU (machine types?) or kvm.  As for vcpu1+, in SEV-ES the
CS:EIP for the APs is taken from a GUIDed table at the end of the OVMF image, and has
actually changed a few months ago when the memory layout changed to support both TDX
and SEV.


Here are the VMSAs for my 2-vcpu SEV-ES VM:


$ hd vmsa/vmsa_cpu0.bin
00000000  00 00 93 00 ff ff 00 00  00 00 00 00 00 00 00 00  |................|
00000010  00 f0 9b 00 ff ff 00 00  00 00 ff ff 00 00 00 00  |................|
00000020  00 00 93 00 ff ff 00 00  00 00 00 00 00 00 00 00  |................|
*
00000060  00 00 00 00 ff ff 00 00  00 00 00 00 00 00 00 00  |................|
00000070  00 00 82 00 ff ff 00 00  00 00 00 00 00 00 00 00  |................|
00000080  00 00 00 00 ff ff 00 00  00 00 00 00 00 00 00 00  |................|
00000090  00 00 8b 00 ff ff 00 00  00 00 00 00 00 00 00 00  |................|
000000a0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
000000d0  00 10 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
000000e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000140  00 00 00 00 00 00 00 00  40 00 00 00 00 00 00 00  |........@.......|
00000150  00 00 00 00 00 00 00 00  10 00 00 00 00 00 00 00  |................|
00000160  00 04 00 00 00 00 00 00  f0 0f ff ff 00 00 00 00  |................|
00000170  02 00 00 00 00 00 00 00  f0 ff 00 00 00 00 00 00  |................|
00000180  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000260  00 00 00 00 00 00 00 00  06 04 07 00 06 04 07 00  |................|
00000270  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000310  10 0f 83 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000320  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
000003e0  00 00 00 00 00 00 00 00  01 00 00 00 00 00 00 00  |................|
000003f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00001000



$ hd vmsa/vmsa_cpu1.bin
00000000  00 00 93 00 ff ff 00 00  00 00 00 00 00 00 00 00  |................|
00000010  00 f0 9b 00 ff ff 00 00  00 00 80 00 00 00 00 00  |................|
00000020  00 00 93 00 ff ff 00 00  00 00 00 00 00 00 00 00  |................|
*
00000060  00 00 00 00 ff ff 00 00  00 00 00 00 00 00 00 00  |................|
00000070  00 00 82 00 ff ff 00 00  00 00 00 00 00 00 00 00  |................|
00000080  00 00 00 00 ff ff 00 00  00 00 00 00 00 00 00 00  |................|
00000090  00 00 8b 00 ff ff 00 00  00 00 00 00 00 00 00 00  |................|
000000a0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
000000d0  00 10 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
000000e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000140  00 00 00 00 00 00 00 00  40 00 00 00 00 00 00 00  |........@.......|
00000150  00 00 00 00 00 00 00 00  10 00 00 00 00 00 00 00  |................|
00000160  00 04 00 00 00 00 00 00  f0 0f ff ff 00 00 00 00  |................|
00000170  02 00 00 00 00 00 00 00  00 b0 00 00 00 00 00 00  |................|
00000180  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000260  00 00 00 00 00 00 00 00  06 04 07 00 06 04 07 00  |................|
00000270  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000310  10 0f 83 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000320  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
000003e0  00 00 00 00 00 00 00 00  01 00 00 00 00 00 00 00  |................|
000003f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00001000





-Dov

Re: [PATCH] docs: Add measurement calculation details to amd-memory-encryption.txt

[Daniel P. Berrangé]

On Thu, Dec 16, 2021 at 12:38:34PM +0200, Dov Murik wrote:
> 
> 
> On 14/12/2021 20:39, Daniel P. Berrangé wrote:
> > On Tue, Dec 14, 2021 at 01:59:10PM +0000, Dov Murik wrote:
> >> Add a section explaining how the Guest Owner should calculate the
> >> expected guest launch measurement for SEV and SEV-ES.
> >>
> >> Also update the name and link to the SEV API Spec document.
> >>
> >> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
> >> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> >> ---
> >>  docs/amd-memory-encryption.txt | 50 +++++++++++++++++++++++++++++++---
> >>  1 file changed, 46 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/docs/amd-memory-encryption.txt b/docs/amd-memory-encryption.txt
> >> index ffca382b5f..f97727482f 100644
> >> --- a/docs/amd-memory-encryption.txt
> >> +++ b/docs/amd-memory-encryption.txt
> >> @@ -43,7 +43,7 @@ The guest policy is passed as plaintext. A hypervisor may choose to read it,
> >>  but should not modify it (any modification of the policy bits will result
> >>  in bad measurement). The guest policy is a 4-byte data structure containing
> >>  several flags that restricts what can be done on a running SEV guest.
> >> -See KM Spec section 3 and 6.2 for more details.
> >> +See SEV API Spec [1] section 3 and 6.2 for more details.
> >>  
> >>  The guest policy can be provided via the 'policy' property (see below)
> >>  
> >> @@ -88,7 +88,7 @@ expects.
> >>  LAUNCH_FINISH finalizes the guest launch and destroys the cryptographic
> >>  context.
> >>  
> >> -See SEV KM API Spec [1] 'Launching a guest' usage flow (Appendix A) for the
> >> +See SEV API Spec [1] 'Launching a guest' usage flow (Appendix A) for the
> >>  complete flow chart.
> >>  
> >>  To launch a SEV guest
> >> @@ -113,6 +113,45 @@ a SEV-ES guest:
> >>   - Requires in-kernel irqchip - the burden is placed on the hypervisor to
> >>     manage booting APs.
> >>  
> >> +Calculating expected guest launch measurement
> >> +---------------------------------------------
> >> +In order to verify the guest launch measurement, The Guest Owner must compute
> >> +it in the exact same way as it is calculated by the AMD-SP.  SEV API Spec [1]
> >> +section 6.5.1 describes the AMD-SP operations:
> >> +
> >> +    GCTX.LD is finalized, producing the hash digest of all plaintext data
> >> +    imported into the guest.
> >> +
> >> +    The launch measurement is calculated as:
> >> +
> >> +    HMAC(0x04 || API_MAJOR || API_MINOR || BUILD || GCTX.POLICY || GCTX.LD || MNONCE; GCTX.TIK)
> >> +
> >> +    where "||" represents concatenation.
> >> +
> >> +The values of API_MAJOR, API_MINOR, BUILD, and GCTX.POLICY can be obtained
> >> +from the 'query-sev' qmp command.
> >> +
> >> +The value of MNONCE is part of the response of 'query-sev-launch-measure': it
> >> +is the last 16 bytes of the base64-decoded data field (see SEV API Spec [1]
> >> +section 6.5.2 Table 52: LAUNCH_MEASURE Measurement Buffer).
> >> +
> >> +The value of GCTX.LD is SHA256(firmware_blob || kernel_hashes_blob || vmsas_blob),
> >> +where:
> >> +
> >> +* firmware_blob is the content of the entire firmware flash file (for example,
> >> +  OVMF.fd).
> > 
> > Lets add a caveat that the firmware flash should be built to be stateless
> > ie that it is not secure to attempt to measure a guest where the firmware
> > uses an NVRAM store.
> > 
> 
> * firmware_blob is the content of the entire firmware flash file (for   
>   example, OVMF.fd).  Note that you must build a stateless firmware file    
>   which doesn't use an NVRAM store, because the NVRAM area is not
>   measured, and therefore it is not secure to use a firmware which uses 
>   state from an NVRAM store.

Looks good to me.

> >> +* if kernel is used, and kernel-hashes=on, then kernel_hashes_blob is the
> >> +  content of PaddedSevHashTable (including the zero padding), which itself
> >> +  includes the hashes of kernel, initrd, and cmdline that are passed to the
> >> +  guest.  The PaddedSevHashTable struct is defined in target/i386/sev.c .
> >> +* if SEV-ES is enabled (policy & 0x4 != 0), vmsas_blob is the concatenation of
> >> +  all VMSAs of the guest vcpus.  Each VMSA is 4096 bytes long; its content is
> >> +  defined inside Linux kernel code as struct vmcb_save_area, or in AMD APM
> >> +  Volume 2 [2] Table B-2: VMCB Layout, State Save Area.
> > 
> > Is there any practical guidance we can give apps on the way the VMSAs
> > can be expected to be initialized ? eg can they assume essentially
> > all fields in vmcb_save_area are 0 initialized except for certain
> > ones ? Is initialization likely to vary at all across KVM or EDK2
> > vesions or something ?
> 
> From my own experience, the VMSA of vcpu0 doesn't change; it is basically what QEMU
> sets up in x86_cpu_reset() (which is mostly zeros but not all).  I don't know if it
> may change in newer QEMU (machine types?) or kvm.  As for vcpu1+, in SEV-ES the
> CS:EIP for the APs is taken from a GUIDed table at the end of the OVMF image, and has
> actually changed a few months ago when the memory layout changed to support both TDX
> and SEV.

That is an unplesantly large number of moving parts that could
potentially impact the expected state :-(  I think we need to
be careful to avoid gratuitous changes, to avoid creating a
combinatorial expansion in the number of possibly valid VMSA
blocks.

It makes me wonder if we need to think about defining some
standard approach for distro vendors (and/or cloud vendors)
to publish the expected contents for various combinations
of their software pieces.

> 
> 
> Here are the VMSAs for my 2-vcpu SEV-ES VM:
> 
> 
> $ hd vmsa/vmsa_cpu0.bin

...snipp...

was there a nice approach / tool you used to capture
this initial state ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] docs: Add measurement calculation details to amd-memory-encryption.txt

[Dov Murik]

On 16/12/2021 18:09, Daniel P. Berrangé wrote:
> On Thu, Dec 16, 2021 at 12:38:34PM +0200, Dov Murik wrote:
>>
>>
>> On 14/12/2021 20:39, Daniel P. Berrangé wrote:
>>> On Tue, Dec 14, 2021 at 01:59:10PM +0000, Dov Murik wrote:
>>>> Add a section explaining how the Guest Owner should calculate the
>>>> expected guest launch measurement for SEV and SEV-ES.
>>>>
>>>> Also update the name and link to the SEV API Spec document.
>>>>
>>>> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
>>>> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
>>>> ---
>>>>  docs/amd-memory-encryption.txt | 50 +++++++++++++++++++++++++++++++---
>>>>  1 file changed, 46 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/docs/amd-memory-encryption.txt b/docs/amd-memory-encryption.txt
>>>> index ffca382b5f..f97727482f 100644
>>>> --- a/docs/amd-memory-encryption.txt
>>>> +++ b/docs/amd-memory-encryption.txt
>>>> @@ -43,7 +43,7 @@ The guest policy is passed as plaintext. A hypervisor may choose to read it,
>>>>  but should not modify it (any modification of the policy bits will result
>>>>  in bad measurement). The guest policy is a 4-byte data structure containing
>>>>  several flags that restricts what can be done on a running SEV guest.
>>>> -See KM Spec section 3 and 6.2 for more details.
>>>> +See SEV API Spec [1] section 3 and 6.2 for more details.
>>>>  
>>>>  The guest policy can be provided via the 'policy' property (see below)
>>>>  
>>>> @@ -88,7 +88,7 @@ expects.
>>>>  LAUNCH_FINISH finalizes the guest launch and destroys the cryptographic
>>>>  context.
>>>>  
>>>> -See SEV KM API Spec [1] 'Launching a guest' usage flow (Appendix A) for the
>>>> +See SEV API Spec [1] 'Launching a guest' usage flow (Appendix A) for the
>>>>  complete flow chart.
>>>>  
>>>>  To launch a SEV guest
>>>> @@ -113,6 +113,45 @@ a SEV-ES guest:
>>>>   - Requires in-kernel irqchip - the burden is placed on the hypervisor to
>>>>     manage booting APs.
>>>>  
>>>> +Calculating expected guest launch measurement
>>>> +---------------------------------------------
>>>> +In order to verify the guest launch measurement, The Guest Owner must compute
>>>> +it in the exact same way as it is calculated by the AMD-SP.  SEV API Spec [1]
>>>> +section 6.5.1 describes the AMD-SP operations:
>>>> +
>>>> +    GCTX.LD is finalized, producing the hash digest of all plaintext data
>>>> +    imported into the guest.
>>>> +
>>>> +    The launch measurement is calculated as:
>>>> +
>>>> +    HMAC(0x04 || API_MAJOR || API_MINOR || BUILD || GCTX.POLICY || GCTX.LD || MNONCE; GCTX.TIK)
>>>> +
>>>> +    where "||" represents concatenation.
>>>> +
>>>> +The values of API_MAJOR, API_MINOR, BUILD, and GCTX.POLICY can be obtained
>>>> +from the 'query-sev' qmp command.
>>>> +
>>>> +The value of MNONCE is part of the response of 'query-sev-launch-measure': it
>>>> +is the last 16 bytes of the base64-decoded data field (see SEV API Spec [1]
>>>> +section 6.5.2 Table 52: LAUNCH_MEASURE Measurement Buffer).
>>>> +
>>>> +The value of GCTX.LD is SHA256(firmware_blob || kernel_hashes_blob || vmsas_blob),
>>>> +where:
>>>> +
>>>> +* firmware_blob is the content of the entire firmware flash file (for example,
>>>> +  OVMF.fd).
>>>
>>> Lets add a caveat that the firmware flash should be built to be stateless
>>> ie that it is not secure to attempt to measure a guest where the firmware
>>> uses an NVRAM store.
>>>
>>
>> * firmware_blob is the content of the entire firmware flash file (for   
>>   example, OVMF.fd).  Note that you must build a stateless firmware file    
>>   which doesn't use an NVRAM store, because the NVRAM area is not
>>   measured, and therefore it is not secure to use a firmware which uses 
>>   state from an NVRAM store.
> 
> Looks good to me.
> 
>>>> +* if kernel is used, and kernel-hashes=on, then kernel_hashes_blob is the
>>>> +  content of PaddedSevHashTable (including the zero padding), which itself
>>>> +  includes the hashes of kernel, initrd, and cmdline that are passed to the
>>>> +  guest.  The PaddedSevHashTable struct is defined in target/i386/sev.c .
>>>> +* if SEV-ES is enabled (policy & 0x4 != 0), vmsas_blob is the concatenation of
>>>> +  all VMSAs of the guest vcpus.  Each VMSA is 4096 bytes long; its content is
>>>> +  defined inside Linux kernel code as struct vmcb_save_area, or in AMD APM
>>>> +  Volume 2 [2] Table B-2: VMCB Layout, State Save Area.
>>>
>>> Is there any practical guidance we can give apps on the way the VMSAs
>>> can be expected to be initialized ? eg can they assume essentially
>>> all fields in vmcb_save_area are 0 initialized except for certain
>>> ones ? Is initialization likely to vary at all across KVM or EDK2
>>> vesions or something ?
>>
>> From my own experience, the VMSA of vcpu0 doesn't change; it is basically what QEMU
>> sets up in x86_cpu_reset() (which is mostly zeros but not all).  I don't know if it
>> may change in newer QEMU (machine types?) or kvm.  As for vcpu1+, in SEV-ES the
>> CS:EIP for the APs is taken from a GUIDed table at the end of the OVMF image, and has
>> actually changed a few months ago when the memory layout changed to support both TDX
>> and SEV.
> 
> That is an unplesantly large number of moving parts that could
> potentially impact the expected state :-(  I think we need to
> be careful to avoid gratuitous changes, to avoid creating a
> combinatorial expansion in the number of possibly valid VMSA
> blocks.
> 
> It makes me wonder if we need to think about defining some
> standard approach for distro vendors (and/or cloud vendors)
> to publish the expected contents for various combinations
> of their software pieces.
> 
>>
>>
>> Here are the VMSAs for my 2-vcpu SEV-ES VM:
>>
>>
>> $ hd vmsa/vmsa_cpu0.bin
> 
> ...snipp...
> 
> was there a nice approach / tool you used to capture
> this initial state ?
> 

I wouldn't qualify this as nice: I ended up modifying my
host kernel's kvm (see patch below).  Later I wrote a
script to parse that hex dump from the kernel log into
proper 4096-byte binary VMSA files.



diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 7fbce342eec4..4e45fe37b93d 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -624,6 +624,12 @@ static int sev_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
                 */
                clflush_cache_range(svm->vmsa, PAGE_SIZE);

+                /* dubek */
+                pr_info("DEBUG_VMSA - cpu %d START ---------------\n", i);
+                print_hex_dump(KERN_INFO, "DEBUG_VMSA", DUMP_PREFIX_OFFSET, 16, 1, svm->vmsa, PAGE_SIZE, true);
+                pr_info("DEBUG_VMSA - cpu %d END ---------------\n", i);
+                /* ----- */
+
                vmsa.handle = sev->handle;
                vmsa.address = __sme_pa(svm->vmsa);
                vmsa.len = PAGE_SIZE;

Re: [PATCH] docs: Add measurement calculation details to amd-memory-encryption.txt

[Daniel P. Berrangé]

On Thu, Dec 16, 2021 at 11:41:27PM +0200, Dov Murik wrote:
> 
> 
> On 16/12/2021 18:09, Daniel P. Berrangé wrote:
> > On Thu, Dec 16, 2021 at 12:38:34PM +0200, Dov Murik wrote:
> >>
> >>
> >> On 14/12/2021 20:39, Daniel P. Berrangé wrote:
> >>> On Tue, Dec 14, 2021 at 01:59:10PM +0000, Dov Murik wrote:
> >>>> Add a section explaining how the Guest Owner should calculate the
> >>>> expected guest launch measurement for SEV and SEV-ES.
> >>>>
> >>>> Also update the name and link to the SEV API Spec document.
> >>>>
> >>>> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
> >>>> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> >>>> ---
> >>>>  docs/amd-memory-encryption.txt | 50 +++++++++++++++++++++++++++++++---
> >>>>  1 file changed, 46 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/docs/amd-memory-encryption.txt b/docs/amd-memory-encryption.txt
> >>>> index ffca382b5f..f97727482f 100644
> >>>> --- a/docs/amd-memory-encryption.txt
> >>>> +++ b/docs/amd-memory-encryption.txt
> >>>> @@ -43,7 +43,7 @@ The guest policy is passed as plaintext. A hypervisor may choose to read it,
> >>>>  but should not modify it (any modification of the policy bits will result
> >>>>  in bad measurement). The guest policy is a 4-byte data structure containing
> >>>>  several flags that restricts what can be done on a running SEV guest.
> >>>> -See KM Spec section 3 and 6.2 for more details.
> >>>> +See SEV API Spec [1] section 3 and 6.2 for more details.
> >>>>  
> >>>>  The guest policy can be provided via the 'policy' property (see below)
> >>>>  
> >>>> @@ -88,7 +88,7 @@ expects.
> >>>>  LAUNCH_FINISH finalizes the guest launch and destroys the cryptographic
> >>>>  context.
> >>>>  
> >>>> -See SEV KM API Spec [1] 'Launching a guest' usage flow (Appendix A) for the
> >>>> +See SEV API Spec [1] 'Launching a guest' usage flow (Appendix A) for the
> >>>>  complete flow chart.
> >>>>  
> >>>>  To launch a SEV guest
> >>>> @@ -113,6 +113,45 @@ a SEV-ES guest:
> >>>>   - Requires in-kernel irqchip - the burden is placed on the hypervisor to
> >>>>     manage booting APs.
> >>>>  
> >>>> +Calculating expected guest launch measurement
> >>>> +---------------------------------------------
> >>>> +In order to verify the guest launch measurement, The Guest Owner must compute
> >>>> +it in the exact same way as it is calculated by the AMD-SP.  SEV API Spec [1]
> >>>> +section 6.5.1 describes the AMD-SP operations:
> >>>> +
> >>>> +    GCTX.LD is finalized, producing the hash digest of all plaintext data
> >>>> +    imported into the guest.
> >>>> +
> >>>> +    The launch measurement is calculated as:
> >>>> +
> >>>> +    HMAC(0x04 || API_MAJOR || API_MINOR || BUILD || GCTX.POLICY || GCTX.LD || MNONCE; GCTX.TIK)
> >>>> +
> >>>> +    where "||" represents concatenation.
> >>>> +
> >>>> +The values of API_MAJOR, API_MINOR, BUILD, and GCTX.POLICY can be obtained
> >>>> +from the 'query-sev' qmp command.
> >>>> +
> >>>> +The value of MNONCE is part of the response of 'query-sev-launch-measure': it
> >>>> +is the last 16 bytes of the base64-decoded data field (see SEV API Spec [1]
> >>>> +section 6.5.2 Table 52: LAUNCH_MEASURE Measurement Buffer).
> >>>> +
> >>>> +The value of GCTX.LD is SHA256(firmware_blob || kernel_hashes_blob || vmsas_blob),
> >>>> +where:
> >>>> +
> >>>> +* firmware_blob is the content of the entire firmware flash file (for example,
> >>>> +  OVMF.fd).
> >>>
> >>> Lets add a caveat that the firmware flash should be built to be stateless
> >>> ie that it is not secure to attempt to measure a guest where the firmware
> >>> uses an NVRAM store.
> >>>
> >>
> >> * firmware_blob is the content of the entire firmware flash file (for   
> >>   example, OVMF.fd).  Note that you must build a stateless firmware file    
> >>   which doesn't use an NVRAM store, because the NVRAM area is not
> >>   measured, and therefore it is not secure to use a firmware which uses 
> >>   state from an NVRAM store.
> > 
> > Looks good to me.
> > 
> >>>> +* if kernel is used, and kernel-hashes=on, then kernel_hashes_blob is the
> >>>> +  content of PaddedSevHashTable (including the zero padding), which itself
> >>>> +  includes the hashes of kernel, initrd, and cmdline that are passed to the
> >>>> +  guest.  The PaddedSevHashTable struct is defined in target/i386/sev.c .
> >>>> +* if SEV-ES is enabled (policy & 0x4 != 0), vmsas_blob is the concatenation of
> >>>> +  all VMSAs of the guest vcpus.  Each VMSA is 4096 bytes long; its content is
> >>>> +  defined inside Linux kernel code as struct vmcb_save_area, or in AMD APM
> >>>> +  Volume 2 [2] Table B-2: VMCB Layout, State Save Area.
> >>>
> >>> Is there any practical guidance we can give apps on the way the VMSAs
> >>> can be expected to be initialized ? eg can they assume essentially
> >>> all fields in vmcb_save_area are 0 initialized except for certain
> >>> ones ? Is initialization likely to vary at all across KVM or EDK2
> >>> vesions or something ?
> >>
> >> From my own experience, the VMSA of vcpu0 doesn't change; it is basically what QEMU
> >> sets up in x86_cpu_reset() (which is mostly zeros but not all).  I don't know if it
> >> may change in newer QEMU (machine types?) or kvm.  As for vcpu1+, in SEV-ES the
> >> CS:EIP for the APs is taken from a GUIDed table at the end of the OVMF image, and has
> >> actually changed a few months ago when the memory layout changed to support both TDX
> >> and SEV.
> > 
> > That is an unplesantly large number of moving parts that could
> > potentially impact the expected state :-(  I think we need to
> > be careful to avoid gratuitous changes, to avoid creating a
> > combinatorial expansion in the number of possibly valid VMSA
> > blocks.
> > 
> > It makes me wonder if we need to think about defining some
> > standard approach for distro vendors (and/or cloud vendors)
> > to publish the expected contents for various combinations
> > of their software pieces.
> > 
> >>
> >>
> >> Here are the VMSAs for my 2-vcpu SEV-ES VM:
> >>
> >>
> >> $ hd vmsa/vmsa_cpu0.bin
> > 
> > ...snipp...
> > 
> > was there a nice approach / tool you used to capture
> > this initial state ?
> > 
> 
> I wouldn't qualify this as nice: I ended up modifying my
> host kernel's kvm (see patch below).  Later I wrote a
> script to parse that hex dump from the kernel log into
> proper 4096-byte binary VMSA files.

Heh, that's basically the same as Sergio Lopez told me he did
for libkrun.

He suggested that it might be desirable to expose this info
in sysfs. Perhaps a entry for debugfs from KVM for each
VM to export the initial state.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 7fbce342eec4..4e45fe37b93d 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -624,6 +624,12 @@ static int sev_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
>                  */
>                 clflush_cache_range(svm->vmsa, PAGE_SIZE);
> 
> +                /* dubek */
> +                pr_info("DEBUG_VMSA - cpu %d START ---------------\n", i);
> +                print_hex_dump(KERN_INFO, "DEBUG_VMSA", DUMP_PREFIX_OFFSET, 16, 1, svm->vmsa, PAGE_SIZE, true);
> +                pr_info("DEBUG_VMSA - cpu %d END ---------------\n", i);
> +                /* ----- */
> +
>                 vmsa.handle = sev->handle;
>                 vmsa.address = __sme_pa(svm->vmsa);
>                 vmsa.len = PAGE_SIZE;
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] docs: Add measurement calculation details to amd-memory-encryption.txt

[Daniel P. Berrangé]

On Thu, Dec 16, 2021 at 11:41:27PM +0200, Dov Murik wrote:
> 
> 
> On 16/12/2021 18:09, Daniel P. Berrangé wrote:
> > On Thu, Dec 16, 2021 at 12:38:34PM +0200, Dov Murik wrote:
> >>
> >>
> >> On 14/12/2021 20:39, Daniel P. Berrangé wrote:
> >>> Is there any practical guidance we can give apps on the way the VMSAs
> >>> can be expected to be initialized ? eg can they assume essentially
> >>> all fields in vmcb_save_area are 0 initialized except for certain
> >>> ones ? Is initialization likely to vary at all across KVM or EDK2
> >>> vesions or something ?
> >>
> >> From my own experience, the VMSA of vcpu0 doesn't change; it is basically what QEMU
> >> sets up in x86_cpu_reset() (which is mostly zeros but not all).  I don't know if it
> >> may change in newer QEMU (machine types?) or kvm.  As for vcpu1+, in SEV-ES the
> >> CS:EIP for the APs is taken from a GUIDed table at the end of the OVMF image, and has
> >> actually changed a few months ago when the memory layout changed to support both TDX
> >> and SEV.
> > 
> > That is an unplesantly large number of moving parts that could
> > potentially impact the expected state :-(  I think we need to
> > be careful to avoid gratuitous changes, to avoid creating a
> > combinatorial expansion in the number of possibly valid VMSA
> > blocks.
> > 
> > It makes me wonder if we need to think about defining some
> > standard approach for distro vendors (and/or cloud vendors)
> > to publish the expected contents for various combinations
> > of their software pieces.
> > 
> >>
> >>
> >> Here are the VMSAs for my 2-vcpu SEV-ES VM:
> >>
> >>
> >> $ hd vmsa/vmsa_cpu0.bin
> > 
> > ...snipp...
> > 
> > was there a nice approach / tool you used to capture
> > this initial state ?
> > 
> 
> I wouldn't qualify this as nice: I ended up modifying my
> host kernel's kvm (see patch below).  Later I wrote a
> script to parse that hex dump from the kernel log into
> proper 4096-byte binary VMSA files.
> 
> 
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 7fbce342eec4..4e45fe37b93d 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -624,6 +624,12 @@ static int sev_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
>                  */
>                 clflush_cache_range(svm->vmsa, PAGE_SIZE);
> 
> +                /* dubek */
> +                pr_info("DEBUG_VMSA - cpu %d START ---------------\n", i);
> +                print_hex_dump(KERN_INFO, "DEBUG_VMSA", DUMP_PREFIX_OFFSET, 16, 1, svm->vmsa, PAGE_SIZE, true);
> +                pr_info("DEBUG_VMSA - cpu %d END ---------------\n", i);
> +                /* ----- */
> +
>                 vmsa.handle = sev->handle;
>                 vmsa.address = __sme_pa(svm->vmsa);
>                 vmsa.len = PAGE_SIZE;

FWIW, I made a 1% less hacky solution by writing a systemtap
script. It will require changing to set the line number for
every single kernel version, but at least it doesn't require
building a custom kernel

$ cat sev-vmsa.stp 
function dump_vmsa(addr:long) {
  printf("VMSA\n")
  for (i = 0; i < 4096 ; i+= 64) {
    printf("%.64M\n", addr + i);
  }
}

probe module("kvm_amd").statement("__sev_launch_update_vmsa@arch/x86/kvm/svm/sev.c:618") {
  dump_vmsa($svm->vmsa)
}

the line number is that of the 'vmsa.handle = sev->handle' assignment

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] docs: Add measurement calculation details to amd-memory-encryption.txt

[Dov Murik]

On 07/01/2022 22:18, Daniel P. Berrangé wrote:
> On Thu, Dec 16, 2021 at 11:41:27PM +0200, Dov Murik wrote:
>>
>>
>> On 16/12/2021 18:09, Daniel P. Berrangé wrote:
>>> On Thu, Dec 16, 2021 at 12:38:34PM +0200, Dov Murik wrote:
>>>>
>>>>
>>>> On 14/12/2021 20:39, Daniel P. Berrangé wrote:
>>>>> Is there any practical guidance we can give apps on the way the VMSAs
>>>>> can be expected to be initialized ? eg can they assume essentially
>>>>> all fields in vmcb_save_area are 0 initialized except for certain
>>>>> ones ? Is initialization likely to vary at all across KVM or EDK2
>>>>> vesions or something ?
>>>>
>>>> From my own experience, the VMSA of vcpu0 doesn't change; it is basically what QEMU
>>>> sets up in x86_cpu_reset() (which is mostly zeros but not all).  I don't know if it
>>>> may change in newer QEMU (machine types?) or kvm.  As for vcpu1+, in SEV-ES the
>>>> CS:EIP for the APs is taken from a GUIDed table at the end of the OVMF image, and has
>>>> actually changed a few months ago when the memory layout changed to support both TDX
>>>> and SEV.
>>>
>>> That is an unplesantly large number of moving parts that could
>>> potentially impact the expected state :-(  I think we need to
>>> be careful to avoid gratuitous changes, to avoid creating a
>>> combinatorial expansion in the number of possibly valid VMSA
>>> blocks.
>>>
>>> It makes me wonder if we need to think about defining some
>>> standard approach for distro vendors (and/or cloud vendors)
>>> to publish the expected contents for various combinations
>>> of their software pieces.
>>>
>>>>
>>>>
>>>> Here are the VMSAs for my 2-vcpu SEV-ES VM:
>>>>
>>>>
>>>> $ hd vmsa/vmsa_cpu0.bin
>>>
>>> ...snipp...
>>>
>>> was there a nice approach / tool you used to capture
>>> this initial state ?
>>>
>>
>> I wouldn't qualify this as nice: I ended up modifying my
>> host kernel's kvm (see patch below).  Later I wrote a
>> script to parse that hex dump from the kernel log into
>> proper 4096-byte binary VMSA files.
>>
>>
>>
>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index 7fbce342eec4..4e45fe37b93d 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -624,6 +624,12 @@ static int sev_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
>>                  */
>>                 clflush_cache_range(svm->vmsa, PAGE_SIZE);
>>
>> +                /* dubek */
>> +                pr_info("DEBUG_VMSA - cpu %d START ---------------\n", i);
>> +                print_hex_dump(KERN_INFO, "DEBUG_VMSA", DUMP_PREFIX_OFFSET, 16, 1, svm->vmsa, PAGE_SIZE, true);
>> +                pr_info("DEBUG_VMSA - cpu %d END ---------------\n", i);
>> +                /* ----- */
>> +
>>                 vmsa.handle = sev->handle;
>>                 vmsa.address = __sme_pa(svm->vmsa);
>>                 vmsa.len = PAGE_SIZE;
> 
> FWIW, I made a 1% less hacky solution by writing a systemtap
> script. It will require changing to set the line number for
> every single kernel version, but at least it doesn't require
> building a custom kernel

Thanks, we'll check it out.  It does require a kernel compiled with
debug info (I assume) to be able to hook the exact line number.

-Dov

> 
> $ cat sev-vmsa.stp 
> function dump_vmsa(addr:long) {
>   printf("VMSA\n")
>   for (i = 0; i < 4096 ; i+= 64) {
>     printf("%.64M\n", addr + i);
>   }
> }
> 
> probe module("kvm_amd").statement("__sev_launch_update_vmsa@arch/x86/kvm/svm/sev.c:618") {
>   dump_vmsa($svm->vmsa)
> }
> 
> the line number is that of the 'vmsa.handle = sev->handle' assignment
> 
> Regards,
> Daniel

Re: [PATCH] docs: Add measurement calculation details to amd-memory-encryption.txt

[Daniel P. Berrangé]

On Mon, Jan 10, 2022 at 01:17:02PM +0200, Dov Murik wrote:
> 
> 
> On 07/01/2022 22:18, Daniel P. Berrangé wrote:
> > On Thu, Dec 16, 2021 at 11:41:27PM +0200, Dov Murik wrote:
> >>
> >>
> >> On 16/12/2021 18:09, Daniel P. Berrangé wrote:
> >>> On Thu, Dec 16, 2021 at 12:38:34PM +0200, Dov Murik wrote:
> >>>>
> >>>>
> >>>> On 14/12/2021 20:39, Daniel P. Berrangé wrote:
> >>>>> Is there any practical guidance we can give apps on the way the VMSAs
> >>>>> can be expected to be initialized ? eg can they assume essentially
> >>>>> all fields in vmcb_save_area are 0 initialized except for certain
> >>>>> ones ? Is initialization likely to vary at all across KVM or EDK2
> >>>>> vesions or something ?
> >>>>
> >>>> From my own experience, the VMSA of vcpu0 doesn't change; it is basically what QEMU
> >>>> sets up in x86_cpu_reset() (which is mostly zeros but not all).  I don't know if it
> >>>> may change in newer QEMU (machine types?) or kvm.  As for vcpu1+, in SEV-ES the
> >>>> CS:EIP for the APs is taken from a GUIDed table at the end of the OVMF image, and has
> >>>> actually changed a few months ago when the memory layout changed to support both TDX
> >>>> and SEV.
> >>>
> >>> That is an unplesantly large number of moving parts that could
> >>> potentially impact the expected state :-(  I think we need to
> >>> be careful to avoid gratuitous changes, to avoid creating a
> >>> combinatorial expansion in the number of possibly valid VMSA
> >>> blocks.
> >>>
> >>> It makes me wonder if we need to think about defining some
> >>> standard approach for distro vendors (and/or cloud vendors)
> >>> to publish the expected contents for various combinations
> >>> of their software pieces.
> >>>
> >>>>
> >>>>
> >>>> Here are the VMSAs for my 2-vcpu SEV-ES VM:
> >>>>
> >>>>
> >>>> $ hd vmsa/vmsa_cpu0.bin
> >>>
> >>> ...snipp...
> >>>
> >>> was there a nice approach / tool you used to capture
> >>> this initial state ?
> >>>
> >>
> >> I wouldn't qualify this as nice: I ended up modifying my
> >> host kernel's kvm (see patch below).  Later I wrote a
> >> script to parse that hex dump from the kernel log into
> >> proper 4096-byte binary VMSA files.
> >>
> >>
> >>
> >> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> >> index 7fbce342eec4..4e45fe37b93d 100644
> >> --- a/arch/x86/kvm/svm/sev.c
> >> +++ b/arch/x86/kvm/svm/sev.c
> >> @@ -624,6 +624,12 @@ static int sev_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
> >>                  */
> >>                 clflush_cache_range(svm->vmsa, PAGE_SIZE);
> >>
> >> +                /* dubek */
> >> +                pr_info("DEBUG_VMSA - cpu %d START ---------------\n", i);
> >> +                print_hex_dump(KERN_INFO, "DEBUG_VMSA", DUMP_PREFIX_OFFSET, 16, 1, svm->vmsa, PAGE_SIZE, true);
> >> +                pr_info("DEBUG_VMSA - cpu %d END ---------------\n", i);
> >> +                /* ----- */
> >> +
> >>                 vmsa.handle = sev->handle;
> >>                 vmsa.address = __sme_pa(svm->vmsa);
> >>                 vmsa.len = PAGE_SIZE;
> > 
> > FWIW, I made a 1% less hacky solution by writing a systemtap
> > script. It will require changing to set the line number for
> > every single kernel version, but at least it doesn't require
> > building a custom kernel
> 
> Thanks, we'll check it out.  It does require a kernel compiled with
> debug info (I assume) to be able to hook the exact line number.

On RHEL / Fedora, you should merely need to install the corresponding
-debuginfo RPM to match your running kernel.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|