linux-kernel - Re: [PATCH v10 01/19] vfio: Mediated device Core driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9d417e40-f4d3-ab3c-47a1-ac730e6e4ae6@nvidia.com>
Date:   Wed, 2 Nov 2016 13:29:28 +0530
From:   Kirti Wankhede <kwankhede@...dia.com>
To:     Jike Song <jike.song@...el.com>
CC:     <alex.williamson@...hat.com>, <pbonzini@...hat.com>,
        <kraxel@...hat.com>, <cjia@...dia.com>, <qemu-devel@...gnu.org>,
        <kvm@...r.kernel.org>, <kevin.tian@...el.com>,
        <bjsdjshi@...ux.vnet.ibm.com>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v10 01/19] vfio: Mediated device Core driver



On 10/29/2016 11:41 PM, Jike Song wrote:
> On 10/29/2016 06:06 PM, Kirti Wankhede wrote:
>>
>>
>> On 10/29/2016 10:00 AM, Jike Song wrote:
>>> On 10/27/2016 05:29 AM, Kirti Wankhede wrote:
>>>> +int mdev_register_device(struct device *dev, const struct parent_ops *ops)
>>>> +{
>>>> +	int ret;
>>>> +	struct parent_device *parent;
>>>> +
>>>> +	/* check for mandatory ops */
>>>> +	if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
>>>> +		return -EINVAL;
>>>> +
>>>> +	dev = get_device(dev);
>>>> +	if (!dev)
>>>> +		return -EINVAL;
>>>> +
>>>> +	mutex_lock(&parent_list_lock);
>>>> +
>>>> +	/* Check for duplicate */
>>>> +	parent = __find_parent_device(dev);
>>>> +	if (parent) {
>>>> +		ret = -EEXIST;
>>>> +		goto add_dev_err;
>>>> +	}
>>>> +
>>>> +	parent = kzalloc(sizeof(*parent), GFP_KERNEL);
>>>> +	if (!parent) {
>>>> +		ret = -ENOMEM;
>>>> +		goto add_dev_err;
>>>> +	}
>>>> +
>>>> +	kref_init(&parent->ref);
>>>> +	mutex_init(&parent->lock);
>>>> +
>>>> +	parent->dev = dev;
>>>> +	parent->ops = ops;
>>>> +
>>>> +	ret = parent_create_sysfs_files(parent);
>>>> +	if (ret) {
>>>> +		mutex_unlock(&parent_list_lock);
>>>> +		mdev_put_parent(parent);
>>>> +		return ret;
>>>> +	}
>>>> +
>>>> +	ret = class_compat_create_link(mdev_bus_compat_class, dev, NULL);
>>>> +	if (ret)
>>>> +		dev_warn(dev, "Failed to create compatibility class link\n");
>>>> +
>>>
>>> Hi Kirti,
>>>
>>> Like I replied to previous version:
>>>
>>> 	http://www.spinics.net/lists/kvm/msg139331.html
>>>
>>
>> Hi Jike,
>>
>> I saw your reply but by that time v10 version of patch series was out
>> for review.
>>
> 
> Ah..yes, I forgot that :)
> 
>>> You can always check if mdev_bus_compat_class already registered
>>> here, and register it if not yet. Same logic should be adopted to
>>> mdev_init.
>>>
>>> Current implementation will simply panic if configured as builtin,
>>> which is rare but far from impossible.
>>>
>>
>> Can you verify attached patch with v10 patch-set whether this works for you?
>> I'll incorporate this change in my next version.
>>
> 
> Seems cool. But would you please also keep the register in mdev_init(),
> just check the 'in case it was already registered' case? Thanks!
> 

The class is used only to keep symbolic to the devices which are
registered to mdev framework. So I think its ok to register this class
when first device is registered.

Thanks,
Kirti