From 1cd2b08f7cc4a57cc1b04f62b6349970d13456c3 Mon Sep 17 00:00:00 2001 From: Jing Zhang Date: Mon, 15 Jan 2024 14:02:09 -0800 Subject: KVM: arm64: selftests: Handle feature fields with nonzero minimum value correctly There are some feature fields with nonzero minimum valid value. Make sure get_safe_value() won't return invalid field values for them. Also fix a bug that wrongly uses the feature bits type as the feature bits sign causing all fields as signed in the get_safe_value() and get_invalid_value(). Fixes: 54a9ea73527d ("KVM: arm64: selftests: Test for setting ID register from usersapce") Reported-by: Zenghui Yu Reported-by: Itaru Kitayama Tested-by: Itaru Kitayama Signed-off-by: Jing Zhang Link: https://lore.kernel.org/r/20240115220210.3966064-2-jingzhangos@google.com Signed-off-by: Oliver Upton --- tools/testing/selftests/kvm/aarch64/set_id_regs.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/kvm/aarch64/set_id_regs.c b/tools/testing/selftests/kvm/aarch64/set_id_regs.c index bac05210b539..16e2338686c1 100644 --- a/tools/testing/selftests/kvm/aarch64/set_id_regs.c +++ b/tools/testing/selftests/kvm/aarch64/set_id_regs.c @@ -32,6 +32,10 @@ struct reg_ftr_bits { enum ftr_type type; uint8_t shift; uint64_t mask; + /* + * For FTR_EXACT, safe_val is used as the exact safe value. + * For FTR_LOWER_SAFE, safe_val is used as the minimal safe value. + */ int64_t safe_val; }; @@ -65,13 +69,13 @@ struct test_feature_reg { static const struct reg_ftr_bits ftr_id_aa64dfr0_el1[] = { S_REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64DFR0_EL1, PMUVer, 0), - REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64DFR0_EL1, DebugVer, 0), + REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64DFR0_EL1, DebugVer, ID_AA64DFR0_EL1_DebugVer_IMP), REG_FTR_END, }; static const struct reg_ftr_bits ftr_id_dfr0_el1[] = { - S_REG_FTR_BITS(FTR_LOWER_SAFE, ID_DFR0_EL1, PerfMon, 0), - REG_FTR_BITS(FTR_LOWER_SAFE, ID_DFR0_EL1, CopDbg, 0), + S_REG_FTR_BITS(FTR_LOWER_SAFE, ID_DFR0_EL1, PerfMon, ID_DFR0_EL1_PerfMon_PMUv3), + REG_FTR_BITS(FTR_LOWER_SAFE, ID_DFR0_EL1, CopDbg, ID_DFR0_EL1_CopDbg_Armv8), REG_FTR_END, }; @@ -224,13 +228,13 @@ uint64_t get_safe_value(const struct reg_ftr_bits *ftr_bits, uint64_t ftr) { uint64_t ftr_max = GENMASK_ULL(ARM64_FEATURE_FIELD_BITS - 1, 0); - if (ftr_bits->type == FTR_UNSIGNED) { + if (ftr_bits->sign == FTR_UNSIGNED) { switch (ftr_bits->type) { case FTR_EXACT: ftr = ftr_bits->safe_val; break; case FTR_LOWER_SAFE: - if (ftr > 0) + if (ftr > ftr_bits->safe_val) ftr--; break; case FTR_HIGHER_SAFE: @@ -252,7 +256,7 @@ uint64_t get_safe_value(const struct reg_ftr_bits *ftr_bits, uint64_t ftr) ftr = ftr_bits->safe_val; break; case FTR_LOWER_SAFE: - if (ftr > 0) + if (ftr > ftr_bits->safe_val) ftr--; break; case FTR_HIGHER_SAFE: @@ -276,7 +280,7 @@ uint64_t get_invalid_value(const struct reg_ftr_bits *ftr_bits, uint64_t ftr) { uint64_t ftr_max = GENMASK_ULL(ARM64_FEATURE_FIELD_BITS - 1, 0); - if (ftr_bits->type == FTR_UNSIGNED) { + if (ftr_bits->sign == FTR_UNSIGNED) { switch (ftr_bits->type) { case FTR_EXACT: ftr = max((uint64_t)ftr_bits->safe_val + 1, ftr + 1); -- cgit v1.2.3-59-g8ed1b From 06fdd894b473c6cc29c9b39b82e0941cefec4e51 Mon Sep 17 00:00:00 2001 From: Sean Christopherson Date: Fri, 2 Feb 2024 15:46:03 -0800 Subject: KVM: selftests: Fix GUEST_PRINTF() format warnings in ARM code Fix a pile of -Wformat warnings in the KVM ARM selftests code, almost all of which are benign "long" versus "long long" issues (selftests are 64-bit only, and the guest printf code treats "ll" the same as "l"). The code itself isn't problematic, but the warnings make it impossible to build ARM selftests with -Werror, which does detect real issues from time to time. Opportunistically have GUEST_ASSERT_BITMAP_REG() interpret set_expected, which is a bool, as an unsigned decimal value, i.e. have it print '0' or '1' instead of '0x0' or '0x1'. Signed-off-by: Sean Christopherson Tested-by: Zenghui Yu Link: https://lore.kernel.org/r/20240202234603.366925-1-seanjc@google.com Signed-off-by: Oliver Upton --- tools/testing/selftests/kvm/aarch64/arch_timer.c | 4 ++-- tools/testing/selftests/kvm/aarch64/debug-exceptions.c | 2 +- tools/testing/selftests/kvm/aarch64/hypercalls.c | 4 ++-- tools/testing/selftests/kvm/aarch64/page_fault_test.c | 2 +- tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c | 12 ++++++------ 5 files changed, 12 insertions(+), 12 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c index 274b8465b42a..d5e8f365aa01 100644 --- a/tools/testing/selftests/kvm/aarch64/arch_timer.c +++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c @@ -158,9 +158,9 @@ static void guest_validate_irq(unsigned int intid, /* Basic 'timer condition met' check */ __GUEST_ASSERT(xcnt >= cval, - "xcnt = 0x%llx, cval = 0x%llx, xcnt_diff_us = 0x%llx", + "xcnt = 0x%lx, cval = 0x%lx, xcnt_diff_us = 0x%lx", xcnt, cval, xcnt_diff_us); - __GUEST_ASSERT(xctl & CTL_ISTATUS, "xcnt = 0x%llx", xcnt); + __GUEST_ASSERT(xctl & CTL_ISTATUS, "xcnt = 0x%lx", xcnt); WRITE_ONCE(shared_data->nr_iter, shared_data->nr_iter + 1); } diff --git a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c index 866002917441..2582c49e525a 100644 --- a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c +++ b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c @@ -365,7 +365,7 @@ static void guest_wp_handler(struct ex_regs *regs) static void guest_ss_handler(struct ex_regs *regs) { - __GUEST_ASSERT(ss_idx < 4, "Expected index < 4, got '%u'", ss_idx); + __GUEST_ASSERT(ss_idx < 4, "Expected index < 4, got '%lu'", ss_idx); ss_addr[ss_idx++] = regs->pc; regs->pstate |= SPSR_SS; } diff --git a/tools/testing/selftests/kvm/aarch64/hypercalls.c b/tools/testing/selftests/kvm/aarch64/hypercalls.c index 31f66ba97228..c62739d897d6 100644 --- a/tools/testing/selftests/kvm/aarch64/hypercalls.c +++ b/tools/testing/selftests/kvm/aarch64/hypercalls.c @@ -105,12 +105,12 @@ static void guest_test_hvc(const struct test_hvc_info *hc_info) case TEST_STAGE_HVC_IFACE_FEAT_DISABLED: case TEST_STAGE_HVC_IFACE_FALSE_INFO: __GUEST_ASSERT(res.a0 == SMCCC_RET_NOT_SUPPORTED, - "a0 = 0x%lx, func_id = 0x%x, arg1 = 0x%llx, stage = %u", + "a0 = 0x%lx, func_id = 0x%x, arg1 = 0x%lx, stage = %u", res.a0, hc_info->func_id, hc_info->arg1, stage); break; case TEST_STAGE_HVC_IFACE_FEAT_ENABLED: __GUEST_ASSERT(res.a0 != SMCCC_RET_NOT_SUPPORTED, - "a0 = 0x%lx, func_id = 0x%x, arg1 = 0x%llx, stage = %u", + "a0 = 0x%lx, func_id = 0x%x, arg1 = 0x%lx, stage = %u", res.a0, hc_info->func_id, hc_info->arg1, stage); break; default: diff --git a/tools/testing/selftests/kvm/aarch64/page_fault_test.c b/tools/testing/selftests/kvm/aarch64/page_fault_test.c index 08a5ca5bed56..7bbd9fb5c8d6 100644 --- a/tools/testing/selftests/kvm/aarch64/page_fault_test.c +++ b/tools/testing/selftests/kvm/aarch64/page_fault_test.c @@ -292,7 +292,7 @@ static void guest_code(struct test_desc *test) static void no_dabt_handler(struct ex_regs *regs) { - GUEST_FAIL("Unexpected dabt, far_el1 = 0x%llx", read_sysreg(far_el1)); + GUEST_FAIL("Unexpected dabt, far_el1 = 0x%lx", read_sysreg(far_el1)); } static void no_iabt_handler(struct ex_regs *regs) diff --git a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c index 9d51b5691349..f8f0c655c723 100644 --- a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c +++ b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c @@ -195,11 +195,11 @@ struct pmc_accessor pmc_accessors[] = { \ if (set_expected) \ __GUEST_ASSERT((_tval & mask), \ - "tval: 0x%lx; mask: 0x%lx; set_expected: 0x%lx", \ + "tval: 0x%lx; mask: 0x%lx; set_expected: %u", \ _tval, mask, set_expected); \ else \ __GUEST_ASSERT(!(_tval & mask), \ - "tval: 0x%lx; mask: 0x%lx; set_expected: 0x%lx", \ + "tval: 0x%lx; mask: 0x%lx; set_expected: %u", \ _tval, mask, set_expected); \ } @@ -286,7 +286,7 @@ static void test_access_pmc_regs(struct pmc_accessor *acc, int pmc_idx) acc->write_typer(pmc_idx, write_data); read_data = acc->read_typer(pmc_idx); __GUEST_ASSERT(read_data == write_data, - "pmc_idx: 0x%lx; acc_idx: 0x%lx; read_data: 0x%lx; write_data: 0x%lx", + "pmc_idx: 0x%x; acc_idx: 0x%lx; read_data: 0x%lx; write_data: 0x%lx", pmc_idx, PMC_ACC_TO_IDX(acc), read_data, write_data); /* @@ -297,14 +297,14 @@ static void test_access_pmc_regs(struct pmc_accessor *acc, int pmc_idx) /* The count value must be 0, as it is disabled and reset */ __GUEST_ASSERT(read_data == 0, - "pmc_idx: 0x%lx; acc_idx: 0x%lx; read_data: 0x%lx", + "pmc_idx: 0x%x; acc_idx: 0x%lx; read_data: 0x%lx", pmc_idx, PMC_ACC_TO_IDX(acc), read_data); write_data = read_data + pmc_idx + 0x12345; acc->write_cntr(pmc_idx, write_data); read_data = acc->read_cntr(pmc_idx); __GUEST_ASSERT(read_data == write_data, - "pmc_idx: 0x%lx; acc_idx: 0x%lx; read_data: 0x%lx; write_data: 0x%lx", + "pmc_idx: 0x%x; acc_idx: 0x%lx; read_data: 0x%lx; write_data: 0x%lx", pmc_idx, PMC_ACC_TO_IDX(acc), read_data, write_data); } @@ -379,7 +379,7 @@ static void guest_code(uint64_t expected_pmcr_n) int i, pmc; __GUEST_ASSERT(expected_pmcr_n <= ARMV8_PMU_MAX_GENERAL_COUNTERS, - "Expected PMCR.N: 0x%lx; ARMv8 general counters: 0x%lx", + "Expected PMCR.N: 0x%lx; ARMv8 general counters: 0x%x", expected_pmcr_n, ARMV8_PMU_MAX_GENERAL_COUNTERS); pmcr = read_sysreg(pmcr_el0); -- cgit v1.2.3-59-g8ed1b From 8cdc71fbf65567dca6f52aac206d91754ad55147 Mon Sep 17 00:00:00 2001 From: Oliver Upton Date: Mon, 12 Feb 2024 21:09:33 +0000 Subject: KVM: selftests: Print timer ctl register in ISTATUS assertion Zenghui noted that the test assertion for the ISTATUS bit is printing the current timer value instead of the control register in the case of failure. While the assertion is sound, printing CNT isn't informative. Change things around to actually print the CTL register value instead. Reported-by: Zenghui Yu Closes: https://lore.kernel.org/kvmarm/3188e6f1-f150-f7d0-6c2b-5b7608b0b012@huawei.com/ Reviewed-by: Zenghui Yu Link: https://lore.kernel.org/r/20240212210932.3095265-2-oliver.upton@linux.dev Signed-off-by: Oliver Upton --- tools/testing/selftests/kvm/aarch64/arch_timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c index d5e8f365aa01..ab4b604d8ec0 100644 --- a/tools/testing/selftests/kvm/aarch64/arch_timer.c +++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c @@ -160,7 +160,7 @@ static void guest_validate_irq(unsigned int intid, __GUEST_ASSERT(xcnt >= cval, "xcnt = 0x%lx, cval = 0x%lx, xcnt_diff_us = 0x%lx", xcnt, cval, xcnt_diff_us); - __GUEST_ASSERT(xctl & CTL_ISTATUS, "xcnt = 0x%lx", xcnt); + __GUEST_ASSERT(xctl & CTL_ISTATUS, "xctl = 0x%lx", xctl); WRITE_ONCE(shared_data->nr_iter, shared_data->nr_iter + 1); } -- cgit v1.2.3-59-g8ed1b From 43b3bedb7cc4348f2885a30e960b63b94d1be381 Mon Sep 17 00:00:00 2001 From: Raghavendra Rao Ananta Date: Wed, 22 Nov 2023 22:15:26 +0000 Subject: KVM: selftests: aarch64: Remove unused functions from vpmu test vpmu_counter_access's disable_counter() carries a bug that disables all the counters that are enabled, instead of just the requested one. Fortunately, it's not an issue as there are no callers of it. Hence, instead of fixing it, remove the definition entirely. Remove enable_counter() as it's unused as well. Signed-off-by: Raghavendra Rao Ananta Reviewed-by: Zenghui Yu Link: https://lore.kernel.org/r/20231122221526.2750966-1-rananta@google.com Signed-off-by: Oliver Upton --- .../testing/selftests/kvm/aarch64/vpmu_counter_access.c | 16 ---------------- 1 file changed, 16 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c index f8f0c655c723..1b51cd11ee93 100644 --- a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c +++ b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c @@ -93,22 +93,6 @@ static inline void write_sel_evtyper(int sel, unsigned long val) isb(); } -static inline void enable_counter(int idx) -{ - uint64_t v = read_sysreg(pmcntenset_el0); - - write_sysreg(BIT(idx) | v, pmcntenset_el0); - isb(); -} - -static inline void disable_counter(int idx) -{ - uint64_t v = read_sysreg(pmcntenset_el0); - - write_sysreg(BIT(idx) | v, pmcntenclr_el0); - isb(); -} - static void pmu_disable_reset(void) { uint64_t pmcr = read_sysreg(pmcr_el0); -- cgit v1.2.3-59-g8ed1b