On 29.02.2016 21:19, Andrew Jones wrote:
> Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx>
> ---
> lib/x86/desc.c | 6 +++---
> lib/x86/vm.c | 6 +++---
> x86/access.c | 4 ++--
> x86/eventinj.c | 8 ++++----
> x86/kvmclock.c | 2 +-
> x86/kvmclock_test.c | 6 +++---
> x86/msr.c | 2 +-
> x86/s3.c | 6 +++---
> x86/svm.c | 12 ++++++------
> x86/taskswitch.c | 2 +-
> x86/taskswitch2.c | 2 +-
> x86/tsc.c | 4 ++--
> x86/tscdeadline_latency.c | 2 +-
> x86/vmx.c | 18 +++++++++---------
> x86/vmx_tests.c | 20 ++++++++++----------
> x86/xsave.c | 2 +-
> 16 files changed, 51 insertions(+), 51 deletions(-)
>
> diff --git a/lib/x86/desc.c b/lib/x86/desc.c
> index 1a80c01283853..0a79a41b56cb1 100644
> --- a/lib/x86/desc.c
> +++ b/lib/x86/desc.c
> @@ -53,7 +53,7 @@ static void check_exception_table(struct ex_regs *regs)
> return;
> }
> }
> - printf("unhandled exception %d\n", regs->vector);
> + printf("unhandled exception %ld\n", regs->vector);
That could also be %lu instead, I think, but it likely does not really
matter here.
> exit(7);
> }
>
> @@ -75,9 +75,9 @@ void do_handle_exception(struct ex_regs *regs)
> exception_handlers[regs->vector](regs);
> return;
> }
> - printf("unhandled cpu exception %d\n", regs->vector);
> + printf("unhandled cpu exception %ld\n", regs->vector);
> if (regs->vector == 14)
> - printf("PF at %p addr %p\n", regs->rip, read_cr2());
> + printf("PF at 0x%lx addr 0x%lx\n", regs->rip, read_cr2());
> exit(7);
> }
>
...
> diff --git a/x86/kvmclock_test.c b/x86/kvmclock_test.c
> index b6da5ebbe1097..c8ffa126803d5 100644
> --- a/x86/kvmclock_test.c
> +++ b/x86/kvmclock_test.c
> @@ -67,7 +67,7 @@ static void kvm_clock_test(void *data)
> ++hv_test_info->warps;
> if (delta < hv_test_info->worst){
> hv_test_info->worst = delta;
> - printf("Worst warp %lld %\n", hv_test_info->worst);
> + printf("Worst warp %lld\n", hv_test_info->worst);
Ah, nice, so this addition of __attribute__((format)) indeed revealed
also some real problematic format strings :-)
> }
> spin_unlock(&hv_test_info->lock);
> }
> @@ -102,8 +102,8 @@ static int cycle_test(int ncpus, int check, struct test_info *ti)
> printf("Total vcpus: %d\n", ncpus);
> printf("Test loops: %ld\n", loops);
> if (check == 1) {
> - printf("Total warps: %lld\n", ti->warps);
> - printf("Total stalls: %lld\n", ti->stalls);
> + printf("Total warps: %" PRId64 "\n", ti->warps);
> + printf("Total stalls: %" PRId64 "\n", ti->stalls);
> printf("Worst warp: %lld\n", ti->worst);
> } else
> printf("TSC cycles: %lld\n", end - begin);
> diff --git a/x86/msr.c b/x86/msr.c
> index ec4710ed478b9..50b13840ed946 100644
> --- a/x86/msr.c
> +++ b/x86/msr.c
> @@ -88,7 +88,7 @@ static void test_msr_rw(int msr_index, unsigned long long input, unsigned long l
> wrmsr(msr_index, input);
> r = rdmsr(msr_index);
> if (expected != r) {
> - printf("testing %s: output = 0x%x:0x%x expected = 0x%x:0x%x\n", sptr, r >> 32, r, expected >> 32, expected);
> + printf("testing %s: output = 0x%llx:0x%llx expected = 0x%llx:0x%llx\n", sptr, r >> 32, r, expected >> 32, expected);
That code looks wrong (before and after your change, but in different
ways ;-)):
r is long long, so let's assume its value is 0x1234567890ABCDEF. You
would then get the following output:
output = 0x12345678:0x1234567890ABCDEF
But the author likely intended:
output = 0x12345678:0x90ABCDEF
Same for the "expected" value.
So either the lower part has explicitely be casted to (uint32_t), or
simply print the whole value only once with llx, without the colon
inbetween.
> }
> report(sptr, expected == r);
> }
...
> diff --git a/x86/svm.c b/x86/svm.c
> index 1046ddf73732f..934b2ae91fa81 100644
> --- a/x86/svm.c
> +++ b/x86/svm.c
> @@ -934,9 +934,9 @@ static bool latency_finished(struct test *test)
>
> static bool latency_check(struct test *test)
> {
> - printf(" Latency VMRUN : max: %d min: %d avg: %d\n", latvmrun_max,
> + printf(" Latency VMRUN : max: %ld min: %ld avg: %ld\n", latvmrun_max,
> latvmrun_min, vmrun_sum / LATENCY_RUNS);
> - printf(" Latency VMEXIT: max: %d min: %d avg: %d\n", latvmexit_max,
> + printf(" Latency VMEXIT: max: %ld min: %ld avg: %ld\n", latvmexit_max,
> latvmexit_min, vmexit_sum / LATENCY_RUNS);
> return true;
> }
> @@ -998,13 +998,13 @@ static bool lat_svm_insn_finished(struct test *test)
>
> static bool lat_svm_insn_check(struct test *test)
> {
> - printf(" Latency VMLOAD: max: %d min: %d avg: %d\n", latvmload_max,
> + printf(" Latency VMLOAD: max: %ld min: %ld avg: %ld\n", latvmload_max,
> latvmload_min, vmload_sum / LATENCY_RUNS);
> - printf(" Latency VMSAVE: max: %d min: %d avg: %d\n", latvmsave_max,
> + printf(" Latency VMSAVE: max: %ld min: %ld avg: %ld\n", latvmsave_max,
> latvmsave_min, vmsave_sum / LATENCY_RUNS);
> - printf(" Latency STGI: max: %d min: %d avg: %d\n", latstgi_max,
> + printf(" Latency STGI: max: %ld min: %ld avg: %ld\n", latstgi_max,
> latstgi_min, stgi_sum / LATENCY_RUNS);
> - printf(" Latency CLGI: max: %d min: %d avg: %d\n", latclgi_max,
> + printf(" Latency CLGI: max: %ld min: %ld avg: %ld\n", latclgi_max,
> latclgi_min, clgi_sum / LATENCY_RUNS);
> return true;
> }
The latxxx_min/max variables seem to be declared with u64 ... so maybe
better use PRIu64 here instead?
> diff --git a/x86/taskswitch2.c b/x86/taskswitch2.c
> index db3e41aa82186..dbc9a2525d01a 100644
> --- a/x86/taskswitch2.c
> +++ b/x86/taskswitch2.c
> @@ -62,7 +62,7 @@ start:
>
> void do_pf_tss(ulong *error_code)
> {
> - printf("PF task is running %x %x\n", error_code, *(ulong*)error_code);
> + printf("PF task is running %p %lx\n", error_code, *(ulong*)error_code);
While you're at it, you could remove the (ulong*) typecast here, too.
> print_current_tss_info();
> if (*(ulong*)error_code == 0x2) /* write access, not present */
> test_count++;
...
> diff --git a/x86/vmx.c b/x86/vmx.c
> index 3fa1a735881f7..e047a59ab1129 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -144,17 +144,17 @@ void print_vmexit_info()
> guest_rip = vmcs_read(GUEST_RIP);
> guest_rsp = vmcs_read(GUEST_RSP);
> printf("VMEXIT info:\n");
> - printf("\tvmexit reason = %d\n", reason);
> - printf("\texit qualification = 0x%x\n", exit_qual);
> - printf("\tBit 31 of reason = %x\n", (vmcs_read(EXI_REASON) >> 31) & 1);
> - printf("\tguest_rip = 0x%llx\n", guest_rip);
> - printf("\tRAX=0x%llx RBX=0x%llx RCX=0x%llx RDX=0x%llx\n",
> + printf("\tvmexit reason = %ld\n", reason);
> + printf("\texit qualification = 0x%lx\n", exit_qual);
> + printf("\tBit 31 of reason = %lx\n", (vmcs_read(EXI_REASON) >> 31) & 1);
> + printf("\tguest_rip = 0x%lx\n", guest_rip);
> + printf("\tRAX=0x%lx RBX=0x%lx RCX=0x%lx RDX=0x%lx\n",
> regs.rax, regs.rbx, regs.rcx, regs.rdx);
> - printf("\tRSP=0x%llx RBP=0x%llx RSI=0x%llx RDI=0x%llx\n",
> + printf("\tRSP=0x%lx RBP=0x%lx RSI=0x%lx RDI=0x%lx\n",
> guest_rsp, regs.rbp, regs.rsi, regs.rdi);
> - printf("\tR8 =0x%llx R9 =0x%llx R10=0x%llx R11=0x%llx\n",
> + printf("\tR8 =0x%lx R9 =0x%lx R10=0x%lx R11=0x%lx\n",
> regs.r8, regs.r9, regs.r10, regs.r11);
> - printf("\tR12=0x%llx R13=0x%llx R14=0x%llx R15=0x%llx\n",
> + printf("\tR12=0x%lx R13=0x%lx R14=0x%lx R15=0x%lx\n",
> regs.r12, regs.r13, regs.r14, regs.r15);
> }
The registers are declared as u64 ... so use PRIx64 for them instead?
> @@ -842,7 +842,7 @@ static int handle_hypercall()
> case HYPERCALL_VMEXIT:
> return VMX_TEST_VMEXIT;
> default:
> - printf("ERROR : Invalid hypercall number : %d\n", hypercall_no);
> + printf("ERROR : Invalid hypercall number : %ld\n", hypercall_no);
> }
> return VMX_TEST_EXIT;
> }
...
> diff --git a/x86/xsave.c b/x86/xsave.c
> index e471835b42fd9..52142d2cdb93b 100644
> --- a/x86/xsave.c
> +++ b/x86/xsave.c
> @@ -75,7 +75,7 @@ void test_xsave(void)
> printf("Legal instruction testing:\n");
>
> supported_xcr0 = get_supported_xcr0();
> - printf("Supported XCR0 bits: 0x%x\n", supported_xcr0);
> + printf("Supported XCR0 bits: 0x%lx\n", supported_xcr0);
Also PRIx64 here?
Thomas
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
