diff mbox series

[2/2] KVM: SVM: Don't flush cache of SEV-encrypted pages if hardware enforces cache coherency across encryption domains

Message ID 20200829005926.5477-2-krish.sadhukhan@oracle.com (mailing list archive)
State New, archived
Headers show
Series [1/2] SVM: Replace numeric value for SME CPUID leaf with a #define | expand

Commit Message

Krish Sadhukhan Aug. 29, 2020, 12:59 a.m. UTC
Some hardware implementations may enforce cache coherency across encryption
domains. In such cases, it's not required to flush SEV-encrypted pages off
cache lines.

Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
 arch/x86/kvm/svm/sev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Tom Lendacky Aug. 31, 2020, 4:14 p.m. UTC | #1
On 8/28/20 7:59 PM, Krish Sadhukhan wrote:
> Some hardware implementations may enforce cache coherency across encryption
> domains. In such cases, it's not required to flush SEV-encrypted pages off
> cache lines.
> 
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> ---
>   arch/x86/kvm/svm/sev.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 402dc4234e39..c8ed8a62d5ef 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -384,7 +384,8 @@ static void sev_clflush_pages(struct page *pages[], unsigned long npages)
>   	uint8_t *page_virtual;
>   	unsigned long i;
>   
> -	if (npages == 0 || pages == NULL)
> +	if ((cpuid_eax(SVM_SME_CPUID_FUNC) & (1u << 10)) || npages == 0 ||

Thanks for the patch. This should really be added as an X86_FEATURE bit, 
and then check that feature here, as opposed to calling CPUID every time. 
Also, there are other places in the kernel that this may be relevant, are 
you investigating those areas, also (e.g. set_memory_encrypted() / 
set_memory_decrypted())?

Thanks,
Tom

> +	    pages == NULL)
>   		return;
>   
>   	for (i = 0; i < npages; i++) {
>
Krish Sadhukhan Sept. 2, 2020, 2:20 a.m. UTC | #2
On 8/31/20 9:14 AM, Tom Lendacky wrote:
> On 8/28/20 7:59 PM, Krish Sadhukhan wrote:
>> Some hardware implementations may enforce cache coherency across 
>> encryption
>> domains. In such cases, it's not required to flush SEV-encrypted 
>> pages off
>> cache lines.
>>
>> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
>> ---
>>   arch/x86/kvm/svm/sev.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index 402dc4234e39..c8ed8a62d5ef 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -384,7 +384,8 @@ static void sev_clflush_pages(struct page 
>> *pages[], unsigned long npages)
>>       uint8_t *page_virtual;
>>       unsigned long i;
>>   -    if (npages == 0 || pages == NULL)
>> +    if ((cpuid_eax(SVM_SME_CPUID_FUNC) & (1u << 10)) || npages == 0 ||
>
> Thanks for the patch. This should really be added as an X86_FEATURE 
> bit, and then check that feature here, as opposed to calling CPUID 
> every time. Also, there are other places in the kernel that this may 
> be relevant, are you investigating those areas, also (e.g. 
> set_memory_encrypted() / set_memory_decrypted())?


Thanks for the suggestions. I will add it as an X86_FEATURE bit.

As for other places, I see that __set_memory_enc_dec() is where we can 
add this condition. Not sure if I have missed anything, but other places 
where we call cpa_flush() and stuff like that, are for regular pages and 
not encrypted one.

I will send out v2 with these changes.


>
> Thanks,
> Tom
>
>> +        pages == NULL)
>>           return;
>>         for (i = 0; i < npages; i++) {
>>
diff mbox series

Patch

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 402dc4234e39..c8ed8a62d5ef 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -384,7 +384,8 @@  static void sev_clflush_pages(struct page *pages[], unsigned long npages)
 	uint8_t *page_virtual;
 	unsigned long i;
 
-	if (npages == 0 || pages == NULL)
+	if ((cpuid_eax(SVM_SME_CPUID_FUNC) & (1u << 10)) || npages == 0 ||
+	    pages == NULL)
 		return;
 
 	for (i = 0; i < npages; i++) {