Re: [PATCH] vdpa: Update man page with added support to configure max vq pair
On Tue, Mar 15, 2022 at 9:14 PM Eli Cohen wrote: > > Update man page to include information how to configure the max > virtqueue pairs for a vdpa device when creating one. > > Signed-off-by: Eli Cohen Acked-by: Jason Wang > --- > man/man8/vdpa-dev.8 | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/man/man8/vdpa-dev.8 b/man/man8/vdpa-dev.8 > index aa21ae3acbd8..432867c65182 100644 > --- a/man/man8/vdpa-dev.8 > +++ b/man/man8/vdpa-dev.8 > @@ -33,6 +33,7 @@ vdpa-dev \- vdpa device configuration > .I MGMTDEV > .RI "[ mac " MACADDR " ]" > .RI "[ mtu " MTU " ]" > +.RI "[ max_vqp " MAX_VQ_PAIRS " ]" > > .ti -8 > .B vdpa dev del > @@ -119,6 +120,11 @@ vdpa dev add name foo mgmtdev vdpa_sim_net mac > 00:11:22:33:44:55 mtu 9000 > Add the vdpa device named foo on the management device vdpa_sim_net with mac > address of 00:11:22:33:44:55 and mtu of 9000 bytes. > .RE > .PP > +vdpa dev add name foo mgmtdev auxiliary/mlx5_core.sf.1 mac 00:11:22:33:44:55 > max_vqp 8 > +.RS 4 > +Add the vdpa device named foo on the management device > auxiliary/mlx5_core.sf.1 with mac address of 00:11:22:33:44:55 and max 8 > virtqueue pairs > +.RE > +.PP > vdpa dev del foo > .RS 4 > Delete the vdpa device named foo which was previously created. > -- > 2.35.1 > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 0/9] treewide: eliminate anonymous module_init & module_exit
On Wed, Mar 16, 2022 at 12:20:01PM -0700, Randy Dunlap wrote: > There are a number of drivers that use "module_init(init)" and > "module_exit(exit)", which are anonymous names and can lead to > confusion or ambiguity when reading System.map, crashes/oops/bugs, > or an initcall_debug log. > > Give each of these init and exit functions unique driver-specific > names to eliminate the anonymous names. I'm not fully sure about the Fixes tags but I don't see that it hurts anything. For the series: Reviewed-by: Ira Weiny > > Example 1: (System.map) > 832fc78c t init > 832fc79e t init > 832fc8f8 t init > 832fca05 t init > 832fcbd2 t init > 83328f0e t init > 8332c5b1 t init > 8332d9eb t init > 8332f0aa t init > 83330e25 t init > 833317a5 t init > 8333dd6b t init > > Example 2: (initcall_debug log) > calling init+0x0/0x12 @ 1 > initcall init+0x0/0x12 returned 0 after 15 usecs > calling init+0x0/0x60 @ 1 > initcall init+0x0/0x60 returned 0 after 2 usecs > calling init+0x0/0x9a @ 1 > initcall init+0x0/0x9a returned 0 after 74 usecs > calling init+0x0/0x73 @ 1 > initcall init+0x0/0x73 returned 0 after 6 usecs > calling init+0x0/0x73 @ 1 > initcall init+0x0/0x73 returned 0 after 4 usecs > calling init+0x0/0xf5 @ 1 > initcall init+0x0/0xf5 returned 0 after 27 usecs > calling init+0x0/0x7d @ 1 > initcall init+0x0/0x7d returned 0 after 11 usecs > calling init+0x0/0xc9 @ 1 > initcall init+0x0/0xc9 returned 0 after 19 usecs > calling init+0x0/0x9d @ 1 > initcall init+0x0/0x9d returned 0 after 37 usecs > calling init+0x0/0x63f @ 1 > initcall init+0x0/0x63f returned 0 after 411 usecs > calling init+0x0/0x171 @ 1 > initcall init+0x0/0x171 returned 0 after 61 usecs > calling init+0x0/0xef @ 1 > initcall init+0x0/0xef returned 0 after 3 usecs > > Cc: "Michael S. Tsirkin" > Cc: Jason Wang > Cc: Paolo Bonzini > Cc: Stefan Hajnoczi > Cc: Jens Axboe > Cc: Amit Shah > Cc: Arnd Bergmann > Cc: Greg Kroah-Hartman > Cc: Eli Cohen > Cc: Saeed Mahameed > Cc: Leon Romanovsky > Cc: Pablo Neira Ayuso > Cc: Jozsef Kadlecsik > Cc: Florian Westphal > Cc: "David S. Miller" > Cc: Jakub Kicinski > Cc: "James E.J. Bottomley" > Cc: "Martin K. Petersen" > Cc: Felipe Balbi > Cc: Michał Mirosław > Cc: Sebastian Andrzej Siewior > Cc: Krzysztof Opasiak > Cc: Igor Kotrasinski > Cc: Valentina Manea > Cc: Shuah Khan > Cc: Shuah Khan > Cc: Jussi Kivilinna > Cc: Joachim Fritschi > Cc: Herbert Xu > Cc: Thomas Gleixner > Cc: Steven Rostedt > Cc: Ingo Molnar > Cc: Karol Herbst > Cc: Pekka Paalanen > Cc: Dave Hansen > Cc: Andy Lutomirski > Cc: Peter Zijlstra > Cc: Borislav Petkov > Cc: "H. Peter Anvin" > Cc: netfilter-de...@vger.kernel.org > Cc: coret...@netfilter.org > Cc: net...@vger.kernel.org > Cc: linux-bl...@vger.kernel.org > Cc: linux-cry...@vger.kernel.org > Cc: linux-r...@vger.kernel.org > Cc: linux-s...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Cc: nouv...@lists.freedesktop.org > Cc: virtualization@lists.linux-foundation.org > Cc: x...@kernel.org > > patches: > [PATCH 1/9] virtio_blk: eliminate anonymous module_init & module_exit > [PATCH 2/9] virtio_console: eliminate anonymous module_init & module_exit > [PATCH 3/9] net: mlx5: eliminate anonymous module_init & module_exit > [PATCH 4/9] netfilter: h323: eliminate anonymous module_init & module_exit > [PATCH 5/9] virtio-scsi: eliminate anonymous module_init & module_exit > [PATCH 6/9] usb: gadget: eliminate anonymous module_init & module_exit > [PATCH 7/9] usb: usbip: eliminate anonymous module_init & module_exit > [PATCH 8/9] x86/crypto: eliminate anonymous module_init & module_exit > [PATCH 9/9] testmmiotrace: eliminate anonymous module_init & module_exit > > diffstat: > arch/x86/crypto/blowfish_glue.c|8 > arch/x86/crypto/camellia_glue.c|8 > arch/x86/crypto/serpent_avx2_glue.c|8 > arch/x86/crypto/twofish_glue.c |8 > arch/x86/crypto/twofish_glue_3way.c|8 > arch/x86/mm/testmmiotrace.c|8 > drivers/block/virtio_blk.c |8 > drivers/char/virtio_console.c |8 > drivers/net/ethernet/mellanox/mlx5/core/main.c |8 > drivers/scsi/virtio_scsi.c |8 > drivers/usb/gadget/legacy/inode.c |8 > drivers/usb/gadget/legacy/serial.c | 10 +- > drivers/usb/gadget/udc/dummy_hcd.c |8 > drivers/usb/usbip/vudc_main.c |8 > net/ipv4/netfilter/nf_nat_h323.c |8 > 15 files changed, 61 insertions(+), 61 deletions(-) ___ Virtualization mailing list
Re: [PATCH 6/9] usb: gadget: eliminate anonymous module_init & module_exit
On Wed, Mar 16, 2022 at 12:20:07PM -0700, Randy Dunlap wrote:
> Eliminate anonymous module_init() and module_exit(), which can lead to
> confusion or ambiguity when reading System.map, crashes/oops/bugs,
> or an initcall_debug log.
>
> Give each of these init and exit functions unique driver-specific
> names to eliminate the anonymous names.
>
> Example 1: (System.map)
> 832fc78c t init
> 832fc79e t init
> 832fc8f8 t init
>
> Example 2: (initcall_debug log)
> calling init+0x0/0x12 @ 1
> initcall init+0x0/0x12 returned 0 after 15 usecs
> calling init+0x0/0x60 @ 1
> initcall init+0x0/0x60 returned 0 after 2 usecs
> calling init+0x0/0x9a @ 1
> initcall init+0x0/0x9a returned 0 after 74 usecs
>
> Fixes: bd25a14edb75 ("usb: gadget: legacy/serial: allow dynamic removal")
> Fixes: 7bb5ea54be47 ("usb gadget serial: use composite gadget framework")
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
I continue to be confused about the latest rules for the Fixes tag but this one
in particular seems completely useless. This is the 'beginning of time' commit
by Linus AFAICT. So do any of these Fixes tags need to be in this series?
Regardless:
Reviewed-by: Ira Weiny
> Signed-off-by: Randy Dunlap
> Cc: Felipe Balbi
> Cc: Michał Mirosław
> Cc: Greg Kroah-Hartman
> Cc: Sebastian Andrzej Siewior
> Cc: linux-...@vger.kernel.org
> ---
> drivers/usb/gadget/legacy/inode.c |8
> drivers/usb/gadget/legacy/serial.c | 10 +-
> drivers/usb/gadget/udc/dummy_hcd.c |8
> 3 files changed, 13 insertions(+), 13 deletions(-)
>
> --- lnx-517-rc8.orig/drivers/usb/gadget/legacy/serial.c
> +++ lnx-517-rc8/drivers/usb/gadget/legacy/serial.c
> @@ -273,7 +273,7 @@ static struct usb_composite_driver gseri
> static int switch_gserial_enable(bool do_enable)
> {
> if (!serial_config_driver.label)
> - /* init() was not called, yet */
> + /* gserial_init() was not called, yet */
> return 0;
>
> if (do_enable)
> @@ -283,7 +283,7 @@ static int switch_gserial_enable(bool do
> return 0;
> }
>
> -static int __init init(void)
> +static int __init gserial_init(void)
> {
> /* We *could* export two configs; that'd be much cleaner...
>* but neither of these product IDs was defined that way.
> @@ -314,11 +314,11 @@ static int __init init(void)
>
> return usb_composite_probe(_driver);
> }
> -module_init(init);
> +module_init(gserial_init);
>
> -static void __exit cleanup(void)
> +static void __exit gserial_cleanup(void)
> {
> if (enable)
> usb_composite_unregister(_driver);
> }
> -module_exit(cleanup);
> +module_exit(gserial_cleanup);
> --- lnx-517-rc8.orig/drivers/usb/gadget/udc/dummy_hcd.c
> +++ lnx-517-rc8/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -2765,7 +2765,7 @@ static struct platform_driver dummy_hcd_
> static struct platform_device *the_udc_pdev[MAX_NUM_UDC];
> static struct platform_device *the_hcd_pdev[MAX_NUM_UDC];
>
> -static int __init init(void)
> +static int __init dummy_hcd_init(void)
> {
> int retval = -ENOMEM;
> int i;
> @@ -2887,9 +2887,9 @@ err_alloc_udc:
> platform_device_put(the_hcd_pdev[i]);
> return retval;
> }
> -module_init(init);
> +module_init(dummy_hcd_init);
>
> -static void __exit cleanup(void)
> +static void __exit dummy_hcd_cleanup(void)
> {
> int i;
>
> @@ -2905,4 +2905,4 @@ static void __exit cleanup(void)
> platform_driver_unregister(_udc_driver);
> platform_driver_unregister(_hcd_driver);
> }
> -module_exit(cleanup);
> +module_exit(dummy_hcd_cleanup);
> --- lnx-517-rc8.orig/drivers/usb/gadget/legacy/inode.c
> +++ lnx-517-rc8/drivers/usb/gadget/legacy/inode.c
> @@ -2101,7 +2101,7 @@ MODULE_ALIAS_FS("gadgetfs");
>
> /*--*/
>
> -static int __init init (void)
> +static int __init gadgetfs_init (void)
> {
> int status;
>
> @@ -2111,12 +2111,12 @@ static int __init init (void)
> shortname, driver_desc);
> return status;
> }
> -module_init (init);
> +module_init (gadgetfs_init);
>
> -static void __exit cleanup (void)
> +static void __exit gadgetfs_cleanup (void)
> {
> pr_debug ("unregister %s\n", shortname);
> unregister_filesystem (_type);
> }
> -module_exit (cleanup);
> +module_exit (gadgetfs_cleanup);
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 1/9] virtio_blk: eliminate anonymous module_init & module_exit
On Thu, Mar 17, 2022 at 3:25 AM Randy Dunlap wrote:
>
> Eliminate anonymous module_init() and module_exit(), which can lead to
> confusion or ambiguity when reading System.map, crashes/oops/bugs,
> or an initcall_debug log.
>
> Give each of these init and exit functions unique driver-specific
> names to eliminate the anonymous names.
>
> Example 1: (System.map)
> 832fc78c t init
> 832fc79e t init
> 832fc8f8 t init
>
> Example 2: (initcall_debug log)
> calling init+0x0/0x12 @ 1
> initcall init+0x0/0x12 returned 0 after 15 usecs
> calling init+0x0/0x60 @ 1
> initcall init+0x0/0x60 returned 0 after 2 usecs
> calling init+0x0/0x9a @ 1
> initcall init+0x0/0x9a returned 0 after 74 usecs
>
> Fixes: e467cde23818 ("Block driver using virtio.")
> Signed-off-by: Randy Dunlap
> Cc: "Michael S. Tsirkin"
> Cc: Jason Wang
> Cc: Paolo Bonzini
> Cc: Stefan Hajnoczi
> Cc: virtualization@lists.linux-foundation.org
> Cc: Jens Axboe
> Cc: linux-bl...@vger.kernel.org
> ---
Acked-by: Jason Wang
> drivers/block/virtio_blk.c |8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> --- lnx-517-rc8.orig/drivers/block/virtio_blk.c
> +++ lnx-517-rc8/drivers/block/virtio_blk.c
> @@ -1058,7 +1058,7 @@ static struct virtio_driver virtio_blk =
> #endif
> };
>
> -static int __init init(void)
> +static int __init virtio_blk_init(void)
> {
> int error;
>
> @@ -1084,14 +1084,14 @@ out_destroy_workqueue:
> return error;
> }
>
> -static void __exit fini(void)
> +static void __exit virtio_blk_fini(void)
> {
> unregister_virtio_driver(_blk);
> unregister_blkdev(major, "virtblk");
> destroy_workqueue(virtblk_wq);
> }
> -module_init(init);
> -module_exit(fini);
> +module_init(virtio_blk_init);
> +module_exit(virtio_blk_fini);
>
> MODULE_DEVICE_TABLE(virtio, id_table);
> MODULE_DESCRIPTION("Virtio block driver");
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 5/9] virtio-scsi: eliminate anonymous module_init & module_exit
On Thu, Mar 17, 2022 at 3:24 AM Randy Dunlap wrote:
>
> Eliminate anonymous module_init() and module_exit(), which can lead to
> confusion or ambiguity when reading System.map, crashes/oops/bugs,
> or an initcall_debug log.
>
> Give each of these init and exit functions unique driver-specific
> names to eliminate the anonymous names.
>
> Example 1: (System.map)
> 832fc78c t init
> 832fc79e t init
> 832fc8f8 t init
>
> Example 2: (initcall_debug log)
> calling init+0x0/0x12 @ 1
> initcall init+0x0/0x12 returned 0 after 15 usecs
> calling init+0x0/0x60 @ 1
> initcall init+0x0/0x60 returned 0 after 2 usecs
> calling init+0x0/0x9a @ 1
> initcall init+0x0/0x9a returned 0 after 74 usecs
>
> Fixes: 4fe74b1cb051 ("[SCSI] virtio-scsi: SCSI driver for QEMU based virtual
> machines")
> Signed-off-by: Randy Dunlap
> Cc: "Michael S. Tsirkin"
> Cc: Jason Wang
> Cc: Paolo Bonzini
> Cc: Stefan Hajnoczi
> Cc: "James E.J. Bottomley"
> Cc: "Martin K. Petersen"
> Cc: linux-s...@vger.kernel.org
> Cc: virtualization@lists.linux-foundation.org
Acked-by: Jason Wang
> ---
> drivers/scsi/virtio_scsi.c |8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> --- lnx-517-rc8.orig/drivers/scsi/virtio_scsi.c
> +++ lnx-517-rc8/drivers/scsi/virtio_scsi.c
> @@ -988,7 +988,7 @@ static struct virtio_driver virtio_scsi_
> .remove = virtscsi_remove,
> };
>
> -static int __init init(void)
> +static int __init virtio_scsi_init(void)
> {
> int ret = -ENOMEM;
>
> @@ -1020,14 +1020,14 @@ error:
> return ret;
> }
>
> -static void __exit fini(void)
> +static void __exit virtio_scsi_fini(void)
> {
> unregister_virtio_driver(_scsi_driver);
> mempool_destroy(virtscsi_cmd_pool);
> kmem_cache_destroy(virtscsi_cmd_cache);
> }
> -module_init(init);
> -module_exit(fini);
> +module_init(virtio_scsi_init);
> +module_exit(virtio_scsi_fini);
>
> MODULE_DEVICE_TABLE(virtio, id_table);
> MODULE_DESCRIPTION("Virtio SCSI HBA driver");
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 9/9] testmmiotrace: eliminate anonymous module_init & module_exit
On Wed, 16 Mar 2022 12:20:10 -0700
Randy Dunlap wrote:
> Eliminate anonymous module_init() and module_exit(), which can lead to
> confusion or ambiguity when reading System.map, crashes/oops/bugs,
> or an initcall_debug log.
>
> Give each of these init and exit functions unique driver-specific
> names to eliminate the anonymous names.
>
> Example 1: (System.map)
> 832fc78c t init
> 832fc79e t init
> 832fc8f8 t init
>
> Example 2: (initcall_debug log)
> calling init+0x0/0x12 @ 1
> initcall init+0x0/0x12 returned 0 after 15 usecs
> calling init+0x0/0x60 @ 1
> initcall init+0x0/0x60 returned 0 after 2 usecs
> calling init+0x0/0x9a @ 1
> initcall init+0x0/0x9a returned 0 after 74 usecs
>
> Fixes: 8b7d89d02ef3 ("x86: mmiotrace - trace memory mapped IO")
> Signed-off-by: Randy Dunlap
> Cc: Thomas Gleixner
> Cc: Steven Rostedt
Acked-by: Steven Rostedt (Google)
-- Steve
> Cc: Ingo Molnar
> Cc: Karol Herbst
> Cc: Pekka Paalanen
> Cc: Dave Hansen
> Cc: Andy Lutomirski
> Cc: Peter Zijlstra
> Cc: Borislav Petkov
> Cc: "H. Peter Anvin"
> Cc: nouv...@lists.freedesktop.org
> Cc: x...@kernel.org
> ---
> arch/x86/mm/testmmiotrace.c |8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> --- lnx-517-rc8.orig/arch/x86/mm/testmmiotrace.c
> +++ lnx-517-rc8/arch/x86/mm/testmmiotrace.c
> @@ -113,7 +113,7 @@ static void do_test_bulk_ioremapping(voi
> synchronize_rcu();
> }
>
> -static int __init init(void)
> +static int __init testmmiotrace_init(void)
> {
> unsigned long size = (read_far) ? (8 << 20) : (16 << 10);
> int ret = security_locked_down(LOCKDOWN_MMIOTRACE);
> @@ -136,11 +136,11 @@ static int __init init(void)
> return 0;
> }
>
> -static void __exit cleanup(void)
> +static void __exit testmmiotrace_cleanup(void)
> {
> pr_debug("unloaded.\n");
> }
>
> -module_init(init);
> -module_exit(cleanup);
> +module_init(testmmiotrace_init);
> +module_exit(testmmiotrace_cleanup);
> MODULE_LICENSE("GPL");
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v1 1/2] vdpa: Add support for querying vendor statistics
On Thu, Mar 17, 2022 at 6:00 AM Si-Wei Liu wrote: > > > > On 3/16/2022 12:10 AM, Eli Cohen wrote: > >> From: Si-Wei Liu > >> Sent: Wednesday, March 16, 2022 8:52 AM > >> To: Eli Cohen > >> Cc: m...@redhat.com; jasow...@redhat.com; > >> virtualization@lists.linux-foundation.org; epere...@redhat.com; > >> amore...@redhat.com; > >> lviv...@redhat.com; sgarz...@redhat.com; Parav Pandit > >> Subject: Re: [PATCH v1 1/2] vdpa: Add support for querying vendor > >> statistics > >> > >> > >> > >> On 3/15/2022 2:10 AM, Eli Cohen wrote: > >> > >> <...snip...> > >> > Say you got a vdpa net device created with 4 data queue pairs and a > control vq. On boot some guest firmware may support just F_CTRL_VQ but > not F_MQ, then the index for the control vq in guest ends up with 2, as > in this case there's only a single queue pair enabled for rx (index 0) > and tx (index 1). From the host driver (e.g. mlx5_vdpa) perspective, the > control vq is the last vq following 8 > >>> If the host sees F_MQ was not negotiated but F_CTRL_VQ was, then it knows > >>> that control VQ index is 2 > >> Right, but I don't see this feature negotiation info getting returned > >> from your vdpa_dev_vendor_stats_fill(), or did I miss something? How do > >> you plan for host user to get this info? If you meant another "vdpa dev > >> show" command to query negotiated features ahead, this won't get the > >> same lock protected as the time you run the stat query. It's very easy > >> to miss that ephemeral queue index. > > Right, so I suggested to include the negotiated features in the netlink > > message > > for the statistics. That would save us from using two system calls to get > > the > > information required and it answers your concern with respect to locking. > > I think Jason was reluctant to adding this attribute to the message but > > can't > > find where he explained the reasoning. > Maybe Jason can clarify and correct me, but I just did not get the same > impression as what you said? I just skimmed through all of the emails in > the thread, only finding that he didn't want device specific attribute > such as queue type to get returned by the vdpa core, which I agree. I'm > not sure if he's explicitly against piggyback negotiated features to aid > userspace parsing the index. I think we need piggyback the negotiated features, otherwise as you mentioned, we will probably get in-consistency. But a question for the "host queue index", as mentioned before. It's something that is not defined in the spec, so technically, vendor can do any mappings between it and the index what guest can see. I feel like we need to clarify it in the spec first. Thanks > > Another way around, vdpa tool may pass down -1 to get_vq_vstat() to > represent the queue index for the control queue - but that's less > favorable as the vdpa core needs to maintain device specific knowledge. > > > > > > data vqs of all 4 pairs, hence got > the 8th index in the rank. Since F_MQ is not negotiated and only 1 data > queue pair enabled, in such event only host qindex 0,1 and 8 have vendor > stats available, and the rest of qindex would get invalid/empty stat. > > Later on say boot continues towards loading the Linux virtio driver, > then guest could successfully negotiate both F_CTRL_VQ and F_MQ > features. In this case, all 8 data virtqueues are fully enabled, the > index for the control vq ends up as 8, following tightly after all the 4 > data queue pairs. Only until both features are negotiated, the guest and > host are able to see consistent view in identifying the control vq. > Since F_MQ is negotiated, all host queues, indexed from 0 through 8, > should have vendor stats available. > > That's why I said the guest qindex is ephemeral and hard to predict > subjected to negotiated features, but host qindex is reliable and more > eligible for command line identification purpose. > > >> <...snip...> > > So what are you actually proposing? Display received and completed > > descriptors > > per queue index without further interpretation? > I'd suggest using a more stable queue id i.e. the host queue index to > represent the qidx (which seems to be what you're doing now?), and > displaying both the host qindex (queue_index_device in the example > below), as well as the guest's (queue_index_driver as below) in the > output: > > >>> Given that per vdpa device you can display statistics only after features > >>> have > >>> been negotiated, you can always know the correct queue index for the > >>> control > >>> VQ. > >> The stats can be displayed only after features are negotiated, and only > >> when the corresponding queue is enabled. If you know it from "vdpa dev > >> show" on day 1 that the control vq and mq features are negotiated, but > >> then on day2 you got nothing for the predicted control vq index, what
Re: [PATCH] virtio-blk: support polling I/O
On Wed, Mar 16, 2022 at 11:33 PM Suwan Kim wrote:
>
> On Wed, Mar 16, 2022 at 10:02:13AM +0800, Jason Wang wrote:
> > On Tue, Mar 15, 2022 at 10:43 PM Suwan Kim wrote:
> > >
> > > On Tue, Mar 15, 2022 at 04:59:23PM +0800, Jason Wang wrote:
> > > > On Mon, Mar 14, 2022 at 8:33 PM Suwan Kim
> > > > wrote:
> > > >
> > > > > On Mon, Mar 14, 2022 at 02:14:53PM +0800, Jason Wang wrote:
> > > > > >
> > > > > > 在 2022/3/11 下午11:28, Suwan Kim 写道:
> > > > > > > diff --git a/include/uapi/linux/virtio_blk.h
> > > > > b/include/uapi/linux/virtio_blk.h
> > > > > > > index d888f013d9ff..3fcaf937afe1 100644
> > > > > > > --- a/include/uapi/linux/virtio_blk.h
> > > > > > > +++ b/include/uapi/linux/virtio_blk.h
> > > > > > > @@ -119,8 +119,9 @@ struct virtio_blk_config {
> > > > > > > * deallocation of one or more of the sectors.
> > > > > > > */
> > > > > > > __u8 write_zeroes_may_unmap;
> > > > > > > + __u8 unused1;
> > > > > > > - __u8 unused1[3];
> > > > > > > + __virtio16 num_poll_queues;
> > > > > > > } __attribute__((packed));
> > > > > >
> > > > > >
> > > > > > This looks like a implementation specific (virtio-blk-pci)
> > > > > > optimization,
> > > > > how
> > > > > > about other implementation like vhost-user-blk?
> > > > >
> > > > > I didn’t consider vhost-user-blk yet. But does vhost-user-blk also
> > > > > use vritio_blk_config as kernel-qemu interface?
> > > > >
> > > >
> > > > Yes, but see below.
> > > >
> > > >
> > > > >
> > > > > Does vhost-user-blk need additional modification to support polling
> > > > > in kernel side?
> > > > >
> > > >
> > > >
> > > > No, but the issue is, things like polling looks not a good candidate for
> > > > the attributes belonging to the device but the driver. So I have more
> > > > questions:
> > > >
> > > > 1) what does it really mean for hardware virtio block devices?
> > > > 2) Does driver polling help for the qemu implementation without polling?
> > > > 3) Using blk_config means we can only get the benefit from the new
> > > > device
> > >
> > > 1) what does it really mean for hardware virtio block devices?
> > > 3) Using blk_config means we can only get the benefit from the new device
> > >
> > > This patch adds dedicated HW queue for polling purpose to virtio
> > > block device.
> > >
> > > So I think it can be a new hw feature. And it can be a new device
> > > that supports hw poll queue.
> >
> > One possible issue is that the "poll" looks more like a
> > software/driver concept other than the device/hardware.
> >
> > >
> > > BTW, I have other idea about it.
> > >
> > > How about adding “num-poll-queues" property as a driver parameter
> > > like NVMe driver, not to QEMU virtio-blk-pci property?
> >
> > It should be fine, but we need to listen to others.
>
> To Michael, Stefan, Max
>
> How about using driver parameter instead of virio_blk_config?
I agree.
Thanks
>
> Regards,
> Suwan Kim
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v1 1/2] vdpa: Add support for querying vendor statistics
On 3/16/2022 12:10 AM, Eli Cohen wrote: From: Si-Wei Liu Sent: Wednesday, March 16, 2022 8:52 AM To: Eli Cohen Cc: m...@redhat.com; jasow...@redhat.com; virtualization@lists.linux-foundation.org; epere...@redhat.com; amore...@redhat.com; lviv...@redhat.com; sgarz...@redhat.com; Parav Pandit Subject: Re: [PATCH v1 1/2] vdpa: Add support for querying vendor statistics On 3/15/2022 2:10 AM, Eli Cohen wrote: <...snip...> Say you got a vdpa net device created with 4 data queue pairs and a control vq. On boot some guest firmware may support just F_CTRL_VQ but not F_MQ, then the index for the control vq in guest ends up with 2, as in this case there's only a single queue pair enabled for rx (index 0) and tx (index 1). From the host driver (e.g. mlx5_vdpa) perspective, the control vq is the last vq following 8 If the host sees F_MQ was not negotiated but F_CTRL_VQ was, then it knows that control VQ index is 2 Right, but I don't see this feature negotiation info getting returned from your vdpa_dev_vendor_stats_fill(), or did I miss something? How do you plan for host user to get this info? If you meant another "vdpa dev show" command to query negotiated features ahead, this won't get the same lock protected as the time you run the stat query. It's very easy to miss that ephemeral queue index. Right, so I suggested to include the negotiated features in the netlink message for the statistics. That would save us from using two system calls to get the information required and it answers your concern with respect to locking. I think Jason was reluctant to adding this attribute to the message but can't find where he explained the reasoning. Maybe Jason can clarify and correct me, but I just did not get the same impression as what you said? I just skimmed through all of the emails in the thread, only finding that he didn't want device specific attribute such as queue type to get returned by the vdpa core, which I agree. I'm not sure if he's explicitly against piggyback negotiated features to aid userspace parsing the index. Another way around, vdpa tool may pass down -1 to get_vq_vstat() to represent the queue index for the control queue - but that's less favorable as the vdpa core needs to maintain device specific knowledge. data vqs of all 4 pairs, hence got the 8th index in the rank. Since F_MQ is not negotiated and only 1 data queue pair enabled, in such event only host qindex 0,1 and 8 have vendor stats available, and the rest of qindex would get invalid/empty stat. Later on say boot continues towards loading the Linux virtio driver, then guest could successfully negotiate both F_CTRL_VQ and F_MQ features. In this case, all 8 data virtqueues are fully enabled, the index for the control vq ends up as 8, following tightly after all the 4 data queue pairs. Only until both features are negotiated, the guest and host are able to see consistent view in identifying the control vq. Since F_MQ is negotiated, all host queues, indexed from 0 through 8, should have vendor stats available. That's why I said the guest qindex is ephemeral and hard to predict subjected to negotiated features, but host qindex is reliable and more eligible for command line identification purpose. <...snip...> So what are you actually proposing? Display received and completed descriptors per queue index without further interpretation? I'd suggest using a more stable queue id i.e. the host queue index to represent the qidx (which seems to be what you're doing now?), and displaying both the host qindex (queue_index_device in the example below), as well as the guest's (queue_index_driver as below) in the output: Given that per vdpa device you can display statistics only after features have been negotiated, you can always know the correct queue index for the control VQ. The stats can be displayed only after features are negotiated, and only when the corresponding queue is enabled. If you know it from "vdpa dev show" on day 1 that the control vq and mq features are negotiated, but then on day2 you got nothing for the predicted control vq index, what would you recommend the host admin to do to get the right qindex again? Re-run the stat query on the same queue index, or check the "vdpa dev show" output again on day 3? This CLI design makes cloud administrator really challenging to follow the dynamics of guest activities were to manage hundreds or thousands of virtual machines... It would be easier, in my opinion, to grasp some well-defined handle that is easily predictable or fixed across the board, for looking up the control virtqueue. This could be a constant host queue index, or a special magic keyword like "qidx ctrlvq". If cloud admin runs vstat query on the control vq using a determined handle but get nothing back, then s/he knows *for sure* the control vq was not available for some reason at the point when the stat was being collected. S/he doesn't even need to care negotiated status via "vdpa dev show" at
Re: [PATCH v2 6/8] drm/shmem-helper: Add generic memory shrinker
On Mon, Mar 14, 2022 at 3:44 PM Dmitry Osipenko
wrote:
>
> Introduce a common DRM SHMEM shrinker. It allows to reduce code
> duplication among DRM drivers, it also handles complicated lockings
> for the drivers. This is initial version of the shrinker that covers
> basic needs of GPU drivers.
>
> This patch is based on a couple ideas borrowed from Rob's Clark MSM
> shrinker and Thomas' Zimmermann variant of SHMEM shrinker.
>
> GPU drivers that want to use generic DRM memory shrinker must support
> generic GEM reservations.
>
> Signed-off-by: Daniel Almeida
> Signed-off-by: Dmitry Osipenko
> ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 194 +
> include/drm/drm_device.h | 4 +
> include/drm/drm_gem.h | 11 ++
> include/drm/drm_gem_shmem_helper.h | 25
> 4 files changed, 234 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 37009418cd28..35be2ee98f11 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -139,6 +139,9 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object
> *shmem)
> {
> struct drm_gem_object *obj = >base;
>
> + /* take out shmem GEM object from the memory shrinker */
> + drm_gem_shmem_madvise(shmem, 0);
> +
> WARN_ON(shmem->vmap_use_count);
>
> if (obj->import_attach) {
> @@ -163,6 +166,42 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object
> *shmem)
> }
> EXPORT_SYMBOL_GPL(drm_gem_shmem_free);
>
> +static void drm_gem_shmem_update_purgeable_status(struct
> drm_gem_shmem_object *shmem)
> +{
> + struct drm_gem_object *obj = >base;
> + struct drm_gem_shmem_shrinker *gem_shrinker =
> obj->dev->shmem_shrinker;
> + size_t page_count = obj->size >> PAGE_SHIFT;
> +
> + if (!gem_shrinker || obj->import_attach || !obj->funcs->purge)
> + return;
> +
> + mutex_lock(>vmap_lock);
> + mutex_lock(>pages_lock);
> + mutex_lock(_shrinker->lock);
> +
> + if (shmem->madv < 0) {
> + list_del_init(>madv_list);
> + goto unlock;
> + } else if (shmem->madv > 0) {
> + if (!list_empty(>madv_list))
> + goto unlock;
> +
> + WARN_ON(gem_shrinker->shrinkable_count + page_count <
> page_count);
> + gem_shrinker->shrinkable_count += page_count;
> +
> + list_add_tail(>madv_list, _shrinker->lru);
> + } else if (!list_empty(>madv_list)) {
> + list_del_init(>madv_list);
> +
> + WARN_ON(gem_shrinker->shrinkable_count < page_count);
> + gem_shrinker->shrinkable_count -= page_count;
> + }
> +unlock:
> + mutex_unlock(_shrinker->lock);
> + mutex_unlock(>pages_lock);
> + mutex_unlock(>vmap_lock);
> +}
> +
> static int drm_gem_shmem_get_pages_locked(struct drm_gem_shmem_object *shmem)
> {
> struct drm_gem_object *obj = >base;
> @@ -366,6 +405,8 @@ int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem,
> ret = drm_gem_shmem_vmap_locked(shmem, map);
> mutex_unlock(>vmap_lock);
>
> + drm_gem_shmem_update_purgeable_status(shmem);
> +
> return ret;
> }
> EXPORT_SYMBOL(drm_gem_shmem_vmap);
> @@ -409,6 +450,8 @@ void drm_gem_shmem_vunmap(struct drm_gem_shmem_object
> *shmem,
> mutex_lock(>vmap_lock);
> drm_gem_shmem_vunmap_locked(shmem, map);
> mutex_unlock(>vmap_lock);
> +
> + drm_gem_shmem_update_purgeable_status(shmem);
> }
> EXPORT_SYMBOL(drm_gem_shmem_vunmap);
>
> @@ -451,6 +494,8 @@ int drm_gem_shmem_madvise(struct drm_gem_shmem_object
> *shmem, int madv)
>
> mutex_unlock(>pages_lock);
>
> + drm_gem_shmem_update_purgeable_status(shmem);
> +
> return (madv >= 0);
> }
> EXPORT_SYMBOL(drm_gem_shmem_madvise);
> @@ -763,6 +808,155 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device
> *dev,
> }
> EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table);
>
> +static struct drm_gem_shmem_shrinker *
> +to_drm_shrinker(struct shrinker *shrinker)
> +{
> + return container_of(shrinker, struct drm_gem_shmem_shrinker, base);
> +}
> +
> +static unsigned long
> +drm_gem_shmem_shrinker_count_objects(struct shrinker *shrinker,
> +struct shrink_control *sc)
> +{
> + struct drm_gem_shmem_shrinker *gem_shrinker =
> to_drm_shrinker(shrinker);
> + u64 count = gem_shrinker->shrinkable_count;
> +
> + if (count >= SHRINK_EMPTY)
> + return SHRINK_EMPTY - 1;
> +
> + return count ?: SHRINK_EMPTY;
> +}
> +
> +static unsigned long
> +drm_gem_shmem_shrinker_scan_objects(struct shrinker *shrinker,
> + struct shrink_control *sc)
> +{
> + struct drm_gem_shmem_shrinker *gem_shrinker =
> to_drm_shrinker(shrinker);
> + struct drm_gem_shmem_object *shmem;
> +
[PATCH 1/9] virtio_blk: eliminate anonymous module_init & module_exit
Eliminate anonymous module_init() and module_exit(), which can lead to
confusion or ambiguity when reading System.map, crashes/oops/bugs,
or an initcall_debug log.
Give each of these init and exit functions unique driver-specific
names to eliminate the anonymous names.
Example 1: (System.map)
832fc78c t init
832fc79e t init
832fc8f8 t init
Example 2: (initcall_debug log)
calling init+0x0/0x12 @ 1
initcall init+0x0/0x12 returned 0 after 15 usecs
calling init+0x0/0x60 @ 1
initcall init+0x0/0x60 returned 0 after 2 usecs
calling init+0x0/0x9a @ 1
initcall init+0x0/0x9a returned 0 after 74 usecs
Fixes: e467cde23818 ("Block driver using virtio.")
Signed-off-by: Randy Dunlap
Cc: "Michael S. Tsirkin"
Cc: Jason Wang
Cc: Paolo Bonzini
Cc: Stefan Hajnoczi
Cc: virtualization@lists.linux-foundation.org
Cc: Jens Axboe
Cc: linux-bl...@vger.kernel.org
---
drivers/block/virtio_blk.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
--- lnx-517-rc8.orig/drivers/block/virtio_blk.c
+++ lnx-517-rc8/drivers/block/virtio_blk.c
@@ -1058,7 +1058,7 @@ static struct virtio_driver virtio_blk =
#endif
};
-static int __init init(void)
+static int __init virtio_blk_init(void)
{
int error;
@@ -1084,14 +1084,14 @@ out_destroy_workqueue:
return error;
}
-static void __exit fini(void)
+static void __exit virtio_blk_fini(void)
{
unregister_virtio_driver(_blk);
unregister_blkdev(major, "virtblk");
destroy_workqueue(virtblk_wq);
}
-module_init(init);
-module_exit(fini);
+module_init(virtio_blk_init);
+module_exit(virtio_blk_fini);
MODULE_DEVICE_TABLE(virtio, id_table);
MODULE_DESCRIPTION("Virtio block driver");
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 9/9] testmmiotrace: eliminate anonymous module_init & module_exit
Eliminate anonymous module_init() and module_exit(), which can lead to
confusion or ambiguity when reading System.map, crashes/oops/bugs,
or an initcall_debug log.
Give each of these init and exit functions unique driver-specific
names to eliminate the anonymous names.
Example 1: (System.map)
832fc78c t init
832fc79e t init
832fc8f8 t init
Example 2: (initcall_debug log)
calling init+0x0/0x12 @ 1
initcall init+0x0/0x12 returned 0 after 15 usecs
calling init+0x0/0x60 @ 1
initcall init+0x0/0x60 returned 0 after 2 usecs
calling init+0x0/0x9a @ 1
initcall init+0x0/0x9a returned 0 after 74 usecs
Fixes: 8b7d89d02ef3 ("x86: mmiotrace - trace memory mapped IO")
Signed-off-by: Randy Dunlap
Cc: Thomas Gleixner
Cc: Steven Rostedt
Cc: Ingo Molnar
Cc: Karol Herbst
Cc: Pekka Paalanen
Cc: Dave Hansen
Cc: Andy Lutomirski
Cc: Peter Zijlstra
Cc: Borislav Petkov
Cc: "H. Peter Anvin"
Cc: nouv...@lists.freedesktop.org
Cc: x...@kernel.org
---
arch/x86/mm/testmmiotrace.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
--- lnx-517-rc8.orig/arch/x86/mm/testmmiotrace.c
+++ lnx-517-rc8/arch/x86/mm/testmmiotrace.c
@@ -113,7 +113,7 @@ static void do_test_bulk_ioremapping(voi
synchronize_rcu();
}
-static int __init init(void)
+static int __init testmmiotrace_init(void)
{
unsigned long size = (read_far) ? (8 << 20) : (16 << 10);
int ret = security_locked_down(LOCKDOWN_MMIOTRACE);
@@ -136,11 +136,11 @@ static int __init init(void)
return 0;
}
-static void __exit cleanup(void)
+static void __exit testmmiotrace_cleanup(void)
{
pr_debug("unloaded.\n");
}
-module_init(init);
-module_exit(cleanup);
+module_init(testmmiotrace_init);
+module_exit(testmmiotrace_cleanup);
MODULE_LICENSE("GPL");
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 5/9] virtio-scsi: eliminate anonymous module_init & module_exit
Eliminate anonymous module_init() and module_exit(), which can lead to
confusion or ambiguity when reading System.map, crashes/oops/bugs,
or an initcall_debug log.
Give each of these init and exit functions unique driver-specific
names to eliminate the anonymous names.
Example 1: (System.map)
832fc78c t init
832fc79e t init
832fc8f8 t init
Example 2: (initcall_debug log)
calling init+0x0/0x12 @ 1
initcall init+0x0/0x12 returned 0 after 15 usecs
calling init+0x0/0x60 @ 1
initcall init+0x0/0x60 returned 0 after 2 usecs
calling init+0x0/0x9a @ 1
initcall init+0x0/0x9a returned 0 after 74 usecs
Fixes: 4fe74b1cb051 ("[SCSI] virtio-scsi: SCSI driver for QEMU based virtual
machines")
Signed-off-by: Randy Dunlap
Cc: "Michael S. Tsirkin"
Cc: Jason Wang
Cc: Paolo Bonzini
Cc: Stefan Hajnoczi
Cc: "James E.J. Bottomley"
Cc: "Martin K. Petersen"
Cc: linux-s...@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
---
drivers/scsi/virtio_scsi.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
--- lnx-517-rc8.orig/drivers/scsi/virtio_scsi.c
+++ lnx-517-rc8/drivers/scsi/virtio_scsi.c
@@ -988,7 +988,7 @@ static struct virtio_driver virtio_scsi_
.remove = virtscsi_remove,
};
-static int __init init(void)
+static int __init virtio_scsi_init(void)
{
int ret = -ENOMEM;
@@ -1020,14 +1020,14 @@ error:
return ret;
}
-static void __exit fini(void)
+static void __exit virtio_scsi_fini(void)
{
unregister_virtio_driver(_scsi_driver);
mempool_destroy(virtscsi_cmd_pool);
kmem_cache_destroy(virtscsi_cmd_cache);
}
-module_init(init);
-module_exit(fini);
+module_init(virtio_scsi_init);
+module_exit(virtio_scsi_fini);
MODULE_DEVICE_TABLE(virtio, id_table);
MODULE_DESCRIPTION("Virtio SCSI HBA driver");
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 0/9] treewide: eliminate anonymous module_init & module_exit
There are a number of drivers that use "module_init(init)" and "module_exit(exit)", which are anonymous names and can lead to confusion or ambiguity when reading System.map, crashes/oops/bugs, or an initcall_debug log. Give each of these init and exit functions unique driver-specific names to eliminate the anonymous names. Example 1: (System.map) 832fc78c t init 832fc79e t init 832fc8f8 t init 832fca05 t init 832fcbd2 t init 83328f0e t init 8332c5b1 t init 8332d9eb t init 8332f0aa t init 83330e25 t init 833317a5 t init 8333dd6b t init Example 2: (initcall_debug log) calling init+0x0/0x12 @ 1 initcall init+0x0/0x12 returned 0 after 15 usecs calling init+0x0/0x60 @ 1 initcall init+0x0/0x60 returned 0 after 2 usecs calling init+0x0/0x9a @ 1 initcall init+0x0/0x9a returned 0 after 74 usecs calling init+0x0/0x73 @ 1 initcall init+0x0/0x73 returned 0 after 6 usecs calling init+0x0/0x73 @ 1 initcall init+0x0/0x73 returned 0 after 4 usecs calling init+0x0/0xf5 @ 1 initcall init+0x0/0xf5 returned 0 after 27 usecs calling init+0x0/0x7d @ 1 initcall init+0x0/0x7d returned 0 after 11 usecs calling init+0x0/0xc9 @ 1 initcall init+0x0/0xc9 returned 0 after 19 usecs calling init+0x0/0x9d @ 1 initcall init+0x0/0x9d returned 0 after 37 usecs calling init+0x0/0x63f @ 1 initcall init+0x0/0x63f returned 0 after 411 usecs calling init+0x0/0x171 @ 1 initcall init+0x0/0x171 returned 0 after 61 usecs calling init+0x0/0xef @ 1 initcall init+0x0/0xef returned 0 after 3 usecs Cc: "Michael S. Tsirkin" Cc: Jason Wang Cc: Paolo Bonzini Cc: Stefan Hajnoczi Cc: Jens Axboe Cc: Amit Shah Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Cc: Eli Cohen Cc: Saeed Mahameed Cc: Leon Romanovsky Cc: Pablo Neira Ayuso Cc: Jozsef Kadlecsik Cc: Florian Westphal Cc: "David S. Miller" Cc: Jakub Kicinski Cc: "James E.J. Bottomley" Cc: "Martin K. Petersen" Cc: Felipe Balbi Cc: Michał Mirosław Cc: Sebastian Andrzej Siewior Cc: Krzysztof Opasiak Cc: Igor Kotrasinski Cc: Valentina Manea Cc: Shuah Khan Cc: Shuah Khan Cc: Jussi Kivilinna Cc: Joachim Fritschi Cc: Herbert Xu Cc: Thomas Gleixner Cc: Steven Rostedt Cc: Ingo Molnar Cc: Karol Herbst Cc: Pekka Paalanen Cc: Dave Hansen Cc: Andy Lutomirski Cc: Peter Zijlstra Cc: Borislav Petkov Cc: "H. Peter Anvin" Cc: netfilter-de...@vger.kernel.org Cc: coret...@netfilter.org Cc: net...@vger.kernel.org Cc: linux-bl...@vger.kernel.org Cc: linux-cry...@vger.kernel.org Cc: linux-r...@vger.kernel.org Cc: linux-s...@vger.kernel.org Cc: linux-...@vger.kernel.org Cc: nouv...@lists.freedesktop.org Cc: virtualization@lists.linux-foundation.org Cc: x...@kernel.org patches: [PATCH 1/9] virtio_blk: eliminate anonymous module_init & module_exit [PATCH 2/9] virtio_console: eliminate anonymous module_init & module_exit [PATCH 3/9] net: mlx5: eliminate anonymous module_init & module_exit [PATCH 4/9] netfilter: h323: eliminate anonymous module_init & module_exit [PATCH 5/9] virtio-scsi: eliminate anonymous module_init & module_exit [PATCH 6/9] usb: gadget: eliminate anonymous module_init & module_exit [PATCH 7/9] usb: usbip: eliminate anonymous module_init & module_exit [PATCH 8/9] x86/crypto: eliminate anonymous module_init & module_exit [PATCH 9/9] testmmiotrace: eliminate anonymous module_init & module_exit diffstat: arch/x86/crypto/blowfish_glue.c|8 arch/x86/crypto/camellia_glue.c|8 arch/x86/crypto/serpent_avx2_glue.c|8 arch/x86/crypto/twofish_glue.c |8 arch/x86/crypto/twofish_glue_3way.c|8 arch/x86/mm/testmmiotrace.c|8 drivers/block/virtio_blk.c |8 drivers/char/virtio_console.c |8 drivers/net/ethernet/mellanox/mlx5/core/main.c |8 drivers/scsi/virtio_scsi.c |8 drivers/usb/gadget/legacy/inode.c |8 drivers/usb/gadget/legacy/serial.c | 10 +- drivers/usb/gadget/udc/dummy_hcd.c |8 drivers/usb/usbip/vudc_main.c |8 net/ipv4/netfilter/nf_nat_h323.c |8 15 files changed, 61 insertions(+), 61 deletions(-) ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 8/9] x86/crypto: eliminate anonymous module_init & module_exit
Eliminate anonymous module_init() and module_exit(), which can lead to
confusion or ambiguity when reading System.map, crashes/oops/bugs,
or an initcall_debug log.
Give each of these init and exit functions unique driver-specific
names to eliminate the anonymous names.
Example 1: (System.map)
832fc78c t init
832fc79e t init
832fc8f8 t init
Example 2: (initcall_debug log)
calling init+0x0/0x12 @ 1
initcall init+0x0/0x12 returned 0 after 15 usecs
calling init+0x0/0x60 @ 1
initcall init+0x0/0x60 returned 0 after 2 usecs
calling init+0x0/0x9a @ 1
initcall init+0x0/0x9a returned 0 after 74 usecs
Fixes: 64b94ceae8c1 ("crypto: blowfish - add x86_64 assembly implementation")
Fixes: 676a38046f4f ("crypto: camellia-x86_64 - module init/exit functions
should be static")
Fixes: 0b95ec56ae19 ("crypto: camellia - add assembler implementation for
x86_64")
Fixes: 56d76c96a9f3 ("crypto: serpent - add AVX2/x86_64 assembler
implementation of serpent cipher")
Fixes: b9f535ffe38f ("[CRYPTO] twofish: i586 assembly version")
Fixes: ff0a70fe0536 ("crypto: twofish-x86_64-3way - module init/exit functions
should be static")
Fixes: 8280daad436e ("crypto: twofish - add 3-way parallel x86_64 assembler
implemention")
Signed-off-by: Randy Dunlap
Cc: Jussi Kivilinna
Cc: Joachim Fritschi
Cc: Herbert Xu
Cc: "David S. Miller"
Cc: linux-cry...@vger.kernel.org
Cc: x...@kernel.org
---
arch/x86/crypto/blowfish_glue.c |8
arch/x86/crypto/camellia_glue.c |8
arch/x86/crypto/serpent_avx2_glue.c |8
arch/x86/crypto/twofish_glue.c |8
arch/x86/crypto/twofish_glue_3way.c |8
5 files changed, 20 insertions(+), 20 deletions(-)
--- lnx-517-rc8.orig/arch/x86/crypto/blowfish_glue.c
+++ lnx-517-rc8/arch/x86/crypto/blowfish_glue.c
@@ -315,7 +315,7 @@ static int force;
module_param(force, int, 0);
MODULE_PARM_DESC(force, "Force module load, ignore CPU blacklist");
-static int __init init(void)
+static int __init blowfish_init(void)
{
int err;
@@ -339,15 +339,15 @@ static int __init init(void)
return err;
}
-static void __exit fini(void)
+static void __exit blowfish_fini(void)
{
crypto_unregister_alg(_cipher_alg);
crypto_unregister_skciphers(bf_skcipher_algs,
ARRAY_SIZE(bf_skcipher_algs));
}
-module_init(init);
-module_exit(fini);
+module_init(blowfish_init);
+module_exit(blowfish_fini);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("Blowfish Cipher Algorithm, asm optimized");
--- lnx-517-rc8.orig/arch/x86/crypto/camellia_glue.c
+++ lnx-517-rc8/arch/x86/crypto/camellia_glue.c
@@ -1377,7 +1377,7 @@ static int force;
module_param(force, int, 0);
MODULE_PARM_DESC(force, "Force module load, ignore CPU blacklist");
-static int __init init(void)
+static int __init camellia_init(void)
{
int err;
@@ -1401,15 +1401,15 @@ static int __init init(void)
return err;
}
-static void __exit fini(void)
+static void __exit camellia_fini(void)
{
crypto_unregister_alg(_cipher_alg);
crypto_unregister_skciphers(camellia_skcipher_algs,
ARRAY_SIZE(camellia_skcipher_algs));
}
-module_init(init);
-module_exit(fini);
+module_init(camellia_init);
+module_exit(camellia_fini);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("Camellia Cipher Algorithm, asm optimized");
--- lnx-517-rc8.orig/arch/x86/crypto/serpent_avx2_glue.c
+++ lnx-517-rc8/arch/x86/crypto/serpent_avx2_glue.c
@@ -96,7 +96,7 @@ static struct skcipher_alg serpent_algs[
static struct simd_skcipher_alg *serpent_simd_algs[ARRAY_SIZE(serpent_algs)];
-static int __init init(void)
+static int __init serpent_avx2_init(void)
{
const char *feature_name;
@@ -115,14 +115,14 @@ static int __init init(void)
serpent_simd_algs);
}
-static void __exit fini(void)
+static void __exit serpent_avx2_fini(void)
{
simd_unregister_skciphers(serpent_algs, ARRAY_SIZE(serpent_algs),
serpent_simd_algs);
}
-module_init(init);
-module_exit(fini);
+module_init(serpent_avx2_init);
+module_exit(serpent_avx2_fini);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("Serpent Cipher Algorithm, AVX2 optimized");
--- lnx-517-rc8.orig/arch/x86/crypto/twofish_glue.c
+++ lnx-517-rc8/arch/x86/crypto/twofish_glue.c
@@ -81,18 +81,18 @@ static struct crypto_alg alg = {
}
};
-static int __init init(void)
+static int __init twofish_glue_init(void)
{
return crypto_register_alg();
}
-static void __exit fini(void)
+static void __exit twofish_glue_fini(void)
{
crypto_unregister_alg();
}
-module_init(init);
-module_exit(fini);
+module_init(twofish_glue_init);
+module_exit(twofish_glue_fini);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION ("Twofish Cipher Algorithm, asm optimized");
--- lnx-517-rc8.orig/arch/x86/crypto/twofish_glue_3way.c
+++
[PATCH 6/9] usb: gadget: eliminate anonymous module_init & module_exit
Eliminate anonymous module_init() and module_exit(), which can lead to
confusion or ambiguity when reading System.map, crashes/oops/bugs,
or an initcall_debug log.
Give each of these init and exit functions unique driver-specific
names to eliminate the anonymous names.
Example 1: (System.map)
832fc78c t init
832fc79e t init
832fc8f8 t init
Example 2: (initcall_debug log)
calling init+0x0/0x12 @ 1
initcall init+0x0/0x12 returned 0 after 15 usecs
calling init+0x0/0x60 @ 1
initcall init+0x0/0x60 returned 0 after 2 usecs
calling init+0x0/0x9a @ 1
initcall init+0x0/0x9a returned 0 after 74 usecs
Fixes: bd25a14edb75 ("usb: gadget: legacy/serial: allow dynamic removal")
Fixes: 7bb5ea54be47 ("usb gadget serial: use composite gadget framework")
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Randy Dunlap
Cc: Felipe Balbi
Cc: Michał Mirosław
Cc: Greg Kroah-Hartman
Cc: Sebastian Andrzej Siewior
Cc: linux-...@vger.kernel.org
---
drivers/usb/gadget/legacy/inode.c |8
drivers/usb/gadget/legacy/serial.c | 10 +-
drivers/usb/gadget/udc/dummy_hcd.c |8
3 files changed, 13 insertions(+), 13 deletions(-)
--- lnx-517-rc8.orig/drivers/usb/gadget/legacy/serial.c
+++ lnx-517-rc8/drivers/usb/gadget/legacy/serial.c
@@ -273,7 +273,7 @@ static struct usb_composite_driver gseri
static int switch_gserial_enable(bool do_enable)
{
if (!serial_config_driver.label)
- /* init() was not called, yet */
+ /* gserial_init() was not called, yet */
return 0;
if (do_enable)
@@ -283,7 +283,7 @@ static int switch_gserial_enable(bool do
return 0;
}
-static int __init init(void)
+static int __init gserial_init(void)
{
/* We *could* export two configs; that'd be much cleaner...
* but neither of these product IDs was defined that way.
@@ -314,11 +314,11 @@ static int __init init(void)
return usb_composite_probe(_driver);
}
-module_init(init);
+module_init(gserial_init);
-static void __exit cleanup(void)
+static void __exit gserial_cleanup(void)
{
if (enable)
usb_composite_unregister(_driver);
}
-module_exit(cleanup);
+module_exit(gserial_cleanup);
--- lnx-517-rc8.orig/drivers/usb/gadget/udc/dummy_hcd.c
+++ lnx-517-rc8/drivers/usb/gadget/udc/dummy_hcd.c
@@ -2765,7 +2765,7 @@ static struct platform_driver dummy_hcd_
static struct platform_device *the_udc_pdev[MAX_NUM_UDC];
static struct platform_device *the_hcd_pdev[MAX_NUM_UDC];
-static int __init init(void)
+static int __init dummy_hcd_init(void)
{
int retval = -ENOMEM;
int i;
@@ -2887,9 +2887,9 @@ err_alloc_udc:
platform_device_put(the_hcd_pdev[i]);
return retval;
}
-module_init(init);
+module_init(dummy_hcd_init);
-static void __exit cleanup(void)
+static void __exit dummy_hcd_cleanup(void)
{
int i;
@@ -2905,4 +2905,4 @@ static void __exit cleanup(void)
platform_driver_unregister(_udc_driver);
platform_driver_unregister(_hcd_driver);
}
-module_exit(cleanup);
+module_exit(dummy_hcd_cleanup);
--- lnx-517-rc8.orig/drivers/usb/gadget/legacy/inode.c
+++ lnx-517-rc8/drivers/usb/gadget/legacy/inode.c
@@ -2101,7 +2101,7 @@ MODULE_ALIAS_FS("gadgetfs");
/*--*/
-static int __init init (void)
+static int __init gadgetfs_init (void)
{
int status;
@@ -2111,12 +2111,12 @@ static int __init init (void)
shortname, driver_desc);
return status;
}
-module_init (init);
+module_init (gadgetfs_init);
-static void __exit cleanup (void)
+static void __exit gadgetfs_cleanup (void)
{
pr_debug ("unregister %s\n", shortname);
unregister_filesystem (_type);
}
-module_exit (cleanup);
+module_exit (gadgetfs_cleanup);
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 4/9] netfilter: h323: eliminate anonymous module_init & module_exit
Eliminate anonymous module_init() and module_exit(), which can lead to
confusion or ambiguity when reading System.map, crashes/oops/bugs,
or an initcall_debug log.
Give each of these init and exit functions unique driver-specific
names to eliminate the anonymous names.
Example 1: (System.map)
832fc78c t init
832fc79e t init
832fc8f8 t init
Example 2: (initcall_debug log)
calling init+0x0/0x12 @ 1
initcall init+0x0/0x12 returned 0 after 15 usecs
calling init+0x0/0x60 @ 1
initcall init+0x0/0x60 returned 0 after 2 usecs
calling init+0x0/0x9a @ 1
initcall init+0x0/0x9a returned 0 after 74 usecs
Fixes: f587de0e2feb ("[NETFILTER]: nf_conntrack/nf_nat: add H.323 helper port")
Signed-off-by: Randy Dunlap
Cc: Pablo Neira Ayuso
Cc: Jozsef Kadlecsik
Cc: Florian Westphal
Cc: netfilter-de...@vger.kernel.org
Cc: coret...@netfilter.org
Cc: "David S. Miller"
Cc: Jakub Kicinski
Cc: net...@vger.kernel.org
---
net/ipv4/netfilter/nf_nat_h323.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
--- lnx-517-rc8.orig/net/ipv4/netfilter/nf_nat_h323.c
+++ lnx-517-rc8/net/ipv4/netfilter/nf_nat_h323.c
@@ -580,7 +580,7 @@ static struct nf_ct_helper_expectfn call
};
//
-static int __init init(void)
+static int __init nf_nat_h323_init(void)
{
BUG_ON(set_h245_addr_hook != NULL);
BUG_ON(set_h225_addr_hook != NULL);
@@ -607,7 +607,7 @@ static int __init init(void)
}
//
-static void __exit fini(void)
+static void __exit nf_nat_h323_fini(void)
{
RCU_INIT_POINTER(set_h245_addr_hook, NULL);
RCU_INIT_POINTER(set_h225_addr_hook, NULL);
@@ -624,8 +624,8 @@ static void __exit fini(void)
}
//
-module_init(init);
-module_exit(fini);
+module_init(nf_nat_h323_init);
+module_exit(nf_nat_h323_fini);
MODULE_AUTHOR("Jing Min Zhao ");
MODULE_DESCRIPTION("H.323 NAT helper");
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 2/9] virtio_console: eliminate anonymous module_init & module_exit
Eliminate anonymous module_init() and module_exit(), which can lead to
confusion or ambiguity when reading System.map, crashes/oops/bugs,
or an initcall_debug log.
Give each of these init and exit functions unique driver-specific
names to eliminate the anonymous names.
Example 1: (System.map)
832fc78c t init
832fc79e t init
832fc8f8 t init
Example 2: (initcall_debug log)
calling init+0x0/0x12 @ 1
initcall init+0x0/0x12 returned 0 after 15 usecs
calling init+0x0/0x60 @ 1
initcall init+0x0/0x60 returned 0 after 2 usecs
calling init+0x0/0x9a @ 1
initcall init+0x0/0x9a returned 0 after 74 usecs
Fixes: 31610434bc35 ("Virtio console driver")
Fixes: 7177876fea83 ("virtio: console: Add ability to remove module")
Signed-off-by: Randy Dunlap
Cc: Amit Shah
Cc: virtualization@lists.linux-foundation.org
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
---
drivers/char/virtio_console.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
--- lnx-517-rc8.orig/drivers/char/virtio_console.c
+++ lnx-517-rc8/drivers/char/virtio_console.c
@@ -2245,7 +2245,7 @@ static struct virtio_driver virtio_rproc
.remove = virtcons_remove,
};
-static int __init init(void)
+static int __init virtio_console_init(void)
{
int err;
@@ -2280,7 +2280,7 @@ free:
return err;
}
-static void __exit fini(void)
+static void __exit virtio_console_fini(void)
{
reclaim_dma_bufs();
@@ -2290,8 +2290,8 @@ static void __exit fini(void)
class_destroy(pdrvdata.class);
debugfs_remove_recursive(pdrvdata.debugfs_dir);
}
-module_init(init);
-module_exit(fini);
+module_init(virtio_console_init);
+module_exit(virtio_console_fini);
MODULE_DESCRIPTION("Virtio console driver");
MODULE_LICENSE("GPL");
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 3/9] net: mlx5: eliminate anonymous module_init & module_exit
Eliminate anonymous module_init() and module_exit(), which can lead to
confusion or ambiguity when reading System.map, crashes/oops/bugs,
or an initcall_debug log.
Give each of these init and exit functions unique driver-specific
names to eliminate the anonymous names.
Example 1: (System.map)
832fc78c t init
832fc79e t init
832fc8f8 t init
Example 2: (initcall_debug log)
calling init+0x0/0x12 @ 1
initcall init+0x0/0x12 returned 0 after 15 usecs
calling init+0x0/0x60 @ 1
initcall init+0x0/0x60 returned 0 after 2 usecs
calling init+0x0/0x9a @ 1
initcall init+0x0/0x9a returned 0 after 74 usecs
Fixes: e126ba97dba9 ("mlx5: Add driver for Mellanox Connect-IB adapters")
Signed-off-by: Randy Dunlap
Cc: Eli Cohen
Cc: Saeed Mahameed
Cc: net...@vger.kernel.org
Cc: Leon Romanovsky
Cc: linux-r...@vger.kernel.org
---
drivers/net/ethernet/mellanox/mlx5/core/main.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
--- lnx-517-rc8.orig/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ lnx-517-rc8/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -1893,7 +1893,7 @@ static void mlx5_core_verify_params(void
}
}
-static int __init init(void)
+static int __init mlx5_init(void)
{
int err;
@@ -1929,7 +1929,7 @@ err_debug:
return err;
}
-static void __exit cleanup(void)
+static void __exit mlx5_cleanup(void)
{
mlx5e_cleanup();
mlx5_sf_driver_unregister();
@@ -1937,5 +1937,5 @@ static void __exit cleanup(void)
mlx5_unregister_debugfs();
}
-module_init(init);
-module_exit(cleanup);
+module_init(mlx5_init);
+module_exit(mlx5_cleanup);
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 7/9] usb: usbip: eliminate anonymous module_init & module_exit
Eliminate anonymous module_init() and module_exit(), which can lead to
confusion or ambiguity when reading System.map, crashes/oops/bugs,
or an initcall_debug log.
Give each of these init and exit functions unique driver-specific
names to eliminate the anonymous names.
Example 1: (System.map)
832fc78c t init
832fc79e t init
832fc8f8 t init
Example 2: (initcall_debug log)
calling init+0x0/0x12 @ 1
initcall init+0x0/0x12 returned 0 after 15 usecs
calling init+0x0/0x60 @ 1
initcall init+0x0/0x60 returned 0 after 2 usecs
calling init+0x0/0x9a @ 1
initcall init+0x0/0x9a returned 0 after 74 usecs
Fixes: 80fd9cd52de6 ("usbip: vudc: Add VUDC main file")
Signed-off-by: Randy Dunlap
Cc: Krzysztof Opasiak
Cc: Igor Kotrasinski
Cc: Greg Kroah-Hartman
Cc: Valentina Manea
Cc: Shuah Khan
Cc: Shuah Khan
Cc: linux-...@vger.kernel.org
---
drivers/usb/usbip/vudc_main.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
--- lnx-517-rc8.orig/drivers/usb/usbip/vudc_main.c
+++ lnx-517-rc8/drivers/usb/usbip/vudc_main.c
@@ -28,7 +28,7 @@ static struct platform_driver vudc_drive
static struct list_head vudc_devices = LIST_HEAD_INIT(vudc_devices);
-static int __init init(void)
+static int __init vudc_init(void)
{
int retval = -ENOMEM;
int i;
@@ -86,9 +86,9 @@ cleanup:
out:
return retval;
}
-module_init(init);
+module_init(vudc_init);
-static void __exit cleanup(void)
+static void __exit vudc_cleanup(void)
{
struct vudc_device *udc_dev = NULL, *udc_dev2 = NULL;
@@ -103,7 +103,7 @@ static void __exit cleanup(void)
}
platform_driver_unregister(_driver);
}
-module_exit(cleanup);
+module_exit(vudc_cleanup);
MODULE_DESCRIPTION("USB over IP Device Controller");
MODULE_AUTHOR("Krzysztof Opasiak, Karol Kosik, Igor Kotrasinski");
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio-blk: support polling I/O
On Wed, Mar 16, 2022 at 05:36:20PM +0200, Max Gurtovoy wrote:
>
> On 3/16/2022 5:32 PM, Suwan Kim wrote:
> > On Wed, Mar 16, 2022 at 10:02:13AM +0800, Jason Wang wrote:
> > > On Tue, Mar 15, 2022 at 10:43 PM Suwan Kim wrote:
> > > > On Tue, Mar 15, 2022 at 04:59:23PM +0800, Jason Wang wrote:
> > > > > On Mon, Mar 14, 2022 at 8:33 PM Suwan Kim
> > > > > wrote:
> > > > >
> > > > > > On Mon, Mar 14, 2022 at 02:14:53PM +0800, Jason Wang wrote:
> > > > > > > 在 2022/3/11 下午11:28, Suwan Kim 写道:
> > > > > > > > diff --git a/include/uapi/linux/virtio_blk.h
> > > > > > b/include/uapi/linux/virtio_blk.h
> > > > > > > > index d888f013d9ff..3fcaf937afe1 100644
> > > > > > > > --- a/include/uapi/linux/virtio_blk.h
> > > > > > > > +++ b/include/uapi/linux/virtio_blk.h
> > > > > > > > @@ -119,8 +119,9 @@ struct virtio_blk_config {
> > > > > > > > * deallocation of one or more of the sectors.
> > > > > > > > */
> > > > > > > > __u8 write_zeroes_may_unmap;
> > > > > > > > + __u8 unused1;
> > > > > > > > - __u8 unused1[3];
> > > > > > > > + __virtio16 num_poll_queues;
> > > > > > > >} __attribute__((packed));
> > > > > > >
> > > > > > > This looks like a implementation specific (virtio-blk-pci)
> > > > > > > optimization,
> > > > > > how
> > > > > > > about other implementation like vhost-user-blk?
> > > > > > I didn’t consider vhost-user-blk yet. But does vhost-user-blk also
> > > > > > use vritio_blk_config as kernel-qemu interface?
> > > > > >
> > > > > Yes, but see below.
> > > > >
> > > > >
> > > > > > Does vhost-user-blk need additional modification to support polling
> > > > > > in kernel side?
> > > > > >
> > > > >
> > > > > No, but the issue is, things like polling looks not a good candidate
> > > > > for
> > > > > the attributes belonging to the device but the driver. So I have more
> > > > > questions:
> > > > >
> > > > > 1) what does it really mean for hardware virtio block devices?
> > > > > 2) Does driver polling help for the qemu implementation without
> > > > > polling?
> > > > > 3) Using blk_config means we can only get the benefit from the new
> > > > > device
> > > > 1) what does it really mean for hardware virtio block devices?
> > > > 3) Using blk_config means we can only get the benefit from the new
> > > > device
> > > >
> > > > This patch adds dedicated HW queue for polling purpose to virtio
> > > > block device.
> > > >
> > > > So I think it can be a new hw feature. And it can be a new device
> > > > that supports hw poll queue.
> > > One possible issue is that the "poll" looks more like a
> > > software/driver concept other than the device/hardware.
> > >
> > > > BTW, I have other idea about it.
> > > >
> > > > How about adding “num-poll-queues" property as a driver parameter
> > > > like NVMe driver, not to QEMU virtio-blk-pci property?
> > > It should be fine, but we need to listen to others.
> > To Michael, Stefan, Max
> >
> > How about using driver parameter instead of virio_blk_config?
>
> Yes. I mentioned that virtio_blk_config shouldn't change.
There are pros and cons to module parameters and configuration space
fields. Both are valid but feel free to drop the configuration space
field.
On note about module parameters: if the guest has a virtio-blk device
with a root file system and another one for benchmarking then it's
unfortunate that the number of poll queues module parameter affects both
devices.
Is there a way to set a module parameter for a specific device instance?
I guess you'd need something else like a sysfs attribute instead but the
implementation is more complex.
I'm fine with a module parameter.
Stefan
signature.asc
Description: PGP signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v5 01/11] driver: platform: Add helper for safer setting of driver_override
On Wed, Mar 16, 2022 at 5:06 PM Krzysztof Kozlowski
wrote:
...
> +int driver_set_override(struct device *dev, const char **override,
> + const char *s, size_t len)
> +{
> + const char *new, *old;
> + char *cp;
> + if (!dev || !override || !s)
> + return -EINVAL;
Sorry, I didn't pay much attention on this. First of all, I would drop
dev checks and simply require that dev should be valid. Do you expect
this can be called when dev is invalid? I would like to hear if it's
anything but theoretical. Second one, is the !s requirement. Do I
understand correctly that the string must be always present? But then
how we NULify the override? Is it possible? Third one is absence of
len check. See below.
> + /*
> +* The stored value will be used in sysfs show callback
> (sysfs_emit()),
> +* which has a length limit of PAGE_SIZE and adds a trailing newline.
> +* Thus we can store one character less to avoid truncation during
> sysfs
> +* show.
> +*/
> + if (len >= (PAGE_SIZE - 1))
> + return -EINVAL;
I would relax this to make sure we can use it if \n is within this limit.
> + cp = strnchr(s, len, '\n');
> + if (cp)
> + len = cp - s;
> +
> + new = kstrndup(s, len, GFP_KERNEL);
Here is a word about the len check.
> + if (!new)
If len == 0, this won't trigger and you have something very
interesting as a result.
One way is to use ZERO_PTR_OR_NULL() another is explicitly check for 0
and issue a (different?) error code.
> + return -ENOMEM;
> +
> + device_lock(dev);
> + old = *override;
> + if (cp != s) {
> + *override = new;
> + } else {
> + kfree(new);
> + *override = NULL;
> + }
> + device_unlock(dev);
> +
> + kfree(old);
> +
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] tools/virtio: Test virtual address range detection
On Tue, Feb 22, 2022 at 11:18:18PM +, Matthew Wilcox wrote:
> On Tue, Feb 22, 2022 at 07:58:33AM +, David Woodhouse wrote:
> > On Tue, 2022-02-22 at 01:31 -0500, Michael S. Tsirkin wrote:
> > > On Mon, Feb 21, 2022 at 05:18:48PM +, David Woodhouse wrote:
> > > >
> > > > [dwoodhou@i7 virtio]$ sudo ~/virtio_test
> > > > Detected virtual address range 0x1000-0x7000
> > > > spurious wakeups: 0x0 started=0x10 completed=0x10
> > > >
> > > > Although in some circumstances I also see a different build failure:
> > > >
> > > > cc -g -O2 -Werror -Wno-maybe-uninitialized -Wall -I. -I../include/ -I
> > > > ../../usr/include/ -Wno-pointer-sign -fno-strict-overflow
> > > > -fno-strict-aliasing -fno-common -MMD -U_FORTIFY_SOURCE -include
> > > > ../../include/linux/kconfig.h -c -o vringh_test.o vringh_test.c
>
> Trying to test this myself ...
>
> $ cd tools/virtio/
> $ make
> ...
> cc -lpthread virtio_test.o virtio_ring.o -o virtio_test
> /usr/bin/ld: virtio_ring.o: in function `spin_lock':
> /home/willy/kernel/folio/tools/virtio/./linux/spinlock.h:16: undefined
> reference to `pthread_spin_lock'
>
> So this is not the only problem here?
FYI, this fixes it for me:
diff --git a/tools/virtio/Makefile b/tools/virtio/Makefile
index 0d7bbe49359d..83b6a522d0d2 100644
--- a/tools/virtio/Makefile
+++ b/tools/virtio/Makefile
@@ -5,7 +5,7 @@ virtio_test: virtio_ring.o virtio_test.o
vringh_test: vringh_test.o vringh.o virtio_ring.o
CFLAGS += -g -O2 -Werror -Wno-maybe-uninitialized -Wall -I. -I../include/ -I
../../usr/include/ -Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing
-fno-common -MMD -U_FORTIFY_SOURCE -include ../../include/linux/kconfig.h
-LDFLAGS += -lpthread
+LDFLAGS += -pthread
vpath %.c ../../drivers/virtio ../../drivers/vhost
mod:
${MAKE} -C `pwd`/../.. M=`pwd`/vhost_test V=${V}
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH AUTOSEL 4.9 1/2] virtio_console: break out of buf poll on remove
From: "Michael S. Tsirkin" [ Upstream commit 0e7174b9d5877130fec41fb4a16e0c2ee4958d44 ] A common pattern for device reset is currently: vdev->config->reset(vdev); .. cleanup .. reset prevents new interrupts from arriving and waits for interrupt handlers to finish. However if - as is common - the handler queues a work request which is flushed during the cleanup stage, we have code adding buffers / trying to get buffers while device is reset. Not good. This was reproduced by running modprobe virtio_console modprobe -r virtio_console in a loop. Fix this up by calling virtio_break_device + flush before reset. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1786239 Signed-off-by: Michael S. Tsirkin Signed-off-by: Sasha Levin --- drivers/char/virtio_console.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 2632b0fdb1b5..a6b6dc204c1f 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -2004,6 +2004,13 @@ static void virtcons_remove(struct virtio_device *vdev) list_del(>list); spin_unlock_irq(_lock); + /* Device is going away, exit any polling for buffers */ + virtio_break_device(vdev); + if (use_multiport(portdev)) + flush_work(>control_work); + else + flush_work(>config_work); + /* Disable interrupts for vqs */ vdev->config->reset(vdev); /* Finish up work that's lined up */ -- 2.34.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH AUTOSEL 4.14 1/3] virtio_console: break out of buf poll on remove
From: "Michael S. Tsirkin" [ Upstream commit 0e7174b9d5877130fec41fb4a16e0c2ee4958d44 ] A common pattern for device reset is currently: vdev->config->reset(vdev); .. cleanup .. reset prevents new interrupts from arriving and waits for interrupt handlers to finish. However if - as is common - the handler queues a work request which is flushed during the cleanup stage, we have code adding buffers / trying to get buffers while device is reset. Not good. This was reproduced by running modprobe virtio_console modprobe -r virtio_console in a loop. Fix this up by calling virtio_break_device + flush before reset. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1786239 Signed-off-by: Michael S. Tsirkin Signed-off-by: Sasha Levin --- drivers/char/virtio_console.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 0fb3a8e62e62..2140d401523f 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -2001,6 +2001,13 @@ static void virtcons_remove(struct virtio_device *vdev) list_del(>list); spin_unlock_irq(_lock); + /* Device is going away, exit any polling for buffers */ + virtio_break_device(vdev); + if (use_multiport(portdev)) + flush_work(>control_work); + else + flush_work(>config_work); + /* Disable interrupts for vqs */ vdev->config->reset(vdev); /* Finish up work that's lined up */ -- 2.34.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH AUTOSEL 4.19 2/6] virtio_console: break out of buf poll on remove
From: "Michael S. Tsirkin" [ Upstream commit 0e7174b9d5877130fec41fb4a16e0c2ee4958d44 ] A common pattern for device reset is currently: vdev->config->reset(vdev); .. cleanup .. reset prevents new interrupts from arriving and waits for interrupt handlers to finish. However if - as is common - the handler queues a work request which is flushed during the cleanup stage, we have code adding buffers / trying to get buffers while device is reset. Not good. This was reproduced by running modprobe virtio_console modprobe -r virtio_console in a loop. Fix this up by calling virtio_break_device + flush before reset. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1786239 Signed-off-by: Michael S. Tsirkin Signed-off-by: Sasha Levin --- drivers/char/virtio_console.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index cdf441942bae..ac0b84afabe7 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1985,6 +1985,13 @@ static void virtcons_remove(struct virtio_device *vdev) list_del(>list); spin_unlock_irq(_lock); + /* Device is going away, exit any polling for buffers */ + virtio_break_device(vdev); + if (use_multiport(portdev)) + flush_work(>control_work); + else + flush_work(>config_work); + /* Disable interrupts for vqs */ vdev->config->reset(vdev); /* Finish up work that's lined up */ -- 2.34.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH AUTOSEL 5.4 3/7] virtio_console: break out of buf poll on remove
From: "Michael S. Tsirkin" [ Upstream commit 0e7174b9d5877130fec41fb4a16e0c2ee4958d44 ] A common pattern for device reset is currently: vdev->config->reset(vdev); .. cleanup .. reset prevents new interrupts from arriving and waits for interrupt handlers to finish. However if - as is common - the handler queues a work request which is flushed during the cleanup stage, we have code adding buffers / trying to get buffers while device is reset. Not good. This was reproduced by running modprobe virtio_console modprobe -r virtio_console in a loop. Fix this up by calling virtio_break_device + flush before reset. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1786239 Signed-off-by: Michael S. Tsirkin Signed-off-by: Sasha Levin --- drivers/char/virtio_console.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index b453029487a1..2660a0c5483a 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1961,6 +1961,13 @@ static void virtcons_remove(struct virtio_device *vdev) list_del(>list); spin_unlock_irq(_lock); + /* Device is going away, exit any polling for buffers */ + virtio_break_device(vdev); + if (use_multiport(portdev)) + flush_work(>control_work); + else + flush_work(>config_work); + /* Disable interrupts for vqs */ vdev->config->reset(vdev); /* Finish up work that's lined up */ -- 2.34.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH AUTOSEL 5.10 08/12] tools/virtio: fix virtio_test execution
From: Stefano Garzarella [ Upstream commit 32f1b53fe8f03d962423ba81f8e92af5839814da ] virtio_test hangs on __vring_new_virtqueue() because `vqs_list_lock` is not initialized. Let's initialize it in vdev_info_init(). Signed-off-by: Stefano Garzarella Link: https://lore.kernel.org/r/20220118150631.167015-1-sgarz...@redhat.com Signed-off-by: Michael S. Tsirkin Acked-by: Jason Wang Signed-off-by: Sasha Levin --- tools/virtio/virtio_test.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c index cb3f29c09aff..23f142af544a 100644 --- a/tools/virtio/virtio_test.c +++ b/tools/virtio/virtio_test.c @@ -130,6 +130,7 @@ static void vdev_info_init(struct vdev_info* dev, unsigned long long features) memset(dev, 0, sizeof *dev); dev->vdev.features = features; INIT_LIST_HEAD(>vdev.vqs); + spin_lock_init(>vdev.vqs_list_lock); dev->buf_size = 1024; dev->buf = malloc(dev->buf_size); assert(dev->buf); -- 2.34.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH AUTOSEL 5.10 06/12] vdpa/mlx5: should verify CTRL_VQ feature exists for MQ
From: Si-Wei Liu
[ Upstream commit 30c22f3816ffef8aa21a000e93c4ee1402a6ea65 ]
Per VIRTIO v1.1 specification, section 5.1.3.1 Feature bit requirements:
"VIRTIO_NET_F_MQ Requires VIRTIO_NET_F_CTRL_VQ".
There's assumption in the mlx5_vdpa multiqueue code that MQ must come
together with CTRL_VQ. However, there's nowhere in the upper layer to
guarantee this assumption would hold. Were there an untrusted driver
sending down MQ without CTRL_VQ, it would compromise various spots for
e.g. is_index_valid() and is_ctrl_vq_idx(). Although this doesn't end
up with immediate panic or security loophole as of today's code, the
chance for this to be taken advantage of due to future code change is
not zero.
Harden the crispy assumption by failing the set_driver_features() call
when seeing (MQ && !CTRL_VQ). For that end, verify_min_features() is
renamed to verify_driver_features() to reflect the fact that it now does
more than just validate the minimum features. verify_driver_features()
is now used to accommodate various checks against the driver features
for set_driver_features().
Signed-off-by: Si-Wei Liu
Link:
https://lore.kernel.org/r/1642206481-30721-3-git-send-email-si-wei@oracle.com
Signed-off-by: Michael S. Tsirkin
Reviewed-by: Eli Cohen
Acked-by: Jason Wang
Signed-off-by: Sasha Levin
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 65d6f8fd81e7..577ff786f11b 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1482,11 +1482,25 @@ static u64 mlx5_vdpa_get_features(struct vdpa_device
*vdev)
return ndev->mvdev.mlx_features;
}
-static int verify_min_features(struct mlx5_vdpa_dev *mvdev, u64 features)
+static int verify_driver_features(struct mlx5_vdpa_dev *mvdev, u64 features)
{
+ /* Minimum features to expect */
if (!(features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM)))
return -EOPNOTSUPP;
+ /* Double check features combination sent down by the driver.
+* Fail invalid features due to absence of the depended feature.
+*
+* Per VIRTIO v1.1 specification, section 5.1.3.1 Feature bit
+* requirements: "VIRTIO_NET_F_MQ Requires VIRTIO_NET_F_CTRL_VQ".
+* By failing the invalid features sent down by untrusted drivers,
+* we're assured the assumption made upon is_index_valid() and
+* is_ctrl_vq_idx() will not be compromised.
+*/
+ if ((features & (BIT_ULL(VIRTIO_NET_F_MQ) |
BIT_ULL(VIRTIO_NET_F_CTRL_VQ))) ==
+BIT_ULL(VIRTIO_NET_F_MQ))
+ return -EINVAL;
+
return 0;
}
@@ -1544,7 +1558,7 @@ static int mlx5_vdpa_set_features(struct vdpa_device
*vdev, u64 features)
print_features(mvdev, features, true);
- err = verify_min_features(mvdev, features);
+ err = verify_driver_features(mvdev, features);
if (err)
return err;
--
2.34.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH AUTOSEL 5.10 05/12] virtio_console: break out of buf poll on remove
From: "Michael S. Tsirkin" [ Upstream commit 0e7174b9d5877130fec41fb4a16e0c2ee4958d44 ] A common pattern for device reset is currently: vdev->config->reset(vdev); .. cleanup .. reset prevents new interrupts from arriving and waits for interrupt handlers to finish. However if - as is common - the handler queues a work request which is flushed during the cleanup stage, we have code adding buffers / trying to get buffers while device is reset. Not good. This was reproduced by running modprobe virtio_console modprobe -r virtio_console in a loop. Fix this up by calling virtio_break_device + flush before reset. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1786239 Signed-off-by: Michael S. Tsirkin Signed-off-by: Sasha Levin --- drivers/char/virtio_console.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 673522874cec..3dd4deb60adb 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1959,6 +1959,13 @@ static void virtcons_remove(struct virtio_device *vdev) list_del(>list); spin_unlock_irq(_lock); + /* Device is going away, exit any polling for buffers */ + virtio_break_device(vdev); + if (use_multiport(portdev)) + flush_work(>control_work); + else + flush_work(>config_work); + /* Disable interrupts for vqs */ vdev->config->reset(vdev); /* Finish up work that's lined up */ -- 2.34.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH AUTOSEL 5.15 09/13] tools/virtio: fix virtio_test execution
From: Stefano Garzarella [ Upstream commit 32f1b53fe8f03d962423ba81f8e92af5839814da ] virtio_test hangs on __vring_new_virtqueue() because `vqs_list_lock` is not initialized. Let's initialize it in vdev_info_init(). Signed-off-by: Stefano Garzarella Link: https://lore.kernel.org/r/20220118150631.167015-1-sgarz...@redhat.com Signed-off-by: Michael S. Tsirkin Acked-by: Jason Wang Signed-off-by: Sasha Levin --- tools/virtio/virtio_test.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c index cb3f29c09aff..23f142af544a 100644 --- a/tools/virtio/virtio_test.c +++ b/tools/virtio/virtio_test.c @@ -130,6 +130,7 @@ static void vdev_info_init(struct vdev_info* dev, unsigned long long features) memset(dev, 0, sizeof *dev); dev->vdev.features = features; INIT_LIST_HEAD(>vdev.vqs); + spin_lock_init(>vdev.vqs_list_lock); dev->buf_size = 1024; dev->buf = malloc(dev->buf_size); assert(dev->buf); -- 2.34.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH AUTOSEL 5.15 07/13] vdpa/mlx5: should verify CTRL_VQ feature exists for MQ
From: Si-Wei Liu
[ Upstream commit 30c22f3816ffef8aa21a000e93c4ee1402a6ea65 ]
Per VIRTIO v1.1 specification, section 5.1.3.1 Feature bit requirements:
"VIRTIO_NET_F_MQ Requires VIRTIO_NET_F_CTRL_VQ".
There's assumption in the mlx5_vdpa multiqueue code that MQ must come
together with CTRL_VQ. However, there's nowhere in the upper layer to
guarantee this assumption would hold. Were there an untrusted driver
sending down MQ without CTRL_VQ, it would compromise various spots for
e.g. is_index_valid() and is_ctrl_vq_idx(). Although this doesn't end
up with immediate panic or security loophole as of today's code, the
chance for this to be taken advantage of due to future code change is
not zero.
Harden the crispy assumption by failing the set_driver_features() call
when seeing (MQ && !CTRL_VQ). For that end, verify_min_features() is
renamed to verify_driver_features() to reflect the fact that it now does
more than just validate the minimum features. verify_driver_features()
is now used to accommodate various checks against the driver features
for set_driver_features().
Signed-off-by: Si-Wei Liu
Link:
https://lore.kernel.org/r/1642206481-30721-3-git-send-email-si-wei@oracle.com
Signed-off-by: Michael S. Tsirkin
Reviewed-by: Eli Cohen
Acked-by: Jason Wang
Signed-off-by: Sasha Levin
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 1afbda216df5..6abbf1a4b06c 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1857,11 +1857,25 @@ static u64 mlx5_vdpa_get_features(struct vdpa_device
*vdev)
return ndev->mvdev.mlx_features;
}
-static int verify_min_features(struct mlx5_vdpa_dev *mvdev, u64 features)
+static int verify_driver_features(struct mlx5_vdpa_dev *mvdev, u64 features)
{
+ /* Minimum features to expect */
if (!(features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM)))
return -EOPNOTSUPP;
+ /* Double check features combination sent down by the driver.
+* Fail invalid features due to absence of the depended feature.
+*
+* Per VIRTIO v1.1 specification, section 5.1.3.1 Feature bit
+* requirements: "VIRTIO_NET_F_MQ Requires VIRTIO_NET_F_CTRL_VQ".
+* By failing the invalid features sent down by untrusted drivers,
+* we're assured the assumption made upon is_index_valid() and
+* is_ctrl_vq_idx() will not be compromised.
+*/
+ if ((features & (BIT_ULL(VIRTIO_NET_F_MQ) |
BIT_ULL(VIRTIO_NET_F_CTRL_VQ))) ==
+BIT_ULL(VIRTIO_NET_F_MQ))
+ return -EINVAL;
+
return 0;
}
@@ -1937,7 +1951,7 @@ static int mlx5_vdpa_set_features(struct vdpa_device
*vdev, u64 features)
print_features(mvdev, features, true);
- err = verify_min_features(mvdev, features);
+ err = verify_driver_features(mvdev, features);
if (err)
return err;
--
2.34.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH AUTOSEL 5.15 06/13] virtio_console: break out of buf poll on remove
From: "Michael S. Tsirkin" [ Upstream commit 0e7174b9d5877130fec41fb4a16e0c2ee4958d44 ] A common pattern for device reset is currently: vdev->config->reset(vdev); .. cleanup .. reset prevents new interrupts from arriving and waits for interrupt handlers to finish. However if - as is common - the handler queues a work request which is flushed during the cleanup stage, we have code adding buffers / trying to get buffers while device is reset. Not good. This was reproduced by running modprobe virtio_console modprobe -r virtio_console in a loop. Fix this up by calling virtio_break_device + flush before reset. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1786239 Signed-off-by: Michael S. Tsirkin Signed-off-by: Sasha Levin --- drivers/char/virtio_console.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 7eaf303a7a86..3adf04766e98 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1956,6 +1956,13 @@ static void virtcons_remove(struct virtio_device *vdev) list_del(>list); spin_unlock_irq(_lock); + /* Device is going away, exit any polling for buffers */ + virtio_break_device(vdev); + if (use_multiport(portdev)) + flush_work(>control_work); + else + flush_work(>config_work); + /* Disable interrupts for vqs */ vdev->config->reset(vdev); /* Finish up work that's lined up */ -- 2.34.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH AUTOSEL 5.16 09/13] tools/virtio: fix virtio_test execution
From: Stefano Garzarella [ Upstream commit 32f1b53fe8f03d962423ba81f8e92af5839814da ] virtio_test hangs on __vring_new_virtqueue() because `vqs_list_lock` is not initialized. Let's initialize it in vdev_info_init(). Signed-off-by: Stefano Garzarella Link: https://lore.kernel.org/r/20220118150631.167015-1-sgarz...@redhat.com Signed-off-by: Michael S. Tsirkin Acked-by: Jason Wang Signed-off-by: Sasha Levin --- tools/virtio/virtio_test.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c index cb3f29c09aff..23f142af544a 100644 --- a/tools/virtio/virtio_test.c +++ b/tools/virtio/virtio_test.c @@ -130,6 +130,7 @@ static void vdev_info_init(struct vdev_info* dev, unsigned long long features) memset(dev, 0, sizeof *dev); dev->vdev.features = features; INIT_LIST_HEAD(>vdev.vqs); + spin_lock_init(>vdev.vqs_list_lock); dev->buf_size = 1024; dev->buf = malloc(dev->buf_size); assert(dev->buf); -- 2.34.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH AUTOSEL 5.16 07/13] vdpa/mlx5: should verify CTRL_VQ feature exists for MQ
From: Si-Wei Liu
[ Upstream commit 30c22f3816ffef8aa21a000e93c4ee1402a6ea65 ]
Per VIRTIO v1.1 specification, section 5.1.3.1 Feature bit requirements:
"VIRTIO_NET_F_MQ Requires VIRTIO_NET_F_CTRL_VQ".
There's assumption in the mlx5_vdpa multiqueue code that MQ must come
together with CTRL_VQ. However, there's nowhere in the upper layer to
guarantee this assumption would hold. Were there an untrusted driver
sending down MQ without CTRL_VQ, it would compromise various spots for
e.g. is_index_valid() and is_ctrl_vq_idx(). Although this doesn't end
up with immediate panic or security loophole as of today's code, the
chance for this to be taken advantage of due to future code change is
not zero.
Harden the crispy assumption by failing the set_driver_features() call
when seeing (MQ && !CTRL_VQ). For that end, verify_min_features() is
renamed to verify_driver_features() to reflect the fact that it now does
more than just validate the minimum features. verify_driver_features()
is now used to accommodate various checks against the driver features
for set_driver_features().
Signed-off-by: Si-Wei Liu
Link:
https://lore.kernel.org/r/1642206481-30721-3-git-send-email-si-wei@oracle.com
Signed-off-by: Michael S. Tsirkin
Reviewed-by: Eli Cohen
Acked-by: Jason Wang
Signed-off-by: Sasha Levin
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index ef6da39ccb3f..ee4385978e6a 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1900,11 +1900,25 @@ static u64 mlx5_vdpa_get_features(struct vdpa_device
*vdev)
return ndev->mvdev.mlx_features;
}
-static int verify_min_features(struct mlx5_vdpa_dev *mvdev, u64 features)
+static int verify_driver_features(struct mlx5_vdpa_dev *mvdev, u64 features)
{
+ /* Minimum features to expect */
if (!(features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM)))
return -EOPNOTSUPP;
+ /* Double check features combination sent down by the driver.
+* Fail invalid features due to absence of the depended feature.
+*
+* Per VIRTIO v1.1 specification, section 5.1.3.1 Feature bit
+* requirements: "VIRTIO_NET_F_MQ Requires VIRTIO_NET_F_CTRL_VQ".
+* By failing the invalid features sent down by untrusted drivers,
+* we're assured the assumption made upon is_index_valid() and
+* is_ctrl_vq_idx() will not be compromised.
+*/
+ if ((features & (BIT_ULL(VIRTIO_NET_F_MQ) |
BIT_ULL(VIRTIO_NET_F_CTRL_VQ))) ==
+BIT_ULL(VIRTIO_NET_F_MQ))
+ return -EINVAL;
+
return 0;
}
@@ -1980,7 +1994,7 @@ static int mlx5_vdpa_set_features(struct vdpa_device
*vdev, u64 features)
print_features(mvdev, features, true);
- err = verify_min_features(mvdev, features);
+ err = verify_driver_features(mvdev, features);
if (err)
return err;
--
2.34.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH AUTOSEL 5.16 06/13] virtio_console: break out of buf poll on remove
From: "Michael S. Tsirkin" [ Upstream commit 0e7174b9d5877130fec41fb4a16e0c2ee4958d44 ] A common pattern for device reset is currently: vdev->config->reset(vdev); .. cleanup .. reset prevents new interrupts from arriving and waits for interrupt handlers to finish. However if - as is common - the handler queues a work request which is flushed during the cleanup stage, we have code adding buffers / trying to get buffers while device is reset. Not good. This was reproduced by running modprobe virtio_console modprobe -r virtio_console in a loop. Fix this up by calling virtio_break_device + flush before reset. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1786239 Signed-off-by: Michael S. Tsirkin Signed-off-by: Sasha Levin --- drivers/char/virtio_console.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 660c5c388c29..f864b17be7e3 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1957,6 +1957,13 @@ static void virtcons_remove(struct virtio_device *vdev) list_del(>list); spin_unlock_irq(_lock); + /* Device is going away, exit any polling for buffers */ + virtio_break_device(vdev); + if (use_multiport(portdev)) + flush_work(>control_work); + else + flush_work(>config_work); + /* Disable interrupts for vqs */ vdev->config->reset(vdev); /* Finish up work that's lined up */ -- 2.34.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 4/8] drm/virtio: Improve DMA API usage for shmem BOs
On 2022-03-14 22:42, Dmitry Osipenko wrote:
DRM API requires the DRM's driver to be backed with the device that can
be used for generic DMA operations. The VirtIO-GPU device can't perform
DMA operations if it uses PCI transport because PCI device driver creates
a virtual VirtIO-GPU device that isn't associated with the PCI. Use PCI's
GPU device for the DRM's device instead of the VirtIO-GPU device and drop
DMA-related hacks from the VirtIO-GPU driver.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_drv.c| 22 +++---
drivers/gpu/drm/virtio/virtgpu_drv.h| 5 +--
drivers/gpu/drm/virtio/virtgpu_kms.c| 7 ++--
drivers/gpu/drm/virtio/virtgpu_object.c | 56 +
drivers/gpu/drm/virtio/virtgpu_vq.c | 13 +++---
5 files changed, 37 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c
b/drivers/gpu/drm/virtio/virtgpu_drv.c
index 5f25a8d15464..8449dad3e65c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -46,9 +46,9 @@ static int virtio_gpu_modeset = -1;
MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
module_param_named(modeset, virtio_gpu_modeset, int, 0400);
-static int virtio_gpu_pci_quirk(struct drm_device *dev, struct virtio_device *vdev)
+static int virtio_gpu_pci_quirk(struct drm_device *dev)
{
- struct pci_dev *pdev = to_pci_dev(vdev->dev.parent);
+ struct pci_dev *pdev = to_pci_dev(dev->dev);
const char *pname = dev_name(>dev);
bool vga = (pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA;
char unique[20];
@@ -101,6 +101,7 @@ static int virtio_gpu_pci_quirk(struct drm_device *dev,
struct virtio_device *vd
static int virtio_gpu_probe(struct virtio_device *vdev)
{
struct drm_device *dev;
+ struct device *dma_dev;
int ret;
if (drm_firmware_drivers_only() && virtio_gpu_modeset == -1)
@@ -109,18 +110,29 @@ static int virtio_gpu_probe(struct virtio_device *vdev)
if (virtio_gpu_modeset == 0)
return -EINVAL;
- dev = drm_dev_alloc(, >dev);
+ /*
+* If GPU's parent is a PCI device, then we will use this PCI device
+* for the DRM's driver device because GPU won't have PCI's IOMMU DMA
+* ops in this case since GPU device is sitting on a separate (from PCI)
+* virtio-bus.
+*/
+ if (!strcmp(vdev->dev.parent->bus->name, "pci"))
Nit: dev_is_pci() ?
However, what about other VirtIO transports? Wouldn't virtio-mmio with
F_ACCESS_PLATFORM be in a similar situation?
Robin.
+ dma_dev = vdev->dev.parent;
+ else
+ dma_dev = >dev;
+
+ dev = drm_dev_alloc(, dma_dev);
if (IS_ERR(dev))
return PTR_ERR(dev);
vdev->priv = dev;
if (!strcmp(vdev->dev.parent->bus->name, "pci")) {
- ret = virtio_gpu_pci_quirk(dev, vdev);
+ ret = virtio_gpu_pci_quirk(dev);
if (ret)
goto err_free;
}
- ret = virtio_gpu_init(dev);
+ ret = virtio_gpu_init(vdev, dev);
if (ret)
goto err_free;
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 0a194aaad419..b2d93cb12ebf 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -100,8 +100,6 @@ struct virtio_gpu_object {
struct virtio_gpu_object_shmem {
struct virtio_gpu_object base;
- struct sg_table *pages;
- uint32_t mapped;
};
struct virtio_gpu_object_vram {
@@ -214,7 +212,6 @@ struct virtio_gpu_drv_cap_cache {
};
struct virtio_gpu_device {
- struct device *dev;
struct drm_device *ddev;
struct virtio_device *vdev;
@@ -282,7 +279,7 @@ extern struct drm_ioctl_desc
virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
void virtio_gpu_create_context(struct drm_device *dev, struct drm_file *file);
/* virtgpu_kms.c */
-int virtio_gpu_init(struct drm_device *dev);
+int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev);
void virtio_gpu_deinit(struct drm_device *dev);
void virtio_gpu_release(struct drm_device *dev);
int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file);
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c
b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 3313b92db531..0d1e3eb61bee 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -110,7 +110,7 @@ static void virtio_gpu_get_capsets(struct virtio_gpu_device
*vgdev,
vgdev->num_capsets = num_capsets;
}
-int virtio_gpu_init(struct drm_device *dev)
+int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
{
static vq_callback_t *callbacks[] = {
virtio_gpu_ctrl_ack, virtio_gpu_cursor_ack
@@ -123,7 +123,7 @@ int virtio_gpu_init(struct drm_device *dev)
u32 num_scanouts,
Re: [PATCH] virtio-blk: support polling I/O
On Tue, Mar 15, 2022 at 10:55:04PM +0900, Suwan Kim wrote:
> On Mon, Mar 14, 2022 at 03:19:01PM +, Stefan Hajnoczi wrote:
> > On Sat, Mar 12, 2022 at 12:28:32AM +0900, Suwan Kim wrote:
> > > This patch supports polling I/O via virtio-blk driver. Polling
> > > feature is enabled based on "VIRTIO_BLK_F_MQ" feature and the number
> > > of polling queues can be set by QEMU virtio-blk-pci property
> > > "num-poll-queues=N". This patch improves the polling I/O throughput
> > > and latency.
> > >
> > > The virtio-blk driver doesn't not have a poll function and a poll
> > > queue and it has been operating in interrupt driven method even if
> > > the polling function is called in the upper layer.
> > >
> > > virtio-blk polling is implemented upon 'batched completion' of block
> > > layer. virtblk_poll() queues completed request to io_comp_batch->req_list
> > > and later, virtblk_complete_batch() calls unmap function and ends
> > > the requests in batch.
> > >
> > > virtio-blk reads the number of queues and poll queues from QEMU
> > > virtio-blk-pci properties ("num-queues=N", "num-poll-queues=M").
> > > It allocates N virtqueues to virtio_blk->vqs[N] and it uses [0..(N-M-1)]
> > > as default queues and [(N-M)..(N-1)] as poll queues. Unlike the default
> > > queues, the poll queues have no callback function.
> > >
> > > Regarding HW-SW queue mapping, the default queue mapping uses the
> > > existing method that condsiders MSI irq vector. But the poll queue
> > > doesn't have an irq, so it uses the regular blk-mq cpu mapping.
> > >
> > > To enable poll queues, "num-poll-queues=N" property of virtio-blk-pci
> > > needs to be added to QEMU command line. For that, I temporarily
> > > implemented the property on QEMU. Please refer to the git repository
> > > below.
> > >
> > > git : https://github.com/asfaca/qemu.git #on master branch commit
> > >
> > > For verifying the improvement, I did Fio polling I/O performance test
> > > with io_uring engine with the options below.
> > > (io_uring, hipri, randread, direct=1, bs=512, iodepth=64 numjobs=N)
> > > I set 4 vcpu and 4 virtio-blk queues - 2 default queues and 2 poll
> > > queues for VM.
> > > (-device virtio-blk-pci,num-queues=4,num-poll-queues=2)
> > > As a result, IOPS and average latency improved about 10%.
> > >
> > > Test result:
> > >
> > > - Fio io_uring poll without virtio-blk poll support
> > > -- numjobs=1 : IOPS = 297K, avg latency = 214.59us
> > > -- numjobs=2 : IOPS = 360K, avg latency = 363.88us
> > > -- numjobs=4 : IOPS = 289K, avg latency = 885.42us
> > >
> > > - Fio io_uring poll with virtio-blk poll support
> > > -- numjobs=1 : IOPS = 332K, avg latency = 192.61us
> > > -- numjobs=2 : IOPS = 371K, avg latency = 348.31us
> > > -- numjobs=4 : IOPS = 321K, avg latency = 795.93us
> >
> > Last year there was a patch series that switched regular queues into
> > polling queues when HIPRI requests were in flight:
> > https://lore.kernel.org/linux-block/20210520141305.355961-1-stefa...@redhat.com/T/
> >
> > The advantage is that polling is possible without prior device
> > configuration, making it easier for users.
> >
> > However, the dynamic approach is a bit more complex and bugs can result
> > in lost irqs (hung I/O). Christoph Hellwig asked for dedicated polling
> > queues, which your patch series now delivers.
> >
> > I think your patch series is worth merging once the comments others have
> > already made have been addressed. I'll keep an eye out for the VIRTIO
> > spec change to extend the virtio-blk configuration space, which needs to
> > be accepted before the Linux can be merged.
>
> Thanks for the feedback :)
> There's a lot of history.. I will try to improve the patch.
>
> It might take some time because it need more discussion about qemu
> device property and I do this in my night time.
I see, it's great that you're making this contribution. Don't worry
about the old patch series I linked. I think your approach is fine.
Stefan
signature.asc
Description: PGP signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v2 0/2] af_vsock: add two new tests for SOCK_SEQPACKET
On Wed, Mar 16, 2022 at 07:25:07AM +, Krasnov Arseniy Vladimirovich wrote:
This adds two tests: for receive timeout and reading to invalid
buffer provided by user. I forgot to put both patches to main
patchset.
Arseniy Krasnov(2):
af_vsock: SOCK_SEQPACKET receive timeout test
af_vsock: SOCK_SEQPACKET broken buffer test
tools/testing/vsock/vsock_test.c | 211 +++
1 file changed, 211 insertions(+)
I think there are only small things to fix, so next series you can
remove RFC (remember to use net-next).
I added the tests to my suite and everything is running correctly.
I also suggest you to solve these little issues that checkpatch has
highlighted to have patches ready for submission :-)
Thanks,
Stefano
$ ./scripts/checkpatch.pl --strict -g master..HEAD
-
Commit 2a1bfb93b51d ("af_vsock: SOCK_SEQPACKET receive timeout test")
-
CHECK: Unnecessary parentheses around 'errno != EAGAIN'
#70: FILE: tools/testing/vsock/vsock_test.c:434:
+ if ((read(fd, , sizeof(dummy)) != -1) ||
+ (errno != EAGAIN)) {
WARNING: From:/Signed-off-by: email name mismatch: 'From: Krasnov Arseniy Vladimirovich
' != 'Signed-off-by: Arseniy Krasnov
'
total: 0 errors, 1 warnings, 1 checks, 97 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
Commit 2a1bfb93b51d ("af_vsock: SOCK_SEQPACKET receive timeout test") has style
problems, please review.
---
Commit 9176bcabcdd7 ("af_vsock: SOCK_SEQPACKET broken buffer test")
---
CHECK: Comparison to NULL could be written "!buf1"
#51: FILE: tools/testing/vsock/vsock_test.c:486:
+ if (buf1 == NULL) {
CHECK: Comparison to NULL could be written "!buf2"
#57: FILE: tools/testing/vsock/vsock_test.c:492:
+ if (buf2 == NULL) {
CHECK: Please don't use multiple blank lines
#152: FILE: tools/testing/vsock/vsock_test.c:587:
+
+
WARNING: From:/Signed-off-by: email name mismatch: 'From: Krasnov Arseniy Vladimirovich
' != 'Signed-off-by: Arseniy Krasnov
'
total: 0 errors, 1 warnings, 3 checks, 150 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
Commit 9176bcabcdd7 ("af_vsock: SOCK_SEQPACKET broken buffer test") has style
problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v4 08/11] vdpa: Use helper for safer setting of driver_override
On Sat, Mar 12, 2022 at 02:28:53PM +0100, Krzysztof Kozlowski wrote:
> Use a helper to set driver_override to reduce amount of duplicated code.
>
> Signed-off-by: Krzysztof Kozlowski
Acked-by: Michael S. Tsirkin
feel free to merge with the rest of the patchset.
> ---
> drivers/vdpa/vdpa.c | 29 -
> include/linux/vdpa.h | 4 +++-
> 2 files changed, 7 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index 1ea525433a5c..2dabed1df35c 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -77,32 +77,11 @@ static ssize_t driver_override_store(struct device *dev,
>const char *buf, size_t count)
> {
> struct vdpa_device *vdev = dev_to_vdpa(dev);
> - const char *driver_override, *old;
> - char *cp;
> + int ret;
>
> - /* 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);
> + ret = driver_set_override(dev, >driver_override, buf, count);
> + if (ret)
> + return ret;
>
> return count;
> }
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index 721089bb4c84..37117404660e 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -64,7 +64,9 @@ 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
> + * @driver_override: driver name to force a match; do not set directly,
> + * because core frees it; use driver_set_override() to
> + * set or clear it.
> * @config: the configuration ops for this device.
> * @cf_mutex: Protects get and set access to configuration layout.
> * @index: device index
> --
> 2.32.0
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v2 2/2] af_vsock: SOCK_SEQPACKET broken buffer test
On Wed, Mar 16, 2022 at 07:29:28AM +, Krasnov Arseniy Vladimirovich wrote:
Add test where sender sends two message, each with own
data pattern. Reader tries to read first to broken buffer:
it has three pages size, but middle page is unmapped. Then,
reader tries to read second message to valid buffer. Test
checks, that uncopied part of first message was dropped
and thus not copied as part of second message.
Signed-off-by: Arseniy Krasnov
---
v1 -> v2:
1) Use 'fprintf()' instead of 'perror()' where 'errno' variable
is not affected.
2) Replace word "invalid" -> "unexpected".
tools/testing/vsock/vsock_test.c | 132 +++
1 file changed, 132 insertions(+)
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index 6d7648cce5aa..1132bcd8ddb7 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include "timeout.h"
#include "control.h"
@@ -465,6 +466,132 @@ static void test_seqpacket_timeout_server(const struct
test_opts *opts)
close(fd);
}
+#define BUF_PATTERN_1 'a'
+#define BUF_PATTERN_2 'b'
+
+static void test_seqpacket_invalid_rec_buffer_client(const struct test_opts
*opts)
+{
+ int fd;
+ unsigned char *buf1;
+ unsigned char *buf2;
+ int buf_size = getpagesize() * 3;
+
+ fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
+ if (fd < 0) {
+ perror("connect");
+ exit(EXIT_FAILURE);
+ }
+
+ buf1 = malloc(buf_size);
+ if (buf1 == NULL) {
+ perror("'malloc()' for 'buf1'");
+ exit(EXIT_FAILURE);
+ }
+
+ buf2 = malloc(buf_size);
+ if (buf2 == NULL) {
+ perror("'malloc()' for 'buf2'");
+ exit(EXIT_FAILURE);
+ }
+
+ memset(buf1, BUF_PATTERN_1, buf_size);
+ memset(buf2, BUF_PATTERN_2, buf_size);
+
+ if (send(fd, buf1, buf_size, 0) != buf_size) {
+ perror("send failed");
+ exit(EXIT_FAILURE);
+ }
+
+ if (send(fd, buf2, buf_size, 0) != buf_size) {
+ perror("send failed");
+ exit(EXIT_FAILURE);
+ }
+
+ close(fd);
+}
+
+static void test_seqpacket_invalid_rec_buffer_server(const struct test_opts
*opts)
+{
+ int fd;
+ unsigned char *broken_buf;
+ unsigned char *valid_buf;
+ int page_size = getpagesize();
+ int buf_size = page_size * 3;
+ ssize_t res;
+ int prot = PROT_READ | PROT_WRITE;
+ int flags = MAP_PRIVATE | MAP_ANONYMOUS;
+ int i;
+
+ fd = vsock_seqpacket_accept(VMADDR_CID_ANY, 1234, NULL);
+ if (fd < 0) {
+ perror("accept");
+ exit(EXIT_FAILURE);
+ }
+
+ /* Setup first buffer. */
+ broken_buf = mmap(NULL, buf_size, prot, flags, -1, 0);
+ if (broken_buf == MAP_FAILED) {
+ perror("mmap for 'broken_buf'");
+ exit(EXIT_FAILURE);
+ }
+
+ /* Unmap "hole" in buffer. */
+ if (munmap(broken_buf + page_size, page_size)) {
+ perror("'broken_buf' setup");
+ exit(EXIT_FAILURE);
+ }
+
+ valid_buf = mmap(NULL, buf_size, prot, flags, -1, 0);
+ if (valid_buf == MAP_FAILED) {
+ perror("mmap for 'valid_buf'");
+ exit(EXIT_FAILURE);
+ }
+
+ /* Try to fill buffer with unmapped middle. */
+ res = read(fd, broken_buf, buf_size);
+ if (res != -1) {
+ fprintf(stderr,
+ "expected 'broken_buf' read(2) failure, got %zi\n",
+ res);
+ exit(EXIT_FAILURE);
+ }
+
+ if (errno != ENOMEM) {
+ perror("unexpected errno of 'broken_buf'");
+ exit(EXIT_FAILURE);
+ }
+
+ /* Try to fill valid buffer. */
+ res = read(fd, valid_buf, buf_size);
+ if (res < 0) {
+ perror("unexpected 'valid_buf' read(2) failure");
+ exit(EXIT_FAILURE);
+ }
+
+ if (res != buf_size) {
+ fprintf(stderr,
+ "invalid 'valid_buf' read(2), got %zi, expected %i\n",
+ res, buf_size);
I would suggest to use always the same pattern in the error messages:
"expected X, got Y".
The rest LGTM.
+ exit(EXIT_FAILURE);
+ }
+
+ for (i = 0; i < buf_size; i++) {
+ if (valid_buf[i] != BUF_PATTERN_2) {
+ fprintf(stderr,
+ "invalid pattern for 'valid_buf' at %i, expected
%hhX, got %hhX\n",
+ i, BUF_PATTERN_2, valid_buf[i]);
+ exit(EXIT_FAILURE);
+ }
+ }
+
+
+ /* Unmap buffers. */
+ munmap(broken_buf, page_size);
+ munmap(broken_buf + page_size * 2, page_size);
+ munmap(valid_buf, buf_size);
+
Re: [RFC PATCH v2 1/2] af_vsock: SOCK_SEQPACKET receive timeout test
On Wed, Mar 16, 2022 at 07:27:45AM +, Krasnov Arseniy Vladimirovich wrote:
Test for receive timeout check: connection is established,
receiver sets timeout, but sender does nothing. Receiver's
'read()' call must return EAGAIN.
Signed-off-by: Arseniy Krasnov
---
v1 -> v2:
1) Check amount of time spent in 'read()'.
The patch looks correct to me, but since it's an RFC and you have to
send another version anyway, here are some minor suggestions :-)
tools/testing/vsock/vsock_test.c | 79
1 file changed, 79 insertions(+)
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index 2a3638c0a008..6d7648cce5aa 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include "timeout.h"
#include "control.h"
@@ -391,6 +392,79 @@ static void test_seqpacket_msg_trunc_server(const struct
test_opts *opts)
close(fd);
}
+static time_t current_nsec(void)
+{
+ struct timespec ts;
+
+ if (clock_gettime(CLOCK_REALTIME, )) {
+ perror("clock_gettime(3) failed");
+ exit(EXIT_FAILURE);
+ }
+
+ return (ts.tv_sec * 10ULL) + ts.tv_nsec;
+}
+
+#define RCVTIMEO_TIMEOUT_SEC 1
+#define READ_OVERHEAD_NSEC 25000 /* 0.25 sec */
+
+static void test_seqpacket_timeout_client(const struct test_opts *opts)
+{
+ int fd;
+ struct timeval tv;
+ char dummy;
+ time_t read_enter_ns;
+ time_t read_overhead_ns;
+
+ fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
+ if (fd < 0) {
+ perror("connect");
+ exit(EXIT_FAILURE);
+ }
+
+ tv.tv_sec = RCVTIMEO_TIMEOUT_SEC;
+ tv.tv_usec = 0;
+
+ if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, (void *), sizeof(tv)) ==
-1) {
+ perror("setsockopt 'SO_RCVTIMEO'");
+ exit(EXIT_FAILURE);
+ }
+
+ read_enter_ns = current_nsec();
+
+ if ((read(fd, , sizeof(dummy)) != -1) ||
+ (errno != EAGAIN)) {
Here we can split in 2 checks like in patch 2, since if read() return
value is >= 0, errno is not set.
+ perror("EAGAIN expected");
+ exit(EXIT_FAILURE);
+ }
+
+ read_overhead_ns = current_nsec() - read_enter_ns -
+ 10ULL * RCVTIMEO_TIMEOUT_SEC;
+
+ if (read_overhead_ns > READ_OVERHEAD_NSEC) {
+ fprintf(stderr,
+ "too much time in read(2) with SO_RCVTIMEO: %lu ns\n",
+ read_overhead_ns);
What about printing also the expected overhead?
+ exit(EXIT_FAILURE);
+ }
+
+ control_writeln("WAITDONE");
+ close(fd);
+}
+
+static void test_seqpacket_timeout_server(const struct test_opts *opts)
+{
+ int fd;
+
+ fd = vsock_seqpacket_accept(VMADDR_CID_ANY, 1234, NULL);
+ if (fd < 0) {
+ perror("accept");
+ exit(EXIT_FAILURE);
+ }
+
+ control_expectln("WAITDONE");
+ close(fd);
+}
+
static struct test_case test_cases[] = {
{
.name = "SOCK_STREAM connection reset",
@@ -431,6 +505,11 @@ static struct test_case test_cases[] = {
.run_client = test_seqpacket_msg_trunc_client,
.run_server = test_seqpacket_msg_trunc_server,
},
+ {
+ .name = "SOCK_SEQPACKET timeout",
+ .run_client = test_seqpacket_timeout_client,
+ .run_server = test_seqpacket_timeout_server,
+ },
{},
};
--
2.25.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v1 1/2] vdpa: Add support for querying vendor statistics
On 3/15/2022 2:10 AM, Eli Cohen wrote:
<...snip...>
Say you got a vdpa net device created with 4 data queue pairs and a
control vq. On boot some guest firmware may support just F_CTRL_VQ but
not F_MQ, then the index for the control vq in guest ends up with 2, as
in this case there's only a single queue pair enabled for rx (index 0)
and tx (index 1). From the host driver (e.g. mlx5_vdpa) perspective, the
control vq is the last vq following 8
If the host sees F_MQ was not negotiated but F_CTRL_VQ was, then it knows
that control VQ index is 2
Right, but I don't see this feature negotiation info getting returned
from your vdpa_dev_vendor_stats_fill(), or did I miss something? How do
you plan for host user to get this info? If you meant another "vdpa dev
show" command to query negotiated features ahead, this won't get the
same lock protected as the time you run the stat query. It's very easy
to miss that ephemeral queue index.
data vqs of all 4 pairs, hence got
the 8th index in the rank. Since F_MQ is not negotiated and only 1 data
queue pair enabled, in such event only host qindex 0,1 and 8 have vendor
stats available, and the rest of qindex would get invalid/empty stat.
Later on say boot continues towards loading the Linux virtio driver,
then guest could successfully negotiate both F_CTRL_VQ and F_MQ
features. In this case, all 8 data virtqueues are fully enabled, the
index for the control vq ends up as 8, following tightly after all the 4
data queue pairs. Only until both features are negotiated, the guest and
host are able to see consistent view in identifying the control vq.
Since F_MQ is negotiated, all host queues, indexed from 0 through 8,
should have vendor stats available.
That's why I said the guest qindex is ephemeral and hard to predict
subjected to negotiated features, but host qindex is reliable and more
eligible for command line identification purpose.
<...snip...>
So what are you actually proposing? Display received and completed descriptors
per queue index without further interpretation?
I'd suggest using a more stable queue id i.e. the host queue index to
represent the qidx (which seems to be what you're doing now?), and
displaying both the host qindex (queue_index_device in the example
below), as well as the guest's (queue_index_driver as below) in the output:
Given that per vdpa device you can display statistics only after features have
been negotiated, you can always know the correct queue index for the control
VQ.
The stats can be displayed only after features are negotiated, and only
when the corresponding queue is enabled. If you know it from "vdpa dev
show" on day 1 that the control vq and mq features are negotiated, but
then on day2 you got nothing for the predicted control vq index, what
would you recommend the host admin to do to get the right qindex again?
Re-run the stat query on the same queue index, or check the "vdpa dev
show" output again on day 3? This CLI design makes cloud administrator
really challenging to follow the dynamics of guest activities were to
manage hundreds or thousands of virtual machines...
It would be easier, in my opinion, to grasp some well-defined handle
that is easily predictable or fixed across the board, for looking up the
control virtqueue. This could be a constant host queue index, or a
special magic keyword like "qidx ctrlvq". If cloud admin runs vstat
query on the control vq using a determined handle but get nothing back,
then s/he knows *for sure* the control vq was not available for some
reason at the point when the stat was being collected. S/he doesn't even
need to care negotiated status via "vdpa dev show" at all. Why bother?
Do you still hold see your proposal required?
Yes, this is essential to any cloud admin that runs stat query on all of
the queues on periodic basis. You'd get some deterministic without
blindly guessing or bothering other irrelevant command.
Thanks,
-Siwei
$ vdpa -jp dev vstats show vdpa-a qidx 8
{
"vstats": {
"vdpa-a": {
"queue_stats": [{
"queue_index_device": 8,
"queue_index_driver": 2,
"queue_type": "control_vq",
"stat_name": [ "received_desc","completed_desc" ],
"stat_value": [ 417776,417775 ],
}]
}
}
}
Optionally, user may use guest queue index gqidx, which is kind of an
ephemeral ID and F_MQ negotiation depended, to query the stat on a
specific guest queue:
$ vdpa -jp dev vstats show vdpa-a gqidx 2
{
"vstats": {
"vdpa-a": {
"queue_stats": [{
"queue_index_device": 8,
"queue_index_driver": 2,
"queue_type": "control_vq",
"stat_name": [ "received_desc","completed_desc" ],
"stat_value": [ 417776,417775 ],
}]
}
}
}
Thanks,
-Siwei
Thanks,
-Siwei
Regards,
-Siwei
Looks
