All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4 v3] x86: AMD: Don't flush cache if hardware enforces cache coherency across encryption domains
@ 2020-09-11 19:25 Krish Sadhukhan
  2020-09-11 19:25 ` [PATCH 1/4 v3] x86: AMD: Replace numeric value for SME CPUID leaf with a #define Krish Sadhukhan
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Krish Sadhukhan @ 2020-09-11 19:25 UTC (permalink / raw)
  To: kvm
  Cc: pbonzini, jmattson, tglx, mingo, bp, x86, sean.j.christopherson,
	vkuznets, wanpengli, joro, dave.hansen, luto, peterz,
	linux-kernel, hpa

In some hardware implementations, coherency between the encrypted and
unencrypted mappings of the same physical page is enforced. In such a system,
it is not required for software to flush the page from all CPU caches in the
system prior to changing the value of the C-bit for a page. This hardware-
enforced cache coherency is indicated by EAX[10] in CPUID leaf 0x8000001f.

Add this as a CPUID feature and skip flushing caches if the feature is present.

v2 -> v3:
        Patch# 2: Moves the addition of the CPUID feature from 
                  early_detect_mem_encrypt() to scattered.c.
        Patch# 3,4: These two are the split of patch# 3 from v2. Patch# 3
                 is for non[PATCH 0/4 v3] x86: AMD: Don't flush encrypted pages if hardware enforces cache coherency-SEV encryptions while patch#4 is for SEV
                 encryptions.

[PATCH 1/4 v3] x86: AMD: Replace numeric value for SME CPUID leaf with a
[PATCH 2/4 v3] x86: AMD: Add hardware-enforced cache coherency as a
[PATCH 3/4 v3] x86: AMD: Don't flush cache if hardware enforces cache
[PATCH 4/4 v3] KVM: SVM: Don't flush cache if hardware enforces cache

 arch/x86/boot/compressed/mem_encrypt.S | 5 +++--
 arch/x86/include/asm/cpufeatures.h     | 6 ++++++
 arch/x86/kernel/cpu/amd.c              | 2 +-
 arch/x86/kernel/cpu/scattered.c        | 5 +++--
 arch/x86/kvm/cpuid.c                   | 2 +-
 arch/x86/kvm/svm/sev.c                 | 3 ++-
 arch/x86/kvm/svm/svm.c                 | 4 ++--
 arch/x86/mm/mem_encrypt_identity.c     | 4 ++--
 arch/x86/mm/pat/set_memory.c           | 2 +-
 9 files changed, 21 insertions(+), 12 deletions(-)

Krish Sadhukhan (4):
      x86: AMD: Replace numeric value for SME CPUID leaf with a #define
      x86: AMD: Add hardware-enforced cache coherency as a CPUID feature
      x86: AMD: Don't flush cache if hardware enforces cache coherency across en
cryption domnains
      KVM: SVM: Don't flush cache if hardware enforces cache coherency across en
cryption domains


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/4 v3] x86: AMD: Replace numeric value for SME CPUID leaf with a #define
  2020-09-11 19:25 [PATCH 0/4 v3] x86: AMD: Don't flush cache if hardware enforces cache coherency across encryption domains Krish Sadhukhan
@ 2020-09-11 19:25 ` Krish Sadhukhan
  2020-09-11 21:21   ` Borislav Petkov
  2020-09-11 19:25 ` [PATCH 2/4 v3] x86: AMD: Add hardware-enforced cache coherency as a CPUID feature Krish Sadhukhan
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Krish Sadhukhan @ 2020-09-11 19:25 UTC (permalink / raw)
  To: kvm
  Cc: pbonzini, jmattson, tglx, mingo, bp, x86, sean.j.christopherson,
	vkuznets, wanpengli, joro, dave.hansen, luto, peterz,
	linux-kernel, hpa

Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
 arch/x86/boot/compressed/mem_encrypt.S | 5 +++--
 arch/x86/include/asm/cpufeatures.h     | 5 +++++
 arch/x86/kernel/cpu/amd.c              | 2 +-
 arch/x86/kernel/cpu/scattered.c        | 4 ++--
 arch/x86/kvm/cpuid.c                   | 2 +-
 arch/x86/kvm/svm/svm.c                 | 4 ++--
 arch/x86/mm/mem_encrypt_identity.c     | 4 ++--
 7 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/arch/x86/boot/compressed/mem_encrypt.S b/arch/x86/boot/compressed/mem_encrypt.S
index dd07e7b41b11..22e30b0c0d19 100644
--- a/arch/x86/boot/compressed/mem_encrypt.S
+++ b/arch/x86/boot/compressed/mem_encrypt.S
@@ -12,6 +12,7 @@
 #include <asm/processor-flags.h>
 #include <asm/msr.h>
 #include <asm/asm-offsets.h>
+#include <asm/cpufeatures.h>
 
 	.text
 	.code32
@@ -31,7 +32,7 @@ SYM_FUNC_START(get_sev_encryption_bit)
 
 	movl	$0x80000000, %eax	/* CPUID to check the highest leaf */
 	cpuid
-	cmpl	$0x8000001f, %eax	/* See if 0x8000001f is available */
+	cmpl	$CPUID_AMD_SME, %eax	/* See if 0x8000001f is available */
 	jb	.Lno_sev
 
 	/*
@@ -40,7 +41,7 @@ SYM_FUNC_START(get_sev_encryption_bit)
 	 *   CPUID Fn8000_001F[EBX] - Bits 5:0
 	 *     Pagetable bit position used to indicate encryption
 	 */
-	movl	$0x8000001f, %eax
+	movl	$CPUID_AMD_SME, %eax
 	cpuid
 	bt	$1, %eax		/* Check if SEV is available */
 	jnc	.Lno_sev
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 2901d5df4366..81335e6fe47d 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -10,6 +10,11 @@
 #include <asm/disabled-features.h>
 #endif
 
+/*
+ * AMD CPUID functions
+ */
+#define CPUID_AMD_SME  0x8000001f	/* Secure Memory Encryption */
+
 /*
  * Defines x86 CPU feature bits
  */
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index dcc3d943c68f..4507ededb978 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -630,7 +630,7 @@ static void early_detect_mem_encrypt(struct cpuinfo_x86 *c)
 		 * will be a value above 32-bits this is still done for
 		 * CONFIG_X86_32 so that accurate values are reported.
 		 */
-		c->x86_phys_bits -= (cpuid_ebx(0x8000001f) >> 6) & 0x3f;
+		c->x86_phys_bits -= (cpuid_ebx(CPUID_AMD_SME) >> 6) & 0x3f;
 
 		if (IS_ENABLED(CONFIG_X86_32))
 			goto clear_all;
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 62b137c3c97a..033c112e03fc 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -39,8 +39,8 @@ static const struct cpuid_bit cpuid_bits[] = {
 	{ X86_FEATURE_CPB,		CPUID_EDX,  9, 0x80000007, 0 },
 	{ X86_FEATURE_PROC_FEEDBACK,    CPUID_EDX, 11, 0x80000007, 0 },
 	{ X86_FEATURE_MBA,		CPUID_EBX,  6, 0x80000008, 0 },
-	{ X86_FEATURE_SME,		CPUID_EAX,  0, 0x8000001f, 0 },
-	{ X86_FEATURE_SEV,		CPUID_EAX,  1, 0x8000001f, 0 },
+	{ X86_FEATURE_SME,		CPUID_EAX,  0, CPUID_AMD_SME, 0 },
+	{ X86_FEATURE_SEV,		CPUID_EAX,  1, CPUID_AMD_SME, 0 },
 	{ 0, 0, 0, 0, 0 }
 };
 
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 3fd6eec202d7..95863e767d3d 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -756,7 +756,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
 		entry->edx = 0;
 		break;
 	case 0x80000000:
-		entry->eax = min(entry->eax, 0x8000001f);
+		entry->eax = min(entry->eax, CPUID_AMD_SME);
 		break;
 	case 0x80000001:
 		cpuid_entry_override(entry, CPUID_8000_0001_EDX);
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 0194336b64a4..a4e92ae399b4 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -749,7 +749,7 @@ static __init void svm_adjust_mmio_mask(void)
 	u64 msr, mask;
 
 	/* If there is no memory encryption support, use existing mask */
-	if (cpuid_eax(0x80000000) < 0x8000001f)
+	if (cpuid_eax(0x80000000) < CPUID_AMD_SME)
 		return;
 
 	/* If memory encryption is not enabled, use existing mask */
@@ -757,7 +757,7 @@ static __init void svm_adjust_mmio_mask(void)
 	if (!(msr & MSR_K8_SYSCFG_MEM_ENCRYPT))
 		return;
 
-	enc_bit = cpuid_ebx(0x8000001f) & 0x3f;
+	enc_bit = cpuid_ebx(CPUID_AMD_SME) & 0x3f;
 	mask_bit = boot_cpu_data.x86_phys_bits;
 
 	/* Increment the mask bit if it is the same as the encryption bit */
diff --git a/arch/x86/mm/mem_encrypt_identity.c b/arch/x86/mm/mem_encrypt_identity.c
index e2b0e2ac07bb..cbe600dd357b 100644
--- a/arch/x86/mm/mem_encrypt_identity.c
+++ b/arch/x86/mm/mem_encrypt_identity.c
@@ -498,7 +498,7 @@ void __init sme_enable(struct boot_params *bp)
 	eax = 0x80000000;
 	ecx = 0;
 	native_cpuid(&eax, &ebx, &ecx, &edx);
-	if (eax < 0x8000001f)
+	if (eax < CPUID_AMD_SME)
 		return;
 
 #define AMD_SME_BIT	BIT(0)
@@ -520,7 +520,7 @@ void __init sme_enable(struct boot_params *bp)
 	 *   CPUID Fn8000_001F[EBX]
 	 *   - Bits 5:0 - Pagetable bit position used to indicate encryption
 	 */
-	eax = 0x8000001f;
+	eax = CPUID_AMD_SME;
 	ecx = 0;
 	native_cpuid(&eax, &ebx, &ecx, &edx);
 	if (!(eax & feature_mask))
-- 
2.18.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/4 v3] x86: AMD: Add hardware-enforced cache coherency as a CPUID feature
  2020-09-11 19:25 [PATCH 0/4 v3] x86: AMD: Don't flush cache if hardware enforces cache coherency across encryption domains Krish Sadhukhan
  2020-09-11 19:25 ` [PATCH 1/4 v3] x86: AMD: Replace numeric value for SME CPUID leaf with a #define Krish Sadhukhan
@ 2020-09-11 19:25 ` Krish Sadhukhan
  2020-09-11 19:36   ` Dave Hansen
  2020-09-11 21:33   ` Borislav Petkov
  2020-09-11 19:26 ` [PATCH 3/4 v3] x86: AMD: Don't flush cache if hardware enforces cache coherency across encryption domnains Krish Sadhukhan
  2020-09-11 19:26 ` [PATCH 4/4 v3] KVM: SVM: Don't flush cache if hardware enforces cache coherency across encryption domains Krish Sadhukhan
  3 siblings, 2 replies; 12+ messages in thread
From: Krish Sadhukhan @ 2020-09-11 19:25 UTC (permalink / raw)
  To: kvm
  Cc: pbonzini, jmattson, tglx, mingo, bp, x86, sean.j.christopherson,
	vkuznets, wanpengli, joro, dave.hansen, luto, peterz,
	linux-kernel, hpa

In some hardware implementations, coherency between the encrypted and
unencrypted mappings of the same physical page is enforced. In such a system,
it is not required for software to flush the page from all CPU caches in the
system prior to changing the value of the C-bit for a page. This hardware-
enforced cache coherency is indicated by EAX[10] in CPUID leaf 0x8000001f.

Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
 arch/x86/include/asm/cpufeatures.h | 1 +
 arch/x86/kernel/cpu/scattered.c    | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 81335e6fe47d..0e5b27ee5931 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -293,6 +293,7 @@
 #define X86_FEATURE_FENCE_SWAPGS_USER	(11*32+ 4) /* "" LFENCE in user entry SWAPGS path */
 #define X86_FEATURE_FENCE_SWAPGS_KERNEL	(11*32+ 5) /* "" LFENCE in kernel entry SWAPGS path */
 #define X86_FEATURE_SPLIT_LOCK_DETECT	(11*32+ 6) /* #AC for split lock */
+#define X86_FEATURE_HW_CACHE_COHERENCY (11*32+ 7) /* AMD hardware-enforced cache coherency */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */
 #define X86_FEATURE_AVX512_BF16		(12*32+ 5) /* AVX512 BFLOAT16 instructions */
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 033c112e03fc..57394fee1d35 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -41,6 +41,7 @@ static const struct cpuid_bit cpuid_bits[] = {
 	{ X86_FEATURE_MBA,		CPUID_EBX,  6, 0x80000008, 0 },
 	{ X86_FEATURE_SME,		CPUID_EAX,  0, CPUID_AMD_SME, 0 },
 	{ X86_FEATURE_SEV,		CPUID_EAX,  1, CPUID_AMD_SME, 0 },
+	{ X86_FEATURE_HW_CACHE_COHERENCY, CPUID_EAX,  10, CPUID_AMD_SME, 0 },
 	{ 0, 0, 0, 0, 0 }
 };
 
-- 
2.18.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/4 v3] x86: AMD: Don't flush cache if hardware enforces cache coherency across encryption domnains
  2020-09-11 19:25 [PATCH 0/4 v3] x86: AMD: Don't flush cache if hardware enforces cache coherency across encryption domains Krish Sadhukhan
  2020-09-11 19:25 ` [PATCH 1/4 v3] x86: AMD: Replace numeric value for SME CPUID leaf with a #define Krish Sadhukhan
  2020-09-11 19:25 ` [PATCH 2/4 v3] x86: AMD: Add hardware-enforced cache coherency as a CPUID feature Krish Sadhukhan
@ 2020-09-11 19:26 ` Krish Sadhukhan
  2020-09-11 19:26 ` [PATCH 4/4 v3] KVM: SVM: Don't flush cache if hardware enforces cache coherency across encryption domains Krish Sadhukhan
  3 siblings, 0 replies; 12+ messages in thread
From: Krish Sadhukhan @ 2020-09-11 19:26 UTC (permalink / raw)
  To: kvm
  Cc: pbonzini, jmattson, tglx, mingo, bp, x86, sean.j.christopherson,
	vkuznets, wanpengli, joro, dave.hansen, luto, peterz,
	linux-kernel, hpa

In some hardware implementations, coherency between the encrypted and
unencrypted mappings of the same physical page is enforced. In such a system,
it is not required for software to flush the page from all CPU caches in the
system prior to changing the value of the C-bit for the page.

Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
 arch/x86/mm/pat/set_memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index d1b2a889f035..78d5511c5edd 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -1999,7 +1999,7 @@ static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc)
 	/*
 	 * Before changing the encryption attribute, we need to flush caches.
 	 */
-	cpa_flush(&cpa, 1);
+	cpa_flush(&cpa, !this_cpu_has(X86_FEATURE_HW_CACHE_COHERENCY));
 
 	ret = __change_page_attr_set_clr(&cpa, 1);
 
-- 
2.18.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/4 v3] KVM: SVM: Don't flush cache if hardware enforces cache coherency across encryption domains
  2020-09-11 19:25 [PATCH 0/4 v3] x86: AMD: Don't flush cache if hardware enforces cache coherency across encryption domains Krish Sadhukhan
                   ` (2 preceding siblings ...)
  2020-09-11 19:26 ` [PATCH 3/4 v3] x86: AMD: Don't flush cache if hardware enforces cache coherency across encryption domnains Krish Sadhukhan
@ 2020-09-11 19:26 ` Krish Sadhukhan
  3 siblings, 0 replies; 12+ messages in thread
From: Krish Sadhukhan @ 2020-09-11 19:26 UTC (permalink / raw)
  To: kvm
  Cc: pbonzini, jmattson, tglx, mingo, bp, x86, sean.j.christopherson,
	vkuznets, wanpengli, joro, dave.hansen, luto, peterz,
	linux-kernel, hpa

In some hardware implementations, coherency between the encrypted and
unencrypted mappings of the same physical page in a VM is enforced. In such a
system, it is not required for software to flush the VM's page from all CPU
caches in the system prior to changing the value of the C-bit for the page.

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..8aa2209f2637 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 (this_cpu_has(X86_FEATURE_HW_CACHE_COHERENCY) || npages == 0 ||
+	    pages == NULL)
 		return;
 
 	for (i = 0; i < npages; i++) {
-- 
2.18.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4 v3] x86: AMD: Add hardware-enforced cache coherency as a CPUID feature
  2020-09-11 19:25 ` [PATCH 2/4 v3] x86: AMD: Add hardware-enforced cache coherency as a CPUID feature Krish Sadhukhan
@ 2020-09-11 19:36   ` Dave Hansen
  2020-09-11 20:10     ` Krish Sadhukhan
  2020-09-11 21:33   ` Borislav Petkov
  1 sibling, 1 reply; 12+ messages in thread
From: Dave Hansen @ 2020-09-11 19:36 UTC (permalink / raw)
  To: Krish Sadhukhan, kvm
  Cc: pbonzini, jmattson, tglx, mingo, bp, x86, sean.j.christopherson,
	vkuznets, wanpengli, joro, dave.hansen, luto, peterz,
	linux-kernel, hpa

On 9/11/20 12:25 PM, Krish Sadhukhan wrote:
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 81335e6fe47d..0e5b27ee5931 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -293,6 +293,7 @@
>  #define X86_FEATURE_FENCE_SWAPGS_USER	(11*32+ 4) /* "" LFENCE in user entry SWAPGS path */
>  #define X86_FEATURE_FENCE_SWAPGS_KERNEL	(11*32+ 5) /* "" LFENCE in kernel entry SWAPGS path */
>  #define X86_FEATURE_SPLIT_LOCK_DETECT	(11*32+ 6) /* #AC for split lock */
> +#define X86_FEATURE_HW_CACHE_COHERENCY (11*32+ 7) /* AMD hardware-enforced cache coherency */

That's an awfully generic name.  We generally have "hardware-enforced
cache coherency" already everywhere. :)

This probably needs to say something about encryption, or even SEV
specifically.  I also don't see this bit in the "AMD64 Architecture
Programmer’s Manual".  Did I look in the wrong spot somehow?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4 v3] x86: AMD: Add hardware-enforced cache coherency as a CPUID feature
  2020-09-11 19:36   ` Dave Hansen
@ 2020-09-11 20:10     ` Krish Sadhukhan
  2020-09-11 20:58       ` Dave Hansen
  0 siblings, 1 reply; 12+ messages in thread
From: Krish Sadhukhan @ 2020-09-11 20:10 UTC (permalink / raw)
  To: Dave Hansen, kvm
  Cc: pbonzini, jmattson, tglx, mingo, bp, x86, sean.j.christopherson,
	vkuznets, wanpengli, joro, dave.hansen, luto, peterz,
	linux-kernel, hpa


On 9/11/20 12:36 PM, Dave Hansen wrote:
> On 9/11/20 12:25 PM, Krish Sadhukhan wrote:
>> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
>> index 81335e6fe47d..0e5b27ee5931 100644
>> --- a/arch/x86/include/asm/cpufeatures.h
>> +++ b/arch/x86/include/asm/cpufeatures.h
>> @@ -293,6 +293,7 @@
>>   #define X86_FEATURE_FENCE_SWAPGS_USER	(11*32+ 4) /* "" LFENCE in user entry SWAPGS path */
>>   #define X86_FEATURE_FENCE_SWAPGS_KERNEL	(11*32+ 5) /* "" LFENCE in kernel entry SWAPGS path */
>>   #define X86_FEATURE_SPLIT_LOCK_DETECT	(11*32+ 6) /* #AC for split lock */
>> +#define X86_FEATURE_HW_CACHE_COHERENCY (11*32+ 7) /* AMD hardware-enforced cache coherency */
> That's an awfully generic name.  We generally have "hardware-enforced
> cache coherency" already everywhere. :)
>
> This probably needs to say something about encryption, or even SEV
> specifically.


How about X86_FEATURE_ENC_CACHE_COHERENCY ?

> I also don't see this bit in the "AMD64 Architecture
> Programmer’s Manual".  Did I look in the wrong spot somehow?
Section 7.10.6 in APM mentions this.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4 v3] x86: AMD: Add hardware-enforced cache coherency as a CPUID feature
  2020-09-11 20:10     ` Krish Sadhukhan
@ 2020-09-11 20:58       ` Dave Hansen
  0 siblings, 0 replies; 12+ messages in thread
From: Dave Hansen @ 2020-09-11 20:58 UTC (permalink / raw)
  To: Krish Sadhukhan, kvm
  Cc: pbonzini, jmattson, tglx, mingo, bp, x86, sean.j.christopherson,
	vkuznets, wanpengli, joro, dave.hansen, luto, peterz,
	linux-kernel, hpa

On 9/11/20 1:10 PM, Krish Sadhukhan wrote:
...
>>> +#define X86_FEATURE_HW_CACHE_COHERENCY (11*32+ 7) /* AMD
>>> hardware-enforced cache coherency */
>> That's an awfully generic name.  We generally have "hardware-enforced
>> cache coherency" already everywhere. :)
>>
>> This probably needs to say something about encryption, or even SEV
>> specifically.
> 
> How about X86_FEATURE_ENC_CACHE_COHERENCY ?

I think X86_FEATURE_SME_COHERENT would be the most appropriate name.
That bit, as defined, looks totally specific to SME.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4 v3] x86: AMD: Replace numeric value for SME CPUID leaf with a #define
  2020-09-11 19:25 ` [PATCH 1/4 v3] x86: AMD: Replace numeric value for SME CPUID leaf with a #define Krish Sadhukhan
@ 2020-09-11 21:21   ` Borislav Petkov
  2020-09-12  6:54     ` Paolo Bonzini
  0 siblings, 1 reply; 12+ messages in thread
From: Borislav Petkov @ 2020-09-11 21:21 UTC (permalink / raw)
  To: Krish Sadhukhan
  Cc: kvm, pbonzini, jmattson, tglx, mingo, x86, sean.j.christopherson,
	vkuznets, wanpengli, joro, dave.hansen, luto, peterz,
	linux-kernel, hpa

On Fri, Sep 11, 2020 at 07:25:58PM +0000, Krish Sadhukhan wrote:

<-- patches need commit message.

...

> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
> index 62b137c3c97a..033c112e03fc 100644
> --- a/arch/x86/kernel/cpu/scattered.c
> +++ b/arch/x86/kernel/cpu/scattered.c
> @@ -39,8 +39,8 @@ static const struct cpuid_bit cpuid_bits[] = {
>  	{ X86_FEATURE_CPB,		CPUID_EDX,  9, 0x80000007, 0 },
>  	{ X86_FEATURE_PROC_FEEDBACK,    CPUID_EDX, 11, 0x80000007, 0 },
>  	{ X86_FEATURE_MBA,		CPUID_EBX,  6, 0x80000008, 0 },
> -	{ X86_FEATURE_SME,		CPUID_EAX,  0, 0x8000001f, 0 },
> -	{ X86_FEATURE_SEV,		CPUID_EAX,  1, 0x8000001f, 0 },
> +	{ X86_FEATURE_SME,		CPUID_EAX,  0, CPUID_AMD_SME, 0 },
> +	{ X86_FEATURE_SEV,		CPUID_EAX,  1, CPUID_AMD_SME, 0 },

So this one gets a name and all the others above don't?

This fact should've given you a hint that there's no need for naming
CPUID leafs - it is easier to grep CPU manuals by the values so you can
drop this patch.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4 v3] x86: AMD: Add hardware-enforced cache coherency as a CPUID feature
  2020-09-11 19:25 ` [PATCH 2/4 v3] x86: AMD: Add hardware-enforced cache coherency as a CPUID feature Krish Sadhukhan
  2020-09-11 19:36   ` Dave Hansen
@ 2020-09-11 21:33   ` Borislav Petkov
  2020-09-11 21:44     ` Tom Lendacky
  1 sibling, 1 reply; 12+ messages in thread
From: Borislav Petkov @ 2020-09-11 21:33 UTC (permalink / raw)
  To: Krish Sadhukhan
  Cc: kvm, pbonzini, jmattson, tglx, mingo, x86, sean.j.christopherson,
	vkuznets, wanpengli, joro, dave.hansen, luto, peterz,
	linux-kernel, hpa, Tom Lendacky

+ Tom.

On Fri, Sep 11, 2020 at 07:25:59PM +0000, Krish Sadhukhan wrote:
> +#define X86_FEATURE_HW_CACHE_COHERENCY (11*32+ 7) /* AMD hardware-enforced cache coherency */

so before you guys paint the bikeshed all kinds of colors :), Tom (CCed)
is digging out the official name. (If it is even uglier, we might keep
on bikeshedding...).

Once you have that, add the "" after the comment - like
X86_FEATURE_FENCE_SWAPGS_USER, for example, so that it doesn't show in
/proc/cpuinfo as luserspace doesn't care about hw coherency between enc
memory.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4 v3] x86: AMD: Add hardware-enforced cache coherency as a CPUID feature
  2020-09-11 21:33   ` Borislav Petkov
@ 2020-09-11 21:44     ` Tom Lendacky
  0 siblings, 0 replies; 12+ messages in thread
From: Tom Lendacky @ 2020-09-11 21:44 UTC (permalink / raw)
  To: Borislav Petkov, Krish Sadhukhan
  Cc: kvm, pbonzini, jmattson, tglx, mingo, x86, sean.j.christopherson,
	vkuznets, wanpengli, joro, dave.hansen, luto, peterz,
	linux-kernel, hpa

On 9/11/20 4:33 PM, Borislav Petkov wrote:
> + Tom.
> 
> On Fri, Sep 11, 2020 at 07:25:59PM +0000, Krish Sadhukhan wrote:
>> +#define X86_FEATURE_HW_CACHE_COHERENCY (11*32+ 7) /* AMD hardware-enforced cache coherency */
> 
> so before you guys paint the bikeshed all kinds of colors :), Tom (CCed)
> is digging out the official name. (If it is even uglier, we might keep
> on bikeshedding...).

I believe the official name is something like CoherencyEnforced. Since 
it's under the 0x8000001f leaf (AMD Secure Encryption), it means that 
coherency is enforced between the same physical address when referenced 
with or without the encryption bit (bare metal or in a guest).

So I kind of like the X86_FEATURE_SME_COHERENT suggestion by Dave, even if 
it also applies to SEV.

Thanks,
Tom

> 
> Once you have that, add the "" after the comment - like
> X86_FEATURE_FENCE_SWAPGS_USER, for example, so that it doesn't show in
> /proc/cpuinfo as luserspace doesn't care about hw coherency between enc
> memory.
> 
> Thx.
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4 v3] x86: AMD: Replace numeric value for SME CPUID leaf with a #define
  2020-09-11 21:21   ` Borislav Petkov
@ 2020-09-12  6:54     ` Paolo Bonzini
  0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2020-09-12  6:54 UTC (permalink / raw)
  To: Borislav Petkov, Krish Sadhukhan
  Cc: kvm, jmattson, tglx, mingo, x86, sean.j.christopherson, vkuznets,
	wanpengli, joro, dave.hansen, luto, peterz, linux-kernel, hpa

On 11/09/20 23:21, Borislav Petkov wrote:
> On Fri, Sep 11, 2020 at 07:25:58PM +0000, Krish Sadhukhan wrote:
> 
> <-- patches need commit message.
> 
> ...
> 
>> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
>> index 62b137c3c97a..033c112e03fc 100644
>> --- a/arch/x86/kernel/cpu/scattered.c
>> +++ b/arch/x86/kernel/cpu/scattered.c
>> @@ -39,8 +39,8 @@ static const struct cpuid_bit cpuid_bits[] = {
>>  	{ X86_FEATURE_CPB,		CPUID_EDX,  9, 0x80000007, 0 },
>>  	{ X86_FEATURE_PROC_FEEDBACK,    CPUID_EDX, 11, 0x80000007, 0 },
>>  	{ X86_FEATURE_MBA,		CPUID_EBX,  6, 0x80000008, 0 },
>> -	{ X86_FEATURE_SME,		CPUID_EAX,  0, 0x8000001f, 0 },
>> -	{ X86_FEATURE_SEV,		CPUID_EAX,  1, 0x8000001f, 0 },
>> +	{ X86_FEATURE_SME,		CPUID_EAX,  0, CPUID_AMD_SME, 0 },
>> +	{ X86_FEATURE_SEV,		CPUID_EAX,  1, CPUID_AMD_SME, 0 },
> 
> So this one gets a name and all the others above don't?
> 
> This fact should've given you a hint that there's no need for naming
> CPUID leafs - it is easier to grep CPU manuals by the values so you can
> drop this patch.

Also, there'd be confusion between the CPUID_* enum in
arch/x86/include/asm/cpufeature.h.

Paolo


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-09-12  6:54 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-11 19:25 [PATCH 0/4 v3] x86: AMD: Don't flush cache if hardware enforces cache coherency across encryption domains Krish Sadhukhan
2020-09-11 19:25 ` [PATCH 1/4 v3] x86: AMD: Replace numeric value for SME CPUID leaf with a #define Krish Sadhukhan
2020-09-11 21:21   ` Borislav Petkov
2020-09-12  6:54     ` Paolo Bonzini
2020-09-11 19:25 ` [PATCH 2/4 v3] x86: AMD: Add hardware-enforced cache coherency as a CPUID feature Krish Sadhukhan
2020-09-11 19:36   ` Dave Hansen
2020-09-11 20:10     ` Krish Sadhukhan
2020-09-11 20:58       ` Dave Hansen
2020-09-11 21:33   ` Borislav Petkov
2020-09-11 21:44     ` Tom Lendacky
2020-09-11 19:26 ` [PATCH 3/4 v3] x86: AMD: Don't flush cache if hardware enforces cache coherency across encryption domnains Krish Sadhukhan
2020-09-11 19:26 ` [PATCH 4/4 v3] KVM: SVM: Don't flush cache if hardware enforces cache coherency across encryption domains Krish Sadhukhan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.