2023-11-28 07:26:22

by Maxim Levitsky

[permalink] [raw]
Subject: Re: [RFC 10/33] KVM: x86: hyper-v: Introduce KVM_HV_GET_VSM_STATE

On Wed, 2023-11-08 at 11:17 +0000, Nicolas Saenz Julienne wrote:
> HVCALL_GET_VP_REGISTERS exposes the VTL call hypercall page entry
> offsets to the guest. This hypercall is implemented in user-space while
> the hypercall page patching happens in-kernel. So expose it as part of
> the partition wide VSM state.
>
> NOTE: Alternatively there is the option of sharing this information
> through a VTL KVM device attribute (the device is introduced in
> subsequent patches).
>
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
> ---
> arch/x86/include/uapi/asm/kvm.h | 5 +++++
> arch/x86/kvm/hyperv.c | 8 ++++++++
> arch/x86/kvm/hyperv.h | 2 ++
> arch/x86/kvm/x86.c | 18 ++++++++++++++++++
> include/uapi/linux/kvm.h | 4 ++++
> 5 files changed, 37 insertions(+)
>
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index f73d137784d7..370483d5d5fd 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -570,4 +570,9 @@ struct kvm_apic_id_groups {
> __u8 n_bits; /* nr of bits used to represent group in the APIC ID */
> };
>
> +/* for KVM_HV_GET_VSM_STATE */
> +struct kvm_hv_vsm_state {
> + __u64 vsm_code_page_offsets;
> +};
> +
> #endif /* _ASM_X86_KVM_H */
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 2cf430f6ddd8..caaa859932c5 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -2990,3 +2990,11 @@ int kvm_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
>
> return 0;
> }
> +
> +int kvm_vm_ioctl_get_hv_vsm_state(struct kvm *kvm, struct kvm_hv_vsm_state *state)
> +{
> + struct kvm_hv* hv = &kvm->arch.hyperv;
> +
> + state->vsm_code_page_offsets = hv->vsm_code_page_offsets.as_u64;
> + return 0;
> +}
> diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
> index 5433107e7cc8..b3d1113efe82 100644
> --- a/arch/x86/kvm/hyperv.h
> +++ b/arch/x86/kvm/hyperv.h
> @@ -261,4 +261,6 @@ static inline bool kvm_hv_vsm_enabled(struct kvm *kvm)
> return kvm->arch.hyperv.hv_enable_vsm;
> }
>
> +int kvm_vm_ioctl_get_hv_vsm_state(struct kvm *kvm, struct kvm_hv_vsm_state *state);
> +
> #endif
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index b0512e433032..57f9c58e1e32 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7132,6 +7132,24 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> r = kvm_vm_ioctl_set_apic_id_groups(kvm, &groups);
> break;
> }
> + case KVM_HV_GET_VSM_STATE: {
> + struct kvm_hv_vsm_state vsm_state;
> +
> + r = -EINVAL;
> + if (!kvm_hv_vsm_enabled(kvm))
> + goto out;
> +
> + r = kvm_vm_ioctl_get_hv_vsm_state(kvm, &vsm_state);
> + if (r)
> + goto out;
> +
> + r = -EFAULT;
> + if (copy_to_user(argp, &vsm_state, sizeof(vsm_state)))
> + goto out;
> +
> + r = 0;
> + break;
> + }
> default:
> r = -ENOTTY;
> }
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 168b6ac6ebe5..03f5c08fd7aa 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -2316,4 +2316,8 @@ struct kvm_create_guest_memfd {
> #define KVM_GUEST_MEMFD_ALLOW_HUGEPAGE (1ULL << 0)
>
> #define KVM_SET_APIC_ID_GROUPS _IOW(KVMIO, 0xd7, struct kvm_apic_id_groups)
> +
> +/* Get/Set Hyper-V VSM state. Available with KVM_CAP_HYPERV_VSM */
> +#define KVM_HV_GET_VSM_STATE _IOR(KVMIO, 0xd5, struct kvm_hv_vsm_state)
> +
> #endif /* __LINUX_KVM_H */

Looks reasonable but if we do hypercall patching in userspace as I suggested,
we might not need this.

Best regards,
Maxim Levitsky