[U-Boot-Users] Please explain this code in drivers/pci.c

Donald White dbwhite at asu.edu
Wed May 21 05:14:53 CEST 2003


Udi,

I am convinced now it is incorrect.  A minimal fix is to make the code look like this:

     /* Check the BAR type and set our address mask */
     if (bar_response & PCI_BASE_ADDRESS_SPACE)
     {
         bar_size = ~(bar_response & PCI_BASE_ADDRESS_IO_MASK) + 1;

         /* round up region base address to a multiple of size */
         io = ((io - 1) | (bar_size - 1)) + 1;
         bar_value = io;
         /* compute new base address for region */
         io = io + bar_size;
     }
     else
     {
         if ( (bar_response & PCI_BASE_ADDRESS_MEM_TYPE_MASK) ==
              PCI_BASE_ADDRESS_MEM_TYPE_64)
             found_mem64 = 1;

         bar_size = ~(bar_response & PCI_BASE_ADDRESS_MEM_MASK) + 1;
         /* round up region base address to multiple of size */
         mem = ((mem - 1) | (bar_size - 1)) + 1;
         bar_value = mem;
         /* compute new region base address */
         mem = mem + bar_size;
     }

The ORing just rounds up the current base for the region to be a multiple of the 
size.  The ORing was in the wrong place and there was no increment of the region 
base to beyond the allocated area.  It worked with cards with single BARs and 
suitable region bases.

This is not optimal, but it is useable for simple cases.

Thanks for the comments,

Don


Udi Finkelstein wrote:
> I've looked at the code you posted, and it looks indeed incorrect. (I 
> didn't look into the entire file in u-boot, so take this with a grain of 
> salt).
> However, and optimal PCI alolcator must sort the BAR size in decreasing 
> orders if it wants to pack the BARs into an area as small as possible.
> If you have BAR0 size 4K and BAR1 size 256M and BAR2 size 4K, then 
> allocating BAR0 from (for example) 0xe0000000 would yield BAR0 range of 
> 0xe0000000-0xe0000fff, BAR1 would be mapped to 0xf0000000 - 0xffffffff, 
> and BAR2 would not fit!
> An optimal allocator must start from the biggest BARs first:
> 
> BAR1 would fit at 0xe0000000-0xefffffff
> BAR2 would fit at 0xf0000000-0xf0ffffff
> BAR0 would fit at 0xf1000000-0xf1000fff
> 
> Udi
> 
> Donald White wrote:
> 
>>
>> Rune Torgersen wrote:
>>
>>>
>>>> Subject: [U-Boot-Users] Please explain this code in drivers/pci.c
>>>
>>>
>>>
>>>
>>>> The value of bar_response is 0xffff8008 which looks good.  This 
>>>> gives bar
>>>
>>>
>>>
>>> size of
>>>
>>>> 0x8000 which also looks good.  The value of mem is initially 
>>>> 0x90000000.
>>>
>>>
>>>
>>> So,
>>>
>>>> (mem -  1) = 0x8fffffff and (bar_size - 1) = 0x7fff).  When these 
>>>> are or'd
>>>
>>>
>>>
>>> one gets
>>>
>>>> 0x8fffffff and adding 1 gives 0x90000000.  This result is that the 
>>>> card is
>>>
>>>
>>>
>>> given 4
>>>
>>>> address bases all at 0x90000000.
>>>
>>>
>>>
>>>
>>>
>>> If the card wants 4 different memory regiuons, I assume it has 4 BAR
>>> registers? Each BAR must have the correct base address, so you have 
>>> to add
>>> the size of the last region to the memory pointer before writing the
>>> address to the next BAR
>>>
>>>
>>
>> I am not sure if you are trying to clarify the question or provide an 
>> answer.  My complaint is that the code does not do what you say.  I 
>> believe the code should provide four unique values for the four BARs, 
>> but it does not.  Each gets the same value because the ORing logic 
>> does not succeed in doing a new base = old base + size.
>>
>> The logic envoked by CONFIG_PCI_PNP gets the calculation right.  If 
>> someone is familiar with the code and can confirm that it is not 
>> behaving as intended, then I will do a patch.  The ORing logic looks 
>> to me to be copied from the Linux kernel where the allocation starts 
>> at the top of the address range and proceeds downward, but maybe it is 
>> an attempt to force alignment.  I don't know.
>>
>> Don
>>
>>
>>
>> -------------------------------------------------------
>> This SF.net email is sponsored by: ObjectStore.
>> If flattening out C++ or Java code to make your application fit in a
>> relational database is painful, don't do it! Check out ObjectStore.
>> Now part of Progress Software. http://www.objectstore.net/sourceforge
>> _______________________________________________
>> U-Boot-Users mailing list
>> U-Boot-Users at lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/u-boot-users
>>
> 






More information about the U-Boot mailing list