On Thu, 2 Aug 2012 16:56:28 -0300, Marcelo Tosatti <[email protected]> wrote:
> >
> > + case MSR_KVM_VCPU_STATE:
> > + vcpu->arch.v_state.vs_page = gfn_to_page(vcpu->kvm, data >>
> > PAGE_SHIFT);
> > + vcpu->arch.v_state.vs_offset = data & ~(PAGE_MASK |
> > KVM_MSR_ENABLED);
>
> Assign vs_offset after success.
>
> > +
> > + if (is_error_page(vcpu->arch.v_state.vs_page)) {
> > + kvm_release_page_clean(vcpu->arch.time_page);
> > + vcpu->arch.v_state.vs_page = NULL;
> > + pr_info("KVM: VCPU_STATE - Unable to pin the page\n");
>
> Missing break or return;
>
> > + }
> > + vcpu->arch.v_state.msr_val = data;
> > + break;
> > +
> > case MSR_IA32_MCG_CTL:
>
> Please verify this code carefully again.
>
> Also leaking the page reference.
>
> > vcpu->arch.apf.msr_val = 0;
> > vcpu->arch.st.msr_val = 0;
> > + vcpu->arch.v_state.msr_val = 0;
>
> Add a newline and comment (or even better a new helper).
> >
> > kvmclock_reset(vcpu);
>
How about something like the below. I have tried to look at time_page
for reference:
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 580abcf..c82cc12 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1604,6 +1604,16 @@ static void kvm_clear_vcpu_state(struct kvm_vcpu *vcpu)
kunmap_atomic(kaddr);
}
+static void kvm_vcpu_state_reset(struct kvm_vcpu *vcpu)
+{
+ vcpu->arch.v_state.msr_val = 0;
+ vcpu->arch.v_state.vs_offset = 0;
+ if (vcpu->arch.v_state.vs_page) {
+ kvm_release_page_dirty(vcpu->arch.v_state.vs_page);
+ vcpu->arch.v_state.vs_page = NULL;
+ }
+}
+
int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
{
bool pr = false;
@@ -1724,14 +1734,17 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr,
u64 data)
break;
case MSR_KVM_VCPU_STATE:
+ kvm_vcpu_state_reset(vcpu);
+
vcpu->arch.v_state.vs_page = gfn_to_page(vcpu->kvm, data >>
PAGE_SHIFT);
- vcpu->arch.v_state.vs_offset = data & ~(PAGE_MASK |
KVM_MSR_ENABLED);
if (is_error_page(vcpu->arch.v_state.vs_page)) {
- kvm_release_page_clean(vcpu->arch.time_page);
+ kvm_release_page_clean(vcpu->arch.v_state.vs_page);
vcpu->arch.v_state.vs_page = NULL;
pr_info("KVM: VCPU_STATE - Unable to pin the page\n");
+ break;
}
+ vcpu->arch.v_state.vs_offset = data & ~(PAGE_MASK |
KVM_MSR_ENABLED);
vcpu->arch.v_state.msr_val = data;
break;
@@ -6053,6 +6066,7 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
{
kvmclock_reset(vcpu);
+ kvm_vcpu_state_reset(vcpu);
free_cpumask_var(vcpu->arch.wbinvd_dirty_mask);
fx_free(vcpu);
@@ -6109,9 +6123,9 @@ int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu)
kvm_make_request(KVM_REQ_EVENT, vcpu);
vcpu->arch.apf.msr_val = 0;
vcpu->arch.st.msr_val = 0;
- vcpu->arch.v_state.msr_val = 0;
kvmclock_reset(vcpu);
+ kvm_vcpu_state_reset(vcpu);
kvm_clear_async_pf_completion_queue(vcpu);
kvm_async_pf_hash_reset(vcpu);
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
