Received-SPF: pass (google.com: domain of linux-kernel+bounces-78068-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <92a21507-b179-49ca-b0e0-d0aea69ab3a3@redhat.com>
Date: Fri, 23 Feb 2024 10:44:58 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v10 0/8] KVM: allow mapping non-refcounted pages
Content-Language: en-US
To: David Stevens <stevensd@chromium.org>,
 Sean Christopherson <seanjc@google.com>
Cc: Yu Zhang <yu.c.zhang@linux.intel.com>,
 Isaku Yamahata <isaku.yamahata@gmail.com>,
 Zhi Wang <zhi.wang.linux@gmail.com>, Maxim Levitsky <mlevitsk@redhat.com>,
 kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, kvm@vger.kernel.org
References: <20240221072528.2702048-1-stevensd@google.com>
From: Paolo Bonzini <pbonzini@redhat.com>
In-Reply-To: <20240221072528.2702048-1-stevensd@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 2/21/24 08:25, David Stevens wrote:
> From: David Stevens <stevensd@chromium.org>
> 
> This patch series adds support for mapping VM_IO and VM_PFNMAP memory
> that is backed by struct pages that aren't currently being refcounted
> (e.g. tail pages of non-compound higher order allocations) into the
> guest.
> 
> Our use case is virtio-gpu blob resources [1], which directly map host
> graphics buffers into the guest as "vram" for the virtio-gpu device.
> This feature currently does not work on systems using the amdgpu driver,
> as that driver allocates non-compound higher order pages via
> ttm_pool_alloc_page().
> 
> First, this series replaces the gfn_to_pfn_memslot() API with a more
> extensible kvm_follow_pfn() API. The updated API rearranges
> gfn_to_pfn_memslot()'s args into a struct and where possible packs the
> bool arguments into a FOLL_ flags argument. The refactoring changes do
> not change any behavior.
> 
>  From there, this series extends the kvm_follow_pfn() API so that
> non-refconuted pages can be safely handled. This invloves adding an
> input parameter to indicate whether the caller can safely use
> non-refcounted pfns and an output parameter to tell the caller whether
> or not the returned page is refcounted. This change includes a breaking
> change, by disallowing non-refcounted pfn mappings by default, as such
> mappings are unsafe. To allow such systems to continue to function, an
> opt-in module parameter is added to allow the unsafe behavior.
> 
> This series only adds support for non-refcounted pages to x86. Other
> MMUs can likely be updated without too much difficulty, but it is not
> needed at this point. Updating other parts of KVM (e.g. pfncache) is not
> straightforward [2].

Looks good to me, apart that two patches were sent twice.  I only have a 
small comment on patch 4, to which I'll reply separately.

Paolo

> [1]
> https://patchwork.kernel.org/project/dri-devel/cover/20200814024000.2485-1-gurchetansingh@chromium.org/
> [2] https://lore.kernel.org/all/ZBEEQtmtNPaEqU1i@google.com/
> 
> v9 -> v10:
>   - Re-add FOLL_GET changes.
>   - Split x86/mmu spte+non-refcount-page patch into two patches.
>   - Rename 'foll' variables to 'kfp'.
>   - Properly gate usage of refcount spte bit when it's not available.
>   - Replace kfm_follow_pfn's is_refcounted_page output parameter with
>     a struct page *refcounted_page pointing to the page in question.
>   - Add patch downgrading BUG_ON to WARN_ON_ONCE.
> v8 -> v9:
>   - Make paying attention to is_refcounted_page mandatory. This means
>     that FOLL_GET is no longer necessary. For compatibility with
>     un-migrated callers, add a temporary parameter to sidestep
>     ref-counting issues.
>   - Add allow_unsafe_mappings, which is a breaking change.
>   - Migrate kvm_vcpu_map and other callsites used by x86 to the new API.
>   - Drop arm and ppc changes.
> v7 -> v8:
>   - Set access bits before releasing mmu_lock.
>   - Pass FOLL_GET on 32-bit x86 or !tdp_enabled.
>   - Refactor FOLL_GET handling, add kvm_follow_refcounted_pfn helper.
>   - Set refcounted bit on >4k pages.
>   - Add comments and apply formatting suggestions.
>   - rebase on kvm next branch.
> v6 -> v7:
>   - Replace __gfn_to_pfn_memslot with a more flexible __kvm_faultin_pfn,
>     and extend that API to support non-refcounted pages (complete
>     rewrite).
> 
> David Stevens (7):
>    KVM: Relax BUG_ON argument validation
>    KVM: mmu: Introduce kvm_follow_pfn()
>    KVM: mmu: Improve handling of non-refcounted pfns
>    KVM: Migrate kvm_vcpu_map() to kvm_follow_pfn()
>    KVM: x86: Migrate to kvm_follow_pfn()
>    KVM: x86/mmu: Track if sptes refer to refcounted pages
>    KVM: x86/mmu: Handle non-refcounted pages
> 
> Sean Christopherson (1):
>    KVM: Assert that a page's refcount is elevated when marking
>      accessed/dirty
> 
>   arch/x86/kvm/mmu/mmu.c          | 104 +++++++---
>   arch/x86/kvm/mmu/mmu_internal.h |   2 +
>   arch/x86/kvm/mmu/paging_tmpl.h  |   7 +-
>   arch/x86/kvm/mmu/spte.c         |   4 +-
>   arch/x86/kvm/mmu/spte.h         |  22 +-
>   arch/x86/kvm/mmu/tdp_mmu.c      |  22 +-
>   arch/x86/kvm/x86.c              |  11 +-
>   include/linux/kvm_host.h        |  53 ++++-
>   virt/kvm/guest_memfd.c          |   8 +-
>   virt/kvm/kvm_main.c             | 349 +++++++++++++++++++-------------
>   virt/kvm/kvm_mm.h               |   3 +-
>   virt/kvm/pfncache.c             |  11 +-
>   12 files changed, 399 insertions(+), 197 deletions(-)
> 
> 
> base-commit: 54be6c6c5ae8e0d93a6c4641cb7528eb0b6ba478