2024-01-10 19:35:53

by Kirill A. Shutemov

[permalink] [raw]
Subject: [PATCH] x86/mm: Fix memory encryption features advertisement

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 has the TDX CPU
feature set.

To avoid confusion, check the cc_vendor directly.

Signed-off-by: Kirill A. Shutemov <[email protected]>
Cc: Dexuan Cui <[email protected]>
Cc: Jeremi Piotrowski <[email protected]>
Signed-off-by: Kirill A. Shutemov <[email protected]>
---
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



2024-01-10 21:33:31

by Tom Lendacky

[permalink] [raw]
Subject: Re: [PATCH] x86/mm: Fix memory encryption features advertisement

On 1/10/24 13:35, 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 has the TDX CPU

s/if it has/if it doesn't have/

> feature set.
>
> To avoid confusion, check the cc_vendor directly.

Is this because of the setting of cc_vendor in hv_vtom_init() without
setting the TDX feature and so "Intel TDX" isn't printed and instead "AMD"
is printed?

If so, the commit message should really have some info about how this
relates to Hyper-V isolation VMs.

Thanks,
Tom

>
> Signed-off-by: Kirill A. Shutemov <[email protected]>
> Cc: Dexuan Cui <[email protected]>
> Cc: Jeremi Piotrowski <[email protected]>
> Signed-off-by: Kirill A. Shutemov <[email protected]>
> ---
> 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 */

2024-01-11 10:43:56

by Kirill A. Shutemov

[permalink] [raw]
Subject: Re: [PATCH] x86/mm: Fix memory encryption features advertisement

On Wed, Jan 10, 2024 at 03:33:13PM -0600, Tom Lendacky wrote:
> On 1/10/24 13:35, 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 has the TDX CPU
>
> s/if it has/if it doesn't have/

Oopsie.

> > feature set.
> >
> > To avoid confusion, check the cc_vendor directly.
>
> Is this because of the setting of cc_vendor in hv_vtom_init() without
> setting the TDX feature and so "Intel TDX" isn't printed and instead "AMD"
> is printed?
>
> If so, the commit message should really have some info about how this
> relates to Hyper-V isolation VMs.

That's one of the issue. But the code is generally has flawed logic that
worth addressing.

Will send v2 with better commit message.

--
Kiryl Shutsemau / Kirill A. Shutemov