Skip to content

Rdtsc in preload mode#1515

Merged
sporksmith merged 9 commits intoshadow:mainfrom
sporksmith:rdtsc-preload
Jul 16, 2021
Merged

Rdtsc in preload mode#1515
sporksmith merged 9 commits intoshadow:mainfrom
sporksmith:rdtsc-preload

Conversation

@sporksmith
Copy link
Copy Markdown
Contributor

@sporksmith sporksmith commented Jul 14, 2021 •
edited
Loading

  • Improve rdtsc emulation to get clock speed via cpuid instead of non-deterministic measurement.
  • Move rdtsc emulation into common library, where it can also be used by shim
  • Emulate rdtsc in the shim when running in preload mode

Progress on #666

@github-actions github-actions bot added the Component: Main Composing the core Shadow executable label Jul 14, 2021
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 14, 2021 •
edited
Loading

Codecov Report

Merging #1515 (cdb2cbb) into main (65c17ed) will decrease coverage by 0.32%.
The diff coverage is 21.39%.

❗ Current head cdb2cbb differs from pull request most recent head e98b7fa. Consider uploading reports for the commit e98b7fa to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1515      +/-   ##
==========================================
- Coverage   53.00%   52.68%   -0.33%     
==========================================
  Files         140      140              
  Lines       21007    21159     +152     
  Branches     5314     5352      +38     
==========================================
+ Hits        11135    11147      +12     
- Misses       6938     7070     +132     
- Partials     2934     2942       +8
Flag Coverage Δ
tests 52.68% <21.39%> (-0.33%) ⬇️

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

Impacted Files Coverage Δ
src/lib/shim/shim.c 27.10% <0.00%> (-7.29%) ⬇️
src/lib/tsc/tsc.h 33.33% <0.00%> (ø)
src/main/host/syscall/signal.c 29.53% <0.00%> (-1.24%) ⬇️
src/lib/tsc/tsc.c 13.48% <13.48%> (ø)
src/lib/tsc/tsc_test.c 36.36% <36.36%> (ø)
src/main/host/thread_ptrace.c 50.16% <100.00%> (+0.83%) ⬆️
...main/core/scheduler/scheduler_policy_host_single.c 75.00% <0.00%> (-0.68%) ⬇️
src/main/core/manager.c 70.90% <0.00%> (-0.44%) ⬇️
... and 11 more

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 65c17ed...e98b7fa. Read the comment docs.

@github-actions github-actions bot added Component: Build Build/install tools and dependencies Component: Libraries Support functions like LD_PRELOAD and logging labels Jul 15, 2021
@sporksmith sporksmith changed the title Rdtsc preload Rdtsc in preload mode Jul 15, 2021
@github-actions github-actions bot added the Component: Testing Unit and integration tests and frameworks label Jul 15, 2021
@github-actions github-actions bot removed the Component: Testing Unit and integration tests and frameworks label Jul 15, 2021
@sporksmith sporksmith marked this pull request as ready for review July 15, 2021 16:39
@sporksmith sporksmith requested a review from stevenengler July 15, 2021 16:44
@sporksmith
Copy link
Copy Markdown
Contributor Author

sporksmith commented Jul 15, 2021

Probably easiest to review one commit at a time - or at least before and after moving the tsc module in 55e1dc5

@sporksmith sporksmith mentioned this pull request Jul 15, 2021
Closed
7 tasks
stevenengler
stevenengler approved these changes Jul 16, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@stevenengler stevenengler left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good! I tried to look for any simple errors, but didn't go in-depth (didn't double check things with the Intel manual or anything).

It seems like using cpuid(0x15, ...) to get the clock frequency probably will work on any newer x86 CPUs, but I'm guessing the brand string method will fail on non-Intel x86 CPUs? Have you checked with any AMD documentation to see what they say about getting the clock frequency? Do we have any systems with AMD CPUs we can test this on?

@sporksmith
Copy link
Copy Markdown
Contributor Author

sporksmith commented Jul 16, 2021

Good question. AFAICT from https://www.amd.com/system/files/TechDocs/25481.pdf, AMD doesn't support cpuid 0x15. It does implement the same API for getting the brand string, but unlike Intel doesn't specify that the speed will be the last token.

I don't have any AMD systems handy to check on; I guess we'll cross that bridge when we get there.

@sporksmith sporksmith enabled auto-merge July 16, 2021 18:34
@sporksmith sporksmith merged commit 7150c55 into shadow:main Jul 16, 2021
@jtracey jtracey mentioned this pull request Jul 16, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stevenengler stevenengler stevenengler approved these changes

Assignees

@sporksmith sporksmith

Labels

Component: Build Build/install tools and dependencies Component: Libraries Support functions like LD_PRELOAD and logging Component: Main Composing the core Shadow executable

Projects

None yet

Milestone

Fully support process-based simulatio...

Development

Successfully merging this pull request may close these issues.

2 participants

@sporksmith @stevenengler