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@xxxxxxxxxx>
> Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> ---
> 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
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
