On Fri, Nov 04, 2016 at 05:54:34PM +0100, Andrew Jones wrote:
> On Wed, Oct 26, 2016 at 03:47:11PM +0800, Peter Xu wrote:
> > Since pci-testdev is a very specific device for QEMU, let's use the new
> > pci_scan_bars() helper, and selectively choose the bars we want.
> >
> > Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> > ---
> > lib/pci.h | 2 ++
> > x86/vmexit.c | 23 ++++++++---------------
> > 2 files changed, 10 insertions(+), 15 deletions(-)
> >
> > diff --git a/lib/pci.h b/lib/pci.h
> > index 56b49f0..e48045c 100644
> > --- a/lib/pci.h
> > +++ b/lib/pci.h
> > @@ -66,6 +66,8 @@ 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
>
> More defines that I think we could live without, but I'll try to stop
> complaining about self-documenting code...
>
> > #define PCI_TESTDEV_NUM_BARS 2
> > #define PCI_TESTDEV_NUM_TESTS 3
> >
> > diff --git a/x86/vmexit.c b/x86/vmexit.c
> > index 2736ab8..880466e 100644
> > --- a/x86/vmexit.c
> > +++ b/x86/vmexit.c
> > @@ -373,7 +373,6 @@ int main(int ac, char **av)
> > int i;
> > unsigned long membar = 0;
> > struct pci_dev pcidev;
> > - int ret;
> >
> > smp_init();
> > setup_vm();
> > @@ -386,20 +385,14 @@ int main(int ac, char **av)
> > pm_tmr_blk = fadt->pm_tmr_blk;
> > printf("PM timer port is %x\n", pm_tmr_blk);
> >
> > - ret = pci_find_dev(&pcidev, PCI_VENDOR_ID_REDHAT,
> > - PCI_DEVICE_ID_REDHAT_TEST);
> > - if (ret == 0) {
> > - 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);
> > - }
> > - }
> > + if (!pci_find_dev(&pcidev, PCI_VENDOR_ID_REDHAT,
> > + PCI_DEVICE_ID_REDHAT_TEST)) {
> > + pci_scan_bars(&pcidev);
I think if you attempt to cache all viable information in a pci_dev
structure then pci_scan_bars() should be mandatory to call when the
pci_dev is initilized. IOW, it needs to be called in pci_dev_init().
> > + assert(pci_bar_is_memory(&pcidev, PCI_TESTDEV_BAR_MEM));
> > + assert(!pci_bar_is_memory(&pcidev, PCI_TESTDEV_BAR_IO));
> > + membar = pcidev.pci_bar[PCI_TESTDEV_BAR_MEM];
> > + pci_test.memaddr = ioremap(membar, PAGE_SIZE);
> > + pci_test.iobar = pcidev.pci_bar[PCI_TESTDEV_BAR_IO];
> > printf("pci-testdev at 0x%x membar %lx iobar %x\n",
> > pcidev.pci_bdf, membar, pci_test.iobar);
> > }
> > --
> > 2.7.4
>
> I'm not sure what we gain by hardcoding the mapping of these bars, besides
> slightly simpler code, but lib/pci-testdev.c does it too, and it should be
> quite safe, so OK.
>
> Reviewed-by: Andrew Jones <drjones@xxxxxxxxxx>
--
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
