[nvmewin] ***UNCHECKED*** RE: a few defects... updated code attached

[Luse, Paul E]

Was able to start testing over the weekend.  No new issues found so please start you review (complete set attached, compare against last applied tag, 'prp_list_count_bug_fix').  Password is intel123

I'll ping key folks for feedback if I don't hear anything in the next week or two.  Again, the fixes are not super complicated so I won't schedule a review meeting unless someone requests it -- if anyone wants to, that's OK, just ask.

Thanks
Paul

From: Luse, Paul E
Sent: Saturday, June 23, 2012 12:41 PM
To: nvmewin at lists.openfabrics.org
Subject: RE: a few defects...

So I finished creating my patch, just need to test, so will send out it on Mon.  It's a few hundred edits so I'm not going to list them one by one but here are the fixes/changes and it will be obvious when you diff them which edit goes with which item:

- report luns still had an issue in the format of the response data, the actual LUN # was 1 byte off so if you really had multiple LUNS, storport wouldn't scan them

- the NVME_CC_IOSQES and CQ definitions in nvmereg.h were wrong (should be 6 and 4 respectively)

- double buffering code needed a few tweaks if anyone tried to enable that they would have found out (tweaks based on changes made since it was originally coded).  In addition, I ran into some HW that needed the PRP2 list pointer 4K aligned (part of debug) as well as the host data buffers so I added code to allocate one page of aligned memory per IO for PRP lists.  For various reasons (mainly mem consumption) I extended the DBL_BUFF define to include some other things that dumb down the driver for HW bring up and debug.  So, I renamed the compile switch to DUMB_DRIVER and when you define it you create a very simple driver via eliminating the following (in addition to making everything 4K aligned)
-- learning mode
-- NUMA and INT steering (limits to just one QP now)

- small bug in get/set features portion of init state machine where we don't do both set & get on all LUNs

- we didn't support DB strides in the driver - they are defined in the spec and yes there's a paragraph about these being useful in SW simulations, etc, but they can also be useful for other reasons as well.  The changes are scattered through a few files and the formulas that you see for calc'ing DB locations come right out of the spec.  In the nvmereg header there were  changes to how we dinfed the structs that overlay the BAR; since strides are between DBs and not QPs I had to eliminate a few QP specific structs and make some generic ones to allow a variable sized array of generic (head or tail) DB register structs to overlay however big the BAR is when mapped into mem in FindAdapter.  Before we had an array of like 1000 QP structs which doesn't work for strides and didn't make sense given everyone's HW will have different sized BARs (I'm assuming)

- we weren't reading the upper 32 of CAP and failed to honor MQES (max Q entries supported by HW).  I moved reading of CAP to findAdapter to we could do this and also keep a copy in the devExt now as well

- we weren't honoring what the ctrl told us about the vol write cache get/set commands in the ID data.  We sent them (via SCSI xlation) even if the card says it can't support the commands (which is odd, I'd think we'd want the card to accept all commands and just tell us when not supported but that's not how the spec reads). So, there are SNTI changes to not send these commands on mode sense/select for the cache page and instead hard code a page back that says 'no write cache'

- a few misc chatham updates, won't go into those.  Next patch I'll remove the CHATHAM define and all its associated code, only need it for a few more weeks

Thx
Paul
From: Luse, Paul E
Sent: Monday, June 18, 2012 12:05 PM
To: nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>
Subject: a few defects...


Just an FYI in case anyone is running into these.  We have fixes for them and will work on getting a patch in either this week or next (want to wait til Ray is able to get the last one applied following the server change).

- report luns still had an issue in the format of the response data, the actual LUN # was 1 byte off so if you really had multiple LUNS, storport wouldn't scan them

- the NVME_CC_IOSQES and CQ definitions in nvmereg.h were wrong (should be 6 and 4 respectively)

- double buffering code needed a few tweaks if anyone tried to enable that they would have found out (tweaks based on changes made since it was originally coded)

- small bug in get/set features portion of init state machine where we don't do both set & get on all LUNs

____________________________________
Paul Luse
Sr. Staff Engineer
PCG Server Software Engineering
Desk: 480.554.3688, Mobile: 480.334.4630

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20120625/79e4ed5c/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: source.zip
Type: application/x-zip-compressed
Size: 160673 bytes
Desc: source.zip
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20120625/79e4ed5c/attachment.bin>