[Openib-windows] SRP in x86 machinise

[Fabian Tillier]

Hi Yossi,

On 9/4/06, Yossi Leybovich <sleybo at mellanox.co.il> wrote:
>
> Fab
>
> The patch for the former problem indeed help
> ( I guess that that the fact you are not calling to srp_free_hca solve the
> double freeing, I also did not understand fully why ).
> Still I was left with the LOCAL_PROTECTION_ERR , the reason for that is
> casting issue in 32 bit machines
> Because SRP register kernel memory the cast from 32 bit to 64 bit use sign
> extension and :
> 0x89830000 move to 0xffffffff89830000

Sign extension of kernel virtual memory regions is correct.

> this change solve tyhe problem (its just suggestion but the idea is to force
> the cast to ignore sign of the address)
>
>
> Index: kernel/srp_descriptors.c
> ===================================================================
> --- kernel/srp_descriptors.c (revision 1647)
> +++ kernel/srp_descriptors.c (working copy)
> @@ -91,10 +91,12 @@
>   }
>
>   /* Register the data segments array memory */
> - mr_create.vaddr =
> p_descriptors->p_recv_data_segments_array;
> + mr_create.vaddr = (void*
> __ptr64)(uint64_t)(ULONG_PTR)p_descriptors->p_recv_data_segments_array;

I think the original is right - you want the address to be sign extended.

>   mr_create.length = p_descriptors->recv_descriptor_count *
> p_descriptors->recv_data_segment_size;
>   mr_create.access_ctrl = IB_AC_LOCAL_WRITE;
> @@ -124,7 +126,7 @@
>    p_descriptor->wr.num_ds = 1;
>    p_descriptor->wr.ds_array = p_descriptor->ds;
>
> -  p_descriptor->ds[0].vaddr = (uint64_t)((void* __ptr64)p_data_segment);
> +  p_descriptor->ds[0].vaddr =
> (uint64_t)(ULONG_PTR)p_data_segment;

Again, the original is right - sign extend the pointer and then cast
to uint64.  I remember testing SRP on 32-bit with the MT23108 driver,
so I suspect the problem is in how MTHCA registers memory - it must
miss the sign extension.

Looking at the register_mr path in the MTHCA driver, the call to
ibv_reg_mr (hca_memory.c at 89) is wrong:

mr_p = ibv_reg_mr(ib_pd_p, map_qp_ibal_acl(p_mr_create->access_ctrl),
		p_mr_create->vaddr, p_mr_create->length,
		(uint64_t)(ULONG_PTR)(void*)p_mr_create->vaddr, um_call );

The hca_va parameter (second to last) has the wrong cast to it.
p_mr_create->vaddr is already a 64-bit pointer, so a simple cast to
uint64 should work.  The cast to (ULONG_PTR)(void*) is truncating the
upper 32 bits and then failing to sign extend.

- Fab