From cdc97921de02a0c1a8324afa03e8b3e30a72bbe6 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 31 Aug 2026 10:48:21 -0700 Subject: [PATCH] scsi: pm8001: Fix SATA FIS offset in struct hw_event_resp In struct hw_event_resp, sas_identify was defined using struct sas_identify_frame, which is 32 bytes long because it includes a 4-byte CRC field. However, PM8001 and PM80xx hardware does not send the CRC in the event response frame, providing only the 28-byte identify payload. Because sas_identify had a size of 32 bytes, the following sata_fis member was located at byte offset 44 instead of the actual hardware offset 40. In hw_event_sata_phy_up(), this discrepancy was worked around with negative pointer arithmetic: ((u8 *)&pPayload->sata_fis - 4). This triggers a compiler error under Clang with -Wstringop-overread: error: 'memcpy' reading 20 bytes from a region of size 0 [-Werror,-Wstringop-overread] memcpy(phy->frame_rcvd, ((u8 *)&pPayload->sata_fis - 4), Move struct sas_identify_frame_local (28 bytes) into pm8001_defs.h, and update struct hw_event_resp in both pm8001_hwi.h and pm80xx_hwi.h to use struct sas_identify_frame_local followed by sata_fis and a 4-byte padding word. This correctly aligns sata_fis at byte offset 40 while maintaining the 64-byte total payload size. Update hw_event_sata_phy_up() in pm8001_hwi.c and pm80xx_hwi.c to copy directly from &pPayload->sata_fis. Compile-tested only. Both the patch and its description have been generated by Gemini. Fixes: dbf9bfe61571 ("[SCSI] pm8001: add SAS/SATA HBA driver") Fixes: f5860992db55 ("[SCSI] pm80xx: Added SPCv/ve specific hardware functionalities and relevant changes in common files") Signed-off-by: Bart Van Assche Link: https://patch.msgid.link/31963d53810f42970447e78ef46030ab13d2e91f.1788198434.git.bvanassche@acm.org Signed-off-by: Martin K. Petersen (Oracle) --- drivers/scsi/pm8001/pm8001_defs.h | 98 +++++++++++++++++++++++++++++ drivers/scsi/pm8001/pm8001_hwi.c | 2 +- drivers/scsi/pm8001/pm8001_hwi.h | 4 +- drivers/scsi/pm8001/pm80xx_hwi.c | 2 +- drivers/scsi/pm8001/pm80xx_hwi.h | 100 +----------------------------- 5 files changed, 106 insertions(+), 100 deletions(-) diff --git a/drivers/scsi/pm8001/pm8001_defs.h b/drivers/scsi/pm8001/pm8001_defs.h index 4e19d61dffbb..a9605ca317f3 100644 --- a/drivers/scsi/pm8001/pm8001_defs.h +++ b/drivers/scsi/pm8001/pm8001_defs.h @@ -41,6 +41,104 @@ #ifndef _PM8001_DEFS_H_ #define _PM8001_DEFS_H_ +#include + +#ifdef __LITTLE_ENDIAN_BITFIELD +struct sas_identify_frame_local { + /* Byte 0 */ + u8 frame_type:4; + u8 dev_type:3; + u8 _un0:1; + + /* Byte 1 */ + u8 _un1; + + /* Byte 2 */ + union { + struct { + u8 _un20:1; + u8 smp_iport:1; + u8 stp_iport:1; + u8 ssp_iport:1; + u8 _un247:4; + }; + u8 initiator_bits; + }; + + /* Byte 3 */ + union { + struct { + u8 _un30:1; + u8 smp_tport:1; + u8 stp_tport:1; + u8 ssp_tport:1; + u8 _un347:4; + }; + u8 target_bits; + }; + + /* Byte 4 - 11 */ + u8 _un4_11[8]; + + /* Byte 12 - 19 */ + u8 sas_addr[SAS_ADDR_SIZE]; + + /* Byte 20 */ + u8 phy_id; + + u8 _un21_27[7]; + +} __packed; + +#elif defined(__BIG_ENDIAN_BITFIELD) +struct sas_identify_frame_local { + /* Byte 0 */ + u8 _un0:1; + u8 dev_type:3; + u8 frame_type:4; + + /* Byte 1 */ + u8 _un1; + + /* Byte 2 */ + union { + struct { + u8 _un247:4; + u8 ssp_iport:1; + u8 stp_iport:1; + u8 smp_iport:1; + u8 _un20:1; + }; + u8 initiator_bits; + }; + + /* Byte 3 */ + union { + struct { + u8 _un347:4; + u8 ssp_tport:1; + u8 stp_tport:1; + u8 smp_tport:1; + u8 _un30:1; + }; + u8 target_bits; + }; + + /* Byte 4 - 11 */ + u8 _un4_11[8]; + + /* Byte 12 - 19 */ + u8 sas_addr[SAS_ADDR_SIZE]; + + /* Byte 20 */ + u8 phy_id; + + u8 _un21_27[7]; +} __packed; +#else +#error "Bitfield order not defined!" +#endif + enum chip_flavors { chip_8001, chip_8008, diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c index fff8d877abb9..55a01e533fd1 100644 --- a/drivers/scsi/pm8001/pm8001_hwi.c +++ b/drivers/scsi/pm8001/pm8001_hwi.c @@ -3208,7 +3208,7 @@ hw_event_sata_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb) phy->sas_phy.oob_mode = SATA_OOB_MODE; sas_notify_phy_event(&phy->sas_phy, PHYE_OOB_DONE, GFP_ATOMIC); spin_lock_irqsave(&phy->sas_phy.frame_rcvd_lock, flags); - memcpy(phy->frame_rcvd, ((u8 *)&pPayload->sata_fis - 4), + memcpy(phy->frame_rcvd, &pPayload->sata_fis, sizeof(struct dev_to_host_fis)); phy->frame_rcvd_size = sizeof(struct dev_to_host_fis); phy->identify.target_port_protocols = SAS_PROTOCOL_SATA; diff --git a/drivers/scsi/pm8001/pm8001_hwi.h b/drivers/scsi/pm8001/pm8001_hwi.h index f1ce8df082b0..a21b1b37a8ea 100644 --- a/drivers/scsi/pm8001/pm8001_hwi.h +++ b/drivers/scsi/pm8001/pm8001_hwi.h @@ -42,6 +42,7 @@ #include #include +#include "pm8001_defs.h" /* for Request Opcode of IOMB */ @@ -229,8 +230,9 @@ struct hw_event_resp { __le32 lr_evt_status_phyid_portid; __le32 evt_param; __le32 npip_portstate; - struct sas_identify_frame sas_identify; + struct sas_identify_frame_local sas_identify; struct dev_to_host_fis sata_fis; + u32 _r_a; } __attribute__((packed, aligned(4))); diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c index 2c0fa7ab33d2..659dbd603a03 100644 --- a/drivers/scsi/pm8001/pm80xx_hwi.c +++ b/drivers/scsi/pm8001/pm80xx_hwi.c @@ -3296,7 +3296,7 @@ hw_event_sata_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb) phy->sas_phy.oob_mode = SATA_OOB_MODE; sas_notify_phy_event(&phy->sas_phy, PHYE_OOB_DONE, GFP_ATOMIC); spin_lock_irqsave(&phy->sas_phy.frame_rcvd_lock, flags); - memcpy(phy->frame_rcvd, ((u8 *)&pPayload->sata_fis - 4), + memcpy(phy->frame_rcvd, &pPayload->sata_fis, sizeof(struct dev_to_host_fis)); phy->frame_rcvd_size = sizeof(struct dev_to_host_fis); phy->identify.target_port_protocols = SAS_PROTOCOL_SATA; diff --git a/drivers/scsi/pm8001/pm80xx_hwi.h b/drivers/scsi/pm8001/pm80xx_hwi.h index d8a63b7fed6a..d6796ba40df9 100644 --- a/drivers/scsi/pm8001/pm80xx_hwi.h +++ b/drivers/scsi/pm8001/pm80xx_hwi.h @@ -43,6 +43,7 @@ #include #include +#include "pm8001_defs.h" /* for Request Opcode of IOMB */ #define OPC_INB_ECHO 1 /* 0x000 */ @@ -236,102 +237,6 @@ /* Port recovery timeout, 10000 ms for PM8006 controller */ #define CHIP_8006_PORT_RECOVERY_TIMEOUT 0x640000 -#ifdef __LITTLE_ENDIAN_BITFIELD -struct sas_identify_frame_local { - /* Byte 0 */ - u8 frame_type:4; - u8 dev_type:3; - u8 _un0:1; - - /* Byte 1 */ - u8 _un1; - - /* Byte 2 */ - union { - struct { - u8 _un20:1; - u8 smp_iport:1; - u8 stp_iport:1; - u8 ssp_iport:1; - u8 _un247:4; - }; - u8 initiator_bits; - }; - - /* Byte 3 */ - union { - struct { - u8 _un30:1; - u8 smp_tport:1; - u8 stp_tport:1; - u8 ssp_tport:1; - u8 _un347:4; - }; - u8 target_bits; - }; - - /* Byte 4 - 11 */ - u8 _un4_11[8]; - - /* Byte 12 - 19 */ - u8 sas_addr[SAS_ADDR_SIZE]; - - /* Byte 20 */ - u8 phy_id; - - u8 _un21_27[7]; - -} __packed; - -#elif defined(__BIG_ENDIAN_BITFIELD) -struct sas_identify_frame_local { - /* Byte 0 */ - u8 _un0:1; - u8 dev_type:3; - u8 frame_type:4; - - /* Byte 1 */ - u8 _un1; - - /* Byte 2 */ - union { - struct { - u8 _un247:4; - u8 ssp_iport:1; - u8 stp_iport:1; - u8 smp_iport:1; - u8 _un20:1; - }; - u8 initiator_bits; - }; - - /* Byte 3 */ - union { - struct { - u8 _un347:4; - u8 ssp_tport:1; - u8 stp_tport:1; - u8 smp_tport:1; - u8 _un30:1; - }; - u8 target_bits; - }; - - /* Byte 4 - 11 */ - u8 _un4_11[8]; - - /* Byte 12 - 19 */ - u8 sas_addr[SAS_ADDR_SIZE]; - - /* Byte 20 */ - u8 phy_id; - - u8 _un21_27[7]; -} __packed; -#else -#error "Bitfield order not defined!" -#endif - struct mpi_msg_hdr { __le32 header; /* Bits [11:0] - Message operation code */ /* Bits [15:12] - Message Category */ @@ -427,8 +332,9 @@ struct hw_event_resp { __le32 lr_status_evt_portid; __le32 evt_param; __le32 phyid_npip_portstate; - struct sas_identify_frame sas_identify; + struct sas_identify_frame_local sas_identify; struct dev_to_host_fis sata_fis; + u32 _r_a; } __attribute__((packed, aligned(4))); /*