From: Sean Christopherson <seanjc@google.com>
To: David Stevens <stevensd@chromium.org>
Cc: Marc Zyngier <maz@kernel.org>,
Paolo Bonzini <pbonzini@redhat.com>,
James Morse <james.morse@arm.com>,
Alexandru Elisei <alexandru.elisei@arm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Will Deacon <will@kernel.org>, Wanpeng Li <wanpengli@tencent.com>,
Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org,
kvm@vger.kernel.org
Subject: Re: [PATCH v4 4/4] KVM: mmu: remove over-aggressive warnings
Date: Wed, 13 Oct 2021 00:02:47 +0000 [thread overview]
Message-ID: <YWYiJy1Z7VZ0SxAd@google.com> (raw)
In-Reply-To: <20210929042908.1313874-5-stevensd@google.com>
On Wed, Sep 29, 2021, David Stevens wrote:
> From: David Stevens <stevensd@chromium.org>
>
> Remove two warnings that require ref counts for pages to be non-zero, as
> mapped pfns from follow_pfn may not have an initialized ref count.
>
> Signed-off-by: David Stevens <stevensd@chromium.org>
> ---
> arch/x86/kvm/mmu/mmu.c | 7 -------
> virt/kvm/kvm_main.c | 2 +-
> 2 files changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 5a1adcc9cfbc..3b469df63bcf 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -617,13 +617,6 @@ static int mmu_spte_clear_track_bits(struct kvm *kvm, u64 *sptep)
>
> pfn = spte_to_pfn(old_spte);
>
> - /*
> - * KVM does not hold the refcount of the page used by
> - * kvm mmu, before reclaiming the page, we should
> - * unmap it from mmu first.
> - */
> - WARN_ON(!kvm_is_reserved_pfn(pfn) && !page_count(pfn_to_page(pfn)));
Have you actually observed false positives with this WARN? I would expect anything
without a struct page to get filtered out by !kvm_is_reserved_pfn(pfn).
If you have observed false positives, I would strongly prefer we find a way to
keep the page_count() sanity check, it has proven very helpful in the past in
finding/debugging bugs during MMU development.
> -
> if (is_accessed_spte(old_spte))
> kvm_set_pfn_accessed(pfn);
>
WARNING: multiple messages have this Message-ID (diff)
From: Sean Christopherson <seanjc@google.com>
To: David Stevens <stevensd@chromium.org>
Cc: Wanpeng Li <wanpengli@tencent.com>,
kvm@vger.kernel.org, Marc Zyngier <maz@kernel.org>,
Joerg Roedel <joro@8bytes.org>,
linux-kernel@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
Will Deacon <will@kernel.org>,
kvmarm@lists.cs.columbia.edu,
linux-arm-kernel@lists.infradead.org,
Jim Mattson <jmattson@google.com>
Subject: Re: [PATCH v4 4/4] KVM: mmu: remove over-aggressive warnings
Date: Wed, 13 Oct 2021 00:02:47 +0000 [thread overview]
Message-ID: <YWYiJy1Z7VZ0SxAd@google.com> (raw)
In-Reply-To: <20210929042908.1313874-5-stevensd@google.com>
On Wed, Sep 29, 2021, David Stevens wrote:
> From: David Stevens <stevensd@chromium.org>
>
> Remove two warnings that require ref counts for pages to be non-zero, as
> mapped pfns from follow_pfn may not have an initialized ref count.
>
> Signed-off-by: David Stevens <stevensd@chromium.org>
> ---
> arch/x86/kvm/mmu/mmu.c | 7 -------
> virt/kvm/kvm_main.c | 2 +-
> 2 files changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 5a1adcc9cfbc..3b469df63bcf 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -617,13 +617,6 @@ static int mmu_spte_clear_track_bits(struct kvm *kvm, u64 *sptep)
>
> pfn = spte_to_pfn(old_spte);
>
> - /*
> - * KVM does not hold the refcount of the page used by
> - * kvm mmu, before reclaiming the page, we should
> - * unmap it from mmu first.
> - */
> - WARN_ON(!kvm_is_reserved_pfn(pfn) && !page_count(pfn_to_page(pfn)));
Have you actually observed false positives with this WARN? I would expect anything
without a struct page to get filtered out by !kvm_is_reserved_pfn(pfn).
If you have observed false positives, I would strongly prefer we find a way to
keep the page_count() sanity check, it has proven very helpful in the past in
finding/debugging bugs during MMU development.
> -
> if (is_accessed_spte(old_spte))
> kvm_set_pfn_accessed(pfn);
>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
WARNING: multiple messages have this Message-ID (diff)
From: Sean Christopherson <seanjc@google.com>
To: David Stevens <stevensd@chromium.org>
Cc: Marc Zyngier <maz@kernel.org>,
Paolo Bonzini <pbonzini@redhat.com>,
James Morse <james.morse@arm.com>,
Alexandru Elisei <alexandru.elisei@arm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Will Deacon <will@kernel.org>, Wanpeng Li <wanpengli@tencent.com>,
Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org,
kvm@vger.kernel.org
Subject: Re: [PATCH v4 4/4] KVM: mmu: remove over-aggressive warnings
Date: Wed, 13 Oct 2021 00:02:47 +0000 [thread overview]
Message-ID: <YWYiJy1Z7VZ0SxAd@google.com> (raw)
In-Reply-To: <20210929042908.1313874-5-stevensd@google.com>
On Wed, Sep 29, 2021, David Stevens wrote:
> From: David Stevens <stevensd@chromium.org>
>
> Remove two warnings that require ref counts for pages to be non-zero, as
> mapped pfns from follow_pfn may not have an initialized ref count.
>
> Signed-off-by: David Stevens <stevensd@chromium.org>
> ---
> arch/x86/kvm/mmu/mmu.c | 7 -------
> virt/kvm/kvm_main.c | 2 +-
> 2 files changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 5a1adcc9cfbc..3b469df63bcf 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -617,13 +617,6 @@ static int mmu_spte_clear_track_bits(struct kvm *kvm, u64 *sptep)
>
> pfn = spte_to_pfn(old_spte);
>
> - /*
> - * KVM does not hold the refcount of the page used by
> - * kvm mmu, before reclaiming the page, we should
> - * unmap it from mmu first.
> - */
> - WARN_ON(!kvm_is_reserved_pfn(pfn) && !page_count(pfn_to_page(pfn)));
Have you actually observed false positives with this WARN? I would expect anything
without a struct page to get filtered out by !kvm_is_reserved_pfn(pfn).
If you have observed false positives, I would strongly prefer we find a way to
keep the page_count() sanity check, it has proven very helpful in the past in
finding/debugging bugs during MMU development.
> -
> if (is_accessed_spte(old_spte))
> kvm_set_pfn_accessed(pfn);
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-10-13 0:02 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-29 4:29 [PATCH v4 0/4] KVM: allow mapping non-refcounted pages David Stevens
2021-09-29 4:29 ` David Stevens
2021-09-29 4:29 ` David Stevens
2021-09-29 4:29 ` [PATCH v4 1/4] KVM: mmu: introduce new gfn_to_pfn_page functions David Stevens
2021-09-29 4:29 ` David Stevens
2021-09-29 4:29 ` David Stevens
2021-09-29 4:29 ` [PATCH v4 2/4] KVM: x86/mmu: use gfn_to_pfn_page David Stevens
2021-09-29 4:29 ` David Stevens
2021-09-29 4:29 ` David Stevens
2021-09-29 4:29 ` [PATCH v4 3/4] KVM: arm64/mmu: " David Stevens
2021-09-29 4:29 ` David Stevens
2021-09-29 4:29 ` David Stevens
2021-09-29 4:29 ` [PATCH v4 4/4] KVM: mmu: remove over-aggressive warnings David Stevens
2021-09-29 4:29 ` David Stevens
2021-09-29 4:29 ` David Stevens
2021-10-13 0:02 ` Sean Christopherson [this message]
2021-10-13 0:02 ` Sean Christopherson
2021-10-13 0:02 ` Sean Christopherson
2021-10-13 3:29 ` David Stevens
2021-10-13 3:29 ` David Stevens
2021-10-13 3:29 ` David Stevens
2021-10-15 0:10 ` Sean Christopherson
2021-10-15 0:10 ` Sean Christopherson
2021-10-15 0:10 ` Sean Christopherson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YWYiJy1Z7VZ0SxAd@google.com \
--to=seanjc@google.com \
--cc=alexandru.elisei@arm.com \
--cc=james.morse@arm.com \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maz@kernel.org \
--cc=pbonzini@redhat.com \
--cc=stevensd@chromium.org \
--cc=suzuki.poulose@arm.com \
--cc=wanpengli@tencent.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.