| Message ID | 20200905132211.412711-1-seanga2@gmail.com |
|---|---|
| State | Accepted |
| Commit | 32cef69da0cbd8d07dcd5b5bec66d2dc94e22be9 |
| Delegated to: | Andes |
| Headers | show |
| Series | riscv: Only enable OF_BOARD_FIXUP for S-Mode | expand |
On 9/5/20 3:22 PM, Sean Anderson wrote: > It is unsafe to enable OF_BOARD_FIXUP only based on OF_SEPARATE. > OF_SEPARATE may indicate that the user wishes U-Boot to use a different > device tree than one obtained via OF_PRIOR_STAGE. However, OF_SEPARATE may > also indicate that the device tree which would be obtained via > OF_PRIOR_STAGE is invalid, nonexistant, or otherwise unusable. In this > latter case, enabling OF_BOARD_FIXUP will result in corruption of the > device tree. To remedy this, only enable OF_BOARD_FIXUP if U-Boot is > configured for S-Mode. > > Fixes: 1c17e55594a394ced7de88d91be294eaf8c564c1 > Signed-off-by: Sean Anderson <seanga2@gmail.com> > --- > > arch/riscv/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 009a545fcf..13fac51483 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -288,6 +288,6 @@ config STACK_SIZE_SHIFT > default 14 > > config OF_BOARD_FIXUP > - default y if OF_SEPARATE > + default y if OF_SEPARATE && RISCV_SMODE OF_BOARD_FIXUP is also defined in dts/Kconfig. Moving the "default" line to dts/Kconfig would simplify the code. Why do we need the dependency on OF_SEPARATE? Isn't RISCV_SMODE enough? Best regards Heinrich > > endmenu >
On 9/6/20 7:18 AM, Heinrich Schuchardt wrote: > On 9/5/20 3:22 PM, Sean Anderson wrote: >> It is unsafe to enable OF_BOARD_FIXUP only based on OF_SEPARATE. >> OF_SEPARATE may indicate that the user wishes U-Boot to use a different >> device tree than one obtained via OF_PRIOR_STAGE. However, OF_SEPARATE may >> also indicate that the device tree which would be obtained via >> OF_PRIOR_STAGE is invalid, nonexistant, or otherwise unusable. In this >> latter case, enabling OF_BOARD_FIXUP will result in corruption of the >> device tree. To remedy this, only enable OF_BOARD_FIXUP if U-Boot is >> configured for S-Mode. >> >> Fixes: 1c17e55594a394ced7de88d91be294eaf8c564c1 >> Signed-off-by: Sean Anderson <seanga2@gmail.com> >> --- >> >> arch/riscv/Kconfig | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >> index 009a545fcf..13fac51483 100644 >> --- a/arch/riscv/Kconfig >> +++ b/arch/riscv/Kconfig >> @@ -288,6 +288,6 @@ config STACK_SIZE_SHIFT >> default 14 >> >> config OF_BOARD_FIXUP >> - default y if OF_SEPARATE >> + default y if OF_SEPARATE && RISCV_SMODE > > > OF_BOARD_FIXUP is also defined in dts/Kconfig. > Moving the "default" line to dts/Kconfig would simplify the code. Probably. > > Why do we need the dependency on OF_SEPARATE? Isn't RISCV_SMODE enough? Because there is no need to do this fixup if we are using sbi's device tree. From the commit referenced above: > Starting from OpenSBI v0.7, the SBI firmware inserts/fixes up the > reserved memory node for PMP protected memory regions. All RISC-V > boards need to copy the reserved memory node from the device tree > provided by the firmware to the device tree used by U-Boot. --Sean
Hi Sean, On Sat, Sep 5, 2020 at 9:22 PM Sean Anderson <seanga2@gmail.com> wrote: > > It is unsafe to enable OF_BOARD_FIXUP only based on OF_SEPARATE. > OF_SEPARATE may indicate that the user wishes U-Boot to use a different > device tree than one obtained via OF_PRIOR_STAGE. However, OF_SEPARATE may > also indicate that the device tree which would be obtained via > OF_PRIOR_STAGE is invalid, nonexistant, or otherwise unusable. In this typo: nonexistent > latter case, enabling OF_BOARD_FIXUP will result in corruption of the > device tree. To remedy this, only enable OF_BOARD_FIXUP if U-Boot is > configured for S-Mode. > > Fixes: 1c17e55594a394ced7de88d91be294eaf8c564c1 nits: the format should be: commit_id ("description") > Signed-off-by: Sean Anderson <seanga2@gmail.com> > --- > > arch/riscv/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 009a545fcf..13fac51483 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -288,6 +288,6 @@ config STACK_SIZE_SHIFT > default 14 > > config OF_BOARD_FIXUP > - default y if OF_SEPARATE > + default y if OF_SEPARATE && RISCV_SMODE Is that your board is running U-Boot M-mode with OF_SEPARATE that does not work? > > endmenu > -- Regards, Bin
On 9/11/20 3:29 AM, Bin Meng wrote: > Hi Sean, > > On Sat, Sep 5, 2020 at 9:22 PM Sean Anderson <seanga2@gmail.com> wrote: >> >> It is unsafe to enable OF_BOARD_FIXUP only based on OF_SEPARATE. >> OF_SEPARATE may indicate that the user wishes U-Boot to use a different >> device tree than one obtained via OF_PRIOR_STAGE. However, OF_SEPARATE may >> also indicate that the device tree which would be obtained via >> OF_PRIOR_STAGE is invalid, nonexistant, or otherwise unusable. In this > > typo: nonexistent > >> latter case, enabling OF_BOARD_FIXUP will result in corruption of the >> device tree. To remedy this, only enable OF_BOARD_FIXUP if U-Boot is >> configured for S-Mode. >> >> Fixes: 1c17e55594a394ced7de88d91be294eaf8c564c1 > > nits: the format should be: commit_id ("description")> >> Signed-off-by: Sean Anderson <seanga2@gmail.com> >> --- >> >> arch/riscv/Kconfig | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >> index 009a545fcf..13fac51483 100644 >> --- a/arch/riscv/Kconfig >> +++ b/arch/riscv/Kconfig >> @@ -288,6 +288,6 @@ config STACK_SIZE_SHIFT >> default 14 >> >> config OF_BOARD_FIXUP >> - default y if OF_SEPARATE >> + default y if OF_SEPARATE && RISCV_SMODE > > Is that your board is running U-Boot M-mode with OF_SEPARATE that does not work? Yes, because the reason we use OF_SEPARATE is because no device tree is passed to U-Boot. Trying to use the device tree passed to U-Boot even though OF_SEPARATE is enabled results in garbage being written to the actual device tree. Without this patch, booting on the K210 randomly fails. > >> >> endmenu >> -- > > Regards, > Bin >
On Fri, Sep 11, 2020 at 6:20 PM Sean Anderson <seanga2@gmail.com> wrote: > > On 9/11/20 3:29 AM, Bin Meng wrote: > > Hi Sean, > > > > On Sat, Sep 5, 2020 at 9:22 PM Sean Anderson <seanga2@gmail.com> wrote: > >> > >> It is unsafe to enable OF_BOARD_FIXUP only based on OF_SEPARATE. > >> OF_SEPARATE may indicate that the user wishes U-Boot to use a different > >> device tree than one obtained via OF_PRIOR_STAGE. However, OF_SEPARATE may > >> also indicate that the device tree which would be obtained via > >> OF_PRIOR_STAGE is invalid, nonexistant, or otherwise unusable. In this > > > > typo: nonexistent > > > >> latter case, enabling OF_BOARD_FIXUP will result in corruption of the > >> device tree. To remedy this, only enable OF_BOARD_FIXUP if U-Boot is > >> configured for S-Mode. > >> > >> Fixes: 1c17e55594a394ced7de88d91be294eaf8c564c1 > > > > nits: the format should be: commit_id ("description")> > >> Signed-off-by: Sean Anderson <seanga2@gmail.com> > >> --- > >> > >> arch/riscv/Kconfig | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > >> index 009a545fcf..13fac51483 100644 > >> --- a/arch/riscv/Kconfig > >> +++ b/arch/riscv/Kconfig > >> @@ -288,6 +288,6 @@ config STACK_SIZE_SHIFT > >> default 14 > >> > >> config OF_BOARD_FIXUP > >> - default y if OF_SEPARATE > >> + default y if OF_SEPARATE && RISCV_SMODE > > > > Is that your board is running U-Boot M-mode with OF_SEPARATE that does not work? > > Yes, because the reason we use OF_SEPARATE is because no device tree is > passed to U-Boot. Trying to use the device tree passed to U-Boot even I don't get it. If no device tree is passed to U-Boot, why using OF_SEPARATE in the first place? > though OF_SEPARATE is enabled results in garbage being written to the What garbage data is written? > actual device tree. Without this patch, booting on the K210 randomly > fails. Regards, Bin
On 9/11/20 10:43 AM, Bin Meng wrote: > On Fri, Sep 11, 2020 at 6:20 PM Sean Anderson <seanga2@gmail.com> wrote: >> >> On 9/11/20 3:29 AM, Bin Meng wrote: >>> Hi Sean, >>> >>> On Sat, Sep 5, 2020 at 9:22 PM Sean Anderson <seanga2@gmail.com> wrote: >>>> >>>> It is unsafe to enable OF_BOARD_FIXUP only based on OF_SEPARATE. >>>> OF_SEPARATE may indicate that the user wishes U-Boot to use a different >>>> device tree than one obtained via OF_PRIOR_STAGE. However, OF_SEPARATE may >>>> also indicate that the device tree which would be obtained via >>>> OF_PRIOR_STAGE is invalid, nonexistant, or otherwise unusable. In this >>> >>> typo: nonexistent >>> >>>> latter case, enabling OF_BOARD_FIXUP will result in corruption of the >>>> device tree. To remedy this, only enable OF_BOARD_FIXUP if U-Boot is >>>> configured for S-Mode. >>>> >>>> Fixes: 1c17e55594a394ced7de88d91be294eaf8c564c1 >>> >>> nits: the format should be: commit_id ("description")> >>>> Signed-off-by: Sean Anderson <seanga2@gmail.com> >>>> --- >>>> >>>> arch/riscv/Kconfig | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >>>> index 009a545fcf..13fac51483 100644 >>>> --- a/arch/riscv/Kconfig >>>> +++ b/arch/riscv/Kconfig >>>> @@ -288,6 +288,6 @@ config STACK_SIZE_SHIFT >>>> default 14 >>>> >>>> config OF_BOARD_FIXUP >>>> - default y if OF_SEPARATE >>>> + default y if OF_SEPARATE && RISCV_SMODE >>> >>> Is that your board is running U-Boot M-mode with OF_SEPARATE that does not work? >> >> Yes, because the reason we use OF_SEPARATE is because no device tree is >> passed to U-Boot. Trying to use the device tree passed to U-Boot even > > I don't get it. If no device tree is passed to U-Boot, why using > OF_SEPARATE in the first place? Because it has to come from somewhere. Where else would U-Boot get the device tree? >> though OF_SEPARATE is enabled results in garbage being written to the > > What garbage data is written? It might not be garbage written. I didn't document the exact failure mode at the time I discovered this bug, so I went back to try and reproduce it for a more thorough analysis. However, I was unable to reproduce this bug, even on the branch where I originally triggered it. I documented my reasoning behind this patch at [1]. In my testing, I could only trigger a "periodic-32" bug. In any case, this behavior could still cause problems in the future. From my testing, on the k210, a1 usually holds some address on the ROM's stack. However, if it (for instance) instead held an address which raised a load access fault, or was misaligned, then booting would fail. In the general case, I was very surpised that U-Boot was using the value of a1 on entry even with OF_SEPARATE specified. I would expect it only to use that value if configured with OF_PRIOR_STAGE. --Sean [1] https://patchwork.ozlabs.org/project/uboot/patch/20200815155237.467720-12-seanga2@gmail.com/#2520514
On Sat, Sep 12, 2020 at 2:25 AM Sean Anderson <seanga2@gmail.com> wrote: > > On 9/11/20 10:43 AM, Bin Meng wrote: > > On Fri, Sep 11, 2020 at 6:20 PM Sean Anderson <seanga2@gmail.com> wrote: > >> > >> On 9/11/20 3:29 AM, Bin Meng wrote: > >>> Hi Sean, > >>> > >>> On Sat, Sep 5, 2020 at 9:22 PM Sean Anderson <seanga2@gmail.com> wrote: > >>>> > >>>> It is unsafe to enable OF_BOARD_FIXUP only based on OF_SEPARATE. > >>>> OF_SEPARATE may indicate that the user wishes U-Boot to use a different > >>>> device tree than one obtained via OF_PRIOR_STAGE. However, OF_SEPARATE may > >>>> also indicate that the device tree which would be obtained via > >>>> OF_PRIOR_STAGE is invalid, nonexistant, or otherwise unusable. In this > >>> > >>> typo: nonexistent > >>> > >>>> latter case, enabling OF_BOARD_FIXUP will result in corruption of the > >>>> device tree. To remedy this, only enable OF_BOARD_FIXUP if U-Boot is > >>>> configured for S-Mode. > >>>> > >>>> Fixes: 1c17e55594a394ced7de88d91be294eaf8c564c1 > >>> > >>> nits: the format should be: commit_id ("description")> > >>>> Signed-off-by: Sean Anderson <seanga2@gmail.com> > >>>> --- > >>>> > >>>> arch/riscv/Kconfig | 2 +- > >>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>> > >>>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > >>>> index 009a545fcf..13fac51483 100644 > >>>> --- a/arch/riscv/Kconfig > >>>> +++ b/arch/riscv/Kconfig > >>>> @@ -288,6 +288,6 @@ config STACK_SIZE_SHIFT > >>>> default 14 > >>>> > >>>> config OF_BOARD_FIXUP > >>>> - default y if OF_SEPARATE > >>>> + default y if OF_SEPARATE && RISCV_SMODE > >>> > >>> Is that your board is running U-Boot M-mode with OF_SEPARATE that does not work? > >> > >> Yes, because the reason we use OF_SEPARATE is because no device tree is > >> passed to U-Boot. Trying to use the device tree passed to U-Boot even > > > > I don't get it. If no device tree is passed to U-Boot, why using > > OF_SEPARATE in the first place? > > Because it has to come from somewhere. Where else would U-Boot get the > device tree? Sounds like there was some misunderstanding on "passed to U-Boot" .. But I got it now. > > >> though OF_SEPARATE is enabled results in garbage being written to the > > > > What garbage data is written? > > It might not be garbage written. I didn't document the exact failure > mode at the time I discovered this bug, so I went back to try and > reproduce it for a more thorough analysis. However, I was unable to > reproduce this bug, even on the branch where I originally triggered it. > I documented my reasoning behind this patch at [1]. In my testing, I > could only trigger a "periodic-32" bug. > > In any case, this behavior could still cause problems in the future. > From my testing, on the k210, a1 usually holds some address on the ROM's > stack. However, if it (for instance) instead held an address which So U-Boot on K210 boots with M-mode from the K210 ROM, and the ROM code does not hold DTB address in a1 before jumping to U-Boot, right? > raised a load access fault, or was misaligned, then booting would fail. > In the general case, I was very surpised that U-Boot was using the value > of a1 on entry even with OF_SEPARATE specified. I would expect it only > to use that value if configured with OF_PRIOR_STAGE. Because U-Boot S-mode needs to fix up the DT when OF_SEPERATE is used. > > --Sean > > [1] https://patchwork.ozlabs.org/project/uboot/patch/20200815155237.467720-12-seanga2@gmail.com/#2520514 Regards, Bin
On 9/14/20 2:38 AM, Bin Meng wrote: > On Sat, Sep 12, 2020 at 2:25 AM Sean Anderson <seanga2@gmail.com> wrote: >> >> On 9/11/20 10:43 AM, Bin Meng wrote: >>> On Fri, Sep 11, 2020 at 6:20 PM Sean Anderson <seanga2@gmail.com> wrote: >>>> >>>> On 9/11/20 3:29 AM, Bin Meng wrote: >>>>> Hi Sean, >>>>> >>>>> On Sat, Sep 5, 2020 at 9:22 PM Sean Anderson <seanga2@gmail.com> wrote: >>>>>> >>>>>> It is unsafe to enable OF_BOARD_FIXUP only based on OF_SEPARATE. >>>>>> OF_SEPARATE may indicate that the user wishes U-Boot to use a different >>>>>> device tree than one obtained via OF_PRIOR_STAGE. However, OF_SEPARATE may >>>>>> also indicate that the device tree which would be obtained via >>>>>> OF_PRIOR_STAGE is invalid, nonexistant, or otherwise unusable. In this >>>>> >>>>> typo: nonexistent >>>>> >>>>>> latter case, enabling OF_BOARD_FIXUP will result in corruption of the >>>>>> device tree. To remedy this, only enable OF_BOARD_FIXUP if U-Boot is >>>>>> configured for S-Mode. >>>>>> >>>>>> Fixes: 1c17e55594a394ced7de88d91be294eaf8c564c1 >>>>> >>>>> nits: the format should be: commit_id ("description")> >>>>>> Signed-off-by: Sean Anderson <seanga2@gmail.com> >>>>>> --- >>>>>> >>>>>> arch/riscv/Kconfig | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >>>>>> index 009a545fcf..13fac51483 100644 >>>>>> --- a/arch/riscv/Kconfig >>>>>> +++ b/arch/riscv/Kconfig >>>>>> @@ -288,6 +288,6 @@ config STACK_SIZE_SHIFT >>>>>> default 14 >>>>>> >>>>>> config OF_BOARD_FIXUP >>>>>> - default y if OF_SEPARATE >>>>>> + default y if OF_SEPARATE && RISCV_SMODE >>>>> >>>>> Is that your board is running U-Boot M-mode with OF_SEPARATE that does not work? >>>> >>>> Yes, because the reason we use OF_SEPARATE is because no device tree is >>>> passed to U-Boot. Trying to use the device tree passed to U-Boot even >>> >>> I don't get it. If no device tree is passed to U-Boot, why using >>> OF_SEPARATE in the first place? >> >> Because it has to come from somewhere. Where else would U-Boot get the >> device tree? > > Sounds like there was some misunderstanding on "passed to U-Boot" .. > But I got it now. > >> >>>> though OF_SEPARATE is enabled results in garbage being written to the >>> >>> What garbage data is written? >> >> It might not be garbage written. I didn't document the exact failure >> mode at the time I discovered this bug, so I went back to try and >> reproduce it for a more thorough analysis. However, I was unable to >> reproduce this bug, even on the branch where I originally triggered it. >> I documented my reasoning behind this patch at [1]. In my testing, I >> could only trigger a "periodic-32" bug. >> >> In any case, this behavior could still cause problems in the future. >> From my testing, on the k210, a1 usually holds some address on the ROM's >> stack. However, if it (for instance) instead held an address which > > So U-Boot on K210 boots with M-mode from the K210 ROM, and the ROM > code does not hold DTB address in a1 before jumping to U-Boot, right? > >> raised a load access fault, or was misaligned, then booting would fail. >> In the general case, I was very surpised that U-Boot was using the value >> of a1 on entry even with OF_SEPARATE specified. I would expect it only >> to use that value if configured with OF_PRIOR_STAGE. > > Because U-Boot S-mode needs to fix up the DT when OF_SEPERATE is used. Right. It's just unexpected because OF_SEPARATE appears to imply to both use a separate device tree and to not use the passed-in device tree. This is because it is mutually exclusive with OF_PRIOR_STAGE. However, with OF_BOARD_FIXUP, it's as if one has selected both OF_SEPARATE and OF_PRIOR_STAGE at once. I think defaulting OF_BOARD_FIXUP to y only S-Mode is more likely to result in unsurprising behavior on new boards. --Sean
On Mon, Sep 14, 2020 at 07:57:04AM -0400, Sean Anderson wrote: > On 9/14/20 2:38 AM, Bin Meng wrote: > > On Sat, Sep 12, 2020 at 2:25 AM Sean Anderson <seanga2@gmail.com> wrote: > >> > >> On 9/11/20 10:43 AM, Bin Meng wrote: > >>> On Fri, Sep 11, 2020 at 6:20 PM Sean Anderson <seanga2@gmail.com> wrote: > >>>> > >>>> On 9/11/20 3:29 AM, Bin Meng wrote: > >>>>> Hi Sean, > >>>>> > >>>>> On Sat, Sep 5, 2020 at 9:22 PM Sean Anderson <seanga2@gmail.com> wrote: > >>>>>> > >>>>>> It is unsafe to enable OF_BOARD_FIXUP only based on OF_SEPARATE. > >>>>>> OF_SEPARATE may indicate that the user wishes U-Boot to use a different > >>>>>> device tree than one obtained via OF_PRIOR_STAGE. However, OF_SEPARATE may > >>>>>> also indicate that the device tree which would be obtained via > >>>>>> OF_PRIOR_STAGE is invalid, nonexistant, or otherwise unusable. In this > >>>>> > >>>>> typo: nonexistent > >>>>> > >>>>>> latter case, enabling OF_BOARD_FIXUP will result in corruption of the > >>>>>> device tree. To remedy this, only enable OF_BOARD_FIXUP if U-Boot is > >>>>>> configured for S-Mode. > >>>>>> > >>>>>> Fixes: 1c17e55594a394ced7de88d91be294eaf8c564c1 > >>>>> > >>>>> nits: the format should be: commit_id ("description")> > >>>>>> Signed-off-by: Sean Anderson <seanga2@gmail.com> > >>>>>> --- > >>>>>> > >>>>>> arch/riscv/Kconfig | 2 +- > >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>>> > >>>>>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > >>>>>> index 009a545fcf..13fac51483 100644 > >>>>>> --- a/arch/riscv/Kconfig > >>>>>> +++ b/arch/riscv/Kconfig > >>>>>> @@ -288,6 +288,6 @@ config STACK_SIZE_SHIFT > >>>>>> default 14 > >>>>>> > >>>>>> config OF_BOARD_FIXUP > >>>>>> - default y if OF_SEPARATE > >>>>>> + default y if OF_SEPARATE && RISCV_SMODE > >>>>> > >>>>> Is that your board is running U-Boot M-mode with OF_SEPARATE that does not work? > >>>> > >>>> Yes, because the reason we use OF_SEPARATE is because no device tree is > >>>> passed to U-Boot. Trying to use the device tree passed to U-Boot even > >>> > >>> I don't get it. If no device tree is passed to U-Boot, why using > >>> OF_SEPARATE in the first place? > >> > >> Because it has to come from somewhere. Where else would U-Boot get the > >> device tree? > > > > Sounds like there was some misunderstanding on "passed to U-Boot" .. > > But I got it now. > > > >> > >>>> though OF_SEPARATE is enabled results in garbage being written to the > >>> > >>> What garbage data is written? > >> > >> It might not be garbage written. I didn't document the exact failure > >> mode at the time I discovered this bug, so I went back to try and > >> reproduce it for a more thorough analysis. However, I was unable to > >> reproduce this bug, even on the branch where I originally triggered it. > >> I documented my reasoning behind this patch at [1]. In my testing, I > >> could only trigger a "periodic-32" bug. > >> > >> In any case, this behavior could still cause problems in the future. > >> From my testing, on the k210, a1 usually holds some address on the ROM's > >> stack. However, if it (for instance) instead held an address which > > > > So U-Boot on K210 boots with M-mode from the K210 ROM, and the ROM > > code does not hold DTB address in a1 before jumping to U-Boot, right? > > > >> raised a load access fault, or was misaligned, then booting would fail. > >> In the general case, I was very surpised that U-Boot was using the value > >> of a1 on entry even with OF_SEPARATE specified. I would expect it only > >> to use that value if configured with OF_PRIOR_STAGE. > > > > Because U-Boot S-mode needs to fix up the DT when OF_SEPERATE is used. > > Right. It's just unexpected because OF_SEPARATE appears to imply to both > use a separate device tree and to not use the passed-in device tree. > This is because it is mutually exclusive with OF_PRIOR_STAGE. However, > with OF_BOARD_FIXUP, it's as if one has selected both OF_SEPARATE and > OF_PRIOR_STAGE at once. I think defaulting OF_BOARD_FIXUP to y only > S-Mode is more likely to result in unsurprising behavior on new boards. > > --Sean Reviewed-by: Leo Liang <ycliang@andestech.com>
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 009a545fcf..13fac51483 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -288,6 +288,6 @@ config STACK_SIZE_SHIFT default 14 config OF_BOARD_FIXUP - default y if OF_SEPARATE + default y if OF_SEPARATE && RISCV_SMODE endmenu
It is unsafe to enable OF_BOARD_FIXUP only based on OF_SEPARATE. OF_SEPARATE may indicate that the user wishes U-Boot to use a different device tree than one obtained via OF_PRIOR_STAGE. However, OF_SEPARATE may also indicate that the device tree which would be obtained via OF_PRIOR_STAGE is invalid, nonexistant, or otherwise unusable. In this latter case, enabling OF_BOARD_FIXUP will result in corruption of the device tree. To remedy this, only enable OF_BOARD_FIXUP if U-Boot is configured for S-Mode. Fixes: 1c17e55594a394ced7de88d91be294eaf8c564c1 Signed-off-by: Sean Anderson <seanga2@gmail.com> --- arch/riscv/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)