Details

Reviewers

Murthy.Bhat_microsemi.com
scottl
mav
scott.benesh_microsemi.com
emaste
mjg
trasz
imp

Commits

rGf4855954c207: smartpqi: Maintenance commit of Microchip smartpqi

Summary

Newly added features and bug fixes in latest Microchip SmartPQI driver.

It includes:

1)Newly added TMF feature.
2)Added newly Huawei & Inspur PCI ID's
3)Fixed smartpqi driver hangs in Z-Pool while running on FreeBSD12.1
4)Fixed flooding dmesg in kernel while the controller is offline during in ioctls.
5)Avoided unnecessary host memory allocation for rcb sg buffers.
6)Fixed race conditions while accessing internal rcb structure.
7)Fixed where Logical volumes exposing two different names to the OS it's due to the system memory is overwritten with DMA stale data.
8)Fixed dynamically unloading a smartpqi driver.
9)Added device_shutdown callback instead of deprecated shutdown_final kernel event in smartpqi driver.
10)Fixed where Os is crashed during physical drive hot removal during heavy IO.
11)Fixed OS crash during controller lockup/offline during heavy IO.
12)Fixed coverity issues in smartpqi driver
13)Fixed system crash while creating and deleting logical volume in a continuous loop.
14)Fixed where the volume size is not exposing to OS when it expands.
15)Added HC3 pci id's.
16)Fixed compiler issues in 12.2 kernel.

Test Plan

Smartpqi driver is tested and qualified by internal quality assurance team.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 37190
Build 34079: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Added bug fixes which is happened recently

I've made only a very brief look, but here are some comments. Also it would be nice to reviewers if you would not mix formatting and real code changes.

sys/dev/smartpqi/smartpqi_cam.c
56	There is strlcpy() to be safe without this.
156	If you are fixing style, then according to style(9) function name should start on a new line.
256	I was going to mention strlcpy() again, but IIRC strings in SCSI INQUIRY are not required to be NULL-terminated, so strncpy() should be just fine there.
510	I suppose this is your local style and consistency within a file is good, but for note this change contradicts FreeBSD style(9).
528	If you are fixing style, there should be a space after "if".
sys/dev/smartpqi/smartpqi_main.c
371	Unfortunately no context provided, but looking on the code in FreeBSD head, below this I see bus_dmamem_alloc() with BUS_DMA_NOWAIT and bus_dmamap_load() without one. I suspect it should actually be backwards: bus_dmamem_alloc() may sleep unless all this function is called in non-sleepable context, while bus_dmamap_load() should not, otherwise os_dma_map() may be called at some point later, when it is too late.
474	I find these checks excessive. Also as I see BSD_FAILUE is defined as -1, while in this context errno should be returned, something like ENXIO may be.
sys/dev/smartpqi/smartpqi_misc.c
182	I haven't looked close where this function is called, but -1 does not very sound like BSD error.

comments resolved

Harbormaster completed remote builds in B31083: Diff 71767.May 14 2020, 1:52 PM

sbruno resigned from this revision.May 14 2020, 5:50 PM

Hi All,

The patch set has been opened from a long time, Kindly please spare some time to review this change.

Thanks,
Srikanth

Added newly fixed system crashes

Harbormaster completed remote builds in B32284: Diff 74383.Jul 13 2020, 7:34 AM

Updated Inbox Driver version to match with latest Out-of-box driver 1x.4014.0.105

Harbormaster completed remote builds in B32293: Diff 74406.Jul 13 2020, 4:40 PM

scott.benesh_microsemi.com accepted this revision.Jul 15 2020, 1:52 PM

This revision is now accepted and ready to land.Jul 15 2020, 1:52 PM

This does not compile with 12.2.

See
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=240145

Can someone look at this?

In D24428#640413, @rainer_ultra-secure.de wrote:

This does not compile with 12.2.

See
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=240145

Can someone look at this?

Yes, we're working on the issues. I will update the latest driver soon.

Fixed system crash while creating and deleting logical volume in a continuous loop.
Fixed where the volume size is not exposing to OS when it expands.
Added HC3 pci id's.
Fixed compiler issues in 12.2 kernel.

This revision now requires review to proceed.Feb 15 2021, 4:31 PM

Harbormaster completed remote builds in B37023: Diff 83937.Feb 15 2021, 4:31 PM

Updated year in Copyright section.

Harbormaster completed remote builds in B37026: Diff 83941.Feb 15 2021, 5:20 PM

In D24428#641982, @papani.srikanth_microchip.com wrote:

Fixed system crash while creating and deleting logical volume in a continuous loop.
Fixed where the volume size is not exposing to OS when it expands.
Added HC3 pci id's.
Fixed compiler issues in 12.2 kernel.

I can't seem to apply the patch cleanly on 12.2-RELEASE-p3.

See my comment in the PR above.

Fixed white space errors

Harbormaster completed remote builds in B37190: Diff 84274.Feb 19 2021, 3:45 PM

In D24428#644381, @rainer_ultra-secure.de wrote:

In D24428#641982, @papani.srikanth_microchip.com wrote:

Fixed system crash while creating and deleting logical volume in a continuous loop.
Fixed where the volume size is not exposing to OS when it expands.
Added HC3 pci id's.
Fixed compiler issues in 12.2 kernel.

I can't seem to apply the patch cleanly on 12.2-RELEASE-p3.

See my comment in the PR above.

Please use the git bash to apply the patch cleanly.

Please excuse me, I am only a user, I only try to get this working.

(f-hosting </root>) 130 # cd /usr/src
(f-hosting <src>) 0 # git apply /root/D24428.diff
/root/D24428.diff:1875: trailing whitespace.
_softs->pci_mem_handle.pqi_bhandle, _offset)
/root/D24428.diff:3930: space before tab in indent.

	DBG_FUNC("IN\n");

/root/D24428.diff:3944: space before tab in indent.

	DBG_FUNC("OUT\n");

/root/D24428.diff:6642: trailing whitespace.

Populate hostwellness time variables in bcd format from FreeBSD format

/root/D24428.diff:7949: space before tab in indent.

		sizeof(raid_req->lun_number));

error: dev/smartpqi/smartpqi_cam.c: No such file or directory
error: dev/smartpqi/smartpqi_cmd.c: No such file or directory
error: dev/smartpqi/smartpqi_defines.h: No such file or directory
error: dev/smartpqi/smartpqi_discovery.c: No such file or directory
error: dev/smartpqi/smartpqi_event.c: No such file or directory
error: dev/smartpqi/smartpqi_helper.c: No such file or directory
error: dev/smartpqi/smartpqi_includes.h: No such file or directory
error: dev/smartpqi/smartpqi_init.c: No such file or directory
error: dev/smartpqi/smartpqi_intr.c: No such file or directory
error: dev/smartpqi/smartpqi_ioctl.h: No such file or directory
error: dev/smartpqi/smartpqi_ioctl.c: No such file or directory
error: dev/smartpqi/smartpqi_main.c: No such file or directory
error: dev/smartpqi/smartpqi_mem.c: No such file or directory
error: dev/smartpqi/smartpqi_misc.c: No such file or directory
error: dev/smartpqi/smartpqi_prototypes.h: No such file or directory
error: dev/smartpqi/smartpqi_queue.c: No such file or directory
error: dev/smartpqi/smartpqi_request.c: No such file or directory
error: dev/smartpqi/smartpqi_response.c: No such file or directory
error: dev/smartpqi/smartpqi_sis.c: No such file or directory
error: dev/smartpqi/smartpqi_structures.h: No such file or directory
error: dev/smartpqi/smartpqi_tag.c: No such file or directory

I know it's not your job - but can you walk me through what I have got to do to get this to compile on 12.2-RELEASE-p3?

In D24428#644782, @rainer_ultra-secure.de wrote:
Please excuse me, I am only a user, I only try to get this working.

(f-hosting </root>) 130 # cd /usr/src
(f-hosting <src>) 0 # git apply /root/D24428.diff
/root/D24428.diff:1875: trailing whitespace.
_softs->pci_mem_handle.pqi_bhandle, _offset)
/root/D24428.diff:3930: space before tab in indent.
	DBG_FUNC("IN\n");
/root/D24428.diff:3944: space before tab in indent.
	DBG_FUNC("OUT\n");
/root/D24428.diff:6642: trailing whitespace.

Populate hostwellness time variables in bcd format from FreeBSD format

/root/D24428.diff:7949: space before tab in indent.
		sizeof(raid_req->lun_number));
error: dev/smartpqi/smartpqi_cam.c: No such file or directory
error: dev/smartpqi/smartpqi_cmd.c: No such file or directory
error: dev/smartpqi/smartpqi_defines.h: No such file or directory
error: dev/smartpqi/smartpqi_discovery.c: No such file or directory
error: dev/smartpqi/smartpqi_event.c: No such file or directory
error: dev/smartpqi/smartpqi_helper.c: No such file or directory
error: dev/smartpqi/smartpqi_includes.h: No such file or directory
error: dev/smartpqi/smartpqi_init.c: No such file or directory
error: dev/smartpqi/smartpqi_intr.c: No such file or directory
error: dev/smartpqi/smartpqi_ioctl.h: No such file or directory
error: dev/smartpqi/smartpqi_ioctl.c: No such file or directory
error: dev/smartpqi/smartpqi_main.c: No such file or directory
error: dev/smartpqi/smartpqi_mem.c: No such file or directory
error: dev/smartpqi/smartpqi_misc.c: No such file or directory
error: dev/smartpqi/smartpqi_prototypes.h: No such file or directory
error: dev/smartpqi/smartpqi_queue.c: No such file or directory
error: dev/smartpqi/smartpqi_request.c: No such file or directory
error: dev/smartpqi/smartpqi_response.c: No such file or directory
error: dev/smartpqi/smartpqi_sis.c: No such file or directory
error: dev/smartpqi/smartpqi_structures.h: No such file or directory
error: dev/smartpqi/smartpqi_tag.c: No such file or directory

I know it's not your job - but can you walk me through what I have got to do to get this to compile on 12.2-RELEASE-p3?

Hi,

Download the latest source code from FreeBSD git hub.

git clone https://github.com/freebsd/freebsd-src.git

switch to directory freebsd-src/sys/dev/smartpqi

Download/copy the raw diff to file in FreeBSD then apply the diff to top of smartpqi directory.

root@root:~/freebsd-src/sys/dev/smartpqi # git apply /root/D24428.diff

root@root:~/freebsd-src/sys/dev/smartpqi # git status

On branch arcpatch-D24428_2
Changes not staged for commit:

(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
      modified:   smartpqi_cam.c
      modified:   smartpqi_cmd.c
      modified:   smartpqi_defines.h
      modified:   smartpqi_discovery.c
      modified:   smartpqi_event.c
      modified:   smartpqi_helper.c
      modified:   smartpqi_includes.h
      modified:   smartpqi_init.c
      modified:   smartpqi_intr.c
      modified:   smartpqi_ioctl.c
      modified:   smartpqi_ioctl.h
      modified:   smartpqi_main.c
      modified:   smartpqi_mem.c
      modified:   smartpqi_misc.c
      modified:   smartpqi_prototypes.h
      modified:   smartpqi_queue.c
      modified:   smartpqi_request.c
      modified:   smartpqi_response.c
      modified:   smartpqi_sis.c
      modified:   smartpqi_structures.h
      modified:   smartpqi_tag.c

no changes added to commit (use "git add" and/or "git commit -a")
root@root:~/freebsd-src/sys/dev/smartpqi #

Copy the smartpqi source files from /freebsd-src/sys/dev/smartpqi to /usr/sys/dev/smartpqi directory.

Configure and install the custom kernel

emaste added a subscriber: emaste.Feb 24 2021, 2:57 PM

papani.srikanth_microchip.com added reviewers: emaste, mjg.Mar 15 2021, 10:57 AM

papani.srikanth_microchip.com edited reviewers, added: trasz; removed: deepak.ukey_microsemi.com, sbruno.Mar 15 2021, 11:02 AM

There's issues with applying this patch. Something seems to have gone amiss in its generation.

It almost applied cleanly to stable/12 branch, but not to the main branch:

% find . -name \*.rej
./sys/dev/smartpqi/smartpqi_mem.c.rej
./sys/dev/smartpqi/smartpqi_queue.c.rej
./sys/dev/smartpqi/smartpqi_defines.h.rej
./sys/dev/smartpqi/smartpqi_cam.c.rej
./sys/dev/smartpqi/smartpqi_misc.c.rej
./sys/dev/smartpqi/smartpqi_main.c.rej
./sys/dev/smartpqi/smartpqi_request.c.rej

In the main branch, it crashed patch :(.
A quick sample of the .rej files shows the diffs likely are easy to resolve by hand, but with such a large patch I'm leery to do so. Add '-l' to patch to cope with whitespace changes didn't seem to help.
So it looks like this patch needs to be regenerated and/or moved to git where patch generation and uploading is a bit more reliable.

(Also commented on a couple of nits that didn't look quite right in the copyright stuff, but that can wait for the patch to get done).

sys/dev/smartpqi/smartpqi_cam.c
3	This could likely be just Copyright 2016-2021 Microchip Technology Inc. and it's subsidiaries. since microsemi is now owned by microchip technologies, iirc. But that's a question for your legal department.
4–5	These two lines likely can just be removed, unless your legal department insists on the language. In which case it should be put on the same line as each of the prior copyright line(s).

In D24428#657918, @imp wrote:
There's issues with applying this patch. Something seems to have gone amiss in its generation.

It almost applied cleanly to stable/12 branch, but not to the main branch:
% find . -name \*.rej
./sys/dev/smartpqi/smartpqi_mem.c.rej
./sys/dev/smartpqi/smartpqi_queue.c.rej
./sys/dev/smartpqi/smartpqi_defines.h.rej
./sys/dev/smartpqi/smartpqi_cam.c.rej
./sys/dev/smartpqi/smartpqi_misc.c.rej
./sys/dev/smartpqi/smartpqi_main.c.rej
./sys/dev/smartpqi/smartpqi_request.c.rej
In the main branch, it crashed patch :(.
A quick sample of the .rej files shows the diffs likely are easy to resolve by hand, but with such a large patch I'm leery to do so. Add '-l' to patch to cope with whitespace changes didn't seem to help.
So it looks like this patch needs to be regenerated and/or moved to git where patch generation and uploading is a bit more reliable.

(Also commented on a couple of nits that didn't look quite right in the copyright stuff, but that can wait for the patch to get done).

Hi,

12.0 stable branch and 12.2 main branch has two different source codes. I've pulled the 12.0 source code and applied the patch. (12.0 stable branch has bug fixes which is done by community but I do not see the same changes in 12.2 main branch because of that the patch is failing on 12.2 main branch).

Updated driver.

Harbormaster completed remote builds in B38081: Diff 86339.Mar 26 2021, 2:55 PM

papani.srikanth_microchip.com edited the summary of this revision. (Show Details)Mar 26 2021, 3:20 PM

papani.srikanth_microchip.com added a reviewer: imp.

papani.srikanth_microchip.com marked an inline comment as done.Mar 26 2021, 3:23 PM

Thanks.

it compiles now (12.2-RELEASE-p5).

However, I will not have to time to actually try it until after Easter.

One question: there's currently firmware 3.56 available, I'm currently on 3.21 (and 2.65 on two other servers, which are on 12.1 still and run a bit more stable).

I would assume this latest driver also requires the latest firmware? Or does it not make any difference?

Are there any adverse effects, if I am forced to boot back into the stock FreeBSD 12.2-RELEASE driver?

These other servers have HP 408i HBAs, and the latest firmware for those is 3.21. It typically takes HP a couple of months before they release updated firmware (and then it takes some more months before it ends up on an SPP).
Do you think there are any adverse effects running this driver with the 3.21 firmware (one problem I have is that the 3.21 firmware is very unstable in combination with 12.2, which is what prompted me to buy the OG Adaptec HBA to begin with...)

Rainer

In D24428#658934, @papani.srikanth_microchip.com wrote:
In D24428#657918, @imp wrote:
There's issues with applying this patch. Something seems to have gone amiss in its generation.

It almost applied cleanly to stable/12 branch, but not to the main branch:
% find . -name \*.rej
./sys/dev/smartpqi/smartpqi_mem.c.rej
./sys/dev/smartpqi/smartpqi_queue.c.rej
./sys/dev/smartpqi/smartpqi_defines.h.rej
./sys/dev/smartpqi/smartpqi_cam.c.rej
./sys/dev/smartpqi/smartpqi_misc.c.rej
./sys/dev/smartpqi/smartpqi_main.c.rej
./sys/dev/smartpqi/smartpqi_request.c.rej
In the main branch, it crashed patch :(.
A quick sample of the .rej files shows the diffs likely are easy to resolve by hand, but with such a large patch I'm leery to do so. Add '-l' to patch to cope with whitespace changes didn't seem to help.
So it looks like this patch needs to be regenerated and/or moved to git where patch generation and uploading is a bit more reliable.

(Also commented on a couple of nits that didn't look quite right in the copyright stuff, but that can wait for the patch to get done).
Hi,

12.0 stable branch and 12.2 main branch has two different source codes. I've pulled the 12.0 source code and applied the patch. (12.0 stable branch has bug fixes which is done by community but I do not see the same changes in 12.2 main branch because of that the patch is failing on 12.2 main branch).

"main" mentioned is literally main git branch, where this change should go first (and it's 14.0-CURRENT at the moment). I have HBA 1100-8i that is misbehaving under load, so I'd really like to try this patch -- could you please rebase this against main?

sys/dev/smartpqi/smartpqi_cam.c
5	also, s/it's/its/ ?

In D24428#661402, @yuripv wrote:
In D24428#658934, @papani.srikanth_microchip.com wrote:
In D24428#657918, @imp wrote:
There's issues with applying this patch. Something seems to have gone amiss in its generation.

It almost applied cleanly to stable/12 branch, but not to the main branch:
% find . -name \*.rej
./sys/dev/smartpqi/smartpqi_mem.c.rej
./sys/dev/smartpqi/smartpqi_queue.c.rej
./sys/dev/smartpqi/smartpqi_defines.h.rej
./sys/dev/smartpqi/smartpqi_cam.c.rej
./sys/dev/smartpqi/smartpqi_misc.c.rej
./sys/dev/smartpqi/smartpqi_main.c.rej
./sys/dev/smartpqi/smartpqi_request.c.rej
In the main branch, it crashed patch :(.
A quick sample of the .rej files shows the diffs likely are easy to resolve by hand, but with such a large patch I'm leery to do so. Add '-l' to patch to cope with whitespace changes didn't seem to help.
So it looks like this patch needs to be regenerated and/or moved to git where patch generation and uploading is a bit more reliable.

(Also commented on a couple of nits that didn't look quite right in the copyright stuff, but that can wait for the patch to get done).
Hi,

12.0 stable branch and 12.2 main branch has two different source codes. I've pulled the 12.0 source code and applied the patch. (12.0 stable branch has bug fixes which is done by community but I do not see the same changes in 12.2 main branch because of that the patch is failing on 12.2 main branch).
"main" mentioned is literally main git branch, where this change should go first (and it's 14.0-CURRENT at the moment). I have HBA 1100-8i that is misbehaving under load, so I'd really like to try this patch -- could you please rebase this against main?

This patch is for 12.2 only, I will push a new patch separately to the main branch.

In D24428#661566, @papani.srikanth_microchip.com wrote:
In D24428#661402, @yuripv wrote:
In D24428#658934, @papani.srikanth_microchip.com wrote:
In D24428#657918, @imp wrote:
There's issues with applying this patch. Something seems to have gone amiss in its generation.

It almost applied cleanly to stable/12 branch, but not to the main branch:
% find . -name \*.rej
./sys/dev/smartpqi/smartpqi_mem.c.rej
./sys/dev/smartpqi/smartpqi_queue.c.rej
./sys/dev/smartpqi/smartpqi_defines.h.rej
./sys/dev/smartpqi/smartpqi_cam.c.rej
./sys/dev/smartpqi/smartpqi_misc.c.rej
./sys/dev/smartpqi/smartpqi_main.c.rej
./sys/dev/smartpqi/smartpqi_request.c.rej
In the main branch, it crashed patch :(.
A quick sample of the .rej files shows the diffs likely are easy to resolve by hand, but with such a large patch I'm leery to do so. Add '-l' to patch to cope with whitespace changes didn't seem to help.
So it looks like this patch needs to be regenerated and/or moved to git where patch generation and uploading is a bit more reliable.

(Also commented on a couple of nits that didn't look quite right in the copyright stuff, but that can wait for the patch to get done).
Hi,

12.0 stable branch and 12.2 main branch has two different source codes. I've pulled the 12.0 source code and applied the patch. (12.0 stable branch has bug fixes which is done by community but I do not see the same changes in 12.2 main branch because of that the patch is failing on 12.2 main branch).
"main" mentioned is literally main git branch, where this change should go first (and it's 14.0-CURRENT at the moment). I have HBA 1100-8i that is misbehaving under load, so I'd really like to try this patch -- could you please rebase this against main?
This patch is for 12.2 only, I will push a new patch separately to the main branch.

Is there a review/patch for main branch yet to test?

In D24428#662938, @yuripv wrote:
In D24428#661566, @papani.srikanth_microchip.com wrote:
In D24428#661402, @yuripv wrote:
In D24428#658934, @papani.srikanth_microchip.com wrote:
In D24428#657918, @imp wrote:
There's issues with applying this patch. Something seems to have gone amiss in its generation.

It almost applied cleanly to stable/12 branch, but not to the main branch:
% find . -name \*.rej
./sys/dev/smartpqi/smartpqi_mem.c.rej
./sys/dev/smartpqi/smartpqi_queue.c.rej
./sys/dev/smartpqi/smartpqi_defines.h.rej
./sys/dev/smartpqi/smartpqi_cam.c.rej
./sys/dev/smartpqi/smartpqi_misc.c.rej
./sys/dev/smartpqi/smartpqi_main.c.rej
./sys/dev/smartpqi/smartpqi_request.c.rej
In the main branch, it crashed patch :(.
A quick sample of the .rej files shows the diffs likely are easy to resolve by hand, but with such a large patch I'm leery to do so. Add '-l' to patch to cope with whitespace changes didn't seem to help.
So it looks like this patch needs to be regenerated and/or moved to git where patch generation and uploading is a bit more reliable.

(Also commented on a couple of nits that didn't look quite right in the copyright stuff, but that can wait for the patch to get done).
Hi,

12.0 stable branch and 12.2 main branch has two different source codes. I've pulled the 12.0 source code and applied the patch. (12.0 stable branch has bug fixes which is done by community but I do not see the same changes in 12.2 main branch because of that the patch is failing on 12.2 main branch).
"main" mentioned is literally main git branch, where this change should go first (and it's 14.0-CURRENT at the moment). I have HBA 1100-8i that is misbehaving under load, so I'd really like to try this patch -- could you please rebase this against main?
This patch is for 12.2 only, I will push a new patch separately to the main branch.
Is there a review/patch for main branch yet to test?

Here is the review patch for main branch.
https://reviews.freebsd.org/D29584

Updated copyright section

Harbormaster completed remote builds in B38320: Diff 86830.Apr 5 2021, 9:32 AM

papani.srikanth_microchip.com marked an inline comment as done.Apr 5 2021, 9:33 AM

Can someone approve/review the code ?

In D24428#663568, @papani.srikanth_microchip.com wrote:

Can someone approve/review the code ?

I'll look at it this afternoon.

Do you also need someone to commit it to the FreeBSD tree too?

In D24428#663851, @imp wrote:

In D24428#663568, @papani.srikanth_microchip.com wrote:

Can someone approve/review the code ?

I'll look at it this afternoon.

Do you also need someone to commit it to the FreeBSD tree too?

I would be happy if someone commit it to the repository.

In D24428#664005, @papani.srikanth_microchip.com wrote:

In D24428#663851, @imp wrote:

In D24428#663568, @papani.srikanth_microchip.com wrote:

Can someone approve/review the code ?

I'll look at it this afternoon.

Do you also need someone to commit it to the FreeBSD tree too?

I would be happy if someone commit it to the repository.

Is it possible to approve the commit since it has been pending for more than a year?

Hi,

I have this slated for production in one week.
I will have to wait a couple of days to see if it fixes the hangs I have.

In D24428#666449, @rainer_ultra-secure.de wrote:

Hi,

I have this slated for production in one week.
I will have to wait a couple of days to see if it fixes the hangs I have.

Thanks for the update.

Hi,

I've been running the code for 2.5 days now, together with the 3.53 firmware.
So far, it hasn't crashed, but I'll give it a couple of more days.

HPE has now, BTW, also updated their "mod" of this firmware:

https://support.hpe.com/hpesc/public/swd/detail?swItemId=MTX_b08c25d0b8d94a99bb3761a2b6&swEnvOid=2000270

In D24428#671669, @rainer_ultra-secure.de wrote:

I've been running the code for 2.5 days now, together with the 3.53 firmware.
So far, it hasn't crashed, but I'll give it a couple of more days.

Report back after the weekend, and I'll comit. I'll say something if I can't commit it to -current. MFC to 13 should be easy. I'm unsure what to do with 12.x.

HPE has now, BTW, also updated their "mod" of this firmware:

https://support.hpe.com/hpesc/public/swd/detail?swItemId=MTX_b08c25d0b8d94a99bb3761a2b6&swEnvOid=2000270

Should I mention this in the commit message as being required? Or just tested with?

I have so far only tested with the official firmware, on an OG HBA:

<HP EG000300JWSJP HPD2> at scbus0 target 64 lun 0 (pass0,da0)
<HP EG000300JWSJP HPD2> at scbus0 target 65 lun 0 (pass1,da1)
<HPE Smart Adapter 3.21> at scbus0 target 66 lun 0 (ses0,pass2)
<HPE E208i-p SR Gen10 3.21> at scbus0 target 1088 lun 1 (pass3)
<HP EG001200JWJNQ HPD3> at scbus1 target 64 lun 0 (pass4,da2)
<HP EG001200JWFVA HPD3> at scbus1 target 65 lun 0 (pass5,da3)
<HP EG001200JWFVA HPD3> at scbus1 target 66 lun 0 (pass6,da4)
<HP EG001200JWFVA HPD3> at scbus1 target 67 lun 0 (pass7,da5)
<HP EG001200JWFVA HPD3> at scbus1 target 68 lun 0 (pass8,da6)
<HP EG001200JWFVA HPD3> at scbus1 target 69 lun 0 (pass9,da7)
<HP EG001200JWFVA HPD3> at scbus1 target 70 lun 0 (pass10,da8)
<HP EG001200JWJNQ HPD3> at scbus1 target 71 lun 0 (pass11,da9)
<Adaptec Smart Adapter 3.53> at scbus1 target 72 lun 0 (ses1,pass12)
<Adaptec 3154-8i 3.53> at scbus1 target 1088 lun 1 (pass13)
<HPE Smart Adapter 3.21> at scbus2 target 64 lun 0 (ses2,pass14)
<HPE P408i-a SR Gen10 3.21> at scbus2 target 1088 lun 1 (pass15)
<Generic- SD/MMC CRW 1.00> at scbus3 target 0 lun 0 (pass16,da10)

The HP HBAs here aren't used apart from the 208, which only has the boot drives hanging on it.

Likely, the next system I'm going to test it on is this:

As HP has released SPP 2021.04.0, which includes firmware 3.53 for these HBAs, I'll update this server next.

https://techlibrary.hpe.com/us/en/enterprise/servers/products/service_pack/spp/index.aspx

But due to other commitments and time away from work, likely not before the 2nd week of May.

FYI.
The server hasn't crashed anymore so far (20d uptime).

Maybe if this could be committed to 12-stable, so it will at some point make it to a release?

Not sure how and when I'm going to test 13, TBH.

Added lockup code code detection in driver

Harbormaster completed remote builds in B39087: Diff 88933.May 10 2021, 10:19 AM

Added lockup code detection in driver

Harbormaster completed remote builds in B39088: Diff 88934.May 10 2021, 10:22 AM

In D24428#677744, @rainer_ultra-secure.de wrote:

FYI.
The server hasn't crashed anymore so far (20d uptime).

Maybe if this could be committed to 12-stable, so it will at some point make it to a release?

Not sure how and when I'm going to test 13, TBH.

Yes, Please commit it to the branch.

The latest code has lockup code detection, there is no fixes made on the latest code other than lockup code detection during controller failure cases states.

Looks good to me.

This revision is now accepted and ready to land.May 13 2021, 2:36 PM

papani.srikanth_microchip.com mentioned this in D30299: smartpqi: clear CCBs allocated on the stack.May 17 2021, 9:01 AM

It seems this code has not yet landed in 12-stable.

Is there a reason for that?

BTW: the server has not booted since I installed the patched kernel.
Because it's a bit of a hassle, I was hoping that 12.3 would show up with the updated driver, at some point.

Would be a shame to have 12.3 without this code.

In D24428#706627, @rainer_ultra-secure.de wrote:

It seems this code has not yet landed in 12-stable.

Is there a reason for that?

BTW: the server has not booted since I installed the patched kernel.
Because it's a bit of a hassle, I was hoping that 12.3 would show up with the updated driver, at some point.

Would be a shame to have 12.3 without this code.

I'd been waiting to hear confirmation that it worked in 12.... if it does, I'll go ahead.

In D24428#706681, @imp wrote:

I'd been waiting to hear confirmation that it worked in 12.... if it does, I'll go ahead.

This patch applies cleanly to stable/12. I have it in my "MFC queue" and I'll push
it in a week unless I hear that it does not work better than what's there now.
Sorry for the delays.

The CAM parts of this look to be good, but I'm not able to verify the hardware functions, or the changes for that are correct.
I'll assume that Microsemi has tested and audited that stuff.

So far, I see no problems. It certainly works better than the currently shipping driver.

I am going to update another server on Monday and then another one on a week later.

Closed by commit rGf4855954c207: smartpqi: Maintenance commit of Microchip smartpqi (authored by papani.srikanth_microchip.com, committed by imp). · Explain WhyJul 31 2021, 2:57 PM

This revision was automatically updated to reflect the committed changes.

imp added a commit: rGf4855954c207: smartpqi: Maintenance commit of Microchip smartpqi.

In D24428#706794, @rainer_ultra-secure.de wrote:

So far, I see no problems. It certainly works better than the currently shipping driver.

OK. That's conclusive enough for me, I've just committed.

smartpqi: Maintenance commit of Microchip smartpqi
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 84274

sys/dev/smartpqi/smartpqi_cam.c

sys/dev/smartpqi/smartpqi_cmd.c

sys/dev/smartpqi/smartpqi_defines.h

sys/dev/smartpqi/smartpqi_discovery.c

sys/dev/smartpqi/smartpqi_event.c

sys/dev/smartpqi/smartpqi_helper.c

sys/dev/smartpqi/smartpqi_includes.h

sys/dev/smartpqi/smartpqi_init.c

sys/dev/smartpqi/smartpqi_intr.c

sys/dev/smartpqi/smartpqi_ioctl.h

sys/dev/smartpqi/smartpqi_ioctl.c

sys/dev/smartpqi/smartpqi_main.c

sys/dev/smartpqi/smartpqi_mem.c

sys/dev/smartpqi/smartpqi_misc.c

sys/dev/smartpqi/smartpqi_prototypes.h

sys/dev/smartpqi/smartpqi_queue.c

sys/dev/smartpqi/smartpqi_request.c

sys/dev/smartpqi/smartpqi_response.c

sys/dev/smartpqi/smartpqi_sis.c

sys/dev/smartpqi/smartpqi_structures.h

sys/dev/smartpqi/smartpqi_tag.c

smartpqi: Maintenance commit of Microchip smartpqiClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 84274

sys/dev/smartpqi/smartpqi_cam.c

sys/dev/smartpqi/smartpqi_cmd.c

sys/dev/smartpqi/smartpqi_defines.h

sys/dev/smartpqi/smartpqi_discovery.c

sys/dev/smartpqi/smartpqi_event.c

sys/dev/smartpqi/smartpqi_helper.c

sys/dev/smartpqi/smartpqi_includes.h

sys/dev/smartpqi/smartpqi_init.c

sys/dev/smartpqi/smartpqi_intr.c

sys/dev/smartpqi/smartpqi_ioctl.h

sys/dev/smartpqi/smartpqi_ioctl.c

sys/dev/smartpqi/smartpqi_main.c

sys/dev/smartpqi/smartpqi_mem.c

sys/dev/smartpqi/smartpqi_misc.c

sys/dev/smartpqi/smartpqi_prototypes.h

sys/dev/smartpqi/smartpqi_queue.c

sys/dev/smartpqi/smartpqi_request.c

sys/dev/smartpqi/smartpqi_response.c

sys/dev/smartpqi/smartpqi_sis.c

sys/dev/smartpqi/smartpqi_structures.h

sys/dev/smartpqi/smartpqi_tag.c

smartpqi: Maintenance commit of Microchip smartpqi
ClosedPublic
Actions

Revision Contents
Changeset List