Subject: [RFC v2-fix-v1 1/3] tdx: Introduce generic protected_guest abstraction

Add a generic way to check if we run with an encrypted guest,
without requiring x86 specific ifdefs. This can then be used in
non architecture specific code. 

is_protected_guest() helper function can be implemented using
arch specific CPU feature flags.

protected_guest_has() is used to check for protected guest
feature flags.

Originally-by: Andi Kleen <[email protected]>
Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
---
arch/Kconfig | 3 +++
arch/x86/Kconfig | 1 +
arch/x86/include/asm/protected_guest.h | 24 ++++++++++++++++++++++++
arch/x86/include/asm/tdx.h | 7 +++++++
arch/x86/kernel/tdx.c | 18 ++++++++++++++++++
include/linux/protected_guest.h | 23 +++++++++++++++++++++++
6 files changed, 76 insertions(+)
create mode 100644 arch/x86/include/asm/protected_guest.h
create mode 100644 include/linux/protected_guest.h

diff --git a/arch/Kconfig b/arch/Kconfig
index ecfd3520b676..98c30312555b 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -956,6 +956,9 @@ config HAVE_ARCH_NVRAM_OPS
config ISA_BUS_API
def_bool ISA

+config ARCH_HAS_PROTECTED_GUEST
+ bool
+
#
# ABI hall of shame
#
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 15e66a99dd41..fc588a64d1a0 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -878,6 +878,7 @@ config INTEL_TDX_GUEST
select PARAVIRT_XL
select X86_X2APIC
select SECURITY_LOCKDOWN_LSM
+ select ARCH_HAS_PROTECTED_GUEST
help
Provide support for running in a trusted domain on Intel processors
equipped with Trusted Domain eXtenstions. TDX is a new Intel
diff --git a/arch/x86/include/asm/protected_guest.h b/arch/x86/include/asm/protected_guest.h
new file mode 100644
index 000000000000..b2838e58ce94
--- /dev/null
+++ b/arch/x86/include/asm/protected_guest.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2020 Intel Corporation */
+#ifndef _ASM_PROTECTED_GUEST
+#define _ASM_PROTECTED_GUEST 1
+
+#include <asm/cpufeature.h>
+#include <asm/tdx.h>
+
+/* Only include through linux/protected_guest.h */
+
+static inline bool is_protected_guest(void)
+{
+ return boot_cpu_has(X86_FEATURE_TDX_GUEST);
+}
+
+static inline bool protected_guest_has(unsigned long flag)
+{
+ if (boot_cpu_has(X86_FEATURE_TDX_GUEST))
+ return tdx_protected_guest_has(flag);
+
+ return false;
+}
+
+#endif
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 597a3e1663d7..53f844200909 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -71,6 +71,8 @@ u64 __tdx_module_call(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
u64 __tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14, u64 r15,
struct tdx_hypercall_output *out);

+bool tdx_protected_guest_has(unsigned long flag);
+
#else // !CONFIG_INTEL_TDX_GUEST

static inline bool is_tdx_guest(void)
@@ -80,6 +82,11 @@ static inline bool is_tdx_guest(void)

static inline void tdx_early_init(void) { };

+static inline bool tdx_protected_guest_has(unsigned long flag)
+{
+ return false;
+}
+
#endif /* CONFIG_INTEL_TDX_GUEST */

#ifdef CONFIG_INTEL_TDX_GUEST_KVM
diff --git a/arch/x86/kernel/tdx.c b/arch/x86/kernel/tdx.c
index 17725646eb30..858e7f3d8f36 100644
--- a/arch/x86/kernel/tdx.c
+++ b/arch/x86/kernel/tdx.c
@@ -7,6 +7,7 @@
#include <asm/vmx.h>

#include <linux/cpu.h>
+#include <linux/protected_guest.h>

/* TDX Module call Leaf IDs */
#define TDINFO 1
@@ -75,6 +76,23 @@ bool is_tdx_guest(void)
}
EXPORT_SYMBOL_GPL(is_tdx_guest);

+bool tdx_protected_guest_has(unsigned long flag)
+{
+ if (!is_tdx_guest())
+ return false;
+
+ switch (flag) {
+ case VM_MEM_ENCRYPT:
+ case VM_MEM_ENCRYPT_ACTIVE:
+ case VM_UNROLL_STRING_IO:
+ case VM_HOST_MEM_ENCRYPT:
+ return true;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL_GPL(tdx_protected_guest_has);
+
static void tdg_get_info(void)
{
u64 ret;
diff --git a/include/linux/protected_guest.h b/include/linux/protected_guest.h
new file mode 100644
index 000000000000..f362eea39bd8
--- /dev/null
+++ b/include/linux/protected_guest.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _LINUX_PROTECTED_GUEST_H
+#define _LINUX_PROTECTED_GUEST_H 1
+
+/* Protected Guest Feature Flags (leave 0-0xff for arch specific flags) */
+
+/* Support for guest encryption */
+#define VM_MEM_ENCRYPT 0x100
+/* Encryption support is active */
+#define VM_MEM_ENCRYPT_ACTIVE 0x101
+/* Support for unrolled string IO */
+#define VM_UNROLL_STRING_IO 0x102
+/* Support for host memory encryption */
+#define VM_HOST_MEM_ENCRYPT 0x103
+
+#ifdef CONFIG_ARCH_HAS_PROTECTED_GUEST
+#include <asm/protected_guest.h>
+#else
+static inline bool is_protected_guest(void) { return false; }
+static inline bool protected_guest_has(unsigned long flag) { return false; }
+#endif
+
+#endif
--
2.25.1


Subject: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction

Add a generic way to check if we run with an encrypted guest,
without requiring x86 specific ifdefs. This can then be used in
non architecture specific code. 

protected_guest_has() is used to check for protected guest
feature flags.

Originally-by: Andi Kleen <[email protected]>
Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
---
Changes since RFC v2-fix-v1:
* Changed the title from "tdx: Introduce generic protected_guest
abstraction" to "x86: Introduce generic protected guest"
* Removed usage of ARCH_HAS_PROTECTED_GUEST and directly called TDX
and AMD specific xx_protected_guest_has() variants from
linux/protected_guest.h.
* Added support for amd_protected_guest_has() helper function.
* Removed redundant is_tdx_guest() check in tdx_protected_guest_has()
function.
* Fixed commit log to reflect the latest changes.

arch/x86/include/asm/mem_encrypt.h | 4 +++
arch/x86/include/asm/tdx.h | 7 ++++++
arch/x86/kernel/tdx.c | 16 ++++++++++++
arch/x86/mm/mem_encrypt.c | 13 ++++++++++
include/linux/protected_guest.h | 40 ++++++++++++++++++++++++++++++
5 files changed, 80 insertions(+)
create mode 100644 include/linux/protected_guest.h

diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
index 9c80c68d75b5..1492b0eb29d0 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -56,6 +56,8 @@ bool sev_es_active(void);

#define __bss_decrypted __section(".bss..decrypted")

+bool amd_protected_guest_has(unsigned long flag);
+
#else /* !CONFIG_AMD_MEM_ENCRYPT */

#define sme_me_mask 0ULL
@@ -86,6 +88,8 @@ early_set_memory_encrypted(unsigned long vaddr, unsigned long size) { return 0;

static inline void mem_encrypt_free_decrypted_mem(void) { }

+static inline bool amd_protected_guest_has(unsigned long flag) { return false; }
+
#define __bss_decrypted

#endif /* CONFIG_AMD_MEM_ENCRYPT */
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index f0c1912837c8..cbfe7479f2a3 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -71,6 +71,8 @@ u64 __tdx_module_call(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
u64 __tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14, u64 r15,
struct tdx_hypercall_output *out);

+bool tdx_protected_guest_has(unsigned long flag);
+
#else // !CONFIG_INTEL_TDX_GUEST

static inline bool is_tdx_guest(void)
@@ -80,6 +82,11 @@ static inline bool is_tdx_guest(void)

static inline void tdx_early_init(void) { };

+static inline bool tdx_protected_guest_has(unsigned long flag)
+{
+ return false;
+}
+
#endif /* CONFIG_INTEL_TDX_GUEST */

#ifdef CONFIG_INTEL_TDX_GUEST_KVM
diff --git a/arch/x86/kernel/tdx.c b/arch/x86/kernel/tdx.c
index 17725646eb30..b1cdb37a8636 100644
--- a/arch/x86/kernel/tdx.c
+++ b/arch/x86/kernel/tdx.c
@@ -7,6 +7,7 @@
#include <asm/vmx.h>

#include <linux/cpu.h>
+#include <linux/protected_guest.h>

/* TDX Module call Leaf IDs */
#define TDINFO 1
@@ -75,6 +76,21 @@ bool is_tdx_guest(void)
}
EXPORT_SYMBOL_GPL(is_tdx_guest);

+bool tdx_protected_guest_has(unsigned long flag)
+{
+ switch (flag) {
+ case VM_MEM_ENCRYPT:
+ case VM_MEM_ENCRYPT_ACTIVE:
+ case VM_UNROLL_STRING_IO:
+ case VM_HOST_MEM_ENCRYPT:
+ case VM_SHARED_MAPPING_INIT:
+ return true;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL_GPL(tdx_protected_guest_has);
+
static void tdg_get_info(void)
{
u64 ret;
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index ff08dc463634..7019eab20096 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -20,6 +20,7 @@
#include <linux/bitops.h>
#include <linux/dma-mapping.h>
#include <linux/virtio_config.h>
+#include <linux/protected_guest.h>

#include <asm/tlbflush.h>
#include <asm/fixmap.h>
@@ -389,6 +390,18 @@ bool noinstr sev_es_active(void)
return sev_status & MSR_AMD64_SEV_ES_ENABLED;
}

+bool amd_protected_guest_has(unsigned long flag)
+{
+ switch (flag) {
+ case VM_MEM_ENCRYPT:
+ case VM_MEM_ENCRYPT_ACTIVE:
+ return true;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL_GPL(amd_protected_guest_has);
+
/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
bool force_dma_unencrypted(struct device *dev)
{
diff --git a/include/linux/protected_guest.h b/include/linux/protected_guest.h
new file mode 100644
index 000000000000..303dfba81d52
--- /dev/null
+++ b/include/linux/protected_guest.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _LINUX_PROTECTED_GUEST_H
+#define _LINUX_PROTECTED_GUEST_H 1
+
+#include <linux/mem_encrypt.h>
+
+/* Protected Guest Feature Flags (leave 0-0xff for arch specific flags) */
+
+/* Support for guest encryption */
+#define VM_MEM_ENCRYPT 0x100
+/* Encryption support is active */
+#define VM_MEM_ENCRYPT_ACTIVE 0x101
+/* Support for unrolled string IO */
+#define VM_UNROLL_STRING_IO 0x102
+/* Support for host memory encryption */
+#define VM_HOST_MEM_ENCRYPT 0x103
+/* Support for shared mapping initialization (after early init) */
+#define VM_SHARED_MAPPING_INIT 0x104
+
+#if defined(CONFIG_INTEL_TDX_GUEST) || defined(CONFIG_AMD_MEM_ENCRYPT)
+
+#include <asm/tdx.h>
+
+static inline bool protected_guest_has(unsigned long flag)
+{
+ if (is_tdx_guest())
+ return tdx_protected_guest_has(flag);
+ else if (mem_encrypt_active())
+ return amd_protected_guest_has(flag);
+
+ return false;
+}
+
+#else
+
+static inline bool protected_guest_has(unsigned long flag) { return false; }
+
+#endif
+
+#endif
--
2.25.1

2021-06-02 17:23:09

by Sean Christopherson

[permalink] [raw]
Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction

On Tue, Jun 01, 2021, Kuppuswamy Sathyanarayanan wrote:
> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
> index 9c80c68d75b5..1492b0eb29d0 100644
> --- a/arch/x86/include/asm/mem_encrypt.h
> +++ b/arch/x86/include/asm/mem_encrypt.h
> @@ -56,6 +56,8 @@ bool sev_es_active(void);
>
> #define __bss_decrypted __section(".bss..decrypted")
>
> +bool amd_protected_guest_has(unsigned long flag);


Why call one by the vendor (amd) and the other by the technology (tdx)?
sev_protected_guest_has() seems like the more logical name, e.g. if AMD CPUs
gain a new non-SEV technology then we'll have a mess.

> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index f0c1912837c8..cbfe7479f2a3 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -71,6 +71,8 @@ u64 __tdx_module_call(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
> u64 __tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14, u64 r15,
> struct tdx_hypercall_output *out);
>
> +bool tdx_protected_guest_has(unsigned long flag);

...

> +static inline bool protected_guest_has(unsigned long flag)
> +{
> + if (is_tdx_guest())
> + return tdx_protected_guest_has(flag);
> + else if (mem_encrypt_active())

Shouldn't this be sev_active()? mem_encrypt_active() will return true for SME,
too.

> + return amd_protected_guest_has(flag);
> +
> + return false;
> +}

2021-06-02 18:19:25

by Tom Lendacky

[permalink] [raw]
Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction

On 6/2/21 12:20 PM, Sean Christopherson wrote:
> On Tue, Jun 01, 2021, Kuppuswamy Sathyanarayanan wrote:
>> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
>> index 9c80c68d75b5..1492b0eb29d0 100644
>> --- a/arch/x86/include/asm/mem_encrypt.h
>> +++ b/arch/x86/include/asm/mem_encrypt.h
>> @@ -56,6 +56,8 @@ bool sev_es_active(void);
>>
>> #define __bss_decrypted __section(".bss..decrypted")
>>
>> +bool amd_protected_guest_has(unsigned long flag);
>
>
> Why call one by the vendor (amd) and the other by the technology (tdx)?
> sev_protected_guest_has() seems like the more logical name, e.g. if AMD CPUs
> gain a new non-SEV technology then we'll have a mess.

The original suggestion from Boris, IIRC, was for protected_guest_has()
function (below) to be:

if (intel)
return intel_protected_guest_has();
else if (amd)
return amd_protected_guest_has();
else
return false;

And then you could check for TDX or SME/SEV in the respective functions.

>
>> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
>> index f0c1912837c8..cbfe7479f2a3 100644
>> --- a/arch/x86/include/asm/tdx.h
>> +++ b/arch/x86/include/asm/tdx.h
>> @@ -71,6 +71,8 @@ u64 __tdx_module_call(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
>> u64 __tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14, u64 r15,
>> struct tdx_hypercall_output *out);
>>
>> +bool tdx_protected_guest_has(unsigned long flag);
>
> ...
>
>> +static inline bool protected_guest_has(unsigned long flag)
>> +{
>> + if (is_tdx_guest())
>> + return tdx_protected_guest_has(flag);
>> + else if (mem_encrypt_active())
>
> Shouldn't this be sev_active()? mem_encrypt_active() will return true for SME,
> too.

I believe Boris was wanting to replace the areas where sme_active() was
specifically checked, too. And so protected_guest_has() can be confusing...

Maybe naming it protected_os_has() or protection_attr_active() might work.
This would then work SME or MKTME as well.

Thanks,
Tom

>
>> + return amd_protected_guest_has(flag);
>> +
>> + return false;
>> +}

2021-06-02 18:21:52

by Tom Lendacky

[permalink] [raw]
Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction

On 6/1/21 4:14 PM, Kuppuswamy Sathyanarayanan wrote:
> Add a generic way to check if we run with an encrypted guest,
> without requiring x86 specific ifdefs. This can then be used in
> non architecture specific code. 
>
> protected_guest_has() is used to check for protected guest
> feature flags.
>
> Originally-by: Andi Kleen <[email protected]>
> Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
> ---
> Changes since RFC v2-fix-v1:
> * Changed the title from "tdx: Introduce generic protected_guest
> abstraction" to "x86: Introduce generic protected guest"
> * Removed usage of ARCH_HAS_PROTECTED_GUEST and directly called TDX
> and AMD specific xx_protected_guest_has() variants from
> linux/protected_guest.h.
> * Added support for amd_protected_guest_has() helper function.
> * Removed redundant is_tdx_guest() check in tdx_protected_guest_has()
> function.
> * Fixed commit log to reflect the latest changes.

...

>
> +bool amd_protected_guest_has(unsigned long flag)
> +{
> + switch (flag) {
> + case VM_MEM_ENCRYPT:
> + case VM_MEM_ENCRYPT_ACTIVE:
> + return true;
> + }
> +
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(amd_protected_guest_has);

This certainly doesn't capture all of the situations where true would need
to be returned. For example, SEV, but not SEV-ES, requires that string I/O
be unrolled, etc.

Thanks,
Tom

Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction



On 6/2/21 11:15 AM, Tom Lendacky wrote:
> The original suggestion from Boris, IIRC, was for protected_guest_has()
> function (below) to be:
>
> if (intel)
> return intel_protected_guest_has();

Yes. But for Intel, I think currently we can only check for is_tdx_guest() here.

if (is_tdx_guest())
return intel_protected_guest_has();

So if we use is_tdx_guest(), it is better to call tdx_protected_guest_has() here.

Once we start using protected_guest_has for other Intel technologies, may be
we can generalize it. Let me know your comments.

> else if (amd)
> return amd_protected_guest_has();
> else
> return false;
>
> And then you could check for TDX or SME/SEV in the respective functions.

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

2021-06-02 18:31:12

by Borislav Petkov

[permalink] [raw]
Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction

On Wed, Jun 02, 2021 at 01:15:23PM -0500, Tom Lendacky wrote:
> The original suggestion from Boris, IIRC, was for protected_guest_has()
> function (below) to be:
>
> if (intel)
> return intel_protected_guest_has();
> else if (amd)
> return amd_protected_guest_has();
> else
> return false;
>
> And then you could check for TDX or SME/SEV in the respective functions.

Yeah, a single function call which calls vendor-specific functions.

If you can point me to a tree with your patches, I can try to hack up
what I mean.

> I believe Boris was wanting to replace the areas where sme_active() was
> specifically checked, too. And so protected_guest_has() can be confusing...

We can always say

protected_guest_has(SME_ACTIVE);

or so and then it is clear.

> Maybe naming it protected_os_has() or protection_attr_active() might work.
> This would then work SME or MKTME as well.

But other names are fine too once we're done with the bikeshedding.

Thx.

--
Regards/Gruss,
Boris.

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

2021-06-02 18:32:38

by Borislav Petkov

[permalink] [raw]
Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction

On Wed, Jun 02, 2021 at 01:19:07PM -0500, Tom Lendacky wrote:
> This certainly doesn't capture all of the situations where true would need
> to be returned. For example, SEV, but not SEV-ES, requires that string I/O
> be unrolled, etc.

Yeah, I believe this would be better done for you guys, ontop, as you
know best what needs to be queried where. So this first patch adding
only a stub should be fine. Or you or someone else does the conversion
ontop of the Intel patch and then all patches go together.

Thx.

--
Regards/Gruss,
Boris.

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

Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction



On 6/2/21 11:29 AM, Borislav Petkov wrote:
> If you can point me to a tree with your patches, I can try to hack up
> what I mean.

https://github.com/intel/tdx/commit/8515b66a0cb27d5ab66eda201285090faee742f7

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction



On 6/2/21 11:19 AM, Tom Lendacky wrote:
> This certainly doesn't capture all of the situations where true would need
> to be returned. For example, SEV, but not SEV-ES, requires that string I/O
> be unrolled, etc.

For AMD following cases should be true right? I can fix it in next version.

case VM_UNROLL_STRING_IO:
case VM_HOST_MEM_ENCRYPT:

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

2021-06-02 18:43:48

by Borislav Petkov

[permalink] [raw]
Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction

On Wed, Jun 02, 2021 at 11:32:18AM -0700, Kuppuswamy, Sathyanarayanan wrote:
>
>
> On 6/2/21 11:29 AM, Borislav Petkov wrote:
> > If you can point me to a tree with your patches, I can try to hack up
> > what I mean.
>
> https://github.com/intel/tdx/commit/8515b66a0cb27d5ab66eda201285090faee742f7

Ok, and which branch or tag?

tdx-guest-v5.12-7 or "guest"?

The github interface is yuck when one wants to look at commits...

--
Regards/Gruss,
Boris.

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

Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction



On 6/2/21 11:39 AM, Borislav Petkov wrote:
> Ok, and which branch or tag?
>
> tdx-guest-v5.12-7 or "guest"?
>
> The github interface is yuck when one wants to look at commits...

please use guest branch.

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

2021-06-03 18:18:51

by Borislav Petkov

[permalink] [raw]
Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction

On Tue, Jun 01, 2021 at 02:14:17PM -0700, Kuppuswamy Sathyanarayanan wrote:
> diff --git a/include/linux/protected_guest.h b/include/linux/protected_guest.h
> new file mode 100644
> index 000000000000..303dfba81d52
> --- /dev/null
> +++ b/include/linux/protected_guest.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef _LINUX_PROTECTED_GUEST_H
> +#define _LINUX_PROTECTED_GUEST_H 1
> +
> +#include <linux/mem_encrypt.h>
> +
> +/* Protected Guest Feature Flags (leave 0-0xff for arch specific flags) */
> +
> +/* Support for guest encryption */
> +#define VM_MEM_ENCRYPT 0x100
> +/* Encryption support is active */
> +#define VM_MEM_ENCRYPT_ACTIVE 0x101
> +/* Support for unrolled string IO */
> +#define VM_UNROLL_STRING_IO 0x102
> +/* Support for host memory encryption */
> +#define VM_HOST_MEM_ENCRYPT 0x103
> +/* Support for shared mapping initialization (after early init) */
> +#define VM_SHARED_MAPPING_INIT 0x104

Ok, a couple of things:

first of all, those flags with that VM_ prefix make me think of
"virtual memory" instead of "virtual machine". So they should be
something else, like, say

PR_G_... for Protected Guest or so. Or PR_GUEST or ...

(yeah, good namespaces are all taken. )

Then, about the function name length, I'm fine if we did:

prot_guest_has()

or something even shorter, if you folks have a good suggestion.

Anyway, below is a diff ontop of your tree with what I think the
barebones of this should be.

As a reply to this message I went and converted sme_active() to use
protected_guest_has() too.

Comments, complaints?

Thx.

---
diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
index 1492b0eb29d0..9c80c68d75b5 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -56,8 +56,6 @@ bool sev_es_active(void);

#define __bss_decrypted __section(".bss..decrypted")

-bool amd_protected_guest_has(unsigned long flag);
-
#else /* !CONFIG_AMD_MEM_ENCRYPT */

#define sme_me_mask 0ULL
@@ -88,8 +86,6 @@ early_set_memory_encrypted(unsigned long vaddr, unsigned long size) { return 0;

static inline void mem_encrypt_free_decrypted_mem(void) { }

-static inline bool amd_protected_guest_has(unsigned long flag) { return false; }
-
#define __bss_decrypted

#endif /* CONFIG_AMD_MEM_ENCRYPT */
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index fa5cd05d3b5b..f09996c6a272 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -11,6 +11,7 @@
#include <linux/types.h>
#include <asm/insn.h>
#include <asm/sev-common.h>
+#include <asm/pgtable_types.h>

#define GHCB_PROTO_OUR 0x0001UL
#define GHCB_PROTOCOL_MAX 1ULL
@@ -81,12 +82,15 @@ static __always_inline void sev_es_nmi_complete(void)
__sev_es_nmi_complete();
}
extern int __init sev_es_efi_map_ghcbs(pgd_t *pgd);
+bool sev_protected_guest_has(unsigned long flag);
+
#else
static inline void sev_es_ist_enter(struct pt_regs *regs) { }
static inline void sev_es_ist_exit(void) { }
static inline int sev_es_setup_ap_jump_table(struct real_mode_header *rmh) { return 0; }
static inline void sev_es_nmi_complete(void) { }
static inline int sev_es_efi_map_ghcbs(pgd_t *pgd) { return 0; }
+static inline bool sev_protected_guest_has(unsigned long flag) { return false; }
#endif

-#endif
+#endif /* __ASM_ENCRYPTED_STATE_H */
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index f7a743d122eb..01a224fdb897 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -1402,3 +1402,14 @@ bool __init handle_vc_boot_ghcb(struct pt_regs *regs)
while (true)
halt();
}
+
+bool sev_protected_guest_has(unsigned long flag)
+{
+ switch (flag) {
+ case VM_MEM_ENCRYPT:
+ case VM_MEM_ENCRYPT_ACTIVE:
+ return true;
+ }
+
+ return false;
+}
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index ced658e79753..49d11bb6e02a 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -391,18 +391,6 @@ bool noinstr sev_es_active(void)
return sev_status & MSR_AMD64_SEV_ES_ENABLED;
}

-bool amd_protected_guest_has(unsigned long flag)
-{
- switch (flag) {
- case VM_MEM_ENCRYPT:
- case VM_MEM_ENCRYPT_ACTIVE:
- return true;
- }
-
- return false;
-}
-EXPORT_SYMBOL_GPL(amd_protected_guest_has);
-
/* Override for DMA direct allocation check - AMD specific initialization */
bool amd_force_dma_unencrypted(struct device *dev)
{
diff --git a/include/linux/protected_guest.h b/include/linux/protected_guest.h
index 6855d5b3e244..bb4b1a06b21f 100644
--- a/include/linux/protected_guest.h
+++ b/include/linux/protected_guest.h
@@ -2,7 +2,9 @@
#ifndef _LINUX_PROTECTED_GUEST_H
#define _LINUX_PROTECTED_GUEST_H 1

-#include <linux/mem_encrypt.h>
+#include <asm/processor.h>
+#include <asm/tdx.h>
+#include <asm/sev.h>

/* Protected Guest Feature Flags (leave 0-0xff for arch specific flags) */

@@ -20,23 +22,18 @@
#define VM_DISABLE_UNCORE_SUPPORT 0x105

#if defined(CONFIG_INTEL_TDX_GUEST) || defined(CONFIG_AMD_MEM_ENCRYPT)
-
-#include <asm/tdx.h>
-
static inline bool protected_guest_has(unsigned long flag)
{
if (is_tdx_guest())
return tdx_protected_guest_has(flag);
- else if (mem_encrypt_active())
- return amd_protected_guest_has(flag);
+ else if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+ return sev_protected_guest_has(flag);

return false;
}

#else
-
static inline bool protected_guest_has(unsigned long flag) { return false; }
-
#endif

-#endif
+#endif /* _LINUX_PROTECTED_GUEST_H */


--
Regards/Gruss,
Boris.

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

Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction



On 6/3/21 11:14 AM, Borislav Petkov wrote:
> On Tue, Jun 01, 2021 at 02:14:17PM -0700, Kuppuswamy Sathyanarayanan wrote:
>> diff --git a/include/linux/protected_guest.h b/include/linux/protected_guest.h
>> new file mode 100644
>> index 000000000000..303dfba81d52
>> --- /dev/null
>> +++ b/include/linux/protected_guest.h
>> @@ -0,0 +1,40 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +#ifndef _LINUX_PROTECTED_GUEST_H
>> +#define _LINUX_PROTECTED_GUEST_H 1
>> +
>> +#include <linux/mem_encrypt.h>
>> +
>> +/* Protected Guest Feature Flags (leave 0-0xff for arch specific flags) */
>> +
>> +/* Support for guest encryption */
>> +#define VM_MEM_ENCRYPT 0x100
>> +/* Encryption support is active */
>> +#define VM_MEM_ENCRYPT_ACTIVE 0x101
>> +/* Support for unrolled string IO */
>> +#define VM_UNROLL_STRING_IO 0x102
>> +/* Support for host memory encryption */
>> +#define VM_HOST_MEM_ENCRYPT 0x103
>> +/* Support for shared mapping initialization (after early init) */
>> +#define VM_SHARED_MAPPING_INIT 0x104
>
> Ok, a couple of things:
>
> first of all, those flags with that VM_ prefix make me think of
> "virtual memory" instead of "virtual machine". So they should be
> something else, like, say
>
> PR_G_... for Protected Guest or so. Or PR_GUEST or ...

I would prefer PR_GUEST over PR_G_

>
> (yeah, good namespaces are all taken. )
>
> Then, about the function name length, I'm fine if we did:
>
> prot_guest_has()
>
> or something even shorter, if you folks have a good suggestion.
>
> Anyway, below is a diff ontop of your tree with what I think the
> barebones of this should be.
>
> As a reply to this message I went and converted sme_active() to use
> protected_guest_has() too.
>
> Comments, complaints?
>
> Thx.
>
> ---
> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
> index 1492b0eb29d0..9c80c68d75b5 100644
> --- a/arch/x86/include/asm/mem_encrypt.h
> +++ b/arch/x86/include/asm/mem_encrypt.h
> @@ -56,8 +56,6 @@ bool sev_es_active(void);
>
> #define __bss_decrypted __section(".bss..decrypted")
>
> -bool amd_protected_guest_has(unsigned long flag);
> -
> #else /* !CONFIG_AMD_MEM_ENCRYPT */
>
> #define sme_me_mask 0ULL
> @@ -88,8 +86,6 @@ early_set_memory_encrypted(unsigned long vaddr, unsigned long size) { return 0;
>
> static inline void mem_encrypt_free_decrypted_mem(void) { }
>
> -static inline bool amd_protected_guest_has(unsigned long flag) { return false; }
> -
> #define __bss_decrypted
>
> #endif /* CONFIG_AMD_MEM_ENCRYPT */
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index fa5cd05d3b5b..f09996c6a272 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -11,6 +11,7 @@
> #include <linux/types.h>
> #include <asm/insn.h>
> #include <asm/sev-common.h>
> +#include <asm/pgtable_types.h>
>
> #define GHCB_PROTO_OUR 0x0001UL
> #define GHCB_PROTOCOL_MAX 1ULL
> @@ -81,12 +82,15 @@ static __always_inline void sev_es_nmi_complete(void)
> __sev_es_nmi_complete();
> }
> extern int __init sev_es_efi_map_ghcbs(pgd_t *pgd);
> +bool sev_protected_guest_has(unsigned long flag);
> +
> #else
> static inline void sev_es_ist_enter(struct pt_regs *regs) { }
> static inline void sev_es_ist_exit(void) { }
> static inline int sev_es_setup_ap_jump_table(struct real_mode_header *rmh) { return 0; }
> static inline void sev_es_nmi_complete(void) { }
> static inline int sev_es_efi_map_ghcbs(pgd_t *pgd) { return 0; }
> +static inline bool sev_protected_guest_has(unsigned long flag) { return false; }
> #endif
>
> -#endif
> +#endif /* __ASM_ENCRYPTED_STATE_H */
> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
> index f7a743d122eb..01a224fdb897 100644
> --- a/arch/x86/kernel/sev.c
> +++ b/arch/x86/kernel/sev.c
> @@ -1402,3 +1402,14 @@ bool __init handle_vc_boot_ghcb(struct pt_regs *regs)
> while (true)
> halt();
> }
> +
> +bool sev_protected_guest_has(unsigned long flag)
> +{
> + switch (flag) {
> + case VM_MEM_ENCRYPT:
> + case VM_MEM_ENCRYPT_ACTIVE:
> + return true;
> + }
> +
> + return false;
> +}

I assume this file will get compiled for both SEV and SME cases.

> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index ced658e79753..49d11bb6e02a 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -391,18 +391,6 @@ bool noinstr sev_es_active(void)
> return sev_status & MSR_AMD64_SEV_ES_ENABLED;
> }
>
> -bool amd_protected_guest_has(unsigned long flag)
> -{
> - switch (flag) {
> - case VM_MEM_ENCRYPT:
> - case VM_MEM_ENCRYPT_ACTIVE:
> - return true;
> - }
> -
> - return false;
> -}
> -EXPORT_SYMBOL_GPL(amd_protected_guest_has);
> -
> /* Override for DMA direct allocation check - AMD specific initialization */
> bool amd_force_dma_unencrypted(struct device *dev)
> {
> diff --git a/include/linux/protected_guest.h b/include/linux/protected_guest.h
> index 6855d5b3e244..bb4b1a06b21f 100644
> --- a/include/linux/protected_guest.h
> +++ b/include/linux/protected_guest.h
> @@ -2,7 +2,9 @@
> #ifndef _LINUX_PROTECTED_GUEST_H
> #define _LINUX_PROTECTED_GUEST_H 1
>
> -#include <linux/mem_encrypt.h>
> +#include <asm/processor.h>
> +#include <asm/tdx.h>
> +#include <asm/sev.h>
>
> /* Protected Guest Feature Flags (leave 0-0xff for arch specific flags) */
>
> @@ -20,23 +22,18 @@
> #define VM_DISABLE_UNCORE_SUPPORT 0x105
>
> #if defined(CONFIG_INTEL_TDX_GUEST) || defined(CONFIG_AMD_MEM_ENCRYPT)
> -
> -#include <asm/tdx.h>
> -
> static inline bool protected_guest_has(unsigned long flag)
> {
> if (is_tdx_guest())
> return tdx_protected_guest_has(flag);
> - else if (mem_encrypt_active())
> - return amd_protected_guest_has(flag);
> + else if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
> + return sev_protected_guest_has(flag);

Since you are checking for AMD vendor ID, why not use amd_protected_guest_has()?

>
> return false;
> }
>
> #else
> -
> static inline bool protected_guest_has(unsigned long flag) { return false; }
> -
> #endif
>
> -#endif
> +#endif /* _LINUX_PROTECTED_GUEST_H */
>
>

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

2021-06-03 18:43:33

by Borislav Petkov

[permalink] [raw]
Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction

Sathya,

please trim your mails when you reply, like I've done in this reply.

Thx.

On Thu, Jun 03, 2021 at 11:33:53AM -0700, Kuppuswamy, Sathyanarayanan wrote:
> I assume this file will get compiled for both SEV and SME cases.

Yap.

> Since you are checking for AMD vendor ID, why not use amd_protected_guest_has()?

Because, as Sean already told you, we should either stick to the
technologies: TDX or SEV or to the vendors: Intel or AMD - but not
either or.

--
Regards/Gruss,
Boris.

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

Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction



On 6/3/21 11:41 AM, Borislav Petkov wrote:
>> Since you are checking for AMD vendor ID, why not use amd_protected_guest_has()?
> Because, as Sean already told you, we should either stick to the
> technologies: TDX or SEV or to the vendors: Intel or AMD - but not
> either or.

Ok. We can go with technologies for now. In future, if protected_guest_has() is extended
for other technologies like MKTME, then we can generalize it base on vendor.

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction



On 6/3/21 11:14 AM, Borislav Petkov wrote:
> On Tue, Jun 01, 2021 at 02:14:17PM -0700, Kuppuswamy Sathyanarayanan wrote:

snip

> diff --git a/include/linux/protected_guest.h b/include/linux/protected_guest.h
> index 6855d5b3e244..bb4b1a06b21f 100644
> --- a/include/linux/protected_guest.h
> +++ b/include/linux/protected_guest.h
> @@ -2,7 +2,9 @@
> #ifndef _LINUX_PROTECTED_GUEST_H
> #define _LINUX_PROTECTED_GUEST_H 1
>
> -#include <linux/mem_encrypt.h>
> +#include <asm/processor.h>
> +#include <asm/tdx.h>
> +#include <asm/sev.h>
>
> /* Protected Guest Feature Flags (leave 0-0xff for arch specific flags) */
>
> @@ -20,23 +22,18 @@
> #define VM_DISABLE_UNCORE_SUPPORT 0x105
>
> #if defined(CONFIG_INTEL_TDX_GUEST) || defined(CONFIG_AMD_MEM_ENCRYPT)
> -
> -#include <asm/tdx.h>
> -

Why move this header outside CONFIG_INTEL_TDX_GUEST or CONFIG_AMD_MEM_ENCRYPT ifdef?

This header only exists in x86 arch code. So it is better to protect it with x86
specific header file.

> static inline bool protected_guest_has(unsigned long flag)
> {
> if (is_tdx_guest())
> return tdx_protected_guest_has(flag);
> - else if (mem_encrypt_active())
> - return amd_protected_guest_has(flag);
> + else if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
> + return sev_protected_guest_has(flag);
>
> return false;
> }
>
> #else
> -
> static inline bool protected_guest_has(unsigned long flag) { return false; }
> -
> #endif
>
> -#endif
> +#endif /* _LINUX_PROTECTED_GUEST_H */
>
>

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

2021-06-07 18:27:30

by Borislav Petkov

[permalink] [raw]
Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction

On Mon, Jun 07, 2021 at 11:01:05AM -0700, Kuppuswamy, Sathyanarayanan wrote:
> Why move this header outside CONFIG_INTEL_TDX_GUEST or
> CONFIG_AMD_MEM_ENCRYPT ifdef?

Because asm headers are usually included at the beginning of another,
possibly generic header. Unless you have a specially particular
reason to put them in additional guarding ifdeffery. Have a look at
include/linux/.

> This header only exists in x86 arch code. So it is better to protect
> it with x86 specific header file.

That doesn't sound like a special reason to me. And compilers are
usually very able at discarding unused symbols so I don't see a problem
with keeping all includes at the top, like it is usually done.

--
Regards/Gruss,
Boris.

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

Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction



On 6/7/21 11:26 AM, Borislav Petkov wrote:
>> This header only exists in x86 arch code. So it is better to protect
>> it with x86 specific header file.
> That doesn't sound like a special reason to me. And compilers are
> usually very able at discarding unused symbols so I don't see a problem
> with keeping all includes at the top, like it is usually done.

I am still not clear. What happens when a driver which includes
linux/protected-guest.h is compiled for non-x86 arch (s390 or arm64)?

Since asm/sev.h and asm/tdx.h exists only in x86_64 arch, IMO, it
should be placed under CONFIG_INTEL_TDX_GUEST or CONFIG_AMD_MEM_ENCRYPT

did I miss anything?

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

2021-06-09 17:51:13

by Borislav Petkov

[permalink] [raw]
Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction

On Wed, Jun 09, 2021 at 07:01:13AM -0700, Kuppuswamy, Sathyanarayanan wrote:
> I am still not clear. What happens when a driver which includes
> linux/protected-guest.h is compiled for non-x86 arch (s390 or arm64)?

I was wondering what felt weird: why is prot{ected,}_guest_has() in a
generic linux/ namespace header and not in an asm/ one?

I think the proper way is for the other arches should be to provide
their own prot_guest_has() implementation which generic code uses and
the generic header would contain only the PR_GUEST_* defines.

Take ioremap() as an example:

arch/x86/include/asm/io.h
arch/arm64/include/asm/io.h
arch/s390/include/asm/io.h
...

and pretty much every arch has that arch-specific io.h header which
defines ioremap() and generic code includes include/linux/io.h which
includes the respective asm/io.h header so that users can call the
respective ioremap() implementation.

prot_guest_has() sounds just the same to me.

Better?

--
Regards/Gruss,
Boris.

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

Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction



On 6/9/21 7:32 AM, Borislav Petkov wrote:
> On Wed, Jun 09, 2021 at 07:01:13AM -0700, Kuppuswamy, Sathyanarayanan wrote:
>> I am still not clear. What happens when a driver which includes
>> linux/protected-guest.h is compiled for non-x86 arch (s390 or arm64)?
>
> I was wondering what felt weird: why is prot{ected,}_guest_has() in a
> generic linux/ namespace header and not in an asm/ one?
>
> I think the proper way is for the other arches should be to provide
> their own prot_guest_has() implementation which generic code uses and
> the generic header would contain only the PR_GUEST_* defines.
>
> Take ioremap() as an example:
>
> arch/x86/include/asm/io.h
> arch/arm64/include/asm/io.h
> arch/s390/include/asm/io.h
> ...
>
> and pretty much every arch has that arch-specific io.h header which
> defines ioremap() and generic code includes include/linux/io.h which
> includes the respective asm/io.h header so that users can call the
> respective ioremap() implementation.
>
> prot_guest_has() sounds just the same to me.

ioremap() is required for all architectures. So I think adding support for it
and creating io.h for every arch seems valid. But are you sure every arch cares
about protected guest support?

IMHO, its better to leave it to arch maintainers to decide if they want
to support protected guest or not.

This can be easily achieved by defining generic arch independent config
option ARCH_HAS_PORTECTED_GUEST.

And any arch which wants to support prot_guest_has() can enable above
config option and create their own asm/protected_guest.h

This model is similar to linux/mem_encrypt.h.

With above suggested change, header file will look like below. And we
don't need implement asm/protected_guest.h for every available arch.

--- a/include/linux/protected_guest.h
+++ b/include/linux/protected_guest.h

#ifndef _LINUX_PROTECTED_GUEST_H
#define _LINUX_PROTECTED_GUEST_H 1

/* Protected Guest Feature Flags (leave 0-0xff for arch specific flags) */

/* Support for guest encryption */
#define PR_GUEST_MEM_ENCRYPT 0x100
/* Encryption support is active */
#define PR_GUEST_MEM_ENCRYPT_ACTIVE 0x101
/* Support for unrolled string IO */
#define PR_GUEST_UNROLL_STRING_IO 0x102
/* Support for host memory encryption */
#define PR_GUEST_HOST_MEM_ENCRYPT 0x103
/* Support for shared mapping initialization (after early init) */
#define PR_GUEST_SHARED_MAPPING_INIT 0x104

#ifdef ARCH_HAS_PROTECTED_GUEST
#include <asm/protected_guest.h>
#else
static inline bool prot_guest_has(unsigned long flag) { return false; }
#endif

#endif /* _LINUX_PROTECTED_GUEST_H */


>
> Better?
>

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

2021-06-09 17:58:15

by Borislav Petkov

[permalink] [raw]
Subject: Re: [RFC v2-fix-v2 1/1] x86: Introduce generic protected guest abstraction

On Wed, Jun 09, 2021 at 07:56:14AM -0700, Kuppuswamy, Sathyanarayanan wrote:
> And any arch which wants to support prot_guest_has() can enable above
> config option and create their own asm/protected_guest.

I wouldnt've done even that but only the x86 asm version of
protected_guest.h and left it to other arches to extend it. I don't
like "preempting" use of functionality by other arches and would
leave them to extend stuff themselves, as they see fit, but ok,
ARCH_HAS_PROTECTED_GUEST sounds clean enough to me too, so sure, that's
fine too.

Thx.

--
Regards/Gruss,
Boris.

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

Subject: [RFC v2-fix-v4 1/1] x86: Introduce generic protected guest abstraction

Add a generic way to check if we run with an encrypted guest,
without requiring x86 specific ifdefs. This can then be used in
non architecture specific code. 

prot_guest_has() is used to check for protected guest feature
flags.

Originally-by: Andi Kleen <[email protected]>
Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
---
Changes since RFC v2-fix-v3:
* Introduced ARCH_HAS_PROTECTED_GUEST and moved arch specific checks to
asm/protected_guest.h

Changes since RFC v2-fix-v2:
* Renamed protected_guest_has() to prot_guest_has().
* Changed flag prefix from VM_ to PR_GUEST_
* Merged Borislav AMD implementation fix.

arch/Kconfig | 3 +++
arch/x86/Kconfig | 2 ++
arch/x86/include/asm/protected_guest.h | 20 ++++++++++++++++++++
arch/x86/include/asm/sev.h | 3 +++
arch/x86/include/asm/tdx.h | 7 +++++++
arch/x86/kernel/sev.c | 15 +++++++++++++++
arch/x86/kernel/tdx.c | 15 +++++++++++++++
arch/x86/mm/mem_encrypt.c | 1 +
include/linux/protected_guest.h | 24 ++++++++++++++++++++++++
9 files changed, 90 insertions(+)
create mode 100644 arch/x86/include/asm/protected_guest.h
create mode 100644 include/linux/protected_guest.h

diff --git a/arch/Kconfig b/arch/Kconfig
index c45b770d3579..3c5bf55ee752 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1011,6 +1011,9 @@ config HAVE_ARCH_NVRAM_OPS
config ISA_BUS_API
def_bool ISA

+config ARCH_HAS_PROTECTED_GUEST
+ bool
+
#
# ABI hall of shame
#
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index a99adc683db9..fc51579e54ad 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -883,6 +883,7 @@ config INTEL_TDX_GUEST
select PARAVIRT_XL
select X86_X2APIC
select SECURITY_LOCKDOWN_LSM
+ select ARCH_HAS_PROTECTED_GUEST
help
Provide support for running in a trusted domain on Intel processors
equipped with Trusted Domain eXtenstions. TDX is a new Intel
@@ -1544,6 +1545,7 @@ config AMD_MEM_ENCRYPT
select ARCH_HAS_FORCE_DMA_UNENCRYPTED
select INSTRUCTION_DECODER
select ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
+ select ARCH_HAS_PROTECTED_GUEST
help
Say yes to enable support for the encryption of system memory.
This requires an AMD processor that supports Secure Memory
diff --git a/arch/x86/include/asm/protected_guest.h b/arch/x86/include/asm/protected_guest.h
new file mode 100644
index 000000000000..137976ef894a
--- /dev/null
+++ b/arch/x86/include/asm/protected_guest.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright (C) 2020 Intel Corporation */
+#ifndef _ASM_PROTECTED_GUEST_H
+#define _ASM_PROTECTED_GUEST_H 1
+
+#include <asm/processor.h>
+#include <asm/tdx.h>
+#include <asm/sev.h>
+
+static inline bool prot_guest_has(unsigned long flag)
+{
+ if (is_tdx_guest())
+ return tdx_protected_guest_has(flag);
+ else if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+ return sev_protected_guest_has(flag);
+
+ return false;
+}
+
+#endif /* _ASM_PROTECTED_GUEST_H */
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index fa5cd05d3b5b..e9b0b93a3157 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -81,12 +81,15 @@ static __always_inline void sev_es_nmi_complete(void)
__sev_es_nmi_complete();
}
extern int __init sev_es_efi_map_ghcbs(pgd_t *pgd);
+bool sev_protected_guest_has(unsigned long flag);
+
#else
static inline void sev_es_ist_enter(struct pt_regs *regs) { }
static inline void sev_es_ist_exit(void) { }
static inline int sev_es_setup_ap_jump_table(struct real_mode_header *rmh) { return 0; }
static inline void sev_es_nmi_complete(void) { }
static inline int sev_es_efi_map_ghcbs(pgd_t *pgd) { return 0; }
+static inline bool sev_protected_guest_has(unsigned long flag) { return false; }
#endif

#endif
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index f0c1912837c8..cbfe7479f2a3 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -71,6 +71,8 @@ u64 __tdx_module_call(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
u64 __tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14, u64 r15,
struct tdx_hypercall_output *out);

+bool tdx_protected_guest_has(unsigned long flag);
+
#else // !CONFIG_INTEL_TDX_GUEST

static inline bool is_tdx_guest(void)
@@ -80,6 +82,11 @@ static inline bool is_tdx_guest(void)

static inline void tdx_early_init(void) { };

+static inline bool tdx_protected_guest_has(unsigned long flag)
+{
+ return false;
+}
+
#endif /* CONFIG_INTEL_TDX_GUEST */

#ifdef CONFIG_INTEL_TDX_GUEST_KVM
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 651b81cd648e..16e5c5f25e6f 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -19,6 +19,7 @@
#include <linux/memblock.h>
#include <linux/kernel.h>
#include <linux/mm.h>
+#include <linux/protected_guest.h>

#include <asm/cpu_entry_area.h>
#include <asm/stacktrace.h>
@@ -1493,3 +1494,17 @@ bool __init handle_vc_boot_ghcb(struct pt_regs *regs)
while (true)
halt();
}
+
+bool sev_protected_guest_has(unsigned long flag)
+{
+ switch (flag) {
+ case PR_GUEST_MEM_ENCRYPT:
+ case PR_GUEST_MEM_ENCRYPT_ACTIVE:
+ case PR_GUEST_UNROLL_STRING_IO:
+ case PR_GUEST_HOST_MEM_ENCRYPT:
+ return true;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL_GPL(sev_protected_guest_has);
diff --git a/arch/x86/kernel/tdx.c b/arch/x86/kernel/tdx.c
index 17725646eb30..111f15c05e24 100644
--- a/arch/x86/kernel/tdx.c
+++ b/arch/x86/kernel/tdx.c
@@ -7,6 +7,7 @@
#include <asm/vmx.h>

#include <linux/cpu.h>
+#include <linux/protected_guest.h>

/* TDX Module call Leaf IDs */
#define TDINFO 1
@@ -75,6 +76,20 @@ bool is_tdx_guest(void)
}
EXPORT_SYMBOL_GPL(is_tdx_guest);

+bool tdx_protected_guest_has(unsigned long flag)
+{
+ switch (flag) {
+ case PR_GUEST_MEM_ENCRYPT:
+ case PR_GUEST_MEM_ENCRYPT_ACTIVE:
+ case PR_GUEST_UNROLL_STRING_IO:
+ case PR_GUEST_SHARED_MAPPING_INIT:
+ return true;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL_GPL(tdx_protected_guest_has);
+
static void tdg_get_info(void)
{
u64 ret;
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index ff08dc463634..d0026bce47df 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -20,6 +20,7 @@
#include <linux/bitops.h>
#include <linux/dma-mapping.h>
#include <linux/virtio_config.h>
+#include <linux/protected_guest.h>

#include <asm/tlbflush.h>
#include <asm/fixmap.h>
diff --git a/include/linux/protected_guest.h b/include/linux/protected_guest.h
new file mode 100644
index 000000000000..0facb8547217
--- /dev/null
+++ b/include/linux/protected_guest.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _LINUX_PROTECTED_GUEST_H
+#define _LINUX_PROTECTED_GUEST_H 1
+
+/* Protected Guest Feature Flags (leave 0-0xff for arch specific flags) */
+
+/* Support for guest encryption */
+#define PR_GUEST_MEM_ENCRYPT 0x100
+/* Encryption support is active */
+#define PR_GUEST_MEM_ENCRYPT_ACTIVE 0x101
+/* Support for unrolled string IO */
+#define PR_GUEST_UNROLL_STRING_IO 0x102
+/* Support for host memory encryption */
+#define PR_GUEST_HOST_MEM_ENCRYPT 0x103
+/* Support for shared mapping initialization (after early init) */
+#define PR_GUEST_SHARED_MAPPING_INIT 0x104
+
+#ifdef CONFIG_ARCH_HAS_PROTECTED_GUEST
+#include <asm/protected_guest.h>
+#else
+static inline bool prot_guest_has(unsigned long flag) { return false; }
+#endif
+
+#endif /* _LINUX_PROTECTED_GUEST_H */
--
2.25.1

Subject: Re: [RFC v2-fix-v4 1/1] x86: Introduce generic protected guest abstraction

Hi All,

On Wed, Jun 9, 2021 at 12:42 PM Kuppuswamy Sathyanarayanan
<[email protected]> wrote:
>
> Add a generic way to check if we run with an encrypted guest,
> without requiring x86 specific ifdefs. This can then be used in
> non architecture specific code.
>
> prot_guest_has() is used to check for protected guest feature
> flags.
>
> Originally-by: Andi Kleen <[email protected]>
> Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
> ---

I have sent a non RFC version of this patch for x86 review. Please use
it for further
discussion.

https://lore.kernel.org/patchwork/patch/1444184/

> Changes since RFC v2-fix-v3:
> * Introduced ARCH_HAS_PROTECTED_GUEST and moved arch specific checks to
> asm/protected_guest.h
>
> Changes since RFC v2-fix-v2:
> * Renamed protected_guest_has() to prot_guest_has().
> * Changed flag prefix from VM_ to PR_GUEST_
> * Merged Borislav AMD implementation fix.




--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer