[ofw] RE: [PATCH] mthca patch [1/2] - vstat uplink info print

[Reuven Amitai]

Hi,
 
This is more clean version of the patch.
The uplink info print was nothing due to pointer wrong location.
ca_attr buffer layout demands PTR_ALIGN of every field of vendor
specific, (after the mandatory fields of ca_attr).
 
Reuven.
 
Index: hw/mthca/kernel/hca_verbs.c

===================================================================

--- hw/mthca/kernel/hca_verbs.c     (revision 1047)

+++ hw/mthca/kernel/hca_verbs.c     (working copy)

@@ -43,7 +43,6 @@

 #include "mx_abi.h"

 #include "mt_pa_cash.h"

 

-#define PTR_ALIGN(size)      (((size) + sizeof(void*) - 1) &
~(sizeof(void*) - 1))

 

 

 // Local declarations

Index: inc/mthca/mthca_vc.h

===================================================================

--- inc/mthca/mthca_vc.h      (revision 1047)

+++ inc/mthca/mthca_vc.h      (working copy)

@@ -33,6 +33,8 @@

 #ifndef MTHCA_VC_H

 #define MTHCA_VC_H

 

+#define PTR_ALIGN(size)      (((size) + sizeof(void*) - 1) &
~(sizeof(void*) - 1))

+

 typedef

 struct _map_crspace {

      unsigned __int64  va;         /* address of CRSPACE, mapped to
user space */

@@ -78,12 +80,12 @@

 

 inline char* mthca_get_board_id(ib_ca_attr_t *ca_attr)

 {

-    return (char*)(ca_attr)+(ca_attr->size - MTHCA_BRD_ID_LEN -
sizeof(uplink_info_t));

+     return (char*)(ca_attr) + (ca_attr->size -
PTR_ALIGN(MTHCA_BRD_ID_LEN) - PTR_ALIGN(sizeof(uplink_info_t)));

 }

 

 inline void* mthca_get_uplink_info(ib_ca_attr_t *ca_attr)

 {

-    return (char*)(ca_attr)+(ca_attr->size - sizeof(uplink_info_t));

+     return (char*)(ca_attr) + (ca_attr->size -
PTR_ALIGN(sizeof(uplink_info_t)));

 }

 

 #endif

 

 


________________________________

From: Sean Hefty [mailto:sean.hefty at intel.com] 
Sent: Thursday, April 10, 2008 9:26 PM
To: Leonid Keller; Fab Tillier; Reuven Amitai; ofw at lists.openfabrics.org
Subject: RE: [ofw] RE: [PATCH] mthca patch [1/2]



Just committing is not the right process to follow.  This should not be
committed until the issue that Fab pointed out is addressed.  The size
is being rounded down, which at least to me looks like the returned
pointers are referencing the wrong memory.

 

>From what you say, this fixes a print.  Why is there a rush to commit a
patch just for a print?

 

________________________________

From: ofw-bounces at lists.openfabrics.org
[mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Leonid Keller
Sent: Wednesday, April 09, 2008 11:59 PM
To: Fab Tillier; Reuven Amitai; ofw at lists.openfabrics.org
Subject: RE: [ofw] RE: [PATCH] mthca patch [1/2]

 

Hi Fab,

This patch fixes printing of uplink information in VSTAT and i'm going
to commit it now, because it improves the current state.

But really all the implementation of vendor-specific information in
query_ca is bad.

We need to define a structure, describing all that info and to work with
names, not calculated offsets.

We'll put it on our todo list.

	 

	
________________________________


	From: ofw-bounces at lists.openfabrics.org
[mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Fab Tillier
	Sent: Wednesday, April 09, 2008 7:44 PM
	To: Reuven Amitai; ofw at lists.openfabrics.org
	Subject: [ofw] RE: [PATCH] mthca patch [1/2]

	Hi Reuven,

	 

	What is the problem with the alignment?  The board ID is a
character string, so shouldn't need any special alignment.  Also, you
are rounding the size down - is that the right thing to do?

	 

	What is the layout of the ca_attr buffer, and what are the
alignment requirements for the different structures appended to the end?

	 

	Is MTHCA_BRD_ID_LEN going to cause the uplink info to be
aligned?

	 

	Thanks,

	-Fab

	 

	From: ofw-bounces at lists.openfabrics.org
[mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Reuven Amitai
	Sent: Wednesday, April 09, 2008 6:04 AM
	To: ofw at lists.openfabrics.org
	Subject: [ofw] [PATCH] mthca patch [1/2]

	 

	Hi,

	 

	The following patch align the returned address.

	 

	Thanks, Reuven.

	 

	Index: inc/mthca/mthca_vc.h

	
===================================================================

	--- inc/mthca/mthca_vc.h      (revision 1047)

	+++ inc/mthca/mthca_vc.h      (working copy)

	@@ -78,12 +78,18 @@

	 

	 inline char* mthca_get_board_id(ib_ca_attr_t *ca_attr)

	 {

	-    return (char*)(ca_attr)+(ca_attr->size - MTHCA_BRD_ID_LEN -
sizeof(uplink_info_t));

	+     int size = (ca_attr->size - MTHCA_BRD_ID_LEN -
sizeof(uplink_info_t));

	+     size &= ~(sizeof(void*) - 1);

	+     

	+     return (char*)(ca_attr)+ size;

	 }

	 

	 inline void* mthca_get_uplink_info(ib_ca_attr_t *ca_attr)

	 {

	-    return (char*)(ca_attr)+(ca_attr->size -
sizeof(uplink_info_t));

	+     int size = (ca_attr->size - sizeof(uplink_info_t));

	+     size &= ~(sizeof(void*) - 1);

	+

	+     return (char*)(ca_attr)+ size;

	 }

	 

	 #endif

	 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20080413/3ef7115f/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mthca_vc.patch
Type: application/octet-stream
Size: 1319 bytes
Desc: mthca_vc.patch
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20080413/3ef7115f/attachment.obj>