2024-02-02 23:46:30

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH] 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 <[email protected]>
---

Compile tested only, mainly because I'm rushing through things on Friday
afternoon, not because testing on ARM is actually problematic for me.

tools/testing/selftests/kvm/aarch64/arch_timer.c | 4 ++--
.../testing/selftests/kvm/aarch64/debug-exceptions.c | 2 +-
tools/testing/selftests/kvm/aarch64/hypercalls.c | 4 ++--
.../testing/selftests/kvm/aarch64/page_fault_test.c | 2 +-
.../selftests/kvm/aarch64/vpmu_counter_access.c | 12 ++++++------
5 files changed, 12 insertions(+), 12 deletions(-)

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);

base-commit: 41bccc98fb7931d63d03f326a746ac4d429c1dd3
--
2.43.0.594.gd9cf4e227d-goog



2024-02-03 22:29:41

by Oliver Upton

[permalink] [raw]
Subject: Re: [PATCH] KVM: selftests: Fix GUEST_PRINTF() format warnings in ARM code

On Fri, Feb 02, 2024 at 03:46:03PM -0800, Sean Christopherson wrote:
> 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 <[email protected]>

This all looks good to me. Just want to confirm before applying: you're
not planning any other patches that'll go through your tree that depend
on this right?

Figured not since you sent it separately, but just want to be sure.

--
Thanks,
Oliver

2024-02-05 23:32:23

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH] KVM: selftests: Fix GUEST_PRINTF() format warnings in ARM code

On Sat, Feb 03, 2024, Oliver Upton wrote:
> On Fri, Feb 02, 2024 at 03:46:03PM -0800, Sean Christopherson wrote:
> > 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 <[email protected]>
>
> This all looks good to me. Just want to confirm before applying: you're
> not planning any other patches that'll go through your tree that depend
> on this right?

Correct. There's one patch in kvm-x86/selftests (see below) that touches some
of the same files, but unless I had a bad -ENOCOFFEE on Friday, there are no conflicts.

[2/5] KVM: selftests: aarch64: Remove redundant newlines
https://github.com/kvm-x86/linux/commit/95be17e4008b

2024-02-08 05:00:28

by Zenghui Yu

[permalink] [raw]
Subject: Re: [PATCH] KVM: selftests: Fix GUEST_PRINTF() format warnings in ARM code

On 2024/2/3 7:46, Sean Christopherson wrote:
> 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 <[email protected]>

Tested-by: Zenghui Yu <[email protected]>

> 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);

Unrelated to your patch, but it looks to me that we actually want to
print 'xctl' instead of 'xcnt' in the second __GUEST_ASSERT().

Thanks,
Zenghui

2024-02-12 20:50:49

by Oliver Upton

[permalink] [raw]
Subject: Re: [PATCH] KVM: selftests: Fix GUEST_PRINTF() format warnings in ARM code

+cc Anup

FYI -- this patch is touching the arch_timer code. I did a test merge
with kvm_riscv_queue and there weren't any conflicts, but in case that
changes this patch will appear on kvm-arm64/misc in my tree.

--
Thanks,
Oliver

On Fri, Feb 02, 2024 at 03:46:03PM -0800, Sean Christopherson wrote:
> 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 <[email protected]>

2024-02-12 20:55:25

by Oliver Upton

[permalink] [raw]
Subject: Re: [PATCH] KVM: selftests: Fix GUEST_PRINTF() format warnings in ARM code

Dammit, forgot to actually CC Anup. I'll blame jet lag.

On Mon, Feb 12, 2024 at 08:50:38PM +0000, Oliver Upton wrote:
> +cc Anup
>
> FYI -- this patch is touching the arch_timer code. I did a test merge
> with kvm_riscv_queue and there weren't any conflicts, but in case that
> changes this patch will appear on kvm-arm64/misc in my tree.
>
> --
> Thanks,
> Oliver
>
> On Fri, Feb 02, 2024 at 03:46:03PM -0800, Sean Christopherson wrote:
> > 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 <[email protected]>

--
Thanks,
Oliver

2024-02-12 21:03:26

by Oliver Upton

[permalink] [raw]
Subject: Re: [PATCH] KVM: selftests: Fix GUEST_PRINTF() format warnings in ARM code

On Fri, 2 Feb 2024 15:46:03 -0800, Sean Christopherson wrote:
> 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'.
>
> [...]

Applied to kvmarm/next, thanks!

[1/1] KVM: selftests: Fix GUEST_PRINTF() format warnings in ARM code
https://git.kernel.org/kvmarm/kvmarm/c/06fdd894b473

--
Best,
Oliver