Project

General

Profile

Actions

Bug #14783

closed

pvscsi modernization

Added by Garrett D'Amore over 1 year ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Category:
driver - device drivers
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Expert
Tags:
Gerrit CR:
External Bug:

Description

The pvscsi driver used with VMware was unfortunately designed to some legacy APIs, and we have observed some issues with it.

  • It's support for hot plug support is "limited" -- we haven't observed that it actually works at all despite the code present (and it has not support for removal)
  • It can run out of resources (contexts) and issues warnings on the console when this occurs.
  • In reset circumstances, it can crash due to a circular lock associated with the completion ring handling

Additionally:

1. The driver should be updated to support modern SCSA DMA interfaces (tran_setup_pkt() and friends)
2. The driver should use the iport(9) interfaces for creating target ports
3. In theory the driver could benefit from LUN notifications (though we don't know if any hypervisors support this, the interface allows for non-zero lun notifications)
4. It should be moved out of intel/ - because VMWare runs on Aarch64, and aarch64 support is anticipated to be added to illumos (some folks have made it work already)
5. It should use correct DMA synchronization methods instead of explicit memory barriers (which are actually more expensive and normally not needed)
6. The DMA attributes need to be corrected -- apart from page alignment considerations some of the other restrictions (e.g. to the lower 32-bits of memory) are actually incorrect.
7. The support for quiescing the SCSI bus is of zero value, and adds a lot of extra complexity -- modern SAS and virtual busses do not need to quiesce SCSI busses (quiesce was created to allow legacy parallel busses to be electrically quiesced so that they could be made safe for configuration changes -- modern busses are hot-plug safe)
8. The attempt to use separate locks for the completion ring and submission rings is misguided -- likely based on thinking from NIC drivers. HBAs move a lot more data per operation, and spend a lot less time holding these locks when interacting with these paths, and the simplicity from a single ring brings clear sustainability benefits without any measurable impact on performance.

When all these changes are made, the pvscsi can be reduced to about 60% of it's original size, with no loss in functionality, and several bugs are fixed along the way. This work can be easily done in a way that the pvscsi driver becomes 100% ddi compliant.


Related issues

Related to illumos gate - Bug #14789: DDI should be more hrtime friendlyNew

Actions
Actions #1

Updated by Electric Monk over 1 year ago

  • Gerrit CR set to 2214
Actions #2

Updated by Patrick Mooney over 1 year ago

  • Related to Bug #14789: DDI should be more hrtime friendly added
Actions #3

Updated by Garrett D'Amore over 1 year ago

I've tested with an ESXi 7.0 server. It has been tested with multiple logical disks, including hot attach (but I could not figure out how to get VMware to let me detach a disk from the guest), and also with RDM (scsi device pass through really), including RDM of a disk imported via fibre channel to the guest.

Testing included boot, dump, etc. creation of pools, verification of the content, etc. A fair amount of load was put on the system to verify it under stress.

Actions #4

Updated by Garrett D'Amore over 1 year ago

  • Status changed from In Progress to Pending RTI
Actions #5

Updated by Electric Monk over 1 year ago

  • Status changed from Pending RTI to Closed
  • % Done changed from 80 to 100

git commit c0586b874d9179e81ca8a124fa6caf98fddb7696

commit  c0586b874d9179e81ca8a124fa6caf98fddb7696
Author: Garrett D'Amore <garrett@damore.org>
Date:   2022-07-11T02:43:50.000Z

    14783 pvscsi modernization
    Reviewed by: Yuri Pankov <ypankov@tintri.com>
    Reviewed by: Patrick Mooney <pmooney@pfmooney.com>
    Reviewed by: Gordon Ross <gordon.w.ross@gmail.com>
    Reviewed by: Jerry Jelinek <gjelinek@gmail.com>
    Approved by: Richard Lowe <richlowe@richlowe.net>

Actions

Also available in: Atom PDF