Re: Patch for review (was: Re: Proposal to revise the new parsing of PCI selectors

From: Stefan Esser <se_at_FreeBSD.org>
Date: Wed, 03 Oct 2007 22:21:57 +0200
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_at_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
>> _at__at_ -486,6 +486,8 _at__at_
>>  	char *ep = strchr(str, '_at_');
>>  	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
Received on Wed Oct 03 2007 - 18:22:03 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:39:18 UTC