Re: [PATCH v2 07/13] mshv: default to safe partition CPU features

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Legacy V1 API callers requesting nested virtualization will no longer receive the nested virtualization CPU feature, breaking ABI compatibility.
--

--- Patch [7]: [PATCH v2 07/13] mshv: default to safe partition CPU features ---
commit 01e208d51ef280f67802036c435ff766571437ae
Author: Wei Liu <[email protected]>

mshv: default to safe partition CPU features

For legacy create-partition callers, disable all processor and xsave
features first, then enable only the default feature set known to be
supported. Passing an all-zero disabled feature mask can make SEV-SNP
partition creation fail on Hyper-V.

> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index 712f45dcef60c..ad2e708fd7539 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
> @@ -2373,15 +2373,102 @@ static long mshv_ioctl_process_pt_flags(void __user *user_arg, u64 *pt_flags,
>  	    args.pt_isolation >= MSHV_PT_ISOLATION_COUNT)
>  		return -EINVAL;
>  
> +	memset(cr_props, 0, sizeof(*cr_props));
> +	memset(isol_props, 0, sizeof(*isol_props));
>  	disabled_procs = &cr_props->disabled_processor_features;
>  	disabled_xsave = &cr_props->disabled_processor_xsave_features;
>  
> +#if IS_ENABLED(CONFIG_X86_64)
> +	/* Disable all processor features first. */
> +	for (i = 0; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++)
> +		disabled_procs->as_uint64[i] = U64_MAX;
> +
> +	/* Enable default features that are known to be supported. */
> +	disabled_procs->sse3_support = 0;

[ ... ]

> +	disabled_procs->cet_ss_support = 0;
> +	disabled_procs->cet_ibt_support = 0;
> +	disabled_procs->vmx_exception_inject_support = 0;

[Severity: High]
Does this omission break legacy callers requesting nested virtualization?

Since all features are disabled by default (U64_MAX), and
disabled_procs->nested_virt_support is not explicitly set to 0 in this
default list, won't nested virtualization remain disabled for the guest CPU
even if the legacy caller sets MSHV_PT_BIT_NESTED_VIRTUALIZATION?

Later in mshv_ioctl_process_pt_flags(), the creation capability is enabled
if requested:

    if (args.pt_flags & BIT(MSHV_PT_BIT_NESTED_VIRTUALIZATION))
            *pt_flags |= HV_PARTITION_CREATION_FLAG_NESTED_VIRTUALIZATION_CAPABLE;

However, because the CPU feature itself is left disabled by the new safe
defaults list, it appears this will break nested virtualization for V1 ABI
callers.

> +
> +	disabled_xsave->as_uint64 = U64_MAX;
> +	disabled_xsave->xsave_support = 0;
> +	disabled_xsave->xsaveopt_support = 0;
> +	disabled_xsave->avx_support = 0;
> +#endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7