From owner-freebsd-current@FreeBSD.ORG Fri Feb 1 00:37:55 2013 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id EF6A7F91 for ; Fri, 1 Feb 2013 00:37:55 +0000 (UTC) (envelope-from neelnatu@gmail.com) Received: from mail-vb0-f51.google.com (mail-vb0-f51.google.com [209.85.212.51]) by mx1.freebsd.org (Postfix) with ESMTP id B63CAE12 for ; Fri, 1 Feb 2013 00:37:55 +0000 (UTC) Received: by mail-vb0-f51.google.com with SMTP id fq11so2106397vbb.38 for ; Thu, 31 Jan 2013 16:37:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type; bh=hY366jadNzK+iBGNSG/LCfYlrHONne3Pd+vjteD3lcM=; b=EEJZdiTx8Ap/WfC2EVIoNnh63C5UlBW8X4jgYsFUbGDX6NqiTM+AwH+toVEXOjSOIf JiMzqB3T29fNG5cci2OlaByL4frjHBMF84BoF8fqyKOiFUlppexCZqZvcCyCzXI9CRQ4 GdCSRCjlpkwySpTWzyFsKanoaG3Ls03nrvfdgIEgbikQPMQVIcUdM2vMaMclLnIAFy7u /Ee39EUuwzuX+xt8NxqcQvdbOsLmMdqsBLbmNm6gIk7Vi+VYNvFi3kz4dmTWNDH+We/5 D3RYVxFHPNifvbH9lVKKjg6d1IdfRMdKbIsUWZqoz63P0YJMOdPmXXvSvIy78oqt08M8 EYQA== MIME-Version: 1.0 X-Received: by 10.58.8.82 with SMTP id p18mr5612286vea.54.1359679068944; Thu, 31 Jan 2013 16:37:48 -0800 (PST) Received: by 10.220.21.141 with HTTP; Thu, 31 Jan 2013 16:37:48 -0800 (PST) In-Reply-To: References: Date: Thu, 31 Jan 2013 16:37:48 -0800 Message-ID: Subject: Re: PATCH: display MSI-X table and pba offsets from "pciconf -c" From: Neel Natu To: Jim Harris Content-Type: text/plain; charset=ISO-8859-1 Cc: current@freebsd.org X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Feb 2013 00:37:56 -0000 Hi Jim, On Thu, Jan 31, 2013 at 3:13 PM, Jim Harris wrote: > > > On Thu, Jan 31, 2013 at 3:52 PM, Neel Natu 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