Re: [Qemu-devel] [Qemu-arm] [PATCH v4 5/6] loader: Implement .hex file loader

[Philippe Mathieu-Daudé]

On 08/15/2018 02:52 PM, Stefan Hajnoczi wrote:
> On Wed, Aug 15, 2018 at 3:18 PM Philippe Mathieu-Daudé <address@hidden> wrote:
>> On 08/13/2018 12:56 PM, Stefan Hajnoczi wrote:
>>> On Fri, Aug 10, 2018 at 02:00:44AM -0300, Philippe Mathieu-Daudé wrote:
>>>> On 08/03/2018 11:47 AM, Stefan Hajnoczi wrote:
>>>>> +        parser->rom_start_address = parser->next_address_to_write;
>>>>> +        parser->current_rom_index = 0;
>>>>> +        break;
>>>>> +
>>>>> +    case START_SEG_ADDR_RECORD:
>>>>> +        if (line->byte_count != 4 && line->address != 0) {
>>>>> +            return -1;
>>>>> +        }
>>>>> +
>>>>> +        /* x86 16-bit CS:IP segmented addressing */
>>>>> +        *(parser->start_addr) = (((line->data[0] << 8) | line->data[1]) 
>>>>> << 4) |
>>>>> +                                (line->data[2] << 8) | line->data[3];
>>>>
>>>> Can you add a qtest for this case?
>>>> For the HEX loader I understand the specs as this is the same parsing as
>>>> the START_LINEAR_ADDR_RECORD case; so I disagree with data[0] and
>>>> data[1] shifts.
>>>
>>> x86 real-mode CS:IP addressing means (CS << 4) + IP.  It produces 24-bit
>>> addresses on 80286 and later.  This is not the same as
>>> START_LINEAR_ADDR_RECORD.
>>
>> OK!
>>
>>>
>>> GNU bfd implements it as follows:
>>>
>>>   abfd->start_address += (HEX4 (buf) << 4) + HEX4 (buf + 4);
>>
>> Hmm any idea why they use +4 ?
> 
> buf is char* and HEX4("0123") produces 0x0123.  So this line evaluates
> ASCII hex input buf="XXXXYYYY" to (0xXXXX << 4) + 0xYYYY.

Haha with this weird indentation (space before parenthesis) I quickly
thought 4 was added to the address, and probably than HEX4 was a
constant... I will avoid too late review ;) sorry =)