Page MenuHomeFreeBSD

Add/enable option to allow tsc as timecounter for VM
Needs ReviewPublic

Authored by lprylli_netflix.com on Mar 19 2019, 9:00 PM.

Details

Reviewers
gallatin
imp
jhb
Summary
Instead of systematically disabling use of tsc as timecounter for
VM, a new option is introduced to allow overriding default
behavior and still enable tsc. The major reason to previously
systematically disallow tsc as timecounter in the VM case is to
allow for VM migration, but in many contexts this is not a restriction.

Some systems rely on efficient and low granularity timecounter, even
when running as VM.  While timecounter choice can be overwritten after
boot, allowing it from kernel boot additionally allows to test the
full boot procedure in a VM with tsc timecounter which can be useful
to diagnose timecounter related issues during that time.

Tested: ran Freebsd native and under VM, checked tsc choices and
behavior.
Test Plan

tested with kernel running on baremetal or VM

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 23313
Build 22343: arc lint + arc unit

Event Timeline

jhb added a comment.Mar 19 2019, 9:10 PM

I'm curious which hypervisor you have tested this under and did you consider using one of the virtualized clocks if one was available? I know that there are some early patches to add kvmclock support to bhyve for example.

sys/x86/x86/tsc.c
57

For FreeBSD we probably want to leave the default setting as-is by setting this to zero. I would perhaps rename the knob/variable as well to be something like 'tsc_vm_enable' and 'hw.tsc.vm_enable'. Finally, I would maybe make this a CTLFLAG_RDTUN sysctl and call it 'machdep.tsc_vm_enable'. Then it is documented via sysctl -d and you can inspect the current value on existing systems, etc. The reason for machdep is that the other existing TSC sysctl is 'machdep.tsc_freq'.

Updating D19648: Add/enable option to allow tsc as timecounter for VM

  • change tunable to machdep.tsc_vm_enable
  • make it a sysctl
  • don't modify default (initial value must be zero).
In D19648#420593, @jhb wrote:

I'm curious which hypervisor you have tested this under and did you consider using one of the virtualized clocks if one was available? I know that there are some early patches to add kvmclock support to bhyve for example.

I have ran experiments with hypervizor either linux/qemu/kvm or freebsd/bhyve and had the same behavior for both. kvmclock could solve the issue in long-term, but it seems some additional parts would be needed in both bhyve and in freebsd kernel (as guest).

I updated the patch. But if you have a pointer to kvmclock patches, I'll be glad to try them.

jhb accepted this revision.Mar 26 2019, 4:24 PM

Tiny style nits can be fixed when this is committed.

sys/x86/x86/tsc.c
89

I would move tsc_vm_enable declaration here and just leave off the ' = 0' (see the tsc_skip_calibration node above for an example of this style).

125

style(9) would want a single tab indent, a blank line before the indent, and parentheses around the return value.

This revision is now accepted and ready to land.Mar 26 2019, 4:24 PM
This revision now requires review to proceed.Mar 26 2019, 10:02 PM
lprylli_netflix.com marked 3 inline comments as done.Mar 26 2019, 10:06 PM

Just pushed style fixes.