On 09/08/2016 10:45 AM, Jike Song wrote:
> On 08/25/2016 05:22 PM, Dong Jia wrote:
>> On Thu, 25 Aug 2016 09:23:53 +0530
>> Kirti Wankhede <kwankhede@xxxxxxxxxx> wrote:
>>
>> [...]
>>
>> Dear Kirti,
>>
>> I just rebased my vfio-ccw patches to this series.
>> With a little fix, which was pointed it out in my reply to the #3
>> patch, it works fine.
>>
>
> Hi Jia,
>
> Sorry I didn't follow a lot in previous discussion, but since
> vfio-mdev in v7 patchset is at least PCI-agnostic, would you share
> with us why you still need a vfio-ccw?
Kind ping :)
Hi Dong Jia,
Since Kirti has confirmed that in v7 it is designed to have only one
vfio-mdev driver for all mdev devices, would you please tell us the
reason of your vfio-ccw? It could possibly be an architectural gap and
the earlier we discuss it the better :)
--
Thanks,
Jike
>>> +static long vfio_mdev_unlocked_ioctl(void *device_data,
>>> + unsigned int cmd, unsigned long arg)
>>> +{
>>> + int ret = 0;
>>> + struct vfio_mdev *vmdev = device_data;
>>> + struct parent_device *parent = vmdev->mdev->parent;
>>> + unsigned long minsz;
>>> +
>>> + switch (cmd) {
>>> + case VFIO_DEVICE_GET_INFO:
>>> + {
>>> + struct vfio_device_info info;
>>> +
>>> + minsz = offsetofend(struct vfio_device_info, num_irqs);
>>> +
>>> + if (copy_from_user(&info, (void __user *)arg, minsz))
>>> + return -EFAULT;
>>> +
>>> + if (info.argsz < minsz)
>>> + return -EINVAL;
>>> +
>>> + if (parent->ops->get_device_info)
>>> + ret = parent->ops->get_device_info(vmdev->mdev, &info);
>>> + else
>>> + return -EINVAL;
>>> +
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (parent->ops->reset)
>>> + info.flags |= VFIO_DEVICE_FLAGS_RESET;
>> Shouldn't this be done inside the get_device_info callback?
>>
>>> +
>>> + memcpy(&vmdev->dev_info, &info, sizeof(info));
>>> +
>>> + return copy_to_user((void __user *)arg, &info, minsz);
>>> + }
>> [...]
>>
>>> +
>>> +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?
>>
>>> + size_t filled;
>>> +
>>> + if (count >= 4 && !(*ppos % 4)) {
>>> + u32 val;
>>> +
>>> + ret = parent->ops->read(mdev, (char *)&val, sizeof(val),
>>> + *ppos);
>>> + if (ret <= 0)
>>> + goto read_err;
>>> +
>>> + if (copy_to_user(buf, &val, sizeof(val)))
>>> + goto read_err;
>>> +
>>> + filled = 4;
>>> + } else if (count >= 2 && !(*ppos % 2)) {
>>> + u16 val;
>>> +
>>> + ret = parent->ops->read(mdev, (char *)&val, sizeof(val),
>>> + *ppos);
>>> + if (ret <= 0)
>>> + goto read_err;
>>> +
>>> + if (copy_to_user(buf, &val, sizeof(val)))
>>> + goto read_err;
>>> +
>>> + filled = 2;
>>> + } else {
>>> + u8 val;
>>> +
>>> + ret = parent->ops->read(mdev, &val, sizeof(val), *ppos);
>>> + if (ret <= 0)
>>> + goto read_err;
>>> +
>>> + if (copy_to_user(buf, &val, sizeof(val)))
>>> + goto read_err;
>>> +
>>> + filled = 1;
>>> + }
>>> +
>>> + count -= filled;
>>> + done += filled;
>>> + *ppos += filled;
>>> + buf += filled;
>>> + }
>>> +
>>> + return done;
>>> +
>>> +read_err:
>>> + return -EFAULT;
>>> +}
>> [...]
>>
>> --------
>> Dong Jia
>>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
