[Openib-windows] RE: User verbs

Leonid Keller leonid at mellanox.co.il
Thu Mar 2 10:22:19 PST 2006


Hi, Fab

I send you a patch, regarding the calling of create_cq and alloc_pd with
appropriate CA handle.
The patch contains a little fix in al_verbs.h and appropriate changes in
the driver, so that both drivers - the current and the new - can work
with the same IBAL. 
In some days I'm going to change the implementation of tools support
(=burning) to make it safe. I'll look through your comments on my first
omplementation and check your suggestion about passing UM CA handle to
vendor_call, so it can save resource_tracking info therein.


Index: E:/svn.wininf/branches/core/al/al_verbs.h
===================================================================
--- E:/svn.wininf/branches/core/al/al_verbs.h (revision 632)
+++ E:/svn.wininf/branches/core/al/al_verbs.h (revision 956)
@@ -71,8 +71,9 @@
   port_num, ca_mod, p_port_attr_mod )
 
 #define verbs_create_cq(h_ca, p_cq_create, h_cq) \
- h_ca->obj.p_ci_ca->verbs.create_cq( h_ca->obj.p_ci_ca->h_ci_ca,\
- h_cq, &p_cq_create->size, &h_cq->h_ci_cq, p_umv_buf )
+ h_ca->obj.p_ci_ca->verbs.create_cq( \
+  (p_umv_buf) ? h_ca->h_um_ca : h_ca->obj.p_ci_ca->h_ci_ca, \
+  h_cq, &p_cq_create->size, &h_cq->h_ci_cq, p_umv_buf )
 
 #define verbs_check_cq(h_cq) ((h_cq)->h_ci_cq)
 #define verbs_destroy_cq(h_cq) \
@@ -169,8 +170,9 @@
   h_qp->h_ci_qp, p_mw_bind, p_rkey )
 
 #define verbs_allocate_pd(h_ca, h_pd) \
- h_ca->obj.p_ci_ca->verbs.allocate_pd(\
-    h_ca->obj.p_ci_ca->h_ci_ca, h_pd->type, &h_pd->h_ci_pd, p_umv_buf )
+ h_ca->obj.p_ci_ca->verbs.allocate_pd( \
+ (p_umv_buf) ? h_ca->h_um_ca : h_ca->obj.p_ci_ca->h_ci_ca, \
+  h_pd->type, &h_pd->h_ci_pd, p_umv_buf )
 
 /*
  * Reference the hardware PD.


Index: S:/svn.wininf/trunk/hw/mt23108/kernel/hca_data.h
===================================================================
--- S:/svn.wininf/trunk/hw/mt23108/kernel/hca_data.h	(revision 439)
+++ S:/svn.wininf/trunk/hw/mt23108/kernel/hca_data.h	(revision 1031)
@@ -212,7 +212,7 @@
 	HH_hca_hndl_t		hh_hndl;
 	HH_hca_dev_t		dev_info;
 	uint8_t				ul_hca_res[1];	// Beginning of
UL resource buffer.
-
+	mlnx_hob_t				*hob_p;
 }	mlnx_um_ca_t;
 
 typedef struct {
 

Index: S:/svn.wininf/trunk/hw/mt23108/kernel/hca_verbs.c
===================================================================
--- S:/svn.wininf/trunk/hw/mt23108/kernel/hca_verbs.c	(revision 401)
+++ S:/svn.wininf/trunk/hw/mt23108/kernel/hca_verbs.c	(revision 1031)
@@ -582,6 +582,7 @@
 	{
 		/* Copy the dev info. */
 		p_um_ca->dev_info = *hca_ul_info;
+		p_um_ca->hob_p = hob_p;
 		*ph_um_ca = (ib_ca_handle_t)p_um_ca;
 		(*(void** __ptr64)p_umv_buf->p_inout_buf) =
p_um_ca->p_mapped_addr;
 		p_umv_buf->status = IB_SUCCESS;
@@ -644,7 +645,7 @@
 		OUT			ib_pd_handle_t
*ph_pd,
 	IN	OUT			ci_umv_buf_t
*p_umv_buf )
 {
-	mlnx_hob_t				*hob_p = (mlnx_hob_t
*)h_ca;
+	mlnx_hob_t				*hob_p;
 	mlnx_hobul_t			*hobul_p;
 	HH_hca_dev_t			*hca_ul_info;
 	HHUL_pd_hndl_t			hhul_pd_hndl = 0;
@@ -655,6 +656,11 @@
 
 	CL_ENTER(MLNX_DBG_TRACE, g_mlnx_dbg_lvl);
 
+	if( p_umv_buf && p_umv_buf->command ) 
+		hob_p = ((mlnx_um_ca_t *)h_ca)->hob_p;
+	else 
+		hob_p = (mlnx_hob_t *)h_ca;
+	
 	hobul_p = mlnx_hobs_get_hobul(hob_p);
 	if (NULL == hobul_p) {
 		status = IB_INVALID_CA_HANDLE;
@@ -1889,7 +1895,7 @@
 {
 	ib_api_status_t			status;
 
-	mlnx_hob_t				*hob_p = (mlnx_hob_t
*)h_ca;
+	mlnx_hob_t				*hob_p;
 	u_int32_t				cq_idx;
 	u_int32_t				cq_num;
 	u_int32_t				cq_size = 0;
@@ -1901,6 +1907,11 @@
 
 	CL_ENTER(MLNX_DBG_TRACE, g_mlnx_dbg_lvl);
 
+	if( p_umv_buf && p_umv_buf->command ) 
+		hob_p = ((mlnx_um_ca_t *)h_ca)->hob_p;
+	else 
+		hob_p = (mlnx_hob_t *)h_ca;
+
 	hobul_p = mlnx_hobs_get_hobul(hob_p);
 	if (NULL == hobul_p) {
 		status = IB_INVALID_CA_HANDLE;



> -----Original Message-----
> From: Fab Tillier [mailto:ftillier at silverstorm.com] 
> Sent: Wednesday, January 11, 2006 7:19 PM
> To: Leonid Keller
> Subject: RE: User verbs
> 
> > From: Leonid Keller [mailto:leonid at mellanox.co.il]
> > Sent: Wednesday, January 11, 2006 8:45 AM
> > 
> > > From: Fab Tillier [mailto:ftillier at silverstorm.com]
> > > Sent: Tuesday, January 10, 2006 8:05 PM
> > >
> > > Hi Leo,
> > >
> > > > From: Leonid Keller [mailto:leonid at mellanox.co.il]
> > > > Sent: Tuesday, January 10, 2006 8:45 AM
> > > >
> > > > Hi Fab !
> > > >
> > > > Working on porting of the user part of the MTHCA driver i came 
> > > > accross the following question:
> > > > Why do you use h_ca (kernel HCA handle) and not h_um_ca 
> (context, 
> > > > returned by mlnx_um_open) in calls to call mlnx_allocate_pd and 
> > > > mlnx_create_cq ?
> > >
> > > The problem comes from the fact that mlnx_um_open isn't 
> called for 
> > > all clients (not kernel ones), so passing h_um_ca to the 
> verbs calls 
> > > would break kernel clients.
> >
> > You didn't understand my suggestion, I guess.
> 
> I had totally missed it!
> 
> > It's quite natural, that user clients have their own _um_open call.
> > It's just an old good work paradigm: h = open(); verb(h); 
> ...; close(h).
> > Pay attention, that h_ca and h_um_ca have the same type 
> ib_ca_handle_t.
> > What I'm suggesting is to call alloc_pd and create_cq with h_ca for 
> > kernel clients and with h_um_ca - for user ones. The verbs 
> will choose 
> > the right casting according to the value of p_umv_buf.
> 
> But this creates an inconsistency with other verbs, which may 
> also benefit from having the h_um_ca.  ci_call is just an 
> example of a call that would benefit, as it would allow the 
> HCA driver to store any CR space mapping information in the 
> h_um_ca structure, making CR space mapping into UM safe.
> 
> So, I'm OK with changing all the verbs to check p_umv_buf and 
> then doing the proper cast.  However, in light of the 
> direction I expect the verbs to go in (where the user 
> multiplexing on open is done in the HCA driver and not IBAL), 
> having all open calls result in some sort of user open call 
> and then using that handle exclusively in all verb calls 
> would align better.
> 
> That is, rename um_open_ca to user_open_ca, and call it for 
> all clients.  For kernel clients, p_umv_buf would be NULL and 
> the HCA driver would know not to try to map resources into 
> user-mode.  Then, every call changes to take the h_um_ca.
> 
> Anyhow, whether we change every call to check p_umv_buf, or 
> change um_open_ca and change the type of the structure for 
> every other verb is pretty much equivalent.  Plus it lets us 
> merge the CR space mapping stuff in time for the OpenIB 1.0 release.
> 
> What do you think?
> 
> - Fab
> 
> 



More information about the ofw mailing list