Re: PATCH: display MSI-X table and pba offsets from "pciconf -c"

On Thu, Jan 31, 2013 at 3:52 PM, Neel Natu <neelnatu_at_gmail.com> wrote:

> Hi,
>
> The following patch teaches pciconf(8) to display the table and pba
> offsets when it displays the MSI-X capability.
>
> The new output format will look like:
>
> cap 11[70] = MSI-X supports 10 messages in map 0x1c[0x0][0x2000] enabled
> OR
> cap 11[70] = MSI-X supports 10 messages in maps 0x10[0x0] and
> 0x14[0x1000] enabled
>
> Any objections to committing the patch?
>

Functionally I think this is a good addition.  More information from
pciconf the better.

Other comments below.


> best
> Neel
>
> Index: usr.sbin/pciconf/cap.c
> ===================================================================
> --- cap.c       (revision 246087)
> +++ cap.c       (working copy)
> _at__at_ -449,22 +449,30 _at__at_
>  static void
>  cap_msix(int fd, struct pci_conf *p, uint8_t ptr)
>  {
> -       uint32_t val;
> +       uint32_t val, table_offset, pba_offset;
>

Variables should be in alphabetical order.


>         uint16_t ctrl;
>         int msgnum, table_bar, pba_bar;
>
>         ctrl = read_config(fd, &p->pc_sel, ptr + PCIR_MSIX_CTRL, 2);
>         msgnum = (ctrl & PCIM_MSIXCTRL_TABLE_SIZE) + 1;
> +
>

Newline added intentionally?


>         val = read_config(fd, &p->pc_sel, ptr + PCIR_MSIX_TABLE, 4);
>         table_bar = PCIR_BAR(val & PCIM_MSIX_BIR_MASK);
> +       table_offset = val & ~PCIM_MSIX_BIR_MASK;
> +
>         val = read_config(fd, &p->pc_sel, ptr + PCIR_MSIX_PBA, 4);
> -       pba_bar = PCIR_BAR(val & PCIM_MSIX_BIR_MASK);
> +       pba_bar = PCIR_BAR(val & PCIM_MSIX_BIR_MASK);
>

Looks like these two lines only have whitespace difference.


> +       pba_offset = val & ~PCIM_MSIX_BIR_MASK;
> +
>         printf("MSI-X supports %d message%s ", msgnum,
>             (msgnum == 1) ? "" : "s");
> -       if (table_bar == pba_bar)
> -               printf("in map 0x%x", table_bar);
> -       else
> -               printf("in maps 0x%x and 0x%x", table_bar, pba_bar);
> +       if (table_bar == pba_bar) {
> +               printf("in map 0x%x[0x%x][0x%x]",
> +                       table_bar, table_offset, pba_offset);
> +       } else {
> +               printf("in maps 0x%x[0x%x] and 0x%x[0x%x]",
> +                       table_bar, table_offset, pba_bar, pba_offset);
> +       }
>

Seems like at least for the case where the table and pba are behind
different BARs, this will exceed 80 characters.  So maybe the maps always
go on a new line?  Similar to what's done at the end of cap_express for
printing the link speed.


>         if (ctrl & PCIM_MSIXCTRL_MSIX_ENABLE)
>                 printf(" enabled");
>  }
> _______________________________________________
> freebsd-current_at_freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe_at_freebsd.org"
>