On Thu, Apr 14, 2016 at 09:43:00AM +0200, Thomas Huth wrote:
> On 11.04.2016 13:04, Alexander Gordeev wrote:
> > Cc: Thomas Huth <thuth@xxxxxxxxxx>
> > Cc: Andrew Jones <drjones@xxxxxxxxxx>
> > Signed-off-by: Alexander Gordeev <agordeev@xxxxxxxxxx>
> > ---
> > lib/asm-generic/pci.h | 6 +++++
> > lib/pci.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > lib/pci.h | 3 +++
> > 3 files changed, 76 insertions(+)
> >
> > diff --git a/lib/asm-generic/pci.h b/lib/asm-generic/pci.h
> > index 15f23079f27e..3f2c6913f0d4 100644
> > --- a/lib/asm-generic/pci.h
> > +++ b/lib/asm-generic/pci.h
> > @@ -22,4 +22,10 @@ phys_addr_t pci_xlate_addr(pcidevaddr_t __unused dev, uint64_t addr)
> > }
> > #endif
> >
> > +#ifndef pci_print_arch
> > +static inline void pci_print_arch(void)
> > +{
> > +}
> > +#endif
> > +
> > #endif
> > diff --git a/lib/pci.c b/lib/pci.c
> > index 46aee60e0f90..a3c680670fe0 100644
> > --- a/lib/pci.c
> > +++ b/lib/pci.c
> > @@ -116,3 +116,70 @@ bool pci_bar_is64(pcidevaddr_t dev, int bar_num)
> > else
> > return false;
> > }
> > +
> > +void pci_type_desc(int type, char *desc, int len)
> > +{
> > + if ((type & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO) {
> > + strcpy(desc, "PIO"); /* strncpy() would be better */
>
> Maybe that's a good point in time now to introduce strncpy to
> kvm-unit-tests? (or maybe even a better variant like strlcpy?)
Andrew?
> > + } else {
> > + static char *str[] = { "32", "1M", "64" };
>
> Since you're depending on external values as array index below, I'd play
> safe here and add a fourth entry like "??" or "RES" or so.
As I got from previous Andrew's explanations we would rather crash
in case of incorrect access.
> > + int idx = (type & PCI_BASE_ADDRESS_MEM_TYPE_MASK) >> 1;
> > + int pfetch = type & PCI_BASE_ADDRESS_MEM_PREFETCH;
> > + snprintf(desc, len, "MEM%s%s", str[idx], pfetch ? "/p" : "");
> > + }
> > +}
> > +
> > +static void pci_dev_print(pcidevaddr_t dev)
> > +{
> > + uint16_t vendor_id = pci_config_readw(dev, PCI_VENDOR_ID);
> > + uint16_t device_id = pci_config_readw(dev, PCI_DEVICE_ID);
> > + uint8_t header = pci_config_readb(dev, PCI_HEADER_TYPE);
>
> I think you should mask away the uppermost bit of the header type byte -
> otherwise your code does not work with multi-function devices.
See below.
> > + uint8_t progif = pci_config_readb(dev, PCI_CLASS_PROG);
> > + uint8_t subclass = pci_config_readb(dev, PCI_CLASS_DEVICE);
> > + uint8_t class = pci_config_readb(dev, PCI_CLASS_DEVICE + 1);
> > + int bar;
> > +
> > + printf("dev %2d fn %d vendor_id %04x device_id %04x type %02x "
> > + "progif %02x class %02x subclass %02x\n",
> > + dev / 8, dev % 8, vendor_id, device_id, header,
> > + progif, class, subclass);
> > +
> > + if (header != PCI_HEADER_TYPE_NORMAL)
> > + return;
if ((header & 0x7f) != PCI_HEADER_TYPE_NORMAL) /* mask multi-function bit */
return;
I would rather masked it here.
?
> > +
> > + for (bar = 0; bar < 6; bar++) {
> > + phys_addr_t start, end;
> > + char desc[8];
> > +
> > + if (!pci_bar_is_valid(dev, bar))
> > + break;
> > +
> > + start = pci_bar_addr(dev, bar);
> > + end = start + pci_bar_size(dev, bar) - 1;
> > +
> > + pci_type_desc(bar, desc, sizeof(desc));
> > +
> > + if (pci_bar_is64(dev, bar)) {
> > + printf("\tBAR#%d,%d [%-7s %" PRIx64 "-%" PRIx64 "]\n",
> > + bar, bar + 1, desc, start, end);
> > + bar++;
> > + } else {
> > + printf("\tBAR#%d [%-7s %02x-%02x]\n",
> > + bar, desc, (uint32_t)start, (uint32_t)end);
> > + }
> > + }
> > +}
>
> Thomas
>
--
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
