From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Date: Thu, 24 Jun 2021 10:21:01 +0000 Subject: Re: [PATCH 2/6] KVM: mmu: also return page from gfn_to_pfn Message-Id: List-Id: References: <20210624035749.4054934-1-stevensd@google.com> <20210624035749.4054934-3-stevensd@google.com> <1624524331.zsin3qejl9.astroid@bobo.none> <201b68a7-10ea-d656-0c1e-5511b1f22674@redhat.com> <1624528342.s2ezcyp90x.astroid@bobo.none> <1624529635.75a1ann91v.astroid@bobo.none> In-Reply-To: <1624529635.75a1ann91v.astroid@bobo.none> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Nicholas Piggin , Aleksandar Markovic , Huacai Chen , Marc Zyngier , Paul Mackerras , David Stevens , Zhenyu Wang , Zhi Wang Cc: Alexandru Elisei , dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, intel-gvt-dev@lists.freedesktop.org, James Morse , Jim Mattson , Joerg Roedel , kvmarm@lists.cs.columbia.edu, kvm-ppc@vger.kernel.org, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, Sean Christopherson , Suzuki K Poulose , Vitaly Kuznetsov , Wanpeng Li , Will Deacon On 24/06/21 12:17, Nicholas Piggin wrote: >> If all callers were updated that is one thing, but from the changelog >> it sounds like that would not happen and there would be some gfn_to_pfn >> users left over. >> >> But yes in the end you would either need to make gfn_to_pfn never return >> a page found via follow_pte, or change all callers to the new way. If >> the plan is for the latter then I guess that's fine. > > Actually in that case anyway I don't see the need -- the existence of > gfn_to_pfn is enough to know it might be buggy. It can just as easily > be grepped for as kvm_pfn_page_unwrap. Sure, but that would leave us with longer function names (gfn_to_pfn_page* instead of gfn_to_pfn*). So the "safe" use is the one that looks worse and the unsafe use is the one that looks safe. > And are gfn_to_page cases also > vulernable to the same issue? No, they're just broken for the VM_IO|VM_PFNMAP case. Paolo > So I think it could be marked deprecated or something if not everything > will be converted in the one series, and don't need to touch all that > arch code with this patch.