Received-SPF: pass (google.com: domain of linux-kernel+bounces-176924-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Sun, 12 May 2024 12:39:43 -0300
From: Jason Gunthorpe <jgg@nvidia.com>
To: Nicolin Chen <nicolinc@nvidia.com>
Cc: will@kernel.org, robin.murphy@arm.com, joro@8bytes.org,
	thierry.reding@gmail.com, vdumpa@nvidia.com, jonathanh@nvidia.com,
	linux-kernel@vger.kernel.org, iommu@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org, linux-tegra@vger.kernel.org
Subject: Re: [PATCH v7 3/6] iommu/arm-smmu-v3: Enforce
 arm_smmu_cmdq_build_sync_cmd
Message-ID: <ZkDiv10JYCjNblv/@nvidia.com>
References: <cover.1715147377.git.nicolinc@nvidia.com>
 <52845c9fdfdd7f38a694e7727f3eabbd10e9f8ee.1715147377.git.nicolinc@nvidia.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <52845c9fdfdd7f38a694e7727f3eabbd10e9f8ee.1715147377.git.nicolinc@nvidia.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?HRdebMU4eKU1X2jn2m6Q2zKc+il89a6cXZY8BVRieMxsZSd1ZjOsHWy5cezt?=
 =?us-ascii?Q?l/hgbr8u1Gejg6kbDANdk7sq2usgwqzcYABsdywh3GifWfKwMps52U5ttwx0?=
 =?us-ascii?Q?4+6UCdNSuvcy3XNQ6gOSsAkm2QCteN17QXmn7TkOqs/ZzsIN4FFW5noMMqpD?=
 =?us-ascii?Q?Hgm3dXxEuZOErH5k9JA/yrW2TG7orN3xSBhdHQW1jMZYmG/3aGXEZ6J2lzgd?=
 =?us-ascii?Q?m6QYv4WPuUs0qTNiba9NF3u+271BX3hiiMRSjf4LwgAL3h3AihXkVrutNA+P?=
 =?us-ascii?Q?HJZywhx+QbEwbw7nkZRQEvOgBvjlxODRLby4vUCumVzxz/p8AUM9pkys1YCz?=
 =?us-ascii?Q?nX03DY7IOXLUPSFQtRL/vi/jwQwMWszg24mmOO3/UFaLX8KAm7A0QvH1xOnq?=
 =?us-ascii?Q?mC2S3YDQvuXvnqpKV7H1qkXsesc82ov4olFHithdJPKA4eDNiitL0MlGZVgD?=
 =?us-ascii?Q?HwAOGqDMchSdnPtrIS61jW23HOtgf4p8joqPStYZnqrzXaOwtiLDi0P3Zval?=
 =?us-ascii?Q?lZ3P4JC+/kseX21NIjsBTuC8WJrVU8iJtLEoXzSCJqNglCGLJYqZo6gKnJFy?=
 =?us-ascii?Q?Zr+JU2qUusGVgo7IVVxf7gAkSJbpFyZsE4xiJD2+bkL1jLsfXog/s9MN749F?=
 =?us-ascii?Q?YZYcAswn7TXS0dHVD37emtiHAG3ZLkjjVG//81Jk4BWmFfPg3NpAdlVePl21?=
 =?us-ascii?Q?5/WqItAPV374mb6Ym2yqjUDZPtkYfwZ00/4KqEfuFxyNhtlGMOHnQyw6jaNq?=
 =?us-ascii?Q?e/Gfk1FNuaEobeOoi9hZ4Ag5lRFvnH09rUXz4W136hspHmoQbUf1dRVkTFC7?=
 =?us-ascii?Q?cA4dMpGJs+Jg8rvreyo//y5Z5x/73PBPQAFoiyMRe5v1I0PmLvLo+hj/7N7a?=
 =?us-ascii?Q?3TdiIHC+OYQhvthCDnsVLIMS6TaOOD8GcwV5RJ9h2h4gfeXR6MW1mzSHNRiO?=
 =?us-ascii?Q?2D7IbOO8tQbyi4R2iaZpyetIUVDwaS+qoCEkZ5pOl2fH+GnC9N2ZDsAOnuwX?=
 =?us-ascii?Q?TxxBewh7CgDm8o0MTwjXa79uL5mtybPjvNLPEgBqfR98juFycGWWUbQ1B0yN?=
 =?us-ascii?Q?rLXMIrm5Nk3eRuv2C7pKp0qFgchlSFnwUyK291T/Gd+iI5k5jzgQneTfzSau?=
 =?us-ascii?Q?z1XqycdrWROtEMh5kCHo6uL1+SUzjmezToxWkQ3uRQEnZO8cS9TIPxmuugc4?=
 =?us-ascii?Q?44h7v2Vi/wgJml7LTiYbNw/gquWMaqatAU4ZKkbLb7d3nDriruvx6uYZ54UL?=
 =?us-ascii?Q?BgFTwuPflLMrnr/0SCugF6ZbQc2Woj8bv5BFni9/EpEN5UAKBCaNfr7ujWLC?=
 =?us-ascii?Q?Zv6SuoR6BWwt3eXSgoXglC2QMuyDywdfFxIBHoN8NzQL7dRPwJsANv1qO3TY?=
 =?us-ascii?Q?0qTJ3G7yH3sGcIiE7IkfmyYcO9MmO/7WaGPbUAYGne7/Ty8QWGpr/yynyHuV?=
 =?us-ascii?Q?JluMmsdG1W6e1SM8gxM5h2x+1WktumsVHe5cGNWjKbepd0fI3ivcSAxesf0k?=
 =?us-ascii?Q?GytorAqQZd5i0VkrEpivl2nS2K9vZ+Y2d9uN9o8u+JMcBZ9DZVd6nVO/dks3?=
 =?us-ascii?Q?qzIKnhBJYSGYAKa1eLiAz2GERz3ANXdjYJDcpWTB?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 315bc7af-acee-48ae-36e4-08dc7299c451
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3849.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 May 2024 15:39:53.0092
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 9kZFopkrdt+cs2DDHECK3T4Xx/n1q+w27GviqXgJU5v/rq/mkae4puME41stZNHu
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR12MB8482

On Tue, May 07, 2024 at 10:56:51PM -0700, Nicolin Chen wrote:
> There is an existing arm_smmu_cmdq_build_sync_cmd() so the driver should
> call it at all places other than going through arm_smmu_cmdq_build_cmd()
> separately. This helps the following patch that adds a CS_NONE quirk for
> tegra241-cmdqv driver.
> 
> Note that this changes the type of CMD_SYNC in __arm_smmu_cmdq_skip_err,
> for ARM_SMMU_OPT_MSIPOLL=true cases, from previously a non-MSI one to an
> MSI one that is proven to still work by a hacking test:
>   nvme: Adding to iommu group 10
>   nvme: --------hacking-----------
>   arm-smmu-v3: unexpected global error reported (0x00000001),
>                this could be serious
>   arm-smmu-v3: CMDQ error (cons 0x01000022): Illegal command
>   arm-smmu-v3: skipping command in error state:
>   arm-smmu-v3:  0x0000000000000000
>   arm-smmu-v3:  0x0000000000000000
>   nvme: -------recovered----------
>   nvme nvme0: 72/0/0 default/read/poll queues
>    nvme0n1: p1 p2

Nice
 
> @@ -350,20 +340,23 @@ static struct arm_smmu_cmdq *arm_smmu_get_cmdq(struct arm_smmu_device *smmu)
>  static void arm_smmu_cmdq_build_sync_cmd(u64 *cmd, struct arm_smmu_device *smmu,
>  					 struct arm_smmu_queue *q, u32 prod)
>  {
> -	struct arm_smmu_cmdq_ent ent = {
> -		.opcode = CMDQ_OP_CMD_SYNC,
> -	};
> +	memset(cmd, 0, 1 << CMDQ_ENT_SZ_SHIFT);

The command would also benifit from its own type someday :\

Maybe this should just be cmd[1] = 0 ?

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason