| Message ID | 20240415162530.3594670-2-jiri@resnulli.us (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | selftests: virtio_net: introduce initial testing infrastructure | expand |
On Tue, Apr 16, 2024 at 12:25 AM Jiri Pirko <jiri@resnulli.us> wrote: > > From: Jiri Pirko <jiri@nvidia.com> > > Currently there is no way for user to set what features the driver > should obey or not, it is hard wired in the code. > > In order to be able to debug the device behavior in case some feature is > disabled, introduce a debugfs infrastructure with couple of files > allowing user to see what features the device advertises and > to set filter for features used by driver. > > Example: > $cat /sys/bus/virtio/devices/virtio0/features > 1110010111111111111101010000110010000000100000000000000000000000 > $ echo "5" >/sys/kernel/debug/virtio/virtio0/filter_feature_add > $ cat /sys/kernel/debug/virtio/virtio0/filter_features > 5 > $ echo "virtio0" > /sys/bus/virtio/drivers/virtio_net/unbind > $ echo "virtio0" > /sys/bus/virtio/drivers/virtio_net/bind > $ cat /sys/bus/virtio/devices/virtio0/features > 1110000111111111111101010000110010000000100000000000000000000000 > > Note that sysfs "features" know already exists, this patch does not > touch it. > > Signed-off-by: Jiri Pirko <jiri@nvidia.com> > --- Note that this can be done already with vp_vdpa feature provisioning: commit c1ca352d371f724f7fb40f016abdb563aa85fe55 Author: Jason Wang <jasowang@redhat.com> Date: Tue Sep 27 15:48:10 2022 +0800 vp_vdpa: support feature provisioning For example: vdpa dev add name dev1 mgmtdev pci/0000:02:00.0 device_features 0x300020000 Thanks
Tue, Apr 16, 2024 at 05:52:41AM CEST, jasowang@redhat.com wrote: >On Tue, Apr 16, 2024 at 12:25 AM Jiri Pirko <jiri@resnulli.us> wrote: >> >> From: Jiri Pirko <jiri@nvidia.com> >> >> Currently there is no way for user to set what features the driver >> should obey or not, it is hard wired in the code. >> >> In order to be able to debug the device behavior in case some feature is >> disabled, introduce a debugfs infrastructure with couple of files >> allowing user to see what features the device advertises and >> to set filter for features used by driver. >> >> Example: >> $cat /sys/bus/virtio/devices/virtio0/features >> 1110010111111111111101010000110010000000100000000000000000000000 >> $ echo "5" >/sys/kernel/debug/virtio/virtio0/filter_feature_add >> $ cat /sys/kernel/debug/virtio/virtio0/filter_features >> 5 >> $ echo "virtio0" > /sys/bus/virtio/drivers/virtio_net/unbind >> $ echo "virtio0" > /sys/bus/virtio/drivers/virtio_net/bind >> $ cat /sys/bus/virtio/devices/virtio0/features >> 1110000111111111111101010000110010000000100000000000000000000000 >> >> Note that sysfs "features" know already exists, this patch does not >> touch it. >> >> Signed-off-by: Jiri Pirko <jiri@nvidia.com> >> --- > >Note that this can be done already with vp_vdpa feature provisioning: > >commit c1ca352d371f724f7fb40f016abdb563aa85fe55 >Author: Jason Wang <jasowang@redhat.com> >Date: Tue Sep 27 15:48:10 2022 +0800 > > vp_vdpa: support feature provisioning > >For example: > >vdpa dev add name dev1 mgmtdev pci/0000:02:00.0 device_features 0x300020000 Sure. My intension was to make the testing possible on any virtio device. Narrowing the testing for vpda would be limitting. > >Thanks >
On Tue, Apr 16, 2024 at 5:37 PM Jiri Pirko <jiri@resnulli.us> wrote: > > Tue, Apr 16, 2024 at 05:52:41AM CEST, jasowang@redhat.com wrote: > >On Tue, Apr 16, 2024 at 12:25 AM Jiri Pirko <jiri@resnulli.us> wrote: > >> > >> From: Jiri Pirko <jiri@nvidia.com> > >> > >> Currently there is no way for user to set what features the driver > >> should obey or not, it is hard wired in the code. > >> > >> In order to be able to debug the device behavior in case some feature is > >> disabled, introduce a debugfs infrastructure with couple of files > >> allowing user to see what features the device advertises and > >> to set filter for features used by driver. > >> > >> Example: > >> $cat /sys/bus/virtio/devices/virtio0/features > >> 1110010111111111111101010000110010000000100000000000000000000000 > >> $ echo "5" >/sys/kernel/debug/virtio/virtio0/filter_feature_add > >> $ cat /sys/kernel/debug/virtio/virtio0/filter_features > >> 5 > >> $ echo "virtio0" > /sys/bus/virtio/drivers/virtio_net/unbind > >> $ echo "virtio0" > /sys/bus/virtio/drivers/virtio_net/bind > >> $ cat /sys/bus/virtio/devices/virtio0/features > >> 1110000111111111111101010000110010000000100000000000000000000000 > >> > >> Note that sysfs "features" know already exists, this patch does not > >> touch it. > >> > >> Signed-off-by: Jiri Pirko <jiri@nvidia.com> > >> --- > > > >Note that this can be done already with vp_vdpa feature provisioning: > > > >commit c1ca352d371f724f7fb40f016abdb563aa85fe55 > >Author: Jason Wang <jasowang@redhat.com> > >Date: Tue Sep 27 15:48:10 2022 +0800 > > > > vp_vdpa: support feature provisioning > > > >For example: > > > >vdpa dev add name dev1 mgmtdev pci/0000:02:00.0 device_features 0x300020000 > > Sure. My intension was to make the testing possible on any virtio > device. It did that actually, vp_vdpa bridge virtio-pci device into vDPA bus with mediation layer (like feature filtering etc). So it can only run on top of standard virtio-pci device. > Narrowing the testing for vpda would be limitting. Unless you want to use other transport like virtio-mmio. Thanks > > > > > >Thanks > > >
Wed, Apr 17, 2024 at 06:37:30AM CEST, jasowang@redhat.com wrote: >On Tue, Apr 16, 2024 at 5:37 PM Jiri Pirko <jiri@resnulli.us> wrote: >> >> Tue, Apr 16, 2024 at 05:52:41AM CEST, jasowang@redhat.com wrote: >> >On Tue, Apr 16, 2024 at 12:25 AM Jiri Pirko <jiri@resnulli.us> wrote: >> >> >> >> From: Jiri Pirko <jiri@nvidia.com> >> >> >> >> Currently there is no way for user to set what features the driver >> >> should obey or not, it is hard wired in the code. >> >> >> >> In order to be able to debug the device behavior in case some feature is >> >> disabled, introduce a debugfs infrastructure with couple of files >> >> allowing user to see what features the device advertises and >> >> to set filter for features used by driver. >> >> >> >> Example: >> >> $cat /sys/bus/virtio/devices/virtio0/features >> >> 1110010111111111111101010000110010000000100000000000000000000000 >> >> $ echo "5" >/sys/kernel/debug/virtio/virtio0/filter_feature_add >> >> $ cat /sys/kernel/debug/virtio/virtio0/filter_features >> >> 5 >> >> $ echo "virtio0" > /sys/bus/virtio/drivers/virtio_net/unbind >> >> $ echo "virtio0" > /sys/bus/virtio/drivers/virtio_net/bind >> >> $ cat /sys/bus/virtio/devices/virtio0/features >> >> 1110000111111111111101010000110010000000100000000000000000000000 >> >> >> >> Note that sysfs "features" know already exists, this patch does not >> >> touch it. >> >> >> >> Signed-off-by: Jiri Pirko <jiri@nvidia.com> >> >> --- >> > >> >Note that this can be done already with vp_vdpa feature provisioning: >> > >> >commit c1ca352d371f724f7fb40f016abdb563aa85fe55 >> >Author: Jason Wang <jasowang@redhat.com> >> >Date: Tue Sep 27 15:48:10 2022 +0800 >> > >> > vp_vdpa: support feature provisioning >> > >> >For example: >> > >> >vdpa dev add name dev1 mgmtdev pci/0000:02:00.0 device_features 0x300020000 >> >> Sure. My intension was to make the testing possible on any virtio >> device. > >It did that actually, vp_vdpa bridge virtio-pci device into vDPA bus >with mediation layer (like feature filtering etc). So it can only run >on top of standard virtio-pci device. > >> Narrowing the testing for vpda would be limitting. > >Unless you want to use other transport like virtio-mmio. Also, the goal is to test virtio_net emulated devices. There are couple of implementation. Non-vdpa. > >Thanks > >> >> >> > >> >Thanks >> > >> >
On Wed, Apr 17, 2024 at 3:23 PM Jiri Pirko <jiri@resnulli.us> wrote: > > Wed, Apr 17, 2024 at 06:37:30AM CEST, jasowang@redhat.com wrote: > >On Tue, Apr 16, 2024 at 5:37 PM Jiri Pirko <jiri@resnulli.us> wrote: > >> > >> Tue, Apr 16, 2024 at 05:52:41AM CEST, jasowang@redhat.com wrote: > >> >On Tue, Apr 16, 2024 at 12:25 AM Jiri Pirko <jiri@resnulli.us> wrote: > >> >> > >> >> From: Jiri Pirko <jiri@nvidia.com> > >> >> > >> >> Currently there is no way for user to set what features the driver > >> >> should obey or not, it is hard wired in the code. > >> >> > >> >> In order to be able to debug the device behavior in case some feature is > >> >> disabled, introduce a debugfs infrastructure with couple of files > >> >> allowing user to see what features the device advertises and > >> >> to set filter for features used by driver. > >> >> > >> >> Example: > >> >> $cat /sys/bus/virtio/devices/virtio0/features > >> >> 1110010111111111111101010000110010000000100000000000000000000000 > >> >> $ echo "5" >/sys/kernel/debug/virtio/virtio0/filter_feature_add > >> >> $ cat /sys/kernel/debug/virtio/virtio0/filter_features > >> >> 5 > >> >> $ echo "virtio0" > /sys/bus/virtio/drivers/virtio_net/unbind > >> >> $ echo "virtio0" > /sys/bus/virtio/drivers/virtio_net/bind > >> >> $ cat /sys/bus/virtio/devices/virtio0/features > >> >> 1110000111111111111101010000110010000000100000000000000000000000 > >> >> > >> >> Note that sysfs "features" know already exists, this patch does not > >> >> touch it. > >> >> > >> >> Signed-off-by: Jiri Pirko <jiri@nvidia.com> > >> >> --- > >> > > >> >Note that this can be done already with vp_vdpa feature provisioning: > >> > > >> >commit c1ca352d371f724f7fb40f016abdb563aa85fe55 > >> >Author: Jason Wang <jasowang@redhat.com> > >> >Date: Tue Sep 27 15:48:10 2022 +0800 > >> > > >> > vp_vdpa: support feature provisioning > >> > > >> >For example: > >> > > >> >vdpa dev add name dev1 mgmtdev pci/0000:02:00.0 device_features 0x300020000 > >> > >> Sure. My intension was to make the testing possible on any virtio > >> device. > > > >It did that actually, vp_vdpa bridge virtio-pci device into vDPA bus > >with mediation layer (like feature filtering etc). So it can only run > >on top of standard virtio-pci device. > > > >> Narrowing the testing for vpda would be limitting. > > > >Unless you want to use other transport like virtio-mmio. > > Also, the goal is to test virtio_net emulated devices. > There are couple > of implementation. Non-vdpa. So what I want to say is, vp_vdpa works for all types of virtio-pci devices no matter if it is emulated or hardware. Thanks > > > > > >Thanks > > > >> > >> > >> > > >> >Thanks > >> > > >> > > >
Thu, Apr 18, 2024 at 02:59:41AM CEST, jasowang@redhat.com wrote: >On Wed, Apr 17, 2024 at 3:23 PM Jiri Pirko <jiri@resnulli.us> wrote: >> >> Wed, Apr 17, 2024 at 06:37:30AM CEST, jasowang@redhat.com wrote: >> >On Tue, Apr 16, 2024 at 5:37 PM Jiri Pirko <jiri@resnulli.us> wrote: >> >> >> >> Tue, Apr 16, 2024 at 05:52:41AM CEST, jasowang@redhat.com wrote: >> >> >On Tue, Apr 16, 2024 at 12:25 AM Jiri Pirko <jiri@resnulli.us> wrote: >> >> >> >> >> >> From: Jiri Pirko <jiri@nvidia.com> >> >> >> >> >> >> Currently there is no way for user to set what features the driver >> >> >> should obey or not, it is hard wired in the code. >> >> >> >> >> >> In order to be able to debug the device behavior in case some feature is >> >> >> disabled, introduce a debugfs infrastructure with couple of files >> >> >> allowing user to see what features the device advertises and >> >> >> to set filter for features used by driver. >> >> >> >> >> >> Example: >> >> >> $cat /sys/bus/virtio/devices/virtio0/features >> >> >> 1110010111111111111101010000110010000000100000000000000000000000 >> >> >> $ echo "5" >/sys/kernel/debug/virtio/virtio0/filter_feature_add >> >> >> $ cat /sys/kernel/debug/virtio/virtio0/filter_features >> >> >> 5 >> >> >> $ echo "virtio0" > /sys/bus/virtio/drivers/virtio_net/unbind >> >> >> $ echo "virtio0" > /sys/bus/virtio/drivers/virtio_net/bind >> >> >> $ cat /sys/bus/virtio/devices/virtio0/features >> >> >> 1110000111111111111101010000110010000000100000000000000000000000 >> >> >> >> >> >> Note that sysfs "features" know already exists, this patch does not >> >> >> touch it. >> >> >> >> >> >> Signed-off-by: Jiri Pirko <jiri@nvidia.com> >> >> >> --- >> >> > >> >> >Note that this can be done already with vp_vdpa feature provisioning: >> >> > >> >> >commit c1ca352d371f724f7fb40f016abdb563aa85fe55 >> >> >Author: Jason Wang <jasowang@redhat.com> >> >> >Date: Tue Sep 27 15:48:10 2022 +0800 >> >> > >> >> > vp_vdpa: support feature provisioning >> >> > >> >> >For example: >> >> > >> >> >vdpa dev add name dev1 mgmtdev pci/0000:02:00.0 device_features 0x300020000 >> >> >> >> Sure. My intension was to make the testing possible on any virtio >> >> device. >> > >> >It did that actually, vp_vdpa bridge virtio-pci device into vDPA bus >> >with mediation layer (like feature filtering etc). So it can only run >> >on top of standard virtio-pci device. >> > >> >> Narrowing the testing for vpda would be limitting. >> > >> >Unless you want to use other transport like virtio-mmio. >> >> Also, the goal is to test virtio_net emulated devices. >> There are couple >> of implementation. Non-vdpa. > >So what I want to say is, vp_vdpa works for all types of virtio-pci >devices no matter if it is emulated or hardware. Sure, but I wanted to have a simple generic way, working on all virtio devices, even the ones backed by a different transport, and without need of extra vdpa layer. > >Thanks > >> >> >> > >> >Thanks >> > >> >> >> >> >> >> > >> >> >Thanks >> >> > >> >> >> > >> >
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig index c17193544268..fc839a354958 100644 --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -178,4 +178,13 @@ config VIRTIO_DMA_SHARED_BUFFER This option adds a flavor of dma buffers that are backed by virtio resources. +config VIRTIO_DEBUG + bool "Debug facilities" + help + Enable this to expose debug facilities over debugfs. + This allows to debug features, to see what features the device + advertises and to set filter for features used by driver. + + If unsure, say N. + endif # VIRTIO_MENU diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile index 73ace62af440..58b2b0489fc9 100644 --- a/drivers/virtio/Makefile +++ b/drivers/virtio/Makefile @@ -13,3 +13,4 @@ obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o obj-$(CONFIG_VIRTIO_VDPA) += virtio_vdpa.o obj-$(CONFIG_VIRTIO_MEM) += virtio_mem.o obj-$(CONFIG_VIRTIO_DMA_SHARED_BUFFER) += virtio_dma_buf.o +obj-$(CONFIG_VIRTIO_DEBUG) += virtio_debug.o diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c index f173587893cb..8d9871145e28 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -274,6 +274,9 @@ static int virtio_dev_probe(struct device *_d) else dev->features = driver_features_legacy & device_features; + /* When debugging, user may filter some features by hand. */ + virtio_debug_device_filter_features(dev); + /* Transport features always preserved to pass to finalize_features. */ for (i = VIRTIO_TRANSPORT_F_START; i < VIRTIO_TRANSPORT_F_END; i++) if (device_features & (1ULL << i)) @@ -463,6 +466,8 @@ int register_virtio_device(struct virtio_device *dev) /* Acknowledge that we've seen the device. */ virtio_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE); + virtio_debug_device_init(dev); + /* * device_add() causes the bus infrastructure to look for a matching * driver. @@ -494,6 +499,7 @@ void unregister_virtio_device(struct virtio_device *dev) int index = dev->index; /* save for after device release */ device_unregister(&dev->dev); + virtio_debug_device_exit(dev); ida_free(&virtio_index_ida, index); } EXPORT_SYMBOL_GPL(unregister_virtio_device); @@ -588,11 +594,13 @@ static int virtio_init(void) { if (bus_register(&virtio_bus) != 0) panic("virtio bus registration failed"); + virtio_debug_init(); return 0; } static void __exit virtio_exit(void) { + virtio_debug_exit(); bus_unregister(&virtio_bus); ida_destroy(&virtio_index_ida); } diff --git a/drivers/virtio/virtio_debug.c b/drivers/virtio/virtio_debug.c new file mode 100644 index 000000000000..28cf30948939 --- /dev/null +++ b/drivers/virtio/virtio_debug.c @@ -0,0 +1,109 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +#include <linux/virtio.h> +#include <linux/virtio_config.h> +#include <linux/debugfs.h> + +static struct dentry *virtio_debugfs_dir; + +static int virtio_debug_device_features_show(struct seq_file *s, void *data) +{ + struct virtio_device *dev = s->private; + u64 device_features; + unsigned int i; + + device_features = dev->config->get_features(dev); + for (i = 0; i < BITS_PER_LONG_LONG; i++) { + if (device_features & (1ULL << i)) + seq_printf(s, "%u\n", i); + } + return 0; +} +DEFINE_SHOW_ATTRIBUTE(virtio_debug_device_features); + +static int virtio_debug_filter_features_show(struct seq_file *s, void *data) +{ + struct virtio_device *dev = s->private; + unsigned int i; + + for (i = 0; i < BITS_PER_LONG_LONG; i++) { + if (dev->debugfs_filter_features & (1ULL << i)) + seq_printf(s, "%u\n", i); + } + return 0; +} +DEFINE_SHOW_ATTRIBUTE(virtio_debug_filter_features); + +static int virtio_debug_filter_features_clear(void *data, u64 val) +{ + struct virtio_device *dev = data; + + if (val == 1) + dev->debugfs_filter_features = 0; + return 0; +} + +DEFINE_DEBUGFS_ATTRIBUTE(virtio_debug_filter_features_clear_fops, NULL, + virtio_debug_filter_features_clear, "%llu\n"); + +static int virtio_debug_filter_feature_add(void *data, u64 val) +{ + struct virtio_device *dev = data; + + if (val >= BITS_PER_LONG_LONG) + return -EINVAL; + dev->debugfs_filter_features |= BIT_ULL_MASK(val); + return 0; +} + +DEFINE_DEBUGFS_ATTRIBUTE(virtio_debug_filter_feature_add_fops, NULL, + virtio_debug_filter_feature_add, "%llu\n"); + +static int virtio_debug_filter_feature_del(void *data, u64 val) +{ + struct virtio_device *dev = data; + + if (val >= BITS_PER_LONG_LONG) + return -EINVAL; + dev->debugfs_filter_features &= ~BIT_ULL_MASK(val); + return 0; +} + +DEFINE_DEBUGFS_ATTRIBUTE(virtio_debug_filter_feature_del_fops, NULL, + virtio_debug_filter_feature_del, "%llu\n"); + +void virtio_debug_device_init(struct virtio_device *dev) +{ + dev->debugfs_dir = debugfs_create_dir(dev_name(&dev->dev), + virtio_debugfs_dir); + debugfs_create_file("device_features", 0400, dev->debugfs_dir, dev, + &virtio_debug_device_features_fops); + debugfs_create_file("filter_features", 0400, dev->debugfs_dir, dev, + &virtio_debug_filter_features_fops); + debugfs_create_file("filter_features_clear", 0200, dev->debugfs_dir, dev, + &virtio_debug_filter_features_clear_fops); + debugfs_create_file("filter_feature_add", 0200, dev->debugfs_dir, dev, + &virtio_debug_filter_feature_add_fops); + debugfs_create_file("filter_feature_del", 0200, dev->debugfs_dir, dev, + &virtio_debug_filter_feature_del_fops); +} + +void virtio_debug_device_filter_features(struct virtio_device *dev) +{ + dev->features &= ~dev->debugfs_filter_features; +} + +void virtio_debug_device_exit(struct virtio_device *dev) +{ + debugfs_remove_recursive(dev->debugfs_dir); +} + +void virtio_debug_init(void) +{ + virtio_debugfs_dir = debugfs_create_dir("virtio", NULL); +} + +void virtio_debug_exit(void) +{ + debugfs_remove_recursive(virtio_debugfs_dir); +} diff --git a/include/linux/virtio.h b/include/linux/virtio.h index b0201747a263..ab3f36c39686 100644 --- a/include/linux/virtio.h +++ b/include/linux/virtio.h @@ -126,6 +126,7 @@ struct virtio_admin_cmd { * @vqs: the list of virtqueues for this device. * @features: the features supported by both driver and device. * @priv: private pointer for the driver's use. + * @debugfs_dir: debugfs directory entry. */ struct virtio_device { int index; @@ -141,6 +142,10 @@ struct virtio_device { struct list_head vqs; u64 features; void *priv; +#ifdef CONFIG_VIRTIO_DEBUG + struct dentry *debugfs_dir; + u64 debugfs_filter_features; +#endif }; #define dev_to_virtio(_dev) container_of_const(_dev, struct virtio_device, dev) @@ -234,4 +239,33 @@ void virtqueue_dma_sync_single_range_for_cpu(struct virtqueue *_vq, dma_addr_t a void virtqueue_dma_sync_single_range_for_device(struct virtqueue *_vq, dma_addr_t addr, unsigned long offset, size_t size, enum dma_data_direction dir); + +#ifdef CONFIG_VIRTIO_DEBUG +void virtio_debug_device_init(struct virtio_device *dev); +void virtio_debug_device_exit(struct virtio_device *dev); +void virtio_debug_device_filter_features(struct virtio_device *dev); +void virtio_debug_init(void); +void virtio_debug_exit(void); +#else +static inline void virtio_debug_device_init(struct virtio_device *dev) +{ +} + +static inline void virtio_debug_device_exit(struct virtio_device *dev) +{ +} + +static inline void virtio_debug_device_filter_features(struct virtio_device *dev) +{ +} + +static inline void virtio_debug_init(void) +{ +} + +static inline void virtio_debug_exit(void) +{ +} +#endif + #endif /* _LINUX_VIRTIO_H */