Re: [Qemu-devel] [PATCH 1/3] linux-user: introduce functions to detect CPU type

[Laurent Vivier]

Le 15/01/2018 à 23:04, Richard Henderson a écrit :
> On 01/13/2018 06:48 AM, Laurent Vivier wrote:
>> From: YunQiang Su <address@hidden>
>>
>> Move CPU type name selection to a function,
>> and add a function to return ELF e_flags.
>>
>> [lv: splitted the patch and some cleanup in get_elf_eflags()]
>> Signed-off-by: Laurent Vivier <address@hidden>
>> ---
> 
> This needs to be split.
> 
>> +int get_elf_eflags(int fd, uint32_t *eflags)
>> +{
>> +    struct elfhdr ehdr;
>> +    off_t offset;
>> +    int ret;
>> +
>> +    /* Read ELF header */
>> +    offset = lseek(fd, 0, SEEK_SET);
>> +    if (offset == (off_t) -1) {
>> +        return -1;
>> +    }
>> +    ret = read(fd, &ehdr, sizeof(ehdr));
>> +    if (ret < sizeof(ehdr)) {
>> +        return -1;
>> +    }
> 
> There is no reason to read the elf header twice -- e_flags has already been
> stored in the struct image_info.

When we set cpu_model, image_info is not initialized.

Do you propose to move cpu_init() after loader_exec()?

>> +static const char *get_cpu_model(int fd)
>> +{
>> +#if defined(TARGET_I386)
>> +#ifdef TARGET_X86_64
>> +    return "qemu64";
>> +#else
>> +    return "qemu32";
>> +#endif
> 
> This should be our opportunity to split this ifdef chain into small inline
> functions within linux-user/*/target_cpu.h.  Pass the e_flags value directly
> instead of a file descriptor.
> 

Good idea.

Thanks,
Laurent