On Fri, May 25, 2012 at 6:12 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote:
> On Fri, May 25, 2012 at 5:10 PM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
>> On Fri, May 25, 2012 at 2:55 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote:
>>> I think we should fix this with a separate patch that removes
>>> PCIBIOS_MAX_MEM_32 altogether, replacing this use with an explicit
>>> 0xffffffff (or some other "max 32-bit value" symbol). I don't think
>>> there's anything arch-specific about this.
>>>
>>> So I'd like to see two patches here:
>>> 1) Avoid allocating 64-bit regions for 32-bit BARs
>>> 2) Try to allocate regions above 4GB for 64-bit BARs
>>
>> Sure. please check updated two patches.
>
> I think the first one looks good.
>
> I'm curious about the second. Why did you add the IORESOURCE_MEM
> test? That's doesn't affect the "start =" piece because
> IORESOURCE_MEM is always set if IORESOURCE_MEM_64 is set.
>
> But it does affect the "end =" part. Previously we limited all I/O
> and 32-bit mem BARs to the low 4GB. This patch makes it so we limit
> 32-bit mem BARs to the low 4GB, but we don't limit I/O BARs. But I/O
> BARs can only have 32 bits of address, so it seems like we should
> limit them the same way as 32-bit mem BARs. So I expected something
> like this:
>
> if (res->flags & IORESOURCE_MEM_64) {
> start = (resource_size_t) (1ULL << 32);
> end = PCI_MAX_RESOURCE;
> } else {
> start = 0;
> end = PCI_MAX_RESOURCE_32;
> }
Another bug here: we're trying to restrict the *bus* addresses we
allocate, but we're applying the limits to *CPU* addresses.
Therefore, this only works as intended when CPU addresses are the same
as bus addresses, i.e., when the host bridge applies no address
translation. That happens to be the case for x86, but is not the case
in general.
I think we need a third patch to fix this problem.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
