Grant Grundler wrote: > On Mon, Nov 30, 2009 at 09:10:54PM -0700, Alex Williamson wrote: >> On Mon, 2009-11-30 at 17:19 -0700, Grant Grundler wrote: >>> On Mon, Nov 30, 2009 at 05:03:32PM -0700, Grant Grundler wrote: >>>> On Mon, Nov 30, 2009 at 02:51:44PM -0700, Alex Williamson wrote: >>>>> Prior to 1f82de10 we always initialized the upper 32bits of the >>>>> prefetchable memory window, regardless of the address range used. >>>>> Now we only touch it for a >32bit address, which means the upper32 >>>>> registers remain whatever the BIOS initialized them too. >>>>> >>>>> It's valid for the BIOS to set the upper32 base/limit to >>>>> 0xffffffff/0x00000000, which makes us program prefetchable ranges >>>>> like 0xffffffffabc00000 - 0x00000000abc00000 >>>>> >>>>> Revert the chunk of 1f82de10 that made this conditional so we always >>>>> write the upper32 registers and remove now unused pref_mem64 variable. >>>>> >>>>> Signed-off-by: Alex Williamson <alex.williamson@xxxxxx> >>>> Reviewed-by: Grant Grundler <grundler@xxxxxxxxxxxxxxxx> >>> NAK this - I messed up. Yinghai is correct. Something else is going on. >>> >>> It might be perfectly OK to read 0xffffffffabc00000 if the bridge >>> isn't using the upper32 Prefetchable register. Maybe the problem is >>> some code is reading the upper32 value without checking that it's valid? >> Apologies for not threading the v2 patch into the original thread. The >> prefetchable base register does support the upper32 bits and it does >> work correctly. However per the pci-to-pci bridge spec, a little lower >> on page 47, devices only supporting 32bit prefetchable ranges are to >> implement the upper32 registers as read-only registers that return zero. >> In the example above, -1 in the upper32 base simply means that base > >> limit, which disables the range. >> >> Further investigation shows that the MEM_64 resource flag is setup for >> this range based on hardware capabilities, but then it gets removed in >> pbus_size_mem() because we want to use the range to map a 32bit option >> ROM. This leaves us entering pci_setup_bridge() with -1 in the upper32 >> base and the MEM_64 flag clear, so we never touch the upper32 base >> register. I think this patch is still a simple, safe solution. Thanks, > > Yup - after reading the PCI-PCI spec a 3rd time. I have to agree. > Alex, sorry for the flip flopping. Pre-2.6.30 code was clearly working. > Please add: > Reviewed-by: Grant Grundler <grundler@xxxxxxxxxxxxxxxx> > > I assumed Yinghai's objection was based on a specific problem he had > seen with writing upper32 register. Bjorn asked the right question. > If there isn't a specific problem, I'd prefer AW's simpler patch. we just should not touch that register if the HW only support 32bit pref mmio. > > I'm also thinking the resource allocation design which uses resource > flags to indicate resources assigned (e.g a resource is 32-bit) rather > than HW attributes is broken. We should be able to allocate 32-bit Option > ROM into a 64-bit prefetchable MMIO window that is programmed with upper32 > as zeros without changing the resource type. The resource allocation > code only be looking at Resource "Type" when (re)programming > window registers. The rest of the time (programming BARs) should be > able to just test "if it fits". IORESOURCE_MEM_64 is the flags that the resource could be assigned to >4g range. so it is NOT assigned resource ... YH -- 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
