[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