Re: [PATCH V4 05/13] hyperv: Add Write/Read MSR registers via ghcb page

[Tianyu Lan]

On 8/28/2021 1:41 AM, Greg KH wrote:
> On Fri, Aug 27, 2021 at 01:21:03PM -0400, Tianyu Lan wrote:
> > From: Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx>
> >
> > Hyperv provides GHCB protocol to write Synthetic Interrupt
> > Controller MSR registers in Isolation VM with AMD SEV SNP
> > and these registers are emulated by hypervisor directly.
> > Hyperv requires to write SINTx MSR registers twice. First
> > writes MSR via GHCB page to communicate with hypervisor
> > and then writes wrmsr instruction to talk with paravisor
> > which runs in VMPL0. Guest OS ID MSR also needs to be set
> > via GHCB page.
> >
> > Signed-off-by: Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx>
> > ---
> > Change since v1:
> >           * Introduce sev_es_ghcb_hv_call_simple() and share code
> >             between SEV and Hyper-V code.
> > Change since v3:
> >           * Pass old_msg_type to hv_signal_eom() as parameter.
> > 	 * Use HV_REGISTER_* marcro instead of HV_X64_MSR_*
> > 	 * Add hv_isolation_type_snp() weak function.
> > 	 * Add maros to set syinc register in ARM code.
> > ---
> >   arch/arm64/include/asm/mshyperv.h |  23 ++++++
> >   arch/x86/hyperv/hv_init.c         |  36 ++--------
> >   arch/x86/hyperv/ivm.c             | 112 ++++++++++++++++++++++++++++++
> >   arch/x86/include/asm/mshyperv.h   |  80 ++++++++++++++++++++-
> >   arch/x86/include/asm/sev.h        |   3 +
> >   arch/x86/kernel/sev-shared.c      |  63 ++++++++++-------
> >   drivers/hv/hv.c                   | 112 ++++++++++++++++++++----------
> >   drivers/hv/hv_common.c            |   6 ++
> >   include/asm-generic/mshyperv.h    |   4 +-
> >   9 files changed, 345 insertions(+), 94 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/mshyperv.h b/arch/arm64/include/asm/mshyperv.h
> > index 20070a847304..ced83297e009 100644
> > --- a/arch/arm64/include/asm/mshyperv.h
> > +++ b/arch/arm64/include/asm/mshyperv.h
> > @@ -41,6 +41,29 @@ static inline u64 hv_get_register(unsigned int reg)
> >   	return hv_get_vpreg(reg);
> >   }
> >   
> > +#define hv_get_simp(val)	{ val = hv_get_register(HV_REGISTER_SIMP); }
> > +#define hv_set_simp(val)	hv_set_register(HV_REGISTER_SIMP, val)
> > +
> > +#define hv_get_siefp(val)	{ val = hv_get_register(HV_REGISTER_SIEFP); }
> > +#define hv_set_siefp(val)	hv_set_register(HV_REGISTER_SIEFP, val)
> > +
> > +#define hv_get_synint_state(int_num, val) {			\
> > +	val = hv_get_register(HV_REGISTER_SINT0 + int_num);	\
> > +	}
> > +
> > +#define hv_set_synint_state(int_num, val)			\
> > +	hv_set_register(HV_REGISTER_SINT0 + int_num, val)
> > +
> > +#define hv_get_synic_state(val) {			\
> > +	val = hv_get_register(HV_REGISTER_SCONTROL);	\
> > +	}
> > +
> > +#define hv_set_synic_state(val)			\
> > +	hv_set_register(HV_REGISTER_SCONTROL, val)
> > +
> > +#define hv_signal_eom(old_msg_type)		 \
> > +	hv_set_register(HV_REGISTER_EOM, 0)
>
> Please just use real inline functions and not #defines if you really
> need it.

OK. Will update. Thanks.