Page MenuHomeFreeBSD
Differential D33285

nvme_ahci: Mark AHCI devices as such in the controller
ClosedPublic
Actions

Authored by imp on Dec 6 2021, 6:01 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Sep 6, 10:00 PM
Unknown Object (File)
Sun, Sep 6, 9:12 PM
Unknown Object (File)
Sun, Sep 6, 6:31 PM
Unknown Object (File)
Sun, Sep 6, 3:15 PM
Unknown Object (File)
Sun, Sep 6, 10:33 AM
Unknown Object (File)
Sat, Sep 5, 4:20 PM
Unknown Object (File)
Sat, Sep 5, 2:24 PM
Unknown Object (File)
Sat, Sep 5, 12:38 PM
View All Files
Subscribers
dab

Details

Reviewers
chuck
mav
ambrisko
adrian
Commits
rGcc20d7570563: nvme_ahci: Mark AHCI devices as such in the controller
rG7cf8d63c884c: nvme_ahci: Mark AHCI devices as such in the controller
Summary

Add a quirk to flag AHCI attachment to the controller. This is for any
of the strategies for attaching nvme devices as children of the AHCI
device for Intel's RAID devices. This also has a side effect of cleaning
up resource allocation from failed nvme_attach calls now.

Sponsored by: Netflix

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

imp created this revision.Dec 6 2021, 6:01 AM
Herald added a subscriber: dab. · View Herald TranscriptDec 6 2021, 6:01 AM
imp requested review of this revision.Dec 6 2021, 6:01 AM
imp added a parent revision: D33284: nvme: Move to a quirk for the Intel alignment data.
imp added a child revision: D33286: nvd: For AHCI attached devices, report ahci bridge.
Harbormaster completed remote builds in B43169: Diff 99515.Dec 6 2021, 6:01 AM
imp added reviewers: chuck, mav, ambrisko, adrian.Dec 6 2021, 6:03 AM
mav requested changes to this revision.Dec 6 2021, 2:56 PM
mav added inline comments.
sys/dev/nvme/nvme_ahci.c
115

The logic is broken here.

This revision now requires changes to proceed.Dec 6 2021, 2:56 PM
imp added inline comments.Dec 6 2021, 3:28 PM
sys/dev/nvme/nvme_ahci.c
115

Doh! I hate those last second changes.... Fixed.

imp updated this revision to Diff 99555.Dec 6 2021, 3:29 PM

fix logic error

Harbormaster completed remote builds in B43191: Diff 99555.Dec 6 2021, 3:29 PM
mav added inline comments.Dec 6 2021, 3:35 PM
sys/dev/nvme/nvme_ahci.c
115

Are you sure this part is really needed? I see ahci_attach() calls nvme_ctrlr_destruct() on failure, freeing those resources already.

imp updated this revision to Diff 99569.Dec 6 2021, 4:37 PM

mav is right: the code was right before since nvme_attach cleans this
up in failure case.

Harbormaster completed remote builds in B43193: Diff 99569.Dec 6 2021, 4:37 PM
imp added inline comments.Dec 6 2021, 4:41 PM
sys/dev/nvme/nvme_ahci.c
115

I thought I was sure.... But I see I had overlooked it... so I've updated..

Though there is one hole: if config_interhook_establish fails, then we don't deconstruct things and generally just trail off in a way that's unsatisfying... I'll look at fixing that (though I'm not sure what can be done)

mav accepted this revision.Dec 6 2021, 5:13 PM
This revision is now accepted and ready to land.Dec 6 2021, 5:13 PM
imp added inline comments.Dec 6 2021, 5:22 PM
sys/dev/nvme/nvme_ahci.c
115

Hmmm, looking at config_intrhook_establish, it can only fail if it was called twice! I'm thinking maybe that should be converted to a panic....

mav added inline comments.Dec 6 2021, 5:24 PM
sys/dev/nvme/nvme_ahci.c
115

Wasn't that a protection against simultaneous detach?

Closed by commit rG7cf8d63c884c: nvme_ahci: Mark AHCI devices as such in the controller (authored by imp). · Explain WhyDec 6 2021, 5:26 PM
This revision was automatically updated to reflect the committed changes.
imp added a commit: rG7cf8d63c884c: nvme_ahci: Mark AHCI devices as such in the controller.
mav added a commit: rGcc20d7570563: nvme_ahci: Mark AHCI devices as such in the controller.Jan 21 2022, 2:29 AM

Revision Contents
Changeset List

PathSize
sys/
dev/
nvme/
9 lines
1 line

Diff 99573

View Options

sys/dev/nvme/nvme_ahci.c

Loading...
View Options

sys/dev/nvme/nvme_private.h

Loading...