| Message ID | 20231004161038.2818327-4-gregory.clement@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for the Mobileye EyeQ5 SoC | expand |
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
在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
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
在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
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
在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
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
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