If you take my suggestion for patch 1, then this patch can be
dropped.
On Mon, Jan 23, 2017 at 08:44:08AM +0100, Alexander Gordeev wrote:
> Function pci_bar_is_valid() does two logical steps:
> 1. Checks BAR validity in the context of all device BARs;
> 2. Checks if the BAR is implemented (AKA exists);
>
> In some cases checking BAR existence (step 2) is redundant.
> So reduce function pci_bar_is_valid() to only checking BAR
> validity(step 1).
>
> In most cases we do need to check BAR existence, but we must
> ensure the BAR validity first (steps 1 and 2). For such cases
> pci_bar_exists() function is introduced. In essence it does
> what pci_bar_is_valid() used to do before this change.
>
> Cc: Thomas Huth <thuth@xxxxxxxxxx>
> Cc: Andrew Jones <drjones@xxxxxxxxxx>
> Signed-off-by: Alexander Gordeev <agordeev@xxxxxxxxxx>
> ---
> lib/pci-host-generic.c | 4 ++--
> lib/pci-testdev.c | 2 +-
> lib/pci.c | 15 +++++++++------
> lib/pci.h | 5 +++++
> x86/vmexit.c | 2 +-
> 5 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/lib/pci-host-generic.c b/lib/pci-host-generic.c
> index 4263365e8288..a881ec5a9d4f 100644
> --- a/lib/pci-host-generic.c
> +++ b/lib/pci-host-generic.c
> @@ -175,8 +175,7 @@ static bool pci_alloc_resource(pcidevaddr_t dev, int bar_num, u64 *addr)
>
> *addr = ~0;
>
> - size = pci_bar_size(dev, bar_num);
> - if (!size)
> + if (!pci_bar_exists(dev, bar_num))
> return false;
>
> bar = pci_bar_get(dev, bar_num);
> @@ -199,6 +198,7 @@ static bool pci_alloc_resource(pcidevaddr_t dev, int bar_num, u64 *addr)
> return false;
> }
>
> + size = pci_bar_size(dev, bar_num);
> pci_addr = ALIGN(as->pci_start + as->allocated, size);
> size += pci_addr - (as->pci_start + as->allocated);
> assert(as->allocated + size <= as->size);
> diff --git a/lib/pci-testdev.c b/lib/pci-testdev.c
> index ad482d3291c7..712ada5c5860 100644
> --- a/lib/pci-testdev.c
> +++ b/lib/pci-testdev.c
> @@ -176,7 +176,7 @@ int pci_testdev(void)
> return -1;
> }
>
> - ret = pci_bar_is_valid(dev, 0) && pci_bar_is_valid(dev, 1);
> + ret = pci_bar_exists(dev, 0) && pci_bar_exists(dev, 1);
> assert(ret);
>
> addr = pci_bar_get_addr(dev, 0);
> diff --git a/lib/pci.c b/lib/pci.c
> index 00fba6e07860..7a3e505ab314 100644
> --- a/lib/pci.c
> +++ b/lib/pci.c
> @@ -124,17 +124,20 @@ bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num)
>
> for (i = 0; i < 6; i++) {
> if (is64) {
> - assert(i == bar_num); /* high part of 64-bit BAR */
> - assert(pci_bar_is64(dev, i));
> + if (i == bar_num) /* high part of 64-bit BAR */
> + return false;
> + if (pci_bar_is64(dev, i))
> + return false;
>
> is64 = false;
> } else {
> is64 = pci_bar_is64(dev, i);
> }
> }
> - assert(!is64); /* incomplete 64-bit BAR */
> + if (is64) /* incomplete 64-bit BAR */
> + return false;
>
> - return pci_bar_size(dev, bar_num);
> + return true;
> }
>
> bool pci_bar_is64(pcidevaddr_t dev, int bar_num)
> @@ -153,12 +156,12 @@ void pci_bar_print(pcidevaddr_t dev, int bar_num)
> phys_addr_t size, start, end;
> uint32_t bar;
>
> - size = pci_bar_size(dev, bar_num);
> - if (!size)
> + if (!pci_bar_exists(dev, bar_num))
> return;
>
> bar = pci_bar_get(dev, bar_num);
> start = pci_bar_get_addr(dev, bar_num);
> + size = pci_bar_size(dev, bar_num);
> end = start + size - 1;
>
> if (pci_bar_is64(dev, bar_num)) {
> diff --git a/lib/pci.h b/lib/pci.h
> index 30f538110610..0b07036f27ac 100644
> --- a/lib/pci.h
> +++ b/lib/pci.h
> @@ -43,6 +43,11 @@ extern bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num);
> extern void pci_bar_print(pcidevaddr_t dev, int bar_num);
> extern void pci_dev_print_id(pcidevaddr_t dev);
>
> +static inline bool pci_bar_exists(pcidevaddr_t dev, int bar_num)
> +{
> + return pci_bar_size(dev, bar_num);
> +}
> +
> int pci_testdev(void);
>
> /*
> diff --git a/x86/vmexit.c b/x86/vmexit.c
> index 2d99d5fdf1c2..32132667b617 100644
> --- a/x86/vmexit.c
> +++ b/x86/vmexit.c
> @@ -388,7 +388,7 @@ int main(int ac, char **av)
> pcidev = pci_find_dev(PCI_VENDOR_ID_REDHAT, PCI_DEVICE_ID_REDHAT_TEST);
> if (pcidev != PCIDEVADDR_INVALID) {
> for (i = 0; i < PCI_TESTDEV_NUM_BARS; i++) {
> - if (!pci_bar_is_valid(pcidev, i)) {
> + if (!pci_bar_exists(pcidev, i)) {
> continue;
> }
> if (pci_bar_is_memory(pcidev, i)) {
> --
> 1.8.3.1
>
