Re: [Qemu-devel] target-i386: Allow tsc-frequency to be larger then 2.147G

[Michael Tokarev <mjt@tls.msk.ru>]

This is a follow-up to a more-or-less trivial commit,
2e84849aa2cc7f220d3b3668f5f7e3c57bb1b590 .  I'm adding
some more context - the whole function in question.


commit 2e84849aa2cc7f220d3b3668f5f7e3c57bb1b590
Author: Don Slutz <Don@CloudSwitch.com>
Date:   Fri Sep 21 20:13:13 2012 -0400

    target-i386: Allow tsc-frequency to be larger then 2.147G

    The check using INT_MAX (2147483647) is wrong in this case.

    Signed-off-by: Fred Oliveira <foliveira@cloudswitch.com>
    Signed-off-by: Don Slutz <Don@CloudSwitch.com>
    Signed-off-by: Stefan Hajnoczi <stefanha@gmail.com>

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 423e009..cbc172e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -846,7 +846,7 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
 {
     X86CPU *cpu = X86_CPU(obj);
     const int64_t min = 0;
-    const int64_t max = INT_MAX;
+    const int64_t max = INT64_MAX;
     int64_t value;

     visit_type_int(v, &value, name, errp);
     if (error_is_set(errp)) {
         return;
     }
     if (value < min || value > max) {
         error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
                   name ? name : "null", value, min, max);
         return;
     }

     cpu->env.tsc_khz = value / 1000;
 }

The patch makes the second test (if value > max)
to be a no-op, since value is of type int64_t,
and max is now INT64_MAX, so value can never be
larger than max.  Overflow can be catched by the
first test (value < 0).

Note this function has another defect: the tsc
frequency is truncated to KHz.  It's okay when
it is called from the default cpu init function,
where the initial value is in khz and is multiplied
by 1000 when calling x86_cpuid_set_tsc_freq(),
but not okay when called as a handler for user-
defined option, like -cpu foo,tsc_frequency=bar.

I'm not sure how often this option is used, however.

Thanks,

/mjt