* [PATCH] imx8m: set sane default value for SPL_LOAD_FIT_ADDRESS @ 2024-10-24 10:01 Rasmus Villemoes 2024-10-24 11:51 ` Marek Vasut 0 siblings, 1 reply; 10+ messages in thread From: Rasmus Villemoes @ 2024-10-24 10:01 UTC (permalink / raw) To: u-boot; +Cc: Marek Vasut, Tim Harvey, Heiko Schocher, Tom Rini, Rasmus Villemoes I enabled IMX_HAB on an imx8mp board, but even though I knew about the implementation, I forgot that I had to provide a sane value for SPL_LOAD_FIT_ADDRESS. The help text for IMX_HAB doesn't mention this implicit requirement, and there's no build-time warning; the default 0x0 value just ends up being returned from board_spl_fit_buffer_addr(), obviously resulting in a non-booting board. The existing imx8m* board configs that set a non-zero value currently all use 0x44000000. The actual value doesn't matter too much, but 0 is always wrong for imx8m platforms. So just use 0x44000000 as default for those platforms. Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> --- boot/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/boot/Kconfig b/boot/Kconfig index 940389d4882..72d1f69afcd 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -231,6 +231,7 @@ config SPL_LOAD_FIT config SPL_LOAD_FIT_ADDRESS hex "load address of fit image" depends on SPL_LOAD_FIT + default 0x44000000 if ARCH_IMX8M default 0x0 help Specify the load address of the fit image that will be loaded -- 2.47.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] imx8m: set sane default value for SPL_LOAD_FIT_ADDRESS 2024-10-24 10:01 [PATCH] imx8m: set sane default value for SPL_LOAD_FIT_ADDRESS Rasmus Villemoes @ 2024-10-24 11:51 ` Marek Vasut 2024-10-24 13:18 ` Rasmus Villemoes 2024-10-26 0:24 ` Fabio Estevam 0 siblings, 2 replies; 10+ messages in thread From: Marek Vasut @ 2024-10-24 11:51 UTC (permalink / raw) To: Rasmus Villemoes, u-boot; +Cc: Tim Harvey, Heiko Schocher, Tom Rini On 10/24/24 12:01 PM, Rasmus Villemoes wrote: > I enabled IMX_HAB on an imx8mp board, but even though I knew about the > implementation, I forgot that I had to provide a sane value for > SPL_LOAD_FIT_ADDRESS. The help text for IMX_HAB doesn't mention this > implicit requirement, and there's no build-time warning; the default > 0x0 value just ends up being returned from > board_spl_fit_buffer_addr(), obviously resulting in a non-booting > board. > > The existing imx8m* board configs that set a non-zero value currently > all use 0x44000000. The actual value doesn't matter too much, but 0 is > always wrong for imx8m platforms. So just use 0x44000000 as default > for those platforms. > > Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> > --- > boot/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/boot/Kconfig b/boot/Kconfig > index 940389d4882..72d1f69afcd 100644 > --- a/boot/Kconfig > +++ b/boot/Kconfig > @@ -231,6 +231,7 @@ config SPL_LOAD_FIT > config SPL_LOAD_FIT_ADDRESS > hex "load address of fit image" > depends on SPL_LOAD_FIT > + default 0x44000000 if ARCH_IMX8M This only applies to HAB , for non-HAB the fitImage can be loaded at arbitrary location, do you need: default 0x44000000 if ARCH_IMX8M && IMX_HAB right ? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] imx8m: set sane default value for SPL_LOAD_FIT_ADDRESS 2024-10-24 11:51 ` Marek Vasut @ 2024-10-24 13:18 ` Rasmus Villemoes 2024-10-24 14:06 ` Marek Vasut 2024-10-26 0:24 ` Fabio Estevam 1 sibling, 1 reply; 10+ messages in thread From: Rasmus Villemoes @ 2024-10-24 13:18 UTC (permalink / raw) To: Marek Vasut; +Cc: u-boot, Tim Harvey, Heiko Schocher, Tom Rini On Thu, Oct 24 2024, Marek Vasut <marex@denx.de> wrote: > On 10/24/24 12:01 PM, Rasmus Villemoes wrote: >> I enabled IMX_HAB on an imx8mp board, but even though I knew about the >> implementation, I forgot that I had to provide a sane value for >> SPL_LOAD_FIT_ADDRESS. The help text for IMX_HAB doesn't mention this >> implicit requirement, and there's no build-time warning; the default >> 0x0 value just ends up being returned from >> board_spl_fit_buffer_addr(), obviously resulting in a non-booting >> board. >> The existing imx8m* board configs that set a non-zero value >> currently >> all use 0x44000000. The actual value doesn't matter too much, but 0 is >> always wrong for imx8m platforms. So just use 0x44000000 as default >> for those platforms. >> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> >> --- >> boot/Kconfig | 1 + >> 1 file changed, 1 insertion(+) >> diff --git a/boot/Kconfig b/boot/Kconfig >> index 940389d4882..72d1f69afcd 100644 >> --- a/boot/Kconfig >> +++ b/boot/Kconfig >> @@ -231,6 +231,7 @@ config SPL_LOAD_FIT >> config SPL_LOAD_FIT_ADDRESS >> hex "load address of fit image" >> depends on SPL_LOAD_FIT >> + default 0x44000000 if ARCH_IMX8M > This only applies to HAB , for non-HAB the fitImage can be loaded at > arbitrary location, do you need: > > default 0x44000000 if ARCH_IMX8M && IMX_HAB > > right ? On IMX8 without HAB, the value is not used at all AFAICT (otherwise the value of 0x0 would have caused trouble already). I don't see the harm of setting some sane value that's actually within DRAM space independent of HAB. Making the default depend on IMX_HAB has the UX problem that if I do "make imx8mp_evk_defconfig", then go about tweaking stuff, then do "make menuconfig" and enable IMX_HAB, SPL_LOAD_FIT_ADDRESS already has the bogus 0x0 value and nothing forces or asks that to be changed. Making the default depend only on SOC (or SOC family or whatever IMX8M is in this context) makes that problem go away. Rasmus ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] imx8m: set sane default value for SPL_LOAD_FIT_ADDRESS 2024-10-24 13:18 ` Rasmus Villemoes @ 2024-10-24 14:06 ` Marek Vasut 2024-10-24 15:20 ` Tom Rini 0 siblings, 1 reply; 10+ messages in thread From: Marek Vasut @ 2024-10-24 14:06 UTC (permalink / raw) To: Rasmus Villemoes; +Cc: u-boot, Tim Harvey, Heiko Schocher, Tom Rini On 10/24/24 3:18 PM, Rasmus Villemoes wrote: > On Thu, Oct 24 2024, Marek Vasut <marex@denx.de> wrote: > >> On 10/24/24 12:01 PM, Rasmus Villemoes wrote: >>> I enabled IMX_HAB on an imx8mp board, but even though I knew about the >>> implementation, I forgot that I had to provide a sane value for >>> SPL_LOAD_FIT_ADDRESS. The help text for IMX_HAB doesn't mention this >>> implicit requirement, and there's no build-time warning; the default >>> 0x0 value just ends up being returned from >>> board_spl_fit_buffer_addr(), obviously resulting in a non-booting >>> board. >>> The existing imx8m* board configs that set a non-zero value >>> currently >>> all use 0x44000000. The actual value doesn't matter too much, but 0 is >>> always wrong for imx8m platforms. So just use 0x44000000 as default >>> for those platforms. >>> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> >>> --- >>> boot/Kconfig | 1 + >>> 1 file changed, 1 insertion(+) >>> diff --git a/boot/Kconfig b/boot/Kconfig >>> index 940389d4882..72d1f69afcd 100644 >>> --- a/boot/Kconfig >>> +++ b/boot/Kconfig >>> @@ -231,6 +231,7 @@ config SPL_LOAD_FIT >>> config SPL_LOAD_FIT_ADDRESS >>> hex "load address of fit image" >>> depends on SPL_LOAD_FIT >>> + default 0x44000000 if ARCH_IMX8M >> This only applies to HAB , for non-HAB the fitImage can be loaded at >> arbitrary location, do you need: >> >> default 0x44000000 if ARCH_IMX8M && IMX_HAB >> >> right ? > > On IMX8 without HAB, the value is not used at all AFAICT (otherwise the > value of 0x0 would have caused trouble already). I don't see the harm of > setting some sane value that's actually within DRAM space independent of > HAB. > > Making the default depend on IMX_HAB has the UX problem that if I do > "make imx8mp_evk_defconfig", then go about tweaking stuff, then do "make > menuconfig" and enable IMX_HAB, SPL_LOAD_FIT_ADDRESS already has the > bogus 0x0 value and nothing forces or asks that to be changed. Making the > default depend only on SOC (or SOC family or whatever IMX8M is in this > context) makes that problem go away. Looking at this closely, common/spl/spl_ram.c does make use of CONFIG_SPL_LOAD_FIT_ADDRESS too, so I think yes, we should have a default for this. iMX6 should have 0x14000000 iMX7 should have 0x84000000 iMX8M should have 0x44000000 Thanks ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] imx8m: set sane default value for SPL_LOAD_FIT_ADDRESS 2024-10-24 14:06 ` Marek Vasut @ 2024-10-24 15:20 ` Tom Rini 2024-10-24 16:13 ` Marek Vasut 0 siblings, 1 reply; 10+ messages in thread From: Tom Rini @ 2024-10-24 15:20 UTC (permalink / raw) To: Marek Vasut; +Cc: Rasmus Villemoes, u-boot, Tim Harvey, Heiko Schocher [-- Attachment #1: Type: text/plain, Size: 2768 bytes --] On Thu, Oct 24, 2024 at 04:06:03PM +0200, Marek Vasut wrote: > On 10/24/24 3:18 PM, Rasmus Villemoes wrote: > > On Thu, Oct 24 2024, Marek Vasut <marex@denx.de> wrote: > > > > > On 10/24/24 12:01 PM, Rasmus Villemoes wrote: > > > > I enabled IMX_HAB on an imx8mp board, but even though I knew about the > > > > implementation, I forgot that I had to provide a sane value for > > > > SPL_LOAD_FIT_ADDRESS. The help text for IMX_HAB doesn't mention this > > > > implicit requirement, and there's no build-time warning; the default > > > > 0x0 value just ends up being returned from > > > > board_spl_fit_buffer_addr(), obviously resulting in a non-booting > > > > board. > > > > The existing imx8m* board configs that set a non-zero value > > > > currently > > > > all use 0x44000000. The actual value doesn't matter too much, but 0 is > > > > always wrong for imx8m platforms. So just use 0x44000000 as default > > > > for those platforms. > > > > Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> > > > > --- > > > > boot/Kconfig | 1 + > > > > 1 file changed, 1 insertion(+) > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > > > index 940389d4882..72d1f69afcd 100644 > > > > --- a/boot/Kconfig > > > > +++ b/boot/Kconfig > > > > @@ -231,6 +231,7 @@ config SPL_LOAD_FIT > > > > config SPL_LOAD_FIT_ADDRESS > > > > hex "load address of fit image" > > > > depends on SPL_LOAD_FIT > > > > + default 0x44000000 if ARCH_IMX8M > > > This only applies to HAB , for non-HAB the fitImage can be loaded at > > > arbitrary location, do you need: > > > > > > default 0x44000000 if ARCH_IMX8M && IMX_HAB > > > > > > right ? > > > > On IMX8 without HAB, the value is not used at all AFAICT (otherwise the > > value of 0x0 would have caused trouble already). I don't see the harm of > > setting some sane value that's actually within DRAM space independent of > > HAB. > > > > Making the default depend on IMX_HAB has the UX problem that if I do > > "make imx8mp_evk_defconfig", then go about tweaking stuff, then do "make > > menuconfig" and enable IMX_HAB, SPL_LOAD_FIT_ADDRESS already has the > > bogus 0x0 value and nothing forces or asks that to be changed. Making the > > default depend only on SOC (or SOC family or whatever IMX8M is in this > > context) makes that problem go away. > Looking at this closely, common/spl/spl_ram.c does make use of > CONFIG_SPL_LOAD_FIT_ADDRESS too, so I think yes, we should have a default > for this. > > iMX6 should have 0x14000000 > iMX7 should have 0x84000000 > iMX8M should have 0x44000000 These differ, slightly, from the value used in CONFIG_SYS_LOAD_ADDR. Could that not be used (and the overall option changed to default SYS_LOAD_ADDR ? -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] imx8m: set sane default value for SPL_LOAD_FIT_ADDRESS 2024-10-24 15:20 ` Tom Rini @ 2024-10-24 16:13 ` Marek Vasut 2024-10-25 8:41 ` Rasmus Villemoes 0 siblings, 1 reply; 10+ messages in thread From: Marek Vasut @ 2024-10-24 16:13 UTC (permalink / raw) To: Tom Rini; +Cc: Rasmus Villemoes, u-boot, Tim Harvey, Heiko Schocher On 10/24/24 5:20 PM, Tom Rini wrote: > On Thu, Oct 24, 2024 at 04:06:03PM +0200, Marek Vasut wrote: >> On 10/24/24 3:18 PM, Rasmus Villemoes wrote: >>> On Thu, Oct 24 2024, Marek Vasut <marex@denx.de> wrote: >>> >>>> On 10/24/24 12:01 PM, Rasmus Villemoes wrote: >>>>> I enabled IMX_HAB on an imx8mp board, but even though I knew about the >>>>> implementation, I forgot that I had to provide a sane value for >>>>> SPL_LOAD_FIT_ADDRESS. The help text for IMX_HAB doesn't mention this >>>>> implicit requirement, and there's no build-time warning; the default >>>>> 0x0 value just ends up being returned from >>>>> board_spl_fit_buffer_addr(), obviously resulting in a non-booting >>>>> board. >>>>> The existing imx8m* board configs that set a non-zero value >>>>> currently >>>>> all use 0x44000000. The actual value doesn't matter too much, but 0 is >>>>> always wrong for imx8m platforms. So just use 0x44000000 as default >>>>> for those platforms. >>>>> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> >>>>> --- >>>>> boot/Kconfig | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> diff --git a/boot/Kconfig b/boot/Kconfig >>>>> index 940389d4882..72d1f69afcd 100644 >>>>> --- a/boot/Kconfig >>>>> +++ b/boot/Kconfig >>>>> @@ -231,6 +231,7 @@ config SPL_LOAD_FIT >>>>> config SPL_LOAD_FIT_ADDRESS >>>>> hex "load address of fit image" >>>>> depends on SPL_LOAD_FIT >>>>> + default 0x44000000 if ARCH_IMX8M >>>> This only applies to HAB , for non-HAB the fitImage can be loaded at >>>> arbitrary location, do you need: >>>> >>>> default 0x44000000 if ARCH_IMX8M && IMX_HAB >>>> >>>> right ? >>> >>> On IMX8 without HAB, the value is not used at all AFAICT (otherwise the >>> value of 0x0 would have caused trouble already). I don't see the harm of >>> setting some sane value that's actually within DRAM space independent of >>> HAB. >>> >>> Making the default depend on IMX_HAB has the UX problem that if I do >>> "make imx8mp_evk_defconfig", then go about tweaking stuff, then do "make >>> menuconfig" and enable IMX_HAB, SPL_LOAD_FIT_ADDRESS already has the >>> bogus 0x0 value and nothing forces or asks that to be changed. Making the >>> default depend only on SOC (or SOC family or whatever IMX8M is in this >>> context) makes that problem go away. >> Looking at this closely, common/spl/spl_ram.c does make use of >> CONFIG_SPL_LOAD_FIT_ADDRESS too, so I think yes, we should have a default >> for this. >> >> iMX6 should have 0x14000000 >> iMX7 should have 0x84000000 >> iMX8M should have 0x44000000 > > These differ, slightly, from the value used in CONFIG_SYS_LOAD_ADDR. > Could that not be used (and the overall option changed to default > SYS_LOAD_ADDR ? If I recall it right, no ... these addresses are where the fitImage is loaded and where HAB does its authentication stuff on the fitImage, that address has to be fixed and well aligned. The SYS_LOAD_ADDR is the destination address where the u-boot-nodtb.bin is copied from the fitImage AFTER the fitImage was authenticated. Maybe Rasmus can correct me here, I might be wrong ... ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] imx8m: set sane default value for SPL_LOAD_FIT_ADDRESS 2024-10-24 16:13 ` Marek Vasut @ 2024-10-25 8:41 ` Rasmus Villemoes 2024-10-25 23:02 ` Marek Vasut 0 siblings, 1 reply; 10+ messages in thread From: Rasmus Villemoes @ 2024-10-25 8:41 UTC (permalink / raw) To: Marek Vasut; +Cc: Tom Rini, u-boot, Tim Harvey, Heiko Schocher On Thu, Oct 24 2024, Marek Vasut <marex@denx.de> wrote: > On 10/24/24 5:20 PM, Tom Rini wrote: >> On Thu, Oct 24, 2024 at 04:06:03PM +0200, Marek Vasut wrote: >>> On 10/24/24 3:18 PM, Rasmus Villemoes wrote: >>>> On Thu, Oct 24 2024, Marek Vasut <marex@denx.de> wrote: >>>> >>>>> On 10/24/24 12:01 PM, Rasmus Villemoes wrote: >>>>>> I enabled IMX_HAB on an imx8mp board, but even though I knew about the >>>>>> implementation, I forgot that I had to provide a sane value for >>>>>> SPL_LOAD_FIT_ADDRESS. The help text for IMX_HAB doesn't mention this >>>>>> implicit requirement, and there's no build-time warning; the default >>>>>> 0x0 value just ends up being returned from >>>>>> board_spl_fit_buffer_addr(), obviously resulting in a non-booting >>>>>> board. >>>>>> The existing imx8m* board configs that set a non-zero value >>>>>> currently >>>>>> all use 0x44000000. The actual value doesn't matter too much, but 0 is >>>>>> always wrong for imx8m platforms. So just use 0x44000000 as default >>>>>> for those platforms. >>>>>> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> >>>>>> --- >>>>>> boot/Kconfig | 1 + >>>>>> 1 file changed, 1 insertion(+) >>>>>> diff --git a/boot/Kconfig b/boot/Kconfig >>>>>> index 940389d4882..72d1f69afcd 100644 >>>>>> --- a/boot/Kconfig >>>>>> +++ b/boot/Kconfig >>>>>> @@ -231,6 +231,7 @@ config SPL_LOAD_FIT >>>>>> config SPL_LOAD_FIT_ADDRESS >>>>>> hex "load address of fit image" >>>>>> depends on SPL_LOAD_FIT >>>>>> + default 0x44000000 if ARCH_IMX8M >>>>> This only applies to HAB , for non-HAB the fitImage can be loaded at >>>>> arbitrary location, do you need: >>>>> >>>>> default 0x44000000 if ARCH_IMX8M && IMX_HAB >>>>> >>>>> right ? >>>> >>>> On IMX8 without HAB, the value is not used at all AFAICT (otherwise the >>>> value of 0x0 would have caused trouble already). I don't see the harm of >>>> setting some sane value that's actually within DRAM space independent of >>>> HAB. >>>> >>>> Making the default depend on IMX_HAB has the UX problem that if I do >>>> "make imx8mp_evk_defconfig", then go about tweaking stuff, then do "make >>>> menuconfig" and enable IMX_HAB, SPL_LOAD_FIT_ADDRESS already has the >>>> bogus 0x0 value and nothing forces or asks that to be changed. Making the >>>> default depend only on SOC (or SOC family or whatever IMX8M is in this >>>> context) makes that problem go away. >>> Looking at this closely, common/spl/spl_ram.c does make use of >>> CONFIG_SPL_LOAD_FIT_ADDRESS too, so I think yes, we should have a default >>> for this. >>> >>> iMX6 should have 0x14000000 >>> iMX7 should have 0x84000000 >>> iMX8M should have 0x44000000 >> These differ, slightly, from the value used in CONFIG_SYS_LOAD_ADDR. >> Could that not be used (and the overall option changed to default >> SYS_LOAD_ADDR ? > If I recall it right, no ... these addresses are where the fitImage is > loaded and where HAB does its authentication stuff on the fitImage, > that address has to be fixed and well aligned. Yes. Fixed certainly; I'm not aware of any specific aligment requirements, but I can't imagine it would need more than 0x1000 alignment, and we'd never use a fixed address less aligned than that anyway. > The SYS_LOAD_ADDR is > the destination address where the u-boot-nodtb.bin is copied from the > fitImage AFTER the fitImage was authenticated. No, I don't think so. Isn't u-boot-nodtb.bin just copied to the value of the load= property in the FIT, which is CONFIG_TEXT_BASE, aka 0x40200000 in the imx8mp case. As for Tom's suggestion to make SPL_LOAD_FIT_ADDRESS simply default to SYS_LOAD_ADDR: Perhaps, it would certainly be better than 0x0 which is almost always bogus. I think it would work for imx8mp_evk, though 0x40200000 and 0x40480000 are only 2.5MiB apart. But I have no idea how many boards would be affected by such a change or how many random ad hoc other CONFIG_FOO_THIS_OR_THAT exists that might or might not clash or overlap. IOW, I'm not signing off on such a patch... But it could be interesting to a least let CI chew on that to see if it's a complete non-starter. Rasmus ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] imx8m: set sane default value for SPL_LOAD_FIT_ADDRESS 2024-10-25 8:41 ` Rasmus Villemoes @ 2024-10-25 23:02 ` Marek Vasut 0 siblings, 0 replies; 10+ messages in thread From: Marek Vasut @ 2024-10-25 23:02 UTC (permalink / raw) To: Rasmus Villemoes; +Cc: Tom Rini, u-boot, Tim Harvey, Heiko Schocher On 10/25/24 10:41 AM, Rasmus Villemoes wrote: > On Thu, Oct 24 2024, Marek Vasut <marex@denx.de> wrote: > >> On 10/24/24 5:20 PM, Tom Rini wrote: >>> On Thu, Oct 24, 2024 at 04:06:03PM +0200, Marek Vasut wrote: >>>> On 10/24/24 3:18 PM, Rasmus Villemoes wrote: >>>>> On Thu, Oct 24 2024, Marek Vasut <marex@denx.de> wrote: >>>>> >>>>>> On 10/24/24 12:01 PM, Rasmus Villemoes wrote: >>>>>>> I enabled IMX_HAB on an imx8mp board, but even though I knew about the >>>>>>> implementation, I forgot that I had to provide a sane value for >>>>>>> SPL_LOAD_FIT_ADDRESS. The help text for IMX_HAB doesn't mention this >>>>>>> implicit requirement, and there's no build-time warning; the default >>>>>>> 0x0 value just ends up being returned from >>>>>>> board_spl_fit_buffer_addr(), obviously resulting in a non-booting >>>>>>> board. >>>>>>> The existing imx8m* board configs that set a non-zero value >>>>>>> currently >>>>>>> all use 0x44000000. The actual value doesn't matter too much, but 0 is >>>>>>> always wrong for imx8m platforms. So just use 0x44000000 as default >>>>>>> for those platforms. >>>>>>> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> >>>>>>> --- >>>>>>> boot/Kconfig | 1 + >>>>>>> 1 file changed, 1 insertion(+) >>>>>>> diff --git a/boot/Kconfig b/boot/Kconfig >>>>>>> index 940389d4882..72d1f69afcd 100644 >>>>>>> --- a/boot/Kconfig >>>>>>> +++ b/boot/Kconfig >>>>>>> @@ -231,6 +231,7 @@ config SPL_LOAD_FIT >>>>>>> config SPL_LOAD_FIT_ADDRESS >>>>>>> hex "load address of fit image" >>>>>>> depends on SPL_LOAD_FIT >>>>>>> + default 0x44000000 if ARCH_IMX8M >>>>>> This only applies to HAB , for non-HAB the fitImage can be loaded at >>>>>> arbitrary location, do you need: >>>>>> >>>>>> default 0x44000000 if ARCH_IMX8M && IMX_HAB >>>>>> >>>>>> right ? >>>>> >>>>> On IMX8 without HAB, the value is not used at all AFAICT (otherwise the >>>>> value of 0x0 would have caused trouble already). I don't see the harm of >>>>> setting some sane value that's actually within DRAM space independent of >>>>> HAB. >>>>> >>>>> Making the default depend on IMX_HAB has the UX problem that if I do >>>>> "make imx8mp_evk_defconfig", then go about tweaking stuff, then do "make >>>>> menuconfig" and enable IMX_HAB, SPL_LOAD_FIT_ADDRESS already has the >>>>> bogus 0x0 value and nothing forces or asks that to be changed. Making the >>>>> default depend only on SOC (or SOC family or whatever IMX8M is in this >>>>> context) makes that problem go away. >>>> Looking at this closely, common/spl/spl_ram.c does make use of >>>> CONFIG_SPL_LOAD_FIT_ADDRESS too, so I think yes, we should have a default >>>> for this. >>>> >>>> iMX6 should have 0x14000000 >>>> iMX7 should have 0x84000000 >>>> iMX8M should have 0x44000000 >>> These differ, slightly, from the value used in CONFIG_SYS_LOAD_ADDR. >>> Could that not be used (and the overall option changed to default >>> SYS_LOAD_ADDR ? >> If I recall it right, no ... these addresses are where the fitImage is >> loaded and where HAB does its authentication stuff on the fitImage, >> that address has to be fixed and well aligned. > > Yes. Fixed certainly; I'm not aware of any specific aligment > requirements, but I can't imagine it would need more than 0x1000 > alignment, and we'd never use a fixed address less aligned than that > anyway. > >> The SYS_LOAD_ADDR is >> the destination address where the u-boot-nodtb.bin is copied from the >> fitImage AFTER the fitImage was authenticated. > > No, I don't think so. Isn't u-boot-nodtb.bin just copied to the value of > the load= property in the FIT, which is CONFIG_TEXT_BASE, aka 0x40200000 > in the imx8mp case. Ah, right, thanks for keeping an eye on this. > As for Tom's suggestion to make SPL_LOAD_FIT_ADDRESS simply default to > SYS_LOAD_ADDR: Perhaps, it would certainly be better than 0x0 which is > almost always bogus. I think it would work for imx8mp_evk, though > 0x40200000 and 0x40480000 are only 2.5MiB apart. But I have no idea how > many boards would be affected by such a change or how many random ad hoc > other CONFIG_FOO_THIS_OR_THAT exists that might or might not clash or > overlap. IOW, I'm not signing off on such a patch... But it could be > interesting to a least let CI chew on that to see if it's a complete > non-starter. How about introducing a default set of text bases and load addresses, with the right spacing, and then start removing the custom ones from board configs ? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] imx8m: set sane default value for SPL_LOAD_FIT_ADDRESS 2024-10-24 11:51 ` Marek Vasut 2024-10-24 13:18 ` Rasmus Villemoes @ 2024-10-26 0:24 ` Fabio Estevam 2024-10-26 0:32 ` Tom Rini 1 sibling, 1 reply; 10+ messages in thread From: Fabio Estevam @ 2024-10-26 0:24 UTC (permalink / raw) To: Marek Vasut Cc: Rasmus Villemoes, u-boot, Tim Harvey, Heiko Schocher, Tom Rini On Thu, Oct 24, 2024 at 8:51 AM Marek Vasut <marex@denx.de> wrote: > > On 10/24/24 12:01 PM, Rasmus Villemoes wrote: > > I enabled IMX_HAB on an imx8mp board, but even though I knew about the > > implementation, I forgot that I had to provide a sane value for > > SPL_LOAD_FIT_ADDRESS. The help text for IMX_HAB doesn't mention this > > implicit requirement, and there's no build-time warning; the default > > 0x0 value just ends up being returned from > > board_spl_fit_buffer_addr(), obviously resulting in a non-booting > > board. > > > > The existing imx8m* board configs that set a non-zero value currently > > all use 0x44000000. The actual value doesn't matter too much, but 0 is > > always wrong for imx8m platforms. So just use 0x44000000 as default > > for those platforms. > > > > Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> Applied, thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] imx8m: set sane default value for SPL_LOAD_FIT_ADDRESS 2024-10-26 0:24 ` Fabio Estevam @ 2024-10-26 0:32 ` Tom Rini 0 siblings, 0 replies; 10+ messages in thread From: Tom Rini @ 2024-10-26 0:32 UTC (permalink / raw) To: Fabio Estevam Cc: Marek Vasut, Rasmus Villemoes, u-boot, Tim Harvey, Heiko Schocher [-- Attachment #1: Type: text/plain, Size: 1081 bytes --] On Fri, Oct 25, 2024 at 09:24:10PM -0300, Fabio Estevam wrote: > On Thu, Oct 24, 2024 at 8:51 AM Marek Vasut <marex@denx.de> wrote: > > > > On 10/24/24 12:01 PM, Rasmus Villemoes wrote: > > > I enabled IMX_HAB on an imx8mp board, but even though I knew about the > > > implementation, I forgot that I had to provide a sane value for > > > SPL_LOAD_FIT_ADDRESS. The help text for IMX_HAB doesn't mention this > > > implicit requirement, and there's no build-time warning; the default > > > 0x0 value just ends up being returned from > > > board_spl_fit_buffer_addr(), obviously resulting in a non-booting > > > board. > > > > > > The existing imx8m* board configs that set a non-zero value currently > > > all use 0x44000000. The actual value doesn't matter too much, but 0 is > > > always wrong for imx8m platforms. So just use 0x44000000 as default > > > for those platforms. > > > > > > Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> > > Applied, thanks. In that moving to a different default is a tangential cleanup, I'm fine with this. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-10-26 4:19 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-24 10:01 [PATCH] imx8m: set sane default value for SPL_LOAD_FIT_ADDRESS Rasmus Villemoes 2024-10-24 11:51 ` Marek Vasut 2024-10-24 13:18 ` Rasmus Villemoes 2024-10-24 14:06 ` Marek Vasut 2024-10-24 15:20 ` Tom Rini 2024-10-24 16:13 ` Marek Vasut 2024-10-25 8:41 ` Rasmus Villemoes 2024-10-25 23:02 ` Marek Vasut 2024-10-26 0:24 ` Fabio Estevam 2024-10-26 0:32 ` Tom Rini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox