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:[EMAIL PROTECTED]
Sent: Thursday, April 10, 2008 9:26 PM
To: Leonid Keller; Fab Tillier; Reuven Amitai; [email protected]
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: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Leonid Keller
Sent: Wednesday, April 09, 2008 11:59 PM
To: Fab Tillier; Reuven Amitai; [email protected]
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: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Fab Tillier
Sent: Wednesday, April 09, 2008 7:44 PM
To: Reuven Amitai; [email protected]
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: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Reuven Amitai
Sent: Wednesday, April 09, 2008 6:04 AM
To: [email protected]
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
mthca_vc.patch
Description: mthca_vc.patch
_______________________________________________ ofw mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
