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: plwDcCOczkxyjBXDzxI0ZNcx7wTGeuJc0978UyfZYUz9iz5Vo2ZVvh5QPBdo9xG2F/M+SxRVdE
 X8sFCVf/9hGHTfFWTWyCSDhkVpjTn5Sgw+bNuVn3ATXvO5WubXo5P6ouT1fOjiwdRlsVozG6OK
 YpETKnU7ZOwQfgtj3Jk+r1dIzA3o2CAMRH4Lm4Uw+2Cfv4dM7/7Jo4ek4XKqJh1E/EIab9UDNO
 nXz1HCOxFh+b43rL37wO8Nj43wi3JlM8+3TYD7TGJ0PRQ949FCIgy1MMULwpv7SCsdJBnsX0Ti
 y9k=
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/VV846qTikpA
Date:   Wed, 24 Mar 2021 18:37:42 +0000
Message-ID: <SN6PR11MB2848C136F6CB4EA66D42FFAEE1639@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: 1a84527a-da55-4042-ab69-08d8eef3e91c
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Mar 2021 18:37:42.7179
 (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: LnFQQiBfxgK/aeKXhzgpwN8lGwqUh/Ve8xV/TWISgfXwcBLo5mYD7Oc3xwBZbpPsnHBPMPtQYrtTIWo8z8RxxQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB5017
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


Acked-by: Don Brace <don.brace@microchip.com>

Thanks for your patch and extra effort.