[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/5] msix_init: assert programming error
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/5] msix_init: assert programming error |
Date: |
Mon, 12 Sep 2016 15:29:35 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Cao jin <address@hidden> 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?
For what it's worth, the new behavior seems consistent with msi_init(),
which is good.
> CC: Markus Armbruster <address@hidden>
> CC: Marcel Apfelbaum <address@hidden>
> CC: Michael S. Tsirkin <address@hidden>
> Signed-off-by: Cao jin <address@hidden>
> ---
> 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);
- Re: [Qemu-devel] [PATCH v2 1/5] msix_init: assert programming error,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH v2 1/5] msix_init: assert programming error, Cao jin, 2016/09/12
- Re: [Qemu-devel] [PATCH v2 1/5] msix_init: assert programming error, Markus Armbruster, 2016/09/13
- Re: [Qemu-devel] [PATCH v2 1/5] msix_init: assert programming error, Alex Williamson, 2016/09/13
- Re: [Qemu-devel] [PATCH v2 1/5] msix_init: assert programming error, Markus Armbruster, 2016/09/29
- Re: [Qemu-devel] [PATCH v2 1/5] msix_init: assert programming error, Alex Williamson, 2016/09/29
- Re: [Qemu-devel] [PATCH v2 1/5] msix_init: assert programming error, Markus Armbruster, 2016/09/30
- Re: [Qemu-devel] [PATCH v2 1/5] msix_init: assert programming error, Dr. David Alan Gilbert, 2016/09/30