Request for review/comments: 32-bit compat for non-x86 architectures

[Nathan Whitehorn]

Kostik Belousov wrote:
> On Wed, Mar 10, 2010 at 09:55:40AM -0600, Nathan Whitehorn wrote:
>   
>> John Baldwin wrote:
>>     
>>> On Wednesday 10 March 2010 9:39:15 am Nathan Whitehorn wrote:
>>>  
>>>       
>>>> John Baldwin wrote:
>>>>    
>>>>         
>>>>> On Tuesday 09 March 2010 11:14:27 pm Nathan Whitehorn wrote:
>>>>>  
>>>>>      
>>>>>           
>>>>>> The patch at 
>>>>>> http://people.freebsd.org/~nwhitehorn/compat_freebsd32.diff 
>>>>>> (pre-generated freebsd32 syscalls stuff is included, which will be done 
>>>>>> in two steps on commit) provides groundwork for supporting 32-bit 
>>>>>> compatibility for 64-bit MIPS and PowerPC systems. It has been tested 
>>>>>> on amd64 and powerpc64, and compile-tested on ia64. There are two main 
>>>>>> parts to the patch:
>>>>>>
>>>>>> 1) COMPAT_IA32 is renamed COMPAT_FREEBSD32, in analogy to 
>>>>>> COMPAT_LINUX32, etc. This requires updating kernel configurations, but 
>>>>>> is less painful than filling machine-independent bits of the kernel 
>>>>>> with #if defined(COMPAT_IA32) || defined(COMPAT_PPC32) || 
>>>>>> defined(COMPAT_MIPS32) || ..., and is no less descriptive than the old 
>>>>>> name.
>>>>>>
>>>>>> 2) Modifications to the freebsd32 compat layer to support big-endian 
>>>>>> architectures.
>>>>>>
>>>>>> I would appreciate any comments, bugs, or test results on ia64.
>>>>>>    
>>>>>>        
>>>>>>             
>>>>> This doesn't look right for non-x86 32-bit ABIs:
>>>>>
>>>>> Index: sys/kern/imgact_elf.c
>>>>> ===================================================================
>>>>> --- sys/kern/imgact_elf.c	(revision 204681)
>>>>> +++ sys/kern/imgact_elf.c	(working copy)
>>>>> @@ -1439,7 +1439,7 @@
>>>>> 		ehdr->e_ident[EI_ABIVERSION] = 0;
>>>>> 		ehdr->e_ident[EI_PAD] = 0;
>>>>> 		ehdr->e_type = ET_CORE;
>>>>> -#if defined(COMPAT_IA32) && __ELF_WORD_SIZE == 32
>>>>> +#if defined(COMPAT_FREEBSD32) && __ELF_WORD_SIZE == 32
>>>>> 		ehdr->e_machine = EM_386;
>>>>> #else
>>>>> 		ehdr->e_machine = ELF_ARCH;
>>>>>  
>>>>>      
>>>>>           
>>>> Good catch! Such are the dangers of sed. How about defining an 
>>>> ELF_ARCH32 in machine/elf.h for this case?
>>>>    
>>>>         
>>> Yes, that sounds good.
>>>
>>>  
>>>       
>>>>> It would be nice to eliminate having <compat/ia32*> includes in MI code 
>>>>> by instead including those headers in appropriate headers in 
>>>>> <machine/*>.  For example, we could change <machine/reg.h> on amd64 and 
>>>>> ia64 to include these headers, perhaps under an #ifdef COMPAT_FREEBSD32.
>>>>>
>>>>> Hmm, actually, I'm quite convinced now that <machine/reg.h> for ia64 and 
>>>>> amd64 should include <compat/ia32/ia32_reg.h> in the #ifdef _KERNEL 
>>>>> section to avoid polluting those includes in MI code.  I'm not sure what 
>>>>> the various <machine/fpu.h> includes are for, but fixing ia32_reg.h 
>>>>> would be a good first step.  It would make your diffs smaller I think.
>>>>>  
>>>>>      
>>>>>           
>>>> This is how it works on powerpc64. I didn't modify amd64 and ia64 in 
>>>> order to avoid making too many changes, but I think you're right that 
>>>> this is a good idea. I'll add that to the patch when fixing the EM_386 
>>>> bit you pointed out above.
>>>>    
>>>>         
>>> Ok, thanks.
>>>
>>>  
>>>       
>> I've updated the patch to incorporate these two changes, at 
>> http://people.freebsd.org/~nwhitehorn/compat_freebsd32_2.diff. Due to 
>> recursive inclusion issues with sys/procfs.h, it also moves prstatus32 
>> and friends to compat/freebsd32/freebsd32.h from ia32_reg.h. They are 
>> MI, and seems like a more appropriate place for them anyway.
>>     
>
> First chunk for the sys_generic.c about ibits/obits looks like a bug fix ?
> If yes, it probably would make sense to commit it separately to be able
> to MFC it.
>
> The same note about chunks that remove #include <compat/ia32...>, if
> possible ?
>   
It is a bug fix, but one that only matters on big-endian systems 
(swizzle_fdbits needs it defined), and so goes into the 
fixes-for-big-endian bucket. Disentangling all of this would be pretty 
difficult, and most of the changes are pointless without their companion 
changes. Some of the big endian bits could be pulled out, I guess, but 
I'm not completely sure what the point of separately MFCing them is.