Peter Maydell <peter.mayd...@linaro.org> writes:
> On Thu, 10 Feb 2022 at 11:30, Alex Bennée <alex.ben...@linaro.org> wrote:
>>
>> The previous numbers were a guess at best and rather arbitrary without
>> taking into account anything that might be loaded. Instead of using
>> guesses based on the state of registers implement a new function that:
>>
>> a) scans the MemoryRegions for the largest RAM block
>> b) iterates through all "ROM" blobs looking for the biggest gap
>>
>> The "ROM" blobs include all code loaded via -kernel and the various
>> -device loader techniques.
>>
>> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
>> Cc: Andrew Strauss <astraus...@gmail.com>
>> Cc: Keith Packard <kei...@keithp.com>
>> Message-Id: <20210601090715.22330-1-alex.ben...@linaro.org>
>>
>
>
>> +/*
>> + * Sort into address order. We break ties between rom-startpoints
>> + * and rom-endpoints in favour of the startpoint, by sorting the 0->1
>> + * transition before the 1->0 transition. Either way round would
>> + * work, but this way saves a little work later by avoiding
>> + * dealing with "gaps" of 0 length.
>> + */
>> +static gint sort_secs(gconstpointer a, gconstpointer b)
>> +{
>> + RomSec *ra = (RomSec *) a;
>> + RomSec *rb = (RomSec *) b;
>> +
>> + if (ra->base == rb->base) {
>> + return ra->se - rb->se;
>> + }
>> + return ra->base > rb->base ? 1 : -1;
>> +}
>
> This sort comparator still doesn't report the equality
> case as actually equal.
When ra->se and rb->se are the same it returns 0. Is that not what you want?
>
>> /*
>> - * Find the chunk of R/W memory containing the address. This is
>> - * used for the SYS_HEAPINFO semihosting call, which should
>> - * probably be using information from the loaded application.
>> + * If we have found the RAM lets iterate through the ROM blobs to
>> + * workout the best place for the remainder of RAM and split it
>
> "work out"
>
>> + * equally between stack and heap.
>> */
>
>> @@ -1201,12 +1205,15 @@ target_ulong do_common_semihosting(CPUState *cs)
>> retvals[2] = ts->stack_base;
>> retvals[3] = 0; /* Stack limit. */
>> #else
>> - limit = current_machine->ram_size;
>> - /* TODO: Make this use the limit of the loaded application. */
>> - retvals[0] = rambase + limit / 2;
>> - retvals[1] = rambase + limit;
>> - retvals[2] = rambase + limit; /* Stack base */
>> - retvals[3] = rambase; /* Stack limit. */
>> + /*
>> + * Reporting 0 indicates we couldn't calculate the real
>> + * values which should force most software to fall back to
>> + * using information it has.
>> + */
>
> What is this comment referring to? We aren't obviously
> reporting 0 here...
Stale comment, deleted.
>
>> + retvals[0] = info.heapbase; /* Heap Base */
>> + retvals[1] = info.heaplimit; /* Heap Limit */
>> + retvals[2] = info.heaplimit; /* Stack base */
>> + retvals[3] = info.heapbase; /* Stack limit. */
>> #endif
>>
>> for (i = 0; i < ARRAY_SIZE(retvals); i++) {
>
> Otherwise
> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
>
> -- PMM
--
Alex Bennée
