On Mon, Feb 24, 2025 at 09:54:56AM +0100, Heinrich Schuchardt wrote:
> On 2/24/25 06:55, Sam Edwards wrote:
> > libfdt 1.6.1+ requires the FDT to be 8-byte aligned and returns an error
> > if not. OpenSBI 1.0+ includes this version of libfdt and will also
> > reject misaligned FDTs.
> >
> > However, OpenSBI cannot indicate the error to the user: since it cannot
> > access the serial console, it can only silently hang. This can be very
> > difficult to diagnose without proper debugging facilities. Therefore,
> > give the U-Boot SPL, which *can* print error messages, an additional
> > check for proper FDT alignment.
> >
> > Signed-off-by: Sam Edwards <cfswo...@gmail.com>
> > ---
> > common/spl/spl_opensbi.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c
> > index 5a26d7c31a4..0ed6afeacc6 100644
> > --- a/common/spl/spl_opensbi.c
> > +++ b/common/spl/spl_opensbi.c
> > @@ -57,6 +57,11 @@ void __noreturn spl_invoke_opensbi(struct spl_image_info
> > *spl_image)
> > hang();
> > }
> >
> > + if (!IS_ALIGNED((uintptr_t)spl_image->fdt_addr, 8)) {
> > + pr_err("SPL image loaded an improperly-aligned device tree\n");
>
> We only use pr_err() in drivers when we copy code from Linux. Otherwise
> use log_err().
>
> As this code is for RISC-V, this patch should have been sent to the
> RISC-V maintainers.
>
> cc: Leo, Rick
>
> SPL size is very restricted on boards where it is responsible for
> initializing DRAM and therefore has to fit into cache. We should only
> add code that is strictly needed to SPL.
>
> What makes you think that this problem can realistically occur?I would like to know if Sam hit this in practice as well. But we have had more than one place where because we don't ensure 8-byte alignment and have use-in-place, that we've broken things. Picking up one of the patches that would address this within U-Boot is on my list now that we've had confirmation of some padding command that's sufficiently portable. -- Tom
signature.asc
Description: PGP signature
