Date: Thu, 31 Jan 2013 16:37:48 -0800 From: Neel Natu <neelnatu@gmail.com> To: Jim Harris <jim.harris@gmail.com> Cc: current@freebsd.org Subject: Re: PATCH: display MSI-X table and pba offsets from "pciconf -c" Message-ID: <CAFgRE9EcJCdTuV30jnPUn%2BmuGWT_jCwUm2%2Bp-oH=kDpYStpKFw@mail.gmail.com> In-Reply-To: <CAJP=Hc-SoYBiu9Jmvg2aUU1DDJ6Dj=McXmHXjmDikA21jy=jhA@mail.gmail.com> References: <CAFgRE9GQjCzEM3C7OnN=Yndph2yHdEBD8TT-%2BmRoBp4QcxLK2A@mail.gmail.com> <CAJP=Hc-SoYBiu9Jmvg2aUU1DDJ6Dj=McXmHXjmDikA21jy=jhA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Jim,
On Thu, Jan 31, 2013 at 3:13 PM, Jim Harris <jim.harris@gmail.com> wrote:
>
>
> On Thu, Jan 31, 2013 at 3:52 PM, Neel Natu <neelnatu@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)
>> @@ -449,22 +449,30 @@
>> 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.
>
Ok.
>>
>> 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?
>
Yes.
>>
>> 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.
>
Yes, there was trailing whitespace there previously.
>>
>> + 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.
>
Ok, the new format will look like:
cap 11[70] = MSI-X supports 10 messages, enabled
Table in map 0x1c[0x0], PBA in map 0x1c[0x2000]
Updated patch at the end of the email.
best
Neel
>>
>> if (ctrl & PCIM_MSIXCTRL_MSIX_ENABLE)
>> printf(" enabled");
>> }
Index: usr.sbin/pciconf/cap.c
===================================================================
--- usr.sbin/pciconf/cap.c (revision 246087)
+++ usr.sbin/pciconf/cap.c (working copy)
@@ -449,24 +449,28 @@
static void
cap_msix(int fd, struct pci_conf *p, uint8_t ptr)
{
- uint32_t val;
+ uint32_t pba_offset, table_offset, val;
+ int msgnum, pba_bar, table_bar;
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;
+
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);
- 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 (ctrl & PCIM_MSIXCTRL_MSIX_ENABLE)
- printf(" enabled");
+ pba_bar = PCIR_BAR(val & PCIM_MSIX_BIR_MASK);
+ pba_offset = val & ~PCIM_MSIX_BIR_MASK;
+
+ printf("MSI-X supports %d message%s%s\n", msgnum,
+ (msgnum == 1) ? "" : "s",
+ (ctrl & PCIM_MSIXCTRL_MSIX_ENABLE) ? ", enabled" : "");
+
+ printf(" ");
+ printf("Table in map 0x%x[0x%x], PBA in map 0x%x[0x%x]",
+ table_bar, table_offset, pba_bar, pba_offset);
}
static void
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFgRE9EcJCdTuV30jnPUn%2BmuGWT_jCwUm2%2Bp-oH=kDpYStpKFw>