Received-SPF: pass (google.com: domain of linux-kernel+bounces-176956-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Sun, 12 May 2024 13:06:13 -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 6/6] iommu/tegra241-cmdqv: Limit CMDs for guest owned
 VINTF
Message-ID: <ZkDo9US02pD6vysO@nvidia.com>
References: <cover.1715147377.git.nicolinc@nvidia.com>
 <062cf0a1e2b8ec6f068262cc68498b8d72b04bcc.1715147377.git.nicolinc@nvidia.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <062cf0a1e2b8ec6f068262cc68498b8d72b04bcc.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?4vkUSyqbLDeDWK3+/+KdAhTJPIGpok7N1k472PuZ40eTgn7pKmkkUpUA86O7?=
 =?us-ascii?Q?AcHCnDYoo/HGdKSf97f/BK97F/RKvpfDqC8+BPygmX8f97XjObiJpBv1saUK?=
 =?us-ascii?Q?SeeYC9zhVYq2WSzqElKMfPNTNM/hQ7y8OjIQuperoYmxWKoFJR3ILcrwcMsQ?=
 =?us-ascii?Q?UzWrRxDooz27D2YoawMKrVF7b4qAJf1/PZ+W8xgmHfo40UGF3wRdyJyQ6Cv/?=
 =?us-ascii?Q?MJNtGIfgPq35vqOzHVIw3IR2q4Z8spg8VcEum8tFOsbTQuZcHHlHl/xt9Oiw?=
 =?us-ascii?Q?MlaDPOHDGk7Tx0nMdDdw81wLsDznZ8SrmkZcNNsrEZjQHxy/r1qxfcXDOvpC?=
 =?us-ascii?Q?/+umH/QuqM4UsU+GtPDC7W3zPZvtZQTyi9P/hWff/PUmTQsfYSz4R0zVlwU1?=
 =?us-ascii?Q?VFjCqbgVuTfVxaB7xq8YSZ+xX+6+bbEZzTsa7MqUAIzsqEdH/bEYcDr+YWeV?=
 =?us-ascii?Q?hel7vA12NmXSWBYAaqk8vLU95d+S18wbfpyUURqfMyfTU+N5neoOIt8/EpY8?=
 =?us-ascii?Q?X4Si6Q+j6JHn6+Xfo6UyGyowlF3o5l82Tj4pYyopHsD40z3gvquYdxS634Uv?=
 =?us-ascii?Q?0PaQKQhYc6kZq6HB1Fp7dr01W1pbFt8mpDWOTS4+b5YHXLNvTxDsGYC9Ba5C?=
 =?us-ascii?Q?sScMqDR5Str5sRyKLLDZqY6ZrPhYhnCcWlmu6Ag+CrlUkg8eNgqdKbl9axya?=
 =?us-ascii?Q?H4bEjsgG6lIhQKTTTkMZRtnnp/x8dEmDNJ81oVYoOA2HAKAYw9E44oqQYBHB?=
 =?us-ascii?Q?3hQJlZVZ/dYj8I/hIZ9zDDYQTLdmog/LxRpEOEpAOZCdp82xwqBbBofvPLEQ?=
 =?us-ascii?Q?99t5OGisuAkFnQ5A/B2cPMXhuFcWdHYj+SnF4gNHr3Y8w0w+VIHa66r98DOW?=
 =?us-ascii?Q?q48IVaxO6TxQzMXau7C9fXcsbtuQ8d+hYTVFmACK2NpNigvdULqrAlN61/0j?=
 =?us-ascii?Q?vgPJC5iTtG+8ttMadyqty4arv9UCbN5mVmQhGh7r0GFP4UtvJf3RFhHrQNFR?=
 =?us-ascii?Q?+LpNsGsOKaXqJYUv+5DxGKn4712YnRgb8FEQlKEtD5X4mm8gG84ypG1tNXcG?=
 =?us-ascii?Q?PAo075T1PxT2DQF000uJ51bzeHe1PQaCotF60dxaK8yAZgf7mQn2JGaRYEna?=
 =?us-ascii?Q?ODupkaUUd3rGmrx1ZeylFkqWvkwbC0hZt/YdYblCzg/aa6YQINP35eAWNR6j?=
 =?us-ascii?Q?gBKaV2KoaoXEQ15BoyAQWyDKNmEW+j/s83S77v2rg2E3PFKLnEax9x2SwMMI?=
 =?us-ascii?Q?K9CUaqJkrU4hv82hnoTQ4RPCVJeysnzktdv80tIU11L0kWLw9NPRy/2T2B7Y?=
 =?us-ascii?Q?cb6S8yYg7QthEcqauzszWKt5UN4He2bZxxfyWUoYyEfnMZemDoOmKyAwtMnv?=
 =?us-ascii?Q?RF4QyBNk4/GoOmdn9sZAVn4O0bQ6vaf7ha4olEkdJ5oDPcr031yMJNrCBQ+k?=
 =?us-ascii?Q?slWATgRjpbbCKprSwSd3kFbomF09/daZyUIbJOjQBgpShNAIYRxyIMFngPEb?=
 =?us-ascii?Q?o/cBIxi/Xb24uHl8JlYnwSUqoZgOIljxLGSrysUlgmGtsQ1gZZtoNPID0CM9?=
 =?us-ascii?Q?HmKN0X0PV48ORN5GhQMOA73g0+d5/7Oe1Mk4Mpal?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7294f779-cded-48db-4a06-08dc72aaf02e
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3849.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 May 2024 17:42:48.1254
 (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: Bb7yuHzxf/kjLK+GHxSzo/NnmmtuZxhTd5lbwdCmunrO8Zv5nS5+JFkM/qv3r7p9
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR12MB6874

On Tue, May 07, 2024 at 10:56:54PM -0700, Nicolin Chen wrote:
> When VCMDQs are assigned to a VINTF owned by a guest (HYP_OWN bit unset),
> only TLB and ATC invalidation commands are supported by the VCMDQ HW. So,
> add a new helper to scan the input cmd to make sure it is supported when
> selecting a queue.
> 
> Note that the guest VM shouldn't have HYP_OWN bit being set regardless of
> guest kernel driver writing it or not, i.e. the hypervisor running in the
> host OS should wire this bit to zero when trapping a write access to this
> VINTF_CONFIG register from a guest kernel.
> 
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 20 ++++++-----
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  5 +--
>  .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c    | 36 ++++++++++++++++++-
>  3 files changed, 49 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index d1098991d64e..baf20e9976d3 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -332,10 +332,11 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
>  	return 0;
>  }
>  
> -static struct arm_smmu_cmdq *arm_smmu_get_cmdq(struct arm_smmu_device *smmu)
> +static struct arm_smmu_cmdq *
> +arm_smmu_get_cmdq(struct arm_smmu_device *smmu, u8 opcode)
>  {
>  	if (arm_smmu_has_tegra241_cmdqv(smmu))
> -		return tegra241_cmdqv_get_cmdq(smmu);
> +		return tegra241_cmdqv_get_cmdq(smmu, opcode);

It is worth a comment descrbing opcode here, I think.. At least the
nesting invalidation will send mixed batches.

opcode is sort of a handle for a group of related commands. But what
is the group? Minimally it is opcode + SYNC, right?

 The caller must only send opcode + SYNC commands to this queue.
 The opcodes XX,YY,ZZ are interchangable and can be sent together.

?

> -struct arm_smmu_cmdq *tegra241_cmdqv_get_cmdq(struct arm_smmu_device *smmu)
> +static bool tegra241_vintf_support_cmd(struct tegra241_vintf *vintf, u8 opcode)
> +{
> +       /* Hypervisor-owned VINTF can execute any command in its VCMDQs */
> +	if (READ_ONCE(vintf->hyp_own))
> +		return true;
> +
> +	/* Guest-owned VINTF must Check against the list of supported CMDs */
> +	switch (opcode) {
> +	case CMDQ_OP_TLBI_NH_ASID:
> +	case CMDQ_OP_TLBI_NH_VA:
> +	case CMDQ_OP_ATC_INV:
> +		return true;
> +	default:
> +		return false;
> +	}

When I look at the nesting patch it also includes SYNC, NH_VAA, and
NH_ALL. Are they supported here? VAA is not supported in the HW at all
right? What about NH_ALL?

It looks fine otherwise

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

Jason