[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YhNyY5ErqQHZ961+@zn.tnic>
Date: Mon, 21 Feb 2022 12:07:15 +0100
From: Borislav Petkov <bp@...en8.de>
To: "Kirill A. Shutemov" <kirill@...temov.name>
Cc: aarcange@...hat.com, ak@...ux.intel.com, dan.j.williams@...el.com,
dave.hansen@...el.com, david@...hat.com, hpa@...or.com,
jgross@...e.com, jmattson@...gle.com, joro@...tes.org,
jpoimboe@...hat.com, kirill.shutemov@...ux.intel.com,
knsathya@...nel.org, linux-kernel@...r.kernel.org, luto@...nel.org,
mingo@...hat.com, pbonzini@...hat.com, peterz@...radead.org,
sathyanarayanan.kuppuswamy@...ux.intel.com, sdeep@...are.com,
seanjc@...gle.com, tglx@...utronix.de, tony.luck@...el.com,
vkuznets@...hat.com, wanpengli@...cent.com, x86@...nel.org,
linux-hyperv@...r.kernel.org,
Brijesh Singh <brijesh.singh@....com>,
Tom Lendacky <thomas.lendacky@....com>
Subject: Re: [PATCHv3.1 2/32] x86/coco: Explicitly declare type of
confidential computing platform
On Sat, Feb 19, 2022 at 03:13:04AM +0300, Kirill A. Shutemov wrote:
> Kernel derives type of confidential computing platform from sme_me_mask
> value and hv_is_isolation_supported(). This detection process will be
> more complicated as more platforms get added.
>
> Declare confidential computing vendor explicitly via cc_init().
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> ---
> arch/x86/include/asm/coco.h | 16 ++++++++++++++++
> arch/x86/kernel/cc_platform.c | 29 +++++++++++++++++------------
> arch/x86/kernel/cpu/mshyperv.c | 3 +++
> arch/x86/mm/mem_encrypt_identity.c | 8 +++++---
> 4 files changed, 41 insertions(+), 15 deletions(-)
> create mode 100644 arch/x86/include/asm/coco.h
>
> diff --git a/arch/x86/include/asm/coco.h b/arch/x86/include/asm/coco.h
> new file mode 100644
> index 000000000000..6e770e0dd683
> --- /dev/null
> +++ b/arch/x86/include/asm/coco.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_X86_COCO_H
> +#define _ASM_X86_COCO_H
> +
> +#include <asm/pgtable_types.h>
> +
> +enum cc_vendor {
> + CC_VENDOR_NONE,
> + CC_VENDOR_AMD,
> + CC_VENDOR_HYPERV,
> + CC_VENDOR_INTEL,
> +};
> +
> +void cc_init(enum cc_vendor);
> +
> +#endif /* _ASM_X86_COCO_H */
> diff --git a/arch/x86/kernel/cc_platform.c b/arch/x86/kernel/cc_platform.c
> index 6a6ffcd978f6..891d3074a16e 100644
> --- a/arch/x86/kernel/cc_platform.c
> +++ b/arch/x86/kernel/cc_platform.c
> @@ -9,18 +9,15 @@
>
> #include <linux/export.h>
> #include <linux/cc_platform.h>
> -#include <linux/mem_encrypt.h>
>
> -#include <asm/mshyperv.h>
> +#include <asm/coco.h>
> #include <asm/processor.h>
>
> -static bool __maybe_unused intel_cc_platform_has(enum cc_attr attr)
> +static enum cc_vendor cc_vendor;
static enum cc_vendor vendor __ro_after_init;
> +
> +static bool intel_cc_platform_has(enum cc_attr attr)
> {
> -#ifdef CONFIG_INTEL_TDX_GUEST
> - return false;
> -#else
> return false;
> -#endif
> }
>
> /*
> @@ -74,12 +71,20 @@ static bool hyperv_cc_platform_has(enum cc_attr attr)
>
> bool cc_platform_has(enum cc_attr attr)
> {
> - if (sme_me_mask)
> + switch (cc_vendor) {
> + case CC_VENDOR_AMD:
> return amd_cc_platform_has(attr);
> -
> - if (hv_is_isolation_supported())
> + case CC_VENDOR_INTEL:
> + return intel_cc_platform_has(attr);
> + case CC_VENDOR_HYPERV:
> return hyperv_cc_platform_has(attr);
> -
> - return false;
> + default:
> + return false;
> + }
> }
> EXPORT_SYMBOL_GPL(cc_platform_has);
> +
> +__init void cc_init(enum cc_vendor vendor)
> +{
> + cc_vendor = vendor;
> +}
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 5a99f993e639..d77cf3a31f07 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -33,6 +33,7 @@
> #include <asm/nmi.h>
> #include <clocksource/hyperv_timer.h>
> #include <asm/numa.h>
> +#include <asm/coco.h>
>
> /* Is Linux running as the root partition? */
> bool hv_root_partition;
> @@ -344,6 +345,8 @@ static void __init ms_hyperv_init_platform(void)
> */
> swiotlb_force = SWIOTLB_FORCE;
> #endif
> + if (hv_get_isolation_type() != HV_ISOLATION_TYPE_NONE)
> + cc_init(CC_VENDOR_HYPERV);
Isn't that supposed to test HV_ISOLATION_TYPE_SNP instead?
I mean, I have no clue what HV_ISOLATION_TYPE_VBS is. It is not used
anywhere in the tree either.
a6c76bb08dc7 ("x86/hyperv: Load/save the Isolation Configuration leaf")
calls it "'VBS' (software-based isolation)" - whatever that means - so
I'm not sure that is going to need the cc-facilities.
For stuff like that you need to use get_maintainers.pl and Cc them
folks:
$ git log -p -1 | ./scripts/get_maintainer.pl | grep -i hyper
"K. Y. Srinivasan" <kys@...rosoft.com> (supporter:Hyper-V/Azure CORE AND DRIVERS)
Haiyang Zhang <haiyangz@...rosoft.com> (supporter:Hyper-V/Azure CORE AND DRIVERS)
Stephen Hemminger <sthemmin@...rosoft.com> (supporter:Hyper-V/Azure CORE AND DRIVERS)
Wei Liu <wei.liu@...nel.org> (supporter:Hyper-V/Azure CORE AND DRIVERS,commit_signer:1/4=25%)
Dexuan Cui <decui@...rosoft.com> (supporter:Hyper-V/Azure CORE AND DRIVERS)
linux-hyperv@...r.kernel.org (open list:Hyper-V/Azure CORE AND DRIVERS)
/me adds the ML to Cc.
> if (hv_max_functions_eax >= HYPERV_CPUID_NESTED_FEATURES) {
> diff --git a/arch/x86/mm/mem_encrypt_identity.c b/arch/x86/mm/mem_encrypt_identity.c
> index 3f0abb403340..eb7fbd85b77e 100644
> --- a/arch/x86/mm/mem_encrypt_identity.c
> +++ b/arch/x86/mm/mem_encrypt_identity.c
> @@ -44,6 +44,7 @@
> #include <asm/setup.h>
> #include <asm/sections.h>
> #include <asm/cmdline.h>
> +#include <asm/coco.h>
>
> #include "mm_internal.h"
>
> @@ -565,8 +566,7 @@ void __init sme_enable(struct boot_params *bp)
> } else {
> /* SEV state cannot be controlled by a command line option */
> sme_me_mask = me_mask;
> - physical_mask &= ~sme_me_mask;
> - return;
> + goto out;
> }
>
> /*
> @@ -600,6 +600,8 @@ void __init sme_enable(struct boot_params *bp)
> sme_me_mask = 0;
> else
> sme_me_mask = active_by_default ? me_mask : 0;
> -
> +out:
> physical_mask &= ~sme_me_mask;
> + if (sme_me_mask)
> + cc_init(CC_VENDOR_AMD);
> }
I guess.
Adding SEV folks to Cc too.
Please use get_maintainer.pl - you should know that - you're not some
newbie who started doing kernel work two weeks ago.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Powered by blists - more mailing lists