Page MenuHomeFreeBSD

Fix isp(4) device probing with 9.x firmware
ClosedPublic

Authored by ken on Jun 20 2024, 2:54 PM.
Tags
Referenced Files
Unknown Object (File)
Tue, Sep 24, 5:01 AM
Unknown Object (File)
Aug 27 2024, 12:44 AM
Unknown Object (File)
Aug 17 2024, 8:13 AM
Unknown Object (File)
Aug 15 2024, 7:09 PM
Unknown Object (File)
Aug 12 2024, 4:22 PM
Unknown Object (File)
Aug 10 2024, 1:26 PM
Unknown Object (File)
Aug 10 2024, 1:25 PM
Unknown Object (File)
Aug 10 2024, 11:26 AM
Subscribers

Details

Summary

The isp(4) driver and ispfw(4) gained firmware for the 27XX and 28XX controllers in revision b0c6b06836351b3908ba5b2a847c89c42d1a46c3

The 9.x Qlogic firmware adds login state for NVMe devices in the top nibble of the login state variables in the port database (isp_pdb_24xx_t in ispmbox.h).

This breaks the check at the end of isp_getpdb() to make sure the device is in the right login state. As a result, it breaks device discovery for many (perhaps all?) FC devices. In my testing with IBM LTO-6 drives attached to a quad port 16Gb Qlogic 2714, they don't show up when they are directly connected (and in loop mode) or connected via a switch (and in fabric mode).

So, mask off the top bits of of the login state before checking it. For now, we're ignoring the NVMe state bits. I suspect there is more that would need to be done in the driver to support NVMe devices, but I don't have newer specs to confirm that. I figured out the top bits are now used for NVMe devices by looking at the Qlogic Linux driver.

This shouldn't break anything in the isp(4) driver, because all of the existing defined PDB states occupy the low nibble.

Test Plan

Connect FC devices to a card that can run the 9.x firmware (27XX or 28XX controller).

Do they probe when connected directly? Do they probe when connected via a switch?

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

ken requested review of this revision.Jun 20 2024, 2:54 PM

None of QLogic documents I have know nothing about NVMe, and this state field is declared is byte there. I have no objections for this patch, but a bit curios what NVMe status do we see there for non-NVMe devices.

In D45660#1042675, @mav wrote:

None of QLogic documents I have know nothing about NVMe, and this state field is declared is byte there. I have no objections for this patch, but a bit curios what NVMe status do we see there for non-NVMe devices.

So here is what the debugging log message in isp_getpdb() shows. isp0 and isp1 are connected to LTO-6 tape drives via an 8Gb switch. isp2 is directly connected to an LTO-6 in loop mode:

isp0: Chan 0 handle 0x0 Port 0xfffc01 flags 0x0 curstate 77 laststate 77
isp0: Chan 0 handle 0x1 Port 0x011b26 flags 0x40a0 curstate 46 laststate 46
isp0: Chan 0 handle 0x7fe Port 0xfffffe flags 0x0 curstate 44 laststate 44
isp0: Chan 0 handle 0x7fe Port 0xfffffe flags 0x0 curstate 44 laststate 44
isp1: Chan 0 handle 0x0 Port 0xfffc01 flags 0x0 curstate 77 laststate 77
isp1: Chan 0 handle 0x1 Port 0x011a26 flags 0x40a0 curstate 46 laststate 46
isp1: Chan 0 handle 0x7fe Port 0xfffffe flags 0x0 curstate 44 laststate 44
isp1: Chan 0 handle 0x7fe Port 0xfffffe flags 0x0 curstate 44 laststate 44
isp2: Chan 0 handle 0x0 Port 0x000026 flags 0x40a0 curstate 46 laststate 46

In D45660#1042742, @ken wrote:

So here is what the debugging log message in isp_getpdb() shows. isp0 and isp1 are connected to LTO-6 tape drives via an 8Gb switch. isp2 is directly connected to an LTO-6 in loop mode:

isp0: Chan 0 handle 0x0 Port 0xfffc01 flags 0x0 curstate 77 laststate 77
isp0: Chan 0 handle 0x1 Port 0x011b26 flags 0x40a0 curstate 46 laststate 46
isp0: Chan 0 handle 0x7fe Port 0xfffffe flags 0x0 curstate 44 laststate 44
isp0: Chan 0 handle 0x7fe Port 0xfffffe flags 0x0 curstate 44 laststate 44
isp1: Chan 0 handle 0x0 Port 0xfffc01 flags 0x0 curstate 77 laststate 77
isp1: Chan 0 handle 0x1 Port 0x011a26 flags 0x40a0 curstate 46 laststate 46
isp1: Chan 0 handle 0x7fe Port 0xfffffe flags 0x0 curstate 44 laststate 44
isp1: Chan 0 handle 0x7fe Port 0xfffffe flags 0x0 curstate 44 laststate 44
isp2: Chan 0 handle 0x0 Port 0x000026 flags 0x40a0 curstate 46 laststate 46

So it seems the upper 4 bits can actually have different values, not some "non-NVMe" constant, as I thought. I am OK with this patch if it solves the issue, but it would definitely be good to understand what it is, in case there is something important.

In D45660#1042762, @mav wrote:
In D45660#1042742, @ken wrote:

So here is what the debugging log message in isp_getpdb() shows. isp0 and isp1 are connected to LTO-6 tape drives via an 8Gb switch. isp2 is directly connected to an LTO-6 in loop mode:

isp0: Chan 0 handle 0x0 Port 0xfffc01 flags 0x0 curstate 77 laststate 77
isp0: Chan 0 handle 0x1 Port 0x011b26 flags 0x40a0 curstate 46 laststate 46
isp0: Chan 0 handle 0x7fe Port 0xfffffe flags 0x0 curstate 44 laststate 44
isp0: Chan 0 handle 0x7fe Port 0xfffffe flags 0x0 curstate 44 laststate 44
isp1: Chan 0 handle 0x0 Port 0xfffc01 flags 0x0 curstate 77 laststate 77
isp1: Chan 0 handle 0x1 Port 0x011a26 flags 0x40a0 curstate 46 laststate 46
isp1: Chan 0 handle 0x7fe Port 0xfffffe flags 0x0 curstate 44 laststate 44
isp1: Chan 0 handle 0x7fe Port 0xfffffe flags 0x0 curstate 44 laststate 44
isp2: Chan 0 handle 0x0 Port 0x000026 flags 0x40a0 curstate 46 laststate 46

So it seems the upper 4 bits can actually have different values, not some "non-NVMe" constant, as I thought. I am OK with this patch if it solves the issue, but it would definitely be good to understand what it is, in case there is something important.

To be certain about it, we will probably have to get the updated specs from Qlogic/Marvell. But this is how the Linux driver handles it:

/* Check for logged in state. */
if (NVME_TARGET(ha, fcport)) {
        current_login_state = pd24->current_login_state >> 4;
        last_login_state = pd24->last_login_state >> 4;
} else {
        current_login_state = pd24->current_login_state & 0xf;
        last_login_state = pd24->last_login_state & 0xf;
}
fcport->current_login_state = pd24->current_login_state;
fcport->last_login_state = pd24->last_login_state;

So for non-NVMe, it just ignores the high nibble, and for NVMe, it ignores the low nibble.

Looks odd to me, but OK.

This revision is now accepted and ready to land.Jun 24 2024, 6:17 PM