Re: [PATCH V2] scsi:spraid: initial commit of Ramaxel spraid driver

[Hannes Reinecke <hare@suse.de>]

On 11/30/21 12:38 PM, Yanling Song wrote:
> On Mon, 29 Nov 2021 14:04:12 +0100
> Hannes Reinecke <hare@suse.de> wrote:
> 
>> On 11/26/21 8:33 AM, Yanling Song wrote:
>>> This initial commit contains Ramaxel's spraid module.
>>>
>>> Signed-off-by: Yanling Song <songyl@ramaxel.com>
>>>
>>> Changes from V1:
>>> 1. Use BSG module to replace with ioctrl
>>> 2. Use author's email as MODULE_AUTHOR
>>> 3. Remove "default=m" in drivers/scsi/spraid/Kconfig
>>> 4. To be changed in the next version:
>>>     a. Use get_unaligned_be*() in spraid_setup_rw_cmd();
>>>     b. Use pr_debug() instead of introducing dev_log_dbg().
>>>
>>> ---
>>>   Documentation/scsi/spraid.rst     |   16 +
>>>   MAINTAINERS                       |    7 +
>>>   drivers/scsi/Kconfig              |    1 +
>>>   drivers/scsi/Makefile             |    1 +
>>>   drivers/scsi/spraid/Kconfig       |   10 +
>>>   drivers/scsi/spraid/Makefile      |    7 +
>>>   drivers/scsi/spraid/spraid.h      |  693 ++++++
>>>   drivers/scsi/spraid/spraid_main.c | 3328
>>> +++++++++++++++++++++++++++++ 8 files changed, 4063 insertions(+)
>>>   create mode 100644 Documentation/scsi/spraid.rst
>>>   create mode 100644 drivers/scsi/spraid/Kconfig
>>>   create mode 100644 drivers/scsi/spraid/Makefile
>>>   create mode 100644 drivers/scsi/spraid/spraid.h
>>>   create mode 100644 drivers/scsi/spraid/spraid_main.c
>>>   
>> Hmm.
>> This entire thing looks like an NVMe controller which is made to look 
>> like a SCSI controller.
>> It even uses most of the NVMe structures.
>> And from what I've seen there is not much SCSI specific in here; I/O
>> and queue setup is pretty much what every NVMe controller does.
>> So why not make it a true NVMe controller?
>> Yes, you would need to discuss with the NVMe folks on how a RAID 
>> controller should look like in NVMe terms.
>> But overall I guess the driver would be far smaller and possibly
>> easier to maintain.
>>
>> So where's the benefit having it as a SCSI driver (apart from the
>> fact that is allows you to side-step having to discuss the interface
>> with NVMexpress.org ...)?
>> Or, to put it the other way round: Is there anything SCSI specific
>> which would prevent such an approach?
>>
> 
> Actually it is a SCSI driver, and it does register a scsi_host_template
> and host does send SCSI commands to our raid controller just like other
> raid controllers. You are right "it looks a lot like NVMe" since we
> believe the communication mechanism of NVME between host and the end
> device is good and it was leveraged when we designed the raid
> controller. That's why it looks like there are some code from NVME
> because the mechanism is the same.
> 
Thank you, but that was precisely my question.

Seeing that the driver is using the NVMe mechanism to communicate
commands between the driver and the hardware, doesn't it make it a NVMe
driver?
Especially as you are sending NVMe commands and not SCSI commands, so
you always will have to re-write the incoming SCSI commands into NVMe
commands, and knowing from experience this is not a good fit.

Hence my question: what exactly is SCSI specific on the hardware side?
Wouldn't an implementation as a NVMe driver be a better fit, as then you
could leverage all the existing code like setup prps, completion
handling etc?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer