| Message ID | 20221223005739.1295925-1-seanjc@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/i915/gvt: KVM: KVMGT fixes and page-track cleanups | expand |
On Fri, Dec 23, 2022 at 12:57:12AM +0000, Sean Christopherson wrote: > Fix a variety of found-by-inspection bugs in KVMGT, and overhaul KVM's > page-track APIs to provide a leaner and cleaner interface. The motivation > for this series is to (significantly) reduce the number of KVM APIs that > KVMGT uses, with a long-term goal of making all kvm_host.h headers > KVM-internal. That said, I think the cleanup itself is worthwhile, > e.g. KVMGT really shouldn't be touching kvm->mmu_lock. > > Note! The KVMGT changes are compile tested only as I don't have the > necessary hardware (AFAIK). Testing, and lots of it, on the KVMGT side > of things is needed and any help on that front would be much appreciated. hi Sean, Thanks for the patch! Could you also provide the commit id that this series is based on? I applied them on top of latest master branch (6.1.0+, 8395ae05cb5a2e31d36106e8c85efa11cda849be) in repo https://github.com/torvalds/linux.git, yet met some conflicts and I fixed them manually. (patch 11 and patch 25). A rough test shows that below mutex_init is missing. But even with this fix, I still met guest hang during guest boots up. Will look into it and have a detailed review next week. diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c index a7ac2ec00196..c274b6a05555 100644 --- a/drivers/gpu/drm/i915/gvt/vgpu.c +++ b/drivers/gpu/drm/i915/gvt/vgpu.c @@ -331,6 +331,7 @@ int intel_gvt_create_vgpu(struct intel_vgpu *vgpu, vgpu->id = ret; vgpu->sched_ctl.weight = conf->weight; mutex_init(&vgpu->vgpu_lock); + mutex_init(&vgpu->gfn_lock); mutex_init(&vgpu->dmabuf_lock); INIT_LIST_HEAD(&vgpu->dmabuf_obj_list_head); INIT_RADIX_TREE(&vgpu->page_track_tree, GFP_KERNEL); Thanks Yan
On Fri, Dec 23, 2022, Yan Zhao wrote: > On Fri, Dec 23, 2022 at 12:57:12AM +0000, Sean Christopherson wrote: > > Fix a variety of found-by-inspection bugs in KVMGT, and overhaul KVM's > > page-track APIs to provide a leaner and cleaner interface. The motivation > > for this series is to (significantly) reduce the number of KVM APIs that > > KVMGT uses, with a long-term goal of making all kvm_host.h headers > > KVM-internal. That said, I think the cleanup itself is worthwhile, > > e.g. KVMGT really shouldn't be touching kvm->mmu_lock. > > > > Note! The KVMGT changes are compile tested only as I don't have the > > necessary hardware (AFAIK). Testing, and lots of it, on the KVMGT side > > of things is needed and any help on that front would be much appreciated. > hi Sean, > Thanks for the patch! > Could you also provide the commit id that this series is based on? The commit ID is provided in the cover letter: base-commit: 9d75a3251adfbcf444681474511b58042a364863 Though you might have a hard time finding that commit as it's from an old version of kvm/queue that's probably since been force pushed. > I applied them on top of latest master branch (6.1.0+, > 8395ae05cb5a2e31d36106e8c85efa11cda849be) in repo > https://github.com/torvalds/linux.git, yet met some conflicts and I > fixed them manually. (patch 11 and patch 25). > > A rough test shows that below mutex_init is missing. > But even with this fix, I still met guest hang during guest boots up. > Will look into it and have a detailed review next week. Thanks again for the reviews and testing! I'll get a v2 out in the next week or so (catching up from holidays) and will be more explicit in documenting the base version.
On Wed, Jan 04, 2023 at 01:01:13AM +0000, Sean Christopherson wrote: > On Fri, Dec 23, 2022, Yan Zhao wrote: > > On Fri, Dec 23, 2022 at 12:57:12AM +0000, Sean Christopherson wrote: > > > Fix a variety of found-by-inspection bugs in KVMGT, and overhaul KVM's > > > page-track APIs to provide a leaner and cleaner interface. The motivation > > > for this series is to (significantly) reduce the number of KVM APIs that > > > KVMGT uses, with a long-term goal of making all kvm_host.h headers > > > KVM-internal. That said, I think the cleanup itself is worthwhile, > > > e.g. KVMGT really shouldn't be touching kvm->mmu_lock. > > > > > > Note! The KVMGT changes are compile tested only as I don't have the > > > necessary hardware (AFAIK). Testing, and lots of it, on the KVMGT side > > > of things is needed and any help on that front would be much appreciated. > > hi Sean, > > Thanks for the patch! > > Could you also provide the commit id that this series is based on? > > The commit ID is provided in the cover letter: > > base-commit: 9d75a3251adfbcf444681474511b58042a364863 > > Though you might have a hard time finding that commit as it's from an old > version of kvm/queue that's probably since been force pushed. > > > I applied them on top of latest master branch (6.1.0+, > > 8395ae05cb5a2e31d36106e8c85efa11cda849be) in repo > > https://github.com/torvalds/linux.git, yet met some conflicts and I > > fixed them manually. (patch 11 and patch 25). > > > > A rough test shows that below mutex_init is missing. > > But even with this fix, I still met guest hang during guest boots up. > > Will look into it and have a detailed review next week. > > Thanks again for the reviews and testing! I'll get a v2 out in the next week or > so (catching up from holidays) and will be more explicit in documenting the base > version. That's fine and it's a pleasure to me :)
Fix a variety of found-by-inspection bugs in KVMGT, and overhaul KVM's page-track APIs to provide a leaner and cleaner interface. The motivation for this series is to (significantly) reduce the number of KVM APIs that KVMGT uses, with a long-term goal of making all kvm_host.h headers KVM-internal. That said, I think the cleanup itself is worthwhile, e.g. KVMGT really shouldn't be touching kvm->mmu_lock. Note! The KVMGT changes are compile tested only as I don't have the necessary hardware (AFAIK). Testing, and lots of it, on the KVMGT side of things is needed and any help on that front would be much appreciated. Sean Christopherson (24): drm/i915/gvt: Verify pfn is "valid" before dereferencing "struct page" KVM: x86/mmu: Factor out helper to get max mapping size of a memslot drm/i915/gvt: Incorporate KVM memslot info into check for 2MiB GTT entry drm/i915/gvt: Verify VFIO-pinned page is THP when shadowing 2M gtt entry drm/i915/gvt: Put the page reference obtained by KVM's gfn_to_pfn() drm/i915/gvt: Don't rely on KVM's gfn_to_pfn() to query possible 2M GTT drm/i915/gvt: Use an "unsigned long" to iterate over memslot gfns drm/i915/gvt: Hoist acquisition of vgpu_lock out to kvmgt_page_track_write() drm/i915/gvt: Protect gfn hash table with dedicated mutex KVM: x86/mmu: Don't rely on page-track mechanism to flush on memslot change KVM: x86/mmu: Don't bounce through page-track mechanism for guest PTEs KVM: drm/i915/gvt: Drop @vcpu from KVM's ->track_write() hook KVM: x86: Reject memslot MOVE operations if KVMGT is attached drm/i915/gvt: Don't bother removing write-protection on to-be-deleted slot KVM: x86/mmu: Move KVM-only page-track declarations to internal header KVM: x86/mmu: Use page-track notifiers iff there are external users KVM: x86/mmu: Drop infrastructure for multiple page-track modes KVM: x86/mmu: Rename page-track APIs to reflect the new reality KVM: x86/mmu: Assert that correct locks are held for page write-tracking KVM: x86/mmu: Bug the VM if write-tracking is used but not enabled KVM: x86/mmu: Drop @slot param from exported/external page-track APIs KVM: x86/mmu: Handle KVM bookkeeping in page-track APIs, not callers KVM: x86/mmu: Add page-track API to query if a gfn is valid drm/i915/gvt: Drop final dependencies on KVM internal details Yan Zhao (3): KVM: x86: Add a new page-track hook to handle memslot deletion drm/i915/gvt: switch from ->track_flush_slot() to ->track_remove_region() KVM: x86: Remove the unused page-track hook track_flush_slot() arch/x86/include/asm/kvm_host.h | 16 +- arch/x86/include/asm/kvm_page_track.h | 67 +++--- arch/x86/kvm/mmu.h | 2 + arch/x86/kvm/mmu/mmu.c | 61 +++--- arch/x86/kvm/mmu/mmu_internal.h | 2 + arch/x86/kvm/mmu/page_track.c | 283 +++++++++++++++----------- arch/x86/kvm/mmu/page_track.h | 59 ++++++ arch/x86/kvm/x86.c | 13 +- drivers/gpu/drm/i915/gvt/gtt.c | 45 ++-- drivers/gpu/drm/i915/gvt/gvt.h | 4 +- drivers/gpu/drm/i915/gvt/kvmgt.c | 138 ++++++------- drivers/gpu/drm/i915/gvt/page_track.c | 10 +- drivers/gpu/drm/i915/gvt/vgpu.c | 1 + 13 files changed, 386 insertions(+), 315 deletions(-) create mode 100644 arch/x86/kvm/mmu/page_track.h base-commit: 9d75a3251adfbcf444681474511b58042a364863