This reduces the number of duplicate string tables for NVMe opcodes
and status codes.
Adjust the formatting of unknown opcodes and status codes to more
closely match nvme(4).
Sponsored by: Chelsio Communications
Differential D50652
nvme: Use routines from nvme_util.c to decode opcodes and status codes jhb on Jun 2 2025, 4:01 PM. Authored by Tags None Referenced Files
Subscribers
Details
This reduces the number of duplicate string tables for NVMe opcodes Adjust the formatting of unknown opcodes and status codes to more Sponsored by: Chelsio Communications
Diff Detail
Event TimelineComment Actions Yea, this kinda break kernels that use nvd. Well, it forces all of cam to be drug in, which may or may not be a big deal. But, those routines can move from CAM to subr_nvme.c, in the future so I guess I'm OK with this. Comment Actions Kernels that use nvd already have to pull in CAM? Well, nvme.ko at least pulls in nvme_sim.c unconditionally. I guess custom kernels without any SATA or SCSI and only NVMe that are also using nvd instead of nda(4)? How common is that use case? Comment Actions Note that nvmf doesn't use any of the decoding bits, those all live in CAM (either in nda for the client side, or in ctl for the target side). An analog would be having subr_sata.c and subr_scsi.c. It's not clear to me though that it will be worth having subr_nvme.c. It would either need to be standard, or we'd have to have a subr_nvme.ko that nvme(4), nda(4), and ctl(4) all depend on. Comment Actions It's a desired case, and people have talked to me about fixing this bug. Or rather wanting it fixed. And NVMe only-systems are quire popular. But the overhead is small in terms of extra memory, so it's not been a bug high on my list of fixing. This makes it just a little more so. It's a non-default case, and we do the right thing when both are in the kernel, so it's fine. And we've been de-emphasizing nvd, which really is only a win for ultra-high transaction rate systems. Comment Actions I tried making nvme_all.c nvme | scbus in sys/conf/files and it almost worked except it needed cam_strvis_sbuf for some routines in nvme_all.c that aren't needed by nvme(4). I'm not sure what's better/worse, but we could add DEV_SCBUS to sys/conf/options and then use that #ifdef in nvme_all.c to limit the bits used in an static kernel without scbus? sys/modules/cam/Makefile would need to define it always perhaps. Not sure if that is too ugly. Comment Actions Yea, that's an interesting notion. My first take is that it might be a bit too ugly since we're moving away from DEV_* device defines where possible... and I wonder how that might interact with the cam.ko module. Maybe we should just leave it alone until we start to do more work with CAM to make it more modular... Comment Actions Actually, how about nvme_util.c. That code is already shared between nvme(4) and cam.ko today. I can just put the sbuf routines under #ifdef _KERNEL as user land doesn't need them. I will add a commit to move them first then rebase this one on top of that. I think I prefer this to adding a subr_nvme.c under sys/kern. |