From 15377f6e79cc6aa08dbafe82607e0bda13ca44b5 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Mon, 29 Aug 2022 17:35:24 +0900 Subject: msix: Assert that specified vector is in range There were several different ways to deal with the situation where the vector specified for a msix function is out of bound: - early return a function and keep progresssing - propagate the error to the caller - mark msix unusable - assert it is in bound - just ignore An out-of-bound vector should not be specified if the device implementation is correct so let msix functions always assert that the specified vector is in range. An exceptional case is virtio-pci, which allows the guest to configure vectors. For virtio-pci, it is more appropriate to introduce its own checks because it is sometimes too late to check the vector range in msix functions. Signed-off-by: Akihiko Odaki Message-Id: <20220829083524.143640-1-akihiko.odaki@daynix.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Yuval Shaia Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
--- hw/net/e1000e.c | 15 +++------------ hw/net/rocker/rocker.c | 23 +++-------------------- hw/net/vmxnet3.c | 27 +++++---------------------- 3 files changed, 11 insertions(+), 54 deletions(-) (limited to 'hw/net') diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c index ac96f76..7523e9f 100644 --- a/hw/net/e1000e.c +++ b/hw/net/e1000e.c @@ -276,25 +276,18 @@ e1000e_unuse_msix_vectors(E1000EState *s, int num_vectors) } } -static bool +static void e1000e_use_msix_vectors(E1000EState *s, int num_vectors) { int i; for (i = 0; i < num_vectors; i++) { - int res = msix_vector_use(PCI_DEVICE(s), i); - if (res < 0) { - trace_e1000e_msix_use_vector_fail(i, res); - e1000e_unuse_msix_vectors(s, i); - return false; - } + msix_vector_use(PCI_DEVICE(s), i); } - return true; } static void e1000e_init_msix(E1000EState *s) { - PCIDevice *d = PCI_DEVICE(s); int res = msix_init(PCI_DEVICE(s), E1000E_MSIX_VEC_NUM, &s->msix, E1000E_MSIX_IDX, E1000E_MSIX_TABLE, @@ -305,9 +298,7 @@ e1000e_init_msix(E1000EState *s) if (res < 0) { trace_e1000e_msix_init_fail(res); } else { - if (!e1000e_use_msix_vectors(s, E1000E_MSIX_VEC_NUM)) { - msix_uninit(d, &s->msix, &s->msix); - } + e1000e_use_msix_vectors(s, E1000E_MSIX_VEC_NUM); } } diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c index d8f3f16..281d43e 100644 --- a/hw/net/rocker/rocker.c +++ b/hw/net/rocker/rocker.c @@ -1212,24 +1212,14 @@ static void rocker_msix_vectors_unuse(Rocker *r, } } -static int rocker_msix_vectors_use(Rocker *r, - unsigned int num_vectors) +static void rocker_msix_vectors_use(Rocker *r, unsigned int num_vectors) { PCIDevice *dev = PCI_DEVICE(r); - int err; int i; for (i = 0; i < num_vectors; i++) { - err = msix_vector_use(dev, i); - if (err) { - goto rollback; - } + msix_vector_use(dev, i); } - return 0; - -rollback: - rocker_msix_vectors_unuse(r, i); - return err; } static int rocker_msix_init(Rocker *r, Error **errp) @@ -1247,16 +1237,9 @@ static int rocker_msix_init(Rocker *r, Error **errp) return err; } - err = rocker_msix_vectors_use(r, ROCKER_MSIX_VEC_COUNT(r->fp_ports)); - if (err) { - goto err_msix_vectors_use; - } + rocker_msix_vectors_use(r, ROCKER_MSIX_VEC_COUNT(r->fp_ports)); return 0; - -err_msix_vectors_use: - msix_uninit(dev, &r->msix_bar, &r->msix_bar); - return err; } static void rocker_msix_uninit(Rocker *r) diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index 0b7acf7..d2ab527 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -2110,20 +2110,14 @@ vmxnet3_unuse_msix_vectors(VMXNET3State *s, int num_vectors) } } -static bool +static void vmxnet3_use_msix_vectors(VMXNET3State *s, int num_vectors) { PCIDevice *d = PCI_DEVICE(s); int i; for (i = 0; i < num_vectors; i++) { - int res = msix_vector_use(d, i); - if (0 > res) { - VMW_WRPRN("Failed to use MSI-X vector %d, error %d", i, res); - vmxnet3_unuse_msix_vectors(s, i); - return false; - } + msix_vector_use(d, i); } - return true; } static bool @@ -2141,13 +2135,8 @@ vmxnet3_init_msix(VMXNET3State *s) VMW_WRPRN("Failed to initialize MSI-X, error %d", res); s->msix_used = false; } else { - if (!vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS)) { - VMW_WRPRN("Failed to use MSI-X vectors, error %d", res); - msix_uninit(d, &s->msix_bar, &s->msix_bar); - s->msix_used = false; - } else { - s->msix_used = true; - } + vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS); + s->msix_used = true; } return s->msix_used; } @@ -2412,19 +2401,13 @@ static const VMStateDescription vmstate_vmxnet3_rxq_descr = { static int vmxnet3_post_load(void *opaque, int version_id) { VMXNET3State *s = opaque; - PCIDevice *d = PCI_DEVICE(s); net_tx_pkt_init(&s->tx_pkt, PCI_DEVICE(s), s->max_tx_frags, s->peer_has_vhdr); net_rx_pkt_init(&s->rx_pkt, s->peer_has_vhdr); if (s->msix_used) { - if (!vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS)) { - VMW_WRPRN("Failed to re-use MSI-X vectors"); - msix_uninit(d, &s->msix_bar, &s->msix_bar); - s->msix_used = false; - return -1; - } + vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS); } if (!vmxnet3_validate_queues(s)) { -- cgit v1.1