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 <[email protected]>
Signed-off-by: Krish Sadhukhan <[email protected]>
---
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
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?
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.
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.
+ 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
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.
>