On 05/22/2018 04:19 AM, Cornelia Huck wrote:
On Mon, 21 May 2018 11:13:58 -0400
Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx> wrote:
On 05/17/2018 03:44 AM, Cornelia Huck wrote:
On Mon, 14 May 2018 15:42:18 -0400
Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx> wrote:
On 05/11/2018 01:18 PM, Halil Pasic wrote:
On 05/07/2018 05:11 PM, Tony Krowiak wrote:
Registers the matrix device created by the VFIO AP device
driver with the VFIO mediated device framework.
Registering the matrix device will create the sysfs
structures needed to create mediated matrix devices
each of which will be used to configure the AP matrix
for a guest and connect it to the VFIO AP device driver.
+static int vfio_ap_mdev_create(struct kobject *kobj, struct
mdev_device *mdev)
+{
+ struct ap_matrix *ap_matrix = to_ap_matrix(mdev_parent_dev(mdev));
+
+ ap_matrix->available_instances--;
+
+ return 0;
+}
+
+static int vfio_ap_mdev_remove(struct mdev_device *mdev)
+{
+ struct ap_matrix *ap_matrix = to_ap_matrix(mdev_parent_dev(mdev));
+
+ ap_matrix->available_instances++;
+
+ return 0;
+}
+
The above functions seem to be called with the lock of this
auto-generated
mdev parent device held. That's why we don't have to care about
synchronization
ourselves, right?
I would assume as much. The comments for the 'struct mdev_parent_ops' in
include/linux/mdev.h do not mention anything about synchronization, nor
did I
see any locking or synchronization in the vfio_ccw implementation after
which
I modeled my code, so frankly it is something I did not consider.
A small comment in the code could be helpful for mdev non-experts.
Hell, I would
even consider documenting it for all mdev -- took me some time to
figure out.
You may want to bring this up with the VFIO mdev maintainers, but I'd be
happy to
include a comment in the functions in question if you think it important.
Important note: There's currently a patch on list that removes the mdev
parent mutex, and it seems there was never intended to be any
serialization in that place by the mdev core. (Look for "vfio/mdev:
Check globally for duplicate devices".)
The patch on the list holds the mdev_list_lock during create and remove
of an mdev device, so it looks like no synchronization is necessary on the
part of the vendor code in the create/remove callbacks; does that sound
about right?
v1/v2 did that; v3/v4 hold the list lock only while the device is added
to the mdev list. v4 also adds a note regarding locking to the
documentation.
I'll add some synchronization to the read/update of available instances.