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

[Kostik Belousov]

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 ?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-amd64/attachments/20100310/0329fdd9/attachment.pgp