Re: [PATCH V3 02/13] x86/HV: Initialize shared memory boundary in the Isolation VM.

[Tianyu Lan]

On 8/13/2021 3:18 AM, Michael Kelley wrote:
> From: Tianyu Lan <ltykernel@xxxxxxxxx> Sent: Monday, August 9, 2021 10:56 AM
> > Subject: [PATCH V3 02/13] x86/HV: Initialize shared memory boundary in the Isolation VM.
>
> As with Patch 1, use the "x86/hyperv:" tag in the Subject line.
>
> > From: Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx>
> >
> > Hyper-V exposes shared memory boundary via cpuid
> > HYPERV_CPUID_ISOLATION_CONFIG and store it in the
> > shared_gpa_boundary of ms_hyperv struct. This prepares
> > to share memory with host for SNP guest.
> >
> > Signed-off-by: Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx>
> > ---
> >   arch/x86/kernel/cpu/mshyperv.c |  2 ++
> >   include/asm-generic/mshyperv.h | 12 +++++++++++-
> >   2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> > index 6b5835a087a3..2b7f396ef1a5 100644
> > --- a/arch/x86/kernel/cpu/mshyperv.c
> > +++ b/arch/x86/kernel/cpu/mshyperv.c
> > @@ -313,6 +313,8 @@ static void __init ms_hyperv_init_platform(void)
> >   	if (ms_hyperv.priv_high & HV_ISOLATION) {
> >   		ms_hyperv.isolation_config_a = cpuid_eax(HYPERV_CPUID_ISOLATION_CONFIG);
> >   		ms_hyperv.isolation_config_b = cpuid_ebx(HYPERV_CPUID_ISOLATION_CONFIG);
> > +		ms_hyperv.shared_gpa_boundary =
> > +			(u64)1 << ms_hyperv.shared_gpa_boundary_bits;
>
> You could use BIT_ULL() here, but it's kind of a shrug.


Good suggestion. Thanks.

> >   		pr_info("Hyper-V: Isolation Config: Group A 0x%x, Group B 0x%x\n",
> >   			ms_hyperv.isolation_config_a, ms_hyperv.isolation_config_b);
> > diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> > index 4269f3174e58..aa26d24a5ca9 100644
> > --- a/include/asm-generic/mshyperv.h
> > +++ b/include/asm-generic/mshyperv.h
> > @@ -35,8 +35,18 @@ struct ms_hyperv_info {
> >   	u32 max_vp_index;
> >   	u32 max_lp_index;
> >   	u32 isolation_config_a;
> > -	u32 isolation_config_b;
> > +	union {
> > +		u32 isolation_config_b;
> > +		struct {
> > +			u32 cvm_type : 4;
> > +			u32 Reserved11 : 1;
> > +			u32 shared_gpa_boundary_active : 1;
> > +			u32 shared_gpa_boundary_bits : 6;
> > +			u32 Reserved12 : 20;
>
> Any reason to name the reserved fields as "11" and "12"?  It
> just looks a bit unusual.  And I'd suggest lowercase "r".

Yes, will update in the next version.

> > +		};
> > +	};
> >   	void  __percpu **ghcb_base;
> > +	u64 shared_gpa_boundary;
> >   };
> >   extern struct ms_hyperv_info ms_hyperv;
> >
> > --
> > 2.25.1