| Message ID | 20210820151933.22401-14-brijesh.singh@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Part1,v5,01/38] x86/mm: Add sev_feature_enabled() helper | expand |
On Fri, Aug 20, 2021 at 10:19:08AM -0500, Brijesh Singh wrote: > The SEV-SNP guest is required to perform GHCB GPA registration. This is > because the hypervisor may prefer that a guest use a consistent and/or > specific GPA for the GHCB associated with a vCPU. For more information, > see the GHCB specification section GHCB GPA Registration. > > During the boot, init_ghcb() allocates a per-cpu GHCB page. On very first > VC exception, the exception handler switch to using the per-cpu GHCB page > allocated during the init_ghcb(). The GHCB page must be registered in > the current vcpu context. > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > arch/x86/kernel/sev-internal.h | 12 ++++++++++++ > arch/x86/kernel/sev.c | 28 ++++++++++++++++++++++++++++ > 2 files changed, 40 insertions(+) > create mode 100644 arch/x86/kernel/sev-internal.h > > diff --git a/arch/x86/kernel/sev-internal.h b/arch/x86/kernel/sev-internal.h > new file mode 100644 > index 000000000000..0fb7324803b4 > --- /dev/null > +++ b/arch/x86/kernel/sev-internal.h > @@ -0,0 +1,12 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Forward declarations for sev-shared.c > + * > + * Author: Brijesh Singh <brijesh.singh@amd.com> > + */ > + > +#ifndef __X86_SEV_INTERNAL_H__ > + > +static void snp_register_ghcb_early(unsigned long paddr); > + > +#endif /* __X86_SEV_INTERNAL_H__ */ I believe you don't need that header if you move __sev_get_ghcb() and snp_register_ghcb() under the #include "sev-shared.c" so that snp_register_ghcb_early() is visible by then. diff --git a/arch/x86/kernel/sev-internal.h b/arch/x86/kernel/sev-internal.h deleted file mode 100644 index 0fb7324803b4..000000000000 --- a/arch/x86/kernel/sev-internal.h +++ /dev/null @@ -1,12 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ -/* - * Forward declarations for sev-shared.c - * - * Author: Brijesh Singh <brijesh.singh@amd.com> - */ - -#ifndef __X86_SEV_INTERNAL_H__ - -static void snp_register_ghcb_early(unsigned long paddr); - -#endif /* __X86_SEV_INTERNAL_H__ */ diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c index 9ab541b893c2..0ec0602e4bed 100644 --- a/arch/x86/kernel/sev.c +++ b/arch/x86/kernel/sev.c @@ -31,8 +31,6 @@ #include <asm/smp.h> #include <asm/cpu.h> -#include "sev-internal.h" - #define DR7_RESET_VALUE 0x400 /* For early boot hypervisor communication in SEV-ES enabled guests */ @@ -200,69 +198,6 @@ void noinstr __sev_es_ist_exit(void) this_cpu_write(cpu_tss_rw.x86_tss.ist[IST_INDEX_VC], *(unsigned long *)ist); } -static void snp_register_ghcb(struct sev_es_runtime_data *data, unsigned long paddr) -{ - if (data->snp_ghcb_registered) - return; - - snp_register_ghcb_early(paddr); - - data->snp_ghcb_registered = true; -} - -/* - * Nothing shall interrupt this code path while holding the per-CPU - * GHCB. The backup GHCB is only for NMIs interrupting this path. - * - * Callers must disable local interrupts around it. - */ -static noinstr struct ghcb *__sev_get_ghcb(struct ghcb_state *state) -{ - struct sev_es_runtime_data *data; - struct ghcb *ghcb; - - WARN_ON(!irqs_disabled()); - - data = this_cpu_read(runtime_data); - ghcb = &data->ghcb_page; - - if (unlikely(data->ghcb_active)) { - /* GHCB is already in use - save its contents */ - - if (unlikely(data->backup_ghcb_active)) { - /* - * Backup-GHCB is also already in use. There is no way - * to continue here so just kill the machine. To make - * panic() work, mark GHCBs inactive so that messages - * can be printed out. - */ - data->ghcb_active = false; - data->backup_ghcb_active = false; - - instrumentation_begin(); - panic("Unable to handle #VC exception! GHCB and Backup GHCB are already in use"); - instrumentation_end(); - } - - /* Mark backup_ghcb active before writing to it */ - data->backup_ghcb_active = true; - - state->ghcb = &data->backup_ghcb; - - /* Backup GHCB content */ - *state->ghcb = *ghcb; - } else { - state->ghcb = NULL; - data->ghcb_active = true; - } - - /* SEV-SNP guest requires that GHCB must be registered. */ - if (sev_feature_enabled(SEV_SNP)) - snp_register_ghcb(data, __pa(ghcb)); - - return ghcb; -} - /* Needed in vc_early_forward_exception */ void do_early_exception(struct pt_regs *regs, int trapnr); @@ -518,6 +453,69 @@ static enum es_result vc_slow_virt_to_phys(struct ghcb *ghcb, struct es_em_ctxt /* Include code shared with pre-decompression boot stage */ #include "sev-shared.c" +static void snp_register_ghcb(struct sev_es_runtime_data *data, unsigned long paddr) +{ + if (data->snp_ghcb_registered) + return; + + snp_register_ghcb_early(paddr); + + data->snp_ghcb_registered = true; +} + +/* + * Nothing shall interrupt this code path while holding the per-CPU + * GHCB. The backup GHCB is only for NMIs interrupting this path. + * + * Callers must disable local interrupts around it. + */ +static noinstr struct ghcb *__sev_get_ghcb(struct ghcb_state *state) +{ + struct sev_es_runtime_data *data; + struct ghcb *ghcb; + + WARN_ON(!irqs_disabled()); + + data = this_cpu_read(runtime_data); + ghcb = &data->ghcb_page; + + if (unlikely(data->ghcb_active)) { + /* GHCB is already in use - save its contents */ + + if (unlikely(data->backup_ghcb_active)) { + /* + * Backup-GHCB is also already in use. There is no way + * to continue here so just kill the machine. To make + * panic() work, mark GHCBs inactive so that messages + * can be printed out. + */ + data->ghcb_active = false; + data->backup_ghcb_active = false; + + instrumentation_begin(); + panic("Unable to handle #VC exception! GHCB and Backup GHCB are already in use"); + instrumentation_end(); + } + + /* Mark backup_ghcb active before writing to it */ + data->backup_ghcb_active = true; + + state->ghcb = &data->backup_ghcb; + + /* Backup GHCB content */ + *state->ghcb = *ghcb; + } else { + state->ghcb = NULL; + data->ghcb_active = true; + } + + /* SEV-SNP guest requires that GHCB must be registered. */ + if (sev_feature_enabled(SEV_SNP)) + snp_register_ghcb(data, __pa(ghcb)); + + return ghcb; +} + static noinstr void __sev_put_ghcb(struct ghcb_state *state) { struct sev_es_runtime_data *data; -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
On 8/23/21 12:35 PM, Borislav Petkov wrote: > On Fri, Aug 20, 2021 at 10:19:08AM -0500, Brijesh Singh wrote: >> The SEV-SNP guest is required to perform GHCB GPA registration. This is >> because the hypervisor may prefer that a guest use a consistent and/or >> specific GPA for the GHCB associated with a vCPU. For more information, >> see the GHCB specification section GHCB GPA Registration. >> >> During the boot, init_ghcb() allocates a per-cpu GHCB page. On very first >> VC exception, the exception handler switch to using the per-cpu GHCB page >> allocated during the init_ghcb(). The GHCB page must be registered in >> the current vcpu context. >> >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> >> --- >> arch/x86/kernel/sev-internal.h | 12 ++++++++++++ >> arch/x86/kernel/sev.c | 28 ++++++++++++++++++++++++++++ >> 2 files changed, 40 insertions(+) >> create mode 100644 arch/x86/kernel/sev-internal.h >> >> diff --git a/arch/x86/kernel/sev-internal.h b/arch/x86/kernel/sev-internal.h >> new file mode 100644 >> index 000000000000..0fb7324803b4 >> --- /dev/null >> +++ b/arch/x86/kernel/sev-internal.h >> @@ -0,0 +1,12 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Forward declarations for sev-shared.c >> + * >> + * Author: Brijesh Singh <brijesh.singh@amd.com> >> + */ >> + >> +#ifndef __X86_SEV_INTERNAL_H__ >> + >> +static void snp_register_ghcb_early(unsigned long paddr); >> + >> +#endif /* __X86_SEV_INTERNAL_H__ */ > > I believe you don't need that header if you move __sev_get_ghcb() > and snp_register_ghcb() under the #include "sev-shared.c" so that > snp_register_ghcb_early() is visible by then. > thanks, I will merge this in next version.
On 8/23/21 2:45 PM, Borislav Petkov wrote: > On Mon, Aug 23, 2021 at 01:56:06PM -0500, Brijesh Singh wrote: >> thanks, I will merge this in next version. > > Thx. > > One more thing I stumbled upon while staring at this, see below. Can you > add it to your set or should I simply apply it now? > I can include it in my series. thanks
diff --git a/arch/x86/kernel/sev-internal.h b/arch/x86/kernel/sev-internal.h new file mode 100644 index 000000000000..0fb7324803b4 --- /dev/null +++ b/arch/x86/kernel/sev-internal.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Forward declarations for sev-shared.c + * + * Author: Brijesh Singh <brijesh.singh@amd.com> + */ + +#ifndef __X86_SEV_INTERNAL_H__ + +static void snp_register_ghcb_early(unsigned long paddr); + +#endif /* __X86_SEV_INTERNAL_H__ */ diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c index 06e6914cdc26..9ab541b893c2 100644 --- a/arch/x86/kernel/sev.c +++ b/arch/x86/kernel/sev.c @@ -31,6 +31,8 @@ #include <asm/smp.h> #include <asm/cpu.h> +#include "sev-internal.h" + #define DR7_RESET_VALUE 0x400 /* For early boot hypervisor communication in SEV-ES enabled guests */ @@ -87,6 +89,13 @@ struct sev_es_runtime_data { * is currently unsupported in SEV-ES guests. */ unsigned long dr7; + + /* + * SEV-SNP requires that the GHCB must be registered before using it. + * The flag below will indicate whether the GHCB is registered, if its + * not registered then sev_es_get_ghcb() will perform the registration. + */ + bool snp_ghcb_registered; }; struct ghcb_state { @@ -191,6 +200,16 @@ void noinstr __sev_es_ist_exit(void) this_cpu_write(cpu_tss_rw.x86_tss.ist[IST_INDEX_VC], *(unsigned long *)ist); } +static void snp_register_ghcb(struct sev_es_runtime_data *data, unsigned long paddr) +{ + if (data->snp_ghcb_registered) + return; + + snp_register_ghcb_early(paddr); + + data->snp_ghcb_registered = true; +} + /* * Nothing shall interrupt this code path while holding the per-CPU * GHCB. The backup GHCB is only for NMIs interrupting this path. @@ -237,6 +256,10 @@ static noinstr struct ghcb *__sev_get_ghcb(struct ghcb_state *state) data->ghcb_active = true; } + /* SEV-SNP guest requires that GHCB must be registered. */ + if (sev_feature_enabled(SEV_SNP)) + snp_register_ghcb(data, __pa(ghcb)); + return ghcb; } @@ -681,6 +704,10 @@ static bool __init setup_ghcb(void) /* Alright - Make the boot-ghcb public */ boot_ghcb = &boot_ghcb_page; + /* SEV-SNP guest requires that GHCB GPA must be registered. */ + if (sev_feature_enabled(SEV_SNP)) + snp_register_ghcb_early(__pa(&boot_ghcb_page)); + return true; } @@ -770,6 +797,7 @@ static void __init init_ghcb(int cpu) data->ghcb_active = false; data->backup_ghcb_active = false; + data->snp_ghcb_registered = false; } void __init sev_es_init_vc_handling(void)
The SEV-SNP guest is required to perform GHCB GPA registration. This is because the hypervisor may prefer that a guest use a consistent and/or specific GPA for the GHCB associated with a vCPU. For more information, see the GHCB specification section GHCB GPA Registration. During the boot, init_ghcb() allocates a per-cpu GHCB page. On very first VC exception, the exception handler switch to using the per-cpu GHCB page allocated during the init_ghcb(). The GHCB page must be registered in the current vcpu context. Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- arch/x86/kernel/sev-internal.h | 12 ++++++++++++ arch/x86/kernel/sev.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 arch/x86/kernel/sev-internal.h