| Message ID | 20241009154953.1073471-4-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: selftests: Morph max_guest_mem to mmu_stress | expand |
On Wed, Oct 09, 2024 at 08:49:42AM -0700, Sean Christopherson wrote: > Return a uint64_t from vcpu_get_reg() instead of having the caller provide > a pointer to storage, as none of the vcpu_get_reg() usage in KVM selftests > accesses a register larger than 64 bits, and vcpu_set_reg() only accepts a > 64-bit value. If a use case comes along that needs to get a register that > is larger than 64 bits, then a utility can be added to assert success and > take a void pointer, but until then, forcing an out param yields ugly code > and prevents feeding the output of vcpu_get_reg() into vcpu_set_reg(). This commit, which is in today's -next as 5c6c7b71a45c9c, breaks the build on arm64: aarch64/psci_test.c: In function ‘host_test_system_off2’: aarch64/psci_test.c:247:9: error: too many arguments to function ‘vcpu_get_reg’ 247 | vcpu_get_reg(target, KVM_REG_ARM_PSCI_VERSION, &psci_version); | ^~~~~~~~~~~~ In file included from aarch64/psci_test.c:18: include/kvm_util.h:705:24: note: declared here 705 | static inline uint64_t vcpu_get_reg(struct kvm_vcpu *vcpu, uint64_t id) | ^~~~~~~~~~~~ At top level: cc1: note: unrecognized command-line option ‘-Wno-gnu-variable-sized-type-not-at -end’ may have been intended to silence earlier diagnostics since the updates done to that file did not take account of 72be5aa6be4 ("KVM: selftests: Add test for PSCI SYSTEM_OFF2") which has been merged in the kvm-arm64 tree.
On Fri, Nov 01, 2024, Mark Brown wrote: > On Wed, Oct 09, 2024 at 08:49:42AM -0700, Sean Christopherson wrote: > > Return a uint64_t from vcpu_get_reg() instead of having the caller provide > > a pointer to storage, as none of the vcpu_get_reg() usage in KVM selftests > > accesses a register larger than 64 bits, and vcpu_set_reg() only accepts a > > 64-bit value. If a use case comes along that needs to get a register that > > is larger than 64 bits, then a utility can be added to assert success and > > take a void pointer, but until then, forcing an out param yields ugly code > > and prevents feeding the output of vcpu_get_reg() into vcpu_set_reg(). > > This commit, which is in today's -next as 5c6c7b71a45c9c, breaks the > build on arm64: > > aarch64/psci_test.c: In function ‘host_test_system_off2’: > aarch64/psci_test.c:247:9: error: too many arguments to function ‘vcpu_get_reg’ > 247 | vcpu_get_reg(target, KVM_REG_ARM_PSCI_VERSION, &psci_version); > | ^~~~~~~~~~~~ > In file included from aarch64/psci_test.c:18: > include/kvm_util.h:705:24: note: declared here > 705 | static inline uint64_t vcpu_get_reg(struct kvm_vcpu *vcpu, uint64_t id) > | ^~~~~~~~~~~~ > At top level: > cc1: note: unrecognized command-line option ‘-Wno-gnu-variable-sized-type-not-at > -end’ may have been intended to silence earlier diagnostics > > since the updates done to that file did not take account of 72be5aa6be4 > ("KVM: selftests: Add test for PSCI SYSTEM_OFF2") which has been merged > in the kvm-arm64 tree. Bugger. In hindsight, it's obvious that of course arch selftests would add usage of vcpu_get_reg(). Unless someone has a better idea, I'll drop the series from kvm-x86, post a new version that applies on linux-next, and then re-apply the series just before the v6.13 merge window (rinse and repeat as needed if more vcpu_get_reg() users come along). That would be a good oppurtunity to do the $(ARCH) directory switch[*] too, e.g. have a "selftests_late" or whatever topic branch. Sorry for the pain Mark, you've been playing janitor for us too much lately. [*] https://lore.kernel.org/all/20240826190116.145945-1-seanjc@google.com
Hey, On Fri, Nov 01, 2024 at 07:48:00AM -0700, Sean Christopherson wrote: > On Fri, Nov 01, 2024, Mark Brown wrote: > > On Wed, Oct 09, 2024 at 08:49:42AM -0700, Sean Christopherson wrote: > > > Return a uint64_t from vcpu_get_reg() instead of having the caller provide > > > a pointer to storage, as none of the vcpu_get_reg() usage in KVM selftests > > > accesses a register larger than 64 bits, and vcpu_set_reg() only accepts a > > > 64-bit value. If a use case comes along that needs to get a register that > > > is larger than 64 bits, then a utility can be added to assert success and > > > take a void pointer, but until then, forcing an out param yields ugly code > > > and prevents feeding the output of vcpu_get_reg() into vcpu_set_reg(). > > > > This commit, which is in today's -next as 5c6c7b71a45c9c, breaks the > > build on arm64: > > > > aarch64/psci_test.c: In function ‘host_test_system_off2’: > > aarch64/psci_test.c:247:9: error: too many arguments to function ‘vcpu_get_reg’ > > 247 | vcpu_get_reg(target, KVM_REG_ARM_PSCI_VERSION, &psci_version); > > | ^~~~~~~~~~~~ > > In file included from aarch64/psci_test.c:18: > > include/kvm_util.h:705:24: note: declared here > > 705 | static inline uint64_t vcpu_get_reg(struct kvm_vcpu *vcpu, uint64_t id) > > | ^~~~~~~~~~~~ > > At top level: > > cc1: note: unrecognized command-line option ‘-Wno-gnu-variable-sized-type-not-at > > -end’ may have been intended to silence earlier diagnostics > > > > since the updates done to that file did not take account of 72be5aa6be4 > > ("KVM: selftests: Add test for PSCI SYSTEM_OFF2") which has been merged > > in the kvm-arm64 tree. > > Bugger. In hindsight, it's obvious that of course arch selftests would add usage > of vcpu_get_reg(). > > Unless someone has a better idea, I'll drop the series from kvm-x86, post a new > version that applies on linux-next, and then re-apply the series just before the > v6.13 merge window (rinse and repeat as needed if more vcpu_get_reg() users come > along). Can you instead just push out a topic branch and let the affected maintainers deal with it? This is the usual way we handle conflicts between trees... > That would be a good oppurtunity to do the $(ARCH) directory switch[*] too, e.g. > have a "selftests_late" or whatever topic branch. The right time to do KVM-wide changes (even selftests) is *early* in the development cycle, not last minute. It gives us plenty of time to iron out the wrinkles. > Sorry for the pain Mark, you've been playing janitor for us too much lately. +1, appreciate your help on this.
On Fri, Nov 01, 2024, Oliver Upton wrote: > Hey, > > On Fri, Nov 01, 2024 at 07:48:00AM -0700, Sean Christopherson wrote: > > On Fri, Nov 01, 2024, Mark Brown wrote: > > > On Wed, Oct 09, 2024 at 08:49:42AM -0700, Sean Christopherson wrote: > > > > Return a uint64_t from vcpu_get_reg() instead of having the caller provide > > > > a pointer to storage, as none of the vcpu_get_reg() usage in KVM selftests > > > > accesses a register larger than 64 bits, and vcpu_set_reg() only accepts a > > > > 64-bit value. If a use case comes along that needs to get a register that > > > > is larger than 64 bits, then a utility can be added to assert success and > > > > take a void pointer, but until then, forcing an out param yields ugly code > > > > and prevents feeding the output of vcpu_get_reg() into vcpu_set_reg(). > > > > > > This commit, which is in today's -next as 5c6c7b71a45c9c, breaks the > > > build on arm64: > > > > > > aarch64/psci_test.c: In function ‘host_test_system_off2’: > > > aarch64/psci_test.c:247:9: error: too many arguments to function ‘vcpu_get_reg’ > > > 247 | vcpu_get_reg(target, KVM_REG_ARM_PSCI_VERSION, &psci_version); > > > | ^~~~~~~~~~~~ > > > In file included from aarch64/psci_test.c:18: > > > include/kvm_util.h:705:24: note: declared here > > > 705 | static inline uint64_t vcpu_get_reg(struct kvm_vcpu *vcpu, uint64_t id) > > > | ^~~~~~~~~~~~ > > > At top level: > > > cc1: note: unrecognized command-line option ‘-Wno-gnu-variable-sized-type-not-at > > > -end’ may have been intended to silence earlier diagnostics > > > > > > since the updates done to that file did not take account of 72be5aa6be4 > > > ("KVM: selftests: Add test for PSCI SYSTEM_OFF2") which has been merged > > > in the kvm-arm64 tree. > > > > Bugger. In hindsight, it's obvious that of course arch selftests would add usage > > of vcpu_get_reg(). > > > > Unless someone has a better idea, I'll drop the series from kvm-x86, post a new > > version that applies on linux-next, and then re-apply the series just before the > > v6.13 merge window (rinse and repeat as needed if more vcpu_get_reg() users come > > along). > > Can you instead just push out a topic branch and let the affected > maintainers deal with it? This is the usual way we handle conflicts > between trees... That'd work too, but as you note below, doing that now throws a wrench in things because essentially all arch maintainers would need merge that topic branch, otherwise linux-next would end up in the same state. > > That would be a good oppurtunity to do the $(ARCH) directory switch[*] too, e.g. > > have a "selftests_late" or whatever topic branch. > > The right time to do KVM-wide changes (even selftests) is *early* in the > development cycle, not last minute. It gives us plenty of time to iron out > the wrinkles. Yeah, that was the original plan, then the stupid strict aliasing bug happened, and I honestly forgot the vcpu_get_reg() changes would need to be consumed by other architectures. Other than letting me forget about this mess a few weeks earlier, there's no good reason to force this into 6.13. So, I'll drop the series from 6.13, post new versions of the this and the $(ARCH) series just before the merge window, and then either send a pull request to Paolo for 6.14 as soon as the 6.13 merge window closes, or ask/bribe Paolo to apply everything directly.
On Fri, Nov 01, 2024 at 08:59:16AM -0700, Sean Christopherson wrote: > > Can you instead just push out a topic branch and let the affected > > maintainers deal with it? This is the usual way we handle conflicts > > between trees... > > That'd work too, but as you note below, doing that now throws a wrench in things > because essentially all arch maintainers would need merge that topic branch, > otherwise linux-next would end up in the same state. TBH, I'm quite happy with that. Recent history has not been particularly convinincing to me that folks are actually testing arm64, let alone compiling for it when applying selftests patches. Otherwise, the alternative of respinning the global change for every -next breakage adds a decent amount of toil and gives the wrong impression of how long our patches have actually been baking in -next. > > > That would be a good oppurtunity to do the $(ARCH) directory switch[*] too, e.g. > > > have a "selftests_late" or whatever topic branch. > > > > The right time to do KVM-wide changes (even selftests) is *early* in the > > development cycle, not last minute. It gives us plenty of time to iron out > > the wrinkles. > > Yeah, that was the original plan, then the stupid strict aliasing bug happened, > and I honestly forgot the vcpu_get_reg() changes would need to be consumed by > other architectures. > > Other than letting me forget about this mess a few weeks earlier, there's no > good reason to force this into 6.13. So, I'll drop the series from 6.13, post > new versions of the this and the $(ARCH) series just before the merge window, > and then either send a pull request to Paolo for 6.14 as soon as the 6.13 merge > window closes, or ask/bribe Paolo to apply everything directly. Sounds good, punting to 6.14 seems reasonable.
On Fri, Nov 01, 2024, Oliver Upton wrote: > On Fri, Nov 01, 2024 at 08:59:16AM -0700, Sean Christopherson wrote: > > > Can you instead just push out a topic branch and let the affected > > > maintainers deal with it? This is the usual way we handle conflicts > > > between trees... > > > > That'd work too, but as you note below, doing that now throws a wrench in things > > because essentially all arch maintainers would need merge that topic branch, > > otherwise linux-next would end up in the same state. > > TBH, I'm quite happy with that. Recent history has not been particularly > convinincing to me that folks are actually testing arm64, let alone > compiling for it when applying selftests patches. FWIW, I did compile all patches on all KVM architectures, including selftests. But my base obviously didn't include the kvm-arm64 branch :-/ One thing I'll add to my workflow would be to do a local merge (and smoke test) of linux-next into kvm-x86 next before pushing it out. This isn't the only snafu this cycle where such a sanity check would have saved me and others a bit of pain. https://lore.kernel.org/all/20241101153857.GAZyT2EdLXKs7ZmDFx@fat_crate.local
On Fri, Nov 01, 2024 at 09:16:42AM -0700, Sean Christopherson wrote: > On Fri, Nov 01, 2024, Oliver Upton wrote: > > On Fri, Nov 01, 2024 at 08:59:16AM -0700, Sean Christopherson wrote: > > > > Can you instead just push out a topic branch and let the affected > > > > maintainers deal with it? This is the usual way we handle conflicts > > > > between trees... > > > > > > That'd work too, but as you note below, doing that now throws a wrench in things > > > because essentially all arch maintainers would need merge that topic branch, > > > otherwise linux-next would end up in the same state. > > > > TBH, I'm quite happy with that. Recent history has not been particularly > > convinincing to me that folks are actually testing arm64, let alone > > compiling for it when applying selftests patches. > > FWIW, I did compile all patches on all KVM architectures, including selftests. > But my base obviously didn't include the kvm-arm64 branch :-/ Oh, that rip wasn't aimed at you, commit 76f972c2cfdf ("KVM: selftests: Fix build on architectures other than x86_64") just came to mind. > One thing I'll add to my workflow would be to do a local merge (and smoke test) > of linux-next into kvm-x86 next before pushing it out. This isn't the only snafu > this cycle where such a sanity check would have saved me and others a bit of pain. Eh, shit happens, that's what -next is for :) The only point I wanted to make was that it is perfectly fine by me to spread the workload w/ a topic branch if things blow up sometime after your changes show up in -next.
On Fri, Nov 01, 2024, Oliver Upton wrote: > On Fri, Nov 01, 2024 at 09:16:42AM -0700, Sean Christopherson wrote: > > One thing I'll add to my workflow would be to do a local merge (and smoke test) > > of linux-next into kvm-x86 next before pushing it out. This isn't the only snafu > > this cycle where such a sanity check would have saved me and others a bit of pain. > > Eh, shit happens, that's what -next is for :) Heh, but I also don't actually test -next, which was another snafu (not my fault this time!) from this cycle[*]. Testing 6.12-next prior to the merge window wouldn't have made that any less painful to bisect, but I think it would at least have allowed me to detect that the issue specifically came in from linux-next, and the bug report would have gotten to PeterZ almost two months earlier. https://lore.kernel.org/all/ZwdA0sbA2tJA3IKh@google.com
On Fri, Nov 01, 2024 at 09:22:30AM -0700, Oliver Upton wrote: > On Fri, Nov 01, 2024 at 09:16:42AM -0700, Sean Christopherson wrote: > > One thing I'll add to my workflow would be to do a local merge (and smoke test) > > of linux-next into kvm-x86 next before pushing it out. This isn't the only snafu > > this cycle where such a sanity check would have saved me and others a bit of pain. > Eh, shit happens, that's what -next is for :) > The only point I wanted to make was that it is perfectly fine by me to > spread the workload w/ a topic branch if things blow up sometime after > your changes show up in -next. Yeah, the -next breakage is a bit annoying but so long as it gets fixed promptly it's kind of what it's there for. It's much more of an issue when things make it into mainline, and can be very problematic if it makes it into a tagged -rc (especially -rc1) or something.
diff --git a/tools/testing/selftests/kvm/aarch64/aarch32_id_regs.c b/tools/testing/selftests/kvm/aarch64/aarch32_id_regs.c index 8e5bd07a3727..447d61cae4db 100644 --- a/tools/testing/selftests/kvm/aarch64/aarch32_id_regs.c +++ b/tools/testing/selftests/kvm/aarch64/aarch32_id_regs.c @@ -97,7 +97,7 @@ static void test_user_raz_wi(struct kvm_vcpu *vcpu) uint64_t reg_id = raz_wi_reg_ids[i]; uint64_t val; - vcpu_get_reg(vcpu, reg_id, &val); + val = vcpu_get_reg(vcpu, reg_id); TEST_ASSERT_EQ(val, 0); /* @@ -106,7 +106,7 @@ static void test_user_raz_wi(struct kvm_vcpu *vcpu) */ vcpu_set_reg(vcpu, reg_id, BAD_ID_REG_VAL); - vcpu_get_reg(vcpu, reg_id, &val); + val = vcpu_get_reg(vcpu, reg_id); TEST_ASSERT_EQ(val, 0); } } @@ -126,14 +126,14 @@ static void test_user_raz_invariant(struct kvm_vcpu *vcpu) uint64_t reg_id = raz_invariant_reg_ids[i]; uint64_t val; - vcpu_get_reg(vcpu, reg_id, &val); + val = vcpu_get_reg(vcpu, reg_id); TEST_ASSERT_EQ(val, 0); r = __vcpu_set_reg(vcpu, reg_id, BAD_ID_REG_VAL); TEST_ASSERT(r < 0 && errno == EINVAL, "unexpected KVM_SET_ONE_REG error: r=%d, errno=%d", r, errno); - vcpu_get_reg(vcpu, reg_id, &val); + val = vcpu_get_reg(vcpu, reg_id); TEST_ASSERT_EQ(val, 0); } } @@ -144,7 +144,7 @@ static bool vcpu_aarch64_only(struct kvm_vcpu *vcpu) { uint64_t val, el0; - vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1), &val); + val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1)); el0 = FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL0), val); return el0 == ID_AA64PFR0_EL1_ELx_64BIT_ONLY; diff --git a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c index 2582c49e525a..b3f3025d2f02 100644 --- a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c +++ b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c @@ -501,7 +501,7 @@ void test_single_step_from_userspace(int test_cnt) TEST_ASSERT(ss_enable, "Unexpected KVM_EXIT_DEBUG"); /* Check if the current pc is expected. */ - vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pc), &pc); + pc = vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pc)); TEST_ASSERT(!test_pc || pc == test_pc, "Unexpected pc 0x%lx (expected 0x%lx)", pc, test_pc); @@ -583,7 +583,7 @@ int main(int argc, char *argv[]) uint64_t aa64dfr0; vm = vm_create_with_one_vcpu(&vcpu, guest_code); - vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64DFR0_EL1), &aa64dfr0); + aa64dfr0 = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64DFR0_EL1)); __TEST_REQUIRE(debug_version(aa64dfr0) >= 6, "Armv8 debug architecture not supported."); kvm_vm_free(vm); diff --git a/tools/testing/selftests/kvm/aarch64/hypercalls.c b/tools/testing/selftests/kvm/aarch64/hypercalls.c index 9d192ce0078d..ec54ec7726e9 100644 --- a/tools/testing/selftests/kvm/aarch64/hypercalls.c +++ b/tools/testing/selftests/kvm/aarch64/hypercalls.c @@ -173,7 +173,7 @@ static void test_fw_regs_before_vm_start(struct kvm_vcpu *vcpu) const struct kvm_fw_reg_info *reg_info = &fw_reg_info[i]; /* First 'read' should be an upper limit of the features supported */ - vcpu_get_reg(vcpu, reg_info->reg, &val); + val = vcpu_get_reg(vcpu, reg_info->reg); TEST_ASSERT(val == FW_REG_ULIMIT_VAL(reg_info->max_feat_bit), "Expected all the features to be set for reg: 0x%lx; expected: 0x%lx; read: 0x%lx", reg_info->reg, FW_REG_ULIMIT_VAL(reg_info->max_feat_bit), val); @@ -184,7 +184,7 @@ static void test_fw_regs_before_vm_start(struct kvm_vcpu *vcpu) "Failed to clear all the features of reg: 0x%lx; ret: %d", reg_info->reg, errno); - vcpu_get_reg(vcpu, reg_info->reg, &val); + val = vcpu_get_reg(vcpu, reg_info->reg); TEST_ASSERT(val == 0, "Expected all the features to be cleared for reg: 0x%lx", reg_info->reg); @@ -214,7 +214,7 @@ static void test_fw_regs_after_vm_start(struct kvm_vcpu *vcpu) * Before starting the VM, the test clears all the bits. * Check if that's still the case. */ - vcpu_get_reg(vcpu, reg_info->reg, &val); + val = vcpu_get_reg(vcpu, reg_info->reg); TEST_ASSERT(val == 0, "Expected all the features to be cleared for reg: 0x%lx", reg_info->reg); diff --git a/tools/testing/selftests/kvm/aarch64/no-vgic-v3.c b/tools/testing/selftests/kvm/aarch64/no-vgic-v3.c index 943d65fc6b0b..f80a519f73bb 100644 --- a/tools/testing/selftests/kvm/aarch64/no-vgic-v3.c +++ b/tools/testing/selftests/kvm/aarch64/no-vgic-v3.c @@ -164,7 +164,7 @@ int main(int argc, char *argv[]) uint64_t pfr0; vm = vm_create_with_one_vcpu(&vcpu, NULL); - vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1), &pfr0); + pfr0 = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1)); __TEST_REQUIRE(FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_GIC), pfr0), "GICv3 not supported."); kvm_vm_free(vm); diff --git a/tools/testing/selftests/kvm/aarch64/psci_test.c b/tools/testing/selftests/kvm/aarch64/psci_test.c index 61731a950def..544ebd2b121b 100644 --- a/tools/testing/selftests/kvm/aarch64/psci_test.c +++ b/tools/testing/selftests/kvm/aarch64/psci_test.c @@ -102,8 +102,8 @@ static void assert_vcpu_reset(struct kvm_vcpu *vcpu) { uint64_t obs_pc, obs_x0; - vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pc), &obs_pc); - vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.regs[0]), &obs_x0); + obs_pc = vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pc)); + obs_x0 = vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.regs[0])); TEST_ASSERT(obs_pc == CPU_ON_ENTRY_ADDR, "unexpected target cpu pc: %lx (expected: %lx)", @@ -143,7 +143,7 @@ static void host_test_cpu_on(void) */ vcpu_power_off(target); - vcpu_get_reg(target, KVM_ARM64_SYS_REG(SYS_MPIDR_EL1), &target_mpidr); + target_mpidr = vcpu_get_reg(target, KVM_ARM64_SYS_REG(SYS_MPIDR_EL1)); vcpu_args_set(source, 1, target_mpidr & MPIDR_HWID_BITMASK); enter_guest(source); diff --git a/tools/testing/selftests/kvm/aarch64/set_id_regs.c b/tools/testing/selftests/kvm/aarch64/set_id_regs.c index 2a3fe7914b72..8a1d92048aef 100644 --- a/tools/testing/selftests/kvm/aarch64/set_id_regs.c +++ b/tools/testing/selftests/kvm/aarch64/set_id_regs.c @@ -336,7 +336,7 @@ static uint64_t test_reg_set_success(struct kvm_vcpu *vcpu, uint64_t reg, uint64_t mask = ftr_bits->mask; uint64_t val, new_val, ftr; - vcpu_get_reg(vcpu, reg, &val); + val = vcpu_get_reg(vcpu, reg); ftr = (val & mask) >> shift; ftr = get_safe_value(ftr_bits, ftr); @@ -346,7 +346,7 @@ static uint64_t test_reg_set_success(struct kvm_vcpu *vcpu, uint64_t reg, val |= ftr; vcpu_set_reg(vcpu, reg, val); - vcpu_get_reg(vcpu, reg, &new_val); + new_val = vcpu_get_reg(vcpu, reg); TEST_ASSERT_EQ(new_val, val); return new_val; @@ -360,7 +360,7 @@ static void test_reg_set_fail(struct kvm_vcpu *vcpu, uint64_t reg, uint64_t val, old_val, ftr; int r; - vcpu_get_reg(vcpu, reg, &val); + val = vcpu_get_reg(vcpu, reg); ftr = (val & mask) >> shift; ftr = get_invalid_value(ftr_bits, ftr); @@ -374,7 +374,7 @@ static void test_reg_set_fail(struct kvm_vcpu *vcpu, uint64_t reg, TEST_ASSERT(r < 0 && errno == EINVAL, "Unexpected KVM_SET_ONE_REG error: r=%d, errno=%d", r, errno); - vcpu_get_reg(vcpu, reg, &val); + val = vcpu_get_reg(vcpu, reg); TEST_ASSERT_EQ(val, old_val); } @@ -471,7 +471,7 @@ static void test_clidr(struct kvm_vcpu *vcpu) uint64_t clidr; int level; - vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CLIDR_EL1), &clidr); + clidr = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CLIDR_EL1)); /* find the first empty level in the cache hierarchy */ for (level = 1; level < 7; level++) { @@ -496,7 +496,7 @@ static void test_ctr(struct kvm_vcpu *vcpu) { u64 ctr; - vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CTR_EL0), &ctr); + ctr = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CTR_EL0)); ctr &= ~CTR_EL0_DIC_MASK; if (ctr & CTR_EL0_IminLine_MASK) ctr--; @@ -512,7 +512,7 @@ static void test_vcpu_ftr_id_regs(struct kvm_vcpu *vcpu) test_clidr(vcpu); test_ctr(vcpu); - vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_MPIDR_EL1), &val); + val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_MPIDR_EL1)); val++; vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_MPIDR_EL1), val); @@ -525,7 +525,7 @@ static void test_assert_id_reg_unchanged(struct kvm_vcpu *vcpu, uint32_t encodin size_t idx = encoding_to_range_idx(encoding); uint64_t observed; - vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(encoding), &observed); + observed = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(encoding)); TEST_ASSERT_EQ(test_reg_vals[idx], observed); } @@ -560,7 +560,7 @@ int main(void) vm = vm_create_with_one_vcpu(&vcpu, guest_code); /* Check for AARCH64 only system */ - vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1), &val); + val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1)); el0 = FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL0), val); aarch64_only = (el0 == ID_AA64PFR0_EL1_ELx_64BIT_ONLY); diff --git a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c index d31b9f64ba14..30d9c9e7ae35 100644 --- a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c +++ b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c @@ -440,8 +440,7 @@ static void create_vpmu_vm(void *guest_code) "Failed to create vgic-v3, skipping"); /* Make sure that PMUv3 support is indicated in the ID register */ - vcpu_get_reg(vpmu_vm.vcpu, - KVM_ARM64_SYS_REG(SYS_ID_AA64DFR0_EL1), &dfr0); + dfr0 = vcpu_get_reg(vpmu_vm.vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64DFR0_EL1)); pmuver = FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer), dfr0); TEST_ASSERT(pmuver != ID_AA64DFR0_EL1_PMUVer_IMP_DEF && pmuver >= ID_AA64DFR0_EL1_PMUVer_IMP, @@ -484,7 +483,7 @@ static void test_create_vpmu_vm_with_pmcr_n(uint64_t pmcr_n, bool expect_fail) create_vpmu_vm(guest_code); vcpu = vpmu_vm.vcpu; - vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0), &pmcr_orig); + pmcr_orig = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0)); pmcr = pmcr_orig; /* @@ -493,7 +492,7 @@ static void test_create_vpmu_vm_with_pmcr_n(uint64_t pmcr_n, bool expect_fail) */ set_pmcr_n(&pmcr, pmcr_n); vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0), pmcr); - vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0), &pmcr); + pmcr = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0)); if (expect_fail) TEST_ASSERT(pmcr_orig == pmcr, @@ -521,7 +520,7 @@ static void run_access_test(uint64_t pmcr_n) vcpu = vpmu_vm.vcpu; /* Save the initial sp to restore them later to run the guest again */ - vcpu_get_reg(vcpu, ARM64_CORE_REG(sp_el1), &sp); + sp = vcpu_get_reg(vcpu, ARM64_CORE_REG(sp_el1)); run_vcpu(vcpu, pmcr_n); @@ -572,12 +571,12 @@ static void run_pmregs_validity_test(uint64_t pmcr_n) * Test if the 'set' and 'clr' variants of the registers * are initialized based on the number of valid counters. */ - vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(set_reg_id), ®_val); + reg_val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(set_reg_id)); TEST_ASSERT((reg_val & (~valid_counters_mask)) == 0, "Initial read of set_reg: 0x%llx has unimplemented counters enabled: 0x%lx", KVM_ARM64_SYS_REG(set_reg_id), reg_val); - vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(clr_reg_id), ®_val); + reg_val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(clr_reg_id)); TEST_ASSERT((reg_val & (~valid_counters_mask)) == 0, "Initial read of clr_reg: 0x%llx has unimplemented counters enabled: 0x%lx", KVM_ARM64_SYS_REG(clr_reg_id), reg_val); @@ -589,12 +588,12 @@ static void run_pmregs_validity_test(uint64_t pmcr_n) */ vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(set_reg_id), max_counters_mask); - vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(set_reg_id), ®_val); + reg_val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(set_reg_id)); TEST_ASSERT((reg_val & (~valid_counters_mask)) == 0, "Read of set_reg: 0x%llx has unimplemented counters enabled: 0x%lx", KVM_ARM64_SYS_REG(set_reg_id), reg_val); - vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(clr_reg_id), ®_val); + reg_val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(clr_reg_id)); TEST_ASSERT((reg_val & (~valid_counters_mask)) == 0, "Read of clr_reg: 0x%llx has unimplemented counters enabled: 0x%lx", KVM_ARM64_SYS_REG(clr_reg_id), reg_val); @@ -625,7 +624,7 @@ static uint64_t get_pmcr_n_limit(void) uint64_t pmcr; create_vpmu_vm(guest_code); - vcpu_get_reg(vpmu_vm.vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0), &pmcr); + pmcr = vcpu_get_reg(vpmu_vm.vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0)); destroy_vpmu_vm(); return get_pmcr_n(pmcr); } diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h index bc7c242480d6..287a3ec06df4 100644 --- a/tools/testing/selftests/kvm/include/kvm_util.h +++ b/tools/testing/selftests/kvm/include/kvm_util.h @@ -702,11 +702,13 @@ static inline int __vcpu_set_reg(struct kvm_vcpu *vcpu, uint64_t id, uint64_t va return __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, ®); } -static inline void vcpu_get_reg(struct kvm_vcpu *vcpu, uint64_t id, void *addr) +static inline uint64_t vcpu_get_reg(struct kvm_vcpu *vcpu, uint64_t id) { - struct kvm_one_reg reg = { .id = id, .addr = (uint64_t)addr }; + uint64_t val; + struct kvm_one_reg reg = { .id = id, .addr = (uint64_t)&val }; vcpu_ioctl(vcpu, KVM_GET_ONE_REG, ®); + return val; } static inline void vcpu_set_reg(struct kvm_vcpu *vcpu, uint64_t id, uint64_t val) { diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c index fe4dc3693112..4af94272a758 100644 --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c @@ -281,8 +281,8 @@ void aarch64_vcpu_setup(struct kvm_vcpu *vcpu, struct kvm_vcpu_init *init) */ vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CPACR_EL1), 3 << 20); - vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_SCTLR_EL1), &sctlr_el1); - vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_TCR_EL1), &tcr_el1); + sctlr_el1 = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_SCTLR_EL1)); + tcr_el1 = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_TCR_EL1)); /* Configure base granule size */ switch (vm->mode) { @@ -360,8 +360,8 @@ void vcpu_arch_dump(FILE *stream, struct kvm_vcpu *vcpu, uint8_t indent) { uint64_t pstate, pc; - vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pstate), &pstate); - vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pc), &pc); + pstate = vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pstate)); + pc = vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pc)); fprintf(stream, "%*spstate: 0x%.16lx pc: 0x%.16lx\n", indent, "", pstate, pc); diff --git a/tools/testing/selftests/kvm/lib/riscv/processor.c b/tools/testing/selftests/kvm/lib/riscv/processor.c index 6ae47b3d6b25..dd663bcf0cc0 100644 --- a/tools/testing/selftests/kvm/lib/riscv/processor.c +++ b/tools/testing/selftests/kvm/lib/riscv/processor.c @@ -221,39 +221,39 @@ void vcpu_arch_dump(FILE *stream, struct kvm_vcpu *vcpu, uint8_t indent) { struct kvm_riscv_core core; - vcpu_get_reg(vcpu, RISCV_CORE_REG(mode), &core.mode); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.pc), &core.regs.pc); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.ra), &core.regs.ra); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.sp), &core.regs.sp); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.gp), &core.regs.gp); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.tp), &core.regs.tp); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t0), &core.regs.t0); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t1), &core.regs.t1); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t2), &core.regs.t2); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s0), &core.regs.s0); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s1), &core.regs.s1); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a0), &core.regs.a0); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a1), &core.regs.a1); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a2), &core.regs.a2); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a3), &core.regs.a3); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a4), &core.regs.a4); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a5), &core.regs.a5); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a6), &core.regs.a6); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a7), &core.regs.a7); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s2), &core.regs.s2); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s3), &core.regs.s3); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s4), &core.regs.s4); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s5), &core.regs.s5); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s6), &core.regs.s6); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s7), &core.regs.s7); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s8), &core.regs.s8); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s9), &core.regs.s9); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s10), &core.regs.s10); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s11), &core.regs.s11); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t3), &core.regs.t3); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t4), &core.regs.t4); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t5), &core.regs.t5); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t6), &core.regs.t6); + core.mode = vcpu_get_reg(vcpu, RISCV_CORE_REG(mode)); + core.regs.pc = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.pc)); + core.regs.ra = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.ra)); + core.regs.sp = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.sp)); + core.regs.gp = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.gp)); + core.regs.tp = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.tp)); + core.regs.t0 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t0)); + core.regs.t1 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t1)); + core.regs.t2 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t2)); + core.regs.s0 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s0)); + core.regs.s1 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s1)); + core.regs.a0 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a0)); + core.regs.a1 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a1)); + core.regs.a2 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a2)); + core.regs.a3 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a3)); + core.regs.a4 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a4)); + core.regs.a5 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a5)); + core.regs.a6 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a6)); + core.regs.a7 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.a7)); + core.regs.s2 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s2)); + core.regs.s3 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s3)); + core.regs.s4 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s4)); + core.regs.s5 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s5)); + core.regs.s6 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s6)); + core.regs.s7 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s7)); + core.regs.s8 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s8)); + core.regs.s9 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s9)); + core.regs.s10 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s10)); + core.regs.s11 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.s11)); + core.regs.t3 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t3)); + core.regs.t4 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t4)); + core.regs.t5 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t5)); + core.regs.t6 = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.t6)); fprintf(stream, " MODE: 0x%lx\n", core.mode); diff --git a/tools/testing/selftests/kvm/riscv/arch_timer.c b/tools/testing/selftests/kvm/riscv/arch_timer.c index 2c792228ac0b..9e370800a6a2 100644 --- a/tools/testing/selftests/kvm/riscv/arch_timer.c +++ b/tools/testing/selftests/kvm/riscv/arch_timer.c @@ -93,7 +93,7 @@ struct kvm_vm *test_vm_create(void) vcpu_init_vector_tables(vcpus[i]); /* Initialize guest timer frequency. */ - vcpu_get_reg(vcpus[0], RISCV_TIMER_REG(frequency), &timer_freq); + timer_freq = vcpu_get_reg(vcpus[0], RISCV_TIMER_REG(frequency)); sync_global_to_guest(vm, timer_freq); pr_debug("timer_freq: %lu\n", timer_freq); diff --git a/tools/testing/selftests/kvm/riscv/ebreak_test.c b/tools/testing/selftests/kvm/riscv/ebreak_test.c index 0e0712854953..cfed6c727bfc 100644 --- a/tools/testing/selftests/kvm/riscv/ebreak_test.c +++ b/tools/testing/selftests/kvm/riscv/ebreak_test.c @@ -60,7 +60,7 @@ int main(void) TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_DEBUG); - vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.pc), &pc); + pc = vcpu_get_reg(vcpu, RISCV_CORE_REG(regs.pc)); TEST_ASSERT_EQ(pc, LABEL_ADDRESS(sw_bp_1)); /* skip sw_bp_1 */ diff --git a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c index f299cbfd23ca..f45c0ecc902d 100644 --- a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c +++ b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c @@ -608,7 +608,7 @@ static void test_vm_events_overflow(void *guest_code) vcpu_init_vector_tables(vcpu); /* Initialize guest timer frequency. */ - vcpu_get_reg(vcpu, RISCV_TIMER_REG(frequency), &timer_freq); + timer_freq = vcpu_get_reg(vcpu, RISCV_TIMER_REG(frequency)); sync_global_to_guest(vm, timer_freq); run_vcpu(vcpu); diff --git a/tools/testing/selftests/kvm/s390x/resets.c b/tools/testing/selftests/kvm/s390x/resets.c index 357943f2bea8..b58f75b381e5 100644 --- a/tools/testing/selftests/kvm/s390x/resets.c +++ b/tools/testing/selftests/kvm/s390x/resets.c @@ -61,7 +61,7 @@ static void test_one_reg(struct kvm_vcpu *vcpu, uint64_t id, uint64_t value) { uint64_t eval_reg; - vcpu_get_reg(vcpu, id, &eval_reg); + eval_reg = vcpu_get_reg(vcpu, id); TEST_ASSERT(eval_reg == value, "value == 0x%lx", value); } diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c index a8d3afa0b86b..cce2520af720 100644 --- a/tools/testing/selftests/kvm/steal_time.c +++ b/tools/testing/selftests/kvm/steal_time.c @@ -269,9 +269,8 @@ static void guest_code(int cpu) static bool is_steal_time_supported(struct kvm_vcpu *vcpu) { uint64_t id = RISCV_SBI_EXT_REG(KVM_RISCV_SBI_EXT_STA); - unsigned long enabled; + unsigned long enabled = vcpu_get_reg(vcpu, id); - vcpu_get_reg(vcpu, id, &enabled); TEST_ASSERT(enabled == 0 || enabled == 1, "Expected boolean result"); return enabled;