[U-Boot] [PATCH] x86: baytrail: PCI is not always mapped to end of ram
Andrew Bradford
andrew at bradfordembedded.com
Mon Jun 1 15:44:31 CEST 2015
Hi Bin,
On 06/01 20:58, Bin Meng wrote:
> Hi Andrew,
>
> On Mon, Jun 1, 2015 at 8:24 PM, Andrew Bradford
> <andrew at bradfordembedded.com> wrote:
> > Hi Bin,
> >
> > On 05/31 14:10, Bin Meng wrote:
> >> Hi Andrew,
> >>
> >> On Sat, May 30, 2015 at 2:27 AM, Andrew Bradford
> >> <andrew at bradfordembedded.com> wrote:
> >> > Hi Bin,
> >> >
> >> > On 05/29 13:00, Bin Meng wrote:
> >> >> Hi Andrew,
> >> >>
> >> >> On Wed, May 27, 2015 at 7:53 PM, Andrew Bradford
> >> >> <andrew at bradfordembedded.com> wrote:
> >> >> > Hi Bin,
> >> >> >
> >> >> > On 05/27 12:21, Bin Meng wrote:
> >> >> >> Hi Andrew,
> >> >> >>
> >> >> >> On Tue, May 26, 2015 at 8:17 PM, Andrew Bradford
> >> >> >> <andrew at bradfordembedded.com> wrote:
> >> >> >> > Hi Bin,
> >> >> >> >
> >> >> >> > On 05/23 23:50, Bin Meng wrote:
> >> >> >> >> +Simon.
> >> >> >> >>
> >> >> >> >> Hi Andrew,
> >> >> >> >>
> >> >> >> >> On Sat, May 23, 2015 at 3:09 AM, <andrew at bradfordembedded.com> wrote:
> >> >> >> >> > From: Andrew Bradford <andrew.bradford at kodakalaris.com>
> >> >> >> >> >
> >> >> >> >> > PCI on Intel Baytrail is mapped to 0x80000000, which is not always at
> >> >> >> >> > the end of SDRAM, such as when running with 4 GiB of SDRAM. The PCI bus
> >> >> >> >> > memory mapping must stay within low memory and so when running with >
> >> >> >> >> > 2 GiB of SDRAM, there is a hole in the SDRAM between 2 GiB and 4 GiB for
> >> >> >> >> > memory mapped IO, such as PCI.
> >> >> >> >>
> >> >> >> >> Are you saying that if we mount 4GB DDR DIMM on the MinnowMax board,
> >> >> >> >> the Intel FSP will only put 0~2G as system RAM space, and leave 2G~4G
> >> >> >> >> as PCI address and other I/Os?
> >> >> >> >
> >> >> >> > Yes. If you mount 4 GiB of SDRAM onto an E3800 processor, then physical
> >> >> >> > addresses from 0 to just below 2 GiB will be SDRAM (as per the HOBs) and
> >> >> >> > also from 4 GiB to 6 GiB (also verified via the HOBs). The space from 2
> >> >> >> > GiB to 4 GiB will be mapped as various other regions.
> >> >> >>
> >> >> >> Ah, that's exactly the information I want :)
> >> >> >>
> >> >> >> > If you see section 4.1.1.1 (page 71 in the January 2015, Revision 3.6)
> >> >> >> > E3800 datasheet, it shows that from 2 GiB to 4 GiB is mapped for PCI,
> >> >> >> > Abort Page, Local APIC, and the Boot Vector. There's a lot of space in
> >> >> >> > this area which appears unused, so I'm unsure as to why the area is so
> >> >> >> > large.
> >> >> >> >
> >> >> >> > I have an Intel Valley Island board with E3825 and a 4 GiB SODIMM. I'm
> >> >> >> > working on getting patches ready for this board but found that if I
> >> >> >> > enabled all 4 GiB of SDRAM that the PCI bus would not function
> >> >> >> > correctly. With this patch then the PCI bus functions (I'm able to do
> >> >> >> > network operations with the RTL8111 Ethernet adapter).
> >> >> >> >
> >> >> >> >> I see from minnowmax.h, the PCI address starts from 0xd0000000.
> >> >> >> >>
> >> >> >> >> #define CONFIG_PCI_MEM_BUS 0xd0000000
> >> >> >> >> #define CONFIG_PCI_MEM_PHYS CONFIG_PCI_MEM_BUS
> >> >> >> >> #define CONFIG_PCI_MEM_SIZE 0x10000000
> >> >> >> >>
> >> >> >> >> #define CONFIG_PCI_PREF_BUS 0xc0000000
> >> >> >> >> #define CONFIG_PCI_PREF_PHYS CONFIG_PCI_PREF_BUS
> >> >> >> >> #define CONFIG_PCI_PREF_SIZE 0x10000000
> >> >> >> >
> >> >> >> > I see that hose->regions+0 is set to CONFIG_PCI_MEM_BUS and
> >> >> >> > hose->regions+2 is set to CONFIG_PCI_PREF_BUS. However I'm modifying
> >> >> >> > hose->regions+3. So the values from minnowmax.h *are* being used. I'm
> >> >> >> > not yet that familiar with PCI configuration, so likely I'm not fully
> >> >> >> > understanding how u-boot sets this up.
> >> >> >> >
> >> >> >>
> >> >> >> The regions+3 is used by the inbound access, eg: PCI device access to
> >> >> >> system memory.
> >> >> >>
> >> >> >> > Possibly my address of 0x80000000 is not correct, even though it works
> >> >> >> > for me. But 0x80000000 is where it was being placed before, since it
> >> >> >> > was going at the end of SDRAM (2GiB on minnowmax).
> >> >> >> >
> >> >> >>
> >> >> >> You understanding is correct. We should only open 2GiB inbound memory
> >> >> >> window for PCI devices.
> >> >> >
> >> >> > But, if you have less than 2 GiB of memory, my patch in theory *could*
> >> >> > break things, right?. It seems like a better approach would be to limit
> >> >> > the size to the lesser of 0x80000000 and gd->ram_size. Does that make
> >> >> > sense?
> >> >> >
> >> >>
> >> >> I think 2GB is a safe value and won't break things. Region 0 and
> >> >> region 3 should not overlap.
> >> >>
> >> >> >> > If I artificially limit the amount of SDRAM by setting the fsp
> >> >> >> > configuration to memory-down and then setting the DRAM configuration
> >> >> >> > values such that I mimmic 1 GiB or 2 GiB of SDRAM, having my patch still
> >> >> >> > provides access to the PCI bus, so with my patch there should be no
> >> >> >> > adverse affects on E3800 systems that have less than 4 GiB of SDRAM.
> >> >> >> > But without my patch, when running with >=4 GiB of SDRAM, PCI accesses
> >> >> >> > end up returning "pci_hose_bus_to_phys: invalid physical address"
> >> >> >> > errors.
> >> >>
> >> >> Can you add some printf to show all of the pci_hose_bus_to_phys()
> >> >> parameters' value here when 4GB RAM is mounted? I'd like to understand
> >> >> how the message "pci_hose_bus_to_phys: invalid physical address" is
> >> >> produced.
> >> >
> >> > Patch of my changes to enable reporting of physical addresses being
> >> > used looks like:
> >> >
> >> > diff --git a/drivers/pci/pci_common.c b/drivers/pci/pci_common.c
> >> > index b9ff23f..3babcb7 100644
> >> > --- a/drivers/pci/pci_common.c
> >> > +++ b/drivers/pci/pci_common.c
> >> > @@ -205,7 +205,7 @@ int __pci_hose_bus_to_phys(struct pci_controller *hose,
> >> > return 0;
> >> > }
> >> > }
> >> > -
> >> > + printf("__pci_hose_bus_to_phys() failed!\n");
> >> > return 1;
> >> > }
> >> >
> >> > @@ -237,6 +237,7 @@ phys_addr_t pci_hose_bus_to_phys(struct pci_controller *hose,
> >> > if (ret)
> >> > puts("pci_hose_bus_to_phys: invalid physical address\n");
> >> >
> >> > + printf("bus_addr: 0x%x \t flags: 0x%lx \t phys_addr: %llx\n", bus_addr, flags, phys_addr);
> >> > return phys_addr;
> >> > }
> >> >
> >> >
> >> > When I only configure 2GB of RAM, this prints bus and physical addresses
> >> > in the 0x7adc0000 range when interfacing to the RTL8111. Everything
> >> > works as expected, and I get only 1 copy of my "failed" message for each
> >> > printing of my bus_addr, flags, and phys_addr data.
> >> >
> >> > When I switch to 4 GB of RAM configured, now __pci_hose_bus_to_phys()
> >> > will not perform the modification to the physical address that gets
> >> > passed to it. Then I see accesses to phys_addr to 0x7adc0000 range but
> >> > phys_addresses are printed as 0 and I get 2 copies of my "failed"
> >> > message for each printing of my bus_addr, flags, and phys_addr data.
> >> > Ethernet does not work in this case.
> >> >
> >> > When I change to 1 GB of RAM, everything works and the phys and bus
> >> > addresses are in the 0x3adc0000 range and I only get 1 "failed" message
> >> > for each printing of bus_addr, flags, and phys_addr data. Just like in
> >> > the 2GB case.
> >> >
> >> > Once I apply my patch to change region+3 to 0x80000000, then both 1GB
> >> > and 2 GB work the same as before, but now with 4 GB things work and the
> >> > output from my new printf()s matches the 2GB operation with phys and bus
> >> > addresses in the 0x7adc0000 range.
> >>
> >> What's the value of gd->ram_size when you have 4GB RAM mounted? My
> >> read of __pci_hose_bus_to_phys() logic is that it should still return
> >> 0x7adc0000 with the logic below.
> >>
> >> if (bus_addr >= res->bus_start && (bus_addr - res->bus_start)
> >> < res->size) {
> >> *pa = (bus_addr - res->bus_start + res->phys_start);
> >> return 0;
> >
> > gd->ram_size is 0x100000000 when I have 4 GB of SDRAM. dram_init() in
>
> Ah, I did not realize gd->ram_size can be exactly 4GiB on the
> MinnowMax board. My previous assumption was it should be something
> like 0xffxxxxxx as there has to be some memory hole configured by the
> FSP, as was seen on my Crown Bay board (it has 1GiB memory but
> gd->ram_size reports 1023MiB). So this explains the failure you saw.
>
> > fsp_dram.c sets the gd->ram_size by simply summing the sizes of all of
> > the HOBs that get returned as resource descriptors of type system or
> > reserved memory.
> >
> > Without patch [1] the meminfo command will report 0 bytes of RAM, but
> > with patch [1] then the meminfo command reports 4 GB of RAM.
> >
> > [1]:http://git.denx.de/?p=u-boot.git;a=commitdiff;h=ea11b401b5ca10b5991e7c65832cfb7db54996c1
> >
> > Without the patch from this thread (not [1]), and with 4 GB of RAM, hose
> > region 3 is bus_start=0, phys_start=0, and size=0. And
> > __pci_hose_bus_to_phys() is going through regions 0, 1, 2, and 3. It's
> > region 3 that's failing. Regions 0, 1, and 2 seem fine with or without
> > my patch from this thread.
> >
> > With the patch from this thread, then the size for region 3 is
> > 0x80000000 and regions 0, 1, and 2 have the same values as without the
> > patch. With the patch, __pci_hose_bus_to_phys() works the same for
> > configurations of 1, 2, or 4 GB of RAM.
> >
> > I'm still concerned about the less than 2 GB of RAM case, as with my
> > patch then region 3 will always be 0x80000000 in size and we may not
> > actually have that much RAM. Likely this won't actually cause a
> > problem, but it still worries me.
> >
>
> This should not matter. The region 3 is just by U-Boot to translate
> the bus address. It is not configured to any hardware registers. But
> if you worry that too much, you can configure region 3 as:
>
> pci_set_region(hose->regions + 3,
> 0,
> 0,
> - gd->ram_size,
> + gd->ram_size < 0x80000000 ? gd->ram_size : 0x80000000;,
> PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
Yes, this would make me worry less :)
I can submit a v2.
> > My test case that fails is trying to run the 'dhcp' command to fetch the
> > Linux kernel over the Ethernet network connected to the RTL8111.
> >
>
> Is your test case fails because of this region3 size equals to 2GiB? I
> don't think so.
My test case failes because region 3 shows a size of 0, without my
patch when you have 4 GB of RAM.
> >> > Sorry I'm relaying information rather than giving direct output from the
> >> > board, but I don't yet have a UART setup on my Valley Island from within
> >> > u-boot, I'm doing all my work via the vesa console and a USB keyboard.
> >> >
> >>
> >> I thought you just wanted to enable the early debug UART on the Valley
> >> Island board before to debug FSP, but it looks to me that you did not
> >> get U-Boot's console on any (legacy or PCI) serial port when U-Boot is
> >> up? Yes? Since you mentioned that Valley Island board has the PCI UART
> >> connected from the BayTrail SoC, I think you can refer to Crown Bay's
> >> implementation (arch/x86/dts/crownbay.dts) to add PCI UART in the
> >> board's device tree so you can have a working U-Boot console over a
> >> PCI UART.
> >>
> >> chosen {
> >> /*
> >> * By default the legacy superio serial port is used as the
> >> * U-Boot serial console. If we want to use UART from Topcliff
> >> * PCH as the console, change this property to &pciuart#.
> >> *
> >> * For example, stdout-path = &pciuart0 will use the first
> >> * UART on Topcliff PCH.
> >> */
> >> stdout-path = "/serial";
> >> };
> >
> > OK, I'll give this a shot soon, thanks for the pointers! :)
Thanks,
Andrew
More information about the U-Boot
mailing list