Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Vitaly Kuznetsov <vkuznets@redhat.com>
To:     Vipin Sharma <vipinsh@google.com>
Cc:     kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        seanjc@google.com, pbonzini@redhat.com, dmatlack@google.com
Subject: Re: [PATCH v2 2/6] KVM: x86: hyper-v: Add extended hypercall
 support in Hyper-v
In-Reply-To: <20221121234026.3037083-3-vipinsh@google.com>
References: <20221121234026.3037083-1-vipinsh@google.com>
 <20221121234026.3037083-3-vipinsh@google.com>
Date:   Tue, 22 Nov 2022 17:29:06 +0100
Message-ID: <87bkozosvh.fsf@ovpn-194-185.brq.redhat.com>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Vipin Sharma <vipinsh@google.com> writes:

> Add support for extended hypercall in Hyper-v. Hyper-v TLFS 6.0b
> describes hypercalls above call code 0x8000 as extended hypercalls.
>
> A Hyper-v hypervisor's guest VM finds availability of extended
> hypercalls via CPUID.0x40000003.EBX BIT(20). If the bit is set then the
> guest can call extended hypercalls.
>
> All extended hypercalls will exit to userspace by default. This allows
> for easy support of future hypercalls without being dependent on KVM
> releases.
>
> If there will be need to process the hypercall in KVM instead of
> userspace then KVM can create a capability which userspace can query to
> know which hypercalls can be handled by the KVM and enable handling
> of those hypercalls.
>
> Signed-off-by: Vipin Sharma <vipinsh@google.com>
> ---
>  arch/x86/kvm/hyperv.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 0b6964ed2e66..8551ef495cc9 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -43,6 +43,12 @@
>  
>  #define KVM_HV_MAX_SPARSE_VCPU_SET_BITS DIV_ROUND_UP(KVM_MAX_VCPUS, HV_VCPUS_PER_SPARSE_BANK)
>  
> +/*
> + * The TLFS carves out 64 possible extended hypercalls, numbered sequentially
> + * after the base capabilities extended hypercall.
> + */
> +#define HV_EXT_CALL_MAX (HV_EXT_CALL_QUERY_CAPABILITIES + 64)
> +

First, I thought there's an off-by-one here (and should be '63') but
then I checked with TLFS and figured out that the limit comes from
HvExtCallQueryCapabilities's response which doesn't include itself
(0x8001) in the mask, this means it can encode

0x8002 == bit0
0x8003 == bit1
..
0x8041 == bit63

so indeed, the last one supported is 0x8041 == 0x8001 + 64

maybe it's worth extending the commont on where '64' comes from.

>  static void stimer_mark_pending(struct kvm_vcpu_hv_stimer *stimer,
>  				bool vcpu_kick);
>  
> @@ -2411,6 +2417,9 @@ static bool hv_check_hypercall_access(struct kvm_vcpu_hv *hv_vcpu, u16 code)
>  	case HVCALL_SEND_IPI:
>  		return hv_vcpu->cpuid_cache.enlightenments_eax &
>  			HV_X64_CLUSTER_IPI_RECOMMENDED;
> +	case HV_EXT_CALL_QUERY_CAPABILITIES ... HV_EXT_CALL_MAX:
> +		return hv_vcpu->cpuid_cache.features_ebx &
> +				HV_ENABLE_EXTENDED_HYPERCALLS;
>  	default:
>  		break;
>  	}
> @@ -2564,6 +2573,12 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>  		}
>  		goto hypercall_userspace_exit;
>  	}
> +	case HV_EXT_CALL_QUERY_CAPABILITIES ... HV_EXT_CALL_MAX:
> +		if (unlikely(hc.fast)) {
> +			ret = HV_STATUS_INVALID_PARAMETER;

I wasn't able to find any statement in TLFS stating whether extended
hypercalls can be 'fast', I can imagine e.g. MemoryHeatHintAsync using
it. Unfortunatelly, our userspace exit will have to be modified to
handle such stuff. This can stay for the time being I guess..

> +			break;
> +		}
> +		goto hypercall_userspace_exit;
>  	default:
>  		ret = HV_STATUS_INVALID_HYPERCALL_CODE;
>  		break;
> @@ -2722,6 +2737,7 @@ int kvm_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
>  
>  			ent->ebx |= HV_POST_MESSAGES;
>  			ent->ebx |= HV_SIGNAL_EVENTS;
> +			ent->ebx |= HV_ENABLE_EXTENDED_HYPERCALLS;
>  
>  			ent->edx |= HV_X64_HYPERCALL_XMM_INPUT_AVAILABLE;
>  			ent->edx |= HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>

-- 
Vitaly