On Tue, Mar 01, 2016 at 06:30:30AM +0100, Thomas Huth wrote:
> 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.
Uff, I guess I was in monkey mode at this point. I'll change it to the
last suggestion, no ':'.
>
> > }
> > 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?
svm unit tests are 64-bit only, so this should be OK.
>
> > 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.
Oh yeah, that's indeed pointless.
>
> > 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?
64-bit only file.
>
> > @@ -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?
Also 64-bit only
Thanks,
drew
--
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
