2011-02-01 19:18:27

by Glauber Costa

[permalink] [raw]

Subject: [PATCH v2] release kvmclock page on reset

---

When a vcpu is reset, kvmclock page keeps being writen to this days.
This is wrong and inconsistent: a cpu reset should take it to its
initial state.

Signed-off-by: Glauber Costa <[email protected]>
CC: Jan Kiszka <[email protected]>
---
arch/x86/kvm/x86.c | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index bcc0efc..c39ab4a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1453,6 +1453,14 @@ static int kvm_pv_enable_async_pf(struct kvm_vcpu *vcpu, u64 data)
return 0;
}

+static void kvmclock_reset(struct kvm_vcpu *vcpu)
+{
+ if (vcpu->arch.time_page) {
+ kvm_release_page_dirty(vcpu->arch.time_page);
+ vcpu->arch.time_page = NULL;
+ }
+}
+
int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
{
switch (msr) {
@@ -1510,10 +1518,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
break;
case MSR_KVM_SYSTEM_TIME_NEW:
case MSR_KVM_SYSTEM_TIME: {
- if (vcpu->arch.time_page) {
- kvm_release_page_dirty(vcpu->arch.time_page);
- vcpu->arch.time_page = NULL;
- }
+ kvmclock_reset(vcpu);

vcpu->arch.time = data;
kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
@@ -5814,10 +5819,7 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)

void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
{
- if (vcpu->arch.time_page) {
- kvm_release_page_dirty(vcpu->arch.time_page);
- vcpu->arch.time_page = NULL;
- }
+ kvmclock_reset(vcpu);

free_cpumask_var(vcpu->arch.wbinvd_dirty_mask);
fx_free(vcpu);
@@ -5878,6 +5880,8 @@ int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu)
kvm_make_request(KVM_REQ_EVENT, vcpu);
vcpu->arch.apf.msr_val = 0;

+ kvmclock_reset(vcpu);
+
kvm_clear_async_pf_completion_queue(vcpu);
kvm_async_pf_hash_reset(vcpu);
vcpu->arch.apf.halted = false;
--
1.7.2.3

2011-02-01 20:28:26

by Jan Kiszka

[permalink] [raw]

Subject: Re: [PATCH v2] release kvmclock page on reset

---

On 2011-02-01 20:16, Glauber Costa wrote:
> When a vcpu is reset, kvmclock page keeps being writen to this days.
> This is wrong and inconsistent: a cpu reset should take it to its
> initial state.
>
> Signed-off-by: Glauber Costa <[email protected]>
> CC: Jan Kiszka <[email protected]>
> ---
> arch/x86/kvm/x86.c | 20 ++++++++++++--------
> 1 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index bcc0efc..c39ab4a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1453,6 +1453,14 @@ static int kvm_pv_enable_async_pf(struct kvm_vcpu *vcpu, u64 data)
> return 0;
> }
>
> +static void kvmclock_reset(struct kvm_vcpu *vcpu)
> +{
> + if (vcpu->arch.time_page) {
> + kvm_release_page_dirty(vcpu->arch.time_page);
> + vcpu->arch.time_page = NULL;
> + }
> +}
> +
> int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> {
> switch (msr) {
> @@ -1510,10 +1518,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> break;
> case MSR_KVM_SYSTEM_TIME_NEW:
> case MSR_KVM_SYSTEM_TIME: {
> - if (vcpu->arch.time_page) {
> - kvm_release_page_dirty(vcpu->arch.time_page);
> - vcpu->arch.time_page = NULL;
> - }
> + kvmclock_reset(vcpu);
>
> vcpu->arch.time = data;
> kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
> @@ -5814,10 +5819,7 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
>
> void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
> {
> - if (vcpu->arch.time_page) {
> - kvm_release_page_dirty(vcpu->arch.time_page);
> - vcpu->arch.time_page = NULL;
> - }
> + kvmclock_reset(vcpu);
>
> free_cpumask_var(vcpu->arch.wbinvd_dirty_mask);
> fx_free(vcpu);
> @@ -5878,6 +5880,8 @@ int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu)
> kvm_make_request(KVM_REQ_EVENT, vcpu);
> vcpu->arch.apf.msr_val = 0;
>
> + kvmclock_reset(vcpu);
> +
> kvm_clear_async_pf_completion_queue(vcpu);
> kvm_async_pf_hash_reset(vcpu);
> vcpu->arch.apf.halted = false;

Looks good.

Thanks,
Jan

---

Attachments:

signature.asc (259.00 B)
OpenPGP digital signature

2011-02-03 15:37:27

by Marcelo Tosatti

[permalink] [raw]

Subject: Re: [PATCH v2] release kvmclock page on reset

---

On Tue, Feb 01, 2011 at 02:16:40PM -0500, Glauber Costa wrote:
> When a vcpu is reset, kvmclock page keeps being writen to this days.
> This is wrong and inconsistent: a cpu reset should take it to its
> initial state.
>
> Signed-off-by: Glauber Costa <[email protected]>
> CC: Jan Kiszka <[email protected]>
> ---
> arch/x86/kvm/x86.c | 20 ++++++++++++--------
> 1 files changed, 12 insertions(+), 8 deletions(-)

Applied, thanks.