[dpdk-dev] [PATCH v3 04/12] linuxapp/vfio: ignore mapping for ioport region

[Santosh Shukla]

On Thu, Jan 7, 2016 at 11:46 PM, Stephen Hemminger <
stephen at networkplumber.org> wrote:

> This looks like the right thing to do. Minor nits.
>
> > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> > index 74f91ba..4077eb6 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> > @@ -760,6 +760,26 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
> >                       return -1;
> >               }
> >
> > +             /* chk for io port region */
> > +             uint32_t ioport_bar;
>
> In general DPDK has followed the kernel practice of putting declarations
> at the start of function/basic block. It is ok by me, but just noting that
> the rest of the code doesn't do it.
>
>
My bad, Thanks!


> > +             ret = pread64(vfio_dev_fd, &ioport_bar, sizeof(ioport_bar),
> > +
>  VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX)
> > +                           + PCI_BASE_ADDRESS_0 + i*4);
> > +
> > +             if (ret != sizeof(ioport_bar)) {
> > +                     RTE_LOG(ERR, EAL,
> > +                             "Cannot read command (%x) from PCI config"
> > +                             "space!\n", PCI_BASE_ADDRESS_0 + i*4);
>
> Please dont split the line of a log message string in mid sentence.
>
>
me to don't like splitting, This was deliberate to keep checkpatch happy,
If we are ok with debug message > 80 line warning I guess  it will improve
code readability.

> +                     return -1;
> > +             }
> > +
> > +             if (ioport_bar & PCI_BASE_ADDRESS_SPACE_IO) {
> > +                     RTE_LOG(INFO, EAL, "\tIgnore mapping since Its a
> i/o"
> > +                                        "port bar (%d) addr : %x\n", i,
> same here
>
> Agreed.


> > +                                        ioport_bar);
> > +                     continue;
> > +             }
> > +
>