From: "Fernando Luis Vázquez Cao" <fernando_b1@lab.ntt.co.jp>
To: Paolo Bonzini <pbonzini@redhat.com>,
Marcelo Tosatti <mtosatti@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>, Will Auld <will.auld@intel.com>,
qemu-devel@nongnu.org, kvm@vger.kernel.org
Subject: Re: [PATCH] target-i386: clear guest TSC on reset
Date: Fri, 06 Dec 2013 17:24:18 +0900 [thread overview]
Message-ID: <52A189B2.4060305@lab.ntt.co.jp> (raw)
In-Reply-To: <52A0AC09.4090202@redhat.com>
On 12/06/2013 01:38 AM, Paolo Bonzini wrote:
> Il 05/12/2013 17:17, Marcelo Tosatti ha scritto:
>>>> I agree it is a bit ugly, but in my testing QEMU seemed to loop over all
>>>> the VCPUS fast enough for the kernel side kvm_write_tsc() to do a
>>>> reasonable job of matching the offsets (the Linux guest did not mark
>>>> the TSC unstable due to the TSCs being unsynchronized). Am I missing
>>>> something?
>> Right, modern kernels (see kvm_write_tsc) perform synchronization, so in
>> theory the "KVM is yet unable to synchronize ..." code is not necessary
>> anymore.
>>
>> I vote for dropping the thing entirely.
When I was writing the original patch I was tempted to do that,
but I feared that it could break older kernels that do not have
TSC synchronization code. Should we care about such uses
(recent QEMU user space + old kernel)?
I also wanted to make sure that the initialization that we do
in kvm_arch_vcpu_postcreate on power up and the subsequent
TSC writeback work well together, but I didn't have time to
test it (reading the code, I would say that the TSC generation
counter may end up being increased a few times but the TSCs
would eventually converge).
> If it can be dropped entirely, I certainly have no problem with starting
> with a simple patch first.
Could we start with the patch that I already sent? It's been
tested, it is conservative in the sense that it does the minimum
necessary to fix an existing bug, and should be easy to
backport. I will be replying to this email with an updated
version that has a more appropriate and less scary patch
description.
I will also be sending a patch that makes the TSC writeback
unconditional, but this one should probably be kept on hold
until it is properly tested.
As a follow-up effort we can work on Paolo's suggestions.
Is this an acceptable way forward?
Thanks,
Fernando
WARNING: multiple messages have this Message-ID (diff)
From: "Fernando Luis Vázquez Cao" <fernando_b1@lab.ntt.co.jp>
To: Paolo Bonzini <pbonzini@redhat.com>,
Marcelo Tosatti <mtosatti@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>, Will Auld <will.auld@intel.com>,
qemu-devel@nongnu.org, kvm@vger.kernel.org
Subject: Re: [Qemu-devel] [PATCH] target-i386: clear guest TSC on reset
Date: Fri, 06 Dec 2013 17:24:18 +0900 [thread overview]
Message-ID: <52A189B2.4060305@lab.ntt.co.jp> (raw)
In-Reply-To: <52A0AC09.4090202@redhat.com>
On 12/06/2013 01:38 AM, Paolo Bonzini wrote:
> Il 05/12/2013 17:17, Marcelo Tosatti ha scritto:
>>>> I agree it is a bit ugly, but in my testing QEMU seemed to loop over all
>>>> the VCPUS fast enough for the kernel side kvm_write_tsc() to do a
>>>> reasonable job of matching the offsets (the Linux guest did not mark
>>>> the TSC unstable due to the TSCs being unsynchronized). Am I missing
>>>> something?
>> Right, modern kernels (see kvm_write_tsc) perform synchronization, so in
>> theory the "KVM is yet unable to synchronize ..." code is not necessary
>> anymore.
>>
>> I vote for dropping the thing entirely.
When I was writing the original patch I was tempted to do that,
but I feared that it could break older kernels that do not have
TSC synchronization code. Should we care about such uses
(recent QEMU user space + old kernel)?
I also wanted to make sure that the initialization that we do
in kvm_arch_vcpu_postcreate on power up and the subsequent
TSC writeback work well together, but I didn't have time to
test it (reading the code, I would say that the TSC generation
counter may end up being increased a few times but the TSCs
would eventually converge).
> If it can be dropped entirely, I certainly have no problem with starting
> with a simple patch first.
Could we start with the patch that I already sent? It's been
tested, it is conservative in the sense that it does the minimum
necessary to fix an existing bug, and should be easy to
backport. I will be replying to this email with an updated
version that has a more appropriate and less scary patch
description.
I will also be sending a patch that makes the TSC writeback
unconditional, but this one should probably be kept on hold
until it is properly tested.
As a follow-up effort we can work on Paolo's suggestions.
Is this an acceptable way forward?
Thanks,
Fernando
next prev parent reply other threads:[~2013-12-06 8:24 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-03 7:08 [PATCH] kvm: clear guest TSC on reset Fernando Luis Vázquez Cao
2013-12-03 8:04 ` Fernando Luis Vázquez Cao
2013-12-05 6:08 ` Fernando Luis Vázquez Cao
2013-12-05 6:08 ` [Qemu-devel] " Fernando Luis Vázquez Cao
2013-12-05 6:15 ` [PATCH] target-i386: " Fernando Luis Vázquez Cao
2013-12-05 6:15 ` [Qemu-devel] " Fernando Luis Vázquez Cao
2013-12-05 9:28 ` Paolo Bonzini
2013-12-05 9:28 ` [Qemu-devel] " Paolo Bonzini
2013-12-05 13:15 ` Fernando Luis Vazquez Cao
2013-12-05 13:15 ` [Qemu-devel] " Fernando Luis Vazquez Cao
2013-12-05 13:53 ` Paolo Bonzini
2013-12-05 13:53 ` [Qemu-devel] " Paolo Bonzini
2013-12-05 15:42 ` Fernando Luis Vazquez Cao
2013-12-05 15:42 ` [Qemu-devel] " Fernando Luis Vazquez Cao
2013-12-05 16:02 ` Paolo Bonzini
2013-12-05 16:02 ` [Qemu-devel] " Paolo Bonzini
2013-12-05 16:40 ` Marcelo Tosatti
2013-12-05 16:40 ` [Qemu-devel] " Marcelo Tosatti
2013-12-05 17:06 ` Marcelo Tosatti
2013-12-05 17:06 ` [Qemu-devel] " Marcelo Tosatti
2013-12-05 16:17 ` Marcelo Tosatti
2013-12-05 16:17 ` [Qemu-devel] " Marcelo Tosatti
2013-12-05 16:38 ` Paolo Bonzini
2013-12-05 16:38 ` [Qemu-devel] " Paolo Bonzini
2013-12-06 8:24 ` Fernando Luis Vázquez Cao [this message]
2013-12-06 8:24 ` Fernando Luis Vázquez Cao
2013-12-06 8:33 ` [PATCH 1//2 v3] " Fernando Luis Vázquez Cao
2013-12-06 8:33 ` [Qemu-devel] " Fernando Luis Vázquez Cao
2013-12-06 8:38 ` [PATCH 2/2] target-i386: do not special case TSC writeback Fernando Luis Vázquez Cao
2013-12-06 8:38 ` [Qemu-devel] " Fernando Luis Vázquez Cao
2013-12-06 8:36 ` [PATCH] target-i386: clear guest TSC on reset Paolo Bonzini
2013-12-06 8:36 ` [Qemu-devel] " Paolo Bonzini
2013-12-06 8:56 ` Fernando Luis Vázquez Cao
2013-12-06 8:56 ` [Qemu-devel] " Fernando Luis Vázquez Cao
2013-12-06 9:08 ` Paolo Bonzini
2013-12-06 9:08 ` [Qemu-devel] " Paolo Bonzini
2013-12-06 9:20 ` Fernando Luis Vazquez Cao
2013-12-06 9:20 ` [Qemu-devel] " Fernando Luis Vazquez Cao
2013-12-06 14:22 ` Marcelo Tosatti
2013-12-06 14:22 ` [Qemu-devel] " Marcelo Tosatti
2013-12-09 8:50 ` Fernando Luis Vázquez Cao
2013-12-09 8:50 ` [Qemu-devel] " Fernando Luis Vázquez Cao
2013-12-12 2:52 ` Fernando Luis Vázquez Cao
2013-12-12 2:52 ` [Qemu-devel] " Fernando Luis Vázquez Cao
2013-12-12 12:18 ` Paolo Bonzini
2013-12-12 12:18 ` [Qemu-devel] " Paolo Bonzini
2013-12-05 16:12 ` Marcelo Tosatti
2013-12-05 16:12 ` [Qemu-devel] " Marcelo Tosatti
2013-12-05 16:32 ` Paolo Bonzini
2013-12-05 16:32 ` [Qemu-devel] " Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52A189B2.4060305@lab.ntt.co.jp \
--to=fernando_b1@lab.ntt.co.jp \
--cc=gleb@kernel.org \
--cc=kvm@vger.kernel.org \
--cc=mtosatti@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=will.auld@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.