Hi, On Mon, 29 Apr 2019 at 07:19, Tom Rini <[email protected]> wrote: > > On Mon, Apr 29, 2019 at 03:06:39PM +0200, Heiko Schocher wrote: > > Hello Simon, > > > > Am 25.04.2019 um 21:24 schrieb Simon Goldschmidt: > > >Am 25.04.2019 um 12:50 schrieb Tom Rini: > > >>On Thu, Apr 25, 2019 at 09:32:22AM +0200, Simon Goldschmidt wrote: > > >>>On Thu, Apr 25, 2019 at 1:59 AM Simon Glass <[email protected]> wrote: > > >>>> > > >>>>Hi, > > >>>> > > >>>>On Wed, 24 Apr 2019 at 05:53, Tom Rini <[email protected]> wrote: > > >>>>> > > >>>>>On Wed, Apr 24, 2019 at 01:49:52PM +0200, Simon Goldschmidt wrote: > > >>>>>>On Wed, Apr 24, 2019 at 1:27 PM Tom Rini <[email protected]> wrote: > > >>>>>>> > > >>>>>>>On Tue, Apr 23, 2019 at 09:54:10PM -0600, Simon Glass wrote: > > >>>>>>>>On Mon, 1 Apr 2019 at 14:01, Simon Goldschmidt > > >>>>>>>><[email protected]> wrote: > > >>>>>>>>> > > >>>>>>>>>If the malloc range passed to mem_malloc_init() is at the end of > > >>>>>>>>>address > > >>>>>>>>>range and 'start + size' overflows to 0, following allocations > > >>>>>>>>>fail as > > >>>>>>>>>mem_malloc_end is zero (which looks like uninitialized). > > >>>>>>>>> > > >>>>>>>>>Fix this by subtracting 1 of 'start + size' overflows to zero. > > >>>>>>>>> > > >>>>>>>>>Signed-off-by: Simon Goldschmidt <[email protected]> > > >>>>>>>>>--- > > >>>>>>>>> > > >>>>>>>>>Changes in v4: None > > >>>>>>>>>Changes in v3: None > > >>>>>>>>> > > >>>>>>>>> common/dlmalloc.c | 4 ++++ > > >>>>>>>>> 1 file changed, 4 insertions(+) > > >>>>>>>> > > >>>>>>>>Reviewed-by: Simon Glass <[email protected]> > > >>>>>>> > > >>>>>>>So, the problem with this patch is that it increases the generic > > >>>>>>>malloc > > >>>>>>>code size ever so slightly and blows up smartweb :( > > >>>>>> > > >>>>>>Ehrm, ok, so how do we proceed? > > >>>>> > > >>>>>A good question. Take a look at spl/u-boot-spl.map on smartweb and see > > >>>>>if, of the malloc functions it doesn't discard there's something that > > >>>>>maybe could be optimized somewhere? > > >>>> > > >>>>I wonder if we should have a Kconfig option like SPL_CHECKS which > > >>>>enables these sorts of minor checks, which may only fix one board at > > >>>>the cost of code size? > > >>>> > > >>>>Then it could be enabled by default, but disabled on this board? > > >>> > > >>>For a bigger change, this might be an idea, but for a change that I can > > >>>cut > > >>>down to 16 or even 8 bytes code size increasement, I don't think having a > > >>>new option would be good. > > >>> > > >>>Anyway, I just tried at work and I don't get the overflow. Tom, which gcc > > >>>are you using to get the size error? It works for me on Debian 9 but > > >>>doesn't > > >>>work with Ubuntu (both times, default cross compiler toolchain > > >>>installed). > > >> > > >>I'm using the gcc-7.3 from kernel.org that we use in travis/etc. > > > > > >Ok, so I have gcc-7.3 on my Ubuntu machine as well. I don't know why 6.3 > > >seems to produce smaller binaries (I thought they were getting smaller > > >with new versions, not larger). > > > > > >However, I've stripped down that patch to +8 Bytes only and sent v5. > > > > Thanks! > > > > Sorry for digging so late in, but I was on vacation... > > > > Hmm.. the smartweb board has only 4k sram for SPL, and I have no chance > > to convert it to DM to get rid of some compiler warnings ... > > > > I am unsure what to do now with this hardware ... > > Well, with regards to SPL + DM, this is one of the cases wherein we just > have-to allow for the SPL driver code at least to be a one-off. If the > "whatever ROM loads of our code" stage can set things up enough such > that we can hand off to a full U-Boot, that's great. If not, this is > then a case where TPL comes in to play, and that really is as one-off as > needed, to load a more general SPL and so forth.
I think DM in SPL is a good thing, so long as there is space. If not, then we should allow non-DM also. I wonder if we need our policy to be written down somewhere? > > But, I'm fine with saying smartweb keeps and maintains whatever SPL code > it needs to use. It's just that in this case, it's not at all a DM > thing, it's a change in malloc. Regards, Simon _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
