[Openib-windows] [PATCH] 2 new vendor calls

[Leonid Keller]

see below

> -----Original Message-----
> From: Fab Tillier [mailto:ftillier at silverstorm.com]
> Sent: Friday, September 23, 2005 9:22 AM
> To: 'Leonid Keller'
> Cc: openib-windows at openib.org
> Subject: RE: [Openib-windows] [PATCH] 2 new vendor calls
> 
> 
> > From: Leonid Keller [mailto:leonid at mellanox.co.il]
> > Sent: Thursday, September 22, 2005 1:09 AM
> > 
> > Hi Fab,
> > 
> > Thank you for your comments.
> > 
> > First of all, the problem with unmapping is in need in 
> releasing of the MDL,
> > not in unmapping itself.
> 
> Right, the problem is getting a pointer to the MDL, for which 
> we shouldn't trust
> the user-mode program.
> 
> > You are right about security and resource tracking: i saw 
> these problems, but
> > didn't find a simple solution.
> > The right solution IMO would have been to add the context 
> of FW_MAP_CRSPACE
> > operation to the process resource tracking information and 
> to perform
> > FW_UNMAP_CRSPACE automatically upon process termination.
> > 
> > Then we could remove the context from FW_MAP_CRSPACE and not provide
> > FW_UNMAP_CRSPACE at all.
> 
> You still want to allow FW_UNMAP_CRSPACE, rather than relying 
> on process
> termination to drive that.  

???
Or my English is wrong or i'm missing here something.
I thought, that i've suggested to perform the unmapping on process
termination, which would allow us not to expose FW_UNMAP_CRSPACE at all.
Is it right thing to do - is another question.
We have a lot of paired APIs like FW_OPEN_IF/FW_CLOSE_IF or
create_qp/destroy_qp.
Why shouldn't we provide here also a paired one ?

> The cleanup in case of abnormal process usage is
> necessary but shouldn't be the norm.
Yes, but, first, we has to do it there anyway, second, you suggest below to
do it on CA cleanup anyway.

> 
> > Your idea of putting the context into CA object seams to me 
> troublesome:
> >         - it prevents several applications to use 
> simultaneously this
> > mechanism (on the same HCA);
> 
> Do we want to allow multiple applications simultaneous access 
> to the FW?  I
> don't think so - it creates the possibility for two different 
> sessions trying to
> update the FW simultaneously, which could result in a corrupt 
> FW image.  
It's - responsibility of the customers (see more reasons below).
We have to warn them wrt that.
[BTW, customer can burn into HCA a corrupt or wrong image - it's a normal
and not too dangerous situation.
He can then fix that by burning the card with one application and a right
image]


> The FW access should be serialized to a single client at a time.

A burning application is just one tool.
There is DevMon, which is mainly used to look at CR space, but allows also
to change it.
There is also trace.exe, which reads debug prints of the FW from the card.
These two applications are highly used by developers and sometimes
simultaneously.
There are or can be also another card management applications, some of them
having additional equirements like mapping DDR or even UAR.
We are talking about tools for developers.
Customer will get a small subset of them.

That's why I don't think, that we may restrict the access to one application
at a time.


> >         - if an application was killed and restarted, there 
> will be a memory
> > leak.
> 
> No, when an application is killed, it's CA object is cleaned 
> up due to the
> resource tracking.  If that CA object tracks the FW CR space 
> mapping, then that
> can get cleaned up automatically.

Nice, just we need to do it in the resource tracking code for not to impose
the above restriction.

> 
> > I don't know enough how the resource tracking mechaism 
> works today and whether
> > there is a simple way to implement the above solution.
> > 
> > Could you enlight me on this ?
> 
> Currently, the user's CA object (the mlnx_um_ca_t) isn't 
> passed into the calls.
> We probably need to change things so that any user's call to 
> ib_open_ca results
> in some call to the HCA driver to establish a user context.  
> Currently, this
> only happens for user-mode clients, 

Are our tools applications not user-mode clients ?

> and that user-mode context isn't used for
> any of the other verbs.  If we can fix things to use a 
> per-user CA context in
> all the verbs calls, then resource cleanup will be handled in 
> the proxy.

Can we pass mlnx_um_ca_t structure to the vendor call ?
If it isn't too complicate, then i'd suggest the following "ultimate"
solution,
which takes in mind future extensions and various ideas, i got so far:

   1. User interface	
	- we provide both FW_MAP_CRSPACE and FW_UNMAP_CRSPACE calls;
	- they will have the same parameter structure:
		struct _FW_MAP_PARAMS {
				PVOID __ptr64 va;	/* mapped address;
OUT for map, IN for UNMAP */
			IN	ULONG	bar_num;	/* 0 - CR, 1 - UAR,
2 - DDR */
			IN	ULONG offset;	/* map/unmap from this
offset */
			IN 	ULONG	size;		/* map/unmap this
size */
		};
	- return codes:
		STATUS_NOT_IMPLEMENTED		- for UAR and DDR space (for
now);	
		STATUS_BUFFER_TOO_SMALL		- not enough parameters;

		STATUS_INVALID PARAMETERS	- wrong parameters;
		STATUS_NONE_MAPPED		- unmap parameters do not
match map ones;	


   2. Implementation
	- we add necessary fielsd to mlnx_um_ca_t;
	- the structure is passed to FW_MAP_CRSPACE/FW_UNMAP_CRSPACE;

	- FW_MAP_CRSPACE store the context; FW_UNMAP_CRSPACE use it for
unmapping;
	- we add resource tracking code, that performs cleanup on appl.
termination.

What do you think ?

> 
> - Fab
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20050925/4f56f6ab/attachment.html>