[ofw] WDK build environment migration thoughts

[Tzachi Dar]

Please note that the reason Alex did things in the order mentioned is
because of the size of his patch. He had to go all over the code, and
look for __ptr64 and there have been more than 600 directs instances of
it and many others were hidden in typedefs.
This forced Alex to use a very formal way of fixing things.
 
FUNC_PTR64 and VOID_PTR64 are defined as nothing. They are only comments
that help us check that Alex did a good job.
One day or the other they should be removed. You can think of them as
comments.
 

> What does sizeof(qp) return?

It will return 4 on 32 bits systems and 8 on 64 systems.

 

I believe that compatibility is broken in the user-user and
kernel-kernel level on 32 bits systems. This is because at the past
functions that have received pointers received them as __ptr64 which
means that the size of the pointer was 64 bits. Today they will be 32
bits only. The portability in the move between user to kernel still
exists as the structs in the ioctls are still 64 bits.

 
Please also note that if we want to support little endian machines as
well as big endian machines than only one Macro has to change. This
should put the padding before the value and not after it.
 
Thanks
Tzachi


________________________________

	From: ofw-bounces at lists.openfabrics.org
[mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Sean Hefty
	Sent: Monday, April 21, 2008 10:48 PM
	To: Alex Naslednikov; Smith, Stan; Ishai Rabinovitz
	Cc: ofw at lists.openfabrics.org
	Subject: RE: [ofw] WDK build environment migration thoughts
	
	

	What exactly are FUNC_PTR64 and VOID_PTR64?  Given:

	 

	typedef struct _ib_qp* VOID_PTR64 ib_qp_handle_t;

	ib_qp_handle_t             qp;

	 

	What does sizeof(qp) return?

	 

	As for the steps, I'm missing something.  Step 1 looks like it
breaks compatibility.  Step 3 looks like it undoes what step 1 did, so
I'm not sure why step 1 was done.  Were these just intermediate steps
that you used to create the actual patch?

	 

	 

	 

	
________________________________


	From: Alex Naslednikov [mailto:alexn at mellanox.co.il] 
	Sent: Monday, April 21, 2008 12:31 PM
	To: Hefty, Sean; Smith, Stan; Ishai Rabinovitz
	Cc: ofw at lists.openfabrics.org
	Subject: RE: [ofw] WDK build environment migration thoughts

	 

	Question: It's still not clear to me how you maintain binary
compatibility with the IBAL APIs.  The IBAL calls take 64-bit pointers
as input parameters.  What are the definitions for the ib_blah_handle_t
types?

	 

	We did the following (major steps):

	1. Replace all ib_<type>_handle_t types by its direct
invocation, say struct _ib_<type>_ *

	2. Next, replace all __ptr64 either by TO_LONG_PTR macro (if
used inside struct definition), or one of void macros (FUNC_PTR64,
VOID_PTR64 etc) otherwise

	Typedefs now looks like : typedef struct _ib_qp* VOID_PTR64
ib_qp_handle_t;

	3. Next, replace back all appearances of (struct ib_<type>_*) by
ib_<type>_handle_t inside function declarations (because of "const"
modifier problems)

	4. Several other minor steps

	 

	XaleX

	 

	
________________________________


	From: Sean Hefty [mailto:sean.hefty at intel.com] 
	Sent: Monday, April 21, 2008 9:01 PM
	To: Alex Naslednikov; Smith, Stan; Ishai Rabinovitz
	Cc: ofw at lists.openfabrics.org
	Subject: RE: [ofw] WDK build environment migration thoughts

	Q2.Does TO_LONG_PTR work for both big endian and little endian
systems?

	TO_LONG_PTR works only with little endian, because we do not
support PPC platform 

	Currently, there's no need to extend it to support big endian

	 

	OFA does support PPC platforms, plus Itanium can be configured
for either little or big endian format.  I'm fine deferring adding this
support, but the code should not assume that it will always be little
endian.

	 

	 

	Q3. How is the padded space initialized?

	Inside sdp, all padded space was initialized by class
constructor (except one specific case)

	In all other code, that not C++, padded space was initialized by
cl_memclr before setting the field and before calling to ioctl procedure

	Please, see my next mail with examples from the code

	 

	It's kind of ugly to require setting padded fields to specific
values.  This only needs to be done when crossing from a 32-bit to
64-bit boundary, so we can restrict this to the kernel proxy.  (Btw, you
can replace the unnamed union and add padding to structures only if
compiling in 32-bit mode.)

	 

	It's still not clear to me how you maintain binary compatibility
with the IBAL APIs.  The IBAL calls take 64-bit pointers as input
parameters.  What are the definitions for the ib_blah_handle_t types?

	 

	- Sean

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20080421/35c9dfd3/attachment.html>