linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] vdpa: add driver_override support
@ 2021-11-04 16:17 Stefano Garzarella
  2021-11-05  3:01 ` Jason Wang
  0 siblings, 1 reply; 8+ messages in thread
From: Stefano Garzarella @ 2021-11-04 16:17 UTC (permalink / raw)
  To: virtualization; +Cc: Parav Pandit, Michael S. Tsirkin, Jason Wang, linux-kernel

`driver_override` allows to control which of the vDPA bus drivers
binds to a vDPA device.

If `driver_override` is not set, the previous behaviour is followed:
devices use the first vDPA bus driver loaded (unless auto binding
is disabled).

Tested on Fedora 34 with driverctl(8):
  $ modprobe virtio-vdpa
  $ modprobe vhost-vdpa
  $ modprobe vdpa-sim-net

  $ vdpa dev add mgmtdev vdpasim_net name dev1

  # dev1 is attached to the first vDPA bus driver loaded
  $ driverctl -b vdpa list-devices
    dev1 virtio_vdpa

  $ driverctl -b vdpa set-override dev1 vhost_vdpa

  $ driverctl -b vdpa list-devices
    dev1 vhost_vdpa [*]

  Note: driverctl(8) integrates with udev so the binding is
  preserved.

Suggested-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 include/linux/vdpa.h |  2 ++
 drivers/vdpa/vdpa.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+)

diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index c3011ccda430..ae34015b37b7 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -64,6 +64,7 @@ struct vdpa_mgmt_dev;
  * struct vdpa_device - representation of a vDPA device
  * @dev: underlying device
  * @dma_dev: the actual device that is performing DMA
+ * @driver_override: driver name to force a match
  * @config: the configuration ops for this device.
  * @cf_mutex: Protects get and set access to configuration layout.
  * @index: device index
@@ -76,6 +77,7 @@ struct vdpa_mgmt_dev;
 struct vdpa_device {
 	struct device dev;
 	struct device *dma_dev;
+	const char *driver_override;
 	const struct vdpa_config_ops *config;
 	struct mutex cf_mutex; /* Protects get/set config */
 	unsigned int index;
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 7332a74a4b00..659231bbfee8 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -52,8 +52,81 @@ static void vdpa_dev_remove(struct device *d)
 		drv->remove(vdev);
 }
 
+static int vdpa_dev_match(struct device *dev, struct device_driver *drv)
+{
+	struct vdpa_device *vdev = dev_to_vdpa(dev);
+
+	/* Check override first, and if set, only use the named driver */
+	if (vdev->driver_override)
+		return strcmp(vdev->driver_override, drv->name) == 0;
+
+	/* Currently devices must be supported by all vDPA bus drivers */
+	return 1;
+}
+
+static ssize_t driver_override_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct vdpa_device *vdev = dev_to_vdpa(dev);
+	const char *driver_override, *old;
+	char *cp;
+
+	/* We need to keep extra room for a newline */
+	if (count >= (PAGE_SIZE - 1))
+		return -EINVAL;
+
+	driver_override = kstrndup(buf, count, GFP_KERNEL);
+	if (!driver_override)
+		return -ENOMEM;
+
+	cp = strchr(driver_override, '\n');
+	if (cp)
+		*cp = '\0';
+
+	device_lock(dev);
+	old = vdev->driver_override;
+	if (strlen(driver_override)) {
+		vdev->driver_override = driver_override;
+	} else {
+		kfree(driver_override);
+		vdev->driver_override = NULL;
+	}
+	device_unlock(dev);
+
+	kfree(old);
+
+	return count;
+}
+
+static ssize_t driver_override_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct vdpa_device *vdev = dev_to_vdpa(dev);
+	ssize_t len;
+
+	device_lock(dev);
+	len = snprintf(buf, PAGE_SIZE, "%s\n", vdev->driver_override);
+	device_unlock(dev);
+
+	return len;
+}
+static DEVICE_ATTR_RW(driver_override);
+
+static struct attribute *vdpa_dev_attrs[] = {
+	&dev_attr_driver_override.attr,
+	NULL,
+};
+
+static const struct attribute_group vdpa_dev_group = {
+	.attrs  = vdpa_dev_attrs,
+};
+__ATTRIBUTE_GROUPS(vdpa_dev);
+
 static struct bus_type vdpa_bus = {
 	.name  = "vdpa",
+	.dev_groups = vdpa_dev_groups,
+	.match = vdpa_dev_match,
 	.probe = vdpa_dev_probe,
 	.remove = vdpa_dev_remove,
 };
@@ -68,6 +141,7 @@ static void vdpa_release_dev(struct device *d)
 
 	ida_simple_remove(&vdpa_index_ida, vdev->index);
 	mutex_destroy(&vdev->cf_mutex);
+	kfree(vdev->driver_override);
 	kfree(vdev);
 }
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] vdpa: add driver_override support
  2021-11-04 16:17 [PATCH] vdpa: add driver_override support Stefano Garzarella
@ 2021-11-05  3:01 ` Jason Wang
  2021-11-05  8:04   ` Stefano Garzarella
  2021-11-08 17:05   ` Stefano Garzarella
  0 siblings, 2 replies; 8+ messages in thread
From: Jason Wang @ 2021-11-05  3:01 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: virtualization, Parav Pandit, Michael S. Tsirkin, linux-kernel

On Fri, Nov 5, 2021 at 12:17 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> `driver_override` allows to control which of the vDPA bus drivers
> binds to a vDPA device.
>
> If `driver_override` is not set, the previous behaviour is followed:
> devices use the first vDPA bus driver loaded (unless auto binding
> is disabled).
>
> Tested on Fedora 34 with driverctl(8):
>   $ modprobe virtio-vdpa
>   $ modprobe vhost-vdpa
>   $ modprobe vdpa-sim-net
>
>   $ vdpa dev add mgmtdev vdpasim_net name dev1
>
>   # dev1 is attached to the first vDPA bus driver loaded
>   $ driverctl -b vdpa list-devices
>     dev1 virtio_vdpa
>
>   $ driverctl -b vdpa set-override dev1 vhost_vdpa
>
>   $ driverctl -b vdpa list-devices
>     dev1 vhost_vdpa [*]
>
>   Note: driverctl(8) integrates with udev so the binding is
>   preserved.
>
> Suggested-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  include/linux/vdpa.h |  2 ++
>  drivers/vdpa/vdpa.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 76 insertions(+)
>
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index c3011ccda430..ae34015b37b7 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -64,6 +64,7 @@ struct vdpa_mgmt_dev;
>   * struct vdpa_device - representation of a vDPA device
>   * @dev: underlying device
>   * @dma_dev: the actual device that is performing DMA
> + * @driver_override: driver name to force a match

This seems useless?

>   * @config: the configuration ops for this device.
>   * @cf_mutex: Protects get and set access to configuration layout.
>   * @index: device index
> @@ -76,6 +77,7 @@ struct vdpa_mgmt_dev;
>  struct vdpa_device {
>         struct device dev;
>         struct device *dma_dev;
> +       const char *driver_override;
>         const struct vdpa_config_ops *config;
>         struct mutex cf_mutex; /* Protects get/set config */
>         unsigned int index;
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index 7332a74a4b00..659231bbfee8 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -52,8 +52,81 @@ static void vdpa_dev_remove(struct device *d)
>                 drv->remove(vdev);
>  }
>
> +static int vdpa_dev_match(struct device *dev, struct device_driver *drv)
> +{
> +       struct vdpa_device *vdev = dev_to_vdpa(dev);
> +
> +       /* Check override first, and if set, only use the named driver */
> +       if (vdev->driver_override)
> +               return strcmp(vdev->driver_override, drv->name) == 0;
> +
> +       /* Currently devices must be supported by all vDPA bus drivers */
> +       return 1;
> +}
> +
> +static ssize_t driver_override_store(struct device *dev,
> +                                    struct device_attribute *attr,
> +                                    const char *buf, size_t count)
> +{
> +       struct vdpa_device *vdev = dev_to_vdpa(dev);
> +       const char *driver_override, *old;
> +       char *cp;
> +
> +       /* We need to keep extra room for a newline */
> +       if (count >= (PAGE_SIZE - 1))
> +               return -EINVAL;
> +
> +       driver_override = kstrndup(buf, count, GFP_KERNEL);
> +       if (!driver_override)
> +               return -ENOMEM;
> +
> +       cp = strchr(driver_override, '\n');
> +       if (cp)
> +               *cp = '\0';
> +
> +       device_lock(dev);
> +       old = vdev->driver_override;
> +       if (strlen(driver_override)) {
> +               vdev->driver_override = driver_override;
> +       } else {
> +               kfree(driver_override);
> +               vdev->driver_override = NULL;
> +       }
> +       device_unlock(dev);
> +
> +       kfree(old);
> +
> +       return count;
> +}
> +
> +static ssize_t driver_override_show(struct device *dev,
> +                                   struct device_attribute *attr, char *buf)
> +{
> +       struct vdpa_device *vdev = dev_to_vdpa(dev);
> +       ssize_t len;
> +
> +       device_lock(dev);
> +       len = snprintf(buf, PAGE_SIZE, "%s\n", vdev->driver_override);
> +       device_unlock(dev);
> +
> +       return len;
> +}
> +static DEVICE_ATTR_RW(driver_override);
> +
> +static struct attribute *vdpa_dev_attrs[] = {
> +       &dev_attr_driver_override.attr,
> +       NULL,
> +};
> +
> +static const struct attribute_group vdpa_dev_group = {
> +       .attrs  = vdpa_dev_attrs,
> +};
> +__ATTRIBUTE_GROUPS(vdpa_dev);
> +
>  static struct bus_type vdpa_bus = {
>         .name  = "vdpa",
> +       .dev_groups = vdpa_dev_groups,

This reminds me that we probably need to document the sysfs interface
in Documentation/ABI/testing/sysfs-bus-vdpa.

But it's not the fault of this patch which look good.

Thanks

> +       .match = vdpa_dev_match,
>         .probe = vdpa_dev_probe,
>         .remove = vdpa_dev_remove,
>  };
> @@ -68,6 +141,7 @@ static void vdpa_release_dev(struct device *d)
>
>         ida_simple_remove(&vdpa_index_ida, vdev->index);
>         mutex_destroy(&vdev->cf_mutex);
> +       kfree(vdev->driver_override);
>         kfree(vdev);
>  }
>
> --
> 2.31.1
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] vdpa: add driver_override support
  2021-11-05  3:01 ` Jason Wang
@ 2021-11-05  8:04   ` Stefano Garzarella
  2021-11-05  8:26     ` Jason Wang
  2021-11-08 17:05   ` Stefano Garzarella
  1 sibling, 1 reply; 8+ messages in thread
From: Stefano Garzarella @ 2021-11-05  8:04 UTC (permalink / raw)
  To: Jason Wang; +Cc: virtualization, Parav Pandit, Michael S. Tsirkin, linux-kernel

On Fri, Nov 05, 2021 at 11:01:30AM +0800, Jason Wang wrote:
>On Fri, Nov 5, 2021 at 12:17 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>>
>> `driver_override` allows to control which of the vDPA bus drivers
>> binds to a vDPA device.
>>
>> If `driver_override` is not set, the previous behaviour is followed:
>> devices use the first vDPA bus driver loaded (unless auto binding
>> is disabled).
>>
>> Tested on Fedora 34 with driverctl(8):
>>   $ modprobe virtio-vdpa
>>   $ modprobe vhost-vdpa
>>   $ modprobe vdpa-sim-net
>>
>>   $ vdpa dev add mgmtdev vdpasim_net name dev1
>>
>>   # dev1 is attached to the first vDPA bus driver loaded
>>   $ driverctl -b vdpa list-devices
>>     dev1 virtio_vdpa
>>
>>   $ driverctl -b vdpa set-override dev1 vhost_vdpa
>>
>>   $ driverctl -b vdpa list-devices
>>     dev1 vhost_vdpa [*]
>>
>>   Note: driverctl(8) integrates with udev so the binding is
>>   preserved.
>>
>> Suggested-by: Jason Wang <jasowang@redhat.com>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> ---
>>  include/linux/vdpa.h |  2 ++
>>  drivers/vdpa/vdpa.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 76 insertions(+)
>>
>> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
>> index c3011ccda430..ae34015b37b7 100644
>> --- a/include/linux/vdpa.h
>> +++ b/include/linux/vdpa.h
>> @@ -64,6 +64,7 @@ struct vdpa_mgmt_dev;
>>   * struct vdpa_device - representation of a vDPA device
>>   * @dev: underlying device
>>   * @dma_dev: the actual device that is performing DMA
>> + * @driver_override: driver name to force a match
>
>This seems useless?

I'm a bit lost, do you mean we should remove the documentation of 
`driver_override`?

>
>>   * @config: the configuration ops for this device.
>>   * @cf_mutex: Protects get and set access to configuration layout.
>>   * @index: device index
>> @@ -76,6 +77,7 @@ struct vdpa_mgmt_dev;
>>  struct vdpa_device {
>>         struct device dev;
>>         struct device *dma_dev;
>> +       const char *driver_override;
>>         const struct vdpa_config_ops *config;
>>         struct mutex cf_mutex; /* Protects get/set config */
>>         unsigned int index;
>> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
>> index 7332a74a4b00..659231bbfee8 100644
>> --- a/drivers/vdpa/vdpa.c
>> +++ b/drivers/vdpa/vdpa.c
>> @@ -52,8 +52,81 @@ static void vdpa_dev_remove(struct device *d)
>>                 drv->remove(vdev);
>>  }
>>
>> +static int vdpa_dev_match(struct device *dev, struct device_driver *drv)
>> +{
>> +       struct vdpa_device *vdev = dev_to_vdpa(dev);
>> +
>> +       /* Check override first, and if set, only use the named driver */
>> +       if (vdev->driver_override)
>> +               return strcmp(vdev->driver_override, drv->name) == 0;
>> +
>> +       /* Currently devices must be supported by all vDPA bus 
>> drivers */
>> +       return 1;
>> +}
>> +
>> +static ssize_t driver_override_store(struct device *dev,
>> +                                    struct device_attribute *attr,
>> +                                    const char *buf, size_t count)
>> +{
>> +       struct vdpa_device *vdev = dev_to_vdpa(dev);
>> +       const char *driver_override, *old;
>> +       char *cp;
>> +
>> +       /* We need to keep extra room for a newline */
>> +       if (count >= (PAGE_SIZE - 1))
>> +               return -EINVAL;
>> +
>> +       driver_override = kstrndup(buf, count, GFP_KERNEL);
>> +       if (!driver_override)
>> +               return -ENOMEM;
>> +
>> +       cp = strchr(driver_override, '\n');
>> +       if (cp)
>> +               *cp = '\0';
>> +
>> +       device_lock(dev);
>> +       old = vdev->driver_override;
>> +       if (strlen(driver_override)) {
>> +               vdev->driver_override = driver_override;
>> +       } else {
>> +               kfree(driver_override);
>> +               vdev->driver_override = NULL;
>> +       }
>> +       device_unlock(dev);
>> +
>> +       kfree(old);
>> +
>> +       return count;
>> +}
>> +
>> +static ssize_t driver_override_show(struct device *dev,
>> +                                   struct device_attribute *attr, char *buf)
>> +{
>> +       struct vdpa_device *vdev = dev_to_vdpa(dev);
>> +       ssize_t len;
>> +
>> +       device_lock(dev);
>> +       len = snprintf(buf, PAGE_SIZE, "%s\n", vdev->driver_override);
>> +       device_unlock(dev);
>> +
>> +       return len;
>> +}
>> +static DEVICE_ATTR_RW(driver_override);
>> +
>> +static struct attribute *vdpa_dev_attrs[] = {
>> +       &dev_attr_driver_override.attr,
>> +       NULL,
>> +};
>> +
>> +static const struct attribute_group vdpa_dev_group = {
>> +       .attrs  = vdpa_dev_attrs,
>> +};
>> +__ATTRIBUTE_GROUPS(vdpa_dev);
>> +
>>  static struct bus_type vdpa_bus = {
>>         .name  = "vdpa",
>> +       .dev_groups = vdpa_dev_groups,
>
>This reminds me that we probably need to document the sysfs interface
>in Documentation/ABI/testing/sysfs-bus-vdpa.
>

Yeah, I'd like to add more documentation. We should also document the 
device life cycle and management API.

It's on my to-do list, I'm prioritizing it!

>But it's not the fault of this patch which look good.

Thanks,
Stefano


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] vdpa: add driver_override support
  2021-11-05  8:04   ` Stefano Garzarella
@ 2021-11-05  8:26     ` Jason Wang
  2021-11-05  8:31       ` Stefano Garzarella
  0 siblings, 1 reply; 8+ messages in thread
From: Jason Wang @ 2021-11-05  8:26 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: virtualization, Parav Pandit, Michael S. Tsirkin, linux-kernel

On Fri, Nov 5, 2021 at 4:05 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Fri, Nov 05, 2021 at 11:01:30AM +0800, Jason Wang wrote:
> >On Fri, Nov 5, 2021 at 12:17 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
> >>
> >> `driver_override` allows to control which of the vDPA bus drivers
> >> binds to a vDPA device.
> >>
> >> If `driver_override` is not set, the previous behaviour is followed:
> >> devices use the first vDPA bus driver loaded (unless auto binding
> >> is disabled).
> >>
> >> Tested on Fedora 34 with driverctl(8):
> >>   $ modprobe virtio-vdpa
> >>   $ modprobe vhost-vdpa
> >>   $ modprobe vdpa-sim-net
> >>
> >>   $ vdpa dev add mgmtdev vdpasim_net name dev1
> >>
> >>   # dev1 is attached to the first vDPA bus driver loaded
> >>   $ driverctl -b vdpa list-devices
> >>     dev1 virtio_vdpa
> >>
> >>   $ driverctl -b vdpa set-override dev1 vhost_vdpa
> >>
> >>   $ driverctl -b vdpa list-devices
> >>     dev1 vhost_vdpa [*]
> >>
> >>   Note: driverctl(8) integrates with udev so the binding is
> >>   preserved.
> >>
> >> Suggested-by: Jason Wang <jasowang@redhat.com>
> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> >> ---
> >>  include/linux/vdpa.h |  2 ++
> >>  drivers/vdpa/vdpa.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 76 insertions(+)
> >>
> >> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> >> index c3011ccda430..ae34015b37b7 100644
> >> --- a/include/linux/vdpa.h
> >> +++ b/include/linux/vdpa.h
> >> @@ -64,6 +64,7 @@ struct vdpa_mgmt_dev;
> >>   * struct vdpa_device - representation of a vDPA device
> >>   * @dev: underlying device
> >>   * @dma_dev: the actual device that is performing DMA
> >> + * @driver_override: driver name to force a match
> >
> >This seems useless?
>
> I'm a bit lost, do you mean we should remove the documentation of
> `driver_override`?

I misread the code which was misled by vdpa_mgmt_dev above:(

The code should be fine.

So:

Acked-by: Jason Wang <jasowang@redhat.com>

>
> >
> >>   * @config: the configuration ops for this device.
> >>   * @cf_mutex: Protects get and set access to configuration layout.
> >>   * @index: device index
> >> @@ -76,6 +77,7 @@ struct vdpa_mgmt_dev;
> >>  struct vdpa_device {
> >>         struct device dev;
> >>         struct device *dma_dev;
> >> +       const char *driver_override;
> >>         const struct vdpa_config_ops *config;
> >>         struct mutex cf_mutex; /* Protects get/set config */
> >>         unsigned int index;
> >> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> >> index 7332a74a4b00..659231bbfee8 100644
> >> --- a/drivers/vdpa/vdpa.c
> >> +++ b/drivers/vdpa/vdpa.c
> >> @@ -52,8 +52,81 @@ static void vdpa_dev_remove(struct device *d)
> >>                 drv->remove(vdev);
> >>  }
> >>
> >> +static int vdpa_dev_match(struct device *dev, struct device_driver *drv)
> >> +{
> >> +       struct vdpa_device *vdev = dev_to_vdpa(dev);
> >> +
> >> +       /* Check override first, and if set, only use the named driver */
> >> +       if (vdev->driver_override)
> >> +               return strcmp(vdev->driver_override, drv->name) == 0;
> >> +
> >> +       /* Currently devices must be supported by all vDPA bus
> >> drivers */
> >> +       return 1;
> >> +}
> >> +
> >> +static ssize_t driver_override_store(struct device *dev,
> >> +                                    struct device_attribute *attr,
> >> +                                    const char *buf, size_t count)
> >> +{
> >> +       struct vdpa_device *vdev = dev_to_vdpa(dev);
> >> +       const char *driver_override, *old;
> >> +       char *cp;
> >> +
> >> +       /* We need to keep extra room for a newline */
> >> +       if (count >= (PAGE_SIZE - 1))
> >> +               return -EINVAL;
> >> +
> >> +       driver_override = kstrndup(buf, count, GFP_KERNEL);
> >> +       if (!driver_override)
> >> +               return -ENOMEM;
> >> +
> >> +       cp = strchr(driver_override, '\n');
> >> +       if (cp)
> >> +               *cp = '\0';
> >> +
> >> +       device_lock(dev);
> >> +       old = vdev->driver_override;
> >> +       if (strlen(driver_override)) {
> >> +               vdev->driver_override = driver_override;
> >> +       } else {
> >> +               kfree(driver_override);
> >> +               vdev->driver_override = NULL;
> >> +       }
> >> +       device_unlock(dev);
> >> +
> >> +       kfree(old);
> >> +
> >> +       return count;
> >> +}
> >> +
> >> +static ssize_t driver_override_show(struct device *dev,
> >> +                                   struct device_attribute *attr, char *buf)
> >> +{
> >> +       struct vdpa_device *vdev = dev_to_vdpa(dev);
> >> +       ssize_t len;
> >> +
> >> +       device_lock(dev);
> >> +       len = snprintf(buf, PAGE_SIZE, "%s\n", vdev->driver_override);
> >> +       device_unlock(dev);
> >> +
> >> +       return len;
> >> +}
> >> +static DEVICE_ATTR_RW(driver_override);
> >> +
> >> +static struct attribute *vdpa_dev_attrs[] = {
> >> +       &dev_attr_driver_override.attr,
> >> +       NULL,
> >> +};
> >> +
> >> +static const struct attribute_group vdpa_dev_group = {
> >> +       .attrs  = vdpa_dev_attrs,
> >> +};
> >> +__ATTRIBUTE_GROUPS(vdpa_dev);
> >> +
> >>  static struct bus_type vdpa_bus = {
> >>         .name  = "vdpa",
> >> +       .dev_groups = vdpa_dev_groups,
> >
> >This reminds me that we probably need to document the sysfs interface
> >in Documentation/ABI/testing/sysfs-bus-vdpa.
> >
>
> Yeah, I'd like to add more documentation. We should also document the
> device life cycle and management API.
>
> It's on my to-do list, I'm prioritizing it!
>
> >But it's not the fault of this patch which look good.
>
> Thanks,
> Stefano
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] vdpa: add driver_override support
  2021-11-05  8:26     ` Jason Wang
@ 2021-11-05  8:31       ` Stefano Garzarella
  0 siblings, 0 replies; 8+ messages in thread
From: Stefano Garzarella @ 2021-11-05  8:31 UTC (permalink / raw)
  To: Jason Wang; +Cc: virtualization, Parav Pandit, Michael S. Tsirkin, linux-kernel

On Fri, Nov 05, 2021 at 04:26:44PM +0800, Jason Wang wrote:
>On Fri, Nov 5, 2021 at 4:05 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>>
>> On Fri, Nov 05, 2021 at 11:01:30AM +0800, Jason Wang wrote:
>> >On Fri, Nov 5, 2021 at 12:17 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>> >>
>> >> `driver_override` allows to control which of the vDPA bus drivers
>> >> binds to a vDPA device.
>> >>
>> >> If `driver_override` is not set, the previous behaviour is followed:
>> >> devices use the first vDPA bus driver loaded (unless auto binding
>> >> is disabled).
>> >>
>> >> Tested on Fedora 34 with driverctl(8):
>> >>   $ modprobe virtio-vdpa
>> >>   $ modprobe vhost-vdpa
>> >>   $ modprobe vdpa-sim-net
>> >>
>> >>   $ vdpa dev add mgmtdev vdpasim_net name dev1
>> >>
>> >>   # dev1 is attached to the first vDPA bus driver loaded
>> >>   $ driverctl -b vdpa list-devices
>> >>     dev1 virtio_vdpa
>> >>
>> >>   $ driverctl -b vdpa set-override dev1 vhost_vdpa
>> >>
>> >>   $ driverctl -b vdpa list-devices
>> >>     dev1 vhost_vdpa [*]
>> >>
>> >>   Note: driverctl(8) integrates with udev so the binding is
>> >>   preserved.
>> >>
>> >> Suggested-by: Jason Wang <jasowang@redhat.com>
>> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> >> ---
>> >>  include/linux/vdpa.h |  2 ++
>> >>  drivers/vdpa/vdpa.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++
>> >>  2 files changed, 76 insertions(+)
>> >>
>> >> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
>> >> index c3011ccda430..ae34015b37b7 100644
>> >> --- a/include/linux/vdpa.h
>> >> +++ b/include/linux/vdpa.h
>> >> @@ -64,6 +64,7 @@ struct vdpa_mgmt_dev;
>> >>   * struct vdpa_device - representation of a vDPA device
>> >>   * @dev: underlying device
>> >>   * @dma_dev: the actual device that is performing DMA
>> >> + * @driver_override: driver name to force a match
>> >
>> >This seems useless?
>>
>> I'm a bit lost, do you mean we should remove the documentation of
>> `driver_override`?
>
>I misread the code which was misled by vdpa_mgmt_dev above:(

Yeah, the same thing happened to me now while double checking ;-)

>
>The code should be fine.
>
>So:
>
>Acked-by: Jason Wang <jasowang@redhat.com>
>

Thanks,
Stefano


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] vdpa: add driver_override support
  2021-11-05  3:01 ` Jason Wang
  2021-11-05  8:04   ` Stefano Garzarella
@ 2021-11-08 17:05   ` Stefano Garzarella
  2021-11-09 13:10     ` Michael S. Tsirkin
  1 sibling, 1 reply; 8+ messages in thread
From: Stefano Garzarella @ 2021-11-08 17:05 UTC (permalink / raw)
  To: Jason Wang, Michael S. Tsirkin; +Cc: virtualization, Parav Pandit, linux-kernel

On Fri, Nov 5, 2021 at 4:01 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Fri, Nov 5, 2021 at 12:17 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
> >
> > `driver_override` allows to control which of the vDPA bus drivers
> > binds to a vDPA device.
> >
> > If `driver_override` is not set, the previous behaviour is followed:
> > devices use the first vDPA bus driver loaded (unless auto binding
> > is disabled).
> >
> > Tested on Fedora 34 with driverctl(8):
> >   $ modprobe virtio-vdpa
> >   $ modprobe vhost-vdpa
> >   $ modprobe vdpa-sim-net
> >
> >   $ vdpa dev add mgmtdev vdpasim_net name dev1
> >
> >   # dev1 is attached to the first vDPA bus driver loaded
> >   $ driverctl -b vdpa list-devices
> >     dev1 virtio_vdpa
> >
> >   $ driverctl -b vdpa set-override dev1 vhost_vdpa
> >
> >   $ driverctl -b vdpa list-devices
> >     dev1 vhost_vdpa [*]
> >
> >   Note: driverctl(8) integrates with udev so the binding is
> >   preserved.
> >
> > Suggested-by: Jason Wang <jasowang@redhat.com>
> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > ---
> >  include/linux/vdpa.h |  2 ++
> >  drivers/vdpa/vdpa.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 76 insertions(+)
> >
> > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> > index c3011ccda430..ae34015b37b7 100644
> > --- a/include/linux/vdpa.h
> > +++ b/include/linux/vdpa.h
> > @@ -64,6 +64,7 @@ struct vdpa_mgmt_dev;
> >   * struct vdpa_device - representation of a vDPA device
> >   * @dev: underlying device
> >   * @dma_dev: the actual device that is performing DMA
> > + * @driver_override: driver name to force a match
>
> This seems useless?
>
> >   * @config: the configuration ops for this device.
> >   * @cf_mutex: Protects get and set access to configuration layout.
> >   * @index: device index
> > @@ -76,6 +77,7 @@ struct vdpa_mgmt_dev;
> >  struct vdpa_device {
> >         struct device dev;
> >         struct device *dma_dev;
> > +       const char *driver_override;
> >         const struct vdpa_config_ops *config;
> >         struct mutex cf_mutex; /* Protects get/set config */
> >         unsigned int index;
> > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> > index 7332a74a4b00..659231bbfee8 100644
> > --- a/drivers/vdpa/vdpa.c
> > +++ b/drivers/vdpa/vdpa.c
> > @@ -52,8 +52,81 @@ static void vdpa_dev_remove(struct device *d)
> >                 drv->remove(vdev);
> >  }
> >
> > +static int vdpa_dev_match(struct device *dev, struct device_driver *drv)
> > +{
> > +       struct vdpa_device *vdev = dev_to_vdpa(dev);
> > +
> > +       /* Check override first, and if set, only use the named driver */
> > +       if (vdev->driver_override)
> > +               return strcmp(vdev->driver_override, drv->name) == 0;
> > +
> > +       /* Currently devices must be supported by all vDPA bus drivers */
> > +       return 1;
> > +}
> > +
> > +static ssize_t driver_override_store(struct device *dev,
> > +                                    struct device_attribute *attr,
> > +                                    const char *buf, size_t count)
> > +{
> > +       struct vdpa_device *vdev = dev_to_vdpa(dev);
> > +       const char *driver_override, *old;
> > +       char *cp;
> > +
> > +       /* We need to keep extra room for a newline */
> > +       if (count >= (PAGE_SIZE - 1))
> > +               return -EINVAL;
> > +
> > +       driver_override = kstrndup(buf, count, GFP_KERNEL);
> > +       if (!driver_override)
> > +               return -ENOMEM;
> > +
> > +       cp = strchr(driver_override, '\n');
> > +       if (cp)
> > +               *cp = '\0';
> > +
> > +       device_lock(dev);
> > +       old = vdev->driver_override;
> > +       if (strlen(driver_override)) {
> > +               vdev->driver_override = driver_override;
> > +       } else {
> > +               kfree(driver_override);
> > +               vdev->driver_override = NULL;
> > +       }
> > +       device_unlock(dev);
> > +
> > +       kfree(old);
> > +
> > +       return count;
> > +}
> > +
> > +static ssize_t driver_override_show(struct device *dev,
> > +                                   struct device_attribute *attr, char *buf)
> > +{
> > +       struct vdpa_device *vdev = dev_to_vdpa(dev);
> > +       ssize_t len;
> > +
> > +       device_lock(dev);
> > +       len = snprintf(buf, PAGE_SIZE, "%s\n", vdev->driver_override);
> > +       device_unlock(dev);
> > +
> > +       return len;
> > +}
> > +static DEVICE_ATTR_RW(driver_override);
> > +
> > +static struct attribute *vdpa_dev_attrs[] = {
> > +       &dev_attr_driver_override.attr,
> > +       NULL,
> > +};
> > +
> > +static const struct attribute_group vdpa_dev_group = {
> > +       .attrs  = vdpa_dev_attrs,
> > +};
> > +__ATTRIBUTE_GROUPS(vdpa_dev);
> > +
> >  static struct bus_type vdpa_bus = {
> >         .name  = "vdpa",
> > +       .dev_groups = vdpa_dev_groups,
>
> This reminds me that we probably need to document the sysfs interface
> in Documentation/ABI/testing/sysfs-bus-vdpa.
>
> But it's not the fault of this patch which look good.

Michael, Jason, about Documentation/ABI/testing/sysfs-bus-vdpa, do you 
think is better to send a follow up patch/series, maybe including also 
others entries (e.g. bind, unbind) or a v2 including the documentation 
of `driver_ovveride`?

Thanks,
Stefano


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] vdpa: add driver_override support
  2021-11-08 17:05   ` Stefano Garzarella
@ 2021-11-09 13:10     ` Michael S. Tsirkin
  2021-11-09 13:31       ` Stefano Garzarella
  0 siblings, 1 reply; 8+ messages in thread
From: Michael S. Tsirkin @ 2021-11-09 13:10 UTC (permalink / raw)
  To: Stefano Garzarella; +Cc: Jason Wang, virtualization, Parav Pandit, linux-kernel

On Mon, Nov 08, 2021 at 06:05:29PM +0100, Stefano Garzarella wrote:
> On Fri, Nov 5, 2021 at 4:01 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Fri, Nov 5, 2021 at 12:17 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
> > >
> > > `driver_override` allows to control which of the vDPA bus drivers
> > > binds to a vDPA device.
> > >
> > > If `driver_override` is not set, the previous behaviour is followed:
> > > devices use the first vDPA bus driver loaded (unless auto binding
> > > is disabled).
> > >
> > > Tested on Fedora 34 with driverctl(8):
> > >   $ modprobe virtio-vdpa
> > >   $ modprobe vhost-vdpa
> > >   $ modprobe vdpa-sim-net
> > >
> > >   $ vdpa dev add mgmtdev vdpasim_net name dev1
> > >
> > >   # dev1 is attached to the first vDPA bus driver loaded
> > >   $ driverctl -b vdpa list-devices
> > >     dev1 virtio_vdpa
> > >
> > >   $ driverctl -b vdpa set-override dev1 vhost_vdpa
> > >
> > >   $ driverctl -b vdpa list-devices
> > >     dev1 vhost_vdpa [*]
> > >
> > >   Note: driverctl(8) integrates with udev so the binding is
> > >   preserved.
> > >
> > > Suggested-by: Jason Wang <jasowang@redhat.com>
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > ---
> > >  include/linux/vdpa.h |  2 ++
> > >  drivers/vdpa/vdpa.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 76 insertions(+)
> > >
> > > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> > > index c3011ccda430..ae34015b37b7 100644
> > > --- a/include/linux/vdpa.h
> > > +++ b/include/linux/vdpa.h
> > > @@ -64,6 +64,7 @@ struct vdpa_mgmt_dev;
> > >   * struct vdpa_device - representation of a vDPA device
> > >   * @dev: underlying device
> > >   * @dma_dev: the actual device that is performing DMA
> > > + * @driver_override: driver name to force a match
> >
> > This seems useless?
> >
> > >   * @config: the configuration ops for this device.
> > >   * @cf_mutex: Protects get and set access to configuration layout.
> > >   * @index: device index
> > > @@ -76,6 +77,7 @@ struct vdpa_mgmt_dev;
> > >  struct vdpa_device {
> > >         struct device dev;
> > >         struct device *dma_dev;
> > > +       const char *driver_override;
> > >         const struct vdpa_config_ops *config;
> > >         struct mutex cf_mutex; /* Protects get/set config */
> > >         unsigned int index;
> > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> > > index 7332a74a4b00..659231bbfee8 100644
> > > --- a/drivers/vdpa/vdpa.c
> > > +++ b/drivers/vdpa/vdpa.c
> > > @@ -52,8 +52,81 @@ static void vdpa_dev_remove(struct device *d)
> > >                 drv->remove(vdev);
> > >  }
> > >
> > > +static int vdpa_dev_match(struct device *dev, struct device_driver *drv)
> > > +{
> > > +       struct vdpa_device *vdev = dev_to_vdpa(dev);
> > > +
> > > +       /* Check override first, and if set, only use the named driver */
> > > +       if (vdev->driver_override)
> > > +               return strcmp(vdev->driver_override, drv->name) == 0;
> > > +
> > > +       /* Currently devices must be supported by all vDPA bus drivers */
> > > +       return 1;
> > > +}
> > > +
> > > +static ssize_t driver_override_store(struct device *dev,
> > > +                                    struct device_attribute *attr,
> > > +                                    const char *buf, size_t count)
> > > +{
> > > +       struct vdpa_device *vdev = dev_to_vdpa(dev);
> > > +       const char *driver_override, *old;
> > > +       char *cp;
> > > +
> > > +       /* We need to keep extra room for a newline */
> > > +       if (count >= (PAGE_SIZE - 1))
> > > +               return -EINVAL;
> > > +
> > > +       driver_override = kstrndup(buf, count, GFP_KERNEL);
> > > +       if (!driver_override)
> > > +               return -ENOMEM;
> > > +
> > > +       cp = strchr(driver_override, '\n');
> > > +       if (cp)
> > > +               *cp = '\0';
> > > +
> > > +       device_lock(dev);
> > > +       old = vdev->driver_override;
> > > +       if (strlen(driver_override)) {
> > > +               vdev->driver_override = driver_override;
> > > +       } else {
> > > +               kfree(driver_override);
> > > +               vdev->driver_override = NULL;
> > > +       }
> > > +       device_unlock(dev);
> > > +
> > > +       kfree(old);
> > > +
> > > +       return count;
> > > +}
> > > +
> > > +static ssize_t driver_override_show(struct device *dev,
> > > +                                   struct device_attribute *attr, char *buf)
> > > +{
> > > +       struct vdpa_device *vdev = dev_to_vdpa(dev);
> > > +       ssize_t len;
> > > +
> > > +       device_lock(dev);
> > > +       len = snprintf(buf, PAGE_SIZE, "%s\n", vdev->driver_override);
> > > +       device_unlock(dev);
> > > +
> > > +       return len;
> > > +}
> > > +static DEVICE_ATTR_RW(driver_override);
> > > +
> > > +static struct attribute *vdpa_dev_attrs[] = {
> > > +       &dev_attr_driver_override.attr,
> > > +       NULL,
> > > +};
> > > +
> > > +static const struct attribute_group vdpa_dev_group = {
> > > +       .attrs  = vdpa_dev_attrs,
> > > +};
> > > +__ATTRIBUTE_GROUPS(vdpa_dev);
> > > +
> > >  static struct bus_type vdpa_bus = {
> > >         .name  = "vdpa",
> > > +       .dev_groups = vdpa_dev_groups,
> >
> > This reminds me that we probably need to document the sysfs interface
> > in Documentation/ABI/testing/sysfs-bus-vdpa.
> >
> > But it's not the fault of this patch which look good.
> 
> Michael, Jason, about Documentation/ABI/testing/sysfs-bus-vdpa, do you 
> think is better to send a follow up patch/series, maybe including also 
> others entries (e.g. bind, unbind) or a v2 including the documentation 
> of `driver_ovveride`?
> 
> Thanks,
> Stefano

I'd include it as a patch in series.

-- 
MST


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] vdpa: add driver_override support
  2021-11-09 13:10     ` Michael S. Tsirkin
@ 2021-11-09 13:31       ` Stefano Garzarella
  0 siblings, 0 replies; 8+ messages in thread
From: Stefano Garzarella @ 2021-11-09 13:31 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Jason Wang, virtualization, Parav Pandit, linux-kernel

On Tue, Nov 09, 2021 at 08:10:58AM -0500, Michael S. Tsirkin wrote:
>On Mon, Nov 08, 2021 at 06:05:29PM +0100, Stefano Garzarella wrote:
>> On Fri, Nov 5, 2021 at 4:01 AM Jason Wang <jasowang@redhat.com> wrote:
>> >
>> > On Fri, Nov 5, 2021 at 12:17 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>> > >
>> > > `driver_override` allows to control which of the vDPA bus drivers
>> > > binds to a vDPA device.
>> > >
>> > > If `driver_override` is not set, the previous behaviour is followed:
>> > > devices use the first vDPA bus driver loaded (unless auto binding
>> > > is disabled).
>> > >
>> > > Tested on Fedora 34 with driverctl(8):
>> > >   $ modprobe virtio-vdpa
>> > >   $ modprobe vhost-vdpa
>> > >   $ modprobe vdpa-sim-net
>> > >
>> > >   $ vdpa dev add mgmtdev vdpasim_net name dev1
>> > >
>> > >   # dev1 is attached to the first vDPA bus driver loaded
>> > >   $ driverctl -b vdpa list-devices
>> > >     dev1 virtio_vdpa
>> > >
>> > >   $ driverctl -b vdpa set-override dev1 vhost_vdpa
>> > >
>> > >   $ driverctl -b vdpa list-devices
>> > >     dev1 vhost_vdpa [*]
>> > >
>> > >   Note: driverctl(8) integrates with udev so the binding is
>> > >   preserved.
>> > >
>> > > Suggested-by: Jason Wang <jasowang@redhat.com>
>> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> > > ---
>> > >  include/linux/vdpa.h |  2 ++
>> > >  drivers/vdpa/vdpa.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++
>> > >  2 files changed, 76 insertions(+)
>> > >
>> > > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
>> > > index c3011ccda430..ae34015b37b7 100644
>> > > --- a/include/linux/vdpa.h
>> > > +++ b/include/linux/vdpa.h
>> > > @@ -64,6 +64,7 @@ struct vdpa_mgmt_dev;
>> > >   * struct vdpa_device - representation of a vDPA device
>> > >   * @dev: underlying device
>> > >   * @dma_dev: the actual device that is performing DMA
>> > > + * @driver_override: driver name to force a match
>> >
>> > This seems useless?
>> >
>> > >   * @config: the configuration ops for this device.
>> > >   * @cf_mutex: Protects get and set access to configuration layout.
>> > >   * @index: device index
>> > > @@ -76,6 +77,7 @@ struct vdpa_mgmt_dev;
>> > >  struct vdpa_device {
>> > >         struct device dev;
>> > >         struct device *dma_dev;
>> > > +       const char *driver_override;
>> > >         const struct vdpa_config_ops *config;
>> > >         struct mutex cf_mutex; /* Protects get/set config */
>> > >         unsigned int index;
>> > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
>> > > index 7332a74a4b00..659231bbfee8 100644
>> > > --- a/drivers/vdpa/vdpa.c
>> > > +++ b/drivers/vdpa/vdpa.c
>> > > @@ -52,8 +52,81 @@ static void vdpa_dev_remove(struct device *d)
>> > >                 drv->remove(vdev);
>> > >  }
>> > >
>> > > +static int vdpa_dev_match(struct device *dev, struct device_driver *drv)
>> > > +{
>> > > +       struct vdpa_device *vdev = dev_to_vdpa(dev);
>> > > +
>> > > +       /* Check override first, and if set, only use the named driver */
>> > > +       if (vdev->driver_override)
>> > > +               return strcmp(vdev->driver_override, drv->name) == 0;
>> > > +
>> > > +       /* Currently devices must be supported by all vDPA bus drivers */
>> > > +       return 1;
>> > > +}
>> > > +
>> > > +static ssize_t driver_override_store(struct device *dev,
>> > > +                                    struct device_attribute *attr,
>> > > +                                    const char *buf, size_t count)
>> > > +{
>> > > +       struct vdpa_device *vdev = dev_to_vdpa(dev);
>> > > +       const char *driver_override, *old;
>> > > +       char *cp;
>> > > +
>> > > +       /* We need to keep extra room for a newline */
>> > > +       if (count >= (PAGE_SIZE - 1))
>> > > +               return -EINVAL;
>> > > +
>> > > +       driver_override = kstrndup(buf, count, GFP_KERNEL);
>> > > +       if (!driver_override)
>> > > +               return -ENOMEM;
>> > > +
>> > > +       cp = strchr(driver_override, '\n');
>> > > +       if (cp)
>> > > +               *cp = '\0';
>> > > +
>> > > +       device_lock(dev);
>> > > +       old = vdev->driver_override;
>> > > +       if (strlen(driver_override)) {
>> > > +               vdev->driver_override = driver_override;
>> > > +       } else {
>> > > +               kfree(driver_override);
>> > > +               vdev->driver_override = NULL;
>> > > +       }
>> > > +       device_unlock(dev);
>> > > +
>> > > +       kfree(old);
>> > > +
>> > > +       return count;
>> > > +}
>> > > +
>> > > +static ssize_t driver_override_show(struct device *dev,
>> > > +                                   struct device_attribute *attr, char *buf)
>> > > +{
>> > > +       struct vdpa_device *vdev = dev_to_vdpa(dev);
>> > > +       ssize_t len;
>> > > +
>> > > +       device_lock(dev);
>> > > +       len = snprintf(buf, PAGE_SIZE, "%s\n", vdev->driver_override);
>> > > +       device_unlock(dev);
>> > > +
>> > > +       return len;
>> > > +}
>> > > +static DEVICE_ATTR_RW(driver_override);
>> > > +
>> > > +static struct attribute *vdpa_dev_attrs[] = {
>> > > +       &dev_attr_driver_override.attr,
>> > > +       NULL,
>> > > +};
>> > > +
>> > > +static const struct attribute_group vdpa_dev_group = {
>> > > +       .attrs  = vdpa_dev_attrs,
>> > > +};
>> > > +__ATTRIBUTE_GROUPS(vdpa_dev);
>> > > +
>> > >  static struct bus_type vdpa_bus = {
>> > >         .name  = "vdpa",
>> > > +       .dev_groups = vdpa_dev_groups,
>> >
>> > This reminds me that we probably need to document the sysfs interface
>> > in Documentation/ABI/testing/sysfs-bus-vdpa.
>> >
>> > But it's not the fault of this patch which look good.
>>
>> Michael, Jason, about Documentation/ABI/testing/sysfs-bus-vdpa, do you
>> think is better to send a follow up patch/series, maybe including also
>> others entries (e.g. bind, unbind) or a v2 including the documentation
>> of `driver_ovveride`?
>>
>> Thanks,
>> Stefano
>
>I'd include it as a patch in series.
>

Okay, so I'll send the documentation in a followup series.

Thanks,
Stefano


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-11-09 13:32 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-04 16:17 [PATCH] vdpa: add driver_override support Stefano Garzarella
2021-11-05  3:01 ` Jason Wang
2021-11-05  8:04   ` Stefano Garzarella
2021-11-05  8:26     ` Jason Wang
2021-11-05  8:31       ` Stefano Garzarella
2021-11-08 17:05   ` Stefano Garzarella
2021-11-09 13:10     ` Michael S. Tsirkin
2021-11-09 13:31       ` Stefano Garzarella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).