mpi3mr: configure larger max I/O size if the HBA firmware supports it
AcceptedPublic
Actions

Authored by chs on Feb 21 2025, 12:35 AM.

Details

Reviewers

chandrakanth.patil_broadcom.com
sumit.saxena_broadcom.com
imp

Summary

The max I/O size that an mpi3mr HBA supports is reported in the IOCFacts
structure (with 0 representing the legacy max I/O size of 1 MB).
By default, set the max I/O size of devices attached to mpi3mr controllers
to the smaller of the HBA's max I/O size and the kernel's maxphys.
Allow this default to be overriden by a global tunable
"hw.mpi3mr.max_sgl_entries" or by a per-controller tunable
"dev.mpi3mr.N.max_sgl_entries".

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 62641
Build 59525: arc lint + arc unit

Event Timeline

chs created this revision.Feb 21 2025, 12:35 AM

Herald added a subscriber: imp. · View Herald TranscriptFeb 21 2025, 12:35 AM

chs requested review of this revision.Feb 21 2025, 12:35 AM

Harbormaster completed remote builds in B62557: Diff 151277.Feb 21 2025, 12:35 AM

chs added reviewers: chandrakanth.patil_broadcom.com, sumit.saxena_broadcom.com, imp.Feb 21 2025, 12:38 AM

This looks pretty good. One question to double check and maybe a slight code improvement. What I don't know is what the real limit of the firmware is and if we should enforce it if it isn't reported.

sys/dev/mpi3mr/mpi3mr.h
342	Since you're inserting in the middle, I'm Just double checking: this is purely a mpi3mr driver thing, not the format of data returned from the card, right? Seems that way, but I want to make sure.
sys/dev/mpi3mr/mpi3mr_pci.c
181	why round up? Also, isn't this this howmany() macro?

This revision is now accepted and ready to land.Feb 22 2025, 3:45 PM

This patch had also tried to resolve some confusion between the OS PAGE_SIZE and the driver's MPI3MR_4K_PGSZ, but I got some cases wrong. I'll fix those in the next version of the patch as well.

sys/dev/mpi3mr/mpi3mr.h
342	Yes, this is the driver's copy of the facts structure. The structure for the firmware's version is Mpi3IOCFactsData_t in mpi3mr/mpi/mpi30_ioc.h. I added this field in the middle because this is where the corresponding field is located in the firmware's version of the structure, and it also matches the linux driver where the driver and firmware structure definitions are much more similar to each other.
sys/dev/mpi3mr/mpi3mr_pci.c
181	It's been a while since I wrote this code and I don't remember any more why I thought rounding was necessary. But looking now at how maxphys is set, I see that it's always a power of two, so no rounding is necessary. I'll change this.