Re: [PATCH 5/6] KVM: PPC: BOOK3S: Use hpte_update_in_progress to track invalid hpte during an hpte update
From: Paul Mackerras <hidden>
Date: 2014-07-02 05:43:57
Also in:
kvm
On Sun, Jun 29, 2014 at 04:47:34PM +0530, Aneesh Kumar K.V wrote:
As per ISA, we first need to mark hpte invalid (V=0) before we update the hpte lower half bits. With virtual page class key protection mechanism we want to send any fault other than key fault to guest directly without searching the hash page table. But then we can get NO_HPTE fault while we are updating the hpte. To track that add a vm specific atomic variable that we check in the fault path to always send the fault to host.
[...]
quoted hunk ↗ jump to hunk
@@ -750,13 +751,15 @@ int kvmppc_book3s_hv_page_fault(struct kvm_run *run, struct kvm_vcpu *vcpu, r &= rcbits | ~(HPTE_R_R | HPTE_R_C); if (be64_to_cpu(hptep[0]) & HPTE_V_VALID) { - /* HPTE was previously valid, so we need to invalidate it */ + /* + * If we had mapped this hpte before, we now need to + * invalidate that. + */ unlock_rmap(rmap); - /* Always mark HPTE_V_ABSENT before invalidating */ - kvmppc_unmap_host_hpte(kvm, hptep); kvmppc_invalidate_hpte(kvm, hptep, index); /* don't lose previous R and C bits */ r |= be64_to_cpu(hptep[1]) & (HPTE_R_R | HPTE_R_C); + hpte_invalidated = true;
So now we're not setting the ABSENT bit before invalidating the HPTE. That means that another guest vcpu could do an H_ENTER which could think that this HPTE is free and use it for another unrelated guest HPTE, which would be bad...
quoted hunk ↗ jump to hunk
@@ -1144,8 +1149,8 @@ static int kvm_test_clear_dirty_npages(struct kvm *kvm, unsigned long *rmapp) npages_dirty = n; eieio(); } - kvmppc_map_host_hpte(kvm, &v, &r); - hptep[0] = cpu_to_be64(v & ~HPTE_V_HVLOCK); + hptep[0] = cpu_to_be64(v & ~HPTE_V_LOCK); + atomic_dec(&kvm->arch.hpte_update_in_progress);
Why are we using LOCK rather than HVLOCK now? (And why didn't you mention this change and its rationale in the patch description?) Paul.