From eeee1086073e0058243c8554738271561bde81f1 Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Thu, 14 Aug 2025 19:32:16 +0200 Subject: [PATCH 01/10] scsi: pm80xx: Restore support for expanders Commit 0f630c58e31a ("scsi: pm80xx: Do not use libsas port ID") broke support for expanders. After the commit, devices behind an expander are no longer detected. Simply reverting the commit restores support for devices behind an expander. Instead of reverting the commit (and reintroducing a helper to get the port), get the port directly from the lldd_port pointer in struct asd_sas_port. Fixes: 0f630c58e31a ("scsi: pm80xx: Do not use libsas port ID") Suggested-by: Igor Pylypiv Reviewed-by: Igor Pylypiv Signed-off-by: Niklas Cassel Link: https://lore.kernel.org/r/20250814173215.1765055-13-cassel@kernel.org Reviewed-by: Damien Le Moal Tested-by: Damien Le Moal Signed-off-by: Martin K. Petersen --- drivers/scsi/pm8001/pm8001_sas.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c index f7067878b34f..753c09363cbb 100644 --- a/drivers/scsi/pm8001/pm8001_sas.c +++ b/drivers/scsi/pm8001/pm8001_sas.c @@ -477,7 +477,7 @@ int pm8001_queue_command(struct sas_task *task, gfp_t gfp_flags) struct pm8001_device *pm8001_dev = dev->lldd_dev; bool internal_abort = sas_is_internal_abort(task); struct pm8001_hba_info *pm8001_ha; - struct pm8001_port *port = NULL; + struct pm8001_port *port; struct pm8001_ccb_info *ccb; unsigned long flags; u32 n_elem = 0; @@ -502,8 +502,7 @@ int pm8001_queue_command(struct sas_task *task, gfp_t gfp_flags) spin_lock_irqsave(&pm8001_ha->lock, flags); - pm8001_dev = dev->lldd_dev; - port = pm8001_ha->phy[pm8001_dev->attached_phy].port; + port = dev->port->lldd_port; if (!internal_abort && (DEV_IS_GONE(pm8001_dev) || !port || !port->port_attached)) { From 251be2f6037fb7ab399f68cd7428ff274133d693 Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Thu, 14 Aug 2025 19:32:17 +0200 Subject: [PATCH 02/10] scsi: pm80xx: Fix array-index-out-of-of-bounds on rmmod Since commit f7b705c238d1 ("scsi: pm80xx: Set phy_attached to zero when device is gone") UBSAN reports: UBSAN: array-index-out-of-bounds in drivers/scsi/pm8001/pm8001_sas.c:786:17 index 28 is out of range for type 'pm8001_phy [16]' on rmmod when using an expander. For a direct attached device, attached_phy contains the local phy id. For a device behind an expander, attached_phy contains the remote phy id, not the local phy id. I.e. while pm8001_ha will have pm8001_ha->chip->n_phy local phys, for a device behind an expander, attached_phy can be much larger than pm8001_ha->chip->n_phy (depending on the amount of phys of the expander). E.g. on my system pm8001_ha has 8 phys with phy ids 0-7. One of the ports has an expander connected. The expander has 31 phys with phy ids 0-30. The pm8001_ha->phy array only contains the phys of the HBA. It does not contain the phys of the expander. Thus, it is wrong to use attached_phy to index the pm8001_ha->phy array for a device behind an expander. Thus, we can only clear phy_attached for devices that are directly attached. Fixes: f7b705c238d1 ("scsi: pm80xx: Set phy_attached to zero when device is gone") Reviewed-by: Igor Pylypiv Signed-off-by: Niklas Cassel Link: https://lore.kernel.org/r/20250814173215.1765055-14-cassel@kernel.org Reviewed-by: Damien Le Moal Tested-by: Damien Le Moal Signed-off-by: Martin K. Petersen --- drivers/scsi/pm8001/pm8001_sas.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c index 753c09363cbb..3e1dac4b820f 100644 --- a/drivers/scsi/pm8001/pm8001_sas.c +++ b/drivers/scsi/pm8001/pm8001_sas.c @@ -749,6 +749,7 @@ static void pm8001_dev_gone_notify(struct domain_device *dev) unsigned long flags = 0; struct pm8001_hba_info *pm8001_ha; struct pm8001_device *pm8001_dev = dev->lldd_dev; + struct domain_device *parent_dev = dev->parent; pm8001_ha = pm8001_find_ha_by_dev(dev); spin_lock_irqsave(&pm8001_ha->lock, flags); @@ -765,7 +766,13 @@ static void pm8001_dev_gone_notify(struct domain_device *dev) spin_lock_irqsave(&pm8001_ha->lock, flags); } PM8001_CHIP_DISP->dereg_dev_req(pm8001_ha, device_id); - pm8001_ha->phy[pm8001_dev->attached_phy].phy_attached = 0; + + /* + * The phy array only contains local phys. Thus, we cannot clear + * phy_attached for a device behind an expander. + */ + if (!(parent_dev && dev_is_expander(parent_dev->dev_type))) + pm8001_ha->phy[pm8001_dev->attached_phy].phy_attached = 0; pm8001_free_dev(pm8001_dev); } else { pm8001_dbg(pm8001_ha, DISC, "Found dev has gone.\n"); From e5eb72c92eb724aa14c50c7d92d1a576dd50d7e6 Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Thu, 14 Aug 2025 19:32:18 +0200 Subject: [PATCH 03/10] scsi: libsas: Add dev_parent_is_expander() helper Many libsas drivers check if the parent of the device is an expander. Create a helper that the libsas drivers will use in follow up commits. Suggested-by: Damien Le Moal Signed-off-by: Niklas Cassel Link: https://lore.kernel.org/r/20250814173215.1765055-15-cassel@kernel.org Reviewed-by: Damien Le Moal Reviewed-by: John Garry Reviewed-by: Jason Yan Signed-off-by: Martin K. Petersen --- drivers/scsi/libsas/sas_expander.c | 5 +---- include/scsi/libsas.h | 8 ++++++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c index 869b5d4db44c..d953225f6cc2 100644 --- a/drivers/scsi/libsas/sas_expander.c +++ b/drivers/scsi/libsas/sas_expander.c @@ -1313,10 +1313,7 @@ static int sas_check_parent_topology(struct domain_device *child) int i; int res = 0; - if (!child->parent) - return 0; - - if (!dev_is_expander(child->parent->dev_type)) + if (!dev_parent_is_expander(child)) return 0; parent_ex = &child->parent->ex_dev; diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h index ba460b6c0374..8d38565e99fa 100644 --- a/include/scsi/libsas.h +++ b/include/scsi/libsas.h @@ -203,6 +203,14 @@ static inline bool dev_is_expander(enum sas_device_type type) type == SAS_FANOUT_EXPANDER_DEVICE; } +static inline bool dev_parent_is_expander(struct domain_device *dev) +{ + if (!dev->parent) + return false; + + return dev_is_expander(dev->parent->dev_type); +} + static inline void INIT_SAS_WORK(struct sas_work *sw, void (*fn)(struct work_struct *)) { INIT_WORK(&sw->work, fn); From 0c0188dd200e4709da72cc3d65c012f34030b950 Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Thu, 14 Aug 2025 19:32:19 +0200 Subject: [PATCH 04/10] scsi: hisi_sas: Use dev_parent_is_expander() helper Make use of the dev_parent_is_expander() helper. Signed-off-by: Niklas Cassel Link: https://lore.kernel.org/r/20250814173215.1765055-16-cassel@kernel.org Reviewed-by: Damien Le Moal Reviewed-by: John Garry Signed-off-by: Martin K. Petersen --- drivers/scsi/hisi_sas/hisi_sas_main.c | 2 +- drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 6 ++---- drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 6 ++---- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c index d1a4cc69d408..30a9c6612651 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_main.c +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c @@ -876,7 +876,7 @@ static int hisi_sas_dev_found(struct domain_device *device) device->lldd_dev = sas_dev; hisi_hba->hw->setup_itct(hisi_hba, sas_dev); - if (parent_dev && dev_is_expander(parent_dev->dev_type)) { + if (dev_parent_is_expander(device)) { int phy_no; phy_no = sas_find_attached_phy_id(&parent_dev->ex_dev, device); diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c index 4431698a5d78..f3516a0611dd 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c @@ -925,7 +925,6 @@ static void setup_itct_v2_hw(struct hisi_hba *hisi_hba, struct device *dev = hisi_hba->dev; u64 qw0, device_id = sas_dev->device_id; struct hisi_sas_itct *itct = &hisi_hba->itct[device_id]; - struct domain_device *parent_dev = device->parent; struct asd_sas_port *sas_port = device->port; struct hisi_sas_port *port = to_hisi_sas_port(sas_port); u64 sas_addr; @@ -942,7 +941,7 @@ static void setup_itct_v2_hw(struct hisi_hba *hisi_hba, break; case SAS_SATA_DEV: case SAS_SATA_PENDING: - if (parent_dev && dev_is_expander(parent_dev->dev_type)) + if (dev_parent_is_expander(device)) qw0 = HISI_SAS_DEV_TYPE_STP << ITCT_HDR_DEV_TYPE_OFF; else qw0 = HISI_SAS_DEV_TYPE_SATA << ITCT_HDR_DEV_TYPE_OFF; @@ -2494,7 +2493,6 @@ static void prep_ata_v2_hw(struct hisi_hba *hisi_hba, { struct sas_task *task = slot->task; struct domain_device *device = task->dev; - struct domain_device *parent_dev = device->parent; struct hisi_sas_device *sas_dev = device->lldd_dev; struct hisi_sas_cmd_hdr *hdr = slot->cmd_hdr; struct asd_sas_port *sas_port = device->port; @@ -2509,7 +2507,7 @@ static void prep_ata_v2_hw(struct hisi_hba *hisi_hba, /* create header */ /* dw0 */ dw0 = port->id << CMD_HDR_PORT_OFF; - if (parent_dev && dev_is_expander(parent_dev->dev_type)) { + if (dev_parent_is_expander(device)) { dw0 |= 3 << CMD_HDR_CMD_OFF; } else { phy_id = device->phy->identify.phy_identifier; diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c index 2f3d61abab3a..2f9e01717ef3 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c @@ -874,7 +874,6 @@ static void setup_itct_v3_hw(struct hisi_hba *hisi_hba, struct device *dev = hisi_hba->dev; u64 qw0, device_id = sas_dev->device_id; struct hisi_sas_itct *itct = &hisi_hba->itct[device_id]; - struct domain_device *parent_dev = device->parent; struct asd_sas_port *sas_port = device->port; struct hisi_sas_port *port = to_hisi_sas_port(sas_port); u64 sas_addr; @@ -891,7 +890,7 @@ static void setup_itct_v3_hw(struct hisi_hba *hisi_hba, break; case SAS_SATA_DEV: case SAS_SATA_PENDING: - if (parent_dev && dev_is_expander(parent_dev->dev_type)) + if (dev_parent_is_expander(device)) qw0 = HISI_SAS_DEV_TYPE_STP << ITCT_HDR_DEV_TYPE_OFF; else qw0 = HISI_SAS_DEV_TYPE_SATA << ITCT_HDR_DEV_TYPE_OFF; @@ -1476,7 +1475,6 @@ static void prep_ata_v3_hw(struct hisi_hba *hisi_hba, { struct sas_task *task = slot->task; struct domain_device *device = task->dev; - struct domain_device *parent_dev = device->parent; struct hisi_sas_device *sas_dev = device->lldd_dev; struct hisi_sas_cmd_hdr *hdr = slot->cmd_hdr; struct asd_sas_port *sas_port = device->port; @@ -1487,7 +1485,7 @@ static void prep_ata_v3_hw(struct hisi_hba *hisi_hba, u32 dw1 = 0, dw2 = 0; hdr->dw0 = cpu_to_le32(port->id << CMD_HDR_PORT_OFF); - if (parent_dev && dev_is_expander(parent_dev->dev_type)) { + if (dev_parent_is_expander(device)) { hdr->dw0 |= cpu_to_le32(3 << CMD_HDR_CMD_OFF); } else { phy_id = device->phy->identify.phy_identifier; From ad6ae22927a7ed411d892d80934610c49882a378 Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Thu, 14 Aug 2025 19:32:20 +0200 Subject: [PATCH 05/10] scsi: isci: Use dev_parent_is_expander() helper Make use of the dev_parent_is_expander() helper. Signed-off-by: Niklas Cassel Link: https://lore.kernel.org/r/20250814173215.1765055-17-cassel@kernel.org Reviewed-by: Damien Le Moal Reviewed-by: John Garry Signed-off-by: Martin K. Petersen --- drivers/scsi/isci/remote_device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/isci/remote_device.c b/drivers/scsi/isci/remote_device.c index 82deb6a83a8c..4c7462965ea1 100644 --- a/drivers/scsi/isci/remote_device.c +++ b/drivers/scsi/isci/remote_device.c @@ -1434,7 +1434,7 @@ static enum sci_status isci_remote_device_construct(struct isci_port *iport, struct domain_device *dev = idev->domain_dev; enum sci_status status; - if (dev->parent && dev_is_expander(dev->parent->dev_type)) + if (dev_parent_is_expander(dev)) status = sci_remote_device_ea_construct(iport, idev); else status = sci_remote_device_da_construct(iport, idev); From 3adf779489839516d61c3ead08cc148a7374b581 Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Thu, 14 Aug 2025 19:32:21 +0200 Subject: [PATCH 06/10] scsi: mvsas: Use dev_parent_is_expander() helper Make use of the dev_parent_is_expander() helper. Signed-off-by: Niklas Cassel Link: https://lore.kernel.org/r/20250814173215.1765055-18-cassel@kernel.org Reviewed-by: Damien Le Moal Reviewed-by: John Garry Signed-off-by: Martin K. Petersen --- drivers/scsi/mvsas/mv_sas.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c index 15b3d9d55a4b..f2e7997d5b9d 100644 --- a/drivers/scsi/mvsas/mv_sas.c +++ b/drivers/scsi/mvsas/mv_sas.c @@ -1175,7 +1175,7 @@ static int mvs_dev_found_notify(struct domain_device *dev, int lock) mvi_device->dev_type = dev->dev_type; mvi_device->mvi_info = mvi; mvi_device->sas_device = dev; - if (parent_dev && dev_is_expander(parent_dev->dev_type)) { + if (dev_parent_is_expander(dev)) { int phy_id; phy_id = sas_find_attached_phy_id(&parent_dev->ex_dev, dev); From 35e388696c3f3b6bf70e2010873c5e0c1d37d579 Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Thu, 14 Aug 2025 19:32:22 +0200 Subject: [PATCH 07/10] scsi: pm80xx: Use dev_parent_is_expander() helper Make use of the dev_parent_is_expander() helper. Signed-off-by: Niklas Cassel Link: https://lore.kernel.org/r/20250814173215.1765055-19-cassel@kernel.org Reviewed-by: Damien Le Moal Reviewed-by: John Garry Reviewed-by: Igor Pylypiv Acked-by: Jack Wang Signed-off-by: Martin K. Petersen --- drivers/scsi/pm8001/pm8001_hwi.c | 8 +++----- drivers/scsi/pm8001/pm8001_sas.c | 5 ++--- drivers/scsi/pm8001/pm80xx_hwi.c | 8 +++----- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c index 42a4eeac24c9..fb4913547b00 100644 --- a/drivers/scsi/pm8001/pm8001_hwi.c +++ b/drivers/scsi/pm8001/pm8001_hwi.c @@ -2163,8 +2163,7 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb) /* Print sas address of IO failed device */ if ((status != IO_SUCCESS) && (status != IO_OVERFLOW) && (status != IO_UNDERFLOW)) { - if (!((t->dev->parent) && - (dev_is_expander(t->dev->parent->dev_type)))) { + if (!dev_parent_is_expander(t->dev)) { for (i = 0, j = 4; j <= 7 && i <= 3; i++, j++) sata_addr_low[i] = pm8001_ha->sas_addr[j]; for (i = 0, j = 0; j <= 3 && i <= 3; i++, j++) @@ -4168,7 +4167,6 @@ static int pm8001_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha, u16 firstBurstSize = 0; u16 ITNT = 2000; struct domain_device *dev = pm8001_dev->sas_device; - struct domain_device *parent_dev = dev->parent; struct pm8001_port *port = dev->port->lldd_port; memset(&payload, 0, sizeof(payload)); @@ -4186,8 +4184,8 @@ static int pm8001_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha, dev_is_expander(pm8001_dev->dev_type)) stp_sspsmp_sata = 0x01; /*ssp or smp*/ } - if (parent_dev && dev_is_expander(parent_dev->dev_type)) - phy_id = parent_dev->ex_dev.ex_phy->phy_id; + if (dev_parent_is_expander(dev)) + phy_id = dev->parent->ex_dev.ex_phy->phy_id; else phy_id = pm8001_dev->attached_phy; opc = OPC_INB_REG_DEV; diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c index 3e1dac4b820f..2bdeace6c6bf 100644 --- a/drivers/scsi/pm8001/pm8001_sas.c +++ b/drivers/scsi/pm8001/pm8001_sas.c @@ -700,7 +700,7 @@ static int pm8001_dev_found_notify(struct domain_device *dev) dev->lldd_dev = pm8001_device; pm8001_device->dev_type = dev->dev_type; pm8001_device->dcompletion = &completion; - if (parent_dev && dev_is_expander(parent_dev->dev_type)) { + if (dev_parent_is_expander(dev)) { int phy_id; phy_id = sas_find_attached_phy_id(&parent_dev->ex_dev, dev); @@ -749,7 +749,6 @@ static void pm8001_dev_gone_notify(struct domain_device *dev) unsigned long flags = 0; struct pm8001_hba_info *pm8001_ha; struct pm8001_device *pm8001_dev = dev->lldd_dev; - struct domain_device *parent_dev = dev->parent; pm8001_ha = pm8001_find_ha_by_dev(dev); spin_lock_irqsave(&pm8001_ha->lock, flags); @@ -771,7 +770,7 @@ static void pm8001_dev_gone_notify(struct domain_device *dev) * The phy array only contains local phys. Thus, we cannot clear * phy_attached for a device behind an expander. */ - if (!(parent_dev && dev_is_expander(parent_dev->dev_type))) + if (!dev_parent_is_expander(dev)) pm8001_ha->phy[pm8001_dev->attached_phy].phy_attached = 0; pm8001_free_dev(pm8001_dev); } else { diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c index c1bae995a412..546d0d26f7a1 100644 --- a/drivers/scsi/pm8001/pm80xx_hwi.c +++ b/drivers/scsi/pm8001/pm80xx_hwi.c @@ -2340,8 +2340,7 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, /* Print sas address of IO failed device */ if ((status != IO_SUCCESS) && (status != IO_OVERFLOW) && (status != IO_UNDERFLOW)) { - if (!((t->dev->parent) && - (dev_is_expander(t->dev->parent->dev_type)))) { + if (!dev_parent_is_expander(t->dev)) { for (i = 0, j = 4; i <= 3 && j <= 7; i++, j++) sata_addr_low[i] = pm8001_ha->sas_addr[j]; for (i = 0, j = 0; i <= 3 && j <= 3; i++, j++) @@ -4780,7 +4779,6 @@ static int pm80xx_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha, u16 firstBurstSize = 0; u16 ITNT = 2000; struct domain_device *dev = pm8001_dev->sas_device; - struct domain_device *parent_dev = dev->parent; struct pm8001_port *port = dev->port->lldd_port; memset(&payload, 0, sizeof(payload)); @@ -4799,8 +4797,8 @@ static int pm80xx_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha, dev_is_expander(pm8001_dev->dev_type)) stp_sspsmp_sata = 0x01; /*ssp or smp*/ } - if (parent_dev && dev_is_expander(parent_dev->dev_type)) - phy_id = parent_dev->ex_dev.ex_phy->phy_id; + if (dev_parent_is_expander(dev)) + phy_id = dev->parent->ex_dev.ex_phy->phy_id; else phy_id = pm8001_dev->attached_phy; From b4ec98303f9fc9b1da0053106716db6a7e002d8b Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Thu, 14 Aug 2025 19:32:23 +0200 Subject: [PATCH 08/10] scsi: pm80xx: Add helper function to get the local phy id Avoid duplicated code by adding a helper to get the local phy id. No functional changes intended. Signed-off-by: Niklas Cassel Link: https://lore.kernel.org/r/20250814173215.1765055-20-cassel@kernel.org Reviewed-by: Damien Le Moal Signed-off-by: Martin K. Petersen --- drivers/scsi/pm8001/pm8001_hwi.c | 7 +++---- drivers/scsi/pm8001/pm8001_sas.c | 10 ++++++++++ drivers/scsi/pm8001/pm8001_sas.h | 1 + drivers/scsi/pm8001/pm80xx_hwi.c | 6 ++---- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c index fb4913547b00..8005995a317c 100644 --- a/drivers/scsi/pm8001/pm8001_hwi.c +++ b/drivers/scsi/pm8001/pm8001_hwi.c @@ -4184,10 +4184,9 @@ static int pm8001_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha, dev_is_expander(pm8001_dev->dev_type)) stp_sspsmp_sata = 0x01; /*ssp or smp*/ } - if (dev_parent_is_expander(dev)) - phy_id = dev->parent->ex_dev.ex_phy->phy_id; - else - phy_id = pm8001_dev->attached_phy; + + phy_id = pm80xx_get_local_phy_id(dev); + opc = OPC_INB_REG_DEV; linkrate = (pm8001_dev->sas_device->linkrate < dev->port->linkrate) ? pm8001_dev->sas_device->linkrate : dev->port->linkrate; diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c index 2bdeace6c6bf..5595913eb7fc 100644 --- a/drivers/scsi/pm8001/pm8001_sas.c +++ b/drivers/scsi/pm8001/pm8001_sas.c @@ -130,6 +130,16 @@ static void pm80xx_get_tag_opcodes(struct sas_task *task, int *ata_op, } } +u32 pm80xx_get_local_phy_id(struct domain_device *dev) +{ + struct pm8001_device *pm8001_dev = dev->lldd_dev; + + if (dev_parent_is_expander(dev)) + return dev->parent->ex_dev.ex_phy->phy_id; + + return pm8001_dev->attached_phy; +} + void pm80xx_show_pending_commands(struct pm8001_hba_info *pm8001_ha, struct pm8001_device *target_pm8001_dev) { diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h index 334485bb2c12..91b2cdf3535c 100644 --- a/drivers/scsi/pm8001/pm8001_sas.h +++ b/drivers/scsi/pm8001/pm8001_sas.h @@ -798,6 +798,7 @@ void pm8001_setds_completion(struct domain_device *dev); void pm8001_tmf_aborted(struct sas_task *task); void pm80xx_show_pending_commands(struct pm8001_hba_info *pm8001_ha, struct pm8001_device *dev); +u32 pm80xx_get_local_phy_id(struct domain_device *dev); #endif diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c index 546d0d26f7a1..31960b72c1e9 100644 --- a/drivers/scsi/pm8001/pm80xx_hwi.c +++ b/drivers/scsi/pm8001/pm80xx_hwi.c @@ -4797,10 +4797,8 @@ static int pm80xx_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha, dev_is_expander(pm8001_dev->dev_type)) stp_sspsmp_sata = 0x01; /*ssp or smp*/ } - if (dev_parent_is_expander(dev)) - phy_id = dev->parent->ex_dev.ex_phy->phy_id; - else - phy_id = pm8001_dev->attached_phy; + + phy_id = pm80xx_get_local_phy_id(dev); opc = OPC_INB_REG_DEV; From ad70c6bc776b53e61c8db6533c833aff0ff5da8b Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Thu, 14 Aug 2025 19:32:24 +0200 Subject: [PATCH 09/10] scsi: pm80xx: Fix pm8001_abort_task() for chip_8006 when using an expander For a direct attached device, attached_phy contains the local phy id. For a device behind an expander, attached_phy contains the remote phy id, not the local phy id. The pm8001_ha->phy array only contains the phys of the HBA. It does not contain the phys of the expander. Thus, you cannot use attached_phy to index the pm8001_ha->phy array, without first verifying that the device is directly attached. Use the pm80xx_get_local_phy_id() helper to make sure that we use the local phy id to index the array, regardless if the device is directly attached or not. Fixes: 869ddbdcae3b ("scsi: pm80xx: corrected SATA abort handling sequence.") Reviewed-by: Igor Pylypiv Signed-off-by: Niklas Cassel Link: https://lore.kernel.org/r/20250814173215.1765055-21-cassel@kernel.org Reviewed-by: Damien Le Moal Signed-off-by: Martin K. Petersen --- drivers/scsi/pm8001/pm8001_sas.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c index 5595913eb7fc..c5354263b45e 100644 --- a/drivers/scsi/pm8001/pm8001_sas.c +++ b/drivers/scsi/pm8001/pm8001_sas.c @@ -1063,7 +1063,7 @@ int pm8001_abort_task(struct sas_task *task) struct pm8001_hba_info *pm8001_ha; struct pm8001_device *pm8001_dev; int rc = TMF_RESP_FUNC_FAILED, ret; - u32 phy_id, port_id; + u32 port_id; struct sas_task_slow slow_task; if (!task->lldd_task || !task->dev) @@ -1072,7 +1072,6 @@ int pm8001_abort_task(struct sas_task *task) dev = task->dev; pm8001_dev = dev->lldd_dev; pm8001_ha = pm8001_find_ha_by_dev(dev); - phy_id = pm8001_dev->attached_phy; if (PM8001_CHIP_DISP->fatal_errors(pm8001_ha)) { // If the controller is seeing fatal errors @@ -1104,7 +1103,8 @@ int pm8001_abort_task(struct sas_task *task) if (pm8001_ha->chip_id == chip_8006) { DECLARE_COMPLETION_ONSTACK(completion_reset); DECLARE_COMPLETION_ONSTACK(completion); - struct pm8001_phy *phy = pm8001_ha->phy + phy_id; + u32 phy_id = pm80xx_get_local_phy_id(dev); + struct pm8001_phy *phy = &pm8001_ha->phy[phy_id]; port_id = phy->port->port_id; /* 1. Set Device state as Recovery */ From 03f69351b63ea9583bd91c60a83d0a8573ea29fb Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Thu, 14 Aug 2025 19:32:25 +0200 Subject: [PATCH 10/10] scsi: pm80xx: Use pm80xx_get_local_phy_id() to access phy array While the current code is perfectly fine (because we verify that the device is directly attached before using attached_phy to index the pm8001_ha->phy array), let's use the pm80xx_get_local_phy_id() helper anyway, to reduce the chance that someone will copy paste this pattern to other parts of the driver. Note that in this specific case, we still need to keep the check that the device is not behind an expander, because we do not want to clear attached_phy of the expander if a device behind the expander disappears (as that would disable all the other devices behind the expander). However, if it is the expander itself that disappears, attached_phy will be cleared, just like it would for any other directly attached device. Signed-off-by: Niklas Cassel Link: https://lore.kernel.org/r/20250814173215.1765055-22-cassel@kernel.org Reviewed-by: Damien Le Moal Reviewed-by: Igor Pylypiv Acked-by: Jack Wang Signed-off-by: Martin K. Petersen --- drivers/scsi/pm8001/pm8001_sas.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c index c5354263b45e..6a8d35aea93a 100644 --- a/drivers/scsi/pm8001/pm8001_sas.c +++ b/drivers/scsi/pm8001/pm8001_sas.c @@ -780,8 +780,11 @@ static void pm8001_dev_gone_notify(struct domain_device *dev) * The phy array only contains local phys. Thus, we cannot clear * phy_attached for a device behind an expander. */ - if (!dev_parent_is_expander(dev)) - pm8001_ha->phy[pm8001_dev->attached_phy].phy_attached = 0; + if (!dev_parent_is_expander(dev)) { + u32 phy_id = pm80xx_get_local_phy_id(dev); + + pm8001_ha->phy[phy_id].phy_attached = 0; + } pm8001_free_dev(pm8001_dev); } else { pm8001_dbg(pm8001_ha, DISC, "Found dev has gone.\n");