Re: [Patch v1] Bug fix - Integer overflow when cpu frequency > u32 max value.

[Elliott Mitchell <ehem+xen@xxxxxxx>]

On Sat, Dec 17, 2022 at 06:42:05PM +0100, Neowutran wrote:
> xen/arch/x86/time.c: Bug fix - Integer overflow when cpu frequency > u32 max 
> value.
> 
> What is was trying to do: I was trying to install QubesOS on my new computer
> (AMD zen4 processor). Guest VM were unusably slow / unusable.
> 
> What is the issue: The cpu frequency reported is wrong for linux guest in HVM
> and PVH mode, and it cause issue with the TSC clocksource (for example).
> 
> Why this patch solved my issue:
> The root cause it that "d->arch.tsc_khz" is a unsigned integer storing
> the cpu frequency in khz. It get multiplied by 1000, so if the cpu frequency
> is over ~4,294 Mhz (u32 max value), then it overflow.
> I am solving the issue by adding an explicit cast to u64 to avoid the 
> overflow.

https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches

You're not matching the instructions.  You need to add a "Signed-off-by"
to indicate *you* wrote this, own the Copyright and are providing this
under the license of the existing files.

Your title line is on the long side.  The discussion is useful for the
message on the xen-devel mailing list, but isn't important for source
code history.  You should put "---" after the commit message and 
discussion below.

I would suggest as a commit message:

--------8<-----------------------------------------------------8<--------
xen/x86: prevent overflow with high frequency TSCs

Promote tsc_khz to a 64-bit type before multiplying by 1000.  Otherwise
just above 4.294GHz the value will overflow.  Processors with clocks
this high are now in production and require this to work correctly.
--------8<-----------------------------------------------------8<--------

Feel free to disagree, though any maintainer is going to want adjustments
to your original commit message.


Note to Xen and Linux distribution maintainers: I suggest this needs a
point release of Xen.  A large processor manufacturer has recently
released such a processor.  A great number of people are going to be
rather unhappy in short order without this fix.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@xxxxxxx  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445