From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kirti Wankhede Subject: Re: [Qemu-devel] [PATCH v7 2/4] vfio: VFIO driver for mediated devices Date: Mon, 19 Sep 2016 23:52:36 +0530 Message-ID: <1a5e06cb-35bf-e7e3-0c0c-1ebf0fa95dcd@nvidia.com> References: <1472097235-6332-1-git-send-email-kwankhede@nvidia.com> <1472097235-6332-3-git-send-email-kwankhede@nvidia.com> <20160825172226.2cd06d71@oc7835276234> <72e3baa7-70d1-c5dd-6cd7-3874e7ea4c01@nvidia.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Cc: , , , , , , To: Dong Jia , Return-path: Received: from hqemgate16.nvidia.com ([216.228.121.65]:12643 "EHLO hqemgate16.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932237AbcISSXR (ORCPT ); Mon, 19 Sep 2016 14:23:17 -0400 In-Reply-To: <72e3baa7-70d1-c5dd-6cd7-3874e7ea4c01@nvidia.com> Sender: kvm-owner@vger.kernel.org List-ID: On 8/26/2016 7:43 PM, Kirti Wankhede wrote: > * PGP Signed: 08/26/2016 at 07:15:44 AM, Decrypted > On 8/25/2016 2:52 PM, Dong Jia wrote: >> On Thu, 25 Aug 2016 09:23:53 +0530 >>> + >>> +static ssize_t vfio_mdev_read(void *device_data, char __user *buf, >>> + size_t count, loff_t *ppos) >>> +{ >>> + struct vfio_mdev *vmdev = device_data; >>> + struct mdev_device *mdev = vmdev->mdev; >>> + struct parent_device *parent = mdev->parent; >>> + unsigned int done = 0; >>> + int ret; >>> + >>> + if (!parent->ops->read) >>> + return -EINVAL; >>> + >>> + while (count) { >> Here, I have to say sorry to you guys for that I didn't notice the >> bad impact of this change to my patches during the v6 discussion. >> >> For vfio-ccw, I introduced an I/O region to input/output I/O >> instruction parameters and results for Qemu. The @count of these data >> currently is 140. So supporting arbitrary lengths in one shot here, and >> also in vfio_mdev_write, seems the better option for this case. >> >> I believe that if the pci drivers want to iterate in a 4 bytes step, you >> can do that in the parent read/write callbacks instead. >> >> What do you think? >> > > I would like to know Alex's thought on this. He raised concern with this > approach in v6 reviews: > "But I think this is exploitable, it lets the user make the kernel > allocate an arbitrarily sized buffer." > Read/write callbacks are for slow path, emulation of mmio region which are mainly device registers. I do feel it shouldn't support arbitrary lengths. Alex, I would like to know your thoughts. Thanks, Kirti