[Qemu-devel] Re: [PATCH v5 17/18] gdbstub: x86: Support for setting segment registers

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]

Anthony Liguori wrote:
> Jan Kiszka wrote:
>> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
>> index eed1f62..b7c8a2f 100644
>> --- a/target-i386/cpu.h
>> +++ b/target-i386/cpu.h
>> @@ -651,6 +651,20 @@ int cpu_get_pic_interrupt(CPUX86State *s);
>>  /* MSDOS compatibility mode FPU exception support */
>>  void cpu_set_ferr(CPUX86State *s);
>>  
>> +static inline unsigned int get_seg_limit(uint32_t e1, uint32_t e2)
>> +{
>> +    unsigned int limit;
>> +    limit = (e1 & 0xffff) | (e2 & 0x000f0000);
>> +    if (e2 & DESC_G_MASK)
>> +        limit = (limit << 12) | 0xfff;
>> +    return limit;
>> +}
>> +
>> +static inline uint32_t get_seg_base(uint32_t e1, uint32_t e2)
>> +{
>> +    return ((e1 >> 16) | ((e2 & 0xff) << 16) | (e2 & 0xff000000));
>> +}
>> +
>>   
> 
> I like this patch but if you're going to export new x86 helper
> functions, please prefix them with cpu_x86. In this case, these helpers
> are awfully low level (they're taking a GDT entry split into two 32-bit
> words).  I would rather see an interface that took a CPUState and a
> segment register index.  In the very least, it won't be very obvious to
> anyone what this API expects to take so a comment would be required.

That was a result of the decision process "quick feature enhancement or
also some more refactoring?". :)

Will think out a new interface instead, leaving those two where they
came from.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]