Re: [PATCH kvm-unit-tests v5 09/14] pci: provide pci_scan_bars()

[Alexander Gordeev <agordeev@redhat.com>]

On Tue, Nov 15, 2016 at 05:25:04PM -0500, Peter Xu wrote:
> Let's provide a more general way to scan PCI bars, rather than read the
> config registers every time.
> 
> Then let x86/vmexit.c leverage pci_scan_bars()
> 
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  lib/pci-host-generic.c |  2 +-
>  lib/pci.c              | 17 ++++++++++++++++-
>  lib/pci.h              |  5 +++++
>  x86/vmexit.c           | 17 ++++++-----------
>  4 files changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/pci-host-generic.c b/lib/pci-host-generic.c
> index 8bad8b1..ad44031 100644
> --- a/lib/pci-host-generic.c
> +++ b/lib/pci-host-generic.c
> @@ -235,7 +235,7 @@ bool pci_probe(void)
>  
>  		cmd = PCI_COMMAND_SERR | PCI_COMMAND_PARITY;
>  
> -		for (i = 0; i < 6; i++) {
> +		for (i = 0; i < PCI_BAR_NUM; i++) {
>  			u64 addr;
>  
>  			if (pci_alloc_resource(dev, i, &addr)) {
> diff --git a/lib/pci.c b/lib/pci.c
> index 8f2356d..462c370 100644
> --- a/lib/pci.c
> +++ b/lib/pci.c
> @@ -207,7 +207,7 @@ static void pci_dev_print(pcidevaddr_t dev)
>  	if ((header & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_NORMAL)
>  		return;
>  
> -	for (i = 0; i < 6; i++) {
> +	for (i = 0; i < PCI_BAR_NUM; i++) {
>  		if (pci_bar_size(&pci_dev, i)) {
>  			printf("\t");
>  			pci_bar_print(&pci_dev, i);
> @@ -227,3 +227,18 @@ void pci_print(void)
>  			pci_dev_print(dev);
>  	}
>  }
> +
> +void pci_scan_bars(struct pci_dev *dev)
> +{
> +	int i;
> +
> +	for (i = 0; i < PCI_BAR_NUM; i++) {
> +		if (!pci_bar_is_valid(dev, i))
> +			continue;
> +		dev->bar[i] = pci_bar_get_addr(dev, i);
> +		if (pci_bar_is64(dev, i)) {
> +			i++;
> +			dev->bar[i] = (phys_addr_t)0;
> +		}
> +	}
> +}
> diff --git a/lib/pci.h b/lib/pci.h
> index 355acd0..302b47c 100644
> --- a/lib/pci.h
> +++ b/lib/pci.h
> @@ -15,13 +15,16 @@ enum {
>  	PCIDEVADDR_INVALID = 0xffff,
>  };
>  
> +#define PCI_BAR_NUM                     6
>  #define PCI_DEVFN_MAX                   256
>  
>  struct pci_dev {
>  	uint16_t bdf;
> +	phys_addr_t bar[PCI_BAR_NUM];

This questions pop up time and again. If bars are 32 or 64 bit?
What if bar is not 64 aligned? etc.

I guess, it worth mentionig in comment that bar[] array here does
not match PCI header binary layout.

But also name 'bar' itself might add to the confusion. May be
Linux-like 'resource' would be better?

>  };
>  
>  extern void pci_dev_init(struct pci_dev *dev, pcidevaddr_t bdf);
> +extern void pci_scan_bars(struct pci_dev *dev);
>  
>  extern bool pci_probe(void);
>  extern void pci_print(void);
> @@ -65,6 +68,8 @@ extern int pci_testdev(void);
>   * pci-testdev supports at least three types of tests (via mmio and
>   * portio BARs): no-eventfd, wildcard-eventfd and datamatch-eventfd
>   */
> +#define PCI_TESTDEV_BAR_MEM		0
> +#define PCI_TESTDEV_BAR_IO		1
>  #define PCI_TESTDEV_NUM_BARS		2
>  #define PCI_TESTDEV_NUM_TESTS		3
>  
> diff --git a/x86/vmexit.c b/x86/vmexit.c
> index 63fa070..a22f43f 100644
> --- a/x86/vmexit.c
> +++ b/x86/vmexit.c
> @@ -389,17 +389,12 @@ int main(int ac, char **av)
>  	ret = pci_find_dev(PCI_VENDOR_ID_REDHAT, PCI_DEVICE_ID_REDHAT_TEST);
>  	if (ret != PCIDEVADDR_INVALID) {
>  		pci_dev_init(&pcidev, ret);
> -		for (i = 0; i < PCI_TESTDEV_NUM_BARS; i++) {
> -			if (!pci_bar_is_valid(&pcidev, i)) {
> -				continue;
> -			}
> -			if (pci_bar_is_memory(&pcidev, i)) {
> -				membar = pci_bar_get_addr(&pcidev, i);
> -				pci_test.memaddr = ioremap(membar, PAGE_SIZE);
> -			} else {
> -				pci_test.iobar = pci_bar_get_addr(&pcidev, i);
> -			}
> -		}
> +		pci_scan_bars(&pcidev);
> +		assert(pci_bar_is_memory(&pcidev, PCI_TESTDEV_BAR_MEM));
> +		assert(!pci_bar_is_memory(&pcidev, PCI_TESTDEV_BAR_IO));
> +		membar = pcidev.bar[PCI_TESTDEV_BAR_MEM];
> +		pci_test.memaddr = ioremap(membar, PAGE_SIZE);
> +		pci_test.iobar = pcidev.bar[PCI_TESTDEV_BAR_IO];
>  		printf("pci-testdev at 0x%x membar %lx iobar %x\n",
>  		       pcidev.bdf, membar, pci_test.iobar);
>  	}
> -- 
> 2.7.4
>