| Message ID | 20250220130546.2289555-2-yangxingui@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: hisi_sas: Fixed IO error caused by port id not updated | expand |
On 20/02/2025 13:05, Xingui Yang wrote: -john.garry@huawei.com (this has not worked in over 2 years ...) > the SAS controller determines the disk to which I/Os are delivered based > on the port id in the DQ entry when SATA disk directly connected. > > When many phys were disconnected immediately and connected again during > I/O sending and port id of phys were changed and used by other link, I/O > may be sent to incorrect disk and data inconsistency on the SATA disk may So is the disk reported gone (from libsas point-of-view) after you unplug? If not, why not? Thanks, John
Hi, John On 2025/2/21 1:35, John Garry wrote: > On 20/02/2025 13:05, Xingui Yang wrote: > > -john.garry@huawei.com (this has not worked in over 2 years ...) Sorry, I used the wrong one. > >> the SAS controller determines the disk to which I/Os are delivered based >> on the port id in the DQ entry when SATA disk directly connected. >> >> When many phys were disconnected immediately and connected again during >> I/O sending and port id of phys were changed and used by other link, I/O >> may be sent to incorrect disk and data inconsistency on the SATA disk may > > > So is the disk reported gone (from libsas point-of-view) after you > unplug? If not, why not? The problem may occur in a scenario where multiple SATA disks are inserted almost at the same time. When phy reset is executed in error processing, other phys are also up, which may cause the hw port id corresponding to the phy to change. The log is as follows: [ 4588.608924] hisi_sas_v3_hw 0000:b4:02.0: phyup: phy2 link_rate=10(sata) [ 4588.609039] sas: phy-8:2 added to port-8:4, phy_mask:0x4 (5000000000000802) [ 4588.609267] sas: DOING DISCOVERY on port 4, pid:69294 [ 4588.609276] hisi_sas_v3_hw 0000:b4:02.0: dev[13:5] found [ 4588.671362] sas: ata40: end_device-8:4: dev error handler [ 4588.846387] hisi_sas_v3_hw 0000:b4:02.0: phydown: phy2 phy_state=0xc3 // phy2's hw port id assign by chip is released [ 4588.846393] hisi_sas_v3_hw 0000:b4:02.0: ignore flutter phy2 down [ 4588.919837] hisi_sas_v3_hw 0000:b4:02.0: phyup: phy3 link_rate=10(sata) // phy3 is assigned the hw port id previously used by phy2 [ 4589.029656] hisi_sas_v3_hw 0000:b4:02.0: phyup: phy2 link_rate=10(sata) // phy2's hw port id is assigned a new one [ 4589.220662] ata40.00: ATA-9: HUH721010ALE600, T3C0, max UDMA/133 [ 4589.220666] ata40.00: 19532873728 sectors, multi 0: LBA48 NCQ (depth 32), AA [ 4589.233022] ata40.00: configured for UDMA/133 In view of the situation corresponding to the above log, the hisi_sas_port.id corresponding to phy2 has not been updated, and the old port id is still used, which will cause the IO delivered to phy2 to be abnormally delivered to the disk of phy3. After force phy, the chip will check whether the phy information matches the port id and intercept this abnormal IO. Thanks. Xingui
Hi Jonh, On 2025/2/24 16:29, John Garry wrote: > On 21/02/2025 01:59, yangxingui wrote: >> Hi, John >> >> On 2025/2/21 1:35, John Garry wrote: >>> On 20/02/2025 13:05, Xingui Yang wrote: >>> >>> -john.garry@huawei.com (this has not worked in over 2 years ...) >> Sorry, I used the wrong one. >>> >>>> the SAS controller determines the disk to which I/Os are delivered >>>> based >>>> on the port id in the DQ entry when SATA disk directly connected. >>>> >>>> When many phys were disconnected immediately and connected again during >>>> I/O sending and port id of phys were changed and used by other link, >>>> I/O >>>> may be sent to incorrect disk and data inconsistency on the SATA >>>> disk may >>> >>> >>> So is the disk reported gone (from libsas point-of-view) after you >>> unplug? If not, why not? >> >> The problem may occur in a scenario where multiple SATA disks are >> inserted almost at the same time. When phy reset is executed in error >> processing, other phys are also up, which may cause the hw port id >> corresponding to the phy to change. The log is as follows: >> >> [ 4588.608924] hisi_sas_v3_hw 0000:b4:02.0: phyup: phy2 >> link_rate=10(sata) >> [ 4588.609039] sas: phy-8:2 added to port-8:4, phy_mask:0x4 >> (5000000000000802) >> [ 4588.609267] sas: DOING DISCOVERY on port 4, pid:69294 >> [ 4588.609276] hisi_sas_v3_hw 0000:b4:02.0: dev[13:5] found >> [ 4588.671362] sas: ata40: end_device-8:4: dev error handler >> [ 4588.846387] hisi_sas_v3_hw 0000:b4:02.0: phydown: phy2 >> phy_state=0xc3 // phy2's hw port id assign by chip is released >> [ 4588.846393] hisi_sas_v3_hw 0000:b4:02.0: ignore flutter phy2 down >> [ 4588.919837] hisi_sas_v3_hw 0000:b4:02.0: phyup: phy3 >> link_rate=10(sata) // phy3 is assigned the hw port id previously used >> by phy2 >> [ 4589.029656] hisi_sas_v3_hw 0000:b4:02.0: phyup: phy2 >> link_rate=10(sata) // phy2's hw port id is assigned a new one >> [ 4589.220662] ata40.00: ATA-9: HUH721010ALE600, T3C0, max UDMA/133 >> [ 4589.220666] ata40.00: 19532873728 sectors, multi 0: LBA48 NCQ >> (depth 32), AA >> [ 4589.233022] ata40.00: configured for UDMA/133 >> >> In view of the situation corresponding to the above log, the >> hisi_sas_port.id corresponding to phy2 has not been updated, and the >> old port id is still used, which will cause the IO delivered to phy2 >> to be abnormally delivered to the disk of phy3. >> >> After force phy, the chip will check whether the phy information >> matches the port id and intercept this abnormal IO. >> > > > So do you mean that all IO to this disk will error? If yes, then this is > good. Yes, IO error or IO result does not meet expectations. As shown in the log below, due to an abnormal port ID, the SNs of the two disks read are the same. [448000.504979] hisi_sas_v3_hw 0000:d4:02.0: phyup: phy1 link_rate=10(sata) [448000.505070] sas: phy-10:1 added to port-10:1, phy_mask:0x2 (5000000000000a01) [448000.505247] sas: DOING DISCOVERY on port 1, pid:2239187 [448000.505255] hisi_sas_v3_hw 0000:d4:02.0: dev[2:5] found [448000.505274] sas: Enter sas_scsi_recover_host busy: 0 failed: 0 [448000.505295] sas: ata31: end_device-10:0: dev error handler [448000.505299] sas: ata32: end_device-10:1: dev error handler [448001.300517] hisi_sas_v3_hw 0000:d4:02.0: phydown: phy1 phy_state=0x1 // phy1's hw port id released [448001.300522] hisi_sas_v3_hw 0000:d4:02.0: ignore flutter phy1 down [448001.436187] hisi_sas_v3_hw 0000:d4:02.0: phyup: phy2 link_rate=10(sata) // phy2 occupies the hardware port ID of phy1 [448001.608766] hisi_sas_v3_hw 0000:d4:02.0: phyup: phy1 link_rate=10(sata) // phy1 was assigned a new hardware port ID [448001.775605] ata32.00: ATA-11: WUH721816ALE6L4, PCGAW660, max UDMA/133 [448002.159364] sas: phy-10:2 added to port-10:2, phy_mask:0x4 (5000000000000a02) [448002.159575] sas: DOING DISCOVERY on port 2, pid:2239187 [448002.159581] hisi_sas_v3_hw 0000:d4:02.0: dev[3:5] found [448002.159602] sas: Enter sas_scsi_recover_host busy: 0 failed: 0 [448002.159623] sas: ata31: end_device-10:0: dev error handler [448002.159633] sas: ata32: end_device-10:1: dev error handler [448002.159636] sas: ata33: end_device-10:2: dev error handler [448002.393349] hisi_sas_v3_hw 0000:d4:02.0: phydown: phy2 phy_state=0x3 [448002.393354] hisi_sas_v3_hw 0000:d4:02.0: ignore flutter phy2 down [448002.684937] hisi_sas_v3_hw 0000:d4:02.0: phyup: phy2 link_rate=10(sata) [448002.851639] ata33.00: ATA-11: WUH721816ALE6L4, PCGAW660, max UDMA/133 [448002.851644] ata33.00: 31251759104 sectors, multi 0: LBA48 NCQ (depth 32) > > But I still don't like the handling in this patch. If we get a phy up, > then the directly-attached disk ideally should be gone already, so > should not have to do this handling. There is no problem when the disk is removed. The current problem is that multiple phy up at the same time. When one of the phys up and enters error handler to execute hardreset, the phy will down and then up. other phy up will probably occupy the hw port id of the previous phy which do hardreset in EH. Thanks, Xingui
Hi, John On 2025/2/24 20:21, John Garry wrote: > On 24/02/2025 09:36, yangxingui wrote: >>> >>> >>> So do you mean that all IO to this disk will error? If yes, then this >>> is good. >> Yes, IO error or IO result does not meet expectations. As shown in the >> log below, due to an abnormal port ID, the SNs of the two disks read >> are the same. > > Do you mean that this is mainline kernel behaviour, below: Yes > >> >> [448000.504979] hisi_sas_v3_hw 0000:d4:02.0: phyup: phy1 >> link_rate=10(sata) >> [448000.505070] sas: phy-10:1 added to port-10:1, phy_mask:0x2 >> (5000000000000a01) >> [448000.505247] sas: DOING DISCOVERY on port 1, pid:2239187 >> [448000.505255] hisi_sas_v3_hw 0000:d4:02.0: dev[2:5] found >> [448000.505274] sas: Enter sas_scsi_recover_host busy: 0 failed: 0 >> [448000.505295] sas: ata31: end_device-10:0: dev error handler >> [448000.505299] sas: ata32: end_device-10:1: dev error handler >> [448001.300517] hisi_sas_v3_hw 0000:d4:02.0: phydown: phy1 >> phy_state=0x1 // phy1's hw port id released >> [448001.300522] hisi_sas_v3_hw 0000:d4:02.0: ignore flutter phy1 down >> [448001.436187] hisi_sas_v3_hw 0000:d4:02.0: phyup: phy2 >> link_rate=10(sata) // phy2 occupies the hardware port ID of phy1 >> [448001.608766] hisi_sas_v3_hw 0000:d4:02.0: phyup: phy1 >> link_rate=10(sata) // phy1 was assigned a new hardware port ID >> [448001.775605] ata32.00: ATA-11: WUH721816ALE6L4, PCGAW660, max UDMA/133 >> [448002.159364] sas: phy-10:2 added to port-10:2, phy_mask:0x4 >> (5000000000000a02) >> [448002.159575] sas: DOING DISCOVERY on port 2, pid:2239187 >> [448002.159581] hisi_sas_v3_hw 0000:d4:02.0: dev[3:5] found >> [448002.159602] sas: Enter sas_scsi_recover_host busy: 0 failed: 0 >> [448002.159623] sas: ata31: end_device-10:0: dev error handler >> [448002.159633] sas: ata32: end_device-10:1: dev error handler >> [448002.159636] sas: ata33: end_device-10:2: dev error handler >> [448002.393349] hisi_sas_v3_hw 0000:d4:02.0: phydown: phy2 phy_state=0x3 >> [448002.393354] hisi_sas_v3_hw 0000:d4:02.0: ignore flutter phy2 down >> [448002.684937] hisi_sas_v3_hw 0000:d4:02.0: phyup: phy2 >> link_rate=10(sata) >> [448002.851639] ata33.00: ATA-11: WUH721816ALE6L4, PCGAW660, max UDMA/133 >> [448002.851644] ata33.00: 31251759104 sectors, multi 0: LBA48 NCQ >> (depth 32) >> >>> >>> But I still don't like the handling in this patch. If we get a phy >>> up, then the directly-attached disk ideally should be gone already, >>> so should not have to do this handling. >> There is no problem when the disk is removed. The current problem is >> that multiple phy up at the same time. When one of the phys up and >> enters error handler to execute hardreset, the phy will down and then >> up. other phy up will probably occupy the hw port id of the previous >> phy which do hardreset in EH. > > Could you do this work (itct update) in lldd_ata_check_ready CB? It's a good idea only for sata disks, but the current problem is not only the scenario of connecting the sata disk. This phenomenon occasionally occurs when the SAS disk is connected after the controller is reset. The following is the log of the stress test recurrence after incorporating the current repair patch. Although we called hisi_sas_refresh_port_id() on controller reset. [ 5387.235015] hisi_sas_v3_hw 0000:74:02.0: I_T nexus reset: internal abort (-5) [ 5387.242126] sas: clear nexus ha [ 5387.245283] hisi_sas_v3_hw 0000:74:02.0: controller resetting... [ 5388.908489] hisi_sas_v3_hw 0000:74:02.0: phyup: phy5 link_rate=10(sata) [ 5388.915090] hisi_sas_v3_hw 0000:74:02.0: phyup: phy6 link_rate=10(sata) [ 5388.934505] hisi_sas_v3_hw 0000:74:02.0: phyup: phy0 link_rate=9(sata) [ 5388.941009] hisi_sas_v3_hw 0000:74:02.0: phyup: phy1 link_rate=9(sata) [ 5388.950976] hisi_sas_v3_hw 0000:74:02.0: phyup: phy4 link_rate=11 [ 5388.957048] hisi_sas_v3_hw 0000:74:02.0: phyup: phy7 link_rate=11 [ 5388.980097] hisi_sas_v3_hw 0000:74:02.0: phyup: phy2 link_rate=11 [ 5388.986169] hisi_sas_v3_hw 0000:74:02.0: phyup: phy3 link_rate=11 // phy3 attached a sas disk. [ 5389.065103] hisi_sas_v3_hw 0000:74:02.0: task prep: SAS port1 not attach device [ 5389.072409] sas: executing TMF task failed 5000c500ae49c8f1 (-70) [ 5389.078492] hisi_sas_v3_hw 0000:74:02.0: task prep: SAS port1 not attach device [ 5389.085780] sas: executing TMF task failed 5000c500ae49c8f1 (-70) [ 5389.091861] hisi_sas_v3_hw 0000:74:02.0: task prep: SAS port1 not attach device [ 5389.099146] sas: executing TMF task failed 5000c500ae49c8f1 (-70) [ 5389.107419] hisi_sas_v3_hw 0000:74:02.0: controller reset complete // controller reset finished [ 5389.113686] hisi_sas_v3_hw 0000:74:02.0: phydown: phy0 phy_state=0xfe [ 5389.120099] hisi_sas_v3_hw 0000:74:02.0: ignore flutter phy0 down [ 5389.136399] hisi_sas_v3_hw 0000:74:02.0: phy3's hw port id changed from 1 to 7 [ 5389.308114] hisi_sas_v3_hw 0000:74:02.0: phyup: phy0 link_rate=9(sata) Thanks, Xingui
On 24/02/2025 13:12, yangxingui wrote: > Hi, John > > On 2025/2/24 20:21, John Garry wrote: >> On 24/02/2025 09:36, yangxingui wrote: >>>> >>>> >>>> So do you mean that all IO to this disk will error? If yes, then >>>> this is good. >>> Yes, IO error or IO result does not meet expectations. As shown in >>> the log below, due to an abnormal port ID, the SNs of the two disks >>> read are the same. >> >> Do you mean that this is mainline kernel behaviour, below: > Yes >> >>> >>> [448000.504979] hisi_sas_v3_hw 0000:d4:02.0: phyup: phy1 >>> link_rate=10(sata) >>> [448000.505070] sas: phy-10:1 added to port-10:1, phy_mask:0x2 >>> (5000000000000a01) >>> [448000.505247] sas: DOING DISCOVERY on port 1, pid:2239187 >>> [448000.505255] hisi_sas_v3_hw 0000:d4:02.0: dev[2:5] found >>> [448000.505274] sas: Enter sas_scsi_recover_host busy: 0 failed: 0 >>> [448000.505295] sas: ata31: end_device-10:0: dev error handler >>> [448000.505299] sas: ata32: end_device-10:1: dev error handler >>> [448001.300517] hisi_sas_v3_hw 0000:d4:02.0: phydown: phy1 >>> phy_state=0x1 // phy1's hw port id released >>> [448001.300522] hisi_sas_v3_hw 0000:d4:02.0: ignore flutter phy1 down >>> [448001.436187] hisi_sas_v3_hw 0000:d4:02.0: phyup: phy2 >>> link_rate=10(sata) // phy2 occupies the hardware port ID of phy1 >>> [448001.608766] hisi_sas_v3_hw 0000:d4:02.0: phyup: phy1 >>> link_rate=10(sata) // phy1 was assigned a new hardware port ID >>> [448001.775605] ata32.00: ATA-11: WUH721816ALE6L4, PCGAW660, max >>> UDMA/133 >>> [448002.159364] sas: phy-10:2 added to port-10:2, phy_mask:0x4 >>> (5000000000000a02) >>> [448002.159575] sas: DOING DISCOVERY on port 2, pid:2239187 >>> [448002.159581] hisi_sas_v3_hw 0000:d4:02.0: dev[3:5] found >>> [448002.159602] sas: Enter sas_scsi_recover_host busy: 0 failed: 0 >>> [448002.159623] sas: ata31: end_device-10:0: dev error handler >>> [448002.159633] sas: ata32: end_device-10:1: dev error handler >>> [448002.159636] sas: ata33: end_device-10:2: dev error handler >>> [448002.393349] hisi_sas_v3_hw 0000:d4:02.0: phydown: phy2 phy_state=0x3 >>> [448002.393354] hisi_sas_v3_hw 0000:d4:02.0: ignore flutter phy2 down >>> [448002.684937] hisi_sas_v3_hw 0000:d4:02.0: phyup: phy2 >>> link_rate=10(sata) >>> [448002.851639] ata33.00: ATA-11: WUH721816ALE6L4, PCGAW660, max >>> UDMA/133 >>> [448002.851644] ata33.00: 31251759104 sectors, multi 0: LBA48 NCQ >>> (depth 32) >>> >>>> >>>> But I still don't like the handling in this patch. If we get a phy >>>> up, then the directly-attached disk ideally should be gone already, >>>> so should not have to do this handling. >>> There is no problem when the disk is removed. The current problem is >>> that multiple phy up at the same time. When one of the phys up and >>> enters error handler to execute hardreset, the phy will down and then >>> up. other phy up will probably occupy the hw port id of the previous >>> phy which do hardreset in EH. >> >> Could you do this work (itct update) in lldd_ata_check_ready CB? > > It's a good idea only for sata disks, but the current problem is not > only the scenario of connecting the sata disk. This phenomenon > occasionally occurs when the SAS disk is connected after the controller > is reset. The following is the log of the stress test recurrence after > incorporating the current repair patch. Although we called > hisi_sas_refresh_port_id() on controller reset. > > [ 5387.235015] hisi_sas_v3_hw 0000:74:02.0: I_T nexus reset: internal > abort (-5) > [ 5387.242126] sas: clear nexus ha > [ 5387.245283] hisi_sas_v3_hw 0000:74:02.0: controller resetting... > [ 5388.908489] hisi_sas_v3_hw 0000:74:02.0: phyup: phy5 link_rate=10(sata) > [ 5388.915090] hisi_sas_v3_hw 0000:74:02.0: phyup: phy6 link_rate=10(sata) > [ 5388.934505] hisi_sas_v3_hw 0000:74:02.0: phyup: phy0 link_rate=9(sata) > [ 5388.941009] hisi_sas_v3_hw 0000:74:02.0: phyup: phy1 link_rate=9(sata) > [ 5388.950976] hisi_sas_v3_hw 0000:74:02.0: phyup: phy4 link_rate=11 > [ 5388.957048] hisi_sas_v3_hw 0000:74:02.0: phyup: phy7 link_rate=11 > [ 5388.980097] hisi_sas_v3_hw 0000:74:02.0: phyup: phy2 link_rate=11 > [ 5388.986169] hisi_sas_v3_hw 0000:74:02.0: phyup: phy3 link_rate=11 // > phy3 attached a sas disk. > [ 5389.065103] hisi_sas_v3_hw 0000:74:02.0: task prep: SAS port1 not > attach device > [ 5389.072409] sas: executing TMF task failed 5000c500ae49c8f1 (-70) > [ 5389.078492] hisi_sas_v3_hw 0000:74:02.0: task prep: SAS port1 not > attach device > [ 5389.085780] sas: executing TMF task failed 5000c500ae49c8f1 (-70) > [ 5389.091861] hisi_sas_v3_hw 0000:74:02.0: task prep: SAS port1 not > attach device > [ 5389.099146] sas: executing TMF task failed 5000c500ae49c8f1 (-70) > [ 5389.107419] hisi_sas_v3_hw 0000:74:02.0: controller reset complete // > controller reset finished > [ 5389.113686] hisi_sas_v3_hw 0000:74:02.0: phydown: phy0 phy_state=0xfe > [ 5389.120099] hisi_sas_v3_hw 0000:74:02.0: ignore flutter phy0 down > [ 5389.136399] hisi_sas_v3_hw 0000:74:02.0: phy3's hw port id changed > from 1 to 7 > [ 5389.308114] hisi_sas_v3_hw 0000:74:02.0: phyup: phy0 link_rate=9(sata) > pm8001 sends sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR,) link reset errors - can you consider doing that in hisi_sas_update_port_id() when you find an inconstant port id?
On 25/02/2025 01:48, yangxingui wrote: >> >> >> pm8001 sends sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR,) >> link reset errors - can you consider doing that in >> hisi_sas_update_port_id() when you find an inconstant port id? > Currently during phyup, the hw port id may change, and the corresponding > hisi_sas_port.id and the port id in itct are not updated synchronously. > The problem caused is not a link error, so we don't need deform port, > just update the port id when phyup. Sure, but I am just trying to keep this simple. If you deform and reform the port - and so lose and find the disk (which does the itct config) - will that solve the problem?
Hi, John On 2025/2/25 16:19, John Garry wrote: > On 25/02/2025 01:48, yangxingui wrote: >>> >>> >>> pm8001 sends sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR,) >>> link reset errors - can you consider doing that in >>> hisi_sas_update_port_id() when you find an inconstant port id? >> Currently during phyup, the hw port id may change, and the >> corresponding hisi_sas_port.id and the port id in itct are not updated >> synchronously. The problem caused is not a link error, so we don't >> need deform port, just update the port id when phyup. > > Sure, but I am just trying to keep this simple. If you deform and reform > the port - and so lose and find the disk (which does the itct config) - > will that solve the problem? 1、phyup ->form port -> eh -> ata reset -> found hw port change -> deform port -> let dev gone -> refound 2、controller reset -> phyup -> finish controller reset -> found hw port change -> deform port -> let dev gone -> refound I also thought about the plan you mentioned in the early days. The above will make the process more complicated and retriggering phyup may result in a new round of port id changes. Lose and find the disk will cause the upper layer IO to report error when controller reset. It seems that it is better to make the upper layer unaware of the hw port id change when phyup in reset, like ata reset or controller reset. ^_^ Thanks, Xingui
On 25/02/2025 09:35, yangxingui wrote: >>>> >>>> pm8001 sends sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR,) >>>> link reset errors - can you consider doing that in >>>> hisi_sas_update_port_id() when you find an inconstant port id? >>> Currently during phyup, the hw port id may change, and the >>> corresponding hisi_sas_port.id and the port id in itct are not >>> updated synchronously. The problem caused is not a link error, so we >>> don't need deform port, just update the port id when phyup. >> >> Sure, but I am just trying to keep this simple. If you deform and >> reform the port - and so lose and find the disk (which does the itct >> config) - will that solve the problem? > > 1、phyup ->form port -> eh -> ata reset -> found hw port change -> > deform port -> let dev gone -> refound > > 2、controller reset -> phyup -> finish controller reset -> found hw port > change -> deform port -> let dev gone -> refound > > I also thought about the plan you mentioned in the early days. The above > will make the process more complicated and retriggering phyup may result > in a new round of port id changes. Lose and find the disk will cause the > upper layer IO to report error when controller reset. It seems that it > is better to make the upper layer unaware of the hw port id change when > phyup in reset, like ata reset or controller reset. ^_^ The lldd_dev_found CB is where you should set the itct, and it is only possible to do that if you report the device gone first. So that seems like a simpler solution.
Hi, John On 2025/2/26 16:57, John Garry wrote: > > The lldd_dev_found CB is where you should set the itct, and it is only > possible to do that if you report the device gone first. So that seems > like a simpler solution. Solution as follow? +static bool hisi_sas_hw_port_id_changed(struct hisi_hba *hisi_hba, int phy_no) +{ + struct hisi_sas_phy *phy = &hisi_hba->phy[phy_no]; + struct asd_sas_phy *sas_phy = &phy->sas_phy; + struct device *dev = hisi_hba->dev; + struct asd_sas_port *sas_port; + struct hisi_sas_port *port; + + if (!sas_phy->port) + return false; + + sas_port = sas_phy->port; + port = to_hisi_sas_port(sas_port); + if (phy->port_id == port->id) + return false; + + dev_info(dev, "phy%d's hw port id changed from %d to %llu\n", + phy_no, port->id, phy->port_id); + + return true; +} + static void hisi_sas_slot_index_clear(struct hisi_hba *hisi_hba, int slot_idx) { void *bitmap = hisi_hba->slot_index_tags; @@ -856,6 +878,14 @@ static void hisi_sas_phyup_work_common(struct work_struct *work, struct asd_sas_phy *sas_phy = &phy->sas_phy; int phy_no = sas_phy->id; + if (hisi_sas_hw_port_id_changed(hisi_hba, phy_no)) { + sas_phy_disconnected(sas_phy); + phy->phy_attached = 0; + sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR, GFP_ATOMIC); + hisi_sas_notify_phy_event(phy, HISI_PHYE_LINK_RESET); + return; + } + Thanks, Xingui
On 27/02/2025 08:33, yangxingui wrote: > Hi, John > > On 2025/2/26 16:57, John Garry wrote: > >> >> The lldd_dev_found CB is where you should set the itct, and it is only >> possible to do that if you report the device gone first. So that seems >> like a simpler solution. Sure, something like that - you just need to get libsas to trigger the proper hw port id assignment for the device. As for specific implementation in the LLDD, that up to you guys. Thanks, John > Solution as follow? > +static bool hisi_sas_hw_port_id_changed(struct hisi_hba *hisi_hba, int > phy_no) > +{ > + struct hisi_sas_phy *phy = &hisi_hba->phy[phy_no]; > + struct asd_sas_phy *sas_phy = &phy->sas_phy; > + struct device *dev = hisi_hba->dev; > + struct asd_sas_port *sas_port; > + struct hisi_sas_port *port; > + > + if (!sas_phy->port) > + return false; > + > + sas_port = sas_phy->port; > + port = to_hisi_sas_port(sas_port); > + if (phy->port_id == port->id) > + return false; > + > + dev_info(dev, "phy%d's hw port id changed from %d to %llu\n", > + phy_no, port->id, phy->port_id); > + > + return true; > +} > + > static void hisi_sas_slot_index_clear(struct hisi_hba *hisi_hba, int > slot_idx) > { > void *bitmap = hisi_hba->slot_index_tags; > @@ -856,6 +878,14 @@ static void hisi_sas_phyup_work_common(struct > work_struct *work, > struct asd_sas_phy *sas_phy = &phy->sas_phy; > int phy_no = sas_phy->id; > > + if (hisi_sas_hw_port_id_changed(hisi_hba, phy_no)) { > + sas_phy_disconnected(sas_phy); > + phy->phy_attached = 0; > + sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR, > GFP_ATOMIC); > + hisi_sas_notify_phy_event(phy, HISI_PHYE_LINK_RESET); > + return; > + } > + > > Thanks, > Xingui
Hi, John On 2025/3/4 17:48, John Garry wrote: > On 27/02/2025 08:33, yangxingui wrote: >> Hi, John >> >> On 2025/2/26 16:57, John Garry wrote: >> >>> >>> The lldd_dev_found CB is where you should set the itct, and it is >>> only possible to do that if you report the device gone first. So that >>> seems like a simpler solution. > > Sure, something like that - you just need to get libsas to trigger the > proper hw port id assignment for the device. As for specific > implementation in the LLDD, that up to you guys. Thank you for your suggestions. The disk was finally restored to normal, but the error handling took a long time. Since the error handling would set the host to the recovery state, other disks on the same host would be blocked for a long time. Log as follow: [ 1351.602899] hisi_sas_v3_hw 0000:74:02.0: phyup: phy3 link_rate=10(sata) [ 1351.610892] sas: phy-1:3 added to port-1:0, phy_mask:0x8 (5000000000000103) [ 1351.611057] sas: DOING DISCOVERY on port 0, pid:7 [ 1351.611068] hisi_sas_v3_hw 0000:74:02.0: dev[1:5] found [ 1351.617396] sas: Enter sas_scsi_recover_host busy: 0 failed: 0 [ 1351.618272] hisi_sas_v3_hw 0000:74:02.0: phyup: phy2 link_rate=10(sata) [ 1351.624300] sas: ata10: end_device-1:0: dev error handler [ 1351.670954] hisi_sas_v3_hw 0000:74:02.0: phydown: phy3 phy_state=0xf7 [ 1351.678388] hisi_sas_v3_hw 0000:74:02.0: ignore flutter phy3 down [ 1351.685465] hisi_sas_v3_hw 0000:74:02.0: phyup: phy4 link_rate=11 [ 1351.841167] hisi_sas_v3_hw 0000:74:02.0: phyup: phy3 link_rate=10(sata) [ 1351.849054] hisi_sas_v3_hw 0000:74:02.0: phy3's hw port id changed from 0 to 1 [ 1351.857967] hisi_sas_v3_hw 0000:74:02.0: phydown: phy3 phy_state=0xf7 [ 1352.006552] hisi_sas_v3_hw 0000:74:02.0: task prep: SATA/STP port0 not attach device [ 1352.015679] hisi_sas_v3_hw 0000:74:02.0: task exec: failed[-70]! [ 1352.022734] sas: lldd_execute_task returned: -70 [ 1352.022754] ata10.00: failed to IDENTIFY (I/O error, err_mask=0x40) [ 1352.041168] hisi_sas_v3_hw 0000:74:02.0: phyup: phy3 link_rate=10(sata) [ 1357.218587] hisi_sas_v3_hw 0000:74:02.0: erroneous completion iptt=4019 task=0000000055585b33 dev id=1 addr=5000000000000103 CQ hdr: 0x8000007 0x10fb3 0x0 0x0 Error info: 0x0 0x0 0x0 0x0 [ 1357.237298] hisi_sas_v3_hw 0000:74:02.0: task prep: SATA/STP port0 not attach device [ 1357.246666] hisi_sas_v3_hw 0000:74:02.0: task exec: failed[-70]! [ 1357.253850] hisi_sas_v3_hw 0000:74:02.0: abort tmf: executing internal task failed: -70 [ 1357.263487] hisi_sas_v3_hw 0000:74:02.0: ata disk 5000000000000103 reset failed [ 1357.272487] hisi_sas_v3_hw 0000:74:02.0: phydown: phy3 phy_state=0xf7 [ 1357.280131] hisi_sas_v3_hw 0000:74:02.0: ignore flutter phy3 down [ 1357.453175] hisi_sas_v3_hw 0000:74:02.0: phyup: phy3 link_rate=10(sata) [ 1357.618532] hisi_sas_v3_hw 0000:74:02.0: task prep: SATA/STP port0 not attach device [ 1357.627915] hisi_sas_v3_hw 0000:74:02.0: task exec: failed[-70]! [ 1357.635086] sas: lldd_execute_task returned: -70 [ 1357.635108] ata10.00: failed to IDENTIFY (I/O error, err_mask=0x40) [ 1362.850591] hisi_sas_v3_hw 0000:74:02.0: erroneous completion iptt=4020 task=0000000055585b33 dev id=1 addr=5000000000000103 CQ hdr: 0x8000007 0x10fb4 0x0 0x0 Error info: 0x0 0x0 0x0 0x0 [ 1362.869450] hisi_sas_v3_hw 0000:74:02.0: task prep: SATA/STP port0 not attach device [ 1362.878890] hisi_sas_v3_hw 0000:74:02.0: task exec: failed[-70]! [ 1362.886100] hisi_sas_v3_hw 0000:74:02.0: abort tmf: executing internal task failed: -70 [ 1362.895779] hisi_sas_v3_hw 0000:74:02.0: ata disk 5000000000000103 reset failed [ 1362.904789] hisi_sas_v3_hw 0000:74:02.0: phydown: phy3 phy_state=0xf7 [ 1362.912447] hisi_sas_v3_hw 0000:74:02.0: ignore flutter phy3 down [ 1363.089166] hisi_sas_v3_hw 0000:74:02.0: phyup: phy3 link_rate=10(sata) [ 1363.254527] hisi_sas_v3_hw 0000:74:02.0: task prep: SATA/STP port0 not attach device [ 1363.263891] hisi_sas_v3_hw 0000:74:02.0: task exec: failed[-70]! [ 1363.271052] sas: lldd_execute_task returned: -70 [ 1363.271077] ata10.00: failed to IDENTIFY (I/O error, err_mask=0x40) [ 1368.482569] hisi_sas_v3_hw 0000:74:02.0: erroneous completion iptt=4021 task=0000000055585b33 dev id=1 addr=5000000000000103 CQ hdr: 0x8000007 0x10fb5 0x0 0x0 Error info: 0x0 0x0 0x0 0x0 [ 1368.501395] hisi_sas_v3_hw 0000:74:02.0: task prep: SATA/STP port0 not attach device [ 1368.510834] hisi_sas_v3_hw 0000:74:02.0: task exec: failed[-70]! [ 1368.518058] hisi_sas_v3_hw 0000:74:02.0: abort tmf: executing internal task failed: -70 [ 1368.527745] hisi_sas_v3_hw 0000:74:02.0: ata disk 5000000000000103 reset failed [ 1368.536750] hisi_sas_v3_hw 0000:74:02.0: phydown: phy3 phy_state=0xf7 [ 1368.544401] hisi_sas_v3_hw 0000:74:02.0: ignore flutter phy3 down [ 1368.721173] hisi_sas_v3_hw 0000:74:02.0: phyup: phy3 link_rate=10(sata) [ 1368.886544] sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 0 tries: 1 [ 1368.896602] sas: sas_probe_sata: for direct-attached device 5000000000000103 returned -19 [ 1368.906510] hisi_sas_v3_hw 0000:74:02.0: dev[1:5] is gone [ 1368.913148] hisi_sas_v3_hw 0000:74:02.0: erroneous completion iptt=4022 task=00000000ee9f5e36 dev id=1 addr=5000000000000103 CQ hdr: 0x8000007 0x10fb6 0x0 0x0 Error info: 0x0 0x0 0x0 0x0 [ 1368.932057] sas: DONE DISCOVERY on port 0, pid:7, result:0 [ 1369.571030] sas: phy-1:3 added to port-1:0, phy_mask:0x8 (5000000000000103) [ 1369.571201] sas: DOING DISCOVERY on port 0, pid:7 [ 1369.571205] hisi_sas_v3_hw 0000:74:02.0: dev[9:5] found [ 1369.577546] sas: Enter sas_scsi_recover_host busy: 0 failed: 0 [ 1369.584543] sas: ata12: end_device-1:0: dev error handler [ 1369.584653] sas: ata11: end_device-1:1: dev error handler [ 1369.611048] hisi_sas_v3_hw 0000:74:02.0: phydown: phy3 phy_state=0xf7 [ 1369.618639] hisi_sas_v3_hw 0000:74:02.0: ignore flutter phy3 down [ 1369.793175] hisi_sas_v3_hw 0000:74:02.0: phyup: phy3 link_rate=10(sata) [ 1374.447031] ata12.00: ATA-10: ST2000NM0055-1V4104, TN05, max UDMA/133 [ 1374.454648] ata12.00: 3907029168 sectors, multi 0: LBA48 NCQ (depth 32) [ 1374.463850] ata12.00: configured for UDMA/133 [ 1374.469363] sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 0 tries: 1 [ 1374.506234] scsi 1:0:6:0: Direct-Access ATA ST2000NM0055-1V4 TN05 PQ: 0 ANSI: 5 [ 1374.515932] sas: DONE DISCOVERY on port 0, pid:7, result:0 [ 1374.515986] sas: sas_form_port: phy3 belongs to port0 already(1)! [ 1374.523240] sas: sas_form_port: phy3 belongs to port0 already(1)! [ 1374.538915] sas: sas_form_port: phy3 belongs to port0 already(1)! Thanks, Xingui
On 05/03/2025 08:16, yangxingui wrote: >> >> Sure, something like that - you just need to get libsas to trigger the >> proper hw port id assignment for the device. As for specific >> implementation in the LLDD, that up to you guys. > > Thank you for your suggestions. The disk was finally restored to normal, > but the error handling took a long time. Since the error handling would > set the host to the recovery state, other disks on the same host would > be blocked for a long time. But that because you have IO inflight for the disk which was unplugged?
Hi, John On 2025/3/6 0:15, John Garry wrote: > On 05/03/2025 08:16, yangxingui wrote: >>> >>> Sure, something like that - you just need to get libsas to trigger >>> the proper hw port id assignment for the device. As for specific >>> implementation in the LLDD, that up to you guys. >> >> Thank you for your suggestions. The disk was finally restored to >> normal, but the error handling took a long time. Since the error >> handling would set the host to the recovery state, other disks on the >> same host would be blocked for a long time. > > But that because you have IO inflight for the disk which was unplugged? No, these are all ATA_CMD_ID_ATA commands newly issued by ata_eh_revalidate_and_attach() during error handling, and will be retried three times. The reason is that after the hw port id is changed, we set it to invalid, and these ATA_CMD_ID_ATA cmds will fail to execute. Thanks, Xingui
Hi, John On 2025/2/25 16:19, John Garry wrote: > On 25/02/2025 01:48, yangxingui wrote: >>> >>> >>> pm8001 sends sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR,) >>> link reset errors - can you consider doing that in >>> hisi_sas_update_port_id() when you find an inconstant port id? >> Currently during phyup, the hw port id may change, and the >> corresponding hisi_sas_port.id and the port id in itct are not updated >> synchronously. The problem caused is not a link error, so we don't >> need deform port, just update the port id when phyup. > > Sure, but I am just trying to keep this simple. If you deform and reform > the port - and so lose and find the disk (which does the itct config) - > will that solve the problem? > We found that we need to perform lose and find for all devices on the port including the local phy and the remote phy. This process still requires traversing the phy information corresponding to all devices to reset and it is also necessary to consider that there is a race between device removal and the current process. it looks similar to solution of update port id directly. And there will be the problem mentioned above. e.g, during error handling, the recovery state will last for more than 15 seconds, affecting the performance of other disks on the same host. Thanks, Xingui
On 10/03/2025 13:09, yangxingui wrote: > On 2025/2/25 16:19, John Garry wrote: >> On 25/02/2025 01:48, yangxingui wrote: >>>> >>>> >>>> pm8001 sends sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR,) >>>> link reset errors - can you consider doing that in >>>> hisi_sas_update_port_id() when you find an inconstant port id? >>> Currently during phyup, the hw port id may change, and the >>> corresponding hisi_sas_port.id and the port id in itct are not >>> updated synchronously. The problem caused is not a link error, so we >>> don't need deform port, just update the port id when phyup. >> >> Sure, but I am just trying to keep this simple. If you deform and >> reform the port - and so lose and find the disk (which does the itct >> config) - will that solve the problem? >> > We found that we need to perform lose and find for all devices on the > port including the local phy and the remote phy. This process still > requires traversing the phy information corresponding to all devices to > reset and it is also necessary to consider that there is a race between > device removal and the current process. it looks similar to solution of > update port id directly. And there will be the problem mentioned above. > e.g, during error handling, the recovery state will last for more than > 15 seconds, affecting the performance of other disks on the same host. How do you even detect the port id inconsistency for the device attached at the remote phy? For this series, you could detect it at the phy up/down handler for the directly attached device - how would it be triggered for the remote phy? John
On 2025/3/11 1:45, John Garry wrote: >>> Sure, but I am just trying to keep this simple. If you deform and >>> reform the port - and so lose and find the disk (which does the itct >>> config) - will that solve the problem? >>> >> We found that we need to perform lose and find for all devices on the >> port including the local phy and the remote phy. This process still >> requires traversing the phy information corresponding to all devices >> to reset and it is also necessary to consider that there is a race >> between device removal and the current process. it looks similar to >> solution of update port id directly. And there will be the problem >> mentioned above. e.g, during error handling, the recovery state will >> last for more than 15 seconds, affecting the performance of other >> disks on the same host. > > How do you even detect the port id inconsistency for the device attached > at the remote phy? For this series, you could detect it at the phy > up/down handler for the directly attached device - how would it be > triggered for the remote phy? When the hardware port ID of the EXP is detected to have changed, the link reset is performed on the local phy of EXP in sequence, which will not trigger the lose and find process of the EXP device, and it will not trigger the lose and find process of the disks connected to the remote phy. Therefore, it is necessary to lose and found all devices separately to update the device's port id in itct. local phy0 --------------------------- disk0 local phy1 --------------------------- disk1 local phy2 --------------------------- disk2 local phy3 --------------------------- disk3 _________ local phy4 --------| |-------- disk4 | | local phy5 --------| |-------- disk5 | | local phy6 --------| EXP |-------- disk6 | | local phy7 --------| |-------- disk7 |_________| Thanks, Xingui .
Hi, John On 2025/3/11 1:45, John Garry wrote: > On 10/03/2025 13:09, yangxingui wrote: >> On 2025/2/25 16:19, John Garry wrote: >>> On 25/02/2025 01:48, yangxingui wrote: >>>>> >>>>> >>>>> pm8001 sends sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR,) >>>>> link reset errors - can you consider doing that in >>>>> hisi_sas_update_port_id() when you find an inconstant port id? >>>> Currently during phyup, the hw port id may change, and the >>>> corresponding hisi_sas_port.id and the port id in itct are not >>>> updated synchronously. The problem caused is not a link error, so we >>>> don't need deform port, just update the port id when phyup. >>> >>> Sure, but I am just trying to keep this simple. If you deform and >>> reform the port - and so lose and find the disk (which does the itct >>> config) - will that solve the problem? >>> >> We found that we need to perform lose and find for all devices on the >> port including the local phy and the remote phy. This process still >> requires traversing the phy information corresponding to all devices >> to reset and it is also necessary to consider that there is a race >> between device removal and the current process. it looks similar to >> solution of update port id directly. And there will be the problem >> mentioned above. e.g, during error handling, the recovery state will >> last for more than 15 seconds, affecting the performance of other >> disks on the same host. > > How do you even detect the port id inconsistency for the device attached > at the remote phy? For this series, you could detect it at the phy > up/down handler for the directly attached device - how would it be > triggered for the remote phy? The current problem we are facing only involves directly attached devices. a new version based on your suggestion. Thanks, Xingui
I'll have a look when I get a chance -----Original Message----- From: yangxingui <yangxingui@huawei.com> Sent: Wednesday, March 12, 2025 9:45 AM To: John Garry <john.g.garry@oracle.com>; liyihang9@huawei.com; yanaijie@huawei.com Cc: jejb@linux.ibm.com; Martin Petersen <martin.petersen@oracle.com>; linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; linuxarm@huawei.com; prime.zeng@huawei.com; liuyonglong@huawei.com; kangfenglong@huawei.com; liyangyang20@huawei.com; f.fangjian@huawei.com; xiabing14@h-partners.com Subject: Re: [PATCH v3 1/3] scsi: hisi_sas: Enable force phy when SATA disk directly connected Hi, John On 2025/3/11 1:45, John Garry wrote: > On 10/03/2025 13:09, yangxingui wrote: >> On 2025/2/25 16:19, John Garry wrote: >>> On 25/02/2025 01:48, yangxingui wrote: >>>>> >>>>> >>>>> pm8001 sends sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR,) >>>>> link reset errors - can you consider doing that in >>>>> hisi_sas_update_port_id() when you find an inconstant port id? >>>> Currently during phyup, the hw port id may change, and the >>>> corresponding hisi_sas_port.id and the port id in itct are not >>>> updated synchronously. The problem caused is not a link error, so we >>>> don't need deform port, just update the port id when phyup. >>> >>> Sure, but I am just trying to keep this simple. If you deform and >>> reform the port - and so lose and find the disk (which does the itct >>> config) - will that solve the problem? >>> >> We found that we need to perform lose and find for all devices on the >> port including the local phy and the remote phy. This process still >> requires traversing the phy information corresponding to all devices >> to reset and it is also necessary to consider that there is a race >> between device removal and the current process. it looks similar to >> solution of update port id directly. And there will be the problem >> mentioned above. e.g, during error handling, the recovery state will >> last for more than 15 seconds, affecting the performance of other >> disks on the same host. > > How do you even detect the port id inconsistency for the device attached > at the remote phy? For this series, you could detect it at the phy > up/down handler for the directly attached device - how would it be > triggered for the remote phy? The current problem we are facing only involves directly attached devices. a new version based on your suggestion. Thanks, Xingui
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c index 6e7f99fcc824..3af991cad07e 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c @@ -2501,6 +2501,7 @@ static void prep_ata_v2_hw(struct hisi_hba *hisi_hba, struct hisi_sas_port *port = to_hisi_sas_port(sas_port); struct sas_ata_task *ata_task = &task->ata_task; struct sas_tmf_task *tmf = slot->tmf; + int phy_id; u8 *buf_cmd; int has_data = 0, hdr_tag = 0; u32 dw0, dw1 = 0, dw2 = 0; @@ -2508,10 +2509,14 @@ 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 (parent_dev && dev_is_expander(parent_dev->dev_type)) { dw0 |= 3 << CMD_HDR_CMD_OFF; - else + } else { + phy_id = device->phy->identify.phy_identifier; + dw0 |= (1U << phy_id) << CMD_HDR_PHY_ID_OFF; + dw0 |= CMD_HDR_FORCE_PHY_MSK; dw0 |= 4 << CMD_HDR_CMD_OFF; + } if (tmf && ata_task->force_phy) { dw0 |= CMD_HDR_FORCE_PHY_MSK; diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c index 095bbf80c34e..6a0656f3b596 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c @@ -359,6 +359,10 @@ #define CMD_HDR_RESP_REPORT_MSK (0x1 << CMD_HDR_RESP_REPORT_OFF) #define CMD_HDR_TLR_CTRL_OFF 6 #define CMD_HDR_TLR_CTRL_MSK (0x3 << CMD_HDR_TLR_CTRL_OFF) +#define CMD_HDR_PHY_ID_OFF 8 +#define CMD_HDR_PHY_ID_MSK (0x1ff << CMD_HDR_PHY_ID_OFF) +#define CMD_HDR_FORCE_PHY_OFF 17 +#define CMD_HDR_FORCE_PHY_MSK (0x1U << CMD_HDR_FORCE_PHY_OFF) #define CMD_HDR_PORT_OFF 18 #define CMD_HDR_PORT_MSK (0xf << CMD_HDR_PORT_OFF) #define CMD_HDR_PRIORITY_OFF 27 @@ -1429,15 +1433,21 @@ static void prep_ata_v3_hw(struct hisi_hba *hisi_hba, struct hisi_sas_cmd_hdr *hdr = slot->cmd_hdr; struct asd_sas_port *sas_port = device->port; struct hisi_sas_port *port = to_hisi_sas_port(sas_port); + int phy_id; u8 *buf_cmd; int has_data = 0, hdr_tag = 0; 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 (parent_dev && dev_is_expander(parent_dev->dev_type)) { hdr->dw0 |= cpu_to_le32(3 << CMD_HDR_CMD_OFF); - else + } else { + phy_id = device->phy->identify.phy_identifier; + hdr->dw0 |= cpu_to_le32((1U << phy_id) + << CMD_HDR_PHY_ID_OFF); + hdr->dw0 |= CMD_HDR_FORCE_PHY_MSK; hdr->dw0 |= cpu_to_le32(4U << CMD_HDR_CMD_OFF); + } switch (task->data_dir) { case DMA_TO_DEVICE:
the SAS controller determines the disk to which I/Os are delivered based on the port id in the DQ entry when SATA disk directly connected. When many phys were disconnected immediately and connected again during I/O sending and port id of phys were changed and used by other link, I/O may be sent to incorrect disk and data inconsistency on the SATA disk may occur during I/O retry with the old port id. So enable force phy, then force the command to be executed in a certain phy, and if the actual phy id of the port does not match the phy configured in the command, the chip will stop delivering the I/O to disk. Fixes: ce60689e12dd ("scsi: hisi_sas: add v3 code to send ATA frame") Signed-off-by: Xingui Yang <yangxingui@huawei.com> Reviewed-by: Yihang Li <liyihang9@huawei.com> --- drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 9 +++++++-- drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 14 ++++++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-)