Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: LEuxF0n67h5ZIIffNwoN62Qh4lH7RVg22EV7oRCY2S+MixvbyzNGz2+gIBOt/bFAFwu4ArMvVi
 auketdcUXObBM58MSdWQs/RhqcbK/QrApK1DWl/ljuImaMFw6/z8qkI7COEwy36xF16C47DLAQ
 +LVhCtlmufn9GhTLetpZpYMbZMBaN2TruX36HlDLW5PRpxKtQ3ZWnGQPl+6gwnR11JEXzOqr24
 DG6VnRPZMTx+e+Y72yRu+TD0suCHzJMcHTcOKdLc2td4L5MLcNOdDfSVPsyGp6fbFmNqFXW1Re
 RqI=
From:   <Don.Brace@microchip.com>
To:     <slyfox@gentoo.org>, <linux-kernel@vger.kernel.org>
CC:     <linux-ia64@vger.kernel.org>, <storagedev@microchip.com>,
        <linux-scsi@vger.kernel.org>, <jszczype@redhat.com>,
        <Scott.Benesh@microchip.com>, <Scott.Teel@microchip.com>,
        <thenzl@redhat.com>, <martin.petersen@oracle.com>,
        <glaubitz@physik.fu-berlin.de>
Subject: RE: [PATCH] hpsa: fix boot on ia64 (atomic_t alignment)
Thread-Topic: [PATCH] hpsa: fix boot on ia64 (atomic_t alignment)
Thread-Index: AQHXF4722AqRbCqQukSHQNjL/VV846qGzRcg
Date:   Tue, 16 Mar 2021 16:30:00 +0000
Message-ID: <SN6PR11MB2848561E3D85A8F55EB86977E16B9@SN6PR11MB2848.namprd11.prod.outlook.com>
References: <5532f9ab-7555-d51b-f4d5-f9b72a61f248@redhat.com>
 <20210312222718.4117508-1-slyfox@gentoo.org>
In-Reply-To: <20210312222718.4117508-1-slyfox@gentoo.org>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 58f613ef-0047-424b-d963-08d8e898becb
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Mar 2021 16:30:00.5789
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: rACOBv81SIDME/i//ZIPrCk2ohSbeQocXoz8Qcx7d0bgBklEw7ybb1n9FU3FQ/39Wj3bbCh7mqWIlnykGnoaQQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR11MB2703
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

-----Original Message-----
From: Sergei Trofimovich [mailto:slyfox@gentoo.org]=20
Subject: [PATCH] hpsa: fix boot on ia64 (atomic_t alignment)

The failure initially observed as boot failure on rx3600 ia64 machine with =
RAID bus controller: Hewlett-Packard Company Smart Array P600:

    kernel unaligned access to 0xe000000105dd8b95, ip=3D0xa000000100b87551
    kernel unaligned access to 0xe000000105dd8e95, ip=3D0xa000000100b87551
    hpsa 0000:14:01.0: Controller reports max supported commands of 0 Using=
 16 instead. Ensure that firmware is up to date.
    swapper/0[1]: error during unaligned kernel access

Here unaligned access comes from 'struct CommandList' that happens to be pa=
cked. The change f749d8b7a ("scsi: hpsa: Correct dev cmds outstanding for r=
etried cmds") introduced unexpected padding and un-aligned atomic_t from na=
tural alignment to something else.

This change does not remove packing annotation from struct but only restore=
s alignment of atomic variable.

The change is tested on the same rx3600 machine.

CC: linux-ia64@vger.kernel.org
CC: storagedev@microchip.com
CC: linux-scsi@vger.kernel.org
CC: Joe Szczypek <jszczype@redhat.com>
CC: Scott Benesh <scott.benesh@microchip.com>
CC: Scott Teel <scott.teel@microchip.com>
CC: Tomas Henzl <thenzl@redhat.com>
CC: "Martin K. Petersen" <martin.petersen@oracle.com>
CC: Don Brace <don.brace@microchip.com>
Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Suggested-by: Don Brace <don.brace@microchip.com>
Fixes: f749d8b7a "scsi: hpsa: Correct dev cmds outstanding for retried cmds=
"
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
---
 drivers/scsi/hpsa_cmd.h | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h index d126bb=
877250..617bdae9a7de 100644
--- a/drivers/scsi/hpsa_cmd.h
+++ b/drivers/scsi/hpsa_cmd.h
@@ -20,6 +20,9 @@
 #ifndef HPSA_CMD_H
 #define HPSA_CMD_H

+#include <linux/build_bug.h> /* static_assert */ #include=20
+<linux/stddef.h> /* offsetof */
+
 /* general boundary defintions */
 #define SENSEINFOBYTES          32 /* may vary between hbas */
 #define SG_ENTRIES_IN_CMD      32 /* Max SG entries excluding chain blocks=
 */
@@ -448,11 +451,20 @@ struct CommandList {
         */
        struct hpsa_scsi_dev_t *phys_disk;

-       bool retry_pending;
+       int retry_pending;
        struct hpsa_scsi_dev_t *device;
        atomic_t refcount; /* Must be last to avoid memset in hpsa_cmd_init=
() */  } __aligned(COMMANDLIST_ALIGNMENT);

+/*
+ * Make sure our embedded atomic variable is aligned. Otherwise we=20
+break atomic
+ * operations on architectures that don't support unaligned atomics like I=
A64.
+ *
+ * Ideally this header should be cleaned up to only mark individual=20
+structs as
+ * packed.
+ */
+static_assert(offsetof(struct CommandList, refcount) %=20
+__alignof__(atomic_t) =3D=3D 0);
+
 /* Max S/G elements in I/O accelerator command */
 #define IOACCEL1_MAXSGENTRIES           24
 #define IOACCEL2_MAXSGENTRIES          28
--
2.30.2

Thank-you for your testing.
I would rather you add the atomic_t alignment check only. The current patch=
 under review has other changes...
https://patchwork.kernel.org/project/linux-scsi/patch/161540317205.18786.58=
21926127237311408.stgit@brunhilda/