From: Sean Christopherson <seanjc@google.com>
To: Yan Zhao <yan.y.zhao@intel.com>
Cc: kvm@vger.kernel.org, intel-gfx@lists.freedesktop.org,
linux-kernel@vger.kernel.org, Ben Gardon <bgardon@google.com>,
Paolo Bonzini <pbonzini@redhat.com>,
intel-gvt-dev@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v3 03/28] drm/i915/gvt: Verify hugepages are contiguous in physical address space
Date: Thu, 18 May 2023 11:04:46 -0700 [thread overview]
Message-ID: <ZGZovmcrdh8NcWqb@google.com> (raw)
In-Reply-To: <ZGXqo+tG35S2c+QQ@yzhao56-desk.sh.intel.com>
On Thu, May 18, 2023, Yan Zhao wrote:
> On Wed, May 17, 2023 at 07:50:26AM -0700, Sean Christopherson wrote:
> > On Tue, May 16, 2023, Yan Zhao wrote:
> > > hi Sean
> > >
> > > Do you think it's necessary to double check that struct page pointers
> > > are also contiguous?
> >
> > No, the virtual address space should be irrelevant. The only way it would be
> > problematic is if something in dma_map_page() expected to be able to access the
> > entire chunk of memory by getting the virtual address of only the first page,
> > but I can't imagine that code is reading or writing memory, let alone doing so
> > across a huge range of memory.
> Yes, I do find arm_iommu version of dma_map_page() access the memory by getting
> virtual address of pages passed in, but it's implemented as page by page, not only
> from the first page.
>
> dma_map_page
> dma_map_page_attrs
> ops->map_page
> arm_iommu_map_page
Heh, thankfully this is ARM specific, which presumably doesn't collide with KVMGT.
> __dma_page_cpu_to_dev
> dma_cache_maint_page
>
> Just a little worried about the condition of PFNs are contiguous
> while they belong to different backends, e.g. one from system memory and
> one from MMIO.
> But I don't know how to avoid this without complicated checks.
> And this condition might not happen in practice.
IMO, assuming that contiguous pfns are vritually contiguous is wrong, i.e. would
be a bug in the other code. The above dma_cache_maint_page() get's this right,
and even has a well written comment to boot.
WARNING: multiple messages have this Message-ID (diff)
From: Sean Christopherson <seanjc@google.com>
To: Yan Zhao <yan.y.zhao@intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Zhenyu Wang <zhenyuw@linux.intel.com>,
Zhi Wang <zhi.a.wang@intel.com>,
kvm@vger.kernel.org, intel-gvt-dev@lists.freedesktop.org,
intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
Ben Gardon <bgardon@google.com>
Subject: Re: [PATCH v3 03/28] drm/i915/gvt: Verify hugepages are contiguous in physical address space
Date: Thu, 18 May 2023 11:04:46 -0700 [thread overview]
Message-ID: <ZGZovmcrdh8NcWqb@google.com> (raw)
In-Reply-To: <ZGXqo+tG35S2c+QQ@yzhao56-desk.sh.intel.com>
On Thu, May 18, 2023, Yan Zhao wrote:
> On Wed, May 17, 2023 at 07:50:26AM -0700, Sean Christopherson wrote:
> > On Tue, May 16, 2023, Yan Zhao wrote:
> > > hi Sean
> > >
> > > Do you think it's necessary to double check that struct page pointers
> > > are also contiguous?
> >
> > No, the virtual address space should be irrelevant. The only way it would be
> > problematic is if something in dma_map_page() expected to be able to access the
> > entire chunk of memory by getting the virtual address of only the first page,
> > but I can't imagine that code is reading or writing memory, let alone doing so
> > across a huge range of memory.
> Yes, I do find arm_iommu version of dma_map_page() access the memory by getting
> virtual address of pages passed in, but it's implemented as page by page, not only
> from the first page.
>
> dma_map_page
> dma_map_page_attrs
> ops->map_page
> arm_iommu_map_page
Heh, thankfully this is ARM specific, which presumably doesn't collide with KVMGT.
> __dma_page_cpu_to_dev
> dma_cache_maint_page
>
> Just a little worried about the condition of PFNs are contiguous
> while they belong to different backends, e.g. one from system memory and
> one from MMIO.
> But I don't know how to avoid this without complicated checks.
> And this condition might not happen in practice.
IMO, assuming that contiguous pfns are vritually contiguous is wrong, i.e. would
be a bug in the other code. The above dma_cache_maint_page() get's this right,
and even has a well written comment to boot.
next prev parent reply other threads:[~2023-05-18 18:04 UTC|newest]
Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-13 0:35 [Intel-gfx] [PATCH v3 00/28] drm/i915/gvt: KVM: KVMGT fixes and page-track cleanups Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 01/28] drm/i915/gvt: Verify pfn is "valid" before dereferencing "struct page" Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 02/28] drm/i915/gvt: remove interface intel_gvt_is_valid_gfn Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-15 11:07 ` [Intel-gfx] " Wang, Zhi A
2023-05-15 11:07 ` Wang, Zhi A
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 03/28] drm/i915/gvt: Verify hugepages are contiguous in physical address space Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-16 9:37 ` [Intel-gfx] " Yan Zhao
2023-05-16 9:37 ` Yan Zhao
2023-05-17 14:50 ` [Intel-gfx] " Sean Christopherson
2023-05-17 14:50 ` Sean Christopherson
2023-05-18 9:06 ` [Intel-gfx] " Yan Zhao
2023-05-18 9:06 ` Yan Zhao
2023-05-18 18:04 ` Sean Christopherson [this message]
2023-05-18 18:04 ` Sean Christopherson
2023-05-19 3:18 ` [Intel-gfx] " Yan Zhao
2023-05-19 3:18 ` Yan Zhao
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 04/28] drm/i915/gvt: Put the page reference obtained by KVM's gfn_to_pfn() Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 05/28] drm/i915/gvt: Explicitly check that vGPU is attached before shadowing Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-15 11:24 ` [Intel-gfx] " Wang, Zhi A
2023-05-15 11:24 ` Wang, Zhi A
2023-05-15 17:57 ` [Intel-gfx] " Sean Christopherson
2023-05-15 17:57 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 06/28] drm/i915/gvt: Error out on an attempt to shadowing an unknown GTT entry type Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-15 11:28 ` [Intel-gfx] " Wang, Zhi A
2023-05-15 11:28 ` Wang, Zhi A
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 07/28] drm/i915/gvt: Don't rely on KVM's gfn_to_pfn() to query possible 2M GTT Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-17 0:57 ` [Intel-gfx] " Yan Zhao
2023-05-17 0:57 ` Yan Zhao
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 08/28] drm/i915/gvt: Use an "unsigned long" to iterate over memslot gfns Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 09/28] drm/i915/gvt: Drop unused helper intel_vgpu_reset_gtt() Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 10/28] drm/i915/gvt: Protect gfn hash table with vgpu_lock Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 11/28] KVM: x86/mmu: Move kvm_arch_flush_shadow_{all, memslot}() to mmu.c Sean Christopherson
2023-05-13 0:35 ` [PATCH v3 11/28] KVM: x86/mmu: Move kvm_arch_flush_shadow_{all,memslot}() " Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 12/28] KVM: x86/mmu: Don't rely on page-track mechanism to flush on memslot change Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-17 2:06 ` [Intel-gfx] " Yan Zhao
2023-05-17 2:06 ` Yan Zhao
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 13/28] KVM: x86/mmu: Don't bounce through page-track mechanism for guest PTEs Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 14/28] KVM: drm/i915/gvt: Drop @vcpu from KVM's ->track_write() hook Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 15/28] KVM: x86: Reject memslot MOVE operations if KVMGT is attached Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 16/28] drm/i915/gvt: Don't bother removing write-protection on to-be-deleted slot Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 17/28] KVM: x86: Add a new page-track hook to handle memslot deletion Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 18/28] drm/i915/gvt: switch from ->track_flush_slot() to ->track_remove_region() Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 19/28] KVM: x86: Remove the unused page-track hook track_flush_slot() Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 20/28] KVM: x86/mmu: Move KVM-only page-track declarations to internal header Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 21/28] KVM: x86/mmu: Use page-track notifiers iff there are external users Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-17 3:27 ` [Intel-gfx] " Yan Zhao
2023-05-17 3:27 ` Yan Zhao
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 22/28] KVM: x86/mmu: Drop infrastructure for multiple page-track modes Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 23/28] KVM: x86/mmu: Rename page-track APIs to reflect the new reality Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 24/28] KVM: x86/mmu: Assert that correct locks are held for page write-tracking Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 25/28] KVM: x86/mmu: Bug the VM if write-tracking is used but not enabled Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 26/28] KVM: x86/mmu: Drop @slot param from exported/external page-track APIs Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:35 ` [Intel-gfx] [PATCH v3 27/28] KVM: x86/mmu: Handle KVM bookkeeping in page-track APIs, not callers Sean Christopherson
2023-05-13 0:35 ` Sean Christopherson
2023-05-13 0:36 ` [Intel-gfx] [PATCH v3 28/28] drm/i915/gvt: Drop final dependencies on KVM internal details Sean Christopherson
2023-05-13 0:36 ` Sean Christopherson
2023-05-13 1:46 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gvt: KVM: KVMGT fixes and page-track cleanups (rev8) Patchwork
2023-05-13 2:00 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2023-06-14 3:06 ` [Intel-gfx] [PATCH v3 00/28] drm/i915/gvt: KVM: KVMGT fixes and page-track cleanups Ma, Yongwei
2023-06-14 3:06 ` Ma, Yongwei
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=ZGZovmcrdh8NcWqb@google.com \
--to=seanjc@google.com \
--cc=bgardon@google.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-gvt-dev@lists.freedesktop.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=yan.y.zhao@intel.com \
/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.