Messages in this thread Patch in this message | ![/](/images/icornerl.gif) | | Subject | Re: [PATCH] KVM: SVM: Mark SEV launch secret pages as dirty. | From | Paolo Bonzini <> | Date | Wed, 23 Sep 2020 18:59:36 +0200 |
| |
On 19/09/20 06:55, Sean Christopherson wrote: > Side topic, while I love the comment (I do, honestly) regarding in-place > encryption, this is the fourth? instance of the same 4-line comment (6 lines > if you count the /* and */. Maybe it's time to do something like > > /* LAUNCH_SECRET does in-place encryption, see sev_clflush_pages(). */ > > and then have the main comment in sev_clflush_pages(). With the addition of > X86_FEATURE_SME_COHERENT, there's even a fantastic location for the comment:
Two of the three instances are a bit different though. What about this which at least shortens the comment to 2 fewer lines:
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index bb0e89c79a04..7b11546e65ba 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -446,10 +446,8 @@ static int sev_launch_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp) } /* - * The LAUNCH_UPDATE command will perform in-place encryption of the - * memory content (i.e it will write the same memory region with C=1). - * It's possible that the cache may contain the data with C=0, i.e., - * unencrypted so invalidate it first. + * Flush before LAUNCH_UPDATE encrypts pages in place, in case the cache + * contains the data that was written unencrypted. */ sev_clflush_pages(inpages, npages); @@ -805,10 +803,8 @@ static int sev_dbg_crypt(struct kvm *kvm, struct kvm_sev_cmd *argp, bool dec) } /* - * The DBG_{DE,EN}CRYPT commands will perform {dec,en}cryption of the - * memory content (i.e it will write the same memory region with C=1). - * It's possible that the cache may contain the data with C=0, i.e., - * unencrypted so invalidate it first. + * Flush before DBG_{DE,EN}CRYPT reads or modifies the pages, flush the + * destination too in case the cache contains its current data. */ sev_clflush_pages(src_p, 1); sev_clflush_pages(dst_p, 1); @@ -870,10 +866,8 @@ static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp) return PTR_ERR(pages); /* - * The LAUNCH_SECRET command will perform in-place encryption of the - * memory content (i.e it will write the same memory region with C=1). - * It's possible that the cache may contain the data with C=0, i.e., - * unencrypted so invalidate it first. + * Flush before LAUNCH_SECRET encrypts pages in place, in case the cache + * contains the data that was written unencrypted. */ sev_clflush_pages(pages, n);
| ![\](/images/icornerr.gif) |