From owner-freebsd-current@FreeBSD.ORG Thu Jan 31 23:14:01 2013 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 455A2FF8 for ; Thu, 31 Jan 2013 23:14:01 +0000 (UTC) (envelope-from jim.harris@gmail.com) Received: from mail-wg0-f47.google.com (mail-wg0-f47.google.com [74.125.82.47]) by mx1.freebsd.org (Postfix) with ESMTP id C91E89FF for ; Thu, 31 Jan 2013 23:14:00 +0000 (UTC) Received: by mail-wg0-f47.google.com with SMTP id dr13so2403530wgb.26 for ; Thu, 31 Jan 2013 15:13:59 -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=Y/NZnWhQHuS7kRVOW0spt79qokXPuHIjjshvaUGTnQ4=; b=kznJD76lkDg7KkYpOSKDMgLRMU4Wb1WWrNM77164Ni3ojCaPlxQJRjiVvOXadukuhw SHRCHfFCeIWTFwyE5neM9AR4dDVGOeKdplXx26KK0r4LBrlHBUgvYUp9WLa+qiu9uj+J pjLHpy6K1jxFl1wbVYo1DUa9XJZSK2qwUXGkv+cOiZym632iVe9aBanThnZIwJU1+9SL JQ5BwNTgW2b/ozwoLSmUfmM3a0yqagZr/3m0g95AZRBt70XS7ULoAkPwY9hOV8wUTHrC DwTHHp1Whf/US+KIgcH2f8bxfLPUGBLg+jaxzGu/T+3mpV9ZtcxpiOMzhedfOwulSm64 1hcw== MIME-Version: 1.0 X-Received: by 10.180.97.4 with SMTP id dw4mr17863049wib.16.1359674039711; Thu, 31 Jan 2013 15:13:59 -0800 (PST) Received: by 10.216.118.7 with HTTP; Thu, 31 Jan 2013 15:13:59 -0800 (PST) In-Reply-To: References: Date: Thu, 31 Jan 2013 16:13:59 -0700 Message-ID: Subject: Re: PATCH: display MSI-X table and pba offsets from "pciconf -c" From: Jim Harris To: Neel Natu Content-Type: text/plain; charset=ISO-8859-1 X-Content-Filtered-By: Mailman/MimeDel 2.1.14 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: Thu, 31 Jan 2013 23:14:01 -0000 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. > 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@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org" >