Project

General

Profile

Actions
Copy link

Bug #14442

closed

NVMe theory statement on spec version is confusing

Added by Robert Mustacchi 7 months ago. Updated 7 months ago.

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

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
1978

Description

When we implemented #9809 we left in a comment in the driver source that the nvme driver only supported v1.2.1 devices. However, that's not been the case and we've also added a number of features from 1.3 and 1.4. This has caused others who have come to work on the driver to be confused as to what's going on. This improves the comment and manual page to better reflect the current reality.

Actions
Copy link
 #1

Updated by Electric Monk 7 months ago

  • Gerrit CR set to 1978
Actions
Copy link
 #2

Updated by Robert Mustacchi 7 months ago

As this just changed comments, I mostly looked at this with wsdiff and found minor relocation changes:

kernel/drv/amd64/nvme
NOTE: ELF .text difference detected.

6432c6432
<     nvme_fill_prp+0x120:                  ba c8 0e 00 00     movl   $0xec8,%edx                                                 
---
>     nvme_fill_prp+0x120:                  ba cb 0e 00 00     movl   $0xecb,%edx
6540c6540
<     nvme_fill_ranges+0x10e:               41 b9 10 0f 00 00  movl   $0xf10,%r9d
---
>     nvme_fill_ranges+0x10e:               41 b9 13 0f 00 00  movl   $0xf13,%r9d
6575c6575
<     nvme_fill_ranges+0x182:               41 b9 fa 0e 00 00  movl   $0xefa,%r9d
---
>     nvme_fill_ranges+0x182:               41 b9 fd 0e 00 00  movl   $0xefd,%r9d
6587c6587
<     nvme_fill_ranges+0x1b3:               41 b9 fb 0e 00 00  movl   $0xefb,%r9d
---
>     nvme_fill_ranges+0x1b3:               41 b9 fe 0e 00 00  movl   $0xefe,%r9d
6682c6682
<     nvme_create_nvm_cmd+0x110:            ba 35 0f 00 00     movl   $0xf35,%edx
---
>     nvme_create_nvm_cmd+0x110:            ba 38 0f 00 00     movl   $0xf38,%edx

I also went and booted this with a system with a bunch of U.2/M.2 based NVMe devices and they all came up.

Actions
Copy link
 #3

Updated by Electric Monk 7 months ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 817002c416c9301664eafbc288940123aa7d6fc9

commit  817002c416c9301664eafbc288940123aa7d6fc9
Author: Robert Mustacchi <rm@fingolfin.org>
Date:   2022-01-25T16:14:38.000Z

    14442 NVMe theory statement on spec version is confusing
    Reviewed by: C Fraire <cfraire@me.com>
    Reviewed by: Hans Rosenfeld <rosenfeld@grumpf.hope-2000.org>
    Reviewed by: Andy Fiddaman <andy@omnios.org>
    Approved by: Gordon Ross <gordon.w.ross@gmail.com>

Actions
Copy link

Also available in: Atom PDF