[ofw] WDK build environment migration thoughts

[Sean Hefty]

1. __ptr64 problem

Briefly speaking, this problem arises when copying 32bit len pointer into 64bit
len pointer. In this case, signed pointer extension will take place.

How it's applicable to WinOF ?  A lot of pointer were declared to be __ptr64
(i.e., to be always "long", even in 32bit kernel systems), that's to preserve on
unique size of structs used in IOCTL calls.  The main problem it will cause is
between 32bit user applications and 64bit kernel application.

When user code do operation like 

s_ptr = &my_struct;

my_type* __ptr64 ptr = s_ptr;

Than kernel will receive ptr with invalid upper bits data (4 bytes FF).

To avoid signed pointer extension, PtrToPtr64() function should be used.

Also, I found some other places where dangerous signed pointer extension took
place, even on 32bit kernel.

Yet another problem that arises with __ptr64 attribute is internal compiler
error (C1001)  in WDK when using __ptr64 pointer to function (callback)

This problem was described in ofw discussion, you can see also :

 <http://blogs.msdn.com/texblog/archive/2005/10/31/487436.aspx>
http://blogs.msdn.com/texblog/archive/2005/10/31/487436.aspx

 <http://lists.openfabrics.org/pipermail/ofw/2007-July/001613.html>
http://lists.openfabrics.org/pipermail/ofw/2007-July/001613.html (posted by Jan
from OFW)

Our solution:

1. Initially, we decided to remove all __ptr64 attributes except those ones
inside IOCTL structures. After, put PtrToPtr64() conversion on every assignment
to long pointer.

(like my_type* __ptr64 ptr = PtrToPtr64(s_ptr);  )

During this solution, we changed a huge amount of code, so patch became
unreadable. And it was difficult to validate that all long pointer (with __ptr64
attribute) were used in a proper manner

2. So, we decided about another solution:

 All __ptr64 occurrences were replaced by either:

 i) TO_LONG_PTR(type, field) macro, when occurred inside structure

ii) VOID_PTR64 macro otherwise (defined as void macro)

#define CONCAT(str1, str2) str1##str2

#define TO_LONG_PTR(type,member_name) \

    union { type member_name;  uint64_t CONCAT(member_name,_padding) ; }

 

Unless CONCAT is used elsewhere, I would not add that #define.  It's too generic
of a name.

Does TO_LONG_PTR work for both big endian and little endian systems?  How is the
padded space initialized?

 

Thus, we can both preserve on a uniform shapes of structs in user and kernel and
to avoid unsafe pointer arithmetic !

The patch now is much more readable, but it sill consist of thousands lines.

 

IBAL uses typedefs everywhere.  Why couldn't the typedefs be redefined as
'UINT64' instead of struct _blah __ptr64?  Compiled code shouldn't notice the
difference, and we'd get compiler warnings for any casting errors, that we could
then clean up.  It doesn't seem like this would produce a patch that's any
bigger or more difficult to read, but the resulting code would be cleaner than
introducing unnamed unions with potential padding that requires additional
initialization.

 

2. Migration to WDK

Main issue here was to preserve on backward compatibility with DDK

 

Personally, I don't see the need to be backward compatible with the DDK.  Let's
just require the latest WDK to build the drivers.

- Sean

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