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

Authored by imp on Dec 6 2021, 6:01 AM.

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.

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.

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.

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)