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
This commit is contained in:
Bryan Venteicher 2021-01-19 04:55:26 +00:00
parent 2bfab35774
commit e6cc42f181
7 changed files with 149 additions and 65 deletions

View File

@ -90,7 +90,8 @@ static int vtballoon_attach(device_t);
static int vtballoon_detach(device_t); static int vtballoon_detach(device_t);
static int vtballoon_config_change(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 int vtballoon_alloc_virtqueues(struct vtballoon_softc *);
static void vtballoon_vq_intr(void *); 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 int vtballoon_sleep(struct vtballoon_softc *);
static void vtballoon_thread(void *); 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) \ #define vtballoon_modern(_sc) \
(((_sc)->vtballoon_features & VIRTIO_F_VERSION_1) != 0) (((_sc)->vtballoon_features & VIRTIO_F_VERSION_1) != 0)
@ -183,14 +184,18 @@ vtballoon_attach(device_t dev)
sc = device_get_softc(dev); sc = device_get_softc(dev);
sc->vtballoon_dev = dev; sc->vtballoon_dev = dev;
virtio_set_feature_desc(dev, vtballoon_feature_desc);
VTBALLOON_LOCK_INIT(sc, device_get_nameunit(dev)); VTBALLOON_LOCK_INIT(sc, device_get_nameunit(dev));
TAILQ_INIT(&sc->vtballoon_pages); TAILQ_INIT(&sc->vtballoon_pages);
vtballoon_add_sysctl(sc); vtballoon_setup_sysctl(sc);
virtio_set_feature_desc(dev, vtballoon_feature_desc); error = vtballoon_setup_features(sc);
vtballoon_negotiate_features(sc); if (error) {
device_printf(dev, "cannot setup features\n");
goto fail;
}
sc->vtballoon_page_frames = malloc(VTBALLOON_PAGES_PER_REQUEST * sc->vtballoon_page_frames = malloc(VTBALLOON_PAGES_PER_REQUEST *
sizeof(uint32_t), M_DEVBUF, M_NOWAIT | M_ZERO); sizeof(uint32_t), M_DEVBUF, M_NOWAIT | M_ZERO);
@ -276,7 +281,7 @@ vtballoon_config_change(device_t dev)
return (1); return (1);
} }
static void static int
vtballoon_negotiate_features(struct vtballoon_softc *sc) vtballoon_negotiate_features(struct vtballoon_softc *sc)
{ {
device_t dev; device_t dev;
@ -286,7 +291,19 @@ vtballoon_negotiate_features(struct vtballoon_softc *sc)
features = VTBALLOON_FEATURES; features = VTBALLOON_FEATURES;
sc->vtballoon_features = virtio_negotiate_features(dev, 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 static int
@ -559,7 +576,7 @@ vtballoon_thread(void *xsc)
} }
static void static void
vtballoon_add_sysctl(struct vtballoon_softc *sc) vtballoon_setup_sysctl(struct vtballoon_softc *sc)
{ {
device_t dev; device_t dev;
struct sysctl_ctx_list *ctx; struct sysctl_ctx_list *ctx;

View File

@ -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 int vtblk_dump(void *, void *, vm_offset_t, off_t, size_t);
static void vtblk_strategy(struct bio *); static void vtblk_strategy(struct bio *);
static void vtblk_negotiate_features(struct vtblk_softc *); static int vtblk_negotiate_features(struct vtblk_softc *);
static void vtblk_setup_features(struct vtblk_softc *); static int vtblk_setup_features(struct vtblk_softc *);
static int vtblk_maximum_segments(struct vtblk_softc *, static int vtblk_maximum_segments(struct vtblk_softc *,
struct virtio_blk_config *); struct virtio_blk_config *);
static int vtblk_alloc_virtqueue(struct vtblk_softc *); static int vtblk_alloc_virtqueue(struct vtblk_softc *);
@ -312,10 +312,10 @@ vtblk_attach(device_t dev)
struct virtio_blk_config blkcfg; struct virtio_blk_config blkcfg;
int error; int error;
virtio_set_feature_desc(dev, vtblk_feature_desc);
sc = device_get_softc(dev); sc = device_get_softc(dev);
sc->vtblk_dev = dev; sc->vtblk_dev = dev;
virtio_set_feature_desc(dev, vtblk_feature_desc);
VTBLK_LOCK_INIT(sc, device_get_nameunit(dev)); VTBLK_LOCK_INIT(sc, device_get_nameunit(dev));
bioq_init(&sc->vtblk_bioq); bioq_init(&sc->vtblk_bioq);
TAILQ_INIT(&sc->vtblk_dump_queue); TAILQ_INIT(&sc->vtblk_dump_queue);
@ -323,7 +323,12 @@ vtblk_attach(device_t dev)
TAILQ_INIT(&sc->vtblk_req_ready); TAILQ_INIT(&sc->vtblk_req_ready);
vtblk_setup_sysctl(sc); 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); vtblk_read_config(sc, &blkcfg);
@ -572,7 +577,7 @@ vtblk_strategy(struct bio *bp)
VTBLK_UNLOCK(sc); VTBLK_UNLOCK(sc);
} }
static void static int
vtblk_negotiate_features(struct vtblk_softc *sc) vtblk_negotiate_features(struct vtblk_softc *sc)
{ {
device_t dev; device_t dev;
@ -583,17 +588,20 @@ vtblk_negotiate_features(struct vtblk_softc *sc)
VTBLK_LEGACY_FEATURES; VTBLK_LEGACY_FEATURES;
sc->vtblk_features = virtio_negotiate_features(dev, 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) vtblk_setup_features(struct vtblk_softc *sc)
{ {
device_t dev; device_t dev;
int error;
dev = sc->vtblk_dev; 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)) if (virtio_with_feature(dev, VIRTIO_RING_F_INDIRECT_DESC))
sc->vtblk_flags |= VTBLK_FLAG_INDIRECT; sc->vtblk_flags |= VTBLK_FLAG_INDIRECT;
@ -603,6 +611,8 @@ vtblk_setup_features(struct vtblk_softc *sc)
/* Legacy. */ /* Legacy. */
if (virtio_with_feature(dev, VIRTIO_BLK_F_BARRIER)) if (virtio_with_feature(dev, VIRTIO_BLK_F_BARRIER))
sc->vtblk_flags |= VTBLK_FLAG_BARRIER; sc->vtblk_flags |= VTBLK_FLAG_BARRIER;
return (0);
} }
static int static int

View File

@ -157,8 +157,8 @@ static int vtcon_attach(device_t);
static int vtcon_detach(device_t); static int vtcon_detach(device_t);
static int vtcon_config_change(device_t); static int vtcon_config_change(device_t);
static void vtcon_setup_features(struct vtcon_softc *); static int vtcon_setup_features(struct vtcon_softc *);
static void vtcon_negotiate_features(struct vtcon_softc *); static int vtcon_negotiate_features(struct vtcon_softc *);
static int vtcon_alloc_scports(struct vtcon_softc *); static int vtcon_alloc_scports(struct vtcon_softc *);
static int vtcon_alloc_virtqueues(struct vtcon_softc *); static int vtcon_alloc_virtqueues(struct vtcon_softc *);
static void vtcon_read_config(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 = device_get_softc(dev);
sc->vtcon_dev = 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_mtx, "vtconmtx", NULL, MTX_DEF);
mtx_init(&sc->vtcon_ctrl_tx_mtx, "vtconctrlmtx", NULL, MTX_DEF); mtx_init(&sc->vtcon_ctrl_tx_mtx, "vtconctrlmtx", NULL, MTX_DEF);
virtio_set_feature_desc(dev, vtcon_feature_desc); error = vtcon_setup_features(sc);
vtcon_setup_features(sc); if (error) {
device_printf(dev, "cannot setup features\n");
goto fail;
}
vtcon_read_config(sc, &concfg); vtcon_read_config(sc, &concfg);
vtcon_determine_max_ports(sc, &concfg); vtcon_determine_max_ports(sc, &concfg);
@ -428,7 +432,7 @@ vtcon_config_change(device_t dev)
return (0); return (0);
} }
static void static int
vtcon_negotiate_features(struct vtcon_softc *sc) vtcon_negotiate_features(struct vtcon_softc *sc)
{ {
device_t dev; device_t dev;
@ -438,22 +442,27 @@ vtcon_negotiate_features(struct vtcon_softc *sc)
features = VTCON_FEATURES; features = VTCON_FEATURES;
sc->vtcon_features = virtio_negotiate_features(dev, 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) vtcon_setup_features(struct vtcon_softc *sc)
{ {
device_t dev; device_t dev;
int error;
dev = sc->vtcon_dev; 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)) if (virtio_with_feature(dev, VIRTIO_CONSOLE_F_SIZE))
sc->vtcon_flags |= VTCON_FLAG_SIZE; sc->vtcon_flags |= VTCON_FLAG_SIZE;
if (virtio_with_feature(dev, VIRTIO_CONSOLE_F_MULTIPORT)) if (virtio_with_feature(dev, VIRTIO_CONSOLE_F_MULTIPORT))
sc->vtcon_flags |= VTCON_FLAG_MULTIPORT; sc->vtcon_flags |= VTCON_FLAG_MULTIPORT;
return (0);
} }
#define VTCON_GET_CONFIG(_dev, _feature, _field, _cfg) \ #define VTCON_GET_CONFIG(_dev, _feature, _field, _cfg) \

View File

@ -102,8 +102,8 @@ static int vtnet_shutdown(device_t);
static int vtnet_attach_completed(device_t); static int vtnet_attach_completed(device_t);
static int vtnet_config_change(device_t); static int vtnet_config_change(device_t);
static void vtnet_negotiate_features(struct vtnet_softc *); static int vtnet_negotiate_features(struct vtnet_softc *);
static void vtnet_setup_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_rxq(struct vtnet_softc *, int);
static int vtnet_init_txq(struct vtnet_softc *, int); static int vtnet_init_txq(struct vtnet_softc *, int);
static int vtnet_alloc_rxtx_queues(struct vtnet_softc *); static int vtnet_alloc_rxtx_queues(struct vtnet_softc *);
@ -434,7 +434,6 @@ vtnet_attach(device_t dev)
sc = device_get_softc(dev); sc = device_get_softc(dev);
sc->vtnet_dev = dev; sc->vtnet_dev = dev;
virtio_set_feature_desc(dev, vtnet_feature_desc); virtio_set_feature_desc(dev, vtnet_feature_desc);
VTNET_CORE_LOCK_INIT(sc); VTNET_CORE_LOCK_INIT(sc);
@ -448,7 +447,12 @@ vtnet_attach(device_t dev)
} }
vtnet_setup_sysctl(sc); 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); error = vtnet_alloc_rx_filters(sc);
if (error) { if (error) {
@ -619,7 +623,7 @@ vtnet_config_change(device_t dev)
return (0); return (0);
} }
static void static int
vtnet_negotiate_features(struct vtnet_softc *sc) vtnet_negotiate_features(struct vtnet_softc *sc)
{ {
device_t dev; device_t dev;
@ -705,17 +709,20 @@ vtnet_negotiate_features(struct vtnet_softc *sc)
sc->vtnet_features = negotiated_features; sc->vtnet_features = negotiated_features;
sc->vtnet_negotiated_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) vtnet_setup_features(struct vtnet_softc *sc)
{ {
device_t dev; device_t dev;
int error;
dev = sc->vtnet_dev; 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)) if (virtio_with_feature(dev, VIRTIO_F_VERSION_1))
sc->vtnet_flags |= VTNET_FLAG_MODERN; sc->vtnet_flags |= VTNET_FLAG_MODERN;
@ -807,6 +814,8 @@ vtnet_setup_features(struct vtnet_softc *sc)
sc->vtnet_flags |= VTNET_FLAG_MQ; sc->vtnet_flags |= VTNET_FLAG_MQ;
} }
} }
return (0);
} }
static int static int

View File

@ -462,8 +462,6 @@ vtpci_modern_finalize_features(device_t dev)
/* /*
* Must re-read the status after setting it to verify the negotiated * Must re-read the status after setting it to verify the negotiated
* features were accepted by the device. * 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); vtpci_modern_set_status(sc, VIRTIO_CONFIG_S_FEATURES_OK);

View File

@ -36,7 +36,6 @@ __FBSDID("$FreeBSD$");
#include <sys/malloc.h> #include <sys/malloc.h>
#include <sys/module.h> #include <sys/module.h>
#include <sys/sglist.h> #include <sys/sglist.h>
#include <sys/callout.h>
#include <sys/random.h> #include <sys/random.h>
#include <sys/stdatomic.h> #include <sys/stdatomic.h>
@ -50,6 +49,7 @@ __FBSDID("$FreeBSD$");
#include <dev/virtio/virtqueue.h> #include <dev/virtio/virtqueue.h>
struct vtrnd_softc { struct vtrnd_softc {
device_t vtrnd_dev;
uint64_t vtrnd_features; uint64_t vtrnd_features;
struct virtqueue *vtrnd_vq; struct virtqueue *vtrnd_vq;
}; };
@ -60,8 +60,9 @@ static int vtrnd_probe(device_t);
static int vtrnd_attach(device_t); static int vtrnd_attach(device_t);
static int vtrnd_detach(device_t); static int vtrnd_detach(device_t);
static void vtrnd_negotiate_features(device_t); static int vtrnd_negotiate_features(struct vtrnd_softc *);
static int vtrnd_alloc_virtqueue(device_t); 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 int vtrnd_harvest(struct vtrnd_softc *, void *, size_t *);
static unsigned vtrnd_read(void *, unsigned); static unsigned vtrnd_read(void *, unsigned);
@ -142,11 +143,16 @@ vtrnd_attach(device_t dev)
int error; int error;
sc = device_get_softc(dev); sc = device_get_softc(dev);
sc->vtrnd_dev = dev;
virtio_set_feature_desc(dev, vtrnd_feature_desc); 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) { if (error) {
device_printf(dev, "cannot allocate virtqueue\n"); device_printf(dev, "cannot allocate virtqueue\n");
goto fail; goto fail;
@ -182,23 +188,38 @@ vtrnd_detach(device_t dev)
return (0); return (0);
} }
static void static int
vtrnd_negotiate_features(device_t dev) vtrnd_negotiate_features(struct vtrnd_softc *sc)
{ {
struct vtrnd_softc *sc; device_t dev;
uint64_t features;
sc = device_get_softc(dev); dev = sc->vtrnd_dev;
sc->vtrnd_features = virtio_negotiate_features(dev, VTRND_FEATURES); features = VTRND_FEATURES;
virtio_finalize_features(dev);
sc->vtrnd_features = virtio_negotiate_features(dev, features);
return (virtio_finalize_features(dev));
} }
static int 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; 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, VQ_ALLOC_INFO_INIT(&vq_info, 0, NULL, sc, &sc->vtrnd_vq,
"%s request", device_get_nameunit(dev)); "%s request", device_get_nameunit(dev));

View File

@ -76,7 +76,8 @@ static int vtscsi_detach(device_t);
static int vtscsi_suspend(device_t); static int vtscsi_suspend(device_t);
static int vtscsi_resume(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 *, static void vtscsi_read_config(struct vtscsi_softc *,
struct virtio_scsi_config *); struct virtio_scsi_config *);
static int vtscsi_maximum_segments(struct vtscsi_softc *, int); 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_enable_vqs_intr(struct vtscsi_softc *);
static void vtscsi_get_tunables(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 *, static void vtscsi_printf_req(struct vtscsi_request *, const char *,
const char *, ...); const char *, ...);
@ -285,22 +286,19 @@ vtscsi_attach(device_t dev)
sc = device_get_softc(dev); sc = device_get_softc(dev);
sc->vtscsi_dev = dev; sc->vtscsi_dev = dev;
virtio_set_feature_desc(dev, vtscsi_feature_desc);
VTSCSI_LOCK_INIT(sc, device_get_nameunit(dev)); VTSCSI_LOCK_INIT(sc, device_get_nameunit(dev));
TAILQ_INIT(&sc->vtscsi_req_free); TAILQ_INIT(&sc->vtscsi_req_free);
vtscsi_get_tunables(sc); vtscsi_get_tunables(sc);
vtscsi_add_sysctl(sc); vtscsi_setup_sysctl(sc);
virtio_set_feature_desc(dev, vtscsi_feature_desc); error = vtscsi_setup_features(sc);
vtscsi_negotiate_features(sc); if (error) {
device_printf(dev, "cannot setup features\n");
if (virtio_with_feature(dev, VIRTIO_RING_F_INDIRECT_DESC)) goto fail;
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;
vtscsi_read_config(sc, &scsicfg); vtscsi_read_config(sc, &scsicfg);
@ -413,7 +411,7 @@ vtscsi_resume(device_t dev)
return (0); return (0);
} }
static void static int
vtscsi_negotiate_features(struct vtscsi_softc *sc) vtscsi_negotiate_features(struct vtscsi_softc *sc)
{ {
device_t dev; device_t dev;
@ -423,7 +421,29 @@ vtscsi_negotiate_features(struct vtscsi_softc *sc)
features = VTSCSI_FEATURES; features = VTSCSI_FEATURES;
sc->vtscsi_features = virtio_negotiate_features(dev, 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) \ #define VTSCSI_GET_CONFIG(_dev, _field, _cfg) \
@ -2287,7 +2307,7 @@ vtscsi_get_tunables(struct vtscsi_softc *sc)
} }
static void static void
vtscsi_add_sysctl(struct vtscsi_softc *sc) vtscsi_setup_sysctl(struct vtscsi_softc *sc)
{ {
device_t dev; device_t dev;
struct vtscsi_statistics *stats; struct vtscsi_statistics *stats;