qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 02/12 v9] linux-user: tilegx: Add target feature


From: Chen Gang
Subject: Re: [Qemu-devel] [PATCH 02/12 v9] linux-user: tilegx: Add target features support within qemu
Date: Sat, 11 Apr 2015 11:37:04 +0800
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.6.0

On 4/11/15 05:51, Peter Maydell wrote:
> On 10 April 2015 at 21:41, Chen Gang <address@hidden> wrote:
>> On 4/10/15 05:31, Peter Maydell wrote:
>>> On 27 March 2015 at 10:49, Chen Gang <address@hidden> wrote:
>>>> +typedef struct target_sigaltstack {
>>>> +    abi_ulong ss_sp;
>>>> +    abi_ulong ss_size;
>>>> +    abi_long ss_flags;
>>>> +} target_stack_t;
>>>
>>> Where does this come from? It doesn't match the kernel's
>>> generic-headers struct layout.
>>>
>>
>> Oh, sorry, originally, I guess, I only copied it from microblaze, did
>> not check kernel.
> 
> These structures are all user-guest-facing ABI, so they must
> match the kernel's structures for your target architecture.
> 
>> I shall use generic-headers which tilegx will use (the result will like
>> alpha has done):
>>
>> typedef struct target_sigaltstack {
>>     abi_ulong ss_sp;
>>     int32_t ss_flags;
>>     int32_t dummy;
>>     abi_ulong ss_size;
>> } target_stack_t;
> 
> This doesn't match the kernel either.
> 
> http://lxr.free-electrons.com/source/include/uapi/asm-generic/signal.h#L111
> 
> You have a pointer, an int and a size_t, so you want
>     abi_ulong ss_sp;
>     abi_int ss_flags;
>     abi_ulong ss_size;
> 
> like aarch64.
>

For me, for tilegx which is always 64-bit, add 'dummy' is more clearer
(but need to use abi_int instead of original int32_t).

And does it pragma packed ()? As far as I know, it doesn't.
 
>>
>> [...]
>>>> +
>>>> +struct target_ipc_perm {
>>>> +    abi_int __key;                      /* Key.  */
>>>> +    abi_uint uid;                       /* Owner's user ID.  */
>>>> +    abi_uint gid;                       /* Owner's group ID.  */
>>>> +    abi_uint cuid;                      /* Creator's user ID.  */
>>>> +    abi_uint cgid;                      /* Creator's group ID.  */
>>>> +    abi_uint mode;                    /* Read/write permission.  */
>>>> +    abi_ushort __seq;                   /* Sequence number.  */
>>>> +    abi_ushort __pad2;
>>>> +    abi_ulong __unused1;
>>>> +    abi_ulong __unused2;
>>>> +};
>>>
>>> Again, doesn't seem to match kernel?
>>>
>>
>> For me, it matches kernel. mode is abi_uint (__kernel_mode_t is 32-bit).
> 
> I'm looking at
> http://lxr.free-electrons.com/source/include/uapi/linux/ipc.h#L9
> which doesn't have that padding and unused fields at the end.
> However the ipc structs are pretty confusing so maybe that's
> the wrong one -- which one are you looking at?
> 

I check the linux-next tree next-20150401 in include/uapi/linux/ipc.h,
the __kernel_mode_t is unsigned int for tile (and also most of 64-bit
targets in qemu, mode is 32-bit), please check again.

it really has no "__pad2" and "__unused?", but after check all the other
targets within qemu, they all have "__pad2" and "__unused?".  May qemu
itself need them? I am not quite sure, but for me, appending them is OK.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed



reply via email to

[Prev in Thread] Current Thread [Next in Thread]