Re: [RFC PATCH V3 01/16] x86/hyperv: Add sev-snp enlightened guest specific config

[Tianyu Lan]

On 2/1/2023 1:34 AM, Michael Kelley (LINUX) wrote:
> > diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> > index 8f83ceec45dc..ace5901ba0fc 100644
> > --- a/arch/x86/kernel/cpu/mshyperv.c
> > +++ b/arch/x86/kernel/cpu/mshyperv.c
> > @@ -273,6 +273,18 @@ static void __init ms_hyperv_init_platform(void)
> >
> >   	hv_max_functions_eax = cpuid_eax(HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS);
> >
> > +	/*
> > +	 * Add custom configuration for SEV-SNP Enlightened guest
> > +	 */
> > +	if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) {
> > +		ms_hyperv.features |= HV_ACCESS_FREQUENCY_MSRS;
> > +		ms_hyperv.misc_features |= HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;
> > +		ms_hyperv.misc_features &= ~HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
> > +		ms_hyperv.hints |= HV_DEPRECATING_AEOI_RECOMMENDED;
> > +		ms_hyperv.hints |= HV_X64_APIC_ACCESS_RECOMMENDED;
> > +		ms_hyperv.hints |= HV_X64_CLUSTER_IPI_RECOMMENDED;
> Two different things are happening in changing the above flags:
>
> 1)  Disabling certain feature that Hyper-V might offer to a guest, such
> as the crash MSRs and Auto EOI.  (In some cases disabling the feature
> means removing the flag.  In other cases in means adding the flag.  But
> the net result is same -- other Hyper-V specific code will not use the
> feature.)  This category is OK.
>
> 2)  Forcing certain features to be treated as enabled.  This category
> is somewhat concerning.  Assuming that Hyper-V is accurately indicating
> which features are available, it seems better to check that the flags
> required by SNP are present, and refuse to boot in SNP mode if not.
> Or is this code handling a different problem, where Hyper-V is not
> indicating that the feature is available, even though it really is?

Agree. The CPUID emulation in SEV-SNP guest may be controlled by the
cpuid table which is passed to kernel via EFI bootloader or hypervisor.
In Hyper-V case, the CPUID table is passed by Hyper-V directly and the
table is built during making guest image. To avoid the confusion here,
will try hiding the change in the cpuid table and double check whether 
these features will be enalbed or disabled on different machine or VM
type.

Thanks.