Page MenuHomeFreeBSD

paravirt: Generalize parts of the XEN timer code into pvclock
ClosedPublic

Authored by bryanv on Jan 3 2015, 11:51 PM.
Tags
None
Referenced Files
F80148635: D1429.diff
Thu, Mar 28, 1:57 PM
Unknown Object (File)
Tue, Mar 12, 5:12 AM
Unknown Object (File)
Tue, Mar 12, 5:12 AM
Unknown Object (File)
Tue, Mar 12, 5:12 AM
Unknown Object (File)
Tue, Mar 12, 5:00 AM
Unknown Object (File)
Jan 22 2024, 11:20 AM
Unknown Object (File)
Jan 13 2024, 11:31 PM
Unknown Object (File)
Jan 12 2024, 10:44 AM
Subscribers

Details

Summary

This will later be used to support KVM clock. I'm not sure if there is a good way to sort out generic upstream Xen headers to include pvclock.h, so at the moment, casts are used to keep the code compiling.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

bryanv retitled this revision from to Generalized parts of the XEN timer code into pvclock.
bryanv updated this object.
bryanv edited the test plan for this revision. (Show Details)
bryanv added a reviewer: royger.
bryanv set the repository for this revision to rS FreeBSD src repository - subversion.
bryanv retitled this revision from Generalized parts of the XEN timer code into pvclock to paravirt: Generalize parts of the XEN timer code into pvclock.Jan 4 2015, 5:58 AM

Thanks for doing this, the change looks fine, but could you wait until Friday so I can test it on Xen before committing it?

sys/x86/include/pvclock.h
50

You don't need the __packed here or above, the layout of those structures already make them packed.

sys/x86/x86/pvclock.c
130

I think this change (from the previous code found in the Xen timer) fixes the theoretical issue described here: https://lists.freebsd.org/pipermail/freebsd-virtualization/2014-January/001926.html

In D1429#6, @royger wrote:

Thanks for doing this, the change looks fine, but could you wait until Friday so I can test it on Xen before committing it?

Absolutely. Thanks for reviewing.

royger edited edge metadata.

I've successfully tested this on a Xen PVHVM guest while performing migrations and it works fine, also tested on Dom0 with the same result.

sys/x86/x86/pvclock.c
153

This doesn't need to be a volatile AFAICT.

This revision is now accepted and ready to land.Jan 9 2015, 11:02 AM
sys/x86/x86/pvclock.c
130

Cool. Bonus fix :)

153

Removed.

rstone added a subscriber: rstone.
rstone added inline comments.
sys/x86/x86/pvclock.c
121

Doesn't this need to happen underneath a critical_enter()? If we switch to a different (virtual) CPU in the loop we could potentially see data for one (physical) CPU but read the TSC from a different CPU.

130

I don't see how this code prevents reading from ti->version twice in the condition?

sys/x86/x86/pvclock.c
121

scratch that, the caller already does the critical_enter() (and it has to be the one to do it).

I think that a CRITICAL_ASSERT(); would be a good idea where to document this important prerequisite.

bryanv updated this revision to Diff 3627.

Closed by commit rS278183 (authored by @bryanv).