Marcelo Tosatti wrote:
> Hi Avi,
>
> Good that you're back.
>
> On Sun, Mar 16, 2008 at 04:00:06PM +0200, Avi Kivity wrote:
>
>> Marcelo Tosatti wrote:
>>
>>> This patchset resends Anthony's QEMU balloon support plus:
>>>
>>> - Truncates the target size to ram size
>>> - Enables madvise() conditioned on KVM_ZAP_GFN ioctl
>>>
>>>
>>>
>> Once mmu notifiers are in, KVM_ZAP_GFN isn't needed. So we have three
>> possible situations:
>>
>> - zap needed, but not available: don't madvise()
>> - zap needed and available: zap and madvise()
>> - zap unneeded: madvise()
>>
>
> Right. That is what the patch does. You just have to fill in
> "have_mmu_notifiers" here:
>
> +int kvm_zap_single_gfn(struct kvm *kvm, gfn_t gfn)
> +{
> + unsigned long addr;
> + int have_mmu_notifiers = 0;
> +
> + down_read(&kvm->slots_lock);
> + addr = gfn_to_hva(kvm, gfn);
> +
> + if (kvm_is_error_hva(addr)) {
> + up_read(&kvm->slots_lock);
> + return -EINVAL;
> + }
> +
> + if (!have_mmu_notifiers) {
> + spin_lock(&kvm->mmu_lock);
> + rmap_nuke(kvm, gfn);
> + spin_unlock(&kvm->mmu_lock);
> + }
> + up_read(&kvm->slots_lock);
>
> So as to avoid rmap_nuke, since that will be done through the madvise()
> path.
>
>
Why not do it in userspace?
I'll likely merge zap directly into the backwards compatibility support
(it won't ever appear in mainline since mmu notifiers will be merged
sooner).
>> Did you find out what's causing the errors in the first place (if zap is
>> not used)? It worries me greatly.
>>
>
> Yes, the problem is that the rmap code does not handle the qemu process
> mappings from vanishing while there is a present rmap. If that happens,
> and there is a fault for a gfn whose qemu mapping has been removed, a
> different physical zero page will be allocated:
>
> rmap a -> gfn 0 -> physical host page 0
> mapping for gfn 0 gets removed
> guest faults in gfn 0 through the same pte "chain"
> rmap a -> gfn 0 -> physical host page 1
>
> When instantiating the shadow mapping for the second time, the
> "is_rmap_pte" check succeeds, so we release the reference grabbed by
> gfn_to_page() at mmu_set_spte(). We now have a shadow mapping pointing
> to a physical page without having an additional reference on that page.
>
> The following makes the host not crash under such a condition, but the
> condition itself is invalid leading to inconsistent state on the guest.
> So IMHO it shouldnt be allowed to happen in the first place.
>
>
The only way to prevent it is with mmu notifiers, which we may not have
for 2.6.25. So I'd like to send this for 2.6.25-rc.
Please add a signoff.
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index f0cdfba..4c93b79 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -1009,6 +1009,21 @@ struct page *gva_to_page(struct kvm_vcpu *vcpu, gva_t
> +gva)
> return page;
> }
>
> +static int was_spte_rmapped(struct kvm *kvm, u64 *spte, struct page *page)
> +{
> + int ret = 0;
> + unsigned long host_pfn = (*spte & PT64_BASE_ADDR_MASK) >> PAGE_SHIFT;
>
hfn_t hfn
> +
> + if (is_rmap_pte(*spte)) {
> + if (host_pfn != page_to_pfn(page))
> + rmap_remove(kvm, spte);
> + else
> + ret = 1;
> + }
> +
> + return ret;
> +}
> +
> static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *shadow_pte,
> unsigned pt_access, unsigned pte_access,
> int user_fault, int write_fault, int dirty,
> @@ -1016,7 +1031,7 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64
> +*shadow_pte,
> struct page *page)
> {
> u64 spte;
> - int was_rmapped = is_rmap_pte(*shadow_pte);
> + int was_rmapped = was_spte_rmapped(vcpu->kvm, shadow_pte, page);
> int was_writeble = is_writeble_pte(*shadow_pte);
>
Calling code with side effects in an initializer is a bit confusing. I
suggest simply inlining the code into mmu_set_spte().
--
error compiling committee.c: too many arguments to function
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel
