git: e30fceb89b7e - main - mps: Use 64-bit chain structures
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 03 Feb 2022 05:35:44 UTC
The branch main has been updated by imp:
URL: https://cgit.FreeBSD.org/src/commit/?id=e30fceb89b7eb51825bdd65f9cc4fbadf107d763
commit e30fceb89b7eb51825bdd65f9cc4fbadf107d763
Author: Warner Losh <imp@FreeBSD.org>
AuthorDate: 2022-02-02 21:36:49 +0000
Commit: Warner Losh <imp@FreeBSD.org>
CommitDate: 2022-02-03 05:35:33 +0000
mps: Use 64-bit chain structures
According to Broadcom, mixing 64-bit SGEs with 32-bit chain entries can
lead to IOC Fault code 0x40000d04. This fault code has been observed to
suddenly increase on certain machines when the OCA firmware images are
deployed. The hardware interprets all elements of a 64-bit SGE, even
ones marked as 32-bit. Depending on the other bits, this will just work,
but sometimes generate the above fault. Broadcom recommends this
practice, and the Linux and NetBSD drivers follow it.
Rework the chaining code to use MPI2_SGE_CHAIN64 instead of
MPI2_SGE_CHAIN32. Adjust MPS_SGC_SIZE from 8 to 12 to match the size of
the new structure. Flag the structure as being 64-bits now. Since
MPS_SGE64_SIZE and MPS_SGC_SIZE are the same now, mps_push_sge could be
simplified (after the same fashion of mpr). The different number of
cases collapse to whether or not there's room for the segments and if
not we need a chain, however these changes haven't been made yet as the
current code handles those cases properly with the new defines.
Made chain_busaddr 64-bits, even though we ask for all allocations to be
below 4GB for this tag. Use it to set both parts of the CHAIN64 address
rather than baking the 4GB assumption. Add asserts around the allocation
to detect and BUSDMA bugs in allocation.
Remove asserts and associated comment in mpi_pre_fw_download and
mpi_pre_fw_upload. The code does not, it seems, depend on this
invariant. The mpr driver has similar code, no asserts and also doesn't
depend on this.
Adjust comments to reflect the updated size.
Sponsored by: Netflix
Reviewed by: scottl, mav
Differential Revision: https://reviews.freebsd.org/D34016
---
sys/dev/mps/mps.c | 16 ++++++++++------
sys/dev/mps/mps_user.c | 12 ------------
sys/dev/mps/mpsvar.h | 4 ++--
3 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/sys/dev/mps/mps.c b/sys/dev/mps/mps.c
index 956a0301e70a..5021acf930a3 100644
--- a/sys/dev/mps/mps.c
+++ b/sys/dev/mps/mps.c
@@ -1382,6 +1382,9 @@ mps_load_chains_cb(void *arg, bus_dma_segment_t *segs, int nsegs, int error)
return;
for (i = 0, o = 0, s = 0; s < nsegs; s++) {
+ KASSERT(segs[s].ds_addr + segs[s].ds_len - 1 <= BUS_SPACE_MAXADDR_32BIT,
+ ("mps: Bad segment address %#jx len %#jx\n", (uintmax_t)segs[s].ds_addr,
+ (uintmax_t)segs[s].ds_len));
for (bo = 0; bo + sc->reqframesz <= segs[s].ds_len;
bo += sc->reqframesz) {
chain = &sc->chains[i++];
@@ -2696,7 +2699,7 @@ mps_deregister_events(struct mps_softc *sc, struct mps_event_handle *handle)
static int
mps_add_chain(struct mps_command *cm)
{
- MPI2_SGE_CHAIN32 *sgc;
+ MPI2_SGE_CHAIN64 *sgc;
struct mps_chain *chain;
u_int space;
@@ -2715,17 +2718,18 @@ mps_add_chain(struct mps_command *cm)
*/
TAILQ_INSERT_TAIL(&cm->cm_chain_list, chain, chain_link);
- sgc = (MPI2_SGE_CHAIN32 *)&cm->cm_sge->MpiChain;
+ sgc = (MPI2_SGE_CHAIN64 *)&cm->cm_sge->MpiChain;
sgc->Length = htole16(space);
sgc->NextChainOffset = 0;
/* TODO Looks like bug in Setting sgc->Flags.
* sgc->Flags = ( MPI2_SGE_FLAGS_CHAIN_ELEMENT | MPI2_SGE_FLAGS_64_BIT_ADDRESSING |
* MPI2_SGE_FLAGS_SYSTEM_ADDRESS) << MPI2_SGE_FLAGS_SHIFT
* This is fine.. because we are not using simple element. In case of
- * MPI2_SGE_CHAIN32, we have separate Length and Flags feild.
+ * MPI2_SGE_CHAIN64, we have separate Length and Flags feild.
*/
- sgc->Flags = MPI2_SGE_FLAGS_CHAIN_ELEMENT;
- sgc->Address = htole32(chain->chain_busaddr);
+ sgc->Flags = MPI2_SGE_FLAGS_CHAIN_ELEMENT | MPI2_SGE_FLAGS_64_BIT_ADDRESSING;
+ sgc->Address.High = htole32(chain->chain_busaddr >> 32);
+ sgc->Address.Low = htole32(chain->chain_busaddr);
cm->cm_sge = (MPI2_SGE_IO_UNION *)&chain->chain->MpiSimple;
cm->cm_sglsize = space;
@@ -2757,7 +2761,7 @@ mps_push_sge(struct mps_command *cm, void *sgep, size_t len, int segsleft)
}
break;
case MPI2_SGE_FLAGS_CHAIN_ELEMENT:
- /* Driver only uses 32-bit chain elements */
+ /* Driver only uses 64-bit chain elements */
if (len != MPS_SGC_SIZE)
panic("CHAIN %p length %u or %zu?", sgep,
MPS_SGC_SIZE, len);
diff --git a/sys/dev/mps/mps_user.c b/sys/dev/mps/mps_user.c
index 9d4aab54562f..4b09b486b0dd 100644
--- a/sys/dev/mps/mps_user.c
+++ b/sys/dev/mps/mps_user.c
@@ -468,12 +468,6 @@ mpi_pre_fw_download(struct mps_command *cm, struct mps_usr_command *cmd)
MPI2_FW_DOWNLOAD_TCSGE tc;
int error;
- /*
- * This code assumes there is room in the request's SGL for
- * the TransactionContext plus at least a SGL chain element.
- */
- CTASSERT(sizeof req->SGL >= sizeof tc + MPS_SGC_SIZE);
-
if (cmd->req_len != sizeof *req)
return (EINVAL);
if (cmd->rpl_len != sizeof *rpl)
@@ -521,12 +515,6 @@ mpi_pre_fw_upload(struct mps_command *cm, struct mps_usr_command *cmd)
MPI2_FW_UPLOAD_REPLY *rpl;
MPI2_FW_UPLOAD_TCSGE tc;
- /*
- * This code assumes there is room in the request's SGL for
- * the TransactionContext plus at least a SGL chain element.
- */
- CTASSERT(sizeof req->SGL >= sizeof tc + MPS_SGC_SIZE);
-
if (cmd->req_len != sizeof *req)
return (EINVAL);
if (cmd->rpl_len != sizeof *rpl)
diff --git a/sys/dev/mps/mpsvar.h b/sys/dev/mps/mpsvar.h
index 9cffd0f730d5..8e3d324215fe 100644
--- a/sys/dev/mps/mpsvar.h
+++ b/sys/dev/mps/mpsvar.h
@@ -53,7 +53,7 @@
#define MPS_MSIX_MAX 16
#define MPS_SGE64_SIZE 12
#define MPS_SGE32_SIZE 8
-#define MPS_SGC_SIZE 8
+#define MPS_SGC_SIZE 12
#define CAN_SLEEP 1
#define NO_SLEEP 0
@@ -199,7 +199,7 @@ typedef void mps_command_callback_t(struct mps_softc *, struct mps_command *cm);
struct mps_chain {
TAILQ_ENTRY(mps_chain) chain_link;
MPI2_SGE_IO_UNION *chain;
- uint32_t chain_busaddr;
+ uint64_t chain_busaddr;
};
/*