diff mbox series

[v9,2/6] x86/kvm/hyper-v: Simplify addition for custom cpuid leafs

Message ID 20200320172839.1144395-3-arilou@gmail.com (mailing list archive)
State New, archived
Headers show
Series x86/kvm/hyper-v: add support for synthetic debugger | expand

Commit Message

Jon Doron March 20, 2020, 5:28 p.m. UTC 
Simlify the code to define a new cpuid leaf group by enabled feature.

This also fixes a bug in which the max cpuid leaf was always set to
HYPERV_CPUID_NESTED_FEATURES regardless if nesting is supported or not.

Any new CPUID group needs to consider the max leaf and be added in the
correct order, in this method there are two rules:
1. Each cpuid leaf group must be order in an ascending order
2. The appending for the cpuid leafs by features also needs to happen by
   ascending order.

Signed-off-by: Jon Doron <arilou@gmail.com>
---
 arch/x86/kvm/hyperv.c | 46 ++++++++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 14 deletions(-)

Comments

Vitaly Kuznetsov March 23, 2020, 10:16 a.m. UTC | #1 
Jon Doron <arilou@gmail.com> writes:

> Simlify the code to define a new cpuid leaf group by enabled feature.
>
> This also fixes a bug in which the max cpuid leaf was always set to
> HYPERV_CPUID_NESTED_FEATURES regardless if nesting is supported or not.
>
> Any new CPUID group needs to consider the max leaf and be added in the
> correct order, in this method there are two rules:
> 1. Each cpuid leaf group must be order in an ascending order
> 2. The appending for the cpuid leafs by features also needs to happen by
>    ascending order.
>
> Signed-off-by: Jon Doron <arilou@gmail.com>
> ---
>  arch/x86/kvm/hyperv.c | 46 ++++++++++++++++++++++++++++++-------------
>  1 file changed, 32 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index a86fda7a1d03..7383c7e7d4af 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1785,27 +1785,45 @@ int kvm_vm_ioctl_hv_eventfd(struct kvm *kvm, struct kvm_hyperv_eventfd *args)
>  	return kvm_hv_eventfd_assign(kvm, args->conn_id, args->fd);
>  }
>  
> +// Must be sorted in ascending order by function

scripts/checkpatch.pl should've complained here, kernel coding style
always requires /* */ 

> +static struct kvm_cpuid_entry2 core_cpuid_entries[] = {
> +	{ .function = HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS },
> +	{ .function = HYPERV_CPUID_INTERFACE },
> +	{ .function = HYPERV_CPUID_VERSION },
> +	{ .function = HYPERV_CPUID_FEATURES },
> +	{ .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
> +	{ .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
> +};
> +
> +static struct kvm_cpuid_entry2 evmcs_cpuid_entries[] = {
> +	{ .function = HYPERV_CPUID_NESTED_FEATURES },
> +};
> +
> +#define HV_MAX_CPUID_ENTRIES \
> +	ARRAY_SIZE(core_cpuid_entries) +\
> +	ARRAY_SIZE(evmcs_cpuid_entries)
> +
>  int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
>  				struct kvm_cpuid_entry2 __user *entries)
>  {
>  	uint16_t evmcs_ver = 0;
> -	struct kvm_cpuid_entry2 cpuid_entries[] = {
> -		{ .function = HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS },
> -		{ .function = HYPERV_CPUID_INTERFACE },
> -		{ .function = HYPERV_CPUID_VERSION },
> -		{ .function = HYPERV_CPUID_FEATURES },
> -		{ .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
> -		{ .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
> -		{ .function = HYPERV_CPUID_NESTED_FEATURES },
> -	};
> -	int i, nent = ARRAY_SIZE(cpuid_entries);
> +	struct kvm_cpuid_entry2 cpuid_entries[HV_MAX_CPUID_ENTRIES];
> +	int i, nent = 0;
> +
> +	/* Set the core cpuid entries required for Hyper-V */
> +	memcpy(&cpuid_entries[nent], &core_cpuid_entries,
> +	       sizeof(core_cpuid_entries));
> +	nent += ARRAY_SIZE(core_cpuid_entries);

Strictly speaking "+=" is not needed here as nent is zero.

>  
>  	if (kvm_x86_ops->nested_get_evmcs_version)
>  		evmcs_ver = kvm_x86_ops->nested_get_evmcs_version(vcpu);
>  
> -	/* Skip NESTED_FEATURES if eVMCS is not supported */
> -	if (!evmcs_ver)
> -		--nent;
> +	if (evmcs_ver) {
> +		/* EVMCS is enabled, add the required EVMCS CPUID leafs */
> +		memcpy(&cpuid_entries[nent], &evmcs_cpuid_entries,
> +		       sizeof(evmcs_cpuid_entries));
> +		nent += ARRAY_SIZE(evmcs_cpuid_entries);
> +	}
>  
>  	if (cpuid->nent < nent)
>  		return -E2BIG;
> @@ -1821,7 +1839,7 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
>  		case HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS:
>  			memcpy(signature, "Linux KVM Hv", 12);
>  
> -			ent->eax = HYPERV_CPUID_NESTED_FEATURES;
> +			ent->eax = cpuid_entries[nent - 1].function;
>  			ent->ebx = signature[0];
>  			ent->ecx = signature[1];
>  			ent->edx = signature[2];

With the nitpicks mentioned above:

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

Patch

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index a86fda7a1d03..7383c7e7d4af 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1785,27 +1785,45 @@  int kvm_vm_ioctl_hv_eventfd(struct kvm *kvm, struct kvm_hyperv_eventfd *args)
 	return kvm_hv_eventfd_assign(kvm, args->conn_id, args->fd);
 }
 
+// Must be sorted in ascending order by function
+static struct kvm_cpuid_entry2 core_cpuid_entries[] = {
+	{ .function = HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS },
+	{ .function = HYPERV_CPUID_INTERFACE },
+	{ .function = HYPERV_CPUID_VERSION },
+	{ .function = HYPERV_CPUID_FEATURES },
+	{ .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
+	{ .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
+};
+
+static struct kvm_cpuid_entry2 evmcs_cpuid_entries[] = {
+	{ .function = HYPERV_CPUID_NESTED_FEATURES },
+};
+
+#define HV_MAX_CPUID_ENTRIES \
+	ARRAY_SIZE(core_cpuid_entries) +\
+	ARRAY_SIZE(evmcs_cpuid_entries)
+
 int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
 				struct kvm_cpuid_entry2 __user *entries)
 {
 	uint16_t evmcs_ver = 0;
-	struct kvm_cpuid_entry2 cpuid_entries[] = {
-		{ .function = HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS },
-		{ .function = HYPERV_CPUID_INTERFACE },
-		{ .function = HYPERV_CPUID_VERSION },
-		{ .function = HYPERV_CPUID_FEATURES },
-		{ .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
-		{ .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
-		{ .function = HYPERV_CPUID_NESTED_FEATURES },
-	};
-	int i, nent = ARRAY_SIZE(cpuid_entries);
+	struct kvm_cpuid_entry2 cpuid_entries[HV_MAX_CPUID_ENTRIES];
+	int i, nent = 0;
+
+	/* Set the core cpuid entries required for Hyper-V */
+	memcpy(&cpuid_entries[nent], &core_cpuid_entries,
+	       sizeof(core_cpuid_entries));
+	nent += ARRAY_SIZE(core_cpuid_entries);
 
 	if (kvm_x86_ops->nested_get_evmcs_version)
 		evmcs_ver = kvm_x86_ops->nested_get_evmcs_version(vcpu);
 
-	/* Skip NESTED_FEATURES if eVMCS is not supported */
-	if (!evmcs_ver)
-		--nent;
+	if (evmcs_ver) {
+		/* EVMCS is enabled, add the required EVMCS CPUID leafs */
+		memcpy(&cpuid_entries[nent], &evmcs_cpuid_entries,
+		       sizeof(evmcs_cpuid_entries));
+		nent += ARRAY_SIZE(evmcs_cpuid_entries);
+	}
 
 	if (cpuid->nent < nent)
 		return -E2BIG;
@@ -1821,7 +1839,7 @@  int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
 		case HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS:
 			memcpy(signature, "Linux KVM Hv", 12);
 
-			ent->eax = HYPERV_CPUID_NESTED_FEATURES;
+			ent->eax = cpuid_entries[nent - 1].function;
 			ent->ebx = signature[0];
 			ent->ecx = signature[1];
 			ent->edx = signature[2];