| Message ID | 20191125134811.16462-1-patrice.chotard@st.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [U-Boot] board_f.c: Insure gd->new_bootstage alignment | expand |
Hi, > From: Patrice CHOTARD <patrice.chotard@st.com> > Sent: lundi 25 novembre 2019 14:48 > > In reserve_bootstage(), in case size is odd, gd->new_bootstage is not aligned. In > bootstage_relocate(), the platform hangs when getting access to data- > >record[i].name. > To avoid this issue, make gd->new_bootstage 16 byte aligned. > > Fixes: ac9cd4805c8b ("bootstage: Correct relocation algorithm") > > Signed-off-by: Patrice Chotard <patrice.chotard@st.com> > Reviewed-by: Vikas MANOCHA <vikas.manocha@st.com> > --- This patch is mandatory for stm32mp1 (ARM plaform with bootstage feature activated). Without this patch, the boot failed for v2020.01-rc3 (crash has struct pointer new_bootstage is not aligned). Reviewed-by: Patrick Delaunay <patrick.delaunay@st.com> Tested-by: Patrick Delaunay <patrick.delaunay@st.com> > common/board_f.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/common/board_f.c b/common/board_f.c index e3591cbaeb..9cd4cfaf5f > 100644 > --- a/common/board_f.c > +++ b/common/board_f.c > @@ -559,6 +559,7 @@ static int reserve_bootstage(void) > int size = bootstage_get_size(); > > gd->start_addr_sp -= size; > + gd->start_addr_sp = ALIGN_DOWN(gd->start_addr_sp, 16); > gd->new_bootstage = map_sysmem(gd->start_addr_sp, size); > debug("Reserving %#x Bytes for bootstage at: %08lx\n", size, > gd->start_addr_sp); > -- > 2.17.1 Thanks Patrick
On 11/26/19 1:16 PM, Patrick DELAUNAY wrote: > Hi, > >> From: Patrice CHOTARD <patrice.chotard@st.com> >> Sent: lundi 25 novembre 2019 14:48 >> >> In reserve_bootstage(), in case size is odd, gd->new_bootstage is not aligned. In >> bootstage_relocate(), the platform hangs when getting access to data- >>> record[i].name. >> To avoid this issue, make gd->new_bootstage 16 byte aligned. >> >> Fixes: ac9cd4805c8b ("bootstage: Correct relocation algorithm") >> >> Signed-off-by: Patrice Chotard <patrice.chotard@st.com> >> Reviewed-by: Vikas MANOCHA <vikas.manocha@st.com> >> --- > > This patch is mandatory for stm32mp1 (ARM plaform with bootstage feature activated). > > Without this patch, the boot failed for v2020.01-rc3 (crash has struct pointer new_bootstage is not aligned). > > Reviewed-by: Patrick Delaunay <patrick.delaunay@st.com> > Tested-by: Patrick Delaunay <patrick.delaunay@st.com> > >> common/board_f.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/common/board_f.c b/common/board_f.c index e3591cbaeb..9cd4cfaf5f >> 100644 >> --- a/common/board_f.c >> +++ b/common/board_f.c >> @@ -559,6 +559,7 @@ static int reserve_bootstage(void) >> int size = bootstage_get_size(); >> >> gd->start_addr_sp -= size; >> + gd->start_addr_sp = ALIGN_DOWN(gd->start_addr_sp, 16); A comment in the code describing why this ALIGN_DOWN() is done would be helpful. Is this value sufficient for all boards? I found references for: x86_64 ABI: https://reviews.llvm.org/D30049: 16 bytes ARMv8: ARMv8 Instruction Set Overview: quad word, 16 bytes Best regards Heinrich >> gd->new_bootstage = map_sysmem(gd->start_addr_sp, size); >> debug("Reserving %#x Bytes for bootstage at: %08lx\n", size, >> gd->start_addr_sp); >> -- >> 2.17.1 > > Thanks > > Patrick >
Hi Heinrich On 11/26/19 6:41 PM, Heinrich Schuchardt wrote: > On 11/26/19 1:16 PM, Patrick DELAUNAY wrote: >> Hi, >> >>> From: Patrice CHOTARD <patrice.chotard@st.com> >>> Sent: lundi 25 novembre 2019 14:48 >>> >>> In reserve_bootstage(), in case size is odd, gd->new_bootstage is not aligned. In >>> bootstage_relocate(), the platform hangs when getting access to data- >>>> record[i].name. >>> To avoid this issue, make gd->new_bootstage 16 byte aligned. >>> >>> Fixes: ac9cd4805c8b ("bootstage: Correct relocation algorithm") >>> >>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com> >>> Reviewed-by: Vikas MANOCHA <vikas.manocha@st.com> >>> --- >> >> This patch is mandatory for stm32mp1 (ARM plaform with bootstage feature activated). >> >> Without this patch, the boot failed for v2020.01-rc3 (crash has struct pointer new_bootstage is not aligned). >> >> Reviewed-by: Patrick Delaunay <patrick.delaunay@st.com> >> Tested-by: Patrick Delaunay <patrick.delaunay@st.com> >> >>> common/board_f.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/common/board_f.c b/common/board_f.c index e3591cbaeb..9cd4cfaf5f >>> 100644 >>> --- a/common/board_f.c >>> +++ b/common/board_f.c >>> @@ -559,6 +559,7 @@ static int reserve_bootstage(void) >>> int size = bootstage_get_size(); >>> >>> gd->start_addr_sp -= size; >>> + gd->start_addr_sp = ALIGN_DOWN(gd->start_addr_sp, 16); > > A comment in the code describing why this ALIGN_DOWN() is done would be > helpful. Ok i will add a comment about that. > > Is this value sufficient for all boards? I simply reuse the same alignment already applied for other memory area : _ start_addr_sp in reserve_stacks() _ new_bloblist in reserve_bloblist() Thanks Patrice > > I found references for: > > x86_64 ABI: https://reviews.llvm.org/D30049: 16 bytes > ARMv8: ARMv8 Instruction Set Overview: quad word, 16 bytes > > Best regards > > Heinrich > >>> gd->new_bootstage = map_sysmem(gd->start_addr_sp, size); >>> debug("Reserving %#x Bytes for bootstage at: %08lx\n", size, >>> gd->start_addr_sp); >>> -- >>> 2.17.1 >> >> Thanks >> >> Patrick >> >
diff --git a/common/board_f.c b/common/board_f.c index e3591cbaeb..9cd4cfaf5f 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -559,6 +559,7 @@ static int reserve_bootstage(void) int size = bootstage_get_size(); gd->start_addr_sp -= size; + gd->start_addr_sp = ALIGN_DOWN(gd->start_addr_sp, 16); gd->new_bootstage = map_sysmem(gd->start_addr_sp, size); debug("Reserving %#x Bytes for bootstage at: %08lx\n", size, gd->start_addr_sp);
In reserve_bootstage(), in case size is odd, gd->new_bootstage is not aligned. In bootstage_relocate(), the platform hangs when getting access to data->record[i].name. To avoid this issue, make gd->new_bootstage 16 byte aligned. Fixes: ac9cd4805c8b ("bootstage: Correct relocation algorithm") Signed-off-by: Patrice Chotard <patrice.chotard@st.com> Reviewed-by: Vikas MANOCHA <vikas.manocha@st.com> --- common/board_f.c | 1 + 1 file changed, 1 insertion(+)