| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240527131022.GAZlSGPm_s0FAu_BRB@fat_crate.local>
Date: Mon, 27 May 2024 15:10:22 +0200
From: Borislav Petkov <bp@...en8.de>
To: Tom Lendacky <thomas.lendacky@....com>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org,
linux-coco@...ts.linux.dev, svsm-devel@...onut-svsm.dev,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
"H. Peter Anvin" <hpa@...or.com>, Andy Lutomirski <luto@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Dan Williams <dan.j.williams@...el.com>,
Michael Roth <michael.roth@....com>,
Ashish Kalra <ashish.kalra@....com>
Subject: Re: [PATCH v4 08/15] x86/sev: Provide SVSM discovery support
On Wed, Apr 24, 2024 at 10:58:04AM -0500, Tom Lendacky wrote:
> The SVSM specification documents an alternative method of discovery for
> the SVSM using a reserved CPUID bit and a reserved MSR.
Yes, and all your code places where you do
if (vmpl)
to check whether the guest is running over a SVSM should do the CPUID
check. We should not be hardcoding the VMPL level to mean a SVSM is
present.
>
> For the CPUID support, the SNP CPUID table is updated to set bit 28 of
s/is updated/update the.../
> the EAX register of the 0x8000001f leaf when an SVSM is present. This bit
> has been reserved for use in this capacity.
>
> For the MSR support, a new reserved MSR 0xc001f000 has been defined. A #VC
> should be generated when accessing this MSR. The #VC handler is expected
> to ignore writes to this MSR and return the physical calling area address
> (CAA) on reads of this MSR.
>
> Signed-off-by: Tom Lendacky <thomas.lendacky@....com>
> ---
> arch/x86/include/asm/cpufeatures.h | 1 +
> arch/x86/include/asm/msr-index.h | 2 ++
> arch/x86/kernel/sev-shared.c | 11 +++++++++++
> arch/x86/kernel/sev.c | 17 +++++++++++++++++
> 4 files changed, 31 insertions(+)
>
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 3c7434329661..a17a81b3189b 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -446,6 +446,7 @@
> #define X86_FEATURE_V_TSC_AUX (19*32+ 9) /* "" Virtual TSC_AUX */
> #define X86_FEATURE_SME_COHERENT (19*32+10) /* "" AMD hardware-enforced cache coherency */
> #define X86_FEATURE_DEBUG_SWAP (19*32+14) /* AMD SEV-ES full debug state swap support */
> +#define X86_FEATURE_SVSM_PRESENT (19*32+28) /* "" SNP SVSM is present */
X86_FEATURE_SVSM is better right?
And then we might even want to show it in /proc/cpuinfo here to really
say that we're running over a SVSM as that might be useful info. Think
alternate injection support for one.
> /* AMD-defined Extended Feature 2 EAX, CPUID level 0x80000021 (EAX), word 20 */
> #define X86_FEATURE_NO_NESTED_DATA_BP (20*32+ 0) /* "" No Nested Data Breakpoints */
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index e022e6eb766c..45ffa27569f4 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -660,6 +660,8 @@
> #define MSR_AMD64_RMP_BASE 0xc0010132
> #define MSR_AMD64_RMP_END 0xc0010133
>
> +#define MSR_SVSM_CAA 0xc001f000
> +
> /* AMD Collaborative Processor Performance Control MSRs */
> #define MSR_AMD_CPPC_CAP1 0xc00102b0
> #define MSR_AMD_CPPC_ENABLE 0xc00102b1
> diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
> index b415b10a0823..50db783f151e 100644
> --- a/arch/x86/kernel/sev-shared.c
> +++ b/arch/x86/kernel/sev-shared.c
> @@ -1561,6 +1561,8 @@ static enum es_result vc_check_opcode_bytes(struct es_em_ctxt *ctxt,
> static void __head setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
> {
> struct snp_secrets_page *secrets_page;
> + struct snp_cpuid_table *cpuid_table;
> + unsigned int i;
> u64 caa;
>
> BUILD_BUG_ON(sizeof(*secrets_page) != PAGE_SIZE);
> @@ -1607,4 +1609,13 @@ static void __head setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
> */
> boot_svsm_caa = (struct svsm_ca *)caa;
> boot_svsm_caa_pa = caa;
> +
> + /* Advertise the SVSM presence via CPUID. */
> + cpuid_table = (struct snp_cpuid_table *)snp_cpuid_get_table();
> + for (i = 0; i < cpuid_table->count; i++) {
> + struct snp_cpuid_fn *fn = &cpuid_table->fn[i];
> +
> + if (fn->eax_in == 0x8000001f)
> + fn->eax |= BIT(28);
> + }
> }
> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
> index 3f4342b31736..69a756781d90 100644
> --- a/arch/x86/kernel/sev.c
> +++ b/arch/x86/kernel/sev.c
> @@ -1326,12 +1326,29 @@ int __init sev_es_efi_map_ghcbs(pgd_t *pgd)
> return 0;
> }
>
> +static enum es_result vc_handle_svsm_caa_msr(struct es_em_ctxt *ctxt)
No need for that helper. And you can reuse the exit_info_1 assignment.
Diff ontop:
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 40eb547d0d6c..7a248d66227e 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -1326,31 +1326,25 @@ int __init sev_es_efi_map_ghcbs(pgd_t *pgd)
return 0;
}
-static enum es_result vc_handle_svsm_caa_msr(struct es_em_ctxt *ctxt)
-{
- struct pt_regs *regs = ctxt->regs;
-
- /* Writes to the SVSM CAA msr are ignored */
- if (ctxt->insn.opcode.bytes[1] == 0x30)
- return ES_OK;
-
- regs->ax = lower_32_bits(this_cpu_read(svsm_caa_pa));
- regs->dx = upper_32_bits(this_cpu_read(svsm_caa_pa));
-
- return ES_OK;
-}
-
static enum es_result vc_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt)
{
struct pt_regs *regs = ctxt->regs;
enum es_result ret;
u64 exit_info_1;
- if (regs->cx == MSR_SVSM_CAA)
- return vc_handle_svsm_caa_msr(ctxt);
-
/* Is it a WRMSR? */
- exit_info_1 = (ctxt->insn.opcode.bytes[1] == 0x30) ? 1 : 0;
+ exit_info_1 = !!(ctxt->insn.opcode.bytes[1] == 0x30);
+
+ if (regs->cx == MSR_SVSM_CAA) {
+ /* Writes to the SVSM CAA msr are ignored */
+ if (exit_info_1)
+ return ES_OK;
+
+ regs->ax = lower_32_bits(this_cpu_read(svsm_caa_pa));
+ regs->dx = upper_32_bits(this_cpu_read(svsm_caa_pa));
+
+ return ES_OK;
+ }
ghcb_set_rcx(ghcb, regs->cx);
if (exit_info_1) {
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Powered by blists - more mailing lists