Re: [PATCH v4 1/2] semihosting/arm-compat: replace heuristic for softmmu SYS_HEAPINFO

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Wed, 23 Jun 2021 at 14:47, Alex Bennée <alex.bennee@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.bennee@linaro.org>
>> Cc: Andrew Strauss <astrauss11@gmail.com>
>> Cc: Keith Packard <keithp@keithp.com>
>> Message-Id: <20210601090715.22330-1-alex.bennee@linaro.org>
>
>> @@ -349,4 +349,20 @@ int rom_add_option(const char *file, int32_t bootindex);
>>   * overflow on real hardware too. */
>>  #define UBOOT_MAX_GUNZIP_BYTES (64 << 20)
>>
<snip>
>> +static gint sort_secs(gconstpointer a, gconstpointer b)
>> +{
>> +    RomSec *ra = (RomSec *) a;
>> +    RomSec *rb = (RomSec *) b;
>
> /*
>  * 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.
>  */
>
>> +
>> +    if (ra->base == rb->base) {
>> +        return ra->se > rb->se ? -1 : 1;
>> +    }
>> +    return ra->base > rb->base ? 1 : -1;
>
> This has forgotten the "equality" case, which you will
> see if two blobs start at the same address (at least in
> theory; at the moment the rom blob loader will try to
> reject overlaps, though it might not do so forever).

I'm confused what you mean by equality case. If both RomSecs have the
same base and the same se flag we need to pick one of them. Are you
saying when ra->se == ra->sb we should take ra? 

-- 
Alex Bennée