[03/11] MIPS: support RAM beyond 32-bit

Message ID 20231004161038.2818327-4-gregory.clement@bootlin.com
State New
Headers
Series Add support for the Mobileye EyeQ5 SoC |

Commit Message

Gregory CLEMENT Oct. 4, 2023, 4:10 p.m. UTC
  From: Vladimir Kondratiev <vladimir.kondratiev@intel.com>

Support platforms where RAM is mapped beyond 32-bit.

The kernel parameter ddr32_alias allows to setup the alias to point
outside the first 4 GB of memory.

Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@intel.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
 arch/mips/kernel/smp-cps.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
  

Comments

Arnd Bergmann Oct. 6, 2023, 11:21 a.m. UTC | #1
On Wed, Oct 4, 2023, at 18:10, Gregory CLEMENT wrote:
> From: Vladimir Kondratiev <vladimir.kondratiev@intel.com>
>
> Support platforms where RAM is mapped beyond 32-bit.
>
> The kernel parameter ddr32_alias allows to setup the alias to point
> outside the first 4 GB of memory.
>
> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@intel.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>

This needs a better explanation, and probably a rewrite.
Having to pass the memory address on the command line does
not sound like an appropriate way to boot the kernel, so
I think either this needs to be detected from the running kernel
itself, or passed through DT.

      Arnd
  
Jiaxun Yang Oct. 7, 2023, 8:14 p.m. UTC | #2
在2023年10月4日十月 下午5:10,Gregory CLEMENT写道:
> From: Vladimir Kondratiev <vladimir.kondratiev@intel.com>
>
> Support platforms where RAM is mapped beyond 32-bit.
>
> The kernel parameter ddr32_alias allows to setup the alias to point
> outside the first 4 GB of memory.

Are you trying to fix the problem that if kernel text is loaded in
XKPHYS there is no way to to set EBASE to that region?

The common practice for other 64bit MIPS system is to load kernel
in KSEG0 and add low 4G mirror with rest of the high memory to buddy
system. By doing this Kernel still have access to all memory beyond
32 bit, the only draw back is Kernel's text and data can't be relocted
beyond 32-bit.

Loading kernel into KSEG0 (i.e. with KBUILD_SYM32) have significant benefit
on performance, so I think you shouldn't try to load kernel into XKPHYS
without a good reason, but it might be helpful to add a BUG_ON at
CPS driver to handle such situation.

Btw: Is your target hardware publicly available? Folks at CIP United
are looking for EyeQ5 boards for a while, they are supporting MIPS R6
support at various projects.

Thanks
Jiaxun

>
> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@intel.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> ---
>  arch/mips/kernel/smp-cps.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c
> index 47e76722a306..fcfb19487612 100644
> --- a/arch/mips/kernel/smp-cps.c
> +++ b/arch/mips/kernel/smp-cps.c
> @@ -34,6 +34,16 @@ static unsigned __init core_vpe_count(unsigned int 
> cluster, unsigned core)
>  	return min(smp_max_threads, mips_cps_numvps(cluster, core));
>  }
> 
> +static int ddr32_alias;
> +
> +static int __init ddr32_alias_setup(char *str)
> +{
> +	get_option(&str, &ddr32_alias);
> +
> +	return 0;
> +}
> +early_param("ddr32_alias", ddr32_alias_setup);
> +
>  /**
>   * plat_core_entry - query reset vector for NMI/reset
>   *
> @@ -52,7 +62,7 @@ static u32 plat_core_entry(void)
>  {
>  #if defined(CONFIG_USE_XKPHYS)
>  	return (UNCAC_ADDR(mips_cps_core_entry) & 0xffffffff)
> -			| CM_GCR_Cx_RESET_BASE_MODE;
> +			| ddr32_alias | CM_GCR_Cx_RESET_BASE_MODE;
>  #else
>  	return CKSEG1ADDR((unsigned long)mips_cps_core_entry);
>  #endif
> -- 
> 2.40.1
  
Gregory CLEMENT Oct. 9, 2023, 3:59 p.m. UTC | #3
Hello Jiaxun,

> 在2023年10月4日十月 下午5:10,Gregory CLEMENT写道:
>> From: Vladimir Kondratiev <vladimir.kondratiev@intel.com>
>>
>> Support platforms where RAM is mapped beyond 32-bit.
>>
>> The kernel parameter ddr32_alias allows to setup the alias to point
>> outside the first 4 GB of memory.
>
> Are you trying to fix the problem that if kernel text is loaded in
> XKPHYS there is no way to to set EBASE to that region?

Yes that exactly we try to fix.

>
> The common practice for other 64bit MIPS system is to load kernel
> in KSEG0 and add low 4G mirror with rest of the high memory to buddy
> system. By doing this Kernel still have access to all memory beyond
> 32 bit, the only draw back is Kernel's text and data can't be relocted
> beyond 32-bit.
>
> Loading kernel into KSEG0 (i.e. with KBUILD_SYM32) have significant benefit
> on performance, so I think you shouldn't try to load kernel into XKPHYS
> without a good reason, but it might be helpful to add a BUG_ON at
> CPS driver to handle such situation.

I guess that being in KSEG0 allows to use shorter pointer.  But in our
case the RAM is physically connected beyond 32bits, so it is not
accessible in KSEG0.

>
> Btw: Is your target hardware publicly available? Folks at CIP United
> are looking for EyeQ5 boards for a while, they are supporting MIPS R6
> support at various projects.

We use evaluation boards and I don't know if they are publicly
available.

Gregory

>
> Thanks
> Jiaxun
>
>>
>> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@intel.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
>> ---
>>  arch/mips/kernel/smp-cps.c | 12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c
>> index 47e76722a306..fcfb19487612 100644
>> --- a/arch/mips/kernel/smp-cps.c
>> +++ b/arch/mips/kernel/smp-cps.c
>> @@ -34,6 +34,16 @@ static unsigned __init core_vpe_count(unsigned int 
>> cluster, unsigned core)
>>  	return min(smp_max_threads, mips_cps_numvps(cluster, core));
>>  }
>> 
>> +static int ddr32_alias;
>> +
>> +static int __init ddr32_alias_setup(char *str)
>> +{
>> +	get_option(&str, &ddr32_alias);
>> +
>> +	return 0;
>> +}
>> +early_param("ddr32_alias", ddr32_alias_setup);
>> +
>>  /**
>>   * plat_core_entry - query reset vector for NMI/reset
>>   *
>> @@ -52,7 +62,7 @@ static u32 plat_core_entry(void)
>>  {
>>  #if defined(CONFIG_USE_XKPHYS)
>>  	return (UNCAC_ADDR(mips_cps_core_entry) & 0xffffffff)
>> -			| CM_GCR_Cx_RESET_BASE_MODE;
>> +			| ddr32_alias | CM_GCR_Cx_RESET_BASE_MODE;
>>  #else
>>  	return CKSEG1ADDR((unsigned long)mips_cps_core_entry);
>>  #endif
>> -- 
>> 2.40.1
>
> -- 
> - Jiaxun
  
Jiaxun Yang Oct. 10, 2023, 8:55 a.m. UTC | #4
在2023年10月9日十月 下午4:59,Gregory CLEMENT写道:
> Hello Jiaxun,
>
>> 在2023年10月4日十月 下午5:10,Gregory CLEMENT写道:
>>> From: Vladimir Kondratiev <vladimir.kondratiev@intel.com>
>>>
>>> Support platforms where RAM is mapped beyond 32-bit.
>>>
>>> The kernel parameter ddr32_alias allows to setup the alias to point
>>> outside the first 4 GB of memory.
>>
>> Are you trying to fix the problem that if kernel text is loaded in
>> XKPHYS there is no way to to set EBASE to that region?
>
> Yes that exactly we try to fix.
>
>>
>> The common practice for other 64bit MIPS system is to load kernel
>> in KSEG0 and add low 4G mirror with rest of the high memory to buddy
>> system. By doing this Kernel still have access to all memory beyond
>> 32 bit, the only draw back is Kernel's text and data can't be relocted
>> beyond 32-bit.
>>
>> Loading kernel into KSEG0 (i.e. with KBUILD_SYM32) have significant benefit
>> on performance, so I think you shouldn't try to load kernel into XKPHYS
>> without a good reason, but it might be helpful to add a BUG_ON at
>> CPS driver to handle such situation.
>
> I guess that being in KSEG0 allows to use shorter pointer.  But in our
> case the RAM is physically connected beyond 32bits, so it is not
> accessible in KSEG0.

For most system there should be a mirror of part of DDR which is accessible
at KSEG0 and kernel runs from here. As per my interpretion of your code EyeQ5
is also doing this? If not could you please briefly describe the memory map?

For Kernel in KSEG0 the pointer is still 64bit but we can use fewer inst
to load ABS pointer into register, see [1].

>>
>> Btw: Is your target hardware publicly available? Folks at CIP United
>> are looking for EyeQ5 boards for a while, they are supporting MIPS R6
>> support at various projects.
>
> We use evaluation boards and I don't know if they are publicly
> available.
>
> Gregory
>
[1]: https://elinux.org/images/1/1f/New-tricks-mips-linux.pdf

Thanks
- Jiaxun
  
Gregory CLEMENT Oct. 11, 2023, 2:46 p.m. UTC | #5
Hello Jiaxun,

> 在2023年10月9日十月 下午4:59,Gregory CLEMENT写道:
>> Hello Jiaxun,
>>
>>> 在2023年10月4日十月 下午5:10,Gregory CLEMENT写道:
>>>> From: Vladimir Kondratiev <vladimir.kondratiev@intel.com>
>>>>
>>>> Support platforms where RAM is mapped beyond 32-bit.
>>>>
>>>> The kernel parameter ddr32_alias allows to setup the alias to point
>>>> outside the first 4 GB of memory.
>>>
>>> Are you trying to fix the problem that if kernel text is loaded in
>>> XKPHYS there is no way to to set EBASE to that region?
>>
>> Yes that exactly we try to fix.
>>
>>>
>>> The common practice for other 64bit MIPS system is to load kernel
>>> in KSEG0 and add low 4G mirror with rest of the high memory to buddy
>>> system. By doing this Kernel still have access to all memory beyond
>>> 32 bit, the only draw back is Kernel's text and data can't be relocted
>>> beyond 32-bit.
>>>
>>> Loading kernel into KSEG0 (i.e. with KBUILD_SYM32) have significant benefit
>>> on performance, so I think you shouldn't try to load kernel into XKPHYS
>>> without a good reason, but it might be helpful to add a BUG_ON at
>>> CPS driver to handle such situation.
>>
>> I guess that being in KSEG0 allows to use shorter pointer.  But in our
>> case the RAM is physically connected beyond 32bits, so it is not
>> accessible in KSEG0.
>
> For most system there should be a mirror of part of DDR which is accessible
> at KSEG0 and kernel runs from here. As per my interpretion of your code EyeQ5
> is also doing this? If not could you please briefly describe the memory map?
>
> For Kernel in KSEG0 the pointer is still 64bit but we can use fewer inst
> to load ABS pointer into register, see [1].
>

There is a kind of mirror but its physical address start at 0x8000000
so beyond the first 512MBytes that are used for KSEG0.

In short the 32bits mapping is the following:

 - the controllers registers of the SoC are located  until 0x8000000,
 - then from 0x8000000 to 0x10000000 there is the alias to low addresses
   of the DDR
 - then the SPIflash is mapped to from 0x10000000 to 0x20000000
 - after the PCIe Memory 32-bit addr space is from 0x20000000 to
   0x40000000

Gregory

> [1]: https://elinux.org/images/1/1f/New-tricks-mips-linux.pdf
  
Jiaxun Yang Oct. 12, 2023, 8:40 p.m. UTC | #6
在2023年10月11日十月 下午3:46,Gregory CLEMENT写道:
> Hello Jiaxun,
>
[...]
>
> There is a kind of mirror but its physical address start at 0x8000000
> so beyond the first 512MBytes that are used for KSEG0.

Really, KSEG0 range is 0x00000000 to 0x20000000, and 0x08000000 to 0x10000000
is definitely within that range.

But I'd agree that 0x08000000 to 0x10000000 (32MB) seems too small for kernel
text and data. So yeah, it makes sense to load kernel into XKPHYS.

My sugesstion is, kernel does not have to be aware of the mirror deisgn.
Say that you have DDR fully mapped at 0x100000000, you can split memory
space into two trunks: 0x08000000 to 0x10000000 and 0x102000000 to end
of the dram. Since memblock always allocate from first continuous range
in system, we can guarantee that ebase is allocated with in the first
trunk.

Thanks

>
> In short the 32bits mapping is the following:
>
>  - the controllers registers of the SoC are located  until 0x8000000,
>  - then from 0x8000000 to 0x10000000 there is the alias to low addresses
>    of the DDR
>  - then the SPIflash is mapped to from 0x10000000 to 0x20000000
>  - after the PCIe Memory 32-bit addr space is from 0x20000000 to
>    0x40000000
>
> Gregory
>
>> [1]: https://elinux.org/images/1/1f/New-tricks-mips-linux.pdf
>
> -- 
> Gregory Clement, Bootlin
> Embedded Linux and Kernel engineering
> http://bootlin.com
  
Maciej W. Rozycki Oct. 24, 2023, 9:05 a.m. UTC | #7
On Thu, 12 Oct 2023, Jiaxun Yang wrote:

> > There is a kind of mirror but its physical address start at 0x8000000
> > so beyond the first 512MBytes that are used for KSEG0.
> 
> Really, KSEG0 range is 0x00000000 to 0x20000000, and 0x08000000 to 0x10000000
> is definitely within that range.
> 
> But I'd agree that 0x08000000 to 0x10000000 (32MB) seems too small for kernel
> text and data. So yeah, it makes sense to load kernel into XKPHYS.

 Hmm, my calculation indicates the range shown spans 128MiB, which I think 
is usually suitably large to hold kernel static text and data even for the 
richest configurations.  Regardless, loading into XKPHYS isn't wrong, with 
some platforms we've been doing it for decades now.

  Maciej
  

Patch

diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c
index 47e76722a306..fcfb19487612 100644
--- a/arch/mips/kernel/smp-cps.c
+++ b/arch/mips/kernel/smp-cps.c
@@ -34,6 +34,16 @@  static unsigned __init core_vpe_count(unsigned int cluster, unsigned core)
 	return min(smp_max_threads, mips_cps_numvps(cluster, core));
 }
 
+static int ddr32_alias;
+
+static int __init ddr32_alias_setup(char *str)
+{
+	get_option(&str, &ddr32_alias);
+
+	return 0;
+}
+early_param("ddr32_alias", ddr32_alias_setup);
+
 /**
  * plat_core_entry - query reset vector for NMI/reset
  *
@@ -52,7 +62,7 @@  static u32 plat_core_entry(void)
 {
 #if defined(CONFIG_USE_XKPHYS)
 	return (UNCAC_ADDR(mips_cps_core_entry) & 0xffffffff)
-			| CM_GCR_Cx_RESET_BASE_MODE;
+			| ddr32_alias | CM_GCR_Cx_RESET_BASE_MODE;
 #else
 	return CKSEG1ADDR((unsigned long)mips_cps_core_entry);
 #endif