2024-03-27 15:46:00

by Borislav Petkov

Subject: [PATCH 0/5] x86/sev: Fix SNP host late disable

From: "Borislav Petkov (AMD)" <[email protected]>

Hi,

the intention to track SNP host status with the CPU feature bit
X86_FEATURE_SEV_SNP was all fine and dandy but that can't work if stuff
needs to be disabled late, after alternatives patching - see patch 5.

Therefore, convert the SNP status tracking to a cc_platform*() bit.

The first two are long overdue cleanups.

If no objections, 3-5 should go in now so that 6.9 releases fixed.

Thx.

Borislav Petkov (AMD) (5):
x86/alternatives: Remove a superfluous newline in _static_cpu_has()
x86/alternatives: Catch late X86_FEATURE modifiers
x86/kvm/Kconfig: Have KVM_AMD_SEV select ARCH_HAS_CC_PLATFORM
x86/cc: Add cc_platform_set/_clear() helpers
x86/CPU/AMD: Track SNP host status with cc_platform_*()

arch/x86/coco/core.c | 52 ++++++++++++++++++++++++++++++
arch/x86/include/asm/cpufeature.h | 11 ++++---
arch/x86/include/asm/sev.h | 4 +--
arch/x86/kernel/cpu/amd.c | 38 +++++++++++++---------
arch/x86/kernel/cpu/cpuid-deps.c | 3 ++
arch/x86/kernel/cpu/mtrr/generic.c | 2 +-
arch/x86/kernel/sev.c | 10 ------
arch/x86/kvm/Kconfig | 1 +
arch/x86/kvm/svm/sev.c | 2 +-
arch/x86/virt/svm/sev.c | 26 ++++++++++-----
drivers/crypto/ccp/sev-dev.c | 2 +-
drivers/iommu/amd/init.c | 4 ++-
include/linux/cc_platform.h | 12 +++++++
13 files changed, 124 insertions(+), 43 deletions(-)

--
2.43.0

2024-03-27 15:46:40

by Borislav Petkov

Subject: [PATCH 2/5] x86/alternatives: Catch late X86_FEATURE modifiers

From: "Borislav Petkov (AMD)" <[email protected]>

After alternatives have been patched, changes to the X86_FEATURE flags
won't take effect and could potentially even be wrong.

Warn about it.

This is something which has been long overdue.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
---
arch/x86/include/asm/cpufeature.h | 8 ++++++--
arch/x86/kernel/cpu/cpuid-deps.c | 3 +++
2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 1ef620d508f4..d0b9c411144b 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -146,8 +146,12 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
extern void setup_clear_cpu_cap(unsigned int bit);
extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);

-#define setup_force_cpu_cap(bit) do { \
- set_cpu_cap(&boot_cpu_data, bit); \
+#define setup_force_cpu_cap(bit) do { \
+ \
+ if (!boot_cpu_has(bit)) \
+ WARN_ON(alternatives_patched); \
+ \
+ set_cpu_cap(&boot_cpu_data, bit); \
set_bit(bit, (unsigned long *)cpu_caps_set); \
} while (0)

diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index b7174209d855..5dd427c6feb2 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -114,6 +114,9 @@ static void do_clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int feature)
if (WARN_ON(feature >= MAX_FEATURE_BITS))
return;

+ if (boot_cpu_has(feature))
+ WARN_ON(alternatives_patched);
+
clear_feature(c, feature);

/* Collect all features to disable, handling dependencies */
--
2.43.0

2024-03-27 15:47:19

by Borislav Petkov

Subject: [PATCH 4/5] x86/cc: Add cc_platform_set/_clear() helpers

From: "Borislav Petkov (AMD)" <[email protected]>

Add functionality to set and/or clear different attributes of the
machine as a confidential computing platform. Add the first one too:
whether the machine is running as a host for SEV-SNP guests.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
---
arch/x86/coco/core.c | 52 +++++++++++++++++++++++++++++++++++++
include/linux/cc_platform.h | 12 +++++++++
2 files changed, 64 insertions(+)

diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
index d07be9d05cd0..8c3fae23d3c6 100644
--- a/arch/x86/coco/core.c
+++ b/arch/x86/coco/core.c
@@ -16,6 +16,11 @@
enum cc_vendor cc_vendor __ro_after_init = CC_VENDOR_NONE;
u64 cc_mask __ro_after_init;

+static struct cc_attr_flags {
+ __u64 host_sev_snp : 1,
+ __resv : 63;
+} cc_flags;
+
static bool noinstr intel_cc_platform_has(enum cc_attr attr)
{
switch (attr) {
@@ -89,6 +94,9 @@ static bool noinstr amd_cc_platform_has(enum cc_attr attr)
case CC_ATTR_GUEST_SEV_SNP:
return sev_status & MSR_AMD64_SEV_SNP_ENABLED;

+ case CC_ATTR_HOST_SEV_SNP:
+ return cc_flags.host_sev_snp;
+
default:
return false;
}
@@ -148,3 +156,47 @@ u64 cc_mkdec(u64 val)
}
}
EXPORT_SYMBOL_GPL(cc_mkdec);
+
+static void amd_cc_platform_clear(enum cc_attr attr)
+{
+ switch (attr) {
+ case CC_ATTR_HOST_SEV_SNP:
+ cc_flags.host_sev_snp = 0;
+ break;
+ default:
+ break;
+ }
+}
+
+void cc_platform_clear(enum cc_attr attr)
+{
+ switch (cc_vendor) {
+ case CC_VENDOR_AMD:
+ amd_cc_platform_clear(attr);
+ break;
+ default:
+ break;
+ }
+}
+
+static void amd_cc_platform_set(enum cc_attr attr)
+{
+ switch (attr) {
+ case CC_ATTR_HOST_SEV_SNP:
+ cc_flags.host_sev_snp = 1;
+ break;
+ default:
+ break;
+ }
+}
+
+void cc_platform_set(enum cc_attr attr)
+{
+ switch (cc_vendor) {
+ case CC_VENDOR_AMD:
+ amd_cc_platform_set(attr);
+ break;
+ default:
+ break;
+ }
+}
diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h
index cb0d6cd1c12f..60693a145894 100644
--- a/include/linux/cc_platform.h
+++ b/include/linux/cc_platform.h
@@ -90,6 +90,14 @@ enum cc_attr {
* Examples include TDX Guest.
*/
CC_ATTR_HOTPLUG_DISABLED,
+
+ /**
+ * @CC_ATTR_HOST_SEV_SNP: AMD SNP enabled on the host.
+ *
+ * The host kernel is running with the necessary features
+ * enabled to run SEV-SNP guests.
+ */
+ CC_ATTR_HOST_SEV_SNP,
};

#ifdef CONFIG_ARCH_HAS_CC_PLATFORM
@@ -107,10 +115,14 @@ enum cc_attr {
* * FALSE - Specified Confidential Computing attribute is not active
*/
bool cc_platform_has(enum cc_attr attr);
+void cc_platform_set(enum cc_attr attr);
+void cc_platform_clear(enum cc_attr attr);

#else /* !CONFIG_ARCH_HAS_CC_PLATFORM */

static inline bool cc_platform_has(enum cc_attr attr) { return false; }
+static inline void cc_platform_set(enum cc_attr attr) { }
+static inline void cc_platform_clear(enum cc_attr attr) { }

#endif /* CONFIG_ARCH_HAS_CC_PLATFORM */

--
2.43.0

2024-03-27 15:51:35

by Borislav Petkov

Subject: [PATCH 1/5] x86/alternatives: Remove a superfluous newline in _static_cpu_has()

From: "Borislav Petkov (AMD)" <[email protected]>

No functional changes.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
---
arch/x86/include/asm/cpufeature.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index a1273698fc43..1ef620d508f4 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -168,8 +168,7 @@ extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);
*/
static __always_inline bool _static_cpu_has(u16 bit)
{
- asm goto(
- ALTERNATIVE_TERNARY("jmp 6f", %P[feature], "", "jmp %l[t_no]")
+ asm goto(ALTERNATIVE_TERNARY("jmp 6f", %P[feature], "", "jmp %l[t_no]")
".pushsection .altinstr_aux,\"ax\"\n"
"6:\n"
" testb %[bitnum]," _ASM_RIP(%P[cap_byte]) "\n"
--
2.43.0

2024-03-27 16:01:33

by Nikolay Borisov

Subject: Re: [PATCH 2/5] x86/alternatives: Catch late X86_FEATURE modifiers

On 27.03.24 г. 17:43 ч., Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <[email protected]>
>
> After alternatives have been patched, changes to the X86_FEATURE flags
> won't take effect and could potentially even be wrong.
>
> Warn about it.
>
> This is something which has been long overdue.
>
> Signed-off-by: Borislav Petkov (AMD) <[email protected]>

nit: While cleaning this bit mind if you also switch
alternatives_patched to a bool?

2024-03-27 16:10:51

by Borislav Petkov

Subject: [PATCH 5/5] x86/CPU/AMD: Track SNP host status with cc_platform_*()

From: "Borislav Petkov (AMD)" <[email protected]>

The host SNP worthiness can determined later, after alternatives have
been patched, in snp_rmptable_init() depending on cmdline options like
iommu=pt which is incompatible with SNP, for example.

Which means that one cannot use X86_FEATURE_SEV_SNP and will need to
have a special flag for that control.

Use that newly added CC_ATTR_HOST_SEV_SNP in the appropriate places.

Move kdump_sev_callback() to its rightfull place, while at it.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
---
arch/x86/include/asm/sev.h | 4 ++--
arch/x86/kernel/cpu/amd.c | 38 ++++++++++++++++++------------
arch/x86/kernel/cpu/mtrr/generic.c | 2 +-
arch/x86/kernel/sev.c | 10 --------
arch/x86/kvm/svm/sev.c | 2 +-
arch/x86/virt/svm/sev.c | 26 +++++++++++++-------
drivers/crypto/ccp/sev-dev.c | 2 +-
drivers/iommu/amd/init.c | 4 +++-
8 files changed, 49 insertions(+), 39 deletions(-)

diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 9477b4053bce..780182cda3ab 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -228,7 +228,6 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct sn
void snp_accept_memory(phys_addr_t start, phys_addr_t end);
u64 snp_get_unsupported_features(u64 status);
u64 sev_get_status(void);
-void kdump_sev_callback(void);
void sev_show_status(void);
#else
static inline void sev_es_ist_enter(struct pt_regs *regs) { }
@@ -258,7 +257,6 @@ static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *in
static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { }
static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
static inline u64 sev_get_status(void) { return 0; }
-static inline void kdump_sev_callback(void) { }
static inline void sev_show_status(void) { }
#endif

@@ -270,6 +268,7 @@ int psmash(u64 pfn);
int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, u32 asid, bool immutable);
int rmp_make_shared(u64 pfn, enum pg_level level);
void snp_leak_pages(u64 pfn, unsigned int npages);
+void kdump_sev_callback(void);
#else
static inline bool snp_probe_rmptable_info(void) { return false; }
static inline int snp_lookup_rmpentry(u64 pfn, bool *assigned, int *level) { return -ENODEV; }
@@ -282,6 +281,7 @@ static inline int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, u32 as
}
static inline int rmp_make_shared(u64 pfn, enum pg_level level) { return -ENODEV; }
static inline void snp_leak_pages(u64 pfn, unsigned int npages) {}
+static inline void kdump_sev_callback(void) { }
#endif

#endif
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 6d8677e80ddb..9bf17c9c29da 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -345,6 +345,28 @@ static void srat_detect_node(struct cpuinfo_x86 *c)
#endif
}

+static void bsp_determine_snp(struct cpuinfo_x86 *c)
+{
+#ifdef CONFIG_ARCH_HAS_CC_PLATFORM
+ cc_vendor = CC_VENDOR_AMD;
+
+ if (cpu_has(c, X86_FEATURE_SEV_SNP)) {
+ /*
+ * RMP table entry format is not architectural and is defined by the
+ * per-processor PPR. Restrict SNP support on the known CPU models
+ * for which the RMP table entry format is currently defined for.
+ */
+ if (!cpu_has(c, X86_FEATURE_HYPERVISOR) &&
+ c->x86 >= 0x19 && snp_probe_rmptable_info()) {
+ cc_platform_set(CC_ATTR_HOST_SEV_SNP);
+ } else {
+ setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
+ cc_platform_clear(CC_ATTR_HOST_SEV_SNP);
+ }
+ }
+#endif
+}
+
static void bsp_init_amd(struct cpuinfo_x86 *c)
{
if (cpu_has(c, X86_FEATURE_CONSTANT_TSC)) {
@@ -452,21 +474,7 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
break;
}

- if (cpu_has(c, X86_FEATURE_SEV_SNP)) {
- /*
- * RMP table entry format is not architectural and it can vary by processor
- * and is defined by the per-processor PPR. Restrict SNP support on the
- * known CPU model and family for which the RMP table entry format is
- * currently defined for.
- */
- if (!boot_cpu_has(X86_FEATURE_ZEN3) &&
- !boot_cpu_has(X86_FEATURE_ZEN4) &&
- !boot_cpu_has(X86_FEATURE_ZEN5))
- setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
- else if (!snp_probe_rmptable_info())
- setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
- }
-
+ bsp_determine_snp(c);
return;

warn:
diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index 422a4ddc2ab7..7b29ebda024f 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -108,7 +108,7 @@ static inline void k8_check_syscfg_dram_mod_en(void)
(boot_cpu_data.x86 >= 0x0f)))
return;

- if (cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return;

rdmsr(MSR_AMD64_SYSCFG, lo, hi);
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index b59b09c2f284..1e1a3c3bd1e8 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2287,16 +2287,6 @@ static int __init snp_init_platform_device(void)
}
device_initcall(snp_init_platform_device);

-void kdump_sev_callback(void)
-{
- /*
- * Do wbinvd() on remote CPUs when SNP is enabled in order to
- * safely do SNP_SHUTDOWN on the local CPU.
- */
- if (cpu_feature_enabled(X86_FEATURE_SEV_SNP))
- wbinvd();
-}
-
void sev_show_status(void)
{
int i;
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index ae0ac12382b9..3d310b473e05 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3174,7 +3174,7 @@ struct page *snp_safe_alloc_page(struct kvm_vcpu *vcpu)
unsigned long pfn;
struct page *p;

- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO);

/*
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index cffe1157a90a..ab0e8448bb6e 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -77,7 +77,7 @@ static int __mfd_enable(unsigned int cpu)
{
u64 val;

- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return 0;

rdmsrl(MSR_AMD64_SYSCFG, val);
@@ -98,7 +98,7 @@ static int __snp_enable(unsigned int cpu)
{
u64 val;

- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return 0;

rdmsrl(MSR_AMD64_SYSCFG, val);
@@ -174,11 +174,11 @@ static int __init snp_rmptable_init(void)
u64 rmptable_size;
u64 val;

- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return 0;

if (!amd_iommu_snp_en)
- return 0;
+ goto nosnp;

if (!probed_rmp_size)
goto nosnp;
@@ -225,7 +225,7 @@ static int __init snp_rmptable_init(void)
return 0;

nosnp:
- setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
+ cc_platform_clear(CC_ATTR_HOST_SEV_SNP);
return -ENOSYS;
}

@@ -246,7 +246,7 @@ static struct rmpentry *__snp_lookup_rmpentry(u64 pfn, int *level)
{
struct rmpentry *large_entry, *entry;

- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return ERR_PTR(-ENODEV);

entry = get_rmpentry(pfn);
@@ -363,7 +363,7 @@ int psmash(u64 pfn)
unsigned long paddr = pfn << PAGE_SHIFT;
int ret;

- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return -ENODEV;

if (!pfn_valid(pfn))
@@ -472,7 +472,7 @@ static int rmpupdate(u64 pfn, struct rmp_state *state)
unsigned long paddr = pfn << PAGE_SHIFT;
int ret, level;

- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return -ENODEV;

level = RMP_TO_PG_LEVEL(state->pagesize);
@@ -558,3 +558,13 @@ void snp_leak_pages(u64 pfn, unsigned int npages)
spin_unlock(&snp_leaked_pages_list_lock);
}
EXPORT_SYMBOL_GPL(snp_leak_pages);
+
+void kdump_sev_callback(void)
+{
+ /*
+ * Do wbinvd() on remote CPUs when SNP is enabled in order to
+ * safely do SNP_SHUTDOWN on the local CPU.
+ */
+ if (cc_platform_has(CC_ATTR_HOST_SEV_SNP))
+ wbinvd();
+}
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index f44efbb89c34..2102377f727b 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1090,7 +1090,7 @@ static int __sev_snp_init_locked(int *error)
void *arg = &data;
int cmd, rc = 0;

- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return -ENODEV;

sev = psp->sev_data;
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index e7a44929f0da..33228c1c8980 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -3228,7 +3228,7 @@ static bool __init detect_ivrs(void)
static void iommu_snp_enable(void)
{
#ifdef CONFIG_KVM_AMD_SEV
- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return;
/*
* The SNP support requires that IOMMU must be enabled, and is
@@ -3236,12 +3236,14 @@ static void iommu_snp_enable(void)
*/
if (no_iommu || iommu_default_passthrough()) {
pr_err("SNP: IOMMU disabled or configured in passthrough mode, SNP cannot be supported.\n");
+ cc_platform_clear(CC_ATTR_HOST_SEV_SNP);
return;
}

amd_iommu_snp_en = check_feature(FEATURE_SNP);
if (!amd_iommu_snp_en) {
pr_err("SNP: IOMMU SNP feature not enabled, SNP cannot be supported.\n");
+ cc_platform_clear(CC_ATTR_HOST_SEV_SNP);
return;
}

--
2.43.0

2024-03-27 16:59:41

by Borislav Petkov

Subject: [PATCH 3/5] x86/kvm/Kconfig: Have KVM_AMD_SEV select ARCH_HAS_CC_PLATFORM

From: "Borislav Petkov (AMD)" <[email protected]>

The functionality to load SEV-SNP guests by the host will soon rely on
cc_platform* helpers because the cpu_feature* API with the early
patching is insufficient when SNP support needs to be disabled late.

Therefore, pull that functionality in.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
---
arch/x86/kvm/Kconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 3aaf7e86a859..0ebdd088f28b 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -122,6 +122,7 @@ config KVM_AMD_SEV
default y
depends on KVM_AMD && X86_64
depends on CRYPTO_DEV_SP_PSP && !(KVM_AMD=y && CRYPTO_DEV_CCP_DD=m)
+ select ARCH_HAS_CC_PLATFORM
help
Provides support for launching Encrypted VMs (SEV) and Encrypted VMs
with Encrypted State (SEV-ES) on AMD processors.
--
2.43.0

2024-03-28 11:51:31

by Jeremi Piotrowski

Subject: Re: [PATCH 5/5] x86/CPU/AMD: Track SNP host status with cc_platform_*()

On 27/03/2024 16:43, Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <[email protected]>
>
> The host SNP worthiness can determined later, after alternatives have
> been patched, in snp_rmptable_init() depending on cmdline options like
> iommu=pt which is incompatible with SNP, for example.
>
> Which means that one cannot use X86_FEATURE_SEV_SNP and will need to
> have a special flag for that control.
>
> Use that newly added CC_ATTR_HOST_SEV_SNP in the appropriate places.
>
> Move kdump_sev_callback() to its rightfull place, while at it.
>
> Signed-off-by: Borislav Petkov (AMD) <[email protected]>
> ---
> arch/x86/include/asm/sev.h | 4 ++--
> arch/x86/kernel/cpu/amd.c | 38 ++++++++++++++++++------------
> arch/x86/kernel/cpu/mtrr/generic.c | 2 +-
> arch/x86/kernel/sev.c | 10 --------
> arch/x86/kvm/svm/sev.c | 2 +-
> arch/x86/virt/svm/sev.c | 26 +++++++++++++-------
> drivers/crypto/ccp/sev-dev.c | 2 +-
> drivers/iommu/amd/init.c | 4 +++-
> 8 files changed, 49 insertions(+), 39 deletions(-)
>
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 9477b4053bce..780182cda3ab 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -228,7 +228,6 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct sn
> void snp_accept_memory(phys_addr_t start, phys_addr_t end);
> u64 snp_get_unsupported_features(u64 status);
> u64 sev_get_status(void);
> -void kdump_sev_callback(void);
> void sev_show_status(void);
> #else
> static inline void sev_es_ist_enter(struct pt_regs *regs) { }
> @@ -258,7 +257,6 @@ static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *in
> static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { }
> static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
> static inline u64 sev_get_status(void) { return 0; }
> -static inline void kdump_sev_callback(void) { }
> static inline void sev_show_status(void) { }
> #endif
>
> @@ -270,6 +268,7 @@ int psmash(u64 pfn);
> int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, u32 asid, bool immutable);
> int rmp_make_shared(u64 pfn, enum pg_level level);
> void snp_leak_pages(u64 pfn, unsigned int npages);
> +void kdump_sev_callback(void);
> #else
> static inline bool snp_probe_rmptable_info(void) { return false; }
> static inline int snp_lookup_rmpentry(u64 pfn, bool *assigned, int *level) { return -ENODEV; }
> @@ -282,6 +281,7 @@ static inline int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, u32 as
> }
> static inline int rmp_make_shared(u64 pfn, enum pg_level level) { return -ENODEV; }
> static inline void snp_leak_pages(u64 pfn, unsigned int npages) {}
> +static inline void kdump_sev_callback(void) { }
> #endif
>
> #endif
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 6d8677e80ddb..9bf17c9c29da 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -345,6 +345,28 @@ static void srat_detect_node(struct cpuinfo_x86 *c)
> #endif
> }
>
> +static void bsp_determine_snp(struct cpuinfo_x86 *c)
> +{
> +#ifdef CONFIG_ARCH_HAS_CC_PLATFORM
> + cc_vendor = CC_VENDOR_AMD;

Shouldn't this line be inside the cpu_has(c, X86_FEATURE_SEV_SNP) check?

> +
> + if (cpu_has(c, X86_FEATURE_SEV_SNP)) {
> + /*
> + * RMP table entry format is not architectural and is defined by the
> + * per-processor PPR. Restrict SNP support on the known CPU models
> + * for which the RMP table entry format is currently defined for.
> + */> + if (!cpu_has(c, X86_FEATURE_HYPERVISOR) &&

How about turning this into a more specific check:

if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP) &&

Thanks,
Jeremi

> + c->x86 >= 0x19 && snp_probe_rmptable_info()) {
> + cc_platform_set(CC_ATTR_HOST_SEV_SNP);
> + } else {
> + setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
> + cc_platform_clear(CC_ATTR_HOST_SEV_SNP);
> + }
> + }
> +#endif
> +}
> +
> static void bsp_init_amd(struct cpuinfo_x86 *c)
> {
> if (cpu_has(c, X86_FEATURE_CONSTANT_TSC)) {
> @@ -452,21 +474,7 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
> break;
> }
>
> - if (cpu_has(c, X86_FEATURE_SEV_SNP)) {
> - /*
> - * RMP table entry format is not architectural and it can vary by processor
> - * and is defined by the per-processor PPR. Restrict SNP support on the
> - * known CPU model and family for which the RMP table entry format is
> - * currently defined for.
> - */
> - if (!boot_cpu_has(X86_FEATURE_ZEN3) &&
> - !boot_cpu_has(X86_FEATURE_ZEN4) &&
> - !boot_cpu_has(X86_FEATURE_ZEN5))
> - setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
> - else if (!snp_probe_rmptable_info())
> - setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
> - }
> -
> + bsp_determine_snp(c);
> return;
>
> warn:

2024-03-28 13:44:15

by Borislav Petkov

Subject: Re: [PATCH 5/5] x86/CPU/AMD: Track SNP host status with cc_platform_*()

On Thu, Mar 28, 2024 at 12:51:17PM +0100, Jeremi Piotrowski wrote:
> Shouldn't this line be inside the cpu_has(c, X86_FEATURE_SEV_SNP) check?

The cc_vendor is not dependent on X86_FEATURE_SEV_SNP.

> How about turning this into a more specific check:
>
> if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP) &&

Why?

The check is "am I running as a hypervisor on baremetal".

--
Regards/Gruss,
Boris.

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

2024-03-28 14:24:43

by Jeremi Piotrowski

Subject: Re: [PATCH 5/5] x86/CPU/AMD: Track SNP host status with cc_platform_*()

On 28/03/2024 14:41, Borislav Petkov wrote:
> On Thu, Mar 28, 2024 at 12:51:17PM +0100, Jeremi Piotrowski wrote:
>> Shouldn't this line be inside the cpu_has(c, X86_FEATURE_SEV_SNP) check?
>
> The cc_vendor is not dependent on X86_FEATURE_SEV_SNP.
>

It's not but if you set it before the check it will be set for all AMD systems,
even if they are neither CC hosts nor CC guests.

cc_vendor being unset is handled correctly in cc_platform_has() checks.

>> How about turning this into a more specific check:
>>
>> if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP) &&
>
> Why?
>

To leave open the possibility of an SNP hypervisor running nested.

> The check is "am I running as a hypervisor on baremetal".
>

I thought you wanted to filter out SEV-SNP guests, which also have X86_FEATURE_SEV_SNP
CPUID bit set.

My understanding is that these are the cases:

CPUID(SEV_SNP) | MSR(SEV_SNP) | what am I
---------------------------------------------
set | set | SNP-guest
set | unset | SNP-host
unset | ?? | not SNP

2024-03-28 15:40:23

by Borislav Petkov

Subject: Re: [PATCH 5/5] x86/CPU/AMD: Track SNP host status with cc_platform_*()

On Thu, Mar 28, 2024 at 03:24:29PM +0100, Jeremi Piotrowski wrote:
> It's not but if you set it before the check it will be set for all AMD
> systems, even if they are neither CC hosts nor CC guests.

That a problem?

It is under a CONFIG_ARCH_HAS_CC_PLATFORM...

> To leave open the possibility of an SNP hypervisor running nested.

But !CC_ATTR_GUEST_SEV_SNP doesn't mean that. It means it is not
a SEV-SNP guest.

> I thought you wanted to filter out SEV-SNP guests, which also have
> X86_FEATURE_SEV_SNP CPUID bit set.

I want to run snp_probe_rmptable_info() only on baremetal where it makes
sense.

> My understanding is that these are the cases:
>
> CPUID(SEV_SNP) | MSR(SEV_SNP) | what am I
> ---------------------------------------------
> set | set | SNP-guest
> set | unset | SNP-host
> unset | ?? | not SNP

So as you can see, we can't use X86_FEATURE_SEV_SNP for anything due to
the late disable need. So we should be moving away from it.

So we need a test for "am I a nested SNP hypervisor?"

So, can your thing clear X86_FEATURE_HYPERVISOR and thus "emulate"
baremetal?

--
Regards/Gruss,
Boris.

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

2024-03-29 15:17:04

by Tom Lendacky

Subject: Re: [PATCH 4/5] x86/cc: Add cc_platform_set/_clear() helpers

On 3/27/24 10:43, Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <[email protected]>
>
> Add functionality to set and/or clear different attributes of the
> machine as a confidential computing platform. Add the first one too:
> whether the machine is running as a host for SEV-SNP guests.
>
> Signed-off-by: Borislav Petkov (AMD) <[email protected]>

Reviewed-by: Tom Lendacky <[email protected]>

> ---
> arch/x86/coco/core.c | 52 +++++++++++++++++++++++++++++++++++++
> include/linux/cc_platform.h | 12 +++++++++
> 2 files changed, 64 insertions(+)
>

2024-03-29 15:17:52

by Tom Lendacky

Subject: Re: [PATCH 5/5] x86/CPU/AMD: Track SNP host status with cc_platform_*()

On 3/27/24 10:43, Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <[email protected]>
>
> The host SNP worthiness can determined later, after alternatives have
> been patched, in snp_rmptable_init() depending on cmdline options like
> iommu=pt which is incompatible with SNP, for example.
>
> Which means that one cannot use X86_FEATURE_SEV_SNP and will need to
> have a special flag for that control.
>
> Use that newly added CC_ATTR_HOST_SEV_SNP in the appropriate places.
>
> Move kdump_sev_callback() to its rightfull place, while at it.
>
> Signed-off-by: Borislav Petkov (AMD) <[email protected]>

If late disabling of CPU feature flags is ever supported in the future, we
should come back and possibly remove this. But until then...

Reviewed-by: Tom Lendacky <[email protected]>

> ---
> arch/x86/include/asm/sev.h | 4 ++--
> arch/x86/kernel/cpu/amd.c | 38 ++++++++++++++++++------------
> arch/x86/kernel/cpu/mtrr/generic.c | 2 +-
> arch/x86/kernel/sev.c | 10 --------
> arch/x86/kvm/svm/sev.c | 2 +-
> arch/x86/virt/svm/sev.c | 26 +++++++++++++-------
> drivers/crypto/ccp/sev-dev.c | 2 +-
> drivers/iommu/amd/init.c | 4 +++-
> 8 files changed, 49 insertions(+), 39 deletions(-)
>

2024-03-29 15:43:07

by Tom Lendacky

Subject: Re: [PATCH 3/5] x86/kvm/Kconfig: Have KVM_AMD_SEV select ARCH_HAS_CC_PLATFORM

On 3/27/24 10:43, Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <[email protected]>
>
> The functionality to load SEV-SNP guests by the host will soon rely on
> cc_platform* helpers because the cpu_feature* API with the early
> patching is insufficient when SNP support needs to be disabled late.
>
> Therefore, pull that functionality in.
>
> Signed-off-by: Borislav Petkov (AMD) <[email protected]>

Reviewed-by: Tom Lendacky <[email protected]>

> ---
> arch/x86/kvm/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>

2024-04-03 04:16:29

by Aithal, Srikanth

Subject: Re: [PATCH 0/5] x86/sev: Fix SNP host late disable

On 3/27/2024 9:13 PM, Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <[email protected]>
>
> Hi,
>
> the intention to track SNP host status with the CPU feature bit
> X86_FEATURE_SEV_SNP was all fine and dandy but that can't work if stuff
> needs to be disabled late, after alternatives patching - see patch 5.
>
> Therefore, convert the SNP status tracking to a cc_platform*() bit.
>
> The first two are long overdue cleanups.
>
> If no objections, 3-5 should go in now so that 6.9 releases fixed.
>
> Thx.
>
> Borislav Petkov (AMD) (5):
> x86/alternatives: Remove a superfluous newline in _static_cpu_has()
> x86/alternatives: Catch late X86_FEATURE modifiers
> x86/kvm/Kconfig: Have KVM_AMD_SEV select ARCH_HAS_CC_PLATFORM
> x86/cc: Add cc_platform_set/_clear() helpers
> x86/CPU/AMD: Track SNP host status with cc_platform_*()
>
> arch/x86/coco/core.c | 52 ++++++++++++++++++++++++++++++
> arch/x86/include/asm/cpufeature.h | 11 ++++---
> arch/x86/include/asm/sev.h | 4 +--
> arch/x86/kernel/cpu/amd.c | 38 +++++++++++++---------
> arch/x86/kernel/cpu/cpuid-deps.c | 3 ++
> arch/x86/kernel/cpu/mtrr/generic.c | 2 +-
> arch/x86/kernel/sev.c | 10 ------
> arch/x86/kvm/Kconfig | 1 +
> arch/x86/kvm/svm/sev.c | 2 +-
> arch/x86/virt/svm/sev.c | 26 ++++++++++-----
> drivers/crypto/ccp/sev-dev.c | 2 +-
> drivers/iommu/amd/init.c | 4 ++-
> include/linux/cc_platform.h | 12 +++++++
> 13 files changed, 124 insertions(+), 43 deletions(-)
>
Tested this patch. I could boot with snp enabled and iommu=pt mode,kexec
as well works fine. Thank you.

Tested-by: Srikanth Aithal <[email protected]>

2024-04-03 18:06:05

by Borislav Petkov

Subject: Re: [PATCH 2/5] x86/alternatives: Catch late X86_FEATURE modifiers

On Wed, Mar 27, 2024 at 05:57:01PM +0200, Nikolay Borisov wrote:
> nit: While cleaning this bit mind if you also switch alternatives_patched to
> a bool?

Busy as hell right now. But I take patches ontop. :-)

--
Regards/Gruss,
Boris.

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

2024-04-04 09:11:26

by tip-bot2 for Alexey Makhalov

Subject: [tip: x86/urgent] x86/kvm/Kconfig: Have KVM_AMD_SEV select ARCH_HAS_CC_PLATFORM

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: 54f5f47b6055c6b57cbc41a440f8ca8b2ec4275a
Gitweb: https://git.kernel.org/tip/54f5f47b6055c6b57cbc41a440f8ca8b2ec4275a
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Wed, 27 Mar 2024 16:43:15 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Thu, 04 Apr 2024 10:40:23 +02:00

x86/kvm/Kconfig: Have KVM_AMD_SEV select ARCH_HAS_CC_PLATFORM

The functionality to load SEV-SNP guests by the host will soon rely on
cc_platform* helpers because the cpu_feature* API with the early
patching is insufficient when SNP support needs to be disabled late.

Therefore, pull that functionality in.

Fixes: 216d106c7ff7 ("x86/sev: Add SEV-SNP host initialization support")
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Reviewed-by: Tom Lendacky <[email protected]>
Tested-by: Srikanth Aithal <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kvm/Kconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 3aaf7e8..0ebdd08 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -122,6 +122,7 @@ config KVM_AMD_SEV
default y
depends on KVM_AMD && X86_64
depends on CRYPTO_DEV_SP_PSP && !(KVM_AMD=y && CRYPTO_DEV_CCP_DD=m)
+ select ARCH_HAS_CC_PLATFORM
help
Provides support for launching Encrypted VMs (SEV) and Encrypted VMs
with Encrypted State (SEV-ES) on AMD processors.

2024-04-04 09:11:29

by tip-bot2 for Alexey Makhalov

Subject: [tip: x86/urgent] x86/cc: Add cc_platform_set/_clear() helpers

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: bc6f707fc0feec72acc2f49c312eb31d257363a3
Gitweb: https://git.kernel.org/tip/bc6f707fc0feec72acc2f49c312eb31d257363a3
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Wed, 27 Mar 2024 16:43:16 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Thu, 04 Apr 2024 10:40:27 +02:00

x86/cc: Add cc_platform_set/_clear() helpers

Add functionality to set and/or clear different attributes of the
machine as a confidential computing platform. Add the first one too:
whether the machine is running as a host for SEV-SNP guests.

Fixes: 216d106c7ff7 ("x86/sev: Add SEV-SNP host initialization support")
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Reviewed-by: Tom Lendacky <[email protected]>
Tested-by: Srikanth Aithal <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/coco/core.c | 52 ++++++++++++++++++++++++++++++++++++-
include/linux/cc_platform.h | 12 ++++++++-
2 files changed, 64 insertions(+)

diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
index ddd4efd..b31ef24 100644
--- a/arch/x86/coco/core.c
+++ b/arch/x86/coco/core.c
@@ -20,6 +20,11 @@
enum cc_vendor cc_vendor __ro_after_init = CC_VENDOR_NONE;
u64 cc_mask __ro_after_init;

+static struct cc_attr_flags {
+ __u64 host_sev_snp : 1,
+ __resv : 63;
+} cc_flags;
+
static bool noinstr intel_cc_platform_has(enum cc_attr attr)
{
switch (attr) {
@@ -93,6 +98,9 @@ static bool noinstr amd_cc_platform_has(enum cc_attr attr)
case CC_ATTR_GUEST_SEV_SNP:
return sev_status & MSR_AMD64_SEV_SNP_ENABLED;

+ case CC_ATTR_HOST_SEV_SNP:
+ return cc_flags.host_sev_snp;
+
default:
return false;
}
@@ -153,6 +161,50 @@ u64 cc_mkdec(u64 val)
}
EXPORT_SYMBOL_GPL(cc_mkdec);

+static void amd_cc_platform_clear(enum cc_attr attr)
+{
+ switch (attr) {
+ case CC_ATTR_HOST_SEV_SNP:
+ cc_flags.host_sev_snp = 0;
+ break;
+ default:
+ break;
+ }
+}
+
+void cc_platform_clear(enum cc_attr attr)
+{
+ switch (cc_vendor) {
+ case CC_VENDOR_AMD:
+ amd_cc_platform_clear(attr);
+ break;
+ default:
+ break;
+ }
+}
+
+static void amd_cc_platform_set(enum cc_attr attr)
+{
+ switch (attr) {
+ case CC_ATTR_HOST_SEV_SNP:
+ cc_flags.host_sev_snp = 1;
+ break;
+ default:
+ break;
+ }
+}
+
+void cc_platform_set(enum cc_attr attr)
+{
+ switch (cc_vendor) {
+ case CC_VENDOR_AMD:
+ amd_cc_platform_set(attr);
+ break;
+ default:
+ break;
+ }
+}
+
__init void cc_random_init(void)
{
/*
diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h
index cb0d6cd..60693a1 100644
--- a/include/linux/cc_platform.h
+++ b/include/linux/cc_platform.h
@@ -90,6 +90,14 @@ enum cc_attr {
* Examples include TDX Guest.
*/
CC_ATTR_HOTPLUG_DISABLED,
+
+ /**
+ * @CC_ATTR_HOST_SEV_SNP: AMD SNP enabled on the host.
+ *
+ * The host kernel is running with the necessary features
+ * enabled to run SEV-SNP guests.
+ */
+ CC_ATTR_HOST_SEV_SNP,
};

#ifdef CONFIG_ARCH_HAS_CC_PLATFORM
@@ -107,10 +115,14 @@ enum cc_attr {
* * FALSE - Specified Confidential Computing attribute is not active
*/
bool cc_platform_has(enum cc_attr attr);
+void cc_platform_set(enum cc_attr attr);
+void cc_platform_clear(enum cc_attr attr);

#else /* !CONFIG_ARCH_HAS_CC_PLATFORM */

static inline bool cc_platform_has(enum cc_attr attr) { return false; }
+static inline void cc_platform_set(enum cc_attr attr) { }
+static inline void cc_platform_clear(enum cc_attr attr) { }

#endif /* CONFIG_ARCH_HAS_CC_PLATFORM */

2024-04-04 09:11:37

by tip-bot2 for Alexey Makhalov

Subject: [tip: x86/urgent] x86/CPU/AMD: Track SNP host status with cc_platform_*()

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: 0ecaefb303de69929dc0036d5021d01cec7ea052
Gitweb: https://git.kernel.org/tip/0ecaefb303de69929dc0036d5021d01cec7ea052
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Wed, 27 Mar 2024 16:43:17 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Thu, 04 Apr 2024 10:40:30 +02:00

x86/CPU/AMD: Track SNP host status with cc_platform_*()

The host SNP worthiness can determined later, after alternatives have
been patched, in snp_rmptable_init() depending on cmdline options like
iommu=pt which is incompatible with SNP, for example.

Which means that one cannot use X86_FEATURE_SEV_SNP and will need to
have a special flag for that control.

Use that newly added CC_ATTR_HOST_SEV_SNP in the appropriate places.

Move kdump_sev_callback() to its rightful place, while at it.

Fixes: 216d106c7ff7 ("x86/sev: Add SEV-SNP host initialization support")
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Reviewed-by: Tom Lendacky <[email protected]>
Tested-by: Srikanth Aithal <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/sev.h | 4 +--
arch/x86/kernel/cpu/amd.c | 38 +++++++++++++++++------------
arch/x86/kernel/cpu/mtrr/generic.c | 2 +-
arch/x86/kernel/sev.c | 10 +--------
arch/x86/kvm/svm/sev.c | 2 +-
arch/x86/virt/svm/sev.c | 26 +++++++++++++-------
drivers/crypto/ccp/sev-dev.c | 2 +-
drivers/iommu/amd/init.c | 4 ++-
8 files changed, 49 insertions(+), 39 deletions(-)

diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 07e125f..7f57382 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -228,7 +228,6 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct sn
void snp_accept_memory(phys_addr_t start, phys_addr_t end);
u64 snp_get_unsupported_features(u64 status);
u64 sev_get_status(void);
-void kdump_sev_callback(void);
void sev_show_status(void);
#else
static inline void sev_es_ist_enter(struct pt_regs *regs) { }
@@ -258,7 +257,6 @@ static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *in
static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { }
static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
static inline u64 sev_get_status(void) { return 0; }
-static inline void kdump_sev_callback(void) { }
static inline void sev_show_status(void) { }
#endif

@@ -270,6 +268,7 @@ int psmash(u64 pfn);
int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, u32 asid, bool immutable);
int rmp_make_shared(u64 pfn, enum pg_level level);
void snp_leak_pages(u64 pfn, unsigned int npages);
+void kdump_sev_callback(void);
#else
static inline bool snp_probe_rmptable_info(void) { return false; }
static inline int snp_lookup_rmpentry(u64 pfn, bool *assigned, int *level) { return -ENODEV; }
@@ -282,6 +281,7 @@ static inline int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, u32 as
}
static inline int rmp_make_shared(u64 pfn, enum pg_level level) { return -ENODEV; }
static inline void snp_leak_pages(u64 pfn, unsigned int npages) {}
+static inline void kdump_sev_callback(void) { }
#endif

#endif
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 6d8677e..9bf17c9 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -345,6 +345,28 @@ static void srat_detect_node(struct cpuinfo_x86 *c)
#endif
}

+static void bsp_determine_snp(struct cpuinfo_x86 *c)
+{
+#ifdef CONFIG_ARCH_HAS_CC_PLATFORM
+ cc_vendor = CC_VENDOR_AMD;
+
+ if (cpu_has(c, X86_FEATURE_SEV_SNP)) {
+ /*
+ * RMP table entry format is not architectural and is defined by the
+ * per-processor PPR. Restrict SNP support on the known CPU models
+ * for which the RMP table entry format is currently defined for.
+ */
+ if (!cpu_has(c, X86_FEATURE_HYPERVISOR) &&
+ c->x86 >= 0x19 && snp_probe_rmptable_info()) {
+ cc_platform_set(CC_ATTR_HOST_SEV_SNP);
+ } else {
+ setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
+ cc_platform_clear(CC_ATTR_HOST_SEV_SNP);
+ }
+ }
+#endif
+}
+
static void bsp_init_amd(struct cpuinfo_x86 *c)
{
if (cpu_has(c, X86_FEATURE_CONSTANT_TSC)) {
@@ -452,21 +474,7 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
break;
}

- if (cpu_has(c, X86_FEATURE_SEV_SNP)) {
- /*
- * RMP table entry format is not architectural and it can vary by processor
- * and is defined by the per-processor PPR. Restrict SNP support on the
- * known CPU model and family for which the RMP table entry format is
- * currently defined for.
- */
- if (!boot_cpu_has(X86_FEATURE_ZEN3) &&
- !boot_cpu_has(X86_FEATURE_ZEN4) &&
- !boot_cpu_has(X86_FEATURE_ZEN5))
- setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
- else if (!snp_probe_rmptable_info())
- setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
- }
-
+ bsp_determine_snp(c);
return;

warn:
diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index 422a4dd..7b29ebd 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -108,7 +108,7 @@ static inline void k8_check_syscfg_dram_mod_en(void)
(boot_cpu_data.x86 >= 0x0f)))
return;

- if (cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return;

rdmsr(MSR_AMD64_SYSCFG, lo, hi);
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 7e1e63c..38ad066 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2284,16 +2284,6 @@ static int __init snp_init_platform_device(void)
}
device_initcall(snp_init_platform_device);

-void kdump_sev_callback(void)
-{
- /*
- * Do wbinvd() on remote CPUs when SNP is enabled in order to
- * safely do SNP_SHUTDOWN on the local CPU.
- */
- if (cpu_feature_enabled(X86_FEATURE_SEV_SNP))
- wbinvd();
-}
-
void sev_show_status(void)
{
int i;
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index ae0ac12..3d310b4 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3174,7 +3174,7 @@ struct page *snp_safe_alloc_page(struct kvm_vcpu *vcpu)
unsigned long pfn;
struct page *p;

- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO);

/*
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index cffe115..ab0e844 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -77,7 +77,7 @@ static int __mfd_enable(unsigned int cpu)
{
u64 val;

- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return 0;

rdmsrl(MSR_AMD64_SYSCFG, val);
@@ -98,7 +98,7 @@ static int __snp_enable(unsigned int cpu)
{
u64 val;

- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return 0;

rdmsrl(MSR_AMD64_SYSCFG, val);
@@ -174,11 +174,11 @@ static int __init snp_rmptable_init(void)
u64 rmptable_size;
u64 val;

- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return 0;

if (!amd_iommu_snp_en)
- return 0;
+ goto nosnp;

if (!probed_rmp_size)
goto nosnp;
@@ -225,7 +225,7 @@ skip_enable:
return 0;

nosnp:
- setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
+ cc_platform_clear(CC_ATTR_HOST_SEV_SNP);
return -ENOSYS;
}

@@ -246,7 +246,7 @@ static struct rmpentry *__snp_lookup_rmpentry(u64 pfn, int *level)
{
struct rmpentry *large_entry, *entry;

- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return ERR_PTR(-ENODEV);

entry = get_rmpentry(pfn);
@@ -363,7 +363,7 @@ int psmash(u64 pfn)
unsigned long paddr = pfn << PAGE_SHIFT;
int ret;

- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return -ENODEV;

if (!pfn_valid(pfn))
@@ -472,7 +472,7 @@ static int rmpupdate(u64 pfn, struct rmp_state *state)
unsigned long paddr = pfn << PAGE_SHIFT;
int ret, level;

- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return -ENODEV;

level = RMP_TO_PG_LEVEL(state->pagesize);
@@ -558,3 +558,13 @@ void snp_leak_pages(u64 pfn, unsigned int npages)
spin_unlock(&snp_leaked_pages_list_lock);
}
EXPORT_SYMBOL_GPL(snp_leak_pages);
+
+void kdump_sev_callback(void)
+{
+ /*
+ * Do wbinvd() on remote CPUs when SNP is enabled in order to
+ * safely do SNP_SHUTDOWN on the local CPU.
+ */
+ if (cc_platform_has(CC_ATTR_HOST_SEV_SNP))
+ wbinvd();
+}
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index f44efbb..2102377 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1090,7 +1090,7 @@ static int __sev_snp_init_locked(int *error)
void *arg = &data;
int cmd, rc = 0;

- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return -ENODEV;

sev = psp->sev_data;
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index e7a4492..33228c1 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -3228,7 +3228,7 @@ out:
static void iommu_snp_enable(void)
{
#ifdef CONFIG_KVM_AMD_SEV
- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
return;
/*
* The SNP support requires that IOMMU must be enabled, and is
@@ -3236,12 +3236,14 @@ static void iommu_snp_enable(void)
*/
if (no_iommu || iommu_default_passthrough()) {
pr_err("SNP: IOMMU disabled or configured in passthrough mode, SNP cannot be supported.\n");
+ cc_platform_clear(CC_ATTR_HOST_SEV_SNP);
return;
}

amd_iommu_snp_en = check_feature(FEATURE_SNP);
if (!amd_iommu_snp_en) {
pr_err("SNP: IOMMU SNP feature not enabled, SNP cannot be supported.\n");
+ cc_platform_clear(CC_ATTR_HOST_SEV_SNP);
return;
}

2024-04-04 09:11:50

by tip-bot2 for Alexey Makhalov

Subject: [tip: x86/alternatives] x86/alternatives: Catch late X86_FEATURE modifiers

The following commit has been merged into the x86/alternatives branch of tip:

Commit-ID: 97784e52f18fe6e0d6516ded8ecd3cec722ce9cc
Gitweb: https://git.kernel.org/tip/97784e52f18fe6e0d6516ded8ecd3cec722ce9cc
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Wed, 27 Mar 2024 16:43:14 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Wed, 03 Apr 2024 20:03:24 +02:00

x86/alternatives: Catch late X86_FEATURE modifiers

After alternatives have been patched, changes to the X86_FEATURE flags
won't take effect and could potentially even be wrong.

Warn about it.

This is something which has been long overdue.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Tested-by: Srikanth Aithal <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/cpufeature.h | 8 ++++++--
arch/x86/kernel/cpu/cpuid-deps.c | 3 +++
2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 7103ba1..e5d8880 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -148,8 +148,12 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
extern void setup_clear_cpu_cap(unsigned int bit);
extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);

-#define setup_force_cpu_cap(bit) do { \
- set_cpu_cap(&boot_cpu_data, bit); \
+#define setup_force_cpu_cap(bit) do { \
+ \
+ if (!boot_cpu_has(bit)) \
+ WARN_ON(alternatives_patched); \
+ \
+ set_cpu_cap(&boot_cpu_data, bit); \
set_bit(bit, (unsigned long *)cpu_caps_set); \
} while (0)

diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index b717420..5dd427c 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -114,6 +114,9 @@ static void do_clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int feature)
if (WARN_ON(feature >= MAX_FEATURE_BITS))
return;

+ if (boot_cpu_has(feature))
+ WARN_ON(alternatives_patched);
+
clear_feature(c, feature);

/* Collect all features to disable, handling dependencies */

2024-04-04 09:12:14

by tip-bot2 for Alexey Makhalov

Subject: [tip: x86/alternatives] x86/alternatives: Remove a superfluous newline in _static_cpu_has()

The following commit has been merged into the x86/alternatives branch of tip:

Commit-ID: e51d20f01f9ef5d19f1a37137696792e605b31e8
Gitweb: https://git.kernel.org/tip/e51d20f01f9ef5d19f1a37137696792e605b31e8
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Wed, 27 Mar 2024 16:43:13 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Wed, 03 Apr 2024 20:02:20 +02:00

x86/alternatives: Remove a superfluous newline in _static_cpu_has()

No functional changes.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Tested-by: Srikanth Aithal <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/cpufeature.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 42157dd..7103ba1 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -170,8 +170,7 @@ extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);
*/
static __always_inline bool _static_cpu_has(u16 bit)
{
- asm goto(
- ALTERNATIVE_TERNARY("jmp 6f", %P[feature], "", "jmp %l[t_no]")
+ asm goto(ALTERNATIVE_TERNARY("jmp 6f", %P[feature], "", "jmp %l[t_no]")
".pushsection .altinstr_aux,\"ax\"\n"
"6:\n"
" testb %[bitnum]," _ASM_RIP(%P[cap_byte]) "\n"

2024-04-04 10:16:59

by tip-bot2 for Alexey Makhalov

Subject: [tip: x86/alternatives] x86/alternatives: Catch late X86_FEATURE modifiers

The following commit has been merged into the x86/alternatives branch of tip:

Commit-ID: 4175b45dec4cd8ae4563bf724d591ab5cc0ad9ce
Gitweb: https://git.kernel.org/tip/4175b45dec4cd8ae4563bf724d591ab5cc0ad9ce
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Wed, 27 Mar 2024 16:43:14 +01:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Thu, 04 Apr 2024 12:09:17 +02:00

x86/alternatives: Catch late X86_FEATURE modifiers

After alternatives have been patched, changes to the X86_FEATURE flags
won't take effect and could potentially even be wrong.

Warn about it.

This is something which has been long overdue.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Tested-by: Srikanth Aithal <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/cpufeature.h | 8 ++++++--
arch/x86/kernel/cpu/cpuid-deps.c | 3 +++
2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 17fd7be..f8d7a06 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -148,8 +148,12 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
extern void setup_clear_cpu_cap(unsigned int bit);
extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);

-#define setup_force_cpu_cap(bit) do { \
- set_cpu_cap(&boot_cpu_data, bit); \
+#define setup_force_cpu_cap(bit) do { \
+ \
+ if (!boot_cpu_has(bit)) \
+ WARN_ON(alternatives_patched); \
+ \
+ set_cpu_cap(&boot_cpu_data, bit); \
set_bit(bit, (unsigned long *)cpu_caps_set); \
} while (0)

diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index b717420..5dd427c 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -114,6 +114,9 @@ static void do_clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int feature)
if (WARN_ON(feature >= MAX_FEATURE_BITS))
return;

+ if (boot_cpu_has(feature))
+ WARN_ON(alternatives_patched);
+
clear_feature(c, feature);

/* Collect all features to disable, handling dependencies */

2024-04-04 10:32:05

by tip-bot2 for Alexey Makhalov

Subject: [tip: x86/alternatives] x86/alternatives: Remove a superfluous newline in _static_cpu_has()

The following commit has been merged into the x86/alternatives branch of tip:

Commit-ID: f317392a317a27a78e755297505e57a6b345f4de
Gitweb: https://git.kernel.org/tip/f317392a317a27a78e755297505e57a6b345f4de
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Thu, 04 Apr 2024 12:04:25 +02:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Thu, 04 Apr 2024 12:09:10 +02:00

x86/alternatives: Remove a superfluous newline in _static_cpu_has()

No functional changes.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/cpufeature.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index f95e1c8..17fd7be 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -170,8 +170,7 @@ extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);
*/
static __always_inline bool _static_cpu_has(u16 bit)
{
- asm goto(
- ALTERNATIVE_TERNARY("jmp 6f", %c[feature], "", "jmp %l[t_no]")
+ asm goto(ALTERNATIVE_TERNARY("jmp 6f", %c[feature], "", "jmp %l[t_no]")
".pushsection .altinstr_aux,\"ax\"\n"
"6:\n"
" testb %[bitnum], %a[cap_byte]\n"

2024-04-04 17:08:39

by Jeremi Piotrowski

Subject: Re: [PATCH 5/5] x86/CPU/AMD: Track SNP host status with cc_platform_*()

On 28/03/2024 16:39, Borislav Petkov wrote:
> On Thu, Mar 28, 2024 at 03:24:29PM +0100, Jeremi Piotrowski wrote:
>> It's not but if you set it before the check it will be set for all AMD
>> systems, even if they are neither CC hosts nor CC guests.
>
> That a problem?
>

No problem now but I did find it odd that cc_vendor will now always be set for AMD but
not for Intel. For Intel the various checks would automatically return true. Something
to look out for in the future when adding CC_ATTR's - no one can assume that the checks
will only run when actively dealing with confidential computing.

> It is under a CONFIG_ARCH_HAS_CC_PLATFORM...
>>> To leave open the possibility of an SNP hypervisor running nested.
>
> But !CC_ATTR_GUEST_SEV_SNP doesn't mean that. It means it is not
> a SEV-SNP guest.
>
>> I thought you wanted to filter out SEV-SNP guests, which also have
>> X86_FEATURE_SEV_SNP CPUID bit set.
>
> I want to run snp_probe_rmptable_info() only on baremetal where it makes
> sense.
>>> My understanding is that these are the cases:
>>
>> CPUID(SEV_SNP) | MSR(SEV_SNP) | what am I
>> ---------------------------------------------
>> set | set | SNP-guest
>> set | unset | SNP-host
>> unset | ?? | not SNP
>
> So as you can see, we can't use X86_FEATURE_SEV_SNP for anything due to
> the late disable need. So we should be moving away from it.
>

I see your point about the disable needing to happen late - but then how about we remove
the setup_clear_cpu_cap(X86_FEATURE_SEV_SNP) too? No code depends on it any more and it would
help my cause as well.

> So we need a test for "am I a nested SNP hypervisor?"
>
> So, can your thing clear X86_FEATURE_HYPERVISOR and thus "emulate"
> baremetal?
>

Can't do that... it is a VM and hypervisor detection and various paravirt interfaces depend on
X86_FEATURE_HYPERVISOR.

2024-04-09 17:12:33

by tip-bot2 for Alexey Makhalov

Subject: [tip: x86/alternatives] x86/alternatives: Catch late X86_FEATURE modifiers

The following commit has been merged into the x86/alternatives branch of tip:

Commit-ID: ee8962082a4413dba1a1b3d3d23490c5221f3b8a
Gitweb: https://git.kernel.org/tip/ee8962082a4413dba1a1b3d3d23490c5221f3b8a
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Wed, 27 Mar 2024 16:43:14 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Tue, 09 Apr 2024 18:03:53 +02:00

x86/alternatives: Catch late X86_FEATURE modifiers

After alternatives have been patched, changes to the X86_FEATURE flags
won't take effect and could potentially even be wrong.

Warn about it.

This is something which has been long overdue.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Tested-by: Srikanth Aithal <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/cpufeature.h | 8 ++++++--
arch/x86/kernel/cpu/cpuid-deps.c | 3 +++
2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 686e92d..f07687d 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -150,8 +150,12 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
extern void setup_clear_cpu_cap(unsigned int bit);
extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);

-#define setup_force_cpu_cap(bit) do { \
- set_cpu_cap(&boot_cpu_data, bit); \
+#define setup_force_cpu_cap(bit) do { \
+ \
+ if (!boot_cpu_has(bit)) \
+ WARN_ON(alternatives_patched); \
+ \
+ set_cpu_cap(&boot_cpu_data, bit); \
set_bit(bit, (unsigned long *)cpu_caps_set); \
} while (0)

diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index b717420..5dd427c 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -114,6 +114,9 @@ static void do_clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int feature)
if (WARN_ON(feature >= MAX_FEATURE_BITS))
return;

+ if (boot_cpu_has(feature))
+ WARN_ON(alternatives_patched);
+
clear_feature(c, feature);

/* Collect all features to disable, handling dependencies */

2024-04-09 17:12:38

by tip-bot2 for Alexey Makhalov

Subject: [tip: x86/asm] x86/alternatives: Remove a superfluous newline in _static_cpu_has()

The following commit has been merged into the x86/asm branch of tip:

Commit-ID: a0c8cf9780359376496bbd6d2be1343badf68af7
Gitweb: https://git.kernel.org/tip/a0c8cf9780359376496bbd6d2be1343badf68af7
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Thu, 04 Apr 2024 12:04:25 +02:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Tue, 09 Apr 2024 17:59:10 +02:00

x86/alternatives: Remove a superfluous newline in _static_cpu_has()

No functional changes.

Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/cpufeature.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index daae5c6..cd90cef 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -168,8 +168,7 @@ extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);
*/
static __always_inline bool _static_cpu_has(u16 bit)
{
- asm goto(
- ALTERNATIVE_TERNARY("jmp 6f", %c[feature], "", "jmp %l[t_no]")
+ asm goto(ALTERNATIVE_TERNARY("jmp 6f", %c[feature], "", "jmp %l[t_no]")
".pushsection .altinstr_aux,\"ax\"\n"
"6:\n"
" testb %[bitnum], %a[cap_byte]\n"

2024-04-24 19:14:32

by Borislav Petkov

Subject: Re: [PATCH 5/5] x86/CPU/AMD: Track SNP host status with cc_platform_*()

On Thu, Apr 04, 2024 at 07:07:26PM +0200, Jeremi Piotrowski wrote:
> On 28/03/2024 16:39, Borislav Petkov wrote:
> > On Thu, Mar 28, 2024 at 03:24:29PM +0100, Jeremi Piotrowski wrote:
> >> It's not but if you set it before the check it will be set for all AMD
> >> systems, even if they are neither CC hosts nor CC guests.
> >
> > That a problem?
> >
>
> No problem now but I did find it odd that cc_vendor will now always be set for AMD but
> not for Intel. For Intel the various checks would automatically return true. Something
> to look out for in the future when adding CC_ATTR's - no one can assume that the checks
> will only run when actively dealing with confidential computing.

Right, I haven't made up my mind fully here yet... setting cc_vendor
*only* when running as some sort of a confidential computing guest kinda
makes sense.

And if it is not set, then that can be used to catch cases where the
cc_* helpers are used outside of confidential computing cases...

Do we want those assertions? I don't know...

> I see your point about the disable needing to happen late - but then how about we remove
> the setup_clear_cpu_cap(X86_FEATURE_SEV_SNP) too? No code depends on it any more and it would
> help my cause as well.
>
> > So we need a test for "am I a nested SNP hypervisor?"
> >
> > So, can your thing clear X86_FEATURE_HYPERVISOR and thus "emulate"
> > baremetal?
> >
>
> Can't do that... it is a VM and hypervisor detection and various paravirt interfaces depend on
> X86_FEATURE_HYPERVISOR.

Right, but "your cause" as you call it above looks like a constant
whack'a'mole game everytime we change something in the kernel when
enabling those things and that breaks your cause.

Do you really want that?

Or would you prefer to define your nested solution properly and then
have upstream code support it like the next well-defined coco platform
instead?

Thx.

--
Regards/Gruss,
Boris.

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