Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
> On 12/13/22 11:24, Tom Rini wrote: > > On Tue, Dec 13, 2022 at 08:42:47AM +0800, Rick Chen wrote: > >> Hi Sean, > >> > >>> On 12/12/22 10:03, Tom Rini wrote: > On Mon, Dec 12, 2022 at 02:45:10PM +0800, Rick Chen wrote: > > Hi Tom > > > >> On Fri, Dec 09, 2022 at 08:48:37AM -0500, Sean Anderson wrote: > >>> On 12/7/22 01:23, Rick Chen wrote: > In RISC-V, it only provide normal mode booting currently. > To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT > to achieve this feature which will be call Fast-Boot mode. By > >>> > >>> Can you name this something different. We already have something > >>> called > >>> fastboot in-tree (the Android-derived protocol) and there's a > >>> Microsoft > >>> technology called fastboot (some kind of hibernation). "OS Boot" isn't > >>> very specific either, since we (almost always) boot an OS. Maybe > >>> "Eagle > >>> mode" by analogy to Falcon mode, which lets SPL directly boot an OS. > >>> > >>> (Is this substantially different from falcon mode anyway?) > >> > >> I was kind of wondering if this is different, really, from Falcon Mode. > >> Falcon Mode didn't initially have to factor in other-firmware as that's > >> not a hard requirement on arm32 like it is on arm64 or risc-v. But my > >> first read of this was that it seems like the RISC-V specific side of > >> doing Falcon Mode and dealing with the prior stage needs correctly. > >> > > > > Yes. It is a little bit different from the Falcon mode (SPL_OS_BOOT=y). > > When I try to enable SPL_OS_BOOT, it will encounter that > > SYS_SPL_ARGS_ADDR and > >jump_to_image_linux() shall be defined but they are un-necessary for > > RISC-V. > > Because the flow of OpenSBI and SPL_OS_BOOT are totally different code > > flow in board_init_r() of common/spl/spl.c. > > That is why I added a new symbol called SPL_OPENSBI_OS_BOOT for this > > RISC-V fast boot implementation. > > Those sound like fairly minor challenges for the same fundamental > concept. We have SYS_SPL_ARGS_ADDR for "where is the device tree to > pass along". We might need to do a little code re-factoring here. But > maybe also a little bit of explaining why we wouldn't be booting to the > OS directly but instead passing back to openSBI to do this? That's not > normally how RISC-V boots the OS, right? Or am I miss-understanding > something here? > > >>> > >>> The usual process is > >>> > >>> ROM -> SPL -> SBI -> U-Boot -> Linux > >> > >> In RISC-V, Linux runs in S-mode, it needs the OpenSBI which plays as > >> M-mode to deal with SBI calls at run-time. > >> So the fast boot idea, I just want to bypass U-Boot and jump to Linux > >> directly and Linux still needs OpenSBI. > >> Hence SPL_OPENSBI can't be disabled here. > > > > So is the flow here: > > a) ROM -> SPL -> SBI -> SPL -> Linux > > or > > b) ROM -> SPL -> SBI -> Linux > > The latter. The regular boot is > > (M) Boot ROM loads SPL and executes it > (M) SPL loads SBI and U-Boot proper and executes SBI > (M) SBI performs initialization and executes U-Boot > (S) U-Boot loads Linux and executes it > (S) Linux boots > > And this would change that to > > (M) Boot ROM loads SPL and executes it > (M) SPL loads SBI and Linux and executes SBI > (M) SBI performs initialization and executes Linux > (S) Linux boots > > So the idea here is to load Linux in SPL instead of U-Boot. But I think this > is > the only way to go for platforms which use OpenSBI. Regular falcon mode would > require a Linux which runs in M-mode. So I don't think we need a separate > config. Hi Sean, Thanks for your explanation. It is very clear. Hi Tom, Sean, Do you have any further suggestions that I can improve or refine this patch ? Thanks, Rick > > --Sean > > > It's not clear to me, and I think that'll help figure out what's best > > here. Since I kinda think we're looking at a more generic issue that we > > see with newer architectures and maybe we already had to solve this (but > > didn't name things well enough) for aarch64. > > > > >
Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
Hi Tom, > On Wed, Dec 14, 2022 at 08:49:03AM +0800, Rick Chen wrote: > > Hi Tom, > > > > > On Tue, Dec 13, 2022 at 10:06:50AM +0800, Rick Chen wrote: > > > > > On Mon, Dec 12, 2022 at 03:49:10PM +0800, Rick Chen wrote: > > > > > > > On 12/7/22 01:23, Rick Chen wrote: > > > > > > > > In RISC-V, it only provide normal mode booting currently. > > > > > > > > To speed up the booting process, here provide > > > > > > > > SPL_OPENSBI_OS_BOOT > > > > > > > > to achieve this feature which will be call Fast-Boot mode. By > > > > > > > > > > > > > > Can you name this something different. We already have something > > > > > > > called > > > > > > > fastboot in-tree (the Android-derived protocol) and there's a > > > > > > > Microsoft > > > > > > > technology called fastboot (some kind of hibernation). "OS Boot" > > > > > > > isn't > > > > > > > very specific either, since we (almost always) boot an OS. Maybe > > > > > > > "Eagle > > > > > > > mode" by analogy to Falcon mode, which lets SPL directly boot an > > > > > > > OS. > > > > > > > > > > > > I think fast boot is a behavior which shall be interpreted widely > > > > > > but > > > > > > not proprietary. > > > > > > Or maybe I can rename it as RISC-V Fast Boot to distinguish them. > > > > > > > > > > > > > > > > > > > > (Is this substantially different from falcon mode anyway?) > > > > > > > > > > > > Please see the explanations to Tom. > > > > > > > > > > > > > > > > > > > > > enabling SPL_OPENSBI_OS_BOOT, it will generate linux.itb instead > > > > > > > > of default u-boot.itb after compiling. It initializes memory > > > > > > > > with > > > > > > > > the U-Boot SPL at the first stage, just like what a regular > > > > > > > > booting > > > > > > > > process (i.e. Normal Boot) does in the beginning. Instead of > > > > > > > > jumping > > > > > > > > to the U-Boot proper from OpenSBI before booting Linux Kernel, > > > > > > > > the > > > > > > > > Fast Boot process jumps directly to Linux Kernel to gain shorter > > > > > > > > booting time. > > > > > > > > > > > > > > > > Signed-off-by: Rick Chen > > > > > > > > --- > > > > > > > > common/spl/Kconfig | 14 ++ > > > > > > > > common/spl/spl_fit.c | 3 ++- > > > > > > > > common/spl/spl_opensbi.c | 25 - > > > > > > > > 3 files changed, 28 insertions(+), 14 deletions(-) > > > > > > > > > > > > > > > > diff --git a/common/spl/Kconfig b/common/spl/Kconfig > > > > > > > > index 05181bdba3..8805aba1b7 100644 > > > > > > > > --- a/common/spl/Kconfig > > > > > > > > +++ b/common/spl/Kconfig > > > > > > > > @@ -1509,6 +1509,20 @@ config SPL_OPENSBI_SCRATCH_OPTIONS > > > > > > > > Options passed to fw_dynamic, for example > > > > > > > > SBI_SCRATCH_NO_BOOT_PRINTS or > > > > > > > > SBI_SCRATCH_DEBUG_PRINTS. > > > > > > > > > > > > > > > > +config SPL_OPENSBI_OS_BOOT > > > > > > > > > > > > > > Please use the same name for the config as for the description. > > > > > > > > > > > > > > > + bool "openSBI Fast Boot" > > > > > > > > + depends on SPL_OPENSBI > > > > > > > > + help > > > > > > > > + Enable this openSBI can jump to Linux Kernel directly. > > > > > > > > > > > > > > Can you put some of the explanation from the commit message here? > > > > > > > > > > > > OK, I will move some messages here from commit messages. > > > > > > > > > > > > > > > > > > > > > + > > > > > > > > +config SPL_OPENSBI_FIT_NAME > > > > > > > > + string "SPL openSBI fit image name" > > > > > > > > + depends on SPL_OPENSBI > > > > > > > > + default "linux.itb" if SPL_OPENSBI_OS_BOOT > > > > > > > > + default "u-boot.itb" > > > > > > > > + help > > > > > > > > + This will help to generate different fit name > > > > > > > > accordingly. > > > > > > > > > > > > > > Why not SPL_FS_LOAD_PAYLOAD_NAME? > > > > > > > > > > > > > > It looks like the code changes below do not use these configs. > > > > > > > Can you > > > > > > > move them to the next patch so it is clearer that they are for > > > > > > > binman? > > > > > > > > > > > > I have saw this config, but it does't support SPL_RAM but only for > > > > > > filesystem > > > > > > That is why I don't leverage it. > > > > > > > > > > > > I can prepare a patch as below if no other concerns: > > > > > > > > > > > > config SPL_FS_LOAD_PAYLOAD_NAME > > > > > > string "File to load for U-Boot from the filesystem" > > > > > > depends on SPL_FS_EXT4 || SPL_FS_FAT || SPL_FS_SQUASHFS || SPL_RAM > > > > > > default "tispl.bin" if SYS_K3_SPL_ATF > > > > > > default "u-boot.itb" if SPL_LOAD_FIT > > > > > > default "linux.itb" if SPL_OPENSBI_OS_BOOT > > > > > > default "u-boot.img" > > > > > > help > > > > > > Filename to read to load U-Boot when reading from filesystem. > > > > > > > > > > But in the case you have it's for the output name, and not at all > > > > > about > > > > > what's being loaded from a filesystem yes? This shouldn't be a > > > > > "SPL_.." > > > > > > > > For
Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
On 12/13/22 11:24, Tom Rini wrote: On Tue, Dec 13, 2022 at 08:42:47AM +0800, Rick Chen wrote: Hi Sean, On 12/12/22 10:03, Tom Rini wrote: On Mon, Dec 12, 2022 at 02:45:10PM +0800, Rick Chen wrote: Hi Tom On Fri, Dec 09, 2022 at 08:48:37AM -0500, Sean Anderson wrote: On 12/7/22 01:23, Rick Chen wrote: In RISC-V, it only provide normal mode booting currently. To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT to achieve this feature which will be call Fast-Boot mode. By Can you name this something different. We already have something called fastboot in-tree (the Android-derived protocol) and there's a Microsoft technology called fastboot (some kind of hibernation). "OS Boot" isn't very specific either, since we (almost always) boot an OS. Maybe "Eagle mode" by analogy to Falcon mode, which lets SPL directly boot an OS. (Is this substantially different from falcon mode anyway?) I was kind of wondering if this is different, really, from Falcon Mode. Falcon Mode didn't initially have to factor in other-firmware as that's not a hard requirement on arm32 like it is on arm64 or risc-v. But my first read of this was that it seems like the RISC-V specific side of doing Falcon Mode and dealing with the prior stage needs correctly. Yes. It is a little bit different from the Falcon mode (SPL_OS_BOOT=y). When I try to enable SPL_OS_BOOT, it will encounter that SYS_SPL_ARGS_ADDR and jump_to_image_linux() shall be defined but they are un-necessary for RISC-V. Because the flow of OpenSBI and SPL_OS_BOOT are totally different code flow in board_init_r() of common/spl/spl.c. That is why I added a new symbol called SPL_OPENSBI_OS_BOOT for this RISC-V fast boot implementation. Those sound like fairly minor challenges for the same fundamental concept. We have SYS_SPL_ARGS_ADDR for "where is the device tree to pass along". We might need to do a little code re-factoring here. But maybe also a little bit of explaining why we wouldn't be booting to the OS directly but instead passing back to openSBI to do this? That's not normally how RISC-V boots the OS, right? Or am I miss-understanding something here? The usual process is ROM -> SPL -> SBI -> U-Boot -> Linux In RISC-V, Linux runs in S-mode, it needs the OpenSBI which plays as M-mode to deal with SBI calls at run-time. So the fast boot idea, I just want to bypass U-Boot and jump to Linux directly and Linux still needs OpenSBI. Hence SPL_OPENSBI can't be disabled here. So is the flow here: a) ROM -> SPL -> SBI -> SPL -> Linux or b) ROM -> SPL -> SBI -> Linux The latter. The regular boot is (M) Boot ROM loads SPL and executes it (M) SPL loads SBI and U-Boot proper and executes SBI (M) SBI performs initialization and executes U-Boot (S) U-Boot loads Linux and executes it (S) Linux boots And this would change that to (M) Boot ROM loads SPL and executes it (M) SPL loads SBI and Linux and executes SBI (M) SBI performs initialization and executes Linux (S) Linux boots So the idea here is to load Linux in SPL instead of U-Boot. But I think this is the only way to go for platforms which use OpenSBI. Regular falcon mode would require a Linux which runs in M-mode. So I don't think we need a separate config. --Sean It's not clear to me, and I think that'll help figure out what's best here. Since I kinda think we're looking at a more generic issue that we see with newer architectures and maybe we already had to solve this (but didn't name things well enough) for aarch64.
Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
On Wed, Dec 14, 2022 at 08:49:03AM +0800, Rick Chen wrote: > Hi Tom, > > > On Tue, Dec 13, 2022 at 10:06:50AM +0800, Rick Chen wrote: > > > > On Mon, Dec 12, 2022 at 03:49:10PM +0800, Rick Chen wrote: > > > > > > On 12/7/22 01:23, Rick Chen wrote: > > > > > > > In RISC-V, it only provide normal mode booting currently. > > > > > > > To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT > > > > > > > to achieve this feature which will be call Fast-Boot mode. By > > > > > > > > > > > > Can you name this something different. We already have something > > > > > > called > > > > > > fastboot in-tree (the Android-derived protocol) and there's a > > > > > > Microsoft > > > > > > technology called fastboot (some kind of hibernation). "OS Boot" > > > > > > isn't > > > > > > very specific either, since we (almost always) boot an OS. Maybe > > > > > > "Eagle > > > > > > mode" by analogy to Falcon mode, which lets SPL directly boot an OS. > > > > > > > > > > I think fast boot is a behavior which shall be interpreted widely but > > > > > not proprietary. > > > > > Or maybe I can rename it as RISC-V Fast Boot to distinguish them. > > > > > > > > > > > > > > > > > (Is this substantially different from falcon mode anyway?) > > > > > > > > > > Please see the explanations to Tom. > > > > > > > > > > > > > > > > > > enabling SPL_OPENSBI_OS_BOOT, it will generate linux.itb instead > > > > > > > of default u-boot.itb after compiling. It initializes memory with > > > > > > > the U-Boot SPL at the first stage, just like what a regular > > > > > > > booting > > > > > > > process (i.e. Normal Boot) does in the beginning. Instead of > > > > > > > jumping > > > > > > > to the U-Boot proper from OpenSBI before booting Linux Kernel, the > > > > > > > Fast Boot process jumps directly to Linux Kernel to gain shorter > > > > > > > booting time. > > > > > > > > > > > > > > Signed-off-by: Rick Chen > > > > > > > --- > > > > > > > common/spl/Kconfig | 14 ++ > > > > > > > common/spl/spl_fit.c | 3 ++- > > > > > > > common/spl/spl_opensbi.c | 25 - > > > > > > > 3 files changed, 28 insertions(+), 14 deletions(-) > > > > > > > > > > > > > > diff --git a/common/spl/Kconfig b/common/spl/Kconfig > > > > > > > index 05181bdba3..8805aba1b7 100644 > > > > > > > --- a/common/spl/Kconfig > > > > > > > +++ b/common/spl/Kconfig > > > > > > > @@ -1509,6 +1509,20 @@ config SPL_OPENSBI_SCRATCH_OPTIONS > > > > > > > Options passed to fw_dynamic, for example > > > > > > > SBI_SCRATCH_NO_BOOT_PRINTS or > > > > > > > SBI_SCRATCH_DEBUG_PRINTS. > > > > > > > > > > > > > > +config SPL_OPENSBI_OS_BOOT > > > > > > > > > > > > Please use the same name for the config as for the description. > > > > > > > > > > > > > + bool "openSBI Fast Boot" > > > > > > > + depends on SPL_OPENSBI > > > > > > > + help > > > > > > > + Enable this openSBI can jump to Linux Kernel directly. > > > > > > > > > > > > Can you put some of the explanation from the commit message here? > > > > > > > > > > OK, I will move some messages here from commit messages. > > > > > > > > > > > > > > > > > > + > > > > > > > +config SPL_OPENSBI_FIT_NAME > > > > > > > + string "SPL openSBI fit image name" > > > > > > > + depends on SPL_OPENSBI > > > > > > > + default "linux.itb" if SPL_OPENSBI_OS_BOOT > > > > > > > + default "u-boot.itb" > > > > > > > + help > > > > > > > + This will help to generate different fit name accordingly. > > > > > > > > > > > > Why not SPL_FS_LOAD_PAYLOAD_NAME? > > > > > > > > > > > > It looks like the code changes below do not use these configs. Can > > > > > > you > > > > > > move them to the next patch so it is clearer that they are for > > > > > > binman? > > > > > > > > > > I have saw this config, but it does't support SPL_RAM but only for > > > > > filesystem > > > > > That is why I don't leverage it. > > > > > > > > > > I can prepare a patch as below if no other concerns: > > > > > > > > > > config SPL_FS_LOAD_PAYLOAD_NAME > > > > > string "File to load for U-Boot from the filesystem" > > > > > depends on SPL_FS_EXT4 || SPL_FS_FAT || SPL_FS_SQUASHFS || SPL_RAM > > > > > default "tispl.bin" if SYS_K3_SPL_ATF > > > > > default "u-boot.itb" if SPL_LOAD_FIT > > > > > default "linux.itb" if SPL_OPENSBI_OS_BOOT > > > > > default "u-boot.img" > > > > > help > > > > > Filename to read to load U-Boot when reading from filesystem. > > > > > > > > But in the case you have it's for the output name, and not at all about > > > > what's being loaded from a filesystem yes? This shouldn't be a "SPL_.." > > > > > > For SPL_RAM purpose, > > > it is just an output name to distinguish normal boot (u-boot.itb) and > > > fast boot (linux.itb) indeed. > > > > > > But for SPL_OPENSBI_OS_BOOT && SPL_MMC && SPL_FAT this combination, > > > it can help to load linux.itb automatically from SD card. > > > > What part of the world is looking for the file named by > >
Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
Hi Tom, > On Tue, Dec 13, 2022 at 10:06:50AM +0800, Rick Chen wrote: > > > On Mon, Dec 12, 2022 at 03:49:10PM +0800, Rick Chen wrote: > > > > > On 12/7/22 01:23, Rick Chen wrote: > > > > > > In RISC-V, it only provide normal mode booting currently. > > > > > > To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT > > > > > > to achieve this feature which will be call Fast-Boot mode. By > > > > > > > > > > Can you name this something different. We already have something > > > > > called > > > > > fastboot in-tree (the Android-derived protocol) and there's a > > > > > Microsoft > > > > > technology called fastboot (some kind of hibernation). "OS Boot" isn't > > > > > very specific either, since we (almost always) boot an OS. Maybe > > > > > "Eagle > > > > > mode" by analogy to Falcon mode, which lets SPL directly boot an OS. > > > > > > > > I think fast boot is a behavior which shall be interpreted widely but > > > > not proprietary. > > > > Or maybe I can rename it as RISC-V Fast Boot to distinguish them. > > > > > > > > > > > > > > (Is this substantially different from falcon mode anyway?) > > > > > > > > Please see the explanations to Tom. > > > > > > > > > > > > > > > enabling SPL_OPENSBI_OS_BOOT, it will generate linux.itb instead > > > > > > of default u-boot.itb after compiling. It initializes memory with > > > > > > the U-Boot SPL at the first stage, just like what a regular booting > > > > > > process (i.e. Normal Boot) does in the beginning. Instead of jumping > > > > > > to the U-Boot proper from OpenSBI before booting Linux Kernel, the > > > > > > Fast Boot process jumps directly to Linux Kernel to gain shorter > > > > > > booting time. > > > > > > > > > > > > Signed-off-by: Rick Chen > > > > > > --- > > > > > > common/spl/Kconfig | 14 ++ > > > > > > common/spl/spl_fit.c | 3 ++- > > > > > > common/spl/spl_opensbi.c | 25 - > > > > > > 3 files changed, 28 insertions(+), 14 deletions(-) > > > > > > > > > > > > diff --git a/common/spl/Kconfig b/common/spl/Kconfig > > > > > > index 05181bdba3..8805aba1b7 100644 > > > > > > --- a/common/spl/Kconfig > > > > > > +++ b/common/spl/Kconfig > > > > > > @@ -1509,6 +1509,20 @@ config SPL_OPENSBI_SCRATCH_OPTIONS > > > > > > Options passed to fw_dynamic, for example > > > > > > SBI_SCRATCH_NO_BOOT_PRINTS or > > > > > > SBI_SCRATCH_DEBUG_PRINTS. > > > > > > > > > > > > +config SPL_OPENSBI_OS_BOOT > > > > > > > > > > Please use the same name for the config as for the description. > > > > > > > > > > > + bool "openSBI Fast Boot" > > > > > > + depends on SPL_OPENSBI > > > > > > + help > > > > > > + Enable this openSBI can jump to Linux Kernel directly. > > > > > > > > > > Can you put some of the explanation from the commit message here? > > > > > > > > OK, I will move some messages here from commit messages. > > > > > > > > > > > > > > > + > > > > > > +config SPL_OPENSBI_FIT_NAME > > > > > > + string "SPL openSBI fit image name" > > > > > > + depends on SPL_OPENSBI > > > > > > + default "linux.itb" if SPL_OPENSBI_OS_BOOT > > > > > > + default "u-boot.itb" > > > > > > + help > > > > > > + This will help to generate different fit name accordingly. > > > > > > > > > > Why not SPL_FS_LOAD_PAYLOAD_NAME? > > > > > > > > > > It looks like the code changes below do not use these configs. Can you > > > > > move them to the next patch so it is clearer that they are for binman? > > > > > > > > I have saw this config, but it does't support SPL_RAM but only for > > > > filesystem > > > > That is why I don't leverage it. > > > > > > > > I can prepare a patch as below if no other concerns: > > > > > > > > config SPL_FS_LOAD_PAYLOAD_NAME > > > > string "File to load for U-Boot from the filesystem" > > > > depends on SPL_FS_EXT4 || SPL_FS_FAT || SPL_FS_SQUASHFS || SPL_RAM > > > > default "tispl.bin" if SYS_K3_SPL_ATF > > > > default "u-boot.itb" if SPL_LOAD_FIT > > > > default "linux.itb" if SPL_OPENSBI_OS_BOOT > > > > default "u-boot.img" > > > > help > > > > Filename to read to load U-Boot when reading from filesystem. > > > > > > But in the case you have it's for the output name, and not at all about > > > what's being loaded from a filesystem yes? This shouldn't be a "SPL_.." > > > > For SPL_RAM purpose, > > it is just an output name to distinguish normal boot (u-boot.itb) and > > fast boot (linux.itb) indeed. > > > > But for SPL_OPENSBI_OS_BOOT && SPL_MMC && SPL_FAT this combination, > > it can help to load linux.itb automatically from SD card. > > What part of the world is looking for the file named by > SPL_FS_LOAD_PAYLOAD_NAME ? (and this is related to what I just sent out, > asking about the boot flow) In the following commit, CONFIG_SPL_FS_LOAD_PAYLOAD_NAME has been moved to Kconfig from ax25-ae350.h That is why I try to add this change to load u-boot.itb or linux.itb from SD card here commit 4a11e34bc9c0f3818f3e847ac51c82d1c9bbb807
Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
On Tue, Dec 13, 2022 at 10:06:50AM +0800, Rick Chen wrote: > > On Mon, Dec 12, 2022 at 03:49:10PM +0800, Rick Chen wrote: > > > > On 12/7/22 01:23, Rick Chen wrote: > > > > > In RISC-V, it only provide normal mode booting currently. > > > > > To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT > > > > > to achieve this feature which will be call Fast-Boot mode. By > > > > > > > > Can you name this something different. We already have something called > > > > fastboot in-tree (the Android-derived protocol) and there's a Microsoft > > > > technology called fastboot (some kind of hibernation). "OS Boot" isn't > > > > very specific either, since we (almost always) boot an OS. Maybe "Eagle > > > > mode" by analogy to Falcon mode, which lets SPL directly boot an OS. > > > > > > I think fast boot is a behavior which shall be interpreted widely but > > > not proprietary. > > > Or maybe I can rename it as RISC-V Fast Boot to distinguish them. > > > > > > > > > > > (Is this substantially different from falcon mode anyway?) > > > > > > Please see the explanations to Tom. > > > > > > > > > > > > enabling SPL_OPENSBI_OS_BOOT, it will generate linux.itb instead > > > > > of default u-boot.itb after compiling. It initializes memory with > > > > > the U-Boot SPL at the first stage, just like what a regular booting > > > > > process (i.e. Normal Boot) does in the beginning. Instead of jumping > > > > > to the U-Boot proper from OpenSBI before booting Linux Kernel, the > > > > > Fast Boot process jumps directly to Linux Kernel to gain shorter > > > > > booting time. > > > > > > > > > > Signed-off-by: Rick Chen > > > > > --- > > > > > common/spl/Kconfig | 14 ++ > > > > > common/spl/spl_fit.c | 3 ++- > > > > > common/spl/spl_opensbi.c | 25 - > > > > > 3 files changed, 28 insertions(+), 14 deletions(-) > > > > > > > > > > diff --git a/common/spl/Kconfig b/common/spl/Kconfig > > > > > index 05181bdba3..8805aba1b7 100644 > > > > > --- a/common/spl/Kconfig > > > > > +++ b/common/spl/Kconfig > > > > > @@ -1509,6 +1509,20 @@ config SPL_OPENSBI_SCRATCH_OPTIONS > > > > > Options passed to fw_dynamic, for example > > > > > SBI_SCRATCH_NO_BOOT_PRINTS or > > > > > SBI_SCRATCH_DEBUG_PRINTS. > > > > > > > > > > +config SPL_OPENSBI_OS_BOOT > > > > > > > > Please use the same name for the config as for the description. > > > > > > > > > + bool "openSBI Fast Boot" > > > > > + depends on SPL_OPENSBI > > > > > + help > > > > > + Enable this openSBI can jump to Linux Kernel directly. > > > > > > > > Can you put some of the explanation from the commit message here? > > > > > > OK, I will move some messages here from commit messages. > > > > > > > > > > > > + > > > > > +config SPL_OPENSBI_FIT_NAME > > > > > + string "SPL openSBI fit image name" > > > > > + depends on SPL_OPENSBI > > > > > + default "linux.itb" if SPL_OPENSBI_OS_BOOT > > > > > + default "u-boot.itb" > > > > > + help > > > > > + This will help to generate different fit name accordingly. > > > > > > > > Why not SPL_FS_LOAD_PAYLOAD_NAME? > > > > > > > > It looks like the code changes below do not use these configs. Can you > > > > move them to the next patch so it is clearer that they are for binman? > > > > > > I have saw this config, but it does't support SPL_RAM but only for > > > filesystem > > > That is why I don't leverage it. > > > > > > I can prepare a patch as below if no other concerns: > > > > > > config SPL_FS_LOAD_PAYLOAD_NAME > > > string "File to load for U-Boot from the filesystem" > > > depends on SPL_FS_EXT4 || SPL_FS_FAT || SPL_FS_SQUASHFS || SPL_RAM > > > default "tispl.bin" if SYS_K3_SPL_ATF > > > default "u-boot.itb" if SPL_LOAD_FIT > > > default "linux.itb" if SPL_OPENSBI_OS_BOOT > > > default "u-boot.img" > > > help > > > Filename to read to load U-Boot when reading from filesystem. > > > > But in the case you have it's for the output name, and not at all about > > what's being loaded from a filesystem yes? This shouldn't be a "SPL_.." > > For SPL_RAM purpose, > it is just an output name to distinguish normal boot (u-boot.itb) and > fast boot (linux.itb) indeed. > > But for SPL_OPENSBI_OS_BOOT && SPL_MMC && SPL_FAT this combination, > it can help to load linux.itb automatically from SD card. What part of the world is looking for the file named by SPL_FS_LOAD_PAYLOAD_NAME ? (and this is related to what I just sent out, asking about the boot flow) > > option, but should be under arch/riscv/Kconfig somewhere, since it's > > changing the general riscv binman.dtsi file. And on a related note, > > I will fall back on the earlier changes and move > SPL_OPENSBI_OS_BOOT and SPL_OPENSBI_FIT_NAME to arch/riscv/Kconfig. > > > please make sure that 'make htmldocs' is happy with your documentation > > changes as I think I see some spacing problems in 4/4. > > Sorry about the spacing problems. I will check it. OK, thanks. -- Tom
Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
On Tue, Dec 13, 2022 at 08:42:47AM +0800, Rick Chen wrote: > Hi Sean, > > > On 12/12/22 10:03, Tom Rini wrote: > > > On Mon, Dec 12, 2022 at 02:45:10PM +0800, Rick Chen wrote: > > >> Hi Tom > > >> > > >>> On Fri, Dec 09, 2022 at 08:48:37AM -0500, Sean Anderson wrote: > > On 12/7/22 01:23, Rick Chen wrote: > > > In RISC-V, it only provide normal mode booting currently. > > > To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT > > > to achieve this feature which will be call Fast-Boot mode. By > > > > Can you name this something different. We already have something called > > fastboot in-tree (the Android-derived protocol) and there's a Microsoft > > technology called fastboot (some kind of hibernation). "OS Boot" isn't > > very specific either, since we (almost always) boot an OS. Maybe "Eagle > > mode" by analogy to Falcon mode, which lets SPL directly boot an OS. > > > > (Is this substantially different from falcon mode anyway?) > > >>> > > >>> I was kind of wondering if this is different, really, from Falcon Mode. > > >>> Falcon Mode didn't initially have to factor in other-firmware as that's > > >>> not a hard requirement on arm32 like it is on arm64 or risc-v. But my > > >>> first read of this was that it seems like the RISC-V specific side of > > >>> doing Falcon Mode and dealing with the prior stage needs correctly. > > >>> > > >> > > >> Yes. It is a little bit different from the Falcon mode (SPL_OS_BOOT=y). > > >> When I try to enable SPL_OS_BOOT, it will encounter that > > >> SYS_SPL_ARGS_ADDR and > > >> jump_to_image_linux() shall be defined but they are un-necessary for > > >> RISC-V. > > >> Because the flow of OpenSBI and SPL_OS_BOOT are totally different code > > >> flow in board_init_r() of common/spl/spl.c. > > >> That is why I added a new symbol called SPL_OPENSBI_OS_BOOT for this > > >> RISC-V fast boot implementation. > > > > > > Those sound like fairly minor challenges for the same fundamental > > > concept. We have SYS_SPL_ARGS_ADDR for "where is the device tree to > > > pass along". We might need to do a little code re-factoring here. But > > > maybe also a little bit of explaining why we wouldn't be booting to the > > > OS directly but instead passing back to openSBI to do this? That's not > > > normally how RISC-V boots the OS, right? Or am I miss-understanding > > > something here? > > > > > > > The usual process is > > > > ROM -> SPL -> SBI -> U-Boot -> Linux > > In RISC-V, Linux runs in S-mode, it needs the OpenSBI which plays as > M-mode to deal with SBI calls at run-time. > So the fast boot idea, I just want to bypass U-Boot and jump to Linux > directly and Linux still needs OpenSBI. > Hence SPL_OPENSBI can't be disabled here. So is the flow here: a) ROM -> SPL -> SBI -> SPL -> Linux or b) ROM -> SPL -> SBI -> Linux It's not clear to me, and I think that'll help figure out what's best here. Since I kinda think we're looking at a more generic issue that we see with newer architectures and maybe we already had to solve this (but didn't name things well enough) for aarch64. -- Tom signature.asc Description: PGP signature
Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
> On Mon, Dec 12, 2022 at 03:49:10PM +0800, Rick Chen wrote: > > > On 12/7/22 01:23, Rick Chen wrote: > > > > In RISC-V, it only provide normal mode booting currently. > > > > To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT > > > > to achieve this feature which will be call Fast-Boot mode. By > > > > > > Can you name this something different. We already have something called > > > fastboot in-tree (the Android-derived protocol) and there's a Microsoft > > > technology called fastboot (some kind of hibernation). "OS Boot" isn't > > > very specific either, since we (almost always) boot an OS. Maybe "Eagle > > > mode" by analogy to Falcon mode, which lets SPL directly boot an OS. > > > > I think fast boot is a behavior which shall be interpreted widely but > > not proprietary. > > Or maybe I can rename it as RISC-V Fast Boot to distinguish them. > > > > > > > > (Is this substantially different from falcon mode anyway?) > > > > Please see the explanations to Tom. > > > > > > > > > enabling SPL_OPENSBI_OS_BOOT, it will generate linux.itb instead > > > > of default u-boot.itb after compiling. It initializes memory with > > > > the U-Boot SPL at the first stage, just like what a regular booting > > > > process (i.e. Normal Boot) does in the beginning. Instead of jumping > > > > to the U-Boot proper from OpenSBI before booting Linux Kernel, the > > > > Fast Boot process jumps directly to Linux Kernel to gain shorter > > > > booting time. > > > > > > > > Signed-off-by: Rick Chen > > > > --- > > > > common/spl/Kconfig | 14 ++ > > > > common/spl/spl_fit.c | 3 ++- > > > > common/spl/spl_opensbi.c | 25 - > > > > 3 files changed, 28 insertions(+), 14 deletions(-) > > > > > > > > diff --git a/common/spl/Kconfig b/common/spl/Kconfig > > > > index 05181bdba3..8805aba1b7 100644 > > > > --- a/common/spl/Kconfig > > > > +++ b/common/spl/Kconfig > > > > @@ -1509,6 +1509,20 @@ config SPL_OPENSBI_SCRATCH_OPTIONS > > > > Options passed to fw_dynamic, for example > > > > SBI_SCRATCH_NO_BOOT_PRINTS or > > > > SBI_SCRATCH_DEBUG_PRINTS. > > > > > > > > +config SPL_OPENSBI_OS_BOOT > > > > > > Please use the same name for the config as for the description. > > > > > > > + bool "openSBI Fast Boot" > > > > + depends on SPL_OPENSBI > > > > + help > > > > + Enable this openSBI can jump to Linux Kernel directly. > > > > > > Can you put some of the explanation from the commit message here? > > > > OK, I will move some messages here from commit messages. > > > > > > > > > + > > > > +config SPL_OPENSBI_FIT_NAME > > > > + string "SPL openSBI fit image name" > > > > + depends on SPL_OPENSBI > > > > + default "linux.itb" if SPL_OPENSBI_OS_BOOT > > > > + default "u-boot.itb" > > > > + help > > > > + This will help to generate different fit name accordingly. > > > > > > Why not SPL_FS_LOAD_PAYLOAD_NAME? > > > > > > It looks like the code changes below do not use these configs. Can you > > > move them to the next patch so it is clearer that they are for binman? > > > > I have saw this config, but it does't support SPL_RAM but only for > > filesystem > > That is why I don't leverage it. > > > > I can prepare a patch as below if no other concerns: > > > > config SPL_FS_LOAD_PAYLOAD_NAME > > string "File to load for U-Boot from the filesystem" > > depends on SPL_FS_EXT4 || SPL_FS_FAT || SPL_FS_SQUASHFS || SPL_RAM > > default "tispl.bin" if SYS_K3_SPL_ATF > > default "u-boot.itb" if SPL_LOAD_FIT > > default "linux.itb" if SPL_OPENSBI_OS_BOOT > > default "u-boot.img" > > help > > Filename to read to load U-Boot when reading from filesystem. > > But in the case you have it's for the output name, and not at all about > what's being loaded from a filesystem yes? This shouldn't be a "SPL_.." For SPL_RAM purpose, it is just an output name to distinguish normal boot (u-boot.itb) and fast boot (linux.itb) indeed. But for SPL_OPENSBI_OS_BOOT && SPL_MMC && SPL_FAT this combination, it can help to load linux.itb automatically from SD card. > option, but should be under arch/riscv/Kconfig somewhere, since it's > changing the general riscv binman.dtsi file. And on a related note, I will fall back on the earlier changes and move SPL_OPENSBI_OS_BOOT and SPL_OPENSBI_FIT_NAME to arch/riscv/Kconfig. > please make sure that 'make htmldocs' is happy with your documentation > changes as I think I see some spacing problems in 4/4. Sorry about the spacing problems. I will check it. Thanks, Rick > > -- > Tom
Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
Hi Tom > On Mon, Dec 12, 2022 at 02:45:10PM +0800, Rick Chen wrote: > > Hi Tom > > > > > On Fri, Dec 09, 2022 at 08:48:37AM -0500, Sean Anderson wrote: > > > > On 12/7/22 01:23, Rick Chen wrote: > > > > > In RISC-V, it only provide normal mode booting currently. > > > > > To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT > > > > > to achieve this feature which will be call Fast-Boot mode. By > > > > > > > > Can you name this something different. We already have something called > > > > fastboot in-tree (the Android-derived protocol) and there's a Microsoft > > > > technology called fastboot (some kind of hibernation). "OS Boot" isn't > > > > very specific either, since we (almost always) boot an OS. Maybe "Eagle > > > > mode" by analogy to Falcon mode, which lets SPL directly boot an OS. > > > > > > > > (Is this substantially different from falcon mode anyway?) > > > > > > I was kind of wondering if this is different, really, from Falcon Mode. > > > Falcon Mode didn't initially have to factor in other-firmware as that's > > > not a hard requirement on arm32 like it is on arm64 or risc-v. But my > > > first read of this was that it seems like the RISC-V specific side of > > > doing Falcon Mode and dealing with the prior stage needs correctly. > > > > > > > Yes. It is a little bit different from the Falcon mode (SPL_OS_BOOT=y). > > When I try to enable SPL_OS_BOOT, it will encounter that SYS_SPL_ARGS_ADDR > > and > > jump_to_image_linux() shall be defined but they are un-necessary for > > RISC-V. > > Because the flow of OpenSBI and SPL_OS_BOOT are totally different code > > flow in board_init_r() of common/spl/spl.c. > > That is why I added a new symbol called SPL_OPENSBI_OS_BOOT for this > > RISC-V fast boot implementation. > > Those sound like fairly minor challenges for the same fundamental > concept. We have SYS_SPL_ARGS_ADDR for "where is the device tree to > pass along". We might need to do a little code re-factoring here. But > maybe also a little bit of explaining why we wouldn't be booting to the > OS directly but instead passing back to openSBI to do this? That's not > normally how RISC-V boots the OS, right? Or am I miss-understanding > something here? Because RISC-V Linux runs in S-Mode, it needs OpenSBI which runs in M-mode to handle SBI calls. So it can not boot Linux (OS) directly from U-Boot SPL but passing back to openSBI to do this. Rick > > -- > Tom
Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
Hi Sean, > On 12/12/22 10:03, Tom Rini wrote: > > On Mon, Dec 12, 2022 at 02:45:10PM +0800, Rick Chen wrote: > >> Hi Tom > >> > >>> On Fri, Dec 09, 2022 at 08:48:37AM -0500, Sean Anderson wrote: > On 12/7/22 01:23, Rick Chen wrote: > > In RISC-V, it only provide normal mode booting currently. > > To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT > > to achieve this feature which will be call Fast-Boot mode. By > > Can you name this something different. We already have something called > fastboot in-tree (the Android-derived protocol) and there's a Microsoft > technology called fastboot (some kind of hibernation). "OS Boot" isn't > very specific either, since we (almost always) boot an OS. Maybe "Eagle > mode" by analogy to Falcon mode, which lets SPL directly boot an OS. > > (Is this substantially different from falcon mode anyway?) > >>> > >>> I was kind of wondering if this is different, really, from Falcon Mode. > >>> Falcon Mode didn't initially have to factor in other-firmware as that's > >>> not a hard requirement on arm32 like it is on arm64 or risc-v. But my > >>> first read of this was that it seems like the RISC-V specific side of > >>> doing Falcon Mode and dealing with the prior stage needs correctly. > >>> > >> > >> Yes. It is a little bit different from the Falcon mode (SPL_OS_BOOT=y). > >> When I try to enable SPL_OS_BOOT, it will encounter that SYS_SPL_ARGS_ADDR > >> and > >> jump_to_image_linux() shall be defined but they are un-necessary for > >> RISC-V. > >> Because the flow of OpenSBI and SPL_OS_BOOT are totally different code > >> flow in board_init_r() of common/spl/spl.c. > >> That is why I added a new symbol called SPL_OPENSBI_OS_BOOT for this > >> RISC-V fast boot implementation. > > > > Those sound like fairly minor challenges for the same fundamental > > concept. We have SYS_SPL_ARGS_ADDR for "where is the device tree to > > pass along". We might need to do a little code re-factoring here. But > > maybe also a little bit of explaining why we wouldn't be booting to the > > OS directly but instead passing back to openSBI to do this? That's not > > normally how RISC-V boots the OS, right? Or am I miss-understanding > > something here? > > > > The usual process is > > ROM -> SPL -> SBI -> U-Boot -> Linux In RISC-V, Linux runs in S-mode, it needs the OpenSBI which plays as M-mode to deal with SBI calls at run-time. So the fast boot idea, I just want to bypass U-Boot and jump to Linux directly and Linux still needs OpenSBI. Hence SPL_OPENSBI can't be disabled here. Rick > > Where SPL loads SBI and U-Boot and tells SBI "please run U-Boot after you are > done booting". But the idea here is to load Linux instead of U-Boot. I think > this is pretty similar to Falcon mode. Actually, when CONFIG_SPL_OPENSBI is > enabled, I think it's reasonable for falcon mode to do exactly that. People > who want the SPL -> Linux path can disable SPL_OPENSBI. > > --Sean
Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
On 12/12/22 10:03, Tom Rini wrote: On Mon, Dec 12, 2022 at 02:45:10PM +0800, Rick Chen wrote: Hi Tom On Fri, Dec 09, 2022 at 08:48:37AM -0500, Sean Anderson wrote: On 12/7/22 01:23, Rick Chen wrote: In RISC-V, it only provide normal mode booting currently. To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT to achieve this feature which will be call Fast-Boot mode. By Can you name this something different. We already have something called fastboot in-tree (the Android-derived protocol) and there's a Microsoft technology called fastboot (some kind of hibernation). "OS Boot" isn't very specific either, since we (almost always) boot an OS. Maybe "Eagle mode" by analogy to Falcon mode, which lets SPL directly boot an OS. (Is this substantially different from falcon mode anyway?) I was kind of wondering if this is different, really, from Falcon Mode. Falcon Mode didn't initially have to factor in other-firmware as that's not a hard requirement on arm32 like it is on arm64 or risc-v. But my first read of this was that it seems like the RISC-V specific side of doing Falcon Mode and dealing with the prior stage needs correctly. Yes. It is a little bit different from the Falcon mode (SPL_OS_BOOT=y). When I try to enable SPL_OS_BOOT, it will encounter that SYS_SPL_ARGS_ADDR and jump_to_image_linux() shall be defined but they are un-necessary for RISC-V. Because the flow of OpenSBI and SPL_OS_BOOT are totally different code flow in board_init_r() of common/spl/spl.c. That is why I added a new symbol called SPL_OPENSBI_OS_BOOT for this RISC-V fast boot implementation. Those sound like fairly minor challenges for the same fundamental concept. We have SYS_SPL_ARGS_ADDR for "where is the device tree to pass along". We might need to do a little code re-factoring here. But maybe also a little bit of explaining why we wouldn't be booting to the OS directly but instead passing back to openSBI to do this? That's not normally how RISC-V boots the OS, right? Or am I miss-understanding something here? The usual process is ROM -> SPL -> SBI -> U-Boot -> Linux Where SPL loads SBI and U-Boot and tells SBI "please run U-Boot after you are done booting". But the idea here is to load Linux instead of U-Boot. I think this is pretty similar to Falcon mode. Actually, when CONFIG_SPL_OPENSBI is enabled, I think it's reasonable for falcon mode to do exactly that. People who want the SPL -> Linux path can disable SPL_OPENSBI. --Sean
Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
On Mon, Dec 12, 2022 at 03:49:10PM +0800, Rick Chen wrote: > > On 12/7/22 01:23, Rick Chen wrote: > > > In RISC-V, it only provide normal mode booting currently. > > > To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT > > > to achieve this feature which will be call Fast-Boot mode. By > > > > Can you name this something different. We already have something called > > fastboot in-tree (the Android-derived protocol) and there's a Microsoft > > technology called fastboot (some kind of hibernation). "OS Boot" isn't > > very specific either, since we (almost always) boot an OS. Maybe "Eagle > > mode" by analogy to Falcon mode, which lets SPL directly boot an OS. > > I think fast boot is a behavior which shall be interpreted widely but > not proprietary. > Or maybe I can rename it as RISC-V Fast Boot to distinguish them. > > > > > (Is this substantially different from falcon mode anyway?) > > Please see the explanations to Tom. > > > > > > enabling SPL_OPENSBI_OS_BOOT, it will generate linux.itb instead > > > of default u-boot.itb after compiling. It initializes memory with > > > the U-Boot SPL at the first stage, just like what a regular booting > > > process (i.e. Normal Boot) does in the beginning. Instead of jumping > > > to the U-Boot proper from OpenSBI before booting Linux Kernel, the > > > Fast Boot process jumps directly to Linux Kernel to gain shorter > > > booting time. > > > > > > Signed-off-by: Rick Chen > > > --- > > > common/spl/Kconfig | 14 ++ > > > common/spl/spl_fit.c | 3 ++- > > > common/spl/spl_opensbi.c | 25 - > > > 3 files changed, 28 insertions(+), 14 deletions(-) > > > > > > diff --git a/common/spl/Kconfig b/common/spl/Kconfig > > > index 05181bdba3..8805aba1b7 100644 > > > --- a/common/spl/Kconfig > > > +++ b/common/spl/Kconfig > > > @@ -1509,6 +1509,20 @@ config SPL_OPENSBI_SCRATCH_OPTIONS > > > Options passed to fw_dynamic, for example > > > SBI_SCRATCH_NO_BOOT_PRINTS or > > > SBI_SCRATCH_DEBUG_PRINTS. > > > > > > +config SPL_OPENSBI_OS_BOOT > > > > Please use the same name for the config as for the description. > > > > > + bool "openSBI Fast Boot" > > > + depends on SPL_OPENSBI > > > + help > > > + Enable this openSBI can jump to Linux Kernel directly. > > > > Can you put some of the explanation from the commit message here? > > OK, I will move some messages here from commit messages. > > > > > > + > > > +config SPL_OPENSBI_FIT_NAME > > > + string "SPL openSBI fit image name" > > > + depends on SPL_OPENSBI > > > + default "linux.itb" if SPL_OPENSBI_OS_BOOT > > > + default "u-boot.itb" > > > + help > > > + This will help to generate different fit name accordingly. > > > > Why not SPL_FS_LOAD_PAYLOAD_NAME? > > > > It looks like the code changes below do not use these configs. Can you > > move them to the next patch so it is clearer that they are for binman? > > I have saw this config, but it does't support SPL_RAM but only for filesystem > That is why I don't leverage it. > > I can prepare a patch as below if no other concerns: > > config SPL_FS_LOAD_PAYLOAD_NAME > string "File to load for U-Boot from the filesystem" > depends on SPL_FS_EXT4 || SPL_FS_FAT || SPL_FS_SQUASHFS || SPL_RAM > default "tispl.bin" if SYS_K3_SPL_ATF > default "u-boot.itb" if SPL_LOAD_FIT > default "linux.itb" if SPL_OPENSBI_OS_BOOT > default "u-boot.img" > help > Filename to read to load U-Boot when reading from filesystem. But in the case you have it's for the output name, and not at all about what's being loaded from a filesystem yes? This shouldn't be a "SPL_.." option, but should be under arch/riscv/Kconfig somewhere, since it's changing the general riscv binman.dtsi file. And on a related note, please make sure that 'make htmldocs' is happy with your documentation changes as I think I see some spacing problems in 4/4. -- Tom signature.asc Description: PGP signature
Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
On Mon, Dec 12, 2022 at 02:45:10PM +0800, Rick Chen wrote: > Hi Tom > > > On Fri, Dec 09, 2022 at 08:48:37AM -0500, Sean Anderson wrote: > > > On 12/7/22 01:23, Rick Chen wrote: > > > > In RISC-V, it only provide normal mode booting currently. > > > > To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT > > > > to achieve this feature which will be call Fast-Boot mode. By > > > > > > Can you name this something different. We already have something called > > > fastboot in-tree (the Android-derived protocol) and there's a Microsoft > > > technology called fastboot (some kind of hibernation). "OS Boot" isn't > > > very specific either, since we (almost always) boot an OS. Maybe "Eagle > > > mode" by analogy to Falcon mode, which lets SPL directly boot an OS. > > > > > > (Is this substantially different from falcon mode anyway?) > > > > I was kind of wondering if this is different, really, from Falcon Mode. > > Falcon Mode didn't initially have to factor in other-firmware as that's > > not a hard requirement on arm32 like it is on arm64 or risc-v. But my > > first read of this was that it seems like the RISC-V specific side of > > doing Falcon Mode and dealing with the prior stage needs correctly. > > > > Yes. It is a little bit different from the Falcon mode (SPL_OS_BOOT=y). > When I try to enable SPL_OS_BOOT, it will encounter that SYS_SPL_ARGS_ADDR and > jump_to_image_linux() shall be defined but they are un-necessary for RISC-V. > Because the flow of OpenSBI and SPL_OS_BOOT are totally different code > flow in board_init_r() of common/spl/spl.c. > That is why I added a new symbol called SPL_OPENSBI_OS_BOOT for this > RISC-V fast boot implementation. Those sound like fairly minor challenges for the same fundamental concept. We have SYS_SPL_ARGS_ADDR for "where is the device tree to pass along". We might need to do a little code re-factoring here. But maybe also a little bit of explaining why we wouldn't be booting to the OS directly but instead passing back to openSBI to do this? That's not normally how RISC-V boots the OS, right? Or am I miss-understanding something here? -- Tom signature.asc Description: PGP signature
Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
> On 12/7/22 01:23, Rick Chen wrote:
> > In RISC-V, it only provide normal mode booting currently.
> > To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT
> > to achieve this feature which will be call Fast-Boot mode. By
>
> Can you name this something different. We already have something called
> fastboot in-tree (the Android-derived protocol) and there's a Microsoft
> technology called fastboot (some kind of hibernation). "OS Boot" isn't
> very specific either, since we (almost always) boot an OS. Maybe "Eagle
> mode" by analogy to Falcon mode, which lets SPL directly boot an OS.
I think fast boot is a behavior which shall be interpreted widely but
not proprietary.
Or maybe I can rename it as RISC-V Fast Boot to distinguish them.
>
> (Is this substantially different from falcon mode anyway?)
Please see the explanations to Tom.
>
> > enabling SPL_OPENSBI_OS_BOOT, it will generate linux.itb instead
> > of default u-boot.itb after compiling. It initializes memory with
> > the U-Boot SPL at the first stage, just like what a regular booting
> > process (i.e. Normal Boot) does in the beginning. Instead of jumping
> > to the U-Boot proper from OpenSBI before booting Linux Kernel, the
> > Fast Boot process jumps directly to Linux Kernel to gain shorter
> > booting time.
> >
> > Signed-off-by: Rick Chen
> > ---
> > common/spl/Kconfig | 14 ++
> > common/spl/spl_fit.c | 3 ++-
> > common/spl/spl_opensbi.c | 25 -
> > 3 files changed, 28 insertions(+), 14 deletions(-)
> >
> > diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> > index 05181bdba3..8805aba1b7 100644
> > --- a/common/spl/Kconfig
> > +++ b/common/spl/Kconfig
> > @@ -1509,6 +1509,20 @@ config SPL_OPENSBI_SCRATCH_OPTIONS
> > Options passed to fw_dynamic, for example
> > SBI_SCRATCH_NO_BOOT_PRINTS or
> > SBI_SCRATCH_DEBUG_PRINTS.
> >
> > +config SPL_OPENSBI_OS_BOOT
>
> Please use the same name for the config as for the description.
>
> > + bool "openSBI Fast Boot"
> > + depends on SPL_OPENSBI
> > + help
> > + Enable this openSBI can jump to Linux Kernel directly.
>
> Can you put some of the explanation from the commit message here?
OK, I will move some messages here from commit messages.
>
> > +
> > +config SPL_OPENSBI_FIT_NAME
> > + string "SPL openSBI fit image name"
> > + depends on SPL_OPENSBI
> > + default "linux.itb" if SPL_OPENSBI_OS_BOOT
> > + default "u-boot.itb"
> > + help
> > + This will help to generate different fit name accordingly.
>
> Why not SPL_FS_LOAD_PAYLOAD_NAME?
>
> It looks like the code changes below do not use these configs. Can you
> move them to the next patch so it is clearer that they are for binman?
I have saw this config, but it does't support SPL_RAM but only for filesystem
That is why I don't leverage it.
I can prepare a patch as below if no other concerns:
config SPL_FS_LOAD_PAYLOAD_NAME
string "File to load for U-Boot from the filesystem"
depends on SPL_FS_EXT4 || SPL_FS_FAT || SPL_FS_SQUASHFS || SPL_RAM
default "tispl.bin" if SYS_K3_SPL_ATF
default "u-boot.itb" if SPL_LOAD_FIT
default "linux.itb" if SPL_OPENSBI_OS_BOOT
default "u-boot.img"
help
Filename to read to load U-Boot when reading from filesystem.
Thanks,
Rick
>
> --Sean
>
> > config SPL_TARGET
> > string "Addtional build targets for 'make'"
> > default "spl/u-boot-spl.srec" if RCAR_GEN2
> > diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> > index c1ed31e367..c5b1dfb3ba 100644
> > --- a/common/spl/spl_fit.c
> > +++ b/common/spl/spl_fit.c
> > @@ -363,7 +363,8 @@ static bool os_takes_devicetree(uint8_t os)
> > case IH_OS_U_BOOT:
> > return true;
> > case IH_OS_LINUX:
> > - return IS_ENABLED(CONFIG_SPL_OS_BOOT);
> > + return IS_ENABLED(CONFIG_SPL_OS_BOOT) ||
> > + IS_ENABLED(CONFIG_SPL_OPENSBI_OS_BOOT);
> > default:
> > return false;
> > }
> > diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c
> > index b0f40076c3..83869c6b18 100644
> > --- a/common/spl/spl_opensbi.c
> > +++ b/common/spl/spl_opensbi.c
> > @@ -20,7 +20,7 @@ DECLARE_GLOBAL_DATA_PTR;
> >
> > struct fw_dynamic_info opensbi_info;
> >
> > -static int spl_opensbi_find_uboot_node(void *blob, int *uboot_node)
> > +static int spl_opensbi_find_os_node(void *blob, int *os_node)
> > {
> > int fit_images_node, node;
> > const char *fit_os;
> > @@ -34,10 +34,9 @@ static int spl_opensbi_find_uboot_node(void *blob, int
> > *uboot_node)
> > if (!fit_os)
> > continue;
> >
> > - if (genimg_get_os_id(fit_os) == IH_OS_U_BOOT) {
> > - *uboot_node = node;
> > - return 0;
> > - }
> > + *os_node = node;
> > +
> > + return 0;
> > }
> >
> > return -ENODEV;
> > @@ -45,8 +44,8 @@ static int
Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
Hi Tom > On Fri, Dec 09, 2022 at 08:48:37AM -0500, Sean Anderson wrote: > > On 12/7/22 01:23, Rick Chen wrote: > > > In RISC-V, it only provide normal mode booting currently. > > > To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT > > > to achieve this feature which will be call Fast-Boot mode. By > > > > Can you name this something different. We already have something called > > fastboot in-tree (the Android-derived protocol) and there's a Microsoft > > technology called fastboot (some kind of hibernation). "OS Boot" isn't > > very specific either, since we (almost always) boot an OS. Maybe "Eagle > > mode" by analogy to Falcon mode, which lets SPL directly boot an OS. > > > > (Is this substantially different from falcon mode anyway?) > > I was kind of wondering if this is different, really, from Falcon Mode. > Falcon Mode didn't initially have to factor in other-firmware as that's > not a hard requirement on arm32 like it is on arm64 or risc-v. But my > first read of this was that it seems like the RISC-V specific side of > doing Falcon Mode and dealing with the prior stage needs correctly. > Yes. It is a little bit different from the Falcon mode (SPL_OS_BOOT=y). When I try to enable SPL_OS_BOOT, it will encounter that SYS_SPL_ARGS_ADDR and jump_to_image_linux() shall be defined but they are un-necessary for RISC-V. Because the flow of OpenSBI and SPL_OS_BOOT are totally different code flow in board_init_r() of common/spl/spl.c. That is why I added a new symbol called SPL_OPENSBI_OS_BOOT for this RISC-V fast boot implementation. Thanks, Rick > -- > Tom
Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
On Fri, Dec 09, 2022 at 08:48:37AM -0500, Sean Anderson wrote: > On 12/7/22 01:23, Rick Chen wrote: > > In RISC-V, it only provide normal mode booting currently. > > To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT > > to achieve this feature which will be call Fast-Boot mode. By > > Can you name this something different. We already have something called > fastboot in-tree (the Android-derived protocol) and there's a Microsoft > technology called fastboot (some kind of hibernation). "OS Boot" isn't > very specific either, since we (almost always) boot an OS. Maybe "Eagle > mode" by analogy to Falcon mode, which lets SPL directly boot an OS. > > (Is this substantially different from falcon mode anyway?) I was kind of wondering if this is different, really, from Falcon Mode. Falcon Mode didn't initially have to factor in other-firmware as that's not a hard requirement on arm32 like it is on arm64 or risc-v. But my first read of this was that it seems like the RISC-V specific side of doing Falcon Mode and dealing with the prior stage needs correctly. -- Tom signature.asc Description: PGP signature
Re: [PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
On 12/7/22 01:23, Rick Chen wrote:
In RISC-V, it only provide normal mode booting currently.
To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT
to achieve this feature which will be call Fast-Boot mode. By
Can you name this something different. We already have something called
fastboot in-tree (the Android-derived protocol) and there's a Microsoft
technology called fastboot (some kind of hibernation). "OS Boot" isn't
very specific either, since we (almost always) boot an OS. Maybe "Eagle
mode" by analogy to Falcon mode, which lets SPL directly boot an OS.
(Is this substantially different from falcon mode anyway?)
enabling SPL_OPENSBI_OS_BOOT, it will generate linux.itb instead
of default u-boot.itb after compiling. It initializes memory with
the U-Boot SPL at the first stage, just like what a regular booting
process (i.e. Normal Boot) does in the beginning. Instead of jumping
to the U-Boot proper from OpenSBI before booting Linux Kernel, the
Fast Boot process jumps directly to Linux Kernel to gain shorter
booting time.
Signed-off-by: Rick Chen
---
common/spl/Kconfig | 14 ++
common/spl/spl_fit.c | 3 ++-
common/spl/spl_opensbi.c | 25 -
3 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 05181bdba3..8805aba1b7 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -1509,6 +1509,20 @@ config SPL_OPENSBI_SCRATCH_OPTIONS
Options passed to fw_dynamic, for example SBI_SCRATCH_NO_BOOT_PRINTS
or
SBI_SCRATCH_DEBUG_PRINTS.
+config SPL_OPENSBI_OS_BOOT
Please use the same name for the config as for the description.
+ bool "openSBI Fast Boot"
+ depends on SPL_OPENSBI
+ help
+ Enable this openSBI can jump to Linux Kernel directly.
Can you put some of the explanation from the commit message here?
+
+config SPL_OPENSBI_FIT_NAME
+ string "SPL openSBI fit image name"
+ depends on SPL_OPENSBI
+ default "linux.itb" if SPL_OPENSBI_OS_BOOT
+ default "u-boot.itb"
+ help
+ This will help to generate different fit name accordingly.
Why not SPL_FS_LOAD_PAYLOAD_NAME?
It looks like the code changes below do not use these configs. Can you
move them to the next patch so it is clearer that they are for binman?
--Sean
config SPL_TARGET
string "Addtional build targets for 'make'"
default "spl/u-boot-spl.srec" if RCAR_GEN2
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index c1ed31e367..c5b1dfb3ba 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -363,7 +363,8 @@ static bool os_takes_devicetree(uint8_t os)
case IH_OS_U_BOOT:
return true;
case IH_OS_LINUX:
- return IS_ENABLED(CONFIG_SPL_OS_BOOT);
+ return IS_ENABLED(CONFIG_SPL_OS_BOOT) ||
+ IS_ENABLED(CONFIG_SPL_OPENSBI_OS_BOOT);
default:
return false;
}
diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c
index b0f40076c3..83869c6b18 100644
--- a/common/spl/spl_opensbi.c
+++ b/common/spl/spl_opensbi.c
@@ -20,7 +20,7 @@ DECLARE_GLOBAL_DATA_PTR;
struct fw_dynamic_info opensbi_info;
-static int spl_opensbi_find_uboot_node(void *blob, int *uboot_node)
+static int spl_opensbi_find_os_node(void *blob, int *os_node)
{
int fit_images_node, node;
const char *fit_os;
@@ -34,10 +34,9 @@ static int spl_opensbi_find_uboot_node(void *blob, int
*uboot_node)
if (!fit_os)
continue;
- if (genimg_get_os_id(fit_os) == IH_OS_U_BOOT) {
- *uboot_node = node;
- return 0;
- }
+ *os_node = node;
+
+ return 0;
}
return -ENODEV;
@@ -45,8 +44,8 @@ static int spl_opensbi_find_uboot_node(void *blob, int
*uboot_node)
void spl_invoke_opensbi(struct spl_image_info *spl_image)
{
- int ret, uboot_node;
- ulong uboot_entry;
+ int ret, os_node;
+ ulong os_entry;
void (*opensbi_entry)(ulong hartid, ulong dtb, ulong info);
if (!spl_image->fdt_addr) {
@@ -54,22 +53,22 @@ void spl_invoke_opensbi(struct spl_image_info *spl_image)
hang();
}
- /* Find U-Boot image in /fit-images */
- ret = spl_opensbi_find_uboot_node(spl_image->fdt_addr, _node);
+ /* Find U-Boot or Linux image in /fit-images */
+ ret = spl_opensbi_find_os_node(spl_image->fdt_addr, _node);
if (ret) {
pr_err("Can't find U-Boot node, %d\n", ret);
hang();
}
- /* Get U-Boot entry point */
- ret = fit_image_get_entry(spl_image->fdt_addr, uboot_node,
_entry);
+ /* Get os entry point */
+ ret = fit_image_get_entry(spl_image->fdt_addr, os_node, _entry);
if (ret)
- ret = fit_image_get_load(spl_image->fdt_addr,
[PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
In RISC-V, it only provide normal mode booting currently.
To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT
to achieve this feature which will be call Fast-Boot mode. By
enabling SPL_OPENSBI_OS_BOOT, it will generate linux.itb instead
of default u-boot.itb after compiling. It initializes memory with
the U-Boot SPL at the first stage, just like what a regular booting
process (i.e. Normal Boot) does in the beginning. Instead of jumping
to the U-Boot proper from OpenSBI before booting Linux Kernel, the
Fast Boot process jumps directly to Linux Kernel to gain shorter
booting time.
Signed-off-by: Rick Chen
---
common/spl/Kconfig | 14 ++
common/spl/spl_fit.c | 3 ++-
common/spl/spl_opensbi.c | 25 -
3 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 05181bdba3..8805aba1b7 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -1509,6 +1509,20 @@ config SPL_OPENSBI_SCRATCH_OPTIONS
Options passed to fw_dynamic, for example SBI_SCRATCH_NO_BOOT_PRINTS
or
SBI_SCRATCH_DEBUG_PRINTS.
+config SPL_OPENSBI_OS_BOOT
+ bool "openSBI Fast Boot"
+ depends on SPL_OPENSBI
+ help
+ Enable this openSBI can jump to Linux Kernel directly.
+
+config SPL_OPENSBI_FIT_NAME
+ string "SPL openSBI fit image name"
+ depends on SPL_OPENSBI
+ default "linux.itb" if SPL_OPENSBI_OS_BOOT
+ default "u-boot.itb"
+ help
+ This will help to generate different fit name accordingly.
+
config SPL_TARGET
string "Addtional build targets for 'make'"
default "spl/u-boot-spl.srec" if RCAR_GEN2
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index c1ed31e367..c5b1dfb3ba 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -363,7 +363,8 @@ static bool os_takes_devicetree(uint8_t os)
case IH_OS_U_BOOT:
return true;
case IH_OS_LINUX:
- return IS_ENABLED(CONFIG_SPL_OS_BOOT);
+ return IS_ENABLED(CONFIG_SPL_OS_BOOT) ||
+ IS_ENABLED(CONFIG_SPL_OPENSBI_OS_BOOT);
default:
return false;
}
diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c
index b0f40076c3..83869c6b18 100644
--- a/common/spl/spl_opensbi.c
+++ b/common/spl/spl_opensbi.c
@@ -20,7 +20,7 @@ DECLARE_GLOBAL_DATA_PTR;
struct fw_dynamic_info opensbi_info;
-static int spl_opensbi_find_uboot_node(void *blob, int *uboot_node)
+static int spl_opensbi_find_os_node(void *blob, int *os_node)
{
int fit_images_node, node;
const char *fit_os;
@@ -34,10 +34,9 @@ static int spl_opensbi_find_uboot_node(void *blob, int
*uboot_node)
if (!fit_os)
continue;
- if (genimg_get_os_id(fit_os) == IH_OS_U_BOOT) {
- *uboot_node = node;
- return 0;
- }
+ *os_node = node;
+
+ return 0;
}
return -ENODEV;
@@ -45,8 +44,8 @@ static int spl_opensbi_find_uboot_node(void *blob, int
*uboot_node)
void spl_invoke_opensbi(struct spl_image_info *spl_image)
{
- int ret, uboot_node;
- ulong uboot_entry;
+ int ret, os_node;
+ ulong os_entry;
void (*opensbi_entry)(ulong hartid, ulong dtb, ulong info);
if (!spl_image->fdt_addr) {
@@ -54,22 +53,22 @@ void spl_invoke_opensbi(struct spl_image_info *spl_image)
hang();
}
- /* Find U-Boot image in /fit-images */
- ret = spl_opensbi_find_uboot_node(spl_image->fdt_addr, _node);
+ /* Find U-Boot or Linux image in /fit-images */
+ ret = spl_opensbi_find_os_node(spl_image->fdt_addr, _node);
if (ret) {
pr_err("Can't find U-Boot node, %d\n", ret);
hang();
}
- /* Get U-Boot entry point */
- ret = fit_image_get_entry(spl_image->fdt_addr, uboot_node,
_entry);
+ /* Get os entry point */
+ ret = fit_image_get_entry(spl_image->fdt_addr, os_node, _entry);
if (ret)
- ret = fit_image_get_load(spl_image->fdt_addr, uboot_node,
_entry);
+ ret = fit_image_get_load(spl_image->fdt_addr, os_node,
_entry);
/* Prepare opensbi_info object */
opensbi_info.magic = FW_DYNAMIC_INFO_MAGIC_VALUE;
opensbi_info.version = FW_DYNAMIC_INFO_VERSION;
- opensbi_info.next_addr = uboot_entry;
+ opensbi_info.next_addr = os_entry;
opensbi_info.next_mode = FW_DYNAMIC_INFO_NEXT_MODE_S;
opensbi_info.options = CONFIG_SPL_OPENSBI_SCRATCH_OPTIONS;
opensbi_info.boot_hart = gd->arch.boot_hart;
--
2.17.1
