Skip to content

TSC: Use CPU model map to find crystal frequency#1524

Merged
sporksmith merged 1 commit intoshadow:mainfrom
sporksmith:tsc-cpuid15
Jul 19, 2021
Merged

TSC: Use CPU model map to find crystal frequency#1524
sporksmith merged 1 commit intoshadow:mainfrom
sporksmith:tsc-cpuid15

Conversation

@sporksmith
Copy link
Copy Markdown
Contributor

In #1519 we found that the
frequency from the CPU brand string can be substantially off from the
TSC rate (3% in this case), so it's worth implementing more of the cpuid
0x15h method of finding the TSC rate.

@github-actions github-actions bot added the Component: Libraries Support functions like LD_PRELOAD and logging label Jul 19, 2021
In shadow#1519 we found that the
frequency from the CPU brand string can be substantially off from the
TSC rate (3% in this case), so it's worth implementing more of the cpuid
0x15h method of finding the TSC rate.
@sporksmith sporksmith added the Type: Bug Error or flaw producing unexpected results label Jul 19, 2021
@sporksmith sporksmith requested a review from stevenengler July 19, 2021 16:33
@sporksmith sporksmith enabled auto-merge (squash) July 19, 2021 16:33
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 19, 2021

Codecov Report

Merging #1524 (e366bef) into main (fc7518b) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1524      +/-   ##
==========================================
- Coverage   52.60%   52.57%   -0.03%     
==========================================
  Files         141      141              
  Lines       21158    21169      +11     
  Branches     5345     5347       +2     
==========================================
  Hits        11130    11130              
- Misses       7084     7096      +12     
+ Partials     2944     2943       -1     
Flag Coverage Δ
tests 52.57% <0.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/lib/tsc/tsc.c 12.00% <0.00%> (-1.49%) ⬇️
src/lib/logger/log_level.c 13.63% <0.00%> (-4.55%) ⬇️
src/main/host/descriptor/timer.c 72.12% <0.00%> (-1.22%) ⬇️
src/lib/tsc/tsc_test.c 30.37% <0.00%> (ø)
src/main/host/thread_ptrace.c 50.32% <0.00%> (+0.16%) ⬆️
src/main/host/network_interface.c 72.75% <0.00%> (+0.28%) ⬆️
src/main/host/descriptor/descriptor.c 75.00% <0.00%> (+0.73%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc7518b...e366bef. Read the comment docs.

Copy link
Copy Markdown
Contributor

@stevenengler stevenengler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may have seen this already, but Linux seems to use cpuid 0x16 as well for certain processors. Maybe it would help here too: https://lore.kernel.org/patchwork/patch/1071759/

@sporksmith sporksmith merged commit f228456 into shadow:main Jul 19, 2021
@sporksmith
Copy link
Copy Markdown
Contributor Author

You may have seen this already, but Linux seems to use cpuid 0x16 as well for certain processors. Maybe it would help here too: https://lore.kernel.org/patchwork/patch/1071759/

I hadn't!

I was poking around some more though and found that the current methods don't look like they'll work for AMD, and haven't been able to find a method that will reliably work - https://community.amd.com/t5/server-gurus-discussions/how-can-one-find-quot-p0-frequency-quot/td-p/292863

Also came across this article, which basically recommends getting it from the kernel (via a custom module because it isn't exposed by default): https://blog.trailofbits.com/2019/10/03/tsc-frequency-for-all-better-profiling-and-benchmarking/.

Given the complexity of finding the TSC rate, and that the only place we've observed to use the TSC to measure wallclock time is the vdso function (probably because of how complex it is), maybe we're better off just picking a simulated TSC rate and working on more thoroughly overriding vdso.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Libraries Support functions like LD_PRELOAD and logging Type: Bug Error or flaw producing unexpected results

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants