On Wed, 14 Oct 2020 18:33:25 +0200, Ahmad Fatoum wrote: > On 10/14/20 5:08 PM, Michael Tretter wrote: > > If the SDRAM is mapped to address 0x0 and an image should be loaded to > > to the SDRAM without offset, Barebox would normally trap the access as a > > null pointer. > > > > However, since Linux kernel commit cfa7ede20f13 ("arm64: set TEXT_OFFSET > > to 0x0 in preparation for removing it entirely") no offset is the > > default for arm64. Therefore, copying the image to 0x0 of the SDRAM is > > necessary. > > > > Disable the zero page trap for copying an image to address 0x0. > > > > Signed-off-by: Michael Tretter <m.tretter@xxxxxxxxxxxxxx> > > --- > > common/uimage.c | 18 ++++++++++++++++-- > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/common/uimage.c b/common/uimage.c > > index a84b8fddc4e7..b1e9b402e98a 100644 > > --- a/common/uimage.c > > +++ b/common/uimage.c > > @@ -27,6 +27,7 @@ > > #include <rtc.h> > > #include <filetype.h> > > #include <memory.h> > > +#include <zero_page.h> > > > > static inline int uimage_is_multi_image(struct uimage_handle *handle) > > { > > @@ -359,7 +360,13 @@ static int uimage_sdram_flush(void *buf, unsigned int len) > > } > > } > > > > - memcpy(uimage_buf + uimage_size, buf, len); > > + if (zero_page_contains((unsigned long)uimage_buf + uimage_size)) { > > + zero_page_disable(); > > + memcpy(uimage_buf + uimage_size, buf, len); > > + zero_page_enable(); > > If this remains, please add a memcpy_notrap or something. Should I check the destination before calling memcpy_notrap or should I always call the memcpy_notrap if there is a possibility to copy to 0x0 and check for the destination within the function? I fear that having such a "simple" function would encourage to use it more often. I would prefer to make the code to use it more clumsy and make it (similar to data_abort_mask()) the responsibility of the caller to be aware that bad things might happen when the zero_page is disabled. > > > + } else { > > + memcpy(uimage_buf + uimage_size, buf, len); > > + } > > > > uimage_size += len; > > > > @@ -388,7 +395,14 @@ struct resource *file_to_sdram(const char *filename, unsigned long adr) > > goto out; > > } > > > > - now = read_full(fd, (void *)(res->start + ofs), BUFSIZ); > > + if (zero_page_contains(res->start + ofs)) { > > + zero_page_disable(); > > + now = read_full(fd, (void *)(res->start + ofs), BUFSIZ); > > + zero_page_enable(); > > And use that new memcpy_notrap here to copy from an intermediate buffer. You open quite a can > of worms when you treat NULL as a valid address. Better have this contained in a single > file instead of hoping the compiler doesn't do a NULL-can't-happen-here optimization > in all that block/cdev/fs code that read_full may call into. Could you explain, what kind of optimization you would expect? Michael > > > + } else { > > + now = read_full(fd, (void *)(res->start + ofs), BUFSIZ); > > + } > > + > > if (now < 0) { > > release_sdram_region(res); > > res = NULL; > > _______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox