The branch main has been updated by bryanv:
URL:
https://cgit.FreeBSD.org/src/commit/?id=e6cc42f181ce346a294ae7aa1d1c321fdc90f241
commit e6cc42f181ce346a294ae7aa1d1c321fdc90f241
Author: Bryan Venteicher <bry...@freebsd.org>
AuthorDate: 2021-01-19 04:55:26 +0000
Commit: Bryan Venteicher <bry...@freebsd.org>
CommitDate: 2021-01-19 04:55:26 +0000
virtio: Handle possible failure of virtio_finalize_features()
Try to standardize how drivers negotiate feature and the
function names
Reviewed by: grehan (mentor)
Differential Revision: https://reviews.freebsd.org/D27930
---
sys/dev/virtio/balloon/virtio_balloon.c | 33 +++++++++++++++------
sys/dev/virtio/block/virtio_blk.c | 28 ++++++++++++------
sys/dev/virtio/console/virtio_console.c | 25 ++++++++++------
sys/dev/virtio/network/if_vtnet.c | 25 ++++++++++------
sys/dev/virtio/pci/virtio_pci_modern.c | 2 --
sys/dev/virtio/random/virtio_random.c | 51 +++++++++++++++++++++++----------
sys/dev/virtio/scsi/virtio_scsi.c | 50 ++++++++++++++++++++++----------
7 files changed, 149 insertions(+), 65 deletions(-)
diff --git a/sys/dev/virtio/balloon/virtio_balloon.c
b/sys/dev/virtio/balloon/virtio_balloon.c
index 0973528887c5..848dd4e9a7f5 100644
--- a/sys/dev/virtio/balloon/virtio_balloon.c
+++ b/sys/dev/virtio/balloon/virtio_balloon.c
@@ -90,7 +90,8 @@ static int vtballoon_attach(device_t);
static int vtballoon_detach(device_t);
static int vtballoon_config_change(device_t);
-static void vtballoon_negotiate_features(struct vtballoon_softc *);
+static int vtballoon_negotiate_features(struct vtballoon_softc *);
+static int vtballoon_setup_features(struct vtballoon_softc *);
static int vtballoon_alloc_virtqueues(struct vtballoon_softc *);
static void vtballoon_vq_intr(void *);
@@ -110,7 +111,7 @@ static void vtballoon_free_page(struct vtballoon_softc *,
vm_page_t);
static int vtballoon_sleep(struct vtballoon_softc *);
static void vtballoon_thread(void *);
-static void vtballoon_add_sysctl(struct vtballoon_softc *);
+static void vtballoon_setup_sysctl(struct vtballoon_softc *);
#define vtballoon_modern(_sc) \
(((_sc)->vtballoon_features & VIRTIO_F_VERSION_1) != 0)
@@ -183,14 +184,18 @@ vtballoon_attach(device_t dev)
sc = device_get_softc(dev);
sc->vtballoon_dev = dev;
+ virtio_set_feature_desc(dev, vtballoon_feature_desc);
VTBALLOON_LOCK_INIT(sc, device_get_nameunit(dev));
TAILQ_INIT(&sc->vtballoon_pages);
- vtballoon_add_sysctl(sc);
+ vtballoon_setup_sysctl(sc);
- virtio_set_feature_desc(dev, vtballoon_feature_desc);
- vtballoon_negotiate_features(sc);
+ error = vtballoon_setup_features(sc);
+ if (error) {
+ device_printf(dev, "cannot setup features\n");
+ goto fail;
+ }
sc->vtballoon_page_frames = malloc(VTBALLOON_PAGES_PER_REQUEST *
sizeof(uint32_t), M_DEVBUF, M_NOWAIT | M_ZERO);
@@ -276,7 +281,7 @@ vtballoon_config_change(device_t dev)
return (1);
}
-static void
+static int
vtballoon_negotiate_features(struct vtballoon_softc *sc)
{
device_t dev;
@@ -286,7 +291,19 @@ vtballoon_negotiate_features(struct vtballoon_softc *sc)
features = VTBALLOON_FEATURES;
sc->vtballoon_features = virtio_negotiate_features(dev, features);
- virtio_finalize_features(dev);
+ return (virtio_finalize_features(dev));
+}
+
+static int
+vtballoon_setup_features(struct vtballoon_softc *sc)
+{
+ int error;
+
+ error = vtballoon_negotiate_features(sc);
+ if (error)
+ return (error);
+
+ return (0);
}
static int
@@ -559,7 +576,7 @@ vtballoon_thread(void *xsc)
}
static void
-vtballoon_add_sysctl(struct vtballoon_softc *sc)
+vtballoon_setup_sysctl(struct vtballoon_softc *sc)
{
device_t dev;
struct sysctl_ctx_list *ctx;
diff --git a/sys/dev/virtio/block/virtio_blk.c
b/sys/dev/virtio/block/virtio_blk.c
index 08df77d6de5b..6056771e3735 100644
--- a/sys/dev/virtio/block/virtio_blk.c
+++ b/sys/dev/virtio/block/virtio_blk.c
@@ -135,8 +135,8 @@ static int vtblk_ioctl(struct disk *, u_long, void *, int,
static int vtblk_dump(void *, void *, vm_offset_t, off_t, size_t);
static void vtblk_strategy(struct bio *);
-static void vtblk_negotiate_features(struct vtblk_softc *);
-static void vtblk_setup_features(struct vtblk_softc *);
+static int vtblk_negotiate_features(struct vtblk_softc *);
+static int vtblk_setup_features(struct vtblk_softc *);
static int vtblk_maximum_segments(struct vtblk_softc *,
struct virtio_blk_config *);
static int vtblk_alloc_virtqueue(struct vtblk_softc *);
@@ -312,10 +312,10 @@ vtblk_attach(device_t dev)
struct virtio_blk_config blkcfg;
int error;
- virtio_set_feature_desc(dev, vtblk_feature_desc);
-
sc = device_get_softc(dev);
sc->vtblk_dev = dev;
+ virtio_set_feature_desc(dev, vtblk_feature_desc);
+
VTBLK_LOCK_INIT(sc, device_get_nameunit(dev));
bioq_init(&sc->vtblk_bioq);
TAILQ_INIT(&sc->vtblk_dump_queue);
@@ -323,7 +323,12 @@ vtblk_attach(device_t dev)
TAILQ_INIT(&sc->vtblk_req_ready);
vtblk_setup_sysctl(sc);
- vtblk_setup_features(sc);
+
+ error = vtblk_setup_features(sc);
+ if (error) {
+ device_printf(dev, "cannot setup features\n");
+ goto fail;
+ }
vtblk_read_config(sc, &blkcfg);
@@ -572,7 +577,7 @@ vtblk_strategy(struct bio *bp)
VTBLK_UNLOCK(sc);
}
-static void
+static int
vtblk_negotiate_features(struct vtblk_softc *sc)
{
device_t dev;
@@ -583,17 +588,20 @@ vtblk_negotiate_features(struct vtblk_softc *sc)
VTBLK_LEGACY_FEATURES;
sc->vtblk_features = virtio_negotiate_features(dev, features);
- virtio_finalize_features(dev);
+ return (virtio_finalize_features(dev));
}
-static void
+static int
vtblk_setup_features(struct vtblk_softc *sc)
{
device_t dev;
+ int error;
dev = sc->vtblk_dev;
- vtblk_negotiate_features(sc);
+ error = vtblk_negotiate_features(sc);
+ if (error)
+ return (error);
if (virtio_with_feature(dev, VIRTIO_RING_F_INDIRECT_DESC))
sc->vtblk_flags |= VTBLK_FLAG_INDIRECT;
@@ -603,6 +611,8 @@ vtblk_setup_features(struct vtblk_softc *sc)
/* Legacy. */
if (virtio_with_feature(dev, VIRTIO_BLK_F_BARRIER))
sc->vtblk_flags |= VTBLK_FLAG_BARRIER;
+
+ return (0);
}
static int
diff --git a/sys/dev/virtio/console/virtio_console.c
b/sys/dev/virtio/console/virtio_console.c
index 0bd7c982e3f5..315eb59716b4 100644
--- a/sys/dev/virtio/console/virtio_console.c
+++ b/sys/dev/virtio/console/virtio_console.c
@@ -157,8 +157,8 @@ static int vtcon_attach(device_t);
static int vtcon_detach(device_t);
static int vtcon_config_change(device_t);
-static void vtcon_setup_features(struct vtcon_softc *);
-static void vtcon_negotiate_features(struct vtcon_softc *);
+static int vtcon_setup_features(struct vtcon_softc *);
+static int vtcon_negotiate_features(struct vtcon_softc *);
static int vtcon_alloc_scports(struct vtcon_softc *);
static int vtcon_alloc_virtqueues(struct vtcon_softc *);
static void vtcon_read_config(struct vtcon_softc *,
@@ -331,12 +331,16 @@ vtcon_attach(device_t dev)
sc = device_get_softc(dev);
sc->vtcon_dev = dev;
+ virtio_set_feature_desc(dev, vtcon_feature_desc);
mtx_init(&sc->vtcon_mtx, "vtconmtx", NULL, MTX_DEF);
mtx_init(&sc->vtcon_ctrl_tx_mtx, "vtconctrlmtx", NULL, MTX_DEF);
- virtio_set_feature_desc(dev, vtcon_feature_desc);
- vtcon_setup_features(sc);
+ error = vtcon_setup_features(sc);
+ if (error) {
+ device_printf(dev, "cannot setup features\n");
+ goto fail;
+ }
vtcon_read_config(sc, &concfg);
vtcon_determine_max_ports(sc, &concfg);
@@ -428,7 +432,7 @@ vtcon_config_change(device_t dev)
return (0);
}
-static void
+static int
vtcon_negotiate_features(struct vtcon_softc *sc)
{
device_t dev;
@@ -438,22 +442,27 @@ vtcon_negotiate_features(struct vtcon_softc *sc)
features = VTCON_FEATURES;
sc->vtcon_features = virtio_negotiate_features(dev, features);
- virtio_finalize_features(dev);
+ return (virtio_finalize_features(dev));
}
-static void
+static int
vtcon_setup_features(struct vtcon_softc *sc)
{
device_t dev;
+ int error;
dev = sc->vtcon_dev;
- vtcon_negotiate_features(sc);
+ error = vtcon_negotiate_features(sc);
+ if (error)
+ return (error);
if (virtio_with_feature(dev, VIRTIO_CONSOLE_F_SIZE))
sc->vtcon_flags |= VTCON_FLAG_SIZE;
if (virtio_with_feature(dev, VIRTIO_CONSOLE_F_MULTIPORT))
sc->vtcon_flags |= VTCON_FLAG_MULTIPORT;
+
+ return (0);
}
#define VTCON_GET_CONFIG(_dev, _feature, _field, _cfg) \
diff --git a/sys/dev/virtio/network/if_vtnet.c
b/sys/dev/virtio/network/if_vtnet.c
index ed3065b61283..8d0770f5ac2d 100644
--- a/sys/dev/virtio/network/if_vtnet.c
+++ b/sys/dev/virtio/network/if_vtnet.c
@@ -102,8 +102,8 @@ static int vtnet_shutdown(device_t);
static int vtnet_attach_completed(device_t);
static int vtnet_config_change(device_t);
-static void vtnet_negotiate_features(struct vtnet_softc *);
-static void vtnet_setup_features(struct vtnet_softc *);
+static int vtnet_negotiate_features(struct vtnet_softc *);
+static int vtnet_setup_features(struct vtnet_softc *);
static int vtnet_init_rxq(struct vtnet_softc *, int);
static int vtnet_init_txq(struct vtnet_softc *, int);
static int vtnet_alloc_rxtx_queues(struct vtnet_softc *);
@@ -434,7 +434,6 @@ vtnet_attach(device_t dev)
sc = device_get_softc(dev);
sc->vtnet_dev = dev;
-
virtio_set_feature_desc(dev, vtnet_feature_desc);
VTNET_CORE_LOCK_INIT(sc);
@@ -448,7 +447,12 @@ vtnet_attach(device_t dev)
}
vtnet_setup_sysctl(sc);
- vtnet_setup_features(sc);
+
+ error = vtnet_setup_features(sc);
+ if (error) {
+ device_printf(dev, "cannot setup features\n");
+ goto fail;
+ }
error = vtnet_alloc_rx_filters(sc);
if (error) {
@@ -619,7 +623,7 @@ vtnet_config_change(device_t dev)
return (0);
}
-static void
+static int
vtnet_negotiate_features(struct vtnet_softc *sc)
{
device_t dev;
@@ -705,17 +709,20 @@ vtnet_negotiate_features(struct vtnet_softc *sc)
sc->vtnet_features = negotiated_features;
sc->vtnet_negotiated_features = negotiated_features;
- virtio_finalize_features(dev);
+ return (virtio_finalize_features(dev));
}
-static void
+static int
vtnet_setup_features(struct vtnet_softc *sc)
{
device_t dev;
+ int error;
dev = sc->vtnet_dev;
- vtnet_negotiate_features(sc);
+ error = vtnet_negotiate_features(sc);
+ if (error)
+ return (error);
if (virtio_with_feature(dev, VIRTIO_F_VERSION_1))
sc->vtnet_flags |= VTNET_FLAG_MODERN;
@@ -807,6 +814,8 @@ vtnet_setup_features(struct vtnet_softc *sc)
sc->vtnet_flags |= VTNET_FLAG_MQ;
}
}
+
+ return (0);
}
static int
diff --git a/sys/dev/virtio/pci/virtio_pci_modern.c
b/sys/dev/virtio/pci/virtio_pci_modern.c
index 09ac0a1232e7..7029d2ff76ce 100644
--- a/sys/dev/virtio/pci/virtio_pci_modern.c
+++ b/sys/dev/virtio/pci/virtio_pci_modern.c
@@ -462,8 +462,6 @@ vtpci_modern_finalize_features(device_t dev)
/*
* Must re-read the status after setting it to verify the negotiated
* features were accepted by the device.
- *
- * BMV: TODO Drivers need to handle possible failure of this method!
*/
vtpci_modern_set_status(sc, VIRTIO_CONFIG_S_FEATURES_OK);
diff --git a/sys/dev/virtio/random/virtio_random.c
b/sys/dev/virtio/random/virtio_random.c
index 8c01b1cf6ae3..ee3a24bb5513 100644
--- a/sys/dev/virtio/random/virtio_random.c
+++ b/sys/dev/virtio/random/virtio_random.c
@@ -36,7 +36,6 @@ __FBSDID("$FreeBSD$");
#include <sys/malloc.h>
#include <sys/module.h>
#include <sys/sglist.h>
-#include <sys/callout.h>
#include <sys/random.h>
#include <sys/stdatomic.h>
@@ -50,6 +49,7 @@ __FBSDID("$FreeBSD$");
#include <dev/virtio/virtqueue.h>
struct vtrnd_softc {
+ device_t vtrnd_dev;
uint64_t vtrnd_features;
struct virtqueue *vtrnd_vq;
};
@@ -60,8 +60,9 @@ static int vtrnd_probe(device_t);
static int vtrnd_attach(device_t);
static int vtrnd_detach(device_t);
-static void vtrnd_negotiate_features(device_t);
-static int vtrnd_alloc_virtqueue(device_t);
+static int vtrnd_negotiate_features(struct vtrnd_softc *);
+static int vtrnd_setup_features(struct vtrnd_softc *);
+static int vtrnd_alloc_virtqueue(struct vtrnd_softc *);
static int vtrnd_harvest(struct vtrnd_softc *, void *, size_t *);
static unsigned vtrnd_read(void *, unsigned);
@@ -142,11 +143,16 @@ vtrnd_attach(device_t dev)
int error;
sc = device_get_softc(dev);
-
+ sc->vtrnd_dev = dev;
virtio_set_feature_desc(dev, vtrnd_feature_desc);
- vtrnd_negotiate_features(dev);
- error = vtrnd_alloc_virtqueue(dev);
+ error = vtrnd_setup_features(sc);
+ if (error) {
+ device_printf(dev, "cannot setup features\n");
+ goto fail;
+ }
+
+ error = vtrnd_alloc_virtqueue(sc);
if (error) {
device_printf(dev, "cannot allocate virtqueue\n");
goto fail;
@@ -182,23 +188,38 @@ vtrnd_detach(device_t dev)
return (0);
}
-static void
-vtrnd_negotiate_features(device_t dev)
+static int
+vtrnd_negotiate_features(struct vtrnd_softc *sc)
{
- struct vtrnd_softc *sc;
+ device_t dev;
+ uint64_t features;
- sc = device_get_softc(dev);
- sc->vtrnd_features = virtio_negotiate_features(dev, VTRND_FEATURES);
- virtio_finalize_features(dev);
+ dev = sc->vtrnd_dev;
+ features = VTRND_FEATURES;
+
+ sc->vtrnd_features = virtio_negotiate_features(dev, features);
+ return (virtio_finalize_features(dev));
}
static int
-vtrnd_alloc_virtqueue(device_t dev)
+vtrnd_setup_features(struct vtrnd_softc *sc)
{
- struct vtrnd_softc *sc;
+ int error;
+
+ error = vtrnd_negotiate_features(sc);
+ if (error)
+ return (error);
+
+ return (0);
+}
+
+static int
+vtrnd_alloc_virtqueue(struct vtrnd_softc *sc)
+{
+ device_t dev;
struct vq_alloc_info vq_info;
- sc = device_get_softc(dev);
+ dev = sc->vtrnd_dev;
VQ_ALLOC_INFO_INIT(&vq_info, 0, NULL, sc, &sc->vtrnd_vq,
"%s request", device_get_nameunit(dev));
diff --git a/sys/dev/virtio/scsi/virtio_scsi.c
b/sys/dev/virtio/scsi/virtio_scsi.c
index f4c716af3725..737b6d0a7a42 100644
--- a/sys/dev/virtio/scsi/virtio_scsi.c
+++ b/sys/dev/virtio/scsi/virtio_scsi.c
@@ -76,7 +76,8 @@ static int vtscsi_detach(device_t);
static int vtscsi_suspend(device_t);
static int vtscsi_resume(device_t);
-static void vtscsi_negotiate_features(struct vtscsi_softc *);
+static int vtscsi_negotiate_features(struct vtscsi_softc *);
+static int vtscsi_setup_features(struct vtscsi_softc *);
static void vtscsi_read_config(struct vtscsi_softc *,
struct virtio_scsi_config *);
static int vtscsi_maximum_segments(struct vtscsi_softc *, int);
@@ -184,7 +185,7 @@ static void vtscsi_disable_vqs_intr(struct vtscsi_softc *);
static void vtscsi_enable_vqs_intr(struct vtscsi_softc *);
static void vtscsi_get_tunables(struct vtscsi_softc *);
-static void vtscsi_add_sysctl(struct vtscsi_softc *);
+static void vtscsi_setup_sysctl(struct vtscsi_softc *);
static void vtscsi_printf_req(struct vtscsi_request *, const char *,
const char *, ...);
@@ -285,22 +286,19 @@ vtscsi_attach(device_t dev)
sc = device_get_softc(dev);
sc->vtscsi_dev = dev;
+ virtio_set_feature_desc(dev, vtscsi_feature_desc);
VTSCSI_LOCK_INIT(sc, device_get_nameunit(dev));
TAILQ_INIT(&sc->vtscsi_req_free);
vtscsi_get_tunables(sc);
- vtscsi_add_sysctl(sc);
-
- virtio_set_feature_desc(dev, vtscsi_feature_desc);
- vtscsi_negotiate_features(sc);
+ vtscsi_setup_sysctl(sc);
- if (virtio_with_feature(dev, VIRTIO_RING_F_INDIRECT_DESC))
- sc->vtscsi_flags |= VTSCSI_FLAG_INDIRECT;
- if (virtio_with_feature(dev, VIRTIO_SCSI_F_INOUT))
- sc->vtscsi_flags |= VTSCSI_FLAG_BIDIRECTIONAL;
- if (virtio_with_feature(dev, VIRTIO_SCSI_F_HOTPLUG))
- sc->vtscsi_flags |= VTSCSI_FLAG_HOTPLUG;
+ error = vtscsi_setup_features(sc);
+ if (error) {
+ device_printf(dev, "cannot setup features\n");
+ goto fail;
+ }
vtscsi_read_config(sc, &scsicfg);
@@ -413,7 +411,7 @@ vtscsi_resume(device_t dev)
return (0);
}
-static void
+static int
vtscsi_negotiate_features(struct vtscsi_softc *sc)
{
device_t dev;
@@ -423,7 +421,29 @@ vtscsi_negotiate_features(struct vtscsi_softc *sc)
features = VTSCSI_FEATURES;
sc->vtscsi_features = virtio_negotiate_features(dev, features);
- virtio_finalize_features(dev);
+ return (virtio_finalize_features(dev));
+}
+
+static int
+vtscsi_setup_features(struct vtscsi_softc *sc)
+{
+ device_t dev;
+ int error;
+
+ dev = sc->vtscsi_dev;
+
+ error = vtscsi_negotiate_features(sc);
+ if (error)
+ return (error);
+
+ if (virtio_with_feature(dev, VIRTIO_RING_F_INDIRECT_DESC))
+ sc->vtscsi_flags |= VTSCSI_FLAG_INDIRECT;
+ if (virtio_with_feature(dev, VIRTIO_SCSI_F_INOUT))
+ sc->vtscsi_flags |= VTSCSI_FLAG_BIDIRECTIONAL;
+ if (virtio_with_feature(dev, VIRTIO_SCSI_F_HOTPLUG))
+ sc->vtscsi_flags |= VTSCSI_FLAG_HOTPLUG;
+
+ return (0);
}
#define VTSCSI_GET_CONFIG(_dev, _field, _cfg) \
@@ -2287,7 +2307,7 @@ vtscsi_get_tunables(struct vtscsi_softc *sc)
}
static void
-vtscsi_add_sysctl(struct vtscsi_softc *sc)
+vtscsi_setup_sysctl(struct vtscsi_softc *sc)
{
device_t dev;
struct vtscsi_statistics *stats;
_______________________________________________
dev-commits-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/dev-commits-src-all
To unsubscribe, send any mail to "dev-commits-src-all-unsubscr...@freebsd.org"
