From owner-freebsd-current@FreeBSD.ORG Fri Feb 1 14:54:24 2013 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 1AA8E2C6; Fri, 1 Feb 2013 14:54:24 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id E74FCAF2; Fri, 1 Feb 2013 14:54:23 +0000 (UTC) Received: from pakbsde14.localnet (unknown [38.105.238.108]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 04EDBB977; Fri, 1 Feb 2013 09:54:23 -0500 (EST) From: John Baldwin To: freebsd-current@freebsd.org Subject: Re: PATCH: display MSI-X table and pba offsets from "pciconf -c" Date: Fri, 1 Feb 2013 09:30:41 -0500 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p22; KDE/4.5.5; amd64; ; ) References: In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201302010930.41743.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Fri, 01 Feb 2013 09:54:23 -0500 (EST) Cc: Jim Harris , current@freebsd.org, Neel Natu 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 14:54:24 -0000 On Thursday, January 31, 2013 7:37:48 pm Neel Natu wrote: > 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. Looks good to me. -- John Baldwin