[U-Boot] x86: baytrail: PCI is not always mapped to end of ram

From: Andrew Bradford <andrew.bradford@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.

Signed-off-by: Andrew Bradford <andrew.bradford@kodakalaris.com>
---
 arch/x86/cpu/baytrail/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

+Simon.

Hi Andrew,

On Sat, May 23, 2015 at 3:09 AM,  <andrew@bradfordembedded.com> wrote:
> From: Andrew Bradford <andrew.bradford@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?

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

> Signed-off-by: Andrew Bradford <andrew.bradford@kodakalaris.com>
> ---
>  arch/x86/cpu/baytrail/pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/cpu/baytrail/pci.c b/arch/x86/cpu/baytrail/pci.c
> index 6c291f9..0a87890 100644
> --- a/arch/x86/cpu/baytrail/pci.c
> +++ b/arch/x86/cpu/baytrail/pci.c
> @@ -39,7 +39,7 @@ void board_pci_setup_hose(struct pci_controller *hose)
>         pci_set_region(hose->regions + 3,
>                        0,
>                        0,
> -                      gd->ram_size,
> +                      0x80000000,
>                        PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
>
>         hose->region_count = 4;
> --

Regards,
Bin

Hi Bin,

On 05/23 23:50, Bin Meng wrote:
> +Simon.
> 
> Hi Andrew,
> 
> On Sat, May 23, 2015 at 3:09 AM,  <andrew@bradfordembedded.com> wrote:
> > From: Andrew Bradford <andrew.bradford@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.

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.

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).

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.

> 
> > Signed-off-by: Andrew Bradford <andrew.bradford@kodakalaris.com>
> > ---
> >  arch/x86/cpu/baytrail/pci.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/cpu/baytrail/pci.c b/arch/x86/cpu/baytrail/pci.c
> > index 6c291f9..0a87890 100644
> > --- a/arch/x86/cpu/baytrail/pci.c
> > +++ b/arch/x86/cpu/baytrail/pci.c
> > @@ -39,7 +39,7 @@ void board_pci_setup_hose(struct pci_controller *hose)
> >         pci_set_region(hose->regions + 3,
> >                        0,
> >                        0,
> > -                      gd->ram_size,
> > +                      0x80000000,
> >                        PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
> >
> >         hose->region_count = 4;
> > --
> 
> Regards,
> Bin

Thanks,
Andrew

Hi Andrew,

On Tue, May 26, 2015 at 8:17 PM, Andrew Bradford
<andrew@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@bradfordembedded.com> wrote:
>> > From: Andrew Bradford <andrew.bradford@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.

> 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.
>

Yes, this all makes sense, so

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Regards,
Bin

Hi Bin,

On 05/27 12:21, Bin Meng wrote:
> Hi Andrew,
> 
> On Tue, May 26, 2015 at 8:17 PM, Andrew Bradford
> <andrew@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@bradfordembedded.com> wrote:
> >> > From: Andrew Bradford <andrew.bradford@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?

> > 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.
> >
> 
> Yes, this all makes sense, so
> 
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Thanks!
-Andrew

Hi Andrew,

On Wed, May 27, 2015 at 7:53 PM, Andrew Bradford
<andrew@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@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@bradfordembedded.com> wrote:
>> >> > From: Andrew Bradford <andrew.bradford@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.

Regards,
Bin