On Mon, Feb 07, 2022 at 11:09:28AM -0500, Tianyu Lan wrote: > From: Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx> > > Hyper-V Isolation VM code uses sev_es_ghcb_hv_call() to read/write MSR > via ghcb page. The SEV-ES guest should call the sev_es_negotiate_protocol() > to negotiate the GHCB protocol version before establishing the GHCB. Call > sev_es_negotiate_protocol() in the hyperv_init_ghcb(). > > Signed-off-by: Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx> > --- > This patch based on the "Add AMD Secure Nested Paging (SEV-SNP) Guest Support" > patchset. https://lore.kernel.org/lkml/20220128171804.569796-1-brijesh.singh@xxxxxxx/ > > arch/x86/hyperv/hv_init.c | 6 ++++++ > arch/x86/include/asm/sev.h | 2 ++ > arch/x86/kernel/sev-shared.c | 2 +- > arch/x86/kernel/sev.c | 4 ++-- > 4 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c > index 24f4a06ac46a..a22303fccf02 100644 > --- a/arch/x86/hyperv/hv_init.c > +++ b/arch/x86/hyperv/hv_init.c > @@ -28,6 +28,7 @@ > #include <linux/syscore_ops.h> > #include <clocksource/hyperv_timer.h> > #include <linux/highmem.h> > +#include <asm/sev.h> > > int hyperv_init_cpuhp; > u64 hv_current_partition_id = ~0ull; > @@ -69,6 +70,11 @@ static int hyperv_init_ghcb(void) > ghcb_base = (void **)this_cpu_ptr(hv_ghcb_pg); > *ghcb_base = ghcb_va; > > + sev_es_negotiate_protocol(); The return value should be checked, right? There is no logical connection between this function call and the wrmsrl below. Is there new information available after calling sev_es_negotiate_protocol? > + > + /* Write ghcb page back after negotiating protocol. */ > + wrmsrl(MSR_AMD64_SEV_ES_GHCB, ghcb_gpa); > + VMGEXIT(); > return 0; > } > [...] > diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c > index 3568b3303314..46c53c4885ee 100644 > --- a/arch/x86/kernel/sev.c > +++ b/arch/x86/kernel/sev.c > @@ -167,12 +167,12 @@ void noinstr __sev_es_ist_exit(void) > this_cpu_write(cpu_tss_rw.x86_tss.ist[IST_INDEX_VC], *(unsigned long *)ist); > } > > -static inline u64 sev_es_rd_ghcb_msr(void) > +inline u64 sev_es_rd_ghcb_msr(void) > { > return __rdmsr(MSR_AMD64_SEV_ES_GHCB); > } > > -static __always_inline void sev_es_wr_ghcb_msr(u64 val) > +__always_inline void sev_es_wr_ghcb_msr(u64 val) Why are these needed? They are not used anywhere in this patch. Thanks, Wei. > { > u32 low, high; > > -- > 2.25.1 >
