Re: [libvirt] [RFC PATCH] Add support for invtsc timer
On Tue, May 06, 2014 at 03:50:48PM +0100, Daniel P. Berrange wrote:
On Tue, May 06, 2014 at 03:27:20PM +0200, Ján Tomko wrote:
> Not yet merged in upstream QEMU:
> https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg05024.html
>
> Add support for invariant TSC timer running at constant rate in
> all ACPI P-, C- and T-states.
What does this do at the hardware level ? This doesn't seem to be
creating a second TSC timer source in the guest, rather it is
setting a property related to the existing TSC timer. So I think
it might make more sense as an attribute for <timer name='tsc'>
element instead.
>
> It can be enabled by specifying:
> <clock>
> <timer name='invtsc' present='yes'/>
> </clock>
> in the domain XML.
>
> Migration and saving the domain does not work with this timer.
Why is that ? The QEMU patch doesn't mention this restriction.
See http://marc.info/?l=qemu-devel&m=139828711719342&w=2
>
> The support for this timer is indicated by bit 8 of EDX after calling
> CPUID with 0x80000007. It does not show up in /proc/cpuinfo [1]
> and since we're calling qemu without 'enforce', it doesn't error
> out if the host doesn't support this.
libvirt really needs to address this bug (lack of "enforce" flag). It
was not so serious a few years ago, but now we have lots of features
that depend on support on KVM kernel code. And now we have this feature
that is unlikely to be included in a CPU model by default and likely to
be configured explicitly, so existing "CPU level" or family/model checks
won't be enough.
QEMU now has the "filtered-features" X86CPU property that can be used by
libvirt to emulate "enforce" behavior with appropriate error reporting.
Maybe I'm mis-interpreting the kernel source, but my reading of
that was that this *does* show up in /proc/cpuinfo, but it is
given the name 'constant_tsc' instead of 'invtsc'.
On my host I see 'constant_tsc' and 'nonstop_tsc' in /proc/cpuinfo
invtsc itself doesn't show up on /proc/cpuinfo directly, though.
nonstop_tsc and constant-tsc are Linux-specific CPU capability flags
that are set because invtsc is present. I find it confusing, I don't
know why Linux doesn't simply show the feature directly like it does for
all other feature flag bits.
> Alternatively, we could expose it in libvirt as a cpu flag:
> <cpu mode='custom' match='exact'>
> <model fallback='forbid'>qemu64</model>
> <feature policy='require' name='invtsc'/>
> </cpu>
> or maybe add +invtsc to qemu args when the 'nonstop_tsc' flag is
> requested?
Yep, I could see that as a valid option. If it is visible
in /proc/cpuinfo, then I think that's a compelling reason for
libvirt to model it as a CPU flag too, rather than pretend it
is a new type of timer when it is just an attribute of the
base TSC timer.
Even if the guest doesn't show it on /proc/cpuinfo, it is a CPUID
feature flag, so this approach seems valid to me.
> [1]:
>
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch...
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
Eduardo