On 13/03/2020 09:25, Jan Beulich wrote: > Plain (unsigned) integer division simply truncates the results. The > overall errors are smaller though if we use proper rounding. (Extend > this to the purely cosmetic aspect of time.c's freq_string(), which > before this change I've frequently observed to report e.g. NN.999MHz > HPET clock speeds.) > > Signed-off-by: Jan Beulich <jbeul...@suse.com> > --- > We could switch at least the first div/rem pair in calibrate_APIC_clock() > to use do_div(), but this would imply switching bus_freq (and then also > result) to unsigned int (as the divisor has to be 32-bit). While I think > there's pretty little risk for bus frequencies to go beyond 4GHz in the > next so many years, I still wasn't certain enough this would be a welcome > change.
Honestly, do_div() is very easy to get wrong (and in security relevant ways in Linux). I'd advocate for phasing its use out, irrespective of this frequency concern. As for 4GHz, I think its unlikely, but better to be safe in the code. > > --- a/xen/arch/x86/apic.c > +++ b/xen/arch/x86/apic.c > @@ -1261,7 +1261,9 @@ static int __init calibrate_APIC_clock(v > /* set up multipliers for accurate timer code */ > bus_freq = result*HZ; > bus_cycle = (u32) (1000000000000LL/bus_freq); /* in pico seconds */ > + bus_cycle += (1000000000000UL % bus_freq) * 2 > bus_freq; These two differ in signedness of the numerator. GCC seems to cope with combining the two into a single div instruction, but I think we should be consistent with the constant nevertheless. Otherwise, Acked-by: Andrew Cooper <andrew.coop...@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
