Essentially v3 of Takahiro's patch. Update cpuid->nent on a successful
KVM_GET_CPUID2 so that userspace knows exactly how many entries were
filled. Add a testcase to verify KVM's ABI.
v3:
- Don't bother updating cpuid->nent in the error path, the data is never
copied to userspace.
- Add testcase to cpuid_test
v2: https://lore.kernel.org/all/[email protected]
v1: https://lore.kernel.org/all/[email protected]
Sean Christopherson (2):
KVM: x86: Update number of entries for KVM_GET_CPUID2 on success, not
failure
KVM: selftests: Extend cpuid_test to verify KVM_GET_CPUID2 "nent"
updates
arch/x86/kvm/cpuid.c | 13 ++++--------
.../testing/selftests/kvm/x86_64/cpuid_test.c | 21 +++++++++++++++++++
2 files changed, 25 insertions(+), 9 deletions(-)
base-commit: b9846a698c9aff4eb2214a06ac83638ad098f33f
--
2.41.0.rc0.172.g3f132b7071-goog
Update cpuid->nent if and only if kvm_vcpu_ioctl_get_cpuid2() succeeds.
The sole caller copies @cpuid to userspace only on success, i.e. the
existing code effectively does nothing.
Arguably, KVM should report the number of entries when returning -E2BIG so
that userspace doesn't have to guess the size, but all other similar KVM
ioctls() don't report the size either, i.e. userspace is conditioned to
guess.
Suggested-by: Takahiro Itazuri <[email protected]>
Link: https://lore.kernel.org/all/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/cpuid.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 0c9660a07b23..241f554f1764 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -501,20 +501,15 @@ int kvm_vcpu_ioctl_get_cpuid2(struct kvm_vcpu *vcpu,
struct kvm_cpuid2 *cpuid,
struct kvm_cpuid_entry2 __user *entries)
{
- int r;
-
- r = -E2BIG;
if (cpuid->nent < vcpu->arch.cpuid_nent)
- goto out;
- r = -EFAULT;
+ return -E2BIG;
+
if (copy_to_user(entries, vcpu->arch.cpuid_entries,
vcpu->arch.cpuid_nent * sizeof(struct kvm_cpuid_entry2)))
- goto out;
- return 0;
+ return -EFAULT;
-out:
cpuid->nent = vcpu->arch.cpuid_nent;
- return r;
+ return 0;
}
/* Mask kvm_cpu_caps for @leaf with the raw CPUID capabilities of this CPU. */
--
2.41.0.rc0.172.g3f132b7071-goog
Verify that KVM reports the actual number of CPUID entries on success, but
doesn't touch the userspace struct on failure (which for better or worse,
is KVM's ABI).
Signed-off-by: Sean Christopherson <[email protected]>
---
.../testing/selftests/kvm/x86_64/cpuid_test.c | 21 +++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/tools/testing/selftests/kvm/x86_64/cpuid_test.c b/tools/testing/selftests/kvm/x86_64/cpuid_test.c
index 2fc3ad9c887e..d3c3aa93f090 100644
--- a/tools/testing/selftests/kvm/x86_64/cpuid_test.c
+++ b/tools/testing/selftests/kvm/x86_64/cpuid_test.c
@@ -163,6 +163,25 @@ static void set_cpuid_after_run(struct kvm_vcpu *vcpu)
ent->eax = eax;
}
+static void test_get_cpuid2(struct kvm_vcpu *vcpu)
+{
+ struct kvm_cpuid2 *cpuid = allocate_kvm_cpuid2(vcpu->cpuid->nent + 1);
+ int i, r;
+
+ vcpu_ioctl(vcpu, KVM_GET_CPUID2, cpuid);
+ TEST_ASSERT(cpuid->nent == vcpu->cpuid->nent,
+ "KVM didn't update nent on success, wanted %u, got %u\n",
+ vcpu->cpuid->nent, cpuid->nent);
+
+ for (i = 0; i < vcpu->cpuid->nent; i++) {
+ cpuid->nent = i;
+ r = __vcpu_ioctl(vcpu, KVM_GET_CPUID2, cpuid);
+ TEST_ASSERT(r && errno == E2BIG, KVM_IOCTL_ERROR(KVM_GET_CPUID2, r));
+ TEST_ASSERT(cpuid->nent == i, "KVM modified nent on failure");
+ }
+ free(cpuid);
+}
+
int main(void)
{
struct kvm_vcpu *vcpu;
@@ -183,5 +202,7 @@ int main(void)
set_cpuid_after_run(vcpu);
+ test_get_cpuid2(vcpu);
+
kvm_vm_free(vm);
}
--
2.41.0.rc0.172.g3f132b7071-goog
On Fri, 26 May 2023 14:03:38 -0700, Sean Christopherson wrote:
> Essentially v3 of Takahiro's patch. Update cpuid->nent on a successful
> KVM_GET_CPUID2 so that userspace knows exactly how many entries were
> filled. Add a testcase to verify KVM's ABI.
>
> v3:
> - Don't bother updating cpuid->nent in the error path, the data is never
> copied to userspace.
> - Add testcase to cpuid_test
>
> [...]
Applied to kvm-x86 misc, thanks!
[1/2] KVM: x86: Update number of entries for KVM_GET_CPUID2 on success, not failure
https://github.com/kvm-x86/linux/commit/ab322c43cce9
[2/2] KVM: selftests: Extend cpuid_test to verify KVM_GET_CPUID2 "nent" updates
https://github.com/kvm-x86/linux/commit/2c7613131998
--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes
Date: Fri, 26 May 2023 14:03:38 -0700
From: Sean Christopherson <[email protected]>
> Essentially v3 of Takahiro's patch. Update cpuid->nent on a successful
> KVM_GET_CPUID2 so that userspace knows exactly how many entries were
> filled. Add a testcase to verify KVM's ABI.
Sorry for my late reply and thank you for posting this revision!
Best regards,
Takahiro Itazuri