Add the mfi(4) ioctl support to mrsas(4)
ClosedPublic
Actions

Authored by ambrisko on Aug 24 2022, 10:49 PM.

Details

Reviewers

imp
emaste
jhb

Commits

rGe315351fc7af: Add the mfi(4) ioctl support to mrsas(4)

Summary

The hardware supported by mfi(4) and mrsas(4) use the same dcmd's.
mfiutil(8) in theory could run on controlled attached to mrsas(4).
It can't since mrsas(4) doesn't have support for the FreeBSD mfi(4)
ioctl. Porting the ioctl from mfi(4) to mrsas(4) would be the first
step in making mrsasutil(8) which is an additional name for mfiutil(8)
but opens /dev/mrsasX instead of /dev/mfiX

PR: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=265794
Tested by: Dan Mahoney <freebsd@gushi.org>

Test Plan

This provides the FreeBSD mfi(4) ioctl in mrsas(4) so that mfiutil(8) can run with mrsas(4). This enables https://reviews.freebsd.org/D36343. I've tested with mrsasutil(8) on various cards over the years and others have found it useful to add to FreeBSD src.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ambrisko created this revision.Aug 24 2022, 10:49 PM

Herald added a subscriber: imp. · View Herald TranscriptAug 24 2022, 10:49 PM

ambrisko requested review of this revision.Aug 24 2022, 10:49 PM

Harbormaster completed remote builds in B47121: Diff 109806.Aug 24 2022, 10:49 PM

ambrisko mentioned this in D36343: Add mrsasutil(8) as alias to mfiutil(8).Aug 24 2022, 10:59 PM

ambrisko edited the test plan for this revision. (Show Details)Aug 24 2022, 11:02 PM

ambrisko added reviewers: imp, emaste.

emaste added inline comments.Jan 11 2023, 3:08 PM

sys/dev/mrsas/mrsas.c

1453

it looks like mrsas_get_softc_instance already has logic to return dev->si_drv1 in certain cases, maybe this logic should be there instead?

static struct mrsas_softc *
mrsas_get_softc_instance(struct cdev *dev, u_long cmd, caddr_t arg)
{
        struct mrsas_softc *sc = NULL;
        struct mrsas_iocpacket *user_ioc = (struct mrsas_iocpacket *)arg;

        if (cmd == MRSAS_IOC_GET_PCI_INFO) {
                sc = dev->si_drv1;
        } else {
...

ambrisko added inline comments.Jan 11 2023, 4:41 PM

sys/dev/mrsas/mrsas.c
1453	I could tweak mrsas_get_softc_instance to also include: if (cmd == MRSAS_IOC_GET_PCI_INFO \|\| cmd == MFIIO_PASSTHRU) { sc = dev->si_drv1; } else { and then use it. I'm not sure if that is helpful or might add to more confusion. I tried to keep the MFI compat. part more localized to mrsas_ioctl. Myself, I would get rid of mrsas_get_softc_instance since this is the only usage and depends on the cmd etc. but that is what the existing code was from Broadcom. I'm not sure if they care much about the legacy HW since they have a new mpi3mr driver for newer HW.

jhb added a child revision: D36343: Add mrsasutil(8) as alias to mfiutil(8).Apr 18 2023, 9:59 PM

jhb accepted this revision.Apr 18 2023, 10:02 PM

jhb added a subscriber: jhb.

jhb added inline comments.

sys/dev/mrsas/mrsas.c
1453	I think either approach is fine. If you wanted to patch mrsas_get_softc_instance I would maybe turn the `if` into a `switch` so it is easier to add new ioctls to that case in the future with a default case for the "lookup by index" case.