[ofw] crash in mlx4 driver

[Leonid Keller]

Hi Sean,

See my comments inline.

> -----Original Message-----
> From: Sean Hefty [mailto:sean.hefty at intel.com] 
> Sent: Monday, March 16, 2009 8:41 AM
> To: Leonid Keller; Fab Tillier; ofw at lists.openfabrics.org
> Subject: RE: [ofw] crash in mlx4 driver
> 
> >I've added this field (p_hca_obj) for WinVerbs, because it 
> needs it now 
> >for performing QueryCa.
> >Before that WinVerbs was using p_hca_dev field, which was wrong, 
> >because this field contains HCA's PDO.
> 
> As Fab pointed out, the field had a dual use.  

I do not quite agree with that. As far as I remember, it was used only
as PDO before WinVerbs\WinMad came.

>It was set to 
> the HCA object (and not the PDO), when responding to the 
> query interface call.  (Path of minimal
> change.)

Once more, my believe that it was WinVerbs\WinMad, that stored in this
field the pointer to an internal structure of HCA driver.
They should have gotten this pointer by calling open_ca like IBAL does
in create_ci_ca().

> 
> >In fact, WinVerbs can perform QueryCa using the existing 
> interface like 
> >IBAL does.
> >If Sean will change WinVerbs to do that, he can remove this field as 
> >unnecessary.
> 
> I'm not following this.  I defined the RDMA_INTERFACE_VERBS 
> structure for winverbs.  What do you mean by 'perform 
> QueryCa'?  Do you mean obtain this interface?

I mean, that WinVerbs\WinMad were using this field (p_hca_dev) to
perform QueryCa.
But QueryCa requires CA handle, so this field was overwritten with the
value of CA handle, which made it to have the "dual usage".
The right way was not to touch this field at all, but to perform OpenCa,
which returns the needed handle.
I added a new field (p_hca_obj) for QueryCa, leaving this change in
WinVerbs\WinMad for you.
But if you agree with the above and have no time for that, I can change
WinVerbs\WinMad and remove this field back.
It is really unnecessary.

> 
> Now that IBAL is in the same device stack as the HCA, does it 
> need the PDO exchanged through the ci_interface_t?
> 
I believe - no, but it is used now in bus_port_mgr, bus_iou_mgr and
IBAL.
If we store PDO in IBAL CA object, we can remove it from the interface.
Fab ?

> My preference is to undo the change to struct 
> RDMA_INTERFACE_VERBS, but if it must be kept for some reason, 
> then at the very least the GUID should be updated.
> This would cause incompatible builds of the drivers to fail 
> to obtain the interfaces.

> 
> - Sean
> 
>