On Tue, Nov 11, 2025 at 03:19:54PM -0800, Nuno Das Neves wrote:
> From: Muminul Islam <[email protected]>
>
> The existing mshv create partition ioctl does not provide a way to
> specify which cpu features are enabled in the guest. Instead, it
> attempts to enable all features and those that are not supported are
> silently disabled by the hypervisor.
>
> This was done to reduce unnecessary complexity and is sufficient for
> many cases. However, new scenarios require fine-grained control over
> these features.
>
> Define a new mshv_create_partition_v2 structure which supports
> passing the disabled processor and xsave feature bits through to the
> create partition hypercall directly.
>
> Introduce a new flag MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES which enables
> the new structure. If unset, the original mshv_create_partition struct
> is used, with the old behavior of enabling all features.
>
> Co-developed-by: Jinank Jain <[email protected]>
> Signed-off-by: Jinank Jain <[email protected]>
> Signed-off-by: Muminul Islam <[email protected]>
> Signed-off-by: Nuno Das Neves <[email protected]>
> ---
> Changes in v4:
> - Change BIT() to BIT_ULL() [Michael Kelley]
> - Enforce pt_num_cpu_fbanks == MSHV_NUM_CPU_FEATURES_BANKS and expect
> that number to never change. In future, additional processor banks
> will be settable as 'early' partition properties. Remove redundant
> code that set default values for unspecified banks [Michael Kelley]
> - Set xsave features to 0 on arm64 [Michael Kelley]
> - Add clarifying comments in a few places
>
> Changes in v3:
> - Remove the new cpu features definitions in hvhdk.h, and retain the
> old behavior of enabling all features for the old struct. For the v2
> struct, still disable unspecified feature banks, since that makes it
> robust to future extensions.
> - Amend comments and commit message to reflect the above
> - Fix unused variable on arm64 [kernel test robot]
>
> Changes in v2:
> - Fix exposure of CONFIG_X86_64 to uapi [kernel test robot]
> - Fix compilation issue on arm64 [kernel test robot]
> ---
> drivers/hv/mshv_root_main.c | 113 +++++++++++++++++++++++++++++-------
> include/uapi/linux/mshv.h | 34 +++++++++++
> 2 files changed, 126 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index d542a0143bb8..9f9438289b60 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
> @@ -1900,43 +1900,114 @@ add_partition(struct mshv_partition *partition)
> return 0;
> }
>
> -static long
> -mshv_ioctl_create_partition(void __user *user_arg, struct device *module_dev)
> +static_assert(MSHV_NUM_CPU_FEATURES_BANKS ==
> + HV_PARTITION_PROCESSOR_FEATURES_BANKS);
> +
> +static long mshv_ioctl_process_pt_flags(void __user *user_arg, u64 *pt_flags,
> + struct hv_partition_creation_properties
> *cr_props,
> + union hv_partition_isolation_properties
> *isol_props)
> {
> - struct mshv_create_partition args;
> - u64 creation_flags;
> - struct hv_partition_creation_properties creation_properties = {};
> - union hv_partition_isolation_properties isolation_properties = {};
> - struct mshv_partition *partition;
> - struct file *file;
> - int fd;
> - long ret;
> + int i;
> + struct mshv_create_partition_v2 args;
> + union hv_partition_processor_features *disabled_procs;
> + union hv_partition_processor_xsave_features *disabled_xsave;
>
> - if (copy_from_user(&args, user_arg, sizeof(args)))
> + /* First, copy v1 struct in case user is on previous versions */
> + if (copy_from_user(&args, user_arg,
> + sizeof(struct mshv_create_partition)))
> return -EFAULT;
>
> if ((args.pt_flags & ~MSHV_PT_FLAGS_MASK) ||
> args.pt_isolation >= MSHV_PT_ISOLATION_COUNT)
> return -EINVAL;
>
> + disabled_procs = &cr_props->disabled_processor_features;
> + disabled_xsave = &cr_props->disabled_processor_xsave_features;
> +
> + /* Check if user provided newer struct with feature fields */
> + if (args.pt_flags & BIT_ULL(MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES)) {
> + if (copy_from_user(&args, user_arg, sizeof(args)))
> + return -EFAULT;
> +
> + if (args.pt_num_cpu_fbanks != MSHV_NUM_CPU_FEATURES_BANKS ||
> + mshv_field_nonzero(args, pt_rsvd) ||
> + mshv_field_nonzero(args, pt_rsvd1))
> + return -EINVAL;
> +
> + /*
> + * Note this assumes MSHV_NUM_CPU_FEATURES_BANKS will never
> + * change and equals HV_PARTITION_PROCESSOR_FEATURES_BANKS
> + * (i.e. 2).
> + *
> + * Further banks (index >= 2) will be modifiable as 'early'
> + * properties via the set partition property hypercall.
> + */
> + for (i = 0; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++)
> + disabled_procs->as_uint64[i] = args.pt_cpu_fbanks[i];
> +
> +#if IS_ENABLED(CONFIG_X86_64)
> + disabled_xsave->as_uint64 = args.pt_disabled_xsave;
> +#else
> + /*
> + * In practice this field is ignored on arm64, but safer to
> + * zero it in case it is ever used.
> + */
> + disabled_xsave->as_uint64 = 0;
Why not explicitly treat non-zero value as invalid here instead?
Isn't it always better than implicitly (and silently) zeroing it?
> +
> + if (mshv_field_nonzero(args, pt_rsvd2))
> + return -EINVAL;
> +#endif
> + } else {
> + /*
> + * v1 behavior: try to enable everything. The hypervisor will
> + * disable features that are not supported. The banks can be
> + * queried via the get partition property hypercall.
> + */
> + for (i = 0; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++)
> + disabled_procs->as_uint64[i] = 0;
> +
> + disabled_xsave->as_uint64 = 0;
> + }
> +
> /* Only support EXO partitions */
> - creation_flags = HV_PARTITION_CREATION_FLAG_EXO_PARTITION |
> -
> HV_PARTITION_CREATION_FLAG_INTERCEPT_MESSAGE_PAGE_ENABLED;
> + *pt_flags = HV_PARTITION_CREATION_FLAG_EXO_PARTITION |
> + HV_PARTITION_CREATION_FLAG_INTERCEPT_MESSAGE_PAGE_ENABLED;
> +
> + if (args.pt_flags & BIT_ULL(MSHV_PT_BIT_LAPIC))
> + *pt_flags |= HV_PARTITION_CREATION_FLAG_LAPIC_ENABLED;
> + if (args.pt_flags & BIT_ULL(MSHV_PT_BIT_X2APIC))
> + *pt_flags |= HV_PARTITION_CREATION_FLAG_X2APIC_CAPABLE;
> + if (args.pt_flags & BIT_ULL(MSHV_PT_BIT_GPA_SUPER_PAGES))
> + *pt_flags |= HV_PARTITION_CREATION_FLAG_GPA_SUPER_PAGES_ENABLED;
>
> - if (args.pt_flags & BIT(MSHV_PT_BIT_LAPIC))
> - creation_flags |= HV_PARTITION_CREATION_FLAG_LAPIC_ENABLED;
> - if (args.pt_flags & BIT(MSHV_PT_BIT_X2APIC))
> - creation_flags |= HV_PARTITION_CREATION_FLAG_X2APIC_CAPABLE;
> - if (args.pt_flags & BIT(MSHV_PT_BIT_GPA_SUPER_PAGES))
> - creation_flags |=
> HV_PARTITION_CREATION_FLAG_GPA_SUPER_PAGES_ENABLED;
> + isol_props->as_uint64 = 0;
These properties are missing the nested one.
I'd recommend squeezing that change in this patch
Thanks,
Stanislav
>
> switch (args.pt_isolation) {
> case MSHV_PT_ISOLATION_NONE:
> - isolation_properties.isolation_type =
> - HV_PARTITION_ISOLATION_TYPE_NONE;
> + isol_props->isolation_type = HV_PARTITION_ISOLATION_TYPE_NONE;
> break;
> }
>
> + return 0;
> +}
> +
> +static long
> +mshv_ioctl_create_partition(void __user *user_arg, struct device *module_dev)
> +{
> + u64 creation_flags;
> + struct hv_partition_creation_properties creation_properties;
> + union hv_partition_isolation_properties isolation_properties;
> + struct mshv_partition *partition;
> + struct file *file;
> + int fd;
> + long ret;
> +
> + ret = mshv_ioctl_process_pt_flags(user_arg, &creation_flags,
> + &creation_properties,
> + &isolation_properties);
> + if (ret)
> + return ret;
> +
> partition = kzalloc(sizeof(*partition), GFP_KERNEL);
> if (!partition)
> return -ENOMEM;
> diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
> index 876bfe4e4227..cf904f3aa201 100644
> --- a/include/uapi/linux/mshv.h
> +++ b/include/uapi/linux/mshv.h
> @@ -26,6 +26,7 @@ enum {
> MSHV_PT_BIT_LAPIC,
> MSHV_PT_BIT_X2APIC,
> MSHV_PT_BIT_GPA_SUPER_PAGES,
> + MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES,
> MSHV_PT_BIT_COUNT,
> };
>
> @@ -41,6 +42,8 @@ enum {
> * @pt_flags: Bitmask of 1 << MSHV_PT_BIT_*
> * @pt_isolation: MSHV_PT_ISOLATION_*
> *
> + * This is the initial/v1 version for backward compatibility.
> + *
> * Returns a file descriptor to act as a handle to a guest partition.
> * At this point the partition is not yet initialized in the hypervisor.
> * Some operations must be done with the partition in this state, e.g.
> setting
> @@ -52,6 +55,37 @@ struct mshv_create_partition {
> __u64 pt_isolation;
> };
>
> +#define MSHV_NUM_CPU_FEATURES_BANKS 2
> +
> +/**
> + * struct mshv_create_partition_v2
> + *
> + * This is extended version of the above initial MSHV_CREATE_PARTITION
> + * ioctl and allows for following additional parameters:
> + *
> + * @pt_num_cpu_fbanks: Must be set to MSHV_NUM_CPU_FEATURES_BANKS.
> + * @pt_cpu_fbanks: Disabled processor feature banks array.
> + * @pt_disabled_xsave: Disabled xsave feature bits.
> + *
> + * pt_cpu_fbanks and pt_disabled_xsave are passed through as-is to the create
> + * partition hypercall.
> + *
> + * Returns : same as above original mshv_create_partition
> + */
> +struct mshv_create_partition_v2 {
> + __u64 pt_flags;
> + __u64 pt_isolation;
> + __u16 pt_num_cpu_fbanks;
> + __u8 pt_rsvd[6]; /* MBZ */
> + __u64 pt_cpu_fbanks[MSHV_NUM_CPU_FEATURES_BANKS];
> + __u64 pt_rsvd1[2]; /* MBZ */
> +#if defined(__x86_64__)
> + __u64 pt_disabled_xsave;
> +#else
> + __u64 pt_rsvd2; /* MBZ */
> +#endif
> +} __packed;
> +
> /* /dev/mshv */
> #define MSHV_CREATE_PARTITION _IOW(MSHV_IOCTL, 0x00, struct
> mshv_create_partition)
>
> --
> 2.34.1
