On 09/12/2016 09:29 PM, Markus Armbruster wrote:
Cao jin <caoj.f...@cn.fujitsu.com> writes:
The input parameters is used for creating the msix capable device, so
they must obey the PCI spec, or else, it should be programming error.
True when the the parameters come from a device model attempting to
define a PCI device violating the spec. But what if the parameters come
from an actual PCI device violating the spec, via device assignment?
Before the patch, on invalid param, the vfio behaviour is:
error_report("vfio: msix_init failed");
then, device create fail.
After the patch, its behaviour is:
asserted.
Do you mean we should still report some useful info to user on invalid
params?
Cao jin
For what it's worth, the new behavior seems consistent with msi_init(),
which is good.
CC: Markus Armbruster <arm...@redhat.com>
CC: Marcel Apfelbaum <mar...@redhat.com>
CC: Michael S. Tsirkin <m...@redhat.com>
Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com>
---
hw/pci/msix.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 0ec1cb1..384a29d 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -253,9 +253,7 @@ int msix_init(struct PCIDevice *dev, unsigned short
nentries,
return -ENOTSUP;
}
- if (nentries < 1 || nentries > PCI_MSIX_FLAGS_QSIZE + 1) {
- return -EINVAL;
- }
+ assert(nentries >= 1 && nentries <= PCI_MSIX_FLAGS_QSIZE + 1);
table_size = nentries * PCI_MSIX_ENTRY_SIZE;
pba_size = QEMU_ALIGN_UP(nentries, 64) / 8;
@@ -266,7 +264,7 @@ int msix_init(struct PCIDevice *dev, unsigned short
nentries,
/* Sanity test: table & pba don't overlap, fit within BARs, min aligned
*/
if ((table_bar_nr == pba_bar_nr &&
ranges_overlap(table_offset, table_size, pba_offset, pba_size)) ||
table_offset + table_size > memory_region_size(table_bar) ||
pba_offset + pba_size > memory_region_size(pba_bar) ||
(table_offset | pba_offset) & PCI_MSIX_FLAGS_BIRMASK) {
- return -EINVAL;
+ assert(0);
}
Instead of
if (... complicated condition ...) {
assert(0);
}
let's write
assert(... negation of the complicated condition ...);
cap = pci_add_capability(dev, PCI_CAP_ID_MSIX, cap_pos, MSIX_CAP_LENGTH);
.
