KVM Archive on lore.kernel.org
 help / color / Atom feed
From: Peter Xu <peterx@redhat.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	cohuck@redhat.com, jgg@ziepe.ca, cai@lca.pw
Subject: Re: [PATCH v3 3/3] vfio-pci: Invalidate mmaps and block MMIO access on disabled memory
Date: Sat, 23 May 2020 15:34:17 -0400
Message-ID: <20200523193417.GI766834@xz-x1> (raw)
In-Reply-To: <159017506369.18853.17306023099999811263.stgit@gimli.home>

Hi, Alex,

On Fri, May 22, 2020 at 01:17:43PM -0600, Alex Williamson wrote:
> @@ -1346,15 +1526,32 @@ static vm_fault_t vfio_pci_mmap_fault(struct vm_fault *vmf)
>  {
>  	struct vm_area_struct *vma = vmf->vma;
>  	struct vfio_pci_device *vdev = vma->vm_private_data;
> +	vm_fault_t ret = VM_FAULT_NOPAGE;
> +
> +	mutex_lock(&vdev->vma_lock);
> +	down_read(&vdev->memory_lock);

I remembered to have seen the fault() handling FAULT_FLAG_RETRY_NOWAIT at least
in the very first version, but it's not here any more...  Could I ask what's
the reason behind?  I probably have missed something along with the versions,
I'm just not sure whether e.g. this would potentially block a GUP caller even
if it's with FOLL_NOWAIT.

Side note: Another thing I thought about when reading this patch - there seems
to have quite some possibility that the VFIO_DEVICE_PCI_HOT_RESET ioctl will
start to return -EBUSY now.  Not a problem for this series, but maybe we should
rememeber to let the userspace handle -EBUSY properly as follow up too, since I
saw QEMU seemed to not handle -EBUSY for host reset path right now.

Thanks,

-- 
Peter Xu


  reply index

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-22 19:17 [PATCH v3 0/3] vfio-pci: Block user access to disabled device MMIO Alex Williamson
2020-05-22 19:17 ` [PATCH v3 1/3] vfio/type1: Support faulting PFNMAP vmas Alex Williamson
2020-05-22 19:17 ` [PATCH v3 2/3] vfio-pci: Fault mmaps to enable vma tracking Alex Williamson
2020-05-22 19:17 ` [PATCH v3 3/3] vfio-pci: Invalidate mmaps and block MMIO access on disabled memory Alex Williamson
2020-05-23 19:34   ` Peter Xu [this message]
2020-05-23 23:06     ` Alex Williamson
2020-05-23 23:52       ` Peter Xu
2020-05-24  0:02         ` Peter Xu
2020-05-25 12:26         ` Jason Gunthorpe
2020-05-25 14:28           ` Peter Xu
2020-05-25 14:46             ` Jason Gunthorpe
2020-05-25 15:11               ` Peter Xu
2020-05-25 16:56                 ` Jason Gunthorpe
2020-05-25 20:56                   ` John Hubbard
2020-05-26  0:37                     ` Jason Gunthorpe
2020-05-26  0:46                       ` John Hubbard
2020-05-26 13:49                       ` Peter Xu
2020-05-26 14:32                         ` Alex Williamson
2020-05-26 14:46                           ` Peter Xu
2020-05-26 15:53                           ` Jason Gunthorpe
2020-05-26 15:57                             ` Alex Williamson
2020-05-22 22:08 ` [PATCH v3 0/3] vfio-pci: Block user access to disabled device MMIO Qian Cai
2020-05-22 22:25   ` Alex Williamson

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=20200523193417.GI766834@xz-x1 \
    --to=peterx@redhat.com \
    --cc=alex.williamson@redhat.com \
    --cc=cai@lca.pw \
    --cc=cohuck@redhat.com \
    --cc=jgg@ziepe.ca \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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