linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv2] x86/mm: Fix memory encryption features advertisement
@ 2024-01-11 11:12 Kirill A. Shutemov
  2024-01-11 14:19 ` Kuppuswamy Sathyanarayanan
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Kirill A. Shutemov @ 2024-01-11 11:12 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen
  Cc: x86, H. Peter Anvin, Tom Lendacky, linux-coco, linux-kernel,
	Kirill A. Shutemov, Dexuan Cui, Jeremi Piotrowski

When memory encryption is enabled, the kernel prints the encryption
flavor that the system supports.

The check assumes that everything is AMD SME/SEV if it doesn't have
the TDX CPU feature set.

Hyper-V vTOM sets cc_vendor to CC_VENDOR_INTEL when it runs as L2 guest
on top of TDX, but not X86_FEATURE_TDX_GUEST. Hyper-V only needs memory
encryption enabled for I/O without the rest of CoCo enabling.

To avoid confusion, check the cc_vendor directly.

Possible alternative is to completely removing the print statement.
For a regular TDX guest, the kernel already prints a message indicating
that it is booting on TDX. Similarly, AMD and Hyper-V can also display
a message during their enumeration process.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Dexuan Cui <decui@microsoft.com>
Cc: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>
---
 arch/x86/mm/mem_encrypt.c | 56 +++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index c290c55b632b..d035bce3a2b0 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -42,38 +42,42 @@ bool force_dma_unencrypted(struct device *dev)
 
 static void print_mem_encrypt_feature_info(void)
 {
-	pr_info("Memory Encryption Features active:");
+	pr_info("Memory Encryption Features active: ");
 
-	if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST)) {
-		pr_cont(" Intel TDX\n");
-		return;
-	}
+	switch (cc_vendor) {
+	case CC_VENDOR_INTEL:
+		pr_cont("Intel TDX\n");
+		break;
+	case CC_VENDOR_AMD:
+		pr_cont("AMD");
 
-	pr_cont(" AMD");
-
-	/* Secure Memory Encryption */
-	if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
+		/* Secure Memory Encryption */
+		if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
 		/*
 		 * SME is mutually exclusive with any of the SEV
 		 * features below.
-		 */
-		pr_cont(" SME\n");
-		return;
+		*/
+			pr_cont(" SME\n");
+			return;
+		}
+
+		/* Secure Encrypted Virtualization */
+		if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
+			pr_cont(" SEV");
+
+		/* Encrypted Register State */
+		if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
+			pr_cont(" SEV-ES");
+
+		/* Secure Nested Paging */
+		if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
+			pr_cont(" SEV-SNP");
+
+		pr_cont("\n");
+		break;
+	default:
+		pr_cont("Unknown\n");
 	}
-
-	/* Secure Encrypted Virtualization */
-	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
-		pr_cont(" SEV");
-
-	/* Encrypted Register State */
-	if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
-		pr_cont(" SEV-ES");
-
-	/* Secure Nested Paging */
-	if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
-		pr_cont(" SEV-SNP");
-
-	pr_cont("\n");
 }
 
 /* Architecture __weak replacement functions */
-- 
2.41.0


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

* Re: [PATCHv2] x86/mm: Fix memory encryption features advertisement
  2024-01-11 11:12 [PATCHv2] x86/mm: Fix memory encryption features advertisement Kirill A. Shutemov
@ 2024-01-11 14:19 ` Kuppuswamy Sathyanarayanan
  2024-01-11 15:14   ` Jeremi Piotrowski
  2024-01-11 20:46   ` Kuppuswamy Sathyanarayanan
  2024-01-11 20:41 ` Tom Lendacky
  2024-01-16 10:36 ` Huang, Kai
  2 siblings, 2 replies; 8+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2024-01-11 14:19 UTC (permalink / raw)
  To: Kirill A. Shutemov, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen
  Cc: x86, H. Peter Anvin, Tom Lendacky, linux-coco, linux-kernel,
	Dexuan Cui, Jeremi Piotrowski



On 1/11/2024 3:12 AM, Kirill A. Shutemov wrote:
> When memory encryption is enabled, the kernel prints the encryption
> flavor that the system supports.
> 
> The check assumes that everything is AMD SME/SEV if it doesn't have
> the TDX CPU feature set.
> 
> Hyper-V vTOM sets cc_vendor to CC_VENDOR_INTEL when it runs as L2 guest
> on top of TDX, but not X86_FEATURE_TDX_GUEST. Hyper-V only needs memory
> encryption enabled for I/O without the rest of CoCo enabling.
> 
> To avoid confusion, check the cc_vendor directly.
> 
> Possible alternative is to completely removing the print statement.
> For a regular TDX guest, the kernel already prints a message indicating
> that it is booting on TDX. Similarly, AMD and Hyper-V can also display
> a message during their enumeration process.

With this change, will it print "Intel TDX" for Hyper-V?

IMO, since there is already a debug message for type identification, we
can remove this part. 

> 
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Dexuan Cui <decui@microsoft.com>
> Cc: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>
> ---
>  arch/x86/mm/mem_encrypt.c | 56 +++++++++++++++++++++------------------
>  1 file changed, 30 insertions(+), 26 deletions(-)
> 
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index c290c55b632b..d035bce3a2b0 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -42,38 +42,42 @@ bool force_dma_unencrypted(struct device *dev)
>  
>  static void print_mem_encrypt_feature_info(void)
>  {
> -	pr_info("Memory Encryption Features active:");
> +	pr_info("Memory Encryption Features active: ");
>  
> -	if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST)) {
> -		pr_cont(" Intel TDX\n");
> -		return;
> -	}
> +	switch (cc_vendor) {
> +	case CC_VENDOR_INTEL:
> +		pr_cont("Intel TDX\n");
> +		break;
> +	case CC_VENDOR_AMD:
> +		pr_cont("AMD");
>  
> -	pr_cont(" AMD");
> -
> -	/* Secure Memory Encryption */
> -	if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
> +		/* Secure Memory Encryption */
> +		if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
>  		/*
>  		 * SME is mutually exclusive with any of the SEV
>  		 * features below.
> -		 */
> -		pr_cont(" SME\n");
> -		return;
> +		*/
> +			pr_cont(" SME\n");
> +			return;
> +		}
> +
> +		/* Secure Encrypted Virtualization */
> +		if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
> +			pr_cont(" SEV");
> +
> +		/* Encrypted Register State */
> +		if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
> +			pr_cont(" SEV-ES");
> +
> +		/* Secure Nested Paging */
> +		if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
> +			pr_cont(" SEV-SNP");
> +
> +		pr_cont("\n");
> +		break;
> +	default:
> +		pr_cont("Unknown\n");
>  	}
> -
> -	/* Secure Encrypted Virtualization */
> -	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
> -		pr_cont(" SEV");
> -
> -	/* Encrypted Register State */
> -	if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
> -		pr_cont(" SEV-ES");
> -
> -	/* Secure Nested Paging */
> -	if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
> -		pr_cont(" SEV-SNP");
> -
> -	pr_cont("\n");
>  }
>  
>  /* Architecture __weak replacement functions */

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

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

* Re: [PATCHv2] x86/mm: Fix memory encryption features advertisement
  2024-01-11 14:19 ` Kuppuswamy Sathyanarayanan
@ 2024-01-11 15:14   ` Jeremi Piotrowski
  2024-01-11 20:46   ` Kuppuswamy Sathyanarayanan
  1 sibling, 0 replies; 8+ messages in thread
From: Jeremi Piotrowski @ 2024-01-11 15:14 UTC (permalink / raw)
  To: Kuppuswamy Sathyanarayanan, Kirill A. Shutemov, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen
  Cc: x86, H. Peter Anvin, Tom Lendacky, linux-coco, linux-kernel, Dexuan Cui

On 11/01/2024 15:19, Kuppuswamy Sathyanarayanan wrote:
> 
> 
> On 1/11/2024 3:12 AM, Kirill A. Shutemov wrote:
>> When memory encryption is enabled, the kernel prints the encryption
>> flavor that the system supports.
>>
>> The check assumes that everything is AMD SME/SEV if it doesn't have
>> the TDX CPU feature set.
>>
>> Hyper-V vTOM sets cc_vendor to CC_VENDOR_INTEL when it runs as L2 guest
>> on top of TDX, but not X86_FEATURE_TDX_GUEST. Hyper-V only needs memory
>> encryption enabled for I/O without the rest of CoCo enabling.
>>
>> To avoid confusion, check the cc_vendor directly.
>>
>> Possible alternative is to completely removing the print statement.
>> For a regular TDX guest, the kernel already prints a message indicating
>> that it is booting on TDX. Similarly, AMD and Hyper-V can also display
>> a message during their enumeration process.
> 
> With this change, will it print "Intel TDX" for Hyper-V?

Yes, I just tested on AMD and Intel and the print is accurate now. Thanks.

Reviewed-by: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>

> 
> IMO, since there is already a debug message for type identification, we
> can remove this part. 
> 

If that's the only way to get a fix merged then so be it, but I appreciate having
the possibility of greping for a single prefix for either vendor that the current
code provides.

>>
>> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>> Cc: Dexuan Cui <decui@microsoft.com>
>> Cc: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>
>> ---
>>  arch/x86/mm/mem_encrypt.c | 56 +++++++++++++++++++++------------------
>>  1 file changed, 30 insertions(+), 26 deletions(-)
>>
>> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
>> index c290c55b632b..d035bce3a2b0 100644
>> --- a/arch/x86/mm/mem_encrypt.c
>> +++ b/arch/x86/mm/mem_encrypt.c
>> @@ -42,38 +42,42 @@ bool force_dma_unencrypted(struct device *dev)
>>  
>>  static void print_mem_encrypt_feature_info(void)
>>  {
>> -	pr_info("Memory Encryption Features active:");
>> +	pr_info("Memory Encryption Features active: ");
>>  
>> -	if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST)) {
>> -		pr_cont(" Intel TDX\n");
>> -		return;
>> -	}
>> +	switch (cc_vendor) {
>> +	case CC_VENDOR_INTEL:
>> +		pr_cont("Intel TDX\n");
>> +		break;
>> +	case CC_VENDOR_AMD:
>> +		pr_cont("AMD");
>>  
>> -	pr_cont(" AMD");
>> -
>> -	/* Secure Memory Encryption */
>> -	if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
>> +		/* Secure Memory Encryption */
>> +		if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
>>  		/*
>>  		 * SME is mutually exclusive with any of the SEV
>>  		 * features below.
>> -		 */
>> -		pr_cont(" SME\n");
>> -		return;
>> +		*/
>> +			pr_cont(" SME\n");
>> +			return;
>> +		}
>> +
>> +		/* Secure Encrypted Virtualization */
>> +		if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
>> +			pr_cont(" SEV");
>> +
>> +		/* Encrypted Register State */
>> +		if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
>> +			pr_cont(" SEV-ES");
>> +
>> +		/* Secure Nested Paging */
>> +		if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
>> +			pr_cont(" SEV-SNP");
>> +
>> +		pr_cont("\n");
>> +		break;
>> +	default:
>> +		pr_cont("Unknown\n");
>>  	}
>> -
>> -	/* Secure Encrypted Virtualization */
>> -	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
>> -		pr_cont(" SEV");
>> -
>> -	/* Encrypted Register State */
>> -	if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
>> -		pr_cont(" SEV-ES");
>> -
>> -	/* Secure Nested Paging */
>> -	if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
>> -		pr_cont(" SEV-SNP");
>> -
>> -	pr_cont("\n");
>>  }
>>  
>>  /* Architecture __weak replacement functions */
>

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

* Re: [PATCHv2] x86/mm: Fix memory encryption features advertisement
  2024-01-11 11:12 [PATCHv2] x86/mm: Fix memory encryption features advertisement Kirill A. Shutemov
  2024-01-11 14:19 ` Kuppuswamy Sathyanarayanan
@ 2024-01-11 20:41 ` Tom Lendacky
  2024-01-16 10:36 ` Huang, Kai
  2 siblings, 0 replies; 8+ messages in thread
From: Tom Lendacky @ 2024-01-11 20:41 UTC (permalink / raw)
  To: Kirill A. Shutemov, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen
  Cc: x86, H. Peter Anvin, linux-coco, linux-kernel, Dexuan Cui,
	Jeremi Piotrowski

On 1/11/24 05:12, Kirill A. Shutemov wrote:
> When memory encryption is enabled, the kernel prints the encryption
> flavor that the system supports.
> 
> The check assumes that everything is AMD SME/SEV if it doesn't have
> the TDX CPU feature set.
> 
> Hyper-V vTOM sets cc_vendor to CC_VENDOR_INTEL when it runs as L2 guest
> on top of TDX, but not X86_FEATURE_TDX_GUEST. Hyper-V only needs memory
> encryption enabled for I/O without the rest of CoCo enabling.
> 
> To avoid confusion, check the cc_vendor directly.
> 
> Possible alternative is to completely removing the print statement.
> For a regular TDX guest, the kernel already prints a message indicating
> that it is booting on TDX. Similarly, AMD and Hyper-V can also display
> a message during their enumeration process.
> 
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Dexuan Cui <decui@microsoft.com>
> Cc: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>

Acked-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>   arch/x86/mm/mem_encrypt.c | 56 +++++++++++++++++++++------------------
>   1 file changed, 30 insertions(+), 26 deletions(-)
> 
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c

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

* Re: [PATCHv2] x86/mm: Fix memory encryption features advertisement
  2024-01-11 14:19 ` Kuppuswamy Sathyanarayanan
  2024-01-11 15:14   ` Jeremi Piotrowski
@ 2024-01-11 20:46   ` Kuppuswamy Sathyanarayanan
  1 sibling, 0 replies; 8+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2024-01-11 20:46 UTC (permalink / raw)
  To: Kirill A. Shutemov, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen
  Cc: x86, H. Peter Anvin, Tom Lendacky, linux-coco, linux-kernel,
	Dexuan Cui, Jeremi Piotrowski



On 1/11/2024 6:19 AM, Kuppuswamy Sathyanarayanan wrote:
> 
> 
> On 1/11/2024 3:12 AM, Kirill A. Shutemov wrote:
>> When memory encryption is enabled, the kernel prints the encryption
>> flavor that the system supports.
>>
>> The check assumes that everything is AMD SME/SEV if it doesn't have
>> the TDX CPU feature set.
>>
>> Hyper-V vTOM sets cc_vendor to CC_VENDOR_INTEL when it runs as L2 guest
>> on top of TDX, but not X86_FEATURE_TDX_GUEST. Hyper-V only needs memory
>> encryption enabled for I/O without the rest of CoCo enabling.
>>
>> To avoid confusion, check the cc_vendor directly.
>>
>> Possible alternative is to completely removing the print statement.
>> For a regular TDX guest, the kernel already prints a message indicating
>> that it is booting on TDX. Similarly, AMD and Hyper-V can also display
>> a message during their enumeration process.
> 
> With this change, will it print "Intel TDX" for Hyper-V?
> 
> IMO, since there is already a debug message for type identification, we
> can remove this part. 
> 
>>
>> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>> Cc: Dexuan Cui <decui@microsoft.com>
>> Cc: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>
>> ---

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

>>  arch/x86/mm/mem_encrypt.c | 56 +++++++++++++++++++++------------------
>>  1 file changed, 30 insertions(+), 26 deletions(-)
>>
>> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
>> index c290c55b632b..d035bce3a2b0 100644
>> --- a/arch/x86/mm/mem_encrypt.c
>> +++ b/arch/x86/mm/mem_encrypt.c
>> @@ -42,38 +42,42 @@ bool force_dma_unencrypted(struct device *dev)
>>  
>>  static void print_mem_encrypt_feature_info(void)
>>  {
>> -	pr_info("Memory Encryption Features active:");
>> +	pr_info("Memory Encryption Features active: ");
>>  
>> -	if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST)) {
>> -		pr_cont(" Intel TDX\n");
>> -		return;
>> -	}
>> +	switch (cc_vendor) {
>> +	case CC_VENDOR_INTEL:
>> +		pr_cont("Intel TDX\n");
>> +		break;
>> +	case CC_VENDOR_AMD:
>> +		pr_cont("AMD");
>>  
>> -	pr_cont(" AMD");
>> -
>> -	/* Secure Memory Encryption */
>> -	if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
>> +		/* Secure Memory Encryption */
>> +		if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
>>  		/*
>>  		 * SME is mutually exclusive with any of the SEV
>>  		 * features below.
>> -		 */
>> -		pr_cont(" SME\n");
>> -		return;
>> +		*/
>> +			pr_cont(" SME\n");
>> +			return;
>> +		}
>> +
>> +		/* Secure Encrypted Virtualization */
>> +		if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
>> +			pr_cont(" SEV");
>> +
>> +		/* Encrypted Register State */
>> +		if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
>> +			pr_cont(" SEV-ES");
>> +
>> +		/* Secure Nested Paging */
>> +		if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
>> +			pr_cont(" SEV-SNP");
>> +
>> +		pr_cont("\n");
>> +		break;
>> +	default:
>> +		pr_cont("Unknown\n");
>>  	}
>> -
>> -	/* Secure Encrypted Virtualization */
>> -	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
>> -		pr_cont(" SEV");
>> -
>> -	/* Encrypted Register State */
>> -	if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
>> -		pr_cont(" SEV-ES");
>> -
>> -	/* Secure Nested Paging */
>> -	if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
>> -		pr_cont(" SEV-SNP");
>> -
>> -	pr_cont("\n");
>>  }
>>  
>>  /* Architecture __weak replacement functions */
> 

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

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

* Re: [PATCHv2] x86/mm: Fix memory encryption features advertisement
  2024-01-11 11:12 [PATCHv2] x86/mm: Fix memory encryption features advertisement Kirill A. Shutemov
  2024-01-11 14:19 ` Kuppuswamy Sathyanarayanan
  2024-01-11 20:41 ` Tom Lendacky
@ 2024-01-16 10:36 ` Huang, Kai
  2024-01-16 10:58   ` kirill.shutemov
  2 siblings, 1 reply; 8+ messages in thread
From: Huang, Kai @ 2024-01-16 10:36 UTC (permalink / raw)
  To: kirill.shutemov, tglx, mingo, bp, dave.hansen
  Cc: hpa, thomas.lendacky, linux-coco, jpiotrowski, linux-kernel, Cui,
	Dexuan, x86

On Thu, 2024-01-11 at 14:12 +0300, Kirill A. Shutemov wrote:
> When memory encryption is enabled, the kernel prints the encryption
> flavor that the system supports.
> 
> The check assumes that everything is AMD SME/SEV if it doesn't have
> the TDX CPU feature set.
> 
> Hyper-V vTOM sets cc_vendor to CC_VENDOR_INTEL when it runs as L2 guest
> on top of TDX, but not X86_FEATURE_TDX_GUEST. Hyper-V only needs memory
> encryption enabled for I/O without the rest of CoCo enabling.
> 
> To avoid confusion, check the cc_vendor directly.
> 
> Possible alternative is to completely removing the print statement.
> For a regular TDX guest, the kernel already prints a message indicating
> that it is booting on TDX. Similarly, AMD and Hyper-V can also display
> a message during their enumeration process.
> 
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Dexuan Cui <decui@microsoft.com>
> Cc: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>

Seems this fix is for userspace running in hyperv environment being able to use
some easy grep to get which coco vendor it is running on?

If so I think it would be nice to mention it too.

Acked-by: Kai Huang <kai.huang@intel.com>

> ---
>  arch/x86/mm/mem_encrypt.c | 56 +++++++++++++++++++++------------------
>  1 file changed, 30 insertions(+), 26 deletions(-)
> 
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index c290c55b632b..d035bce3a2b0 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -42,38 +42,42 @@ bool force_dma_unencrypted(struct device *dev)
>  
>  static void print_mem_encrypt_feature_info(void)
>  {
> -	pr_info("Memory Encryption Features active:");
> +	pr_info("Memory Encryption Features active: ");
>  
> -	if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST)) {
> -		pr_cont(" Intel TDX\n");
> -		return;
> -	}
> +	switch (cc_vendor) {
> +	case CC_VENDOR_INTEL:
> +		pr_cont("Intel TDX\n");
> +		break;
> +	case CC_VENDOR_AMD:
> +		pr_cont("AMD");
>  
> -	pr_cont(" AMD");
> -
> -	/* Secure Memory Encryption */
> -	if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
> +		/* Secure Memory Encryption */
> +		if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) {
>  		/*
>  		 * SME is mutually exclusive with any of the SEV
>  		 * features below.
> -		 */
> -		pr_cont(" SME\n");
> -		return;
> +		*/
> +			pr_cont(" SME\n");
> +			return;
> +		}
> +
> +		/* Secure Encrypted Virtualization */
> +		if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
> +			pr_cont(" SEV");
> +
> +		/* Encrypted Register State */
> +		if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
> +			pr_cont(" SEV-ES");
> +
> +		/* Secure Nested Paging */
> +		if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
> +			pr_cont(" SEV-SNP");
> +
> +		pr_cont("\n");
> +		break;
> +	default:
> +		pr_cont("Unknown\n");
>  	}
> -
> -	/* Secure Encrypted Virtualization */
> -	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
> -		pr_cont(" SEV");
> -
> -	/* Encrypted Register State */
> -	if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
> -		pr_cont(" SEV-ES");
> -
> -	/* Secure Nested Paging */
> -	if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
> -		pr_cont(" SEV-SNP");
> -
> -	pr_cont("\n");
>  }
>  
>  /* Architecture __weak replacement functions */


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

* Re: [PATCHv2] x86/mm: Fix memory encryption features advertisement
  2024-01-16 10:36 ` Huang, Kai
@ 2024-01-16 10:58   ` kirill.shutemov
  2024-01-16 21:16     ` Huang, Kai
  0 siblings, 1 reply; 8+ messages in thread
From: kirill.shutemov @ 2024-01-16 10:58 UTC (permalink / raw)
  To: Huang, Kai
  Cc: tglx, mingo, bp, dave.hansen, hpa, thomas.lendacky, linux-coco,
	jpiotrowski, linux-kernel, Cui, Dexuan, x86

On Tue, Jan 16, 2024 at 10:36:10AM +0000, Huang, Kai wrote:
> On Thu, 2024-01-11 at 14:12 +0300, Kirill A. Shutemov wrote:
> > When memory encryption is enabled, the kernel prints the encryption
> > flavor that the system supports.
> > 
> > The check assumes that everything is AMD SME/SEV if it doesn't have
> > the TDX CPU feature set.
> > 
> > Hyper-V vTOM sets cc_vendor to CC_VENDOR_INTEL when it runs as L2 guest
> > on top of TDX, but not X86_FEATURE_TDX_GUEST. Hyper-V only needs memory
> > encryption enabled for I/O without the rest of CoCo enabling.
> > 
> > To avoid confusion, check the cc_vendor directly.
> > 
> > Possible alternative is to completely removing the print statement.
> > For a regular TDX guest, the kernel already prints a message indicating
> > that it is booting on TDX. Similarly, AMD and Hyper-V can also display
> > a message during their enumeration process.
> > 
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Cc: Dexuan Cui <decui@microsoft.com>
> > Cc: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>
> 
> Seems this fix is for userspace running in hyperv environment being able to use
> some easy grep to get which coco vendor it is running on?

Making decision in userspace by	grepping dmesg is bad idea and nobody
should do this. It can easily give false result: dmesg is not ABI, format
can change and ring buffer has finite size, the message can be overridden.

If we need a way for userspace to discover which CoCo environment it runs
on, we need proper ABI for that. Maybe sysfs file or something.

> If so I think it would be nice to mention it too.
> 
> Acked-by: Kai Huang <kai.huang@intel.com>

Thanks.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

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

* Re: [PATCHv2] x86/mm: Fix memory encryption features advertisement
  2024-01-16 10:58   ` kirill.shutemov
@ 2024-01-16 21:16     ` Huang, Kai
  0 siblings, 0 replies; 8+ messages in thread
From: Huang, Kai @ 2024-01-16 21:16 UTC (permalink / raw)
  To: kirill.shutemov
  Cc: Cui, Dexuan, bp, dave.hansen, jpiotrowski, hpa, mingo, tglx,
	thomas.lendacky, linux-kernel, linux-coco, x86

On Tue, 2024-01-16 at 13:58 +0300, kirill.shutemov@linux.intel.com wrote:
> On Tue, Jan 16, 2024 at 10:36:10AM +0000, Huang, Kai wrote:
> > On Thu, 2024-01-11 at 14:12 +0300, Kirill A. Shutemov wrote:
> > > When memory encryption is enabled, the kernel prints the encryption
> > > flavor that the system supports.
> > > 
> > > The check assumes that everything is AMD SME/SEV if it doesn't have
> > > the TDX CPU feature set.
> > > 
> > > Hyper-V vTOM sets cc_vendor to CC_VENDOR_INTEL when it runs as L2 guest
> > > on top of TDX, but not X86_FEATURE_TDX_GUEST. Hyper-V only needs memory
> > > encryption enabled for I/O without the rest of CoCo enabling.
> > > 
> > > To avoid confusion, check the cc_vendor directly.
> > > 
> > > Possible alternative is to completely removing the print statement.
> > > For a regular TDX guest, the kernel already prints a message indicating
> > > that it is booting on TDX. Similarly, AMD and Hyper-V can also display
> > > a message during their enumeration process.
> > > 
> > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > > Cc: Dexuan Cui <decui@microsoft.com>
> > > Cc: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>
> > 
> > Seems this fix is for userspace running in hyperv environment being able to use
> > some easy grep to get which coco vendor it is running on?
> 
> Making decision in userspace by	grepping dmesg is bad idea and nobody
> should do this. It can easily give false result: dmesg is not ABI, format
> can change and ring buffer has finite size, the message can be overridden.
> 
> If we need a way for userspace to discover which CoCo environment it runs
> on, we need proper ABI for that. Maybe sysfs file or something.

Yeah agreed.  :-)


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

end of thread, other threads:[~2024-01-16 21:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-11 11:12 [PATCHv2] x86/mm: Fix memory encryption features advertisement Kirill A. Shutemov
2024-01-11 14:19 ` Kuppuswamy Sathyanarayanan
2024-01-11 15:14   ` Jeremi Piotrowski
2024-01-11 20:46   ` Kuppuswamy Sathyanarayanan
2024-01-11 20:41 ` Tom Lendacky
2024-01-16 10:36 ` Huang, Kai
2024-01-16 10:58   ` kirill.shutemov
2024-01-16 21:16     ` Huang, Kai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).