Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 12 May 2025 08:43:49 +0200
From:      "Herbert J. Skuhra" <herbert@gojira.at>
To:        Warner Losh <imp@bsdimp.com>
Cc:        Warner Losh <imp@freebsd.org>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-branches@freebsd.org
Subject:   Re: git: 80b069913d49 - stable/14 - mpi3mr: Divert large WriteSame IOs to firmware if unmap and ndob bits are set
Message-ID:  <87msbipbve.wl-herbert@gojira.at>
In-Reply-To: <CANCZdfrP1=CK500TNZXJwajqDoOLK_p-b0n71%2BMUe6EabfmTNw@mail.gmail.com>
References:  <202504301721.53UHLjlw095104@gitrepo.freebsd.org> <878qn8r3t6.wl-herbert@gojira.at> <CANCZdfrP1=CK500TNZXJwajqDoOLK_p-b0n71%2BMUe6EabfmTNw@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
Yes, thanks. Merge to releng/14.3?=20

On Thu, 08 May 2025 17:44:59 +0200, Warner Losh wrote:
>=20
> I think that I've fixed this.
>=20
> Warner
>=20
> On Wed, May 7, 2025 at 6:41=E2=80=AFAM Herbert J. Skuhra <herbert@gojira.=
at> wrote:
> >
> > On Wed, 30 Apr 2025 19:21:45 +0200, Warner Losh <imp@FreeBSD.org> wrote:
> > >
> > > The branch stable/14 has been updated by imp:
> > >
> > > URL: https://cgit.FreeBSD.org/src/commit/?id=3D80b069913d496bd73b0ea8=
e515b6bf3706530ea1
> > >
> > > commit 80b069913d496bd73b0ea8e515b6bf3706530ea1
> > > Author:     Chandrakanth patil <chandrakanth.patil@broadcom.com>
> > > AuthorDate: 2024-06-06 08:38:52 +0000
> > > Commit:     Warner Losh <imp@FreeBSD.org>
> > > CommitDate: 2025-04-30 17:05:52 +0000
> > >
> > >     mpi3mr: Divert large WriteSame IOs to firmware if unmap and ndob =
bits are set
> > >
> > >     Firmware advertises the transfer lenght for writesame commands to=
 driver during init.
> > >     So for any writesame IOs with ndob and unmap bit set and transfer=
 lengh is greater
> > >     than the max write same length specified by the firmware, then di=
rect those commands
> > >     to firmware instead of hardware otherwise hardware will break.
> > >
> > >     Reviewed by:            imp
> > >     Approved by:            imp
> > >     Differential revision:  https://reviews.freebsd.org/D44452
> > >
> > >     (cherry picked from commit 3f3a15543a6721100dda0e4219eb48ecbe3573=
1a)
> > > ---
> > >  sys/dev/mpi3mr/mpi3mr.c     | 16 ++++++++++++++++
> > >  sys/dev/mpi3mr/mpi3mr.h     |  2 ++
> > >  sys/dev/mpi3mr/mpi3mr_cam.c | 35 +++++++++++++++++++++++++++++++++++
> > >  sys/dev/mpi3mr/mpi3mr_cam.h |  1 +
> > >  sys/modules/mpi3mr/Makefile |  3 +++
> > >  5 files changed, 57 insertions(+)
> > >
> > > diff --git a/sys/dev/mpi3mr/mpi3mr.c b/sys/dev/mpi3mr/mpi3mr.c
> > > index 03fea4bdfcc7..a7bc459c1db8 100644
> > > --- a/sys/dev/mpi3mr/mpi3mr.c
> > > +++ b/sys/dev/mpi3mr/mpi3mr.c
> > > @@ -2177,6 +2177,8 @@ static int mpi3mr_issue_iocinit(struct mpi3mr_s=
oftc *sc)
> > >       time_in_msec =3D (now.tv_sec * 1000 + now.tv_usec/1000);
> > >       iocinit_req.TimeStamp =3D htole64(time_in_msec);
> > >
> > > +     iocinit_req.MsgFlags |=3D MPI3_IOCINIT_MSGFLAGS_WRITESAMEDIVERT=
_SUPPORTED;
> > > +
> > >       init_completion(&sc->init_cmds.completion);
> > >       retval =3D mpi3mr_submit_admin_cmd(sc, &iocinit_req,
> > >           sizeof(iocinit_req));
> > > @@ -3340,6 +3342,19 @@ void mpi3mr_update_device(struct mpi3mr_softc =
*sc,
> > >               break;
> > >       }
> > >
> > > +     switch (flags & MPI3_DEVICE0_FLAGS_MAX_WRITE_SAME_MASK) {
> > > +     case MPI3_DEVICE0_FLAGS_MAX_WRITE_SAME_256_LB:
> > > +             tgtdev->ws_len =3D 256;
> > > +             break;
> > > +     case MPI3_DEVICE0_FLAGS_MAX_WRITE_SAME_2048_LB:
> > > +             tgtdev->ws_len =3D 2048;
> > > +             break;
> > > +     case MPI3_DEVICE0_FLAGS_MAX_WRITE_SAME_NO_LIMIT:
> > > +     default:
> > > +             tgtdev->ws_len =3D 0;
> > > +             break;
> > > +     }
> > > +
> > >       switch (tgtdev->dev_type) {
> > >       case MPI3_DEVICE_DEVFORM_SAS_SATA:
> > >       {
> > > @@ -5649,6 +5664,7 @@ static void mpi3mr_invalidate_devhandles(struct=
 mpi3mr_softc *sc)
> > >                       target->io_throttle_enabled =3D 0;
> > >                       target->io_divert =3D 0;
> > >                       target->throttle_group =3D NULL;
> > > +                     target->ws_len =3D 0;
> > >               }
> > >       }
> > >       mtx_unlock_spin(&sc->target_lock);
> > > diff --git a/sys/dev/mpi3mr/mpi3mr.h b/sys/dev/mpi3mr/mpi3mr.h
> > > index 2f91b0b702dd..fa50ed035fc2 100644
> > > --- a/sys/dev/mpi3mr/mpi3mr.h
> > > +++ b/sys/dev/mpi3mr/mpi3mr.h
> > > @@ -232,6 +232,8 @@ extern char fmt_os_ver[16];
> > >
> > >  #define MPI3MR_PERIODIC_DELAY        1       /* 1 second heartbeat/w=
atchdog check */
> > >
> > > +#define      WRITE_SAME_32   0x0d
> > > +
> > >  struct completion {
> > >       unsigned int done;
> > >       struct mtx lock;
> > > diff --git a/sys/dev/mpi3mr/mpi3mr_cam.c b/sys/dev/mpi3mr/mpi3mr_cam.c
> > > index d4cb7e9265dd..dca194a5c8cd 100644
> > > --- a/sys/dev/mpi3mr/mpi3mr_cam.c
> > > +++ b/sys/dev/mpi3mr/mpi3mr_cam.c
> > > @@ -83,6 +83,7 @@
> > >  #include "mpi3mr.h"
> > >  #include <sys/time.h>                        /* XXX for pcpu.h */
> > >  #include <sys/pcpu.h>                        /* XXX for PCPU_GET */
> > > +#include <asm/unaligned.h>
> > >
> > >  #define      smp_processor_id()  PCPU_GET(cpuid)
> > >
> > > @@ -102,6 +103,37 @@ extern void mpi3mr_add_sg_single(void *paddr, U8=
 flags, U32 length,
> > >
> > >  static U32 event_count;
> > >
> > > +static
> > > +inline void mpi3mr_divert_ws(Mpi3SCSIIORequest_t *req,
> > > +                          struct ccb_scsiio *csio,
> > > +                          U16 ws_len)
> > > +{
> > > +     U8 unmap =3D 0, ndob =3D 0;
> > > +     U32 num_blocks =3D 0;
> > > +     U8 opcode =3D scsiio_cdb_ptr(csio)[0];
> > > +     U16 service_action =3D ((scsiio_cdb_ptr(csio)[8] << 8) | scsiio=
_cdb_ptr(csio)[9]);
> > > +
> > > +
> > > +     if (opcode =3D=3D WRITE_SAME_16 ||
> > > +        (opcode =3D=3D VARIABLE_LEN_CDB &&
> > > +         service_action =3D=3D WRITE_SAME_32)) {
> > > +
> > > +             int unmap_ndob_index =3D (opcode =3D=3D WRITE_SAME_16) =
? 1 : 10;
> > > +
> > > +             unmap =3D scsiio_cdb_ptr(csio)[unmap_ndob_index] & 0x08;
> > > +             ndob =3D scsiio_cdb_ptr(csio)[unmap_ndob_index] & 0x01;
> > > +             num_blocks =3D get_unaligned_be32(scsiio_cdb_ptr(csio) +
> > > +                                             ((opcode =3D=3D WRITE_S=
AME_16) ? 10 : 28));
> > > +
> > > +             /* Check conditions for diversion to firmware */
> > > +             if (unmap && ndob && num_blocks > ws_len) {
> > > +                     req->MsgFlags |=3D MPI3_SCSIIO_MSGFLAGS_DIVERT_=
TO_FIRMWARE;
> > > +                     req->Flags =3D htole32(le32toh(req->Flags) |
> > > +                                          MPI3_SCSIIO_FLAGS_DIVERT_R=
EASON_WRITE_SAME_TOO_LARGE);
> > > +             }
> > > +     }
> > > +}
> > > +
> > >  static void mpi3mr_prepare_sgls(void *arg,
> > >       bus_dma_segment_t *segs, int nsegs, int error)
> > >  {
> > > @@ -1080,6 +1112,9 @@ mpi3mr_action_scsiio(struct mpi3mr_cam_softc *c=
am_sc, union ccb *ccb)
> > >               break;
> > >       }
> > >
> > > +     if (targ->ws_len)
> > > +             mpi3mr_divert_ws(req, csio, targ->ws_len);
> > > +
> > >       req->Flags =3D htole32(mpi_control);
> > >
> > >       if (csio->ccb_h.flags & CAM_CDB_POINTER)
> > > diff --git a/sys/dev/mpi3mr/mpi3mr_cam.h b/sys/dev/mpi3mr/mpi3mr_cam.h
> > > index 3a0526217f86..115ce0c4b8d7 100644
> > > --- a/sys/dev/mpi3mr/mpi3mr_cam.h
> > > +++ b/sys/dev/mpi3mr/mpi3mr_cam.h
> > > @@ -121,6 +121,7 @@ struct mpi3mr_target {
> > >       struct mpi3mr_throttle_group_info *throttle_group;
> > >       uint64_t        q_depth;
> > >       enum mpi3mr_target_state state;
> > > +     uint16_t        ws_len;
> > >  };
> > >
> > >  struct mpi3mr_cam_softc {
> > > diff --git a/sys/modules/mpi3mr/Makefile b/sys/modules/mpi3mr/Makefile
> > > index 3f1f63a94ac3..39aa2e3f0ddd 100644
> > > --- a/sys/modules/mpi3mr/Makefile
> > > +++ b/sys/modules/mpi3mr/Makefile
> > > @@ -6,6 +6,9 @@ SRCS=3D mpi3mr_pci.c mpi3mr.c mpi3mr_cam.c mpi3mr_app=
.c
> > >  SRCS+=3D       opt_cam.h
> > >  SRCS+=3D       device_if.h bus_if.h pci_if.h
> > >
> > > +CFLAGS+=3D -I${SRCTOP}/sys/compat/linuxkpi/common/include
> > > +DEBUG_FLAGS=3D -g
> >   ^^^^^^^^^^^^^^^^
> > > +
> > >  .include <bsd.kmod.mk>
> > >
> > >  CWARNFLAGS.mpi3mr_sas.c=3D             ${NO_WUNNEEDED_INTERNAL_DECL}
> >
> > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D249068#c2
> >
> > --
> > Herbert



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?87msbipbve.wl-herbert>