xemu-project/xemu
1
0
Fork 0
mirror of https://github.com/xemu-project/xemu.git synced 2025-02-17 02:29:12 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

msix: Assert that specified vector is in range

Browse Source 
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 <akihiko.odaki@daynix.com>
Message-Id: <20220829083524.143640-1-akihiko.odaki@daynix.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Yuval Shaia <yuval.shaia.ml@gmail.com>
Signed-off-by: Akihiko Odaki &lt;<a href="mailto:akihiko.odaki@daynix.com" target="_blank">akihiko.odaki@daynix.com</a>&gt;<br>
This commit is contained in:
Akihiko Odaki 2022-08-29 17:35:24 +09:00 committed by Michael S. Tsirkin
parent 3b3112501d
commit 15377f6e79
9 changed files with 74 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
hw/net/e1000e.c
View File

@ -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);
}
}

23
hw/net/rocker/rocker.c
View File

@ -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)

27
hw/net/vmxnet3.c
View File

@ -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)) {

5
hw/nvme/ctrl.c
View File

@ -4744,11 +4744,8 @@ static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl *n, uint64_t dma_addr,
uint16_t cqid, uint16_t vector, uint16_t size,
uint16_t irq_enabled)
{
int ret;
if (msix_enabled(&n->parent_obj)) {
ret = msix_vector_use(&n->parent_obj, vector);
assert(ret == 0);
msix_vector_use(&n->parent_obj, vector);
}
cq->ctrl = n;
cq->cqid = cqid;

24
hw/pci/msix.c
View File

@ -136,17 +136,12 @@ static void msix_handle_mask_update(PCIDevice *dev, int vector, bool was_masked)
}
}
void msix_set_mask(PCIDevice *dev, int vector, bool mask, Error **errp)
void msix_set_mask(PCIDevice *dev, int vector, bool mask)
{
ERRP_GUARD();
unsigned offset;
bool was_masked;
if (vector > dev->msix_entries_nr) {
error_setg(errp, "msix: vector %d not allocated. max vector is %d",
vector, dev->msix_entries_nr);
return;
}
assert(vector < dev->msix_entries_nr);
offset = vector * PCI_MSIX_ENTRY_SIZE + PCI_MSIX_ENTRY_VECTOR_CTRL;
@ -522,7 +517,9 @@ void msix_notify(PCIDevice *dev, unsigned vector)
{
MSIMessage msg;
if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector]) {
assert(vector < dev->msix_entries_nr);
if (!dev->msix_entry_used[vector]) {
return;
}
@ -558,20 +555,17 @@ void msix_reset(PCIDevice *dev)
* don't want to follow the spec suggestion can declare all vectors as used. */
/* Mark vector as used. */
int msix_vector_use(PCIDevice *dev, unsigned vector)
void msix_vector_use(PCIDevice *dev, unsigned vector)
{
if (vector >= dev->msix_entries_nr) {
return -EINVAL;
}
assert(vector < dev->msix_entries_nr);
dev->msix_entry_used[vector]++;
return 0;
}
/* Mark vector as unused. */
void msix_vector_unuse(PCIDevice *dev, unsigned vector)
{
if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector]) {
assert(vector < dev->msix_entries_nr);
if (!dev->msix_entry_used[vector]) {
return;
}
if (--dev->msix_entry_used[vector]) {

7
hw/rdma/vmw/pvrdma_main.c
View File

@ -307,12 +307,7 @@ static int init_msix(PCIDevice *pdev)
}
for (i = 0; i < RDMA_MAX_INTRS; i++) {
rc = msix_vector_use(PCI_DEVICE(dev), i);
if (rc < 0) {
rdma_error_report("Fail mark MSI-X vector %d", i);
uninit_msix(pdev, i);
return rc;
}
msix_vector_use(PCI_DEVICE(dev), i);
}
return 0;

9
hw/remote/vfio-user-obj.c
View File

@ -602,17 +602,10 @@ static void vfu_msix_irq_state(vfu_ctx_t *vfu_ctx, uint32_t start,
uint32_t count, bool mask)
{
VfuObject *o = vfu_get_private(vfu_ctx);
Error *err = NULL;
uint32_t vector;
for (vector = start; vector < count; vector++) {
msix_set_mask(o->pci_dev, vector, mask, &err);
if (err) {
VFU_OBJECT_ERROR(o, "vfu: %s: %s", o->device,
error_get_pretty(err));
error_free(err);
err = NULL;
}
msix_set_mask(o->pci_dev, vector, mask);
}
}

67
hw/virtio/virtio-pci.c
View File

@ -71,9 +71,11 @@ static void virtio_pci_notify(DeviceState *d, uint16_t vector)
{
VirtIOPCIProxy *proxy = to_virtio_pci_proxy_fast(d);
if (msix_enabled(&proxy->pci_dev))
msix_notify(&proxy->pci_dev, vector);
else {
if (msix_enabled(&proxy->pci_dev)) {
if (vector != VIRTIO_NO_VECTOR) {
msix_notify(&proxy->pci_dev, vector);
}
} else {
VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
pci_set_irq(&proxy->pci_dev, qatomic_read(&vdev->isr) & 1);
}
@ -175,6 +177,7 @@ static int virtio_pci_load_config(DeviceState *d, QEMUFile *f)
{
VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
uint16_t vector;
int ret;
ret = pci_device_load(&proxy->pci_dev, f);
@ -184,12 +187,17 @@ static int virtio_pci_load_config(DeviceState *d, QEMUFile *f)
msix_unuse_all_vectors(&proxy->pci_dev);
msix_load(&proxy->pci_dev, f);
if (msix_present(&proxy->pci_dev)) {
qemu_get_be16s(f, &vdev->config_vector);
qemu_get_be16s(f, &vector);
if (vector != VIRTIO_NO_VECTOR && vector >= proxy->nvectors) {
return -EINVAL;
}
} else {
vdev->config_vector = VIRTIO_NO_VECTOR;
vector = VIRTIO_NO_VECTOR;
}
if (vdev->config_vector != VIRTIO_NO_VECTOR) {
return msix_vector_use(&proxy->pci_dev, vdev->config_vector);
vdev->config_vector = vector;
if (vector != VIRTIO_NO_VECTOR) {
msix_vector_use(&proxy->pci_dev, vector);
}
return 0;
}
@ -202,12 +210,15 @@ static int virtio_pci_load_queue(DeviceState *d, int n, QEMUFile *f)
uint16_t vector;
if (msix_present(&proxy->pci_dev)) {
qemu_get_be16s(f, &vector);
if (vector != VIRTIO_NO_VECTOR && vector >= proxy->nvectors) {
return -EINVAL;
}
} else {
vector = VIRTIO_NO_VECTOR;
}
virtio_queue_set_vector(vdev, n, vector);
if (vector != VIRTIO_NO_VECTOR) {
return msix_vector_use(&proxy->pci_dev, vector);
msix_vector_use(&proxy->pci_dev, vector);
}
return 0;
@ -299,6 +310,7 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
{
VirtIOPCIProxy *proxy = opaque;
VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
uint16_t vector;
hwaddr pa;
switch (addr) {
@ -352,18 +364,28 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
}
break;
case VIRTIO_MSI_CONFIG_VECTOR:
msix_vector_unuse(&proxy->pci_dev, vdev->config_vector);
if (vdev->config_vector != VIRTIO_NO_VECTOR) {
msix_vector_unuse(&proxy->pci_dev, vdev->config_vector);
}
/* Make it possible for guest to discover an error took place. */
if (msix_vector_use(&proxy->pci_dev, val) < 0)
if (val < proxy->nvectors) {
msix_vector_use(&proxy->pci_dev, val);
} else {
val = VIRTIO_NO_VECTOR;
}
vdev->config_vector = val;
break;
case VIRTIO_MSI_QUEUE_VECTOR:
msix_vector_unuse(&proxy->pci_dev,
virtio_queue_vector(vdev, vdev->queue_sel));
vector = virtio_queue_vector(vdev, vdev->queue_sel);
if (vector != VIRTIO_NO_VECTOR) {
msix_vector_unuse(&proxy->pci_dev, vector);
}
/* Make it possible for guest to discover an error took place. */
if (msix_vector_use(&proxy->pci_dev, val) < 0)
if (val < proxy->nvectors) {
msix_vector_use(&proxy->pci_dev, val);
} else {
val = VIRTIO_NO_VECTOR;
}
virtio_queue_set_vector(vdev, vdev->queue_sel, val);
break;
default:
@ -1266,6 +1288,7 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
{
VirtIOPCIProxy *proxy = opaque;
VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
uint16_t vector;
if (vdev == NULL) {
return;
@ -1287,9 +1310,13 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
}
break;
case VIRTIO_PCI_COMMON_MSIX:
msix_vector_unuse(&proxy->pci_dev, vdev->config_vector);
if (vdev->config_vector != VIRTIO_NO_VECTOR) {
msix_vector_unuse(&proxy->pci_dev, vdev->config_vector);
}
/* Make it possible for guest to discover an error took place. */
if (msix_vector_use(&proxy->pci_dev, val) < 0) {
if (val < proxy->nvectors) {
msix_vector_use(&proxy->pci_dev, val);
} else {
val = VIRTIO_NO_VECTOR;
}
vdev->config_vector = val;
@ -1321,10 +1348,14 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
proxy->vqs[vdev->queue_sel].num);
break;
case VIRTIO_PCI_COMMON_Q_MSIX:
msix_vector_unuse(&proxy->pci_dev,
virtio_queue_vector(vdev, vdev->queue_sel));
vector = virtio_queue_vector(vdev, vdev->queue_sel);
if (vector != VIRTIO_NO_VECTOR) {
msix_vector_unuse(&proxy->pci_dev, vector);
}
/* Make it possible for guest to discover an error took place. */
if (msix_vector_use(&proxy->pci_dev, val) < 0) {
if (val < proxy->nvectors) {
msix_vector_use(&proxy->pci_dev, val);
} else {
val = VIRTIO_NO_VECTOR;
}
virtio_queue_set_vector(vdev, vdev->queue_sel, val);

4
include/hw/pci/msix.h
View File

@ -33,10 +33,10 @@ bool msix_is_masked(PCIDevice *dev, unsigned vector);
void msix_set_pending(PCIDevice *dev, unsigned vector);
void msix_clr_pending(PCIDevice *dev, int vector);
int msix_vector_use(PCIDevice *dev, unsigned vector);
void msix_vector_use(PCIDevice *dev, unsigned vector);
void msix_vector_unuse(PCIDevice *dev, unsigned vector);
void msix_unuse_all_vectors(PCIDevice *dev);
void msix_set_mask(PCIDevice *dev, int vector, bool mask, Error **errp);
void msix_set_mask(PCIDevice *dev, int vector, bool mask);
void msix_notify(PCIDevice *dev, unsigned vector);