From owner-freebsd-current@FreeBSD.ORG Wed Oct 3 20:22:03 2007 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 97C4216A476 for ; Wed, 3 Oct 2007 20:22:03 +0000 (UTC) (envelope-from se@FreeBSD.org) Received: from spacemail2-out.mgmt.space.net (spacemail2-out.mgmt.Space.Net [194.97.149.148]) by mx1.freebsd.org (Postfix) with ESMTP id 9225813C4AA for ; Wed, 3 Oct 2007 20:22:02 +0000 (UTC) (envelope-from se@FreeBSD.org) X-SpaceNet-SBRS: None X-IronPort-AV: E=Sophos;i="4.21,226,1188770400"; d="scan'208";a="60966951" Received: from mail.atsec.com ([195.30.252.105]) by spacemail2-out.mgmt.space.net with ESMTP; 03 Oct 2007 22:22:01 +0200 Received: from [192.168.0.12] (p5087616F.dip.t-dialin.net [80.135.97.111]) (Authenticated sender: se@atsec.com) by mail.atsec.com (Postfix) with ESMTP id 8BB88720920; Wed, 3 Oct 2007 22:22:00 +0200 (CEST) Message-ID: <4703F9E5.7050108@FreeBSD.org> Date: Wed, 03 Oct 2007 22:21:57 +0200 From: Stefan Esser User-Agent: Thunderbird 2.0.0.6 (Windows/20070728) MIME-Version: 1.0 To: Marius Strobl References: <20070930114914.GB38896@alchemy.franken.de> <4700ECC8.4090702@FreeBSD.org> <20071001132548.GE55741@alchemy.franken.de> <200710011420.31077.jhb@freebsd.org> <20071003105357.GA27749@Gatekeeper.FreeBSD.org> <20071003131853.GE98412@alchemy.franken.de> In-Reply-To: <20071003131853.GE98412@alchemy.franken.de> X-Enigmail-Version: 0.95.3 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Michael Butler , Marcel Moolenaar , =?ISO-8859-1?Q?Stefan_E=DFer?= , current@FreeBSD.org, John Baldwin Subject: Re: Patch for review (was: Re: Proposal to revise the new parsing of PCI selectors X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 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: Wed, 03 Oct 2007 20:22:03 -0000 Marius Strobl schrieb: > On Wed, Oct 03, 2007 at 12:53:57PM +0200, Stefan Eer wrote: >> On 2007-10-01 14:20 -0400, John Baldwin wrote: >>> On Monday 01 October 2007 09:25:48 am Marius Strobl wrote: >>>> On Mon, Oct 01, 2007 at 02:49:12PM +0200, Stefan Esser wrote: >>>>> Well, since it was me who chose to parse it that way, when pciconf >>>>> saw the light of day, I can say that the logical extension appears >>>>> to be the support of 3 formats for the PCI device selector: >>>>> >>>>> pci1:2:3:4 (full, domain/bus/slot/function, required if domain!=0) >>>>> pci2:3:4 (abridged, in case the domain is "0") >>>>> pci2:3 (abridged, in case the domain and function are "0") >>>> I'm ok with what you propose, I'd wait for John to comment >>>> whether he sees any issues regarding the hints feature he is >>>> working on though. >>> This sounds good to me. >> Ok, I've tested the following patch, which also restores a feature >> of the original code, when it was not clear, whether the separator >> character was supposed to be ":" or "." (i.e., the new version does >> accept both ":" and "." as separator). This would allow to use the >> same selectors (with ".") in pciconf and the hints file ... >> >> I'd of course be willing to commit both changes separately (first >> the parsing of selectors with 2, 3 or 4 elements, then equivalence >> of ":" and "." as separators). >> >> The code wrapped in "#if 0" is not to be committed, I've included >> it just in case anybody wants to perform some tests and to check >> the parsing results. >> >> Regards, STefan >> >> >> >> Index: usr.sbin/pciconf/pciconf.c >> =================================================================== >> RCS file: /usr/cvs/src/usr.sbin/pciconf/pciconf.c,v >> retrieving revision 1.28 >> diff -u -3 -r1.28 pciconf.c >> --- usr.sbin/pciconf/pciconf.c 30 Sep 2007 11:05:17 -0000 1.28 >> +++ usr.sbin/pciconf/pciconf.c 3 Oct 2007 10:33:03 -0000 >> @@ -486,6 +486,8 @@ >> char *ep = strchr(str, '@'); >> char *epbase; >> struct pcisel sel; >> + u_int8_t selarr[4]; >> + int i; >> > > Generally looks good. Note that the domain in theory can be a > 32-bit value (chosen based on what the old alpha hose code > used; Linux seems to limit it to 16-bit) though. Thank you for your comments. I've got to admit that I did not look at any PCI standard after 2.1 (which I bought to work on the FreeBSD PCI code, a long time ago) in detai ... While I can not imagine a machine with that many domains, it is easy enough to fix the code (i.e. make all elements in selarr[] unsigned 32-bit integers and then cut off high bits when in the assignment to sel structure components. BTW: I do not plan to check for overflow of the selector fiels. This does only affect the bus part of a selector, which causes pci256:0:0 to be an alias of pci0:0:0. But this is old behaviour, and while it is easy to fix, I don't see a need. OTOH, the check would be simple to implement. Hmmm, perhaps as an add on to this patch??? Regards, STefan