KVM Archive on lore.kernel.org
 help / color / Atom feed
From: Jason Gunthorpe <jgg@ziepe.ca>
To: Peter Xu <peterx@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	cohuck@redhat.com
Subject: Re: [PATCH v2 2/3] vfio-pci: Fault mmaps to enable vma tracking
Date: Thu, 7 May 2020 20:56:33 -0300
Message-ID: <20200507235633.GL26002@ziepe.ca> (raw)
In-Reply-To: <20200507222223.GR228260@xz-x1>

On Thu, May 07, 2020 at 06:22:23PM -0400, Peter Xu wrote:
> On Thu, May 07, 2020 at 04:03:34PM -0600, Alex Williamson wrote:
> > On Thu, 7 May 2020 17:47:44 -0400
> > Peter Xu <peterx@redhat.com> wrote:
> > 
> > > Hi, Alex,
> > > 
> > > On Tue, May 05, 2020 at 03:54:53PM -0600, Alex Williamson wrote:
> > > > +/*
> > > > + * Zap mmaps on open so that we can fault them in on access and therefore
> > > > + * our vma_list only tracks mappings accessed since last zap.
> > > > + */
> > > > +static void vfio_pci_mmap_open(struct vm_area_struct *vma)
> > > > +{
> > > > +	zap_vma_ptes(vma, vma->vm_start, vma->vm_end - vma->vm_start);  
> > > 
> > > A pure question: is this only a safety-belt or it is required in some known
> > > scenarios?
> > 
> > It's not required.  I originally did this so that I'm not allocating a
> > vma_list entry in a path where I can't return error, but as Jason
> > suggested I could zap here only in the case that I do encounter that
> > allocation fault.  However I still like consolidating the vma_list
> > handling to the vm_ops .fault and .close callbacks and potentially we
> > reduce the zap latency by keeping the vma_list to actual users, which
> > we'll get to eventually anyway in the VM case as memory BARs are sized
> > and assigned addresses.
> 
> Yes, I don't see much problem either on doing the vma_list maintainance only in
> .fault() and .close().  My understandingg is that the worst case is the perf
> critical applications (e.g. DPDK) could pre-fault these MMIO region easily
> during setup if they want.  My question was majorly about whether the vma
> should be guaranteed to have no mapping at all when .open() is called.  But I
> agree with you that it's always good to have that as safety-belt anyways.

If the VMA has a mapping then that specific VMA has to be in the
linked list.

So if the zap is skipped then the you have to allocate something and
add to the linked list to track the VMA with mapping.

It is not a 'safety belt'

Jason

  reply index

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-05 21:54 [PATCH v2 0/3] vfio-pci: Block user access to disabled device MMIO Alex Williamson
2020-05-05 21:54 ` [PATCH v2 1/3] vfio/type1: Support faulting PFNMAP vmas Alex Williamson
2020-05-07 21:24   ` Peter Xu
2020-05-07 21:47     ` Alex Williamson
2020-05-07 23:54     ` Jason Gunthorpe
2020-05-08  2:19       ` Peter Xu
2020-05-08 12:10         ` Jason Gunthorpe
2020-05-08 14:30           ` Peter Xu
2020-05-08 15:05             ` Jason Gunthorpe
2020-05-08 15:42               ` Alex Williamson
2020-05-08 16:05                 ` Peter Xu
2020-05-08 18:39   ` Peter Xu
2020-05-05 21:54 ` [PATCH v2 2/3] vfio-pci: Fault mmaps to enable vma tracking Alex Williamson
2020-05-07 21:47   ` Peter Xu
2020-05-07 22:03     ` Alex Williamson
2020-05-07 22:22       ` Peter Xu
2020-05-07 23:56         ` Jason Gunthorpe [this message]
2020-05-08  2:16           ` Peter Xu
2020-05-08  6:44             ` Jason Wang
2020-05-08 14:27               ` Peter Xu
2020-05-08 12:08             ` Jason Gunthorpe
2020-05-08 14:26               ` Peter Xu
2020-05-05 21:55 ` [PATCH v2 3/3] vfio-pci: Invalidate mmaps and block MMIO access on disabled memory Alex Williamson
2020-05-22  2:39   ` Qian Cai
2020-05-22  4:18     ` Alex Williamson
2020-05-07 21:59 ` [PATCH v2 0/3] vfio-pci: Block user access to disabled device MMIO Peter Xu
2020-05-07 22:34   ` Alex Williamson
2020-05-08  2:31     ` Peter Xu

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=20200507235633.GL26002@ziepe.ca \
    --to=jgg@ziepe.ca \
    --cc=alex.williamson@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterx@redhat.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

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org
	public-inbox-index kvm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git