On 16/03/2020 08:58, Jan Beulich wrote: > On 13.03.2020 16:14, Andrew Cooper wrote: >> 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. > > IOW you'd like me to change the other line too, to have a UL > suffix? If so, at that point I'd drop the stray cast, too.
That would be fine yes. ~Andrew > >> Otherwise, Acked-by: Andrew Cooper <andrew.coop...@citrix.com> > > Thanks, but please let me know if the above is a correct > understanding of mine. > > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel