Page MenuHomeFreeBSD
Differential D53296

nvme host buffer: increase alignment
Needs ReviewPublic
Actions

Authored by br on Thu, Oct 23, 10:46 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Oct 24, 4:49 AM
Unknown Object (File)
Fri, Oct 24, 1:51 AM
Unknown Object (File)
Fri, Oct 24, 1:33 AM
Unknown Object (File)
Thu, Oct 23, 8:48 PM
Unknown Object (File)
Thu, Oct 23, 8:19 PM
Unknown Object (File)
Thu, Oct 23, 7:01 PM
Subscribers
dab

Details

Reviewers
imp
Summary

This patch fixes the following issue discovered on Codasip Prime platform. It increases host memory buffer alignment to 32 bytes.

nvme0: Allocated 64MB host memory buffer
pcib0: Master decode errornvme0: SET_FEATURES (09) sqid:0 cid:15 nsid:0 cdw10:0000000d cdw11:00000001
nvme0: CONTROLLER_PATHING_ERROR (03/60) crd:0 m:0 dnr:0 p:1 sqid:0 cid:15 cdw0:0
nvme0: nvme_ctrlr_hmb_enable failed!

There is no errors shown when the patch is applied.
The controller is operational with or without the patch (no speed difference).

Test Plan

Tested on Codasip Prime

nda0 at nvme0 bus 0 scbus0 target 0 lun 1
nda0: <CT1000P310SSD8 VACR001 25184FFB4BF8>
nda0: Serial Number 25184FFB4BF8
nda0: nvme version 2.0
nda0: 953869MB (1953525168 512 byte sectors)

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

br created this revision.Thu, Oct 23, 10:46 AM
Herald added a subscriber: dab. · View Herald TranscriptThu, Oct 23, 10:46 AM
br requested review of this revision.Thu, Oct 23, 10:46 AM
br edited the test plan for this revision. (Show Details)Thu, Oct 23, 10:51 AM
brooks added a reviewer: imp.Thu, Oct 23, 10:54 AM
br edited the test plan for this revision. (Show Details)Thu, Oct 23, 10:57 AM
br edited the test plan for this revision. (Show Details)
br edited the summary of this revision. (Show Details)Thu, Oct 23, 11:02 AM
br edited the test plan for this revision. (Show Details)
br edited the summary of this revision. (Show Details)
imp added a comment.Thu, Oct 23, 1:55 PM

why is 32 and better than 16?
Why not 64 or 128? Where does the 32 come from?

imp added a comment.EditedThu, Oct 23, 2:23 PM

NVM Express ® Base Specification, revision 2.0
5.27.1.10 Host Memory Buffer (Feature Identifier 0Dh), (Optional)
"Host Memory Descriptor List Lower Address (HMDLLA): This field specifies the least significant 32 bits of the physical location of the Host Memory Descriptor List (refer to Figure 335) for the Host Memory Buffer. This address shall be 16 byte aligned, indicated by bits 3:0 being cleared to 0h."

So why is 32-bytes needed?

I've also been fielding bugs where not-quite-aligned-enough buffers are passed into the load routine that loads bogus PTPs as a result of it.

Also, seems like we should be logging cdw12,13,14,15 on these errors as well...

br added a comment.Thu, Oct 23, 4:49 PM

Good points. I will check with Codasip if this is something to do with CHERI (I don't see any connection however)

br added a comment.Fri, Oct 24, 7:08 AM

What I see when 32 byte requested it actually gives us PAGE_SIZE aligned address, which works.

When 16 byte (by spec) requested it gives a 16 byte aligned address which does not work.

I guess that Codasip did not see an issue because Linux allocates HMB by pages:
https://github.com/torvalds/linux/blob/master/drivers/nvme/host/pci.c#L2309

Revision Contents
Changeset List

PathSize
sys/
dev/
nvme/
2 lines

Diff 164870

View Options

sys/dev/nvme/nvme_ctrlr.c

Loading...