| Message ID | 20230329081932.79831-3-alexghiti@rivosinc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f1581626071c8e37c58c5e8f0b4126b17172a211 |
| Headers | show |
| Series | Fixes for dtb mapping | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Series has a cover letter |
| conchuod/tree_selection | success | Guessed tree name to be fixes at HEAD e89c2e815e76 |
| conchuod/fixes_present | success | Fixes tag present in non-next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 1 and now 1 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 18 this patch: 18 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 18 this patch: 18 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 3 this patch: 3 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 11 lines checked |
| conchuod/source_inline | success | Was 0 now: 0 |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
On Wed, Mar 29, 2023 at 10:19:31AM +0200, Alexandre Ghiti wrote: > early_init_dt_verify() is already called in parse_dtb() and since the dtb > address does not change anymore (it is now in the fixmap region), no need > to reset initial_boot_params by calling early_init_dt_verify() again. > > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > --- > arch/riscv/kernel/setup.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > index 542eed85ad2c..a059b73f4ddb 100644 > --- a/arch/riscv/kernel/setup.c > +++ b/arch/riscv/kernel/setup.c > @@ -278,10 +278,7 @@ void __init setup_arch(char **cmdline_p) > #if IS_ENABLED(CONFIG_BUILTIN_DTB) > unflatten_and_copy_device_tree(); > #else > - if (early_init_dt_verify(__va(XIP_FIXUP(dtb_early_pa)))) > - unflatten_device_tree(); Silly question maybe, but since it isn't explicitly mentioned, the XIP_FIXUP bits no longer matter? Also, in related news, I assume you don't have a QEMU setup that can do boot an XIP kernel? Cheers, Conor. > - else > - pr_err("No DTB found in kernel mappings\n"); > + unflatten_device_tree(); > #endif > misc_mem_init(); > > -- > 2.37.2 >
On Wed, Mar 29, 2023 at 4:37 PM Conor Dooley <conor@kernel.org> wrote: > > On Wed, Mar 29, 2023 at 10:19:31AM +0200, Alexandre Ghiti wrote: > > early_init_dt_verify() is already called in parse_dtb() and since the dtb > > address does not change anymore (it is now in the fixmap region), no need > > to reset initial_boot_params by calling early_init_dt_verify() again. > > > > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > > --- > > arch/riscv/kernel/setup.c | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > > index 542eed85ad2c..a059b73f4ddb 100644 > > --- a/arch/riscv/kernel/setup.c > > +++ b/arch/riscv/kernel/setup.c > > @@ -278,10 +278,7 @@ void __init setup_arch(char **cmdline_p) > > #if IS_ENABLED(CONFIG_BUILTIN_DTB) > > unflatten_and_copy_device_tree(); > > #else > > - if (early_init_dt_verify(__va(XIP_FIXUP(dtb_early_pa)))) > > - unflatten_device_tree(); > > Silly question maybe, but since it isn't explicitly mentioned, the > XIP_FIXUP bits no longer matter? The XIP_FIXUP is only needed when translating virtual to physical addresses, but that does not mean I did not break it, I haven't considered XIP at all... > Also, in related news, I assume you don't have a QEMU setup that can do > boot an XIP kernel? I haven't booted a XIP kernel for a long time now, here are my notes from that time: https://github.com/AlexGhiti/alexghiti.github.io/blob/main/xip/XIP.md > > Cheers, > Conor. > > > - else > > - pr_err("No DTB found in kernel mappings\n"); > > + unflatten_device_tree(); > > #endif > > misc_mem_init(); > > > > -- > > 2.37.2 > >
On Wed, Mar 29, 2023 at 04:52:45PM +0200, Alexandre Ghiti wrote: > On Wed, Mar 29, 2023 at 4:37 PM Conor Dooley <conor@kernel.org> wrote: > > > > On Wed, Mar 29, 2023 at 10:19:31AM +0200, Alexandre Ghiti wrote: > > > early_init_dt_verify() is already called in parse_dtb() and since the dtb > > > address does not change anymore (it is now in the fixmap region), no need > > > to reset initial_boot_params by calling early_init_dt_verify() again. > > > > > > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > > > --- > > > arch/riscv/kernel/setup.c | 5 +---- > > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > > > index 542eed85ad2c..a059b73f4ddb 100644 > > > --- a/arch/riscv/kernel/setup.c > > > +++ b/arch/riscv/kernel/setup.c > > > @@ -278,10 +278,7 @@ void __init setup_arch(char **cmdline_p) > > > #if IS_ENABLED(CONFIG_BUILTIN_DTB) > > > unflatten_and_copy_device_tree(); > > > #else > > > - if (early_init_dt_verify(__va(XIP_FIXUP(dtb_early_pa)))) > > > - unflatten_device_tree(); > > > > Silly question maybe, but since it isn't explicitly mentioned, the > > XIP_FIXUP bits no longer matter? > > The XIP_FIXUP is only needed when translating virtual to physical > addresses, but that does not mean I did not break it, I haven't > considered XIP at all... So, what currently happens is that, during early boot, we call parse_dtb() right at the beginning of setup_arch(). That calls early_init_dt_scan(dtb_early_pa), which in turn calls early_init_dt_verify(dtb_early_pa). Here, relatively late during boot, we are coming along and calling the function again. This existed prior to the XIP stuff landing, but the specific XIP_FIXUP handling looks to be the fallout from: https://lore.kernel.org/linux-riscv/82a05081-5662-c787-44e4-d480774ce31c@ghiti.fr/ The check in the first place was added by Anup's move away from fixmap for dtb stuff, which makes me wonder - should this actually be part of 1/3? Something, something we no longer need to do this because these addresses no longer change as per 1/3? > > Also, in related news, I assume you don't have a QEMU setup that can do > > boot an XIP kernel? > > I haven't booted a XIP kernel for a long time now, here are my notes > from that time: > https://github.com/AlexGhiti/alexghiti.github.io/blob/main/xip/XIP.md Right, I'll have to get around to trying that. We never put any investment into QEMU internally, nor run any CI against it, so the HSS doesn't actually work in QEMU anymore. Assertion failures due to missing peripheral emulation :( Probably don't need the HSS though and can do a direct kernel boot, I'll have to see if that works for this XIP stuff, I know it does for regular kernels. Cheers, Conor.
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index 542eed85ad2c..a059b73f4ddb 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -278,10 +278,7 @@ void __init setup_arch(char **cmdline_p) #if IS_ENABLED(CONFIG_BUILTIN_DTB) unflatten_and_copy_device_tree(); #else - if (early_init_dt_verify(__va(XIP_FIXUP(dtb_early_pa)))) - unflatten_device_tree(); - else - pr_err("No DTB found in kernel mappings\n"); + unflatten_device_tree(); #endif misc_mem_init();
early_init_dt_verify() is already called in parse_dtb() and since the dtb address does not change anymore (it is now in the fixmap region), no need to reset initial_boot_params by calling early_init_dt_verify() again. Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> --- arch/riscv/kernel/setup.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)