Il 08/10/2013 19:08, Alex Bligh ha scritto: > > On 8 Oct 2013, at 17:56, Paolo Bonzini wrote: > >>> Arguably the patch could document why removing the check for deadline > >>> INT32_MAX >>> (the bug for bug compatibility) is safe, as I couldn't entirely convince >>> myself it >>> was, mostly because I couldn't see why it was doing it in the first place. >> >> I couldn't convince myself that it is _not_ safe :) and it made the code >> more complicated. As soon as a deadline appears, qemu_clock_warp() will >> be called again and update the icount_warp_timer. >> >> Ok to move that to a separate patch? > > To be honest I put it in out of an abundance of caution. I am very > tempted to take it out and see what breaks. As far as I can see all > the time arithmetic is not int64_t; perhaps this was not always the > case. I was more checking you hadn't removed it by accident. Perhaps > just add "special casing deadlines > INT32_MAX removed as all > arithmetic now 64 bit". > > There is another offender in tcg_cpu_exec. > > deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL); > > /* Maintain prior (possibly buggy) behaviour where if no deadline > * was set (as there is no QEMU_CLOCK_VIRTUAL timer) or it is more > than > * INT32_MAX nanoseconds ahead, we still use INT32_MAX > * nanoseconds. > */ > if ((deadline < 0) || (deadline > INT32_MAX)) { > deadline = INT32_MAX; > } > > count = qemu_icount_round(deadline); > qemu_icount += count; > decr = (count > 0xffff) ? 0xffff : count; > count -= decr; > env->icount_decr.u16.low = decr; > env->icount_extra = count; > > This implies that qemu_icount_round() cannot take a 64 bit int. > > static int64_t qemu_icount_round(int64_t count) > { > return (count + (1 << icount_time_shift) - 1) >> icount_time_shift; > } > > I would have thought it better if qemu_icount_round just > dealt sensibly with negative parameters. >
I'll clean that up separately. Thanks, Paolo