Date: Wed, 03 Oct 2007 22:21:57 +0200 From: Stefan Esser <se@FreeBSD.org> To: Marius Strobl <marius@alchemy.franken.de> Cc: Michael Butler <imb@protected-networks.net>, Marcel Moolenaar <xcllnt@mac.com>, =?ISO-8859-1?Q?Stefan_E=DFer?= <se@localhost.FreeBSD.org>, current@FreeBSD.org, John Baldwin <jhb@FreeBSD.org> Subject: Re: Patch for review (was: Re: Proposal to revise the new parsing of PCI selectors Message-ID: <4703F9E5.7050108@FreeBSD.org> In-Reply-To: <20071003131853.GE98412@alchemy.franken.de> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <jhb@freebsd.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4703F9E5.7050108>