Re: [PATCH] scsi: mpi3mr: use DevicePage0 link rate only for direct-attached targets

[Илья Хомяков]

Hello.
After sending the email, I didn't receive any response from anyone.

Regards,
Ilya.

пн, 12 окт. 2026 г. в 18:48, Thorsten Leemhuis <regressions@xxxxxxxxxxxxx>:
>
> On 8/4/26 16:28, Ilya Khomyakov wrote:
> > This patch fixes a regression in the Broadcom MPI3 Storage Controller
> > driver under drivers/scsi/mpi3mr/.
>
> Ilya, what's the status of this regression fix? It looks like it fall
> through the cracks. Or was there some progress that I missed (which can
> easily happen!)
>
> Ciao, Thorsten
> > Commit c273c14b0294 ("scsi: mpi3mr: Use negotiated link rate from
> > DevicePage0") changed mpi3mr to prefer the cached DevicePage0 rate.
> >
> > DevicePage0 stores negotiated_link_rate as one standalone MPI3 SAS
> > link-rate code, while SAS PHY Page 0 and SAS Expander Page 1 store logical
> > and physical rates as two nibbles of one packed byte.
> >
> > mpi3mr_get_sas_negotiated_logical_linkrate() currently sends both formats
> > through the same high-nibble extraction at its common exit. A valid
> > DevicePage0 value of 0x0b or 0x0c is therefore converted to zero. The later
> > minimum-rate safeguard then publishes the value as 1.5 Gbit/s.
> >
> > For example:
> >
> >   DevicePage0 value:            0x0b
> >   packed-field extraction:      (0x0b & 0xf0) >> 4 = 0x00
> >   minimum-rate substitution:    0x00 -> 0x08
> >   Linux SAS transport result:   1.5 Gbit/s
> >
> > There is also a topology distinction. The caller uses the returned value to
> > update the parent sas_phy. In an expander topology with the HBA-to-expander
> > link limited to 12 Gbit/s, DevicePage0 reported 0x0b for every tested
> > target, while Expander Page 1 reported 0xcc for several disk-facing PHYs
> > that were operating at 22.5 Gbit/s. The cached target value therefore
> > cannot unconditionally replace the local parent expander PHY value.
> >
> > The original failure was reproduced on an eHBA 9600 controller with a
> > SAS4016 IOC and a 46-PHY Microchip expander. Before the fix, a target with
> > DevicePage0 negotiated_link_rate 0x0b could remain visible in sysfs as
> > 1.5 Gbit/s when a replayed topology event contained current=0x0b and
> > previous=0x0b and therefore skipped the Linux-side update.
> >
> > Validate the standalone DevicePage0 code and use it directly only for a
> > directly attached target. For an expander-attached target, retain the
> > existing Expander Page 1 path because it describes the local parent
> > expander PHY being updated. Retain the SAS PHY Page 0 fallback when a
> > directly attached cached value is invalid.
> >
> > The topology-aware variant was tested in an out-of-tree mpi3mr 8.17.1.0.
> > build. After boot, Linux reported:
> >
> >   PHY 18: 12.0 Gbit/s
> >   PHY 19: 12.0 Gbit/s
> >   PHY 24: 22.5 Gbit/s
> >   PHY 25: 22.5 Gbit/s
> >   PHY 26: 22.5 Gbit/s
> >   PHY 27: 22.5 Gbit/s
> >   PHY 28: 12.0 Gbit/s
> >   PHY 30: 22.5 Gbit/s
> >
> > No tested PHY was incorrectly reported as 1.5 Gbit/s, and the 22.5 Gbit/s
> > disk-facing rates were preserved even though DevicePage0 contained 0x0b.
> >
> > Fixes: c273c14b0294 ("scsi: mpi3mr: Use negotiated link rate from DevicePage0")
> > Signed-off-by: Ilya Khomyakov <khomyakovilya@xxxxxxxxx>
> >
> > ---
> >  drivers/scsi/mpi3mr/mpi3mr_transport.c | 46 ++++++++++++++++++++++----
> >  1 file changed, 39 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c
> > index 240f67a..740fccc 100644
> > --- a/drivers/scsi/mpi3mr/mpi3mr_transport.c
> > +++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c
> > @@ -587,6 +587,30 @@ static enum sas_linkrate mpi3mr_convert_phy_link_rate(u8 link_rate)
> >       return rc;
> >  }
> >
> > +/**
> > + * mpi3mr_sas_link_rate_valid - validate a standalone SAS link-rate code
> > + * @link_rate: MPI3 standalone SAS negotiated link-rate code
> > + *
> > + * DevicePage0 stores one plain negotiated link-rate code. Accept only
> > + * negotiated data rates supported by the SAS transport conversion code;
> > + * reserved or transitional values must use the existing PHY-page path.
> > + *
> > + * Return: true for a supported negotiated data rate, false otherwise.
> > + */
> > +static bool mpi3mr_sas_link_rate_valid(u8 link_rate)
> > +{
> > +     switch (link_rate) {
> > +     case MPI3_SAS_NEG_LINK_RATE_1_5:
> > +     case MPI3_SAS_NEG_LINK_RATE_3_0:
> > +     case MPI3_SAS_NEG_LINK_RATE_6_0:
> > +     case MPI3_SAS_NEG_LINK_RATE_12_0:
> > +     case MPI3_SAS_NEG_LINK_RATE_22_5:
> > +             return true;
> > +     default:
> > +             return false;
> > +     }
> > +}
> > +
> >  /**
> >   * mpi3mr_delete_sas_phy - Remove a single phy from port
> >   * @mrioc: Adapter instance reference
> > @@ -2292,18 +2316,26 @@ void mpi3mr_expander_remove(struct mpi3mr_ioc *mrioc, u64 sas_address,
> >  static u8 mpi3mr_get_sas_negotiated_logical_linkrate(struct mpi3mr_ioc *mrioc,
> >       struct mpi3mr_tgt_dev *tgtdev)
> >  {
> > -     u8 link_rate = MPI3_SAS_NEG_LINK_RATE_1_5, phy_number;
> > +     u8 cached_link_rate, link_rate = MPI3_SAS_NEG_LINK_RATE_1_5;
> > +     u8 phy_number;
> >       struct mpi3_sas_expander_page1 expander_pg1;
> >       struct mpi3_sas_phy_page0 phy_pg0;
> >       u32 phynum_handle;
> >       u16 ioc_status;
> >
> > -     /* First, try to use link rate from DevicePage0 (populated by firmware) */
> > -     if (tgtdev->dev_spec.sas_sata_inf.negotiated_link_rate >=
> > -         MPI3_SAS_NEG_LINK_RATE_1_5) {
> > -             link_rate = tgtdev->dev_spec.sas_sata_inf.negotiated_link_rate;
> > -             goto out;
> > -     }
> > +     cached_link_rate =
> > +             tgtdev->dev_spec.sas_sata_inf.negotiated_link_rate;
> > +
> > +     /*
> > +      * For a directly attached target, DevicePage0 and the parent host PHY
> > +      * describe the same link, so the standalone cached code can be used
> > +      * without packed-field decoding. For an expander-attached target, the
> > +      * caller updates the parent expander PHY and DevicePage0 can differ from
> > +      * that local segment; retain the Expander Page 1 read in that case.
> > +      */
> > +     if ((tgtdev->devpg0_flag & MPI3_DEVICE0_FLAGS_ATT_METHOD_DIR_ATTACHED) &&
> > +         mpi3mr_sas_link_rate_valid(cached_link_rate))
> > +             return cached_link_rate;
> >
> >       /* Fallback to reading from phy pages if DevicePage0 value not available */
> >       phy_number = tgtdev->dev_spec.sas_sata_inf.phy_id;
>