Marcelo Tosatti wrote:
On Sat, Jul 05, 2008 at 08:25:30PM +0300, Avi Kivity wrote:
@@ -1955,6 +1955,22 @@ void kvm_mmu_slot_remove_write_access(st
        }
 }
 +int kvm_mmu_slot_has_shadowed_page(struct kvm *kvm, int slot)
+{
+       struct kvm_mmu_page *sp;
+       int ret = 0;
+
+       spin_lock(&kvm->mmu_lock);
+       list_for_each_entry(sp, &kvm->arch.active_mmu_pages, link) {
+               if (test_bit(slot, &sp->slot_bitmap)) {
+                       ret = -EINVAL;
+                       break;
+               }
+       }
+       spin_unlock(&kvm->mmu_lock);
+       return ret;
+}
+
I don't like the guest influencing host actions in this way. It's just a guest.

But I think it's unneeded. kvm_mmu_zap_page() will mark a root shadow page invalid and force all vcpus to reload it, so all that's needed is to keep the mmu spinlock held while removing the slot.

You're still keeping a shadowed page around with sp->gfn pointing to
non-existant memslot. The code generally makes the assumption that
gfn_to_memslot(gfn) on shadowed info will not fail.

kvm_mmu_zap_page -> unaccount_shadowed, for example.


The page has already been zapped, so we might as well unaccount_shadowed() on the first run. It needs to be moved until after the reload_remote_mmus() call, though.

The other option is to harden gfn_to_memslot() callers to handle
failure, is that saner?

I don't think so.

--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

--
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

Reply via email to