* [PATCH v7 0/6] Add Tegra241 (Grace) CMDQV Support (part 1/2)
@ 2024-05-08 5:56 Nicolin Chen
2024-05-08 5:56 ` [PATCH v7 1/6] iommu/arm-smmu-v3: Make symbols public for CONFIG_TEGRA241_CMDQV Nicolin Chen
` (5 more replies)
0 siblings, 6 replies; 16+ messages in thread
From: Nicolin Chen @ 2024-05-08 5:56 UTC (permalink / raw)
To: will, robin.murphy, jgg
Cc: joro, thierry.reding, vdumpa, jonathanh, linux-kernel, iommu,
linux-arm-kernel, linux-tegra
NVIDIA's Tegra241 (Grace) SoC has a CMDQ-Virtualization (CMDQV) hardware
that extends standard ARM SMMUv3 to support multiple command queues with
virtualization capabilities. Though this is similar to the ECMDQ in SMMU
v3.3, CMDQV provides additional Virtual Interfaces (VINTFs) allowing VMs
to have their own VINTFs and Virtual Command Queues (VCMDQs). The VCMDQs
can only execute a limited set of commands, mainly invalidation commands
when exclusively used by the VMs, compared to the standard SMMUv3 CMDQ.
Thus, there are two parts of patch series to add its support: the basic
in-kernel support as part 1, and the user-space support as part 2.
The in-kernel support is to detect/configure the CMDQV hardware and then
allocate a VINTF with some VCMDQs for the kernel/hypervisor to use. Like
ECMDQ, CMDQV also allows the kernel to use multiple VCMDQs, giving some
limited performance improvement: up to 20% reduction of TLB invalidation
time was measured by a multi-threaded DMA unmap benchmark, compared to a
single queue.
The user-space support is to provide uAPIs (via IOMMUFD) for hypervisors
in user space to passthrough VCMDQs to VMs, allowing these VMs to access
the VCMDQs directly without trappings, i.e. no VM Exits. This gives huge
performance improvements: 70% to 90% reductions of TLB invalidation time
were measured by various DMA unmap tests running in a guest OS, compared
to a nested SMMU CMDQ (with trappings).
This is the part-1 series:
- Preparatory changes to share the existing SMMU functions
- A new CMDQV driver and extending the SMMUv3 driver to interact with
the new driver
- Limit the commands for a guest kernel.
It's available on Github:
https://github.com/nicolinc/iommufd/commits/vcmdq_in_kernel-v7
And the part-2 RFC series is also sent for discussion:
https://lore.kernel.org/all/cover.1712978212.git.nicolinc@nvidia.com/
Note that this in-kernel support isn't confined to host kernels running
on Grace-powered servers, but is also used by guest kernels running on
VMs virtualized on those servers. So, those VMs must install the driver,
ideally before the part 2 is merged. So, later those servers would only
need to upgrade their host kernels without bothering the VMs.
Thank you!
Changelog
v7:
* Moved all public symbols into one single patch
* Enforced a command batch to use the same cmdq
* Enforced the use of arm_smmu_cmdq_build_sync_cmd()
* Reworked the tegra241-cmdqv driver patch
- Dropped logging macros, cmdqv->dev, and atomic
- Dropped devm_* and added tegra241_cmdqv_device_remove()
- Moved all structure allocations to cmdqv's probe() from
device_reset() where only register configurations remains
- Switched the config macros to inline functions
- Optimized ISR routine with 64-bit reading MMIO
- Scan once per batch against command list
- Reorganized function locations
- Minor readability changes
v6:
https://lore.kernel.org/all/cover.1714451595.git.nicolinc@nvidia.com/
* Reordered the patch sequence to fix git-bisect break
* Added a status cache to cmdqv/vintf/vcmdq structure
* Added gerror/gerrorn value match in hw_deinit()
* Minimized changes in __arm_smmu_cmdq_skip_err()
* Preallocated VCMDQs to VINTFs for stablility
v5:
https://lore.kernel.org/all/cover.1712977210.git.nicolinc@nvidia.com/
* Improved print/mmio helpers
* Added proper register reset routines
* Reorganized init/deinit functions to share with VIOMMU callbacks in
the upcoming part-2 user-space series (RFC)
v4:
https://lore.kernel.org/all/cover.1711690673.git.nicolinc@nvidia.com/
* Rebased on v6.9-rc1
* Renamed to "tegra241-cmdqv", following other Grace kernel patches
* Added a set of print and MMIO helpers
* Reworked the guest limitation patch
v3:
https://lore.kernel.org/all/20211119071959.16706-1-nicolinc@nvidia.com/
* Dropped VMID and mdev patches to redesign later based on IOMMUFD
* Separated HYP_OWN part for guest support into a new patch
* Added new preparatory changes
v2:
https://lore.kernel.org/all/20210831025923.15812-1-nicolinc@nvidia.com/
* Added mdev interface support for hypervisor and VMs
* Added preparatory changes for mdev interface implementation
* PATCH-12 Changed ->issue_cmdlist() to ->get_cmdq() for a better
integration with recently merged ECMDQ-related changes
v1:
https://lore.kernel.org/all/20210723193140.9690-1-nicolinc@nvidia.com/
Nate Watterson (1):
iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace)
CMDQV
Nicolin Chen (5):
iommu/arm-smmu-v3: Make symbols public for CONFIG_TEGRA241_CMDQV
iommu/arm-smmu-v3: Issue a batch of commands to the same cmdq
iommu/arm-smmu-v3: Enforce arm_smmu_cmdq_build_sync_cmd
iommu/arm-smmu-v3: Add CS_NONE quirk for CONFIG_TEGRA241_CMDQV
iommu/tegra241-cmdqv: Limit CMDs for guest owned VINTF
MAINTAINERS | 1 +
drivers/iommu/Kconfig | 11 +
drivers/iommu/arm/arm-smmu-v3/Makefile | 1 +
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 164 ++--
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 67 +-
.../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 911 ++++++++++++++++++
6 files changed, 1088 insertions(+), 67 deletions(-)
create mode 100644 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
--
2.43.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v7 1/6] iommu/arm-smmu-v3: Make symbols public for CONFIG_TEGRA241_CMDQV
2024-05-08 5:56 [PATCH v7 0/6] Add Tegra241 (Grace) CMDQV Support (part 1/2) Nicolin Chen
@ 2024-05-08 5:56 ` Nicolin Chen
2024-05-08 5:56 ` [PATCH v7 2/6] iommu/arm-smmu-v3: Issue a batch of commands to the same cmdq Nicolin Chen
` (4 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Nicolin Chen @ 2024-05-08 5:56 UTC (permalink / raw)
To: will, robin.murphy, jgg
Cc: joro, thierry.reding, vdumpa, jonathanh, linux-kernel, iommu,
linux-arm-kernel, linux-tegra
The symbols __arm_smmu_cmdq_skip_err(), arm_smmu_init_one_queue(), and
arm_smmu_cmdq_init() need to be used by the tegra241-cmdqv compilation
unit in a following patch. Remove the static and put prototypes in the
header.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
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 | 9 +++++++++
2 files changed, 18 insertions(+), 11 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 1ad0937760c6..31ada4c7b87d 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -366,8 +366,8 @@ static void arm_smmu_cmdq_build_sync_cmd(u64 *cmd, struct arm_smmu_device *smmu,
arm_smmu_cmdq_build_cmd(cmd, &ent);
}
-static void __arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu,
- struct arm_smmu_queue *q)
+void __arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu,
+ struct arm_smmu_queue *q)
{
static const char * const cerror_str[] = {
[CMDQ_ERR_CERROR_NONE_IDX] = "No error",
@@ -3121,12 +3121,10 @@ static struct iommu_ops arm_smmu_ops = {
};
/* Probing and initialisation functions */
-static int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
- struct arm_smmu_queue *q,
- void __iomem *page,
- unsigned long prod_off,
- unsigned long cons_off,
- size_t dwords, const char *name)
+int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
+ struct arm_smmu_queue *q, void __iomem *page,
+ unsigned long prod_off, unsigned long cons_off,
+ size_t dwords, const char *name)
{
size_t qsz;
@@ -3164,9 +3162,9 @@ static int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
return 0;
}
-static int arm_smmu_cmdq_init(struct arm_smmu_device *smmu)
+int arm_smmu_cmdq_init(struct arm_smmu_device *smmu,
+ struct arm_smmu_cmdq *cmdq)
{
- struct arm_smmu_cmdq *cmdq = &smmu->cmdq;
unsigned int nents = 1 << cmdq->q.llq.max_n_shift;
atomic_set(&cmdq->owner_prod, 0);
@@ -3191,7 +3189,7 @@ static int arm_smmu_init_queues(struct arm_smmu_device *smmu)
if (ret)
return ret;
- ret = arm_smmu_cmdq_init(smmu);
+ ret = arm_smmu_cmdq_init(smmu, &smmu->cmdq);
if (ret)
return ret;
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 1242a086c9f9..67b0ca0b1b79 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -794,6 +794,15 @@ bool arm_smmu_free_asid(struct arm_smmu_ctx_desc *cd);
int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, int ssid,
unsigned long iova, size_t size);
+void __arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu,
+ struct arm_smmu_queue *q);
+int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
+ struct arm_smmu_queue *q, void __iomem *page,
+ unsigned long prod_off, unsigned long cons_off,
+ size_t dwords, const char *name);
+int arm_smmu_cmdq_init(struct arm_smmu_device *smmu,
+ struct arm_smmu_cmdq *cmdq);
+
#ifdef CONFIG_ARM_SMMU_V3_SVA
bool arm_smmu_sva_supported(struct arm_smmu_device *smmu);
bool arm_smmu_master_sva_supported(struct arm_smmu_master *master);
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 2/6] iommu/arm-smmu-v3: Issue a batch of commands to the same cmdq
2024-05-08 5:56 [PATCH v7 0/6] Add Tegra241 (Grace) CMDQV Support (part 1/2) Nicolin Chen
2024-05-08 5:56 ` [PATCH v7 1/6] iommu/arm-smmu-v3: Make symbols public for CONFIG_TEGRA241_CMDQV Nicolin Chen
@ 2024-05-08 5:56 ` Nicolin Chen
2024-05-12 15:34 ` Jason Gunthorpe
2024-05-08 5:56 ` [PATCH v7 3/6] iommu/arm-smmu-v3: Enforce arm_smmu_cmdq_build_sync_cmd Nicolin Chen
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Nicolin Chen @ 2024-05-08 5:56 UTC (permalink / raw)
To: will, robin.murphy, jgg
Cc: joro, thierry.reding, vdumpa, jonathanh, linux-kernel, iommu,
linux-arm-kernel, linux-tegra
The driver calls in different places the arm_smmu_get_cmdq() helper, and
it's fine to do so since the helper always returns the single SMMU CMDQ.
However, with NVIDIA CMDQV extension or SMMU ECMDQ, there can be multiple
cmdqs in the system to select one from. And either case requires a batch
of commands to be issued to the same cmdq. Thus, a cmdq has to be decided
in the higher-level callers.
Add a cmdq pointer in arm_smmu_cmdq_batch structure, and decide the cmdq
when initializing the batch. Pass it down to the bottom function. Update
__arm_smmu_cmdq_issue_cmd() accordingly for single command issuers.
Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 44 +++++++++++++--------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 +
2 files changed, 29 insertions(+), 16 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 31ada4c7b87d..3d2962f6dbda 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -588,11 +588,11 @@ static void arm_smmu_cmdq_poll_valid_map(struct arm_smmu_cmdq *cmdq,
/* Wait for the command queue to become non-full */
static int arm_smmu_cmdq_poll_until_not_full(struct arm_smmu_device *smmu,
+ struct arm_smmu_cmdq *cmdq,
struct arm_smmu_ll_queue *llq)
{
unsigned long flags;
struct arm_smmu_queue_poll qp;
- struct arm_smmu_cmdq *cmdq = arm_smmu_get_cmdq(smmu);
int ret = 0;
/*
@@ -623,11 +623,11 @@ static int arm_smmu_cmdq_poll_until_not_full(struct arm_smmu_device *smmu,
* Must be called with the cmdq lock held in some capacity.
*/
static int __arm_smmu_cmdq_poll_until_msi(struct arm_smmu_device *smmu,
+ struct arm_smmu_cmdq *cmdq,
struct arm_smmu_ll_queue *llq)
{
int ret = 0;
struct arm_smmu_queue_poll qp;
- struct arm_smmu_cmdq *cmdq = arm_smmu_get_cmdq(smmu);
u32 *cmd = (u32 *)(Q_ENT(&cmdq->q, llq->prod));
queue_poll_init(smmu, &qp);
@@ -647,10 +647,10 @@ static int __arm_smmu_cmdq_poll_until_msi(struct arm_smmu_device *smmu,
* Must be called with the cmdq lock held in some capacity.
*/
static int __arm_smmu_cmdq_poll_until_consumed(struct arm_smmu_device *smmu,
+ struct arm_smmu_cmdq *cmdq,
struct arm_smmu_ll_queue *llq)
{
struct arm_smmu_queue_poll qp;
- struct arm_smmu_cmdq *cmdq = arm_smmu_get_cmdq(smmu);
u32 prod = llq->prod;
int ret = 0;
@@ -697,12 +697,13 @@ static int __arm_smmu_cmdq_poll_until_consumed(struct arm_smmu_device *smmu,
}
static int arm_smmu_cmdq_poll_until_sync(struct arm_smmu_device *smmu,
+ struct arm_smmu_cmdq *cmdq,
struct arm_smmu_ll_queue *llq)
{
if (smmu->options & ARM_SMMU_OPT_MSIPOLL)
- return __arm_smmu_cmdq_poll_until_msi(smmu, llq);
+ return __arm_smmu_cmdq_poll_until_msi(smmu, cmdq, llq);
- return __arm_smmu_cmdq_poll_until_consumed(smmu, llq);
+ return __arm_smmu_cmdq_poll_until_consumed(smmu, cmdq, llq);
}
static void arm_smmu_cmdq_write_entries(struct arm_smmu_cmdq *cmdq, u64 *cmds,
@@ -739,13 +740,13 @@ static void arm_smmu_cmdq_write_entries(struct arm_smmu_cmdq *cmdq, u64 *cmds,
* CPU will appear before any of the commands from the other CPU.
*/
static int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
+ struct arm_smmu_cmdq *cmdq,
u64 *cmds, int n, bool sync)
{
u64 cmd_sync[CMDQ_ENT_DWORDS];
u32 prod;
unsigned long flags;
bool owner;
- struct arm_smmu_cmdq *cmdq = arm_smmu_get_cmdq(smmu);
struct arm_smmu_ll_queue llq, head;
int ret = 0;
@@ -759,7 +760,7 @@ static int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
while (!queue_has_space(&llq, n + sync)) {
local_irq_restore(flags);
- if (arm_smmu_cmdq_poll_until_not_full(smmu, &llq))
+ if (arm_smmu_cmdq_poll_until_not_full(smmu, cmdq, &llq))
dev_err_ratelimited(smmu->dev, "CMDQ timeout\n");
local_irq_save(flags);
}
@@ -835,7 +836,7 @@ static int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
/* 5. If we are inserting a CMD_SYNC, we must wait for it to complete */
if (sync) {
llq.prod = queue_inc_prod_n(&llq, n);
- ret = arm_smmu_cmdq_poll_until_sync(smmu, &llq);
+ ret = arm_smmu_cmdq_poll_until_sync(smmu, cmdq, &llq);
if (ret) {
dev_err_ratelimited(smmu->dev,
"CMD_SYNC timeout at 0x%08x [hwprod 0x%08x, hwcons 0x%08x]\n",
@@ -870,7 +871,8 @@ static int __arm_smmu_cmdq_issue_cmd(struct arm_smmu_device *smmu,
return -EINVAL;
}
- return arm_smmu_cmdq_issue_cmdlist(smmu, cmd, 1, sync);
+ return arm_smmu_cmdq_issue_cmdlist(
+ smmu, arm_smmu_get_cmdq(smmu), cmd, 1, sync);
}
static int arm_smmu_cmdq_issue_cmd(struct arm_smmu_device *smmu,
@@ -885,6 +887,13 @@ static int arm_smmu_cmdq_issue_cmd_with_sync(struct arm_smmu_device *smmu,
return __arm_smmu_cmdq_issue_cmd(smmu, ent, true);
}
+static void arm_smmu_cmdq_batch_init(struct arm_smmu_device *smmu,
+ struct arm_smmu_cmdq_batch *cmds)
+{
+ cmds->num = 0;
+ cmds->cmdq = arm_smmu_get_cmdq(smmu);
+}
+
static void arm_smmu_cmdq_batch_add(struct arm_smmu_device *smmu,
struct arm_smmu_cmdq_batch *cmds,
struct arm_smmu_cmdq_ent *cmd)
@@ -893,12 +902,14 @@ static void arm_smmu_cmdq_batch_add(struct arm_smmu_device *smmu,
if (cmds->num == CMDQ_BATCH_ENTRIES - 1 &&
(smmu->options & ARM_SMMU_OPT_CMDQ_FORCE_SYNC)) {
- arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmds, cmds->num, true);
+ arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmdq, cmds->cmds,
+ cmds->num, true);
cmds->num = 0;
}
if (cmds->num == CMDQ_BATCH_ENTRIES) {
- arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmds, cmds->num, false);
+ arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmdq, cmds->cmds,
+ cmds->num, false);
cmds->num = 0;
}
@@ -915,7 +926,8 @@ static void arm_smmu_cmdq_batch_add(struct arm_smmu_device *smmu,
static int arm_smmu_cmdq_batch_submit(struct arm_smmu_device *smmu,
struct arm_smmu_cmdq_batch *cmds)
{
- return arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmds, cmds->num, true);
+ return arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmdq, cmds->cmds,
+ cmds->num, true);
}
static void arm_smmu_page_response(struct device *dev, struct iopf_fault *unused,
@@ -1156,7 +1168,7 @@ static void arm_smmu_sync_cd(struct arm_smmu_master *master,
},
};
- cmds.num = 0;
+ arm_smmu_cmdq_batch_init(smmu, &cmds);
for (i = 0; i < master->num_streams; i++) {
cmd.cfgi.sid = master->streams[i].id;
arm_smmu_cmdq_batch_add(smmu, &cmds, &cmd);
@@ -1995,7 +2007,7 @@ static int arm_smmu_atc_inv_master(struct arm_smmu_master *master)
arm_smmu_atc_inv_to_cmd(IOMMU_NO_PASID, 0, 0, &cmd);
- cmds.num = 0;
+ arm_smmu_cmdq_batch_init(master->smmu, &cmds);
for (i = 0; i < master->num_streams; i++) {
cmd.atc.sid = master->streams[i].id;
arm_smmu_cmdq_batch_add(master->smmu, &cmds, &cmd);
@@ -2035,7 +2047,7 @@ int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, int ssid,
arm_smmu_atc_inv_to_cmd(ssid, iova, size, &cmd);
- cmds.num = 0;
+ arm_smmu_cmdq_batch_init(smmu_domain->smmu, &cmds);
spin_lock_irqsave(&smmu_domain->devices_lock, flags);
list_for_each_entry(master, &smmu_domain->devices, domain_head) {
@@ -2112,7 +2124,7 @@ static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd,
num_pages++;
}
- cmds.num = 0;
+ arm_smmu_cmdq_batch_init(smmu_domain->smmu, &cmds);
while (iova < end) {
if (smmu->features & ARM_SMMU_FEAT_RANGE_INV) {
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 67b0ca0b1b79..8e4fbf4f50f3 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -564,6 +564,7 @@ struct arm_smmu_cmdq {
struct arm_smmu_cmdq_batch {
u64 cmds[CMDQ_BATCH_ENTRIES * CMDQ_ENT_DWORDS];
+ struct arm_smmu_cmdq *cmdq;
int num;
};
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 3/6] iommu/arm-smmu-v3: Enforce arm_smmu_cmdq_build_sync_cmd
2024-05-08 5:56 [PATCH v7 0/6] Add Tegra241 (Grace) CMDQV Support (part 1/2) Nicolin Chen
2024-05-08 5:56 ` [PATCH v7 1/6] iommu/arm-smmu-v3: Make symbols public for CONFIG_TEGRA241_CMDQV Nicolin Chen
2024-05-08 5:56 ` [PATCH v7 2/6] iommu/arm-smmu-v3: Issue a batch of commands to the same cmdq Nicolin Chen
@ 2024-05-08 5:56 ` Nicolin Chen
2024-05-12 15:39 ` Jason Gunthorpe
2024-05-08 5:56 ` [PATCH v7 4/6] iommu/arm-smmu-v3: Add CS_NONE quirk for CONFIG_TEGRA241_CMDQV Nicolin Chen
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Nicolin Chen @ 2024-05-08 5:56 UTC (permalink / raw)
To: will, robin.murphy, jgg
Cc: joro, thierry.reding, vdumpa, jonathanh, linux-kernel, iommu,
linux-arm-kernel, linux-tegra
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
Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 36 ++++++++-------------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 3 --
2 files changed, 13 insertions(+), 26 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 3d2962f6dbda..569bca1a8a1f 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -325,16 +325,6 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
cmd[0] |= FIELD_PREP(CMDQ_RESUME_0_RESP, ent->resume.resp);
cmd[1] |= FIELD_PREP(CMDQ_RESUME_1_STAG, ent->resume.stag);
break;
- case CMDQ_OP_CMD_SYNC:
- if (ent->sync.msiaddr) {
- cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_CS, CMDQ_SYNC_0_CS_IRQ);
- cmd[1] |= ent->sync.msiaddr & CMDQ_SYNC_1_MSIADDR_MASK;
- } else {
- cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_CS, CMDQ_SYNC_0_CS_SEV);
- }
- cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_MSH, ARM_SMMU_SH_ISH);
- cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_MSIATTR, ARM_SMMU_MEMATTR_OIWB);
- break;
default:
return -ENOENT;
}
@@ -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);
+ cmd[0] = FIELD_PREP(CMDQ_0_OP, CMDQ_OP_CMD_SYNC) |
+ FIELD_PREP(CMDQ_SYNC_0_MSH, ARM_SMMU_SH_ISH) |
+ FIELD_PREP(CMDQ_SYNC_0_MSIATTR, ARM_SMMU_MEMATTR_OIWB);
+
+ if (!(smmu->options & ARM_SMMU_OPT_MSIPOLL)) {
+ cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_CS, CMDQ_SYNC_0_CS_SEV);
+ return;
+ }
/*
* Beware that Hi16xx adds an extra 32 bits of goodness to its MSI
* payload, so the write will zero the entire command on that platform.
*/
- if (smmu->options & ARM_SMMU_OPT_MSIPOLL) {
- ent.sync.msiaddr = q->base_dma + Q_IDX(&q->llq, prod) *
- q->ent_dwords * 8;
- }
-
- arm_smmu_cmdq_build_cmd(cmd, &ent);
+ cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_CS, CMDQ_SYNC_0_CS_IRQ);
+ cmd[1] = (q->base_dma + Q_IDX(&q->llq, prod) * q->ent_dwords * 8) &
+ CMDQ_SYNC_1_MSIADDR_MASK;
}
void __arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu,
@@ -380,9 +373,6 @@ void __arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu,
u64 cmd[CMDQ_ENT_DWORDS];
u32 cons = readl_relaxed(q->cons_reg);
u32 idx = FIELD_GET(CMDQ_CONS_ERR, cons);
- struct arm_smmu_cmdq_ent cmd_sync = {
- .opcode = CMDQ_OP_CMD_SYNC,
- };
dev_err(smmu->dev, "CMDQ error (cons 0x%08x): %s\n", cons,
idx < ARRAY_SIZE(cerror_str) ? cerror_str[idx] : "Unknown");
@@ -416,7 +406,7 @@ void __arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu,
dev_err(smmu->dev, "\t0x%016llx\n", (unsigned long long)cmd[i]);
/* Convert the erroneous command into a CMD_SYNC */
- arm_smmu_cmdq_build_cmd(cmd, &cmd_sync);
+ arm_smmu_cmdq_build_sync_cmd(cmd, smmu, q, cons);
queue_write(Q_ENT(q, cons), cmd, q->ent_dwords);
}
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 8e4fbf4f50f3..180c0b1e0658 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -512,9 +512,6 @@ struct arm_smmu_cmdq_ent {
} resume;
#define CMDQ_OP_CMD_SYNC 0x46
- struct {
- u64 msiaddr;
- } sync;
};
};
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 4/6] iommu/arm-smmu-v3: Add CS_NONE quirk for CONFIG_TEGRA241_CMDQV
2024-05-08 5:56 [PATCH v7 0/6] Add Tegra241 (Grace) CMDQV Support (part 1/2) Nicolin Chen
` (2 preceding siblings ...)
2024-05-08 5:56 ` [PATCH v7 3/6] iommu/arm-smmu-v3: Enforce arm_smmu_cmdq_build_sync_cmd Nicolin Chen
@ 2024-05-08 5:56 ` Nicolin Chen
2024-05-08 5:56 ` [PATCH v7 5/6] iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV Nicolin Chen
2024-05-08 5:56 ` [PATCH v7 6/6] iommu/tegra241-cmdqv: Limit CMDs for guest owned VINTF Nicolin Chen
5 siblings, 0 replies; 16+ messages in thread
From: Nicolin Chen @ 2024-05-08 5:56 UTC (permalink / raw)
To: will, robin.murphy, jgg
Cc: joro, thierry.reding, vdumpa, jonathanh, linux-kernel, iommu,
linux-arm-kernel, linux-tegra
The CMDQV extension in NVIDIA Tegra241 SoC only supports CS_NONE in the
CS field of CMD_SYNC. Add a quirk flag to accommodate that.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 8 +++++++-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 3 +++
2 files changed, 10 insertions(+), 1 deletion(-)
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 569bca1a8a1f..2d8eb7c08a85 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -345,6 +345,11 @@ static void arm_smmu_cmdq_build_sync_cmd(u64 *cmd, struct arm_smmu_device *smmu,
FIELD_PREP(CMDQ_SYNC_0_MSH, ARM_SMMU_SH_ISH) |
FIELD_PREP(CMDQ_SYNC_0_MSIATTR, ARM_SMMU_MEMATTR_OIWB);
+ if (q->quirks & CMDQ_QUIRK_SYNC_CS_NONE_ONLY) {
+ cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_CS, CMDQ_SYNC_0_CS_NONE);
+ return;
+ }
+
if (!(smmu->options & ARM_SMMU_OPT_MSIPOLL)) {
cmd[0] |= FIELD_PREP(CMDQ_SYNC_0_CS, CMDQ_SYNC_0_CS_SEV);
return;
@@ -690,7 +695,8 @@ static int arm_smmu_cmdq_poll_until_sync(struct arm_smmu_device *smmu,
struct arm_smmu_cmdq *cmdq,
struct arm_smmu_ll_queue *llq)
{
- if (smmu->options & ARM_SMMU_OPT_MSIPOLL)
+ if (smmu->options & ARM_SMMU_OPT_MSIPOLL &&
+ !(cmdq->q.quirks & CMDQ_QUIRK_SYNC_CS_NONE_ONLY))
return __arm_smmu_cmdq_poll_until_msi(smmu, cmdq, llq);
return __arm_smmu_cmdq_poll_until_consumed(smmu, cmdq, llq);
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 180c0b1e0658..01227c0de290 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -543,6 +543,9 @@ struct arm_smmu_queue {
u32 __iomem *prod_reg;
u32 __iomem *cons_reg;
+
+#define CMDQ_QUIRK_SYNC_CS_NONE_ONLY BIT(0) /* CMD_SYNC CS field supports CS_NONE only */
+ u32 quirks;
};
struct arm_smmu_queue_poll {
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 5/6] iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV
2024-05-08 5:56 [PATCH v7 0/6] Add Tegra241 (Grace) CMDQV Support (part 1/2) Nicolin Chen
` (3 preceding siblings ...)
2024-05-08 5:56 ` [PATCH v7 4/6] iommu/arm-smmu-v3: Add CS_NONE quirk for CONFIG_TEGRA241_CMDQV Nicolin Chen
@ 2024-05-08 5:56 ` Nicolin Chen
2024-05-12 15:54 ` Jason Gunthorpe
2024-05-08 5:56 ` [PATCH v7 6/6] iommu/tegra241-cmdqv: Limit CMDs for guest owned VINTF Nicolin Chen
5 siblings, 1 reply; 16+ messages in thread
From: Nicolin Chen @ 2024-05-08 5:56 UTC (permalink / raw)
To: will, robin.murphy, jgg
Cc: joro, thierry.reding, vdumpa, jonathanh, linux-kernel, iommu,
linux-arm-kernel, linux-tegra
From: Nate Watterson <nwatterson@nvidia.com>
NVIDIA's Tegra241 Soc has a CMDQ-Virtualization (CMDQV) hardware, extending
the standard ARM SMMU v3 IP to support multiple VCMDQs with virtualization
capabilities. In terms of command queue, they are very like a standard SMMU
CMDQ (or ECMDQs), but only support CS_NONE in the CS field of CMD_SYNC.
Add a new tegra241-cmdqv driver, and insert its structure pointer into the
existing arm_smmu_device, and then add related function calls in the SMMUv3
driver to interact with the CMDQV driver.
In the CMDQV driver, add a minimal part for the in-kernel support: reserve
VINTF0 for in-kernel use, and assign some of the VCMDQs to the VINTF0, and
select one VCMDQ based on the current CPU ID to execute supported commands.
This multi-queue design for in-kernel use gives some limited improvements:
up to 20% reduction of invalidation time was measured by a multi-threaded
DMA unmap benchmark, compared to a single queue.
The other part of the CMDQV driver will be user-space support that gives a
hypervisor running on the host OS to talk to the driver for virtualization
use cases, allowing VMs to use VCMDQs without trappings, i.e. no VM Exits.
This is designed based on IOMMUFD, and its RFC series is also under review.
It will provide a guest OS a bigger improvement: 70% to 90% reductions of
TLB invalidation time were measured by DMA unmap tests running in a guest,
compared to nested SMMU CMDQ (with trappings).
However, it is very important for this in-kernel support to get merged and
installed to VMs running on Grace-powered servers as soon as possible. So,
later those servers would only need to upgrade their host kernels for the
user-space support.
As the initial version, the CMDQV driver only supports ACPI configurations.
Signed-off-by: Nate Watterson <nwatterson@nvidia.com>
Co-developed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
MAINTAINERS | 1 +
drivers/iommu/Kconfig | 11 +
drivers/iommu/arm/arm-smmu-v3/Makefile | 1 +
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 52 +-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 50 +
.../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 877 ++++++++++++++++++
6 files changed, 980 insertions(+), 12 deletions(-)
create mode 100644 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
diff --git a/MAINTAINERS b/MAINTAINERS
index ec0284125e8f..3022058f9aae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21740,6 +21740,7 @@ M: Thierry Reding <thierry.reding@gmail.com>
R: Krishna Reddy <vdumpa@nvidia.com>
L: linux-tegra@vger.kernel.org
S: Supported
+F: drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
F: drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
F: drivers/iommu/tegra*
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 66325210c8c9..1e27d9777136 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -423,6 +423,17 @@ config ARM_SMMU_V3_KUNIT_TEST
Enable this option to unit-test arm-smmu-v3 driver functions.
If unsure, say N.
+
+config TEGRA241_CMDQV
+ bool "NVIDIA Tegra241 CMDQ-V extension support for ARM SMMUv3"
+ depends on ACPI
+ help
+ Support for NVIDIA CMDQ-Virtualization extension for ARM SMMUv3. The
+ CMDQ-V extension is similar to v3.3 ECMDQ for multi command queues
+ support, except with virtualization capabilities.
+
+ Say Y here if your system is NVIDIA Tegra241 (Grace) or it has the same
+ CMDQ-V extension.
endif
config S390_IOMMU
diff --git a/drivers/iommu/arm/arm-smmu-v3/Makefile b/drivers/iommu/arm/arm-smmu-v3/Makefile
index 0b97054b3929..f1d64fda36a4 100644
--- a/drivers/iommu/arm/arm-smmu-v3/Makefile
+++ b/drivers/iommu/arm/arm-smmu-v3/Makefile
@@ -3,4 +3,5 @@ obj-$(CONFIG_ARM_SMMU_V3) += arm_smmu_v3.o
arm_smmu_v3-objs-y += arm-smmu-v3.o
arm_smmu_v3-objs-$(CONFIG_ARM_SMMU_V3_SVA) += arm-smmu-v3-sva.o
arm_smmu_v3-objs-$(CONFIG_ARM_SMMU_V3_KUNIT_TEST) += arm-smmu-v3-test.o
+arm_smmu_v3-objs-$(CONFIG_TEGRA241_CMDQV) += tegra241-cmdqv.o
arm_smmu_v3-objs := $(arm_smmu_v3-objs-y)
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 2d8eb7c08a85..d1098991d64e 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -334,6 +334,9 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
static struct arm_smmu_cmdq *arm_smmu_get_cmdq(struct arm_smmu_device *smmu)
{
+ if (arm_smmu_has_tegra241_cmdqv(smmu))
+ return tegra241_cmdqv_get_cmdq(smmu);
+
return &smmu->cmdq;
}
@@ -3588,6 +3591,15 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
return ret;
}
+ if (arm_smmu_has_tegra241_cmdqv(smmu)) {
+ ret = tegra241_cmdqv_device_reset(smmu);
+ if (ret) {
+ dev_warn(smmu->dev,
+ "tegra241_cmdqv: falling back to cmdq\n");
+ tegra241_cmdqv_device_remove(smmu);
+ }
+ }
+
/* Invalidate any cached configuration */
cmd.opcode = CMDQ_OP_CFGI_ALL;
arm_smmu_cmdq_issue_cmd_with_sync(smmu, &cmd);
@@ -3956,6 +3968,8 @@ static int arm_smmu_device_acpi_probe(struct platform_device *pdev,
if (iort_smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE)
smmu->features |= ARM_SMMU_FEAT_COHERENCY;
+ smmu->tegra241_cmdqv = tegra241_cmdqv_acpi_probe(smmu, node);
+
return 0;
}
#else
@@ -4060,11 +4074,14 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
/* Base address */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res)
- return -EINVAL;
+ if (!res) {
+ ret = -EINVAL;
+ goto out;
+ }
if (resource_size(res) < arm_smmu_resource_size(smmu)) {
dev_err(dev, "MMIO region too small (%pr)\n", res);
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}
ioaddr = res->start;
@@ -4073,14 +4090,18 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
* the PMCG registers which are reserved by the PMU driver.
*/
smmu->base = arm_smmu_ioremap(dev, ioaddr, ARM_SMMU_REG_SZ);
- if (IS_ERR(smmu->base))
- return PTR_ERR(smmu->base);
+ if (IS_ERR(smmu->base)) {
+ ret = PTR_ERR(smmu->base);
+ goto out;
+ }
if (arm_smmu_resource_size(smmu) > SZ_64K) {
smmu->page1 = arm_smmu_ioremap(dev, ioaddr + SZ_64K,
ARM_SMMU_REG_SZ);
- if (IS_ERR(smmu->page1))
- return PTR_ERR(smmu->page1);
+ if (IS_ERR(smmu->page1)) {
+ ret = PTR_ERR(smmu->page1);
+ goto out;
+ }
} else {
smmu->page1 = smmu->base;
}
@@ -4106,12 +4127,12 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
/* Probe the h/w */
ret = arm_smmu_device_hw_probe(smmu);
if (ret)
- return ret;
+ goto out;
/* Initialise in-memory data structures */
ret = arm_smmu_init_structures(smmu);
if (ret)
- return ret;
+ goto out;
/* Record our private device structure */
platform_set_drvdata(pdev, smmu);
@@ -4122,28 +4143,35 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
/* Reset the device */
ret = arm_smmu_device_reset(smmu);
if (ret)
- return ret;
+ goto out;
/* And we're up. Go go go! */
ret = iommu_device_sysfs_add(&smmu->iommu, dev, NULL,
"smmu3.%pa", &ioaddr);
if (ret)
- return ret;
+ goto out;
ret = iommu_device_register(&smmu->iommu, &arm_smmu_ops, dev);
if (ret) {
dev_err(dev, "Failed to register iommu\n");
iommu_device_sysfs_remove(&smmu->iommu);
- return ret;
+ goto out;
}
return 0;
+
+out:
+ if (arm_smmu_has_tegra241_cmdqv(smmu))
+ tegra241_cmdqv_device_remove(smmu);
+ return ret;
}
static void arm_smmu_device_remove(struct platform_device *pdev)
{
struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
+ if (arm_smmu_has_tegra241_cmdqv(smmu))
+ tegra241_cmdqv_device_remove(smmu);
iommu_device_unregister(&smmu->iommu);
iommu_device_sysfs_remove(&smmu->iommu);
arm_smmu_device_disable(smmu);
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 01227c0de290..604e26a292e7 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -14,6 +14,9 @@
#include <linux/mmzone.h>
#include <linux/sizes.h>
+struct acpi_iort_node;
+struct tegra241_cmdqv;
+
/* MMIO registers */
#define ARM_SMMU_IDR0 0x0
#define IDR0_ST_LVL GENMASK(28, 27)
@@ -685,6 +688,12 @@ struct arm_smmu_device {
struct rb_root streams;
struct mutex streams_mutex;
+
+ /*
+ * Pointer to NVIDIA Tegra241 CMDQ-Virtualization Extension support,
+ * similar to v3.3 ECMDQ except with virtualization capabilities.
+ */
+ struct tegra241_cmdqv *tegra241_cmdqv;
};
struct arm_smmu_stream {
@@ -859,4 +868,45 @@ static inline void arm_smmu_sva_remove_dev_pasid(struct iommu_domain *domain,
{
}
#endif /* CONFIG_ARM_SMMU_V3_SVA */
+
+#ifdef CONFIG_TEGRA241_CMDQV
+static inline bool arm_smmu_has_tegra241_cmdqv(struct arm_smmu_device *smmu)
+{
+ return !!smmu->tegra241_cmdqv;
+}
+
+struct tegra241_cmdqv *tegra241_cmdqv_acpi_probe(struct arm_smmu_device *smmu,
+ struct acpi_iort_node *node);
+void tegra241_cmdqv_device_remove(struct arm_smmu_device *smmu);
+int tegra241_cmdqv_device_reset(struct arm_smmu_device *smmu);
+struct arm_smmu_cmdq *tegra241_cmdqv_get_cmdq(struct arm_smmu_device *smmu);
+#else /* CONFIG_TEGRA241_CMDQV */
+static inline bool arm_smmu_has_tegra241_cmdqv(struct arm_smmu_device *smmu)
+{
+ return false;
+}
+
+static inline struct tegra241_cmdqv *
+tegra241_cmdqv_acpi_probe(struct arm_smmu_device *smmu,
+ struct acpi_iort_node *node)
+{
+ return NULL;
+}
+
+static inline void tegra241_cmdqv_device_remove(struct arm_smmu_device *smmu)
+{
+}
+
+static inline int tegra241_cmdqv_device_reset(struct arm_smmu_device *smmu)
+{
+ return -ENODEV;
+}
+
+static inline struct arm_smmu_cmdq *
+tegra241_cmdqv_get_cmdq(struct arm_smmu_device *smmu)
+{
+ return NULL;
+}
+#endif /* CONFIG_TEGRA241_CMDQV */
+
#endif /* _ARM_SMMU_V3_H */
diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
new file mode 100644
index 000000000000..ec4767e3859e
--- /dev/null
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -0,0 +1,877 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (C) 2021-2024 NVIDIA CORPORATION & AFFILIATES. */
+
+#define dev_fmt(fmt) "tegra241_cmdqv: " fmt
+
+#include <linux/acpi.h>
+#include <linux/debugfs.h>
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+#include <linux/iommu.h>
+#include <linux/iopoll.h>
+
+#include <acpi/acpixf.h>
+
+#include "arm-smmu-v3.h"
+
+#define TEGRA241_CMDQV_HID "NVDA200C"
+
+/* CMDQV register page base and size defines */
+#define TEGRA241_CMDQV_CONFIG_BASE (0)
+#define TEGRA241_CMDQV_CONFIG_SIZE (SZ_64K)
+#define TEGRA241_VCMDQ_PAGE0_BASE (TEGRA241_CMDQV_CONFIG_BASE + SZ_64K)
+#define TEGRA241_VCMDQ_PAGE1_BASE (TEGRA241_VCMDQ_PAGE0_BASE + SZ_64K)
+#define TEGRA241_VINTF_PAGE_BASE (TEGRA241_VCMDQ_PAGE1_BASE + SZ_64K)
+
+/* CMDQV global base regs */
+#define TEGRA241_CMDQV_CONFIG 0x0000
+#define CMDQV_EN BIT(0)
+
+#define TEGRA241_CMDQV_PARAM 0x0004
+#define CMDQV_NUM_VINTF_LOG2 GENMASK(11, 8)
+#define CMDQV_NUM_VCMDQ_LOG2 GENMASK(7, 4)
+
+#define TEGRA241_CMDQV_STATUS 0x0008
+#define CMDQV_ENABLED BIT(0)
+
+#define TEGRA241_CMDQV_VINTF_ERR_MAP 0x0014
+#define TEGRA241_CMDQV_VINTF_INT_MASK 0x001C
+#define TEGRA241_CMDQV_CMDQ_ERR_MAP_64(m) \
+ (0x0024 + 0x8*(m))
+
+#define TEGRA241_CMDQV_CMDQ_ALLOC(q) (0x0200 + 0x4*(q))
+#define CMDQV_CMDQ_ALLOC_VINTF GENMASK(20, 15)
+#define CMDQV_CMDQ_ALLOC_LVCMDQ GENMASK(7, 1)
+#define CMDQV_CMDQ_ALLOCATED BIT(0)
+
+/* VINTF base regs */
+#define TEGRA241_VINTF(v) (0x1000 + 0x100*(v))
+
+#define TEGRA241_VINTF_CONFIG 0x0000
+#define VINTF_HYP_OWN BIT(17)
+#define VINTF_VMID GENMASK(16, 1)
+#define VINTF_EN BIT(0)
+
+#define TEGRA241_VINTF_STATUS 0x0004
+#define VINTF_STATUS GENMASK(3, 1)
+#define VINTF_ENABLED BIT(0)
+
+#define TEGRA241_VINTF_LVCMDQ_ERR_MAP_64(m) \
+ (0x00C0 + 0x8*(m))
+#define LVCMDQ_ERR_MAP_NUM_64 2
+
+/* VCMDQ base regs */
+/* -- PAGE0 -- */
+#define TEGRA241_VCMDQ_PAGE0(q) (TEGRA241_VCMDQ_PAGE0_BASE + 0x80*(q))
+
+#define TEGRA241_VCMDQ_CONS 0x00000
+#define VCMDQ_CONS_ERR GENMASK(30, 24)
+
+#define TEGRA241_VCMDQ_PROD 0x00004
+
+#define TEGRA241_VCMDQ_CONFIG 0x00008
+#define VCMDQ_EN BIT(0)
+
+#define TEGRA241_VCMDQ_STATUS 0x0000C
+#define VCMDQ_ENABLED BIT(0)
+
+#define TEGRA241_VCMDQ_GERROR 0x00010
+#define TEGRA241_VCMDQ_GERRORN 0x00014
+
+/* -- PAGE1 -- */
+#define TEGRA241_VCMDQ_PAGE1(q) (TEGRA241_VCMDQ_PAGE1_BASE + 0x80*(q))
+#define VCMDQ_ADDR GENMASK(47, 5)
+#define VCMDQ_LOG2SIZE GENMASK(4, 0)
+
+#define TEGRA241_VCMDQ_BASE 0x00000
+#define TEGRA241_VCMDQ_CONS_INDX_BASE 0x00008
+
+/* VINTF logical-VCMDQ pages */
+#define TEGRA241_VINTFi_PAGE0(i) (TEGRA241_VINTF_PAGE_BASE + SZ_128K*(i))
+#define TEGRA241_VINTFi_PAGE1(i) (TEGRA241_VINTFi_PAGE0(i) + SZ_64K)
+#define TEGRA241_VINTFi_LVCMDQ_PAGE0(i, q) \
+ (TEGRA241_VINTFi_PAGE0(i) + 0x80*(q))
+#define TEGRA241_VINTFi_LVCMDQ_PAGE1(i, q) \
+ (TEGRA241_VINTFi_PAGE1(i) + 0x80*(q))
+
+/* MMIO helpers */
+#define cmdqv_readl(_cmdqv, _regname) \
+ readl((_cmdqv)->base + TEGRA241_CMDQV_##_regname)
+#define cmdqv_readl_relaxed(_cmdqv, _regname) \
+ readl_relaxed((_cmdqv)->base + TEGRA241_CMDQV_##_regname)
+#define cmdqv_readq_relaxed(_cmdqv, _regname) \
+ readq_relaxed((_cmdqv)->base + TEGRA241_CMDQV_##_regname)
+#define cmdqv_writel(_cmdqv, val, _regname) \
+ writel((val), (_cmdqv)->base + TEGRA241_CMDQV_##_regname)
+#define cmdqv_writel_relaxed(_cmdqv, val, _regname) \
+ writel_relaxed((val), (_cmdqv)->base + TEGRA241_CMDQV_##_regname)
+
+#define vintf_readl(_vintf, _regname) \
+ readl((_vintf)->base + TEGRA241_VINTF_##_regname)
+#define vintf_readq_relaxed(_vintf, _regname) \
+ readq_relaxed((_vintf)->base + TEGRA241_VINTF_##_regname)
+#define vintf_readl_relaxed(_vintf, _regname) \
+ readl_relaxed((_vintf)->base + TEGRA241_VINTF_##_regname)
+#define vintf_writel(_vintf, val, _regname) \
+ writel((val), (_vintf)->base + TEGRA241_VINTF_##_regname)
+#define vintf_writel_relaxed(_vintf, val, _regname) \
+ writel_relaxed((val), (_vintf)->base + TEGRA241_VINTF_##_regname)
+
+#define vcmdq_page0_readl(_vcmdq, _regname) \
+ readl((_vcmdq)->page0 + TEGRA241_VCMDQ_##_regname)
+#define vcmdq_page0_readl_relaxed(_vcmdq, _regname) \
+ readl_relaxed((_vcmdq)->page0 + TEGRA241_VCMDQ_##_regname)
+#define vcmdq_page0_writel(_vcmdq, val, _regname) \
+ writel((val), (_vcmdq)->page0 + TEGRA241_VCMDQ_##_regname)
+#define vcmdq_page0_writel_relaxed(_vcmdq, val, _regname) \
+ writel_relaxed((val), (_vcmdq)->page0 + TEGRA241_VCMDQ_##_regname)
+
+#define vcmdq_page1_readl(_vcmdq, reg) \
+ readl((_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
+#define vcmdq_page1_readl_relaxed(_vcmdq, reg) \
+ readl_relaxed((_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
+#define vcmdq_page1_readq_relaxed(_vcmdq, reg) \
+ readq_relaxed((_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
+#define vcmdq_page1_writel(_vcmdq, val, reg) \
+ writel((val), (_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
+#define vcmdq_page1_writel_relaxed(_vcmdq, val, reg) \
+ writel_relaxed((val), (_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
+#define vcmdq_page1_writeq(_vcmdq, val, reg) \
+ writeq((val), (_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
+#define vcmdq_page1_writeq_relaxed(_vcmdq, val, reg) \
+ writeq_relaxed((val), (_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
+
+
+static bool disable_cmdqv;
+module_param(disable_cmdqv, bool, 0444);
+MODULE_PARM_DESC(disable_cmdqv,
+ "This allows to disable CMDQV HW and use default SMMU internal CMDQ.");
+
+static bool bypass_vcmdq;
+module_param(bypass_vcmdq, bool, 0444);
+MODULE_PARM_DESC(bypass_vcmdq,
+ "This allows to bypass VCMDQ for debugging use or perf comparison.");
+
+/**
+ * struct tegra241_vcmdq - Virtual Command Queue
+ * @idx: Global index in the CMDQV
+ * @lidx: Logical index in the VINTF
+ * @enabled: Enable status
+ * @cmdqv: Parent CMDQV pointer
+ * @vintf: Parent VINTF pointer
+ * @cmdq: Command Queue struct
+ * @page0: MMIO Page0 base address
+ * @page1: MMIO Page1 base address
+ */
+struct tegra241_vcmdq {
+ u16 idx;
+ u16 lidx;
+
+ bool enabled;
+
+ struct tegra241_cmdqv *cmdqv;
+ struct tegra241_vintf *vintf;
+ struct arm_smmu_cmdq cmdq;
+
+ void __iomem *page0;
+ void __iomem *page1;
+};
+
+/**
+ * struct tegra241_vintf - Virtual Interface
+ * @idx: Global index in the CMDQV
+ * @enabled: Enable status
+ * @cmdqv: Parent CMDQV pointer
+ * @lvcmdqs: List of logical VCMDQ pointers
+ * @base: MMIO base address
+ */
+struct tegra241_vintf {
+ u16 idx;
+
+ bool enabled;
+
+ struct tegra241_cmdqv *cmdqv;
+ struct tegra241_vcmdq **lvcmdqs;
+
+ void __iomem *base;
+};
+
+/**
+ * struct tegra241_cmdqv - CMDQ-V for SMMUv3
+ * @smmu: Parent SMMUv3 pointer
+ * @base: MMIO base address
+ * @irq: IRQ number
+ * @num_vintfs: Total number of VINTFs
+ * @num_vcmdqs: Total number of VCMDQs
+ * @num_lvcmdqs_per_vintf: Number of logical VCMDQs per VINTF
+ * @vintf_ids: VINTF id allocator
+ * @vtinfs: List of VINTFs
+ */
+struct tegra241_cmdqv {
+ struct arm_smmu_device *smmu;
+
+ void __iomem *base;
+ int irq;
+
+ /* CMDQV Hardware Params */
+ u16 num_vintfs;
+ u16 num_vcmdqs;
+ u16 num_lvcmdqs_per_vintf;
+
+ struct ida vintf_ids;
+
+ struct tegra241_vintf **vintfs;
+};
+
+/* Config and Polling Helpers */
+
+static inline int tegra241_cmdqv_write_config(struct tegra241_cmdqv *cmdqv,
+ void __iomem *addr_config,
+ void __iomem *addr_status,
+ u32 regval, const char *header,
+ bool *out_enabled)
+{
+ bool en = regval & BIT(0);
+ int ret;
+
+ writel(regval, addr_config);
+ ret = readl_poll_timeout(addr_status, regval,
+ en ? regval & BIT(0) : !(regval & BIT(0)),
+ 1, ARM_SMMU_POLL_TIMEOUT_US);
+ if (ret)
+ dev_err(cmdqv->smmu->dev, "%sfailed to %sable, STATUS=0x%08X\n",
+ header, en ? "en" : "dis", regval);
+ if (out_enabled)
+ WRITE_ONCE(*out_enabled, regval & BIT(0));
+ return ret;
+}
+
+static inline int cmdqv_write_config(struct tegra241_cmdqv *cmdqv, u32 regval)
+{
+ return tegra241_cmdqv_write_config(cmdqv,
+ cmdqv->base + TEGRA241_CMDQV_CONFIG,
+ cmdqv->base + TEGRA241_CMDQV_STATUS,
+ regval, "CMDQV: ", NULL);
+}
+
+static inline int vintf_write_config(struct tegra241_vintf *vintf, u32 regval)
+{
+ char header[16];
+
+ snprintf(header, 16, "VINTF%u: ", vintf->idx);
+ return tegra241_cmdqv_write_config(vintf->cmdqv,
+ vintf->base + TEGRA241_VINTF_CONFIG,
+ vintf->base + TEGRA241_VINTF_STATUS,
+ regval, header, &vintf->enabled);
+}
+
+static inline const char *lvcmdq_error_header(struct tegra241_vcmdq *vcmdq)
+{
+ static char header[32];
+
+ if (WARN_ON(!vcmdq->vintf))
+ return "";
+ snprintf(header, 32, "VINTF%u: VCMDQ%u/LVCMDQ%u: ",
+ vcmdq->vintf->idx, vcmdq->idx, vcmdq->lidx);
+ return header;
+}
+
+static inline int vcmdq_write_config(struct tegra241_vcmdq *vcmdq, u32 regval)
+{
+ return tegra241_cmdqv_write_config(vcmdq->cmdqv,
+ vcmdq->page0 + TEGRA241_VCMDQ_CONFIG,
+ vcmdq->page0 + TEGRA241_VCMDQ_STATUS,
+ regval, lvcmdq_error_header(vcmdq),
+ &vcmdq->enabled);
+}
+
+/* ISR Functions */
+
+static void tegra241_vintf0_handle_error(struct tegra241_vintf *vintf)
+{
+ int i;
+
+ for (i = 0; i < LVCMDQ_ERR_MAP_NUM_64; i++) {
+ u64 lmap = vintf_readq_relaxed(vintf, LVCMDQ_ERR_MAP_64(i));
+
+ while (lmap) {
+ unsigned long lidx = __ffs64(lmap) - 1;
+ struct tegra241_vcmdq *vcmdq = vintf->lvcmdqs[lidx];
+ u32 gerror = vcmdq_page0_readl_relaxed(vcmdq, GERROR);
+
+ __arm_smmu_cmdq_skip_err(vintf->cmdqv->smmu,
+ &vcmdq->cmdq.q);
+ vcmdq_page0_writel(vcmdq, gerror, GERRORN);
+ lmap &= ~BIT_ULL(lidx);
+ }
+ }
+}
+
+static irqreturn_t tegra241_cmdqv_isr(int irq, void *devid)
+{
+ struct tegra241_cmdqv *cmdqv = (struct tegra241_cmdqv *)devid;
+ u64 vintf_map = cmdqv_readq_relaxed(cmdqv, VINTF_ERR_MAP);
+
+ dev_warn(cmdqv->smmu->dev,
+ "unexpected error reported. vintf_map: %016llx, vcmdq_map: %016llx%016llx\n",
+ vintf_map, cmdqv_readq_relaxed(cmdqv, CMDQ_ERR_MAP_64(1)),
+ cmdqv_readq_relaxed(cmdqv, CMDQ_ERR_MAP_64(0)));
+
+ /* Handle VINTF0 and its LVCMDQs */
+ if (vintf_map & BIT_ULL(0))
+ tegra241_vintf0_handle_error(cmdqv->vintfs[0]);
+
+ return IRQ_HANDLED;
+}
+
+/* Command Queue Selecting Function */
+
+struct arm_smmu_cmdq *tegra241_cmdqv_get_cmdq(struct arm_smmu_device *smmu)
+{
+ struct tegra241_cmdqv *cmdqv = smmu->tegra241_cmdqv;
+ struct tegra241_vintf *vintf = cmdqv->vintfs[0];
+ struct tegra241_vcmdq *vcmdq;
+ u16 lidx;
+
+ if (READ_ONCE(bypass_vcmdq))
+ return &smmu->cmdq;
+
+ /* Use SMMU CMDQ if VINTF0 is uninitialized */
+ if (!READ_ONCE(vintf->enabled))
+ return &smmu->cmdq;
+
+ /*
+ * Select a LVCMDQ to use. Here we use a temporal solution to
+ * balance out traffic on cmdq issuing: each cmdq has its own
+ * lock, if all cpus issue cmdlist using the same cmdq, only
+ * one CPU at a time can enter the process, while the others
+ * will be spinning at the same lock.
+ */
+ lidx = smp_processor_id() % cmdqv->num_lvcmdqs_per_vintf;
+ vcmdq = vintf->lvcmdqs[lidx];
+ if (!vcmdq || !READ_ONCE(vcmdq->enabled))
+ return &smmu->cmdq;
+ return &vcmdq->cmdq;
+}
+
+/* Device Reset (HW init/deinit) Functions */
+
+static void tegra241_vcmdq_hw_deinit(struct tegra241_vcmdq *vcmdq)
+{
+ u32 gerrorn, gerror;
+
+ if (vcmdq_write_config(vcmdq, 0)) {
+ dev_err(vcmdq->cmdqv->smmu->dev,
+ "%sGERRORN=0x%X, GERROR=0x%X, CONS=0x%X\n",
+ lvcmdq_error_header(vcmdq),
+ vcmdq_page0_readl_relaxed(vcmdq, GERRORN),
+ vcmdq_page0_readl_relaxed(vcmdq, GERROR),
+ vcmdq_page0_readl_relaxed(vcmdq, CONS));
+ }
+ vcmdq_page0_writel_relaxed(vcmdq, 0, PROD);
+ vcmdq_page0_writel_relaxed(vcmdq, 0, CONS);
+ vcmdq_page1_writeq_relaxed(vcmdq, 0, BASE);
+ vcmdq_page1_writeq_relaxed(vcmdq, 0, CONS_INDX_BASE);
+
+ gerrorn = vcmdq_page0_readl_relaxed(vcmdq, GERRORN);
+ gerror = vcmdq_page0_readl_relaxed(vcmdq, GERROR);
+ if (gerror != gerrorn) {
+ dev_warn(vcmdq->cmdqv->smmu->dev,
+ "%suncleared error detected, resetting\n",
+ lvcmdq_error_header(vcmdq));
+ vcmdq_page0_writel(vcmdq, gerror, GERRORN);
+ }
+
+ dev_dbg(vcmdq->cmdqv->smmu->dev,
+ "%sdeinited\n", lvcmdq_error_header(vcmdq));
+}
+
+static int tegra241_vcmdq_hw_init(struct tegra241_vcmdq *vcmdq)
+{
+ int ret;
+
+ /* Reset VCMDQ */
+ tegra241_vcmdq_hw_deinit(vcmdq);
+
+ /* Configure and enable VCMDQ */
+ vcmdq_page1_writeq_relaxed(vcmdq, vcmdq->cmdq.q.q_base, BASE);
+
+ ret = vcmdq_write_config(vcmdq, VCMDQ_EN);
+ if (ret) {
+ dev_err(vcmdq->cmdqv->smmu->dev,
+ "%sGERRORN=0x%X, GERROR=0x%X, CONS=0x%X\n",
+ lvcmdq_error_header(vcmdq),
+ vcmdq_page0_readl_relaxed(vcmdq, GERRORN),
+ vcmdq_page0_readl_relaxed(vcmdq, GERROR),
+ vcmdq_page0_readl_relaxed(vcmdq, CONS));
+ return ret;
+ }
+
+ dev_dbg(vcmdq->cmdqv->smmu->dev,
+ "%sinited\n", lvcmdq_error_header(vcmdq));
+ return 0;
+}
+
+static void tegra241_vintf_hw_deinit(struct tegra241_vintf *vintf)
+{
+ u16 lidx;
+
+ for (lidx = 0; lidx < vintf->cmdqv->num_lvcmdqs_per_vintf; lidx++)
+ if (vintf->lvcmdqs && vintf->lvcmdqs[lidx])
+ tegra241_vcmdq_hw_deinit(vintf->lvcmdqs[lidx]);
+ vintf_write_config(vintf, 0);
+}
+
+static int tegra241_vintf_hw_init(struct tegra241_vintf *vintf, bool hyp_own)
+{
+ u32 regval;
+ u16 lidx;
+ int ret;
+
+ /* Reset VINTF */
+ tegra241_vintf_hw_deinit(vintf);
+
+ /* Configure and enable VINTF */
+ regval = FIELD_PREP(VINTF_HYP_OWN, hyp_own);
+ vintf_writel(vintf, regval, CONFIG);
+
+ ret = vintf_write_config(vintf, regval | VINTF_EN);
+ if (ret)
+ return ret;
+
+ for (lidx = 0; lidx < vintf->cmdqv->num_lvcmdqs_per_vintf; lidx++) {
+ if (vintf->lvcmdqs && vintf->lvcmdqs[lidx]) {
+ ret = tegra241_vcmdq_hw_init(vintf->lvcmdqs[lidx]);
+ if (ret) {
+ tegra241_vintf_hw_deinit(vintf);
+ return ret;
+ }
+ }
+ }
+
+ return 0;
+}
+
+int tegra241_cmdqv_device_reset(struct arm_smmu_device *smmu)
+{
+ struct tegra241_cmdqv *cmdqv = smmu->tegra241_cmdqv;
+ u16 qidx, lidx, idx;
+ u32 regval;
+ int ret;
+
+ /* Reset CMDQV */
+ regval = cmdqv_readl_relaxed(cmdqv, CONFIG);
+ ret = cmdqv_write_config(cmdqv, regval & ~CMDQV_EN);
+ if (ret)
+ return ret;
+ ret = cmdqv_write_config(cmdqv, regval | CMDQV_EN);
+ if (ret)
+ return ret;
+
+ /* Assign preallocated global VCMDQs to each VINTF as LVCMDQs */
+ for (idx = 0, qidx = 0; idx < cmdqv->num_vintfs; idx++) {
+ for (lidx = 0; lidx < cmdqv->num_lvcmdqs_per_vintf; lidx++) {
+ regval = FIELD_PREP(CMDQV_CMDQ_ALLOC_VINTF, idx);
+ regval |= FIELD_PREP(CMDQV_CMDQ_ALLOC_LVCMDQ, lidx);
+ regval |= CMDQV_CMDQ_ALLOCATED;
+ cmdqv_writel_relaxed(cmdqv, regval, CMDQ_ALLOC(qidx++));
+ }
+ }
+
+ return tegra241_vintf_hw_init(cmdqv->vintfs[0], true);
+}
+
+/* Probe Functions */
+
+static int tegra241_cmdqv_acpi_is_memory(struct acpi_resource *res, void *data)
+{
+ struct resource_win win;
+
+ return !acpi_dev_resource_address_space(res, &win);
+}
+
+static int tegra241_cmdqv_acpi_get_irqs(struct acpi_resource *ares, void *data)
+{
+ struct resource r;
+ int *irq = data;
+
+ if (*irq <= 0 && acpi_dev_resource_interrupt(ares, 0, &r))
+ *irq = r.start;
+ return 1; /* No need to add resource to the list */
+}
+
+static struct tegra241_cmdqv *
+tegra241_cmdqv_find_resource(struct arm_smmu_device *smmu,
+ struct acpi_iort_node *node)
+{
+ struct tegra241_cmdqv *cmdqv = NULL;
+ struct device *dev = smmu->dev;
+ struct list_head resource_list;
+ struct resource_entry *rentry;
+ struct acpi_device *adev;
+ const char *match_uid;
+ int ret;
+
+ if (acpi_disabled)
+ return NULL;
+
+ /* Look for a device in the DSDT whose _UID matches the SMMU node ID */
+ match_uid = kasprintf(GFP_KERNEL, "%u", node->identifier);
+ adev = acpi_dev_get_first_match_dev(TEGRA241_CMDQV_HID, match_uid, -1);
+ kfree(match_uid);
+
+ if (!adev)
+ return NULL;
+
+ dev_info(dev, "found companion CMDQV device, %s\n",
+ dev_name(&adev->dev));
+
+ INIT_LIST_HEAD(&resource_list);
+ ret = acpi_dev_get_resources(adev, &resource_list,
+ tegra241_cmdqv_acpi_is_memory, NULL);
+ if (ret < 0) {
+ dev_err(dev, "failed to get memory resource: %d\n", ret);
+ goto put_dev;
+ }
+
+ cmdqv = kzalloc(sizeof(*cmdqv), GFP_KERNEL);
+ if (!cmdqv)
+ goto free_list;
+
+ cmdqv->smmu = smmu;
+
+ rentry = list_first_entry_or_null(&resource_list,
+ struct resource_entry, node);
+ if (!rentry) {
+ dev_err(dev, "failed to get memory resource entry\n");
+ goto free_cmdqv;
+ }
+
+ cmdqv->base = ioremap(rentry->res->start, resource_size(rentry->res));
+ if (IS_ERR(cmdqv->base)) {
+ dev_err(dev, "failed to ioremap: %ld\n", PTR_ERR(cmdqv->base));
+ goto free_cmdqv;
+ }
+
+ acpi_dev_free_resource_list(&resource_list);
+
+ INIT_LIST_HEAD(&resource_list);
+
+ ret = acpi_dev_get_resources(adev, &resource_list,
+ tegra241_cmdqv_acpi_get_irqs, &cmdqv->irq);
+ if (ret < 0 || cmdqv->irq <= 0) {
+ dev_warn(dev, "no interrupt. errors will not be reported\n");
+ } else {
+ ret = request_irq(cmdqv->irq, tegra241_cmdqv_isr, 0,
+ "tegra241-cmdqv", cmdqv);
+ if (ret) {
+ dev_err(dev, "failed to request irq (%d): %d\n",
+ cmdqv->irq, ret);
+ goto iounmap;
+ }
+ }
+
+ goto free_list;
+
+iounmap:
+ iounmap(cmdqv->base);
+free_cmdqv:
+ kfree(cmdqv);
+ cmdqv = NULL;
+free_list:
+ acpi_dev_free_resource_list(&resource_list);
+put_dev:
+ put_device(&adev->dev);
+
+ return cmdqv;
+}
+
+static int tegra241_vcmdq_alloc_smmu_cmdq(struct tegra241_vcmdq *vcmdq)
+{
+ struct arm_smmu_device *smmu = vcmdq->cmdqv->smmu;
+ struct arm_smmu_cmdq *cmdq = &vcmdq->cmdq;
+ struct arm_smmu_queue *q = &cmdq->q;
+ char name[16];
+ int ret;
+
+ snprintf(name, 16, "vcmdq%u", vcmdq->idx);
+
+ q->llq.max_n_shift = ilog2(SZ_64K >> CMDQ_ENT_SZ_SHIFT);
+
+ /* Use the common helper to init the VCMDQ, and then... */
+ ret = arm_smmu_init_one_queue(smmu, q, vcmdq->page0,
+ TEGRA241_VCMDQ_PROD, TEGRA241_VCMDQ_CONS,
+ CMDQ_ENT_DWORDS, name);
+ if (ret)
+ return ret;
+
+ /* ...override q_base to write VCMDQ_BASE registers */
+ q->q_base = q->base_dma & VCMDQ_ADDR;
+ q->q_base |= FIELD_PREP(VCMDQ_LOG2SIZE, q->llq.max_n_shift);
+
+ /* All VCMDQs support CS_NONE only for CMD_SYNC */
+ q->quirks = CMDQ_QUIRK_SYNC_CS_NONE_ONLY;
+
+ return arm_smmu_cmdq_init(smmu, cmdq);
+}
+
+static void tegra241_vcmdq_free_smmu_cmdq(struct tegra241_vcmdq *vcmdq)
+{
+ struct arm_smmu_queue *q = &vcmdq->cmdq.q;
+ size_t nents = 1 << q->llq.max_n_shift;
+ size_t qsz = nents << CMDQ_ENT_SZ_SHIFT;
+
+ if (!q->base)
+ return;
+ dmam_free_coherent(vcmdq->cmdqv->smmu->dev, qsz, q->base, q->base_dma);
+}
+
+static int tegra241_vintf_init_lvcmdq(struct tegra241_vintf *vintf, u16 lidx,
+ struct tegra241_vcmdq *vcmdq)
+{
+ struct tegra241_cmdqv *cmdqv = vintf->cmdqv;
+ u16 idx = vintf->idx;
+
+ vcmdq->idx = idx * cmdqv->num_lvcmdqs_per_vintf + lidx;
+ vcmdq->lidx = lidx;
+ vcmdq->cmdqv = cmdqv;
+ vcmdq->vintf = vintf;
+ vcmdq->page0 = cmdqv->base + TEGRA241_VINTFi_LVCMDQ_PAGE0(idx, lidx);
+ vcmdq->page1 = cmdqv->base + TEGRA241_VINTFi_LVCMDQ_PAGE1(idx, lidx);
+
+ vintf->lvcmdqs[lidx] = vcmdq;
+ return 0;
+}
+
+static void tegra241_vintf_deinit_lvcmdq(struct tegra241_vintf *vintf, u16 lidx)
+{
+ vintf->lvcmdqs[lidx] = NULL;
+}
+
+static struct tegra241_vcmdq *
+tegra241_vintf0_alloc_lvcmdq(struct tegra241_vintf *vintf, u16 lidx)
+{
+ struct tegra241_cmdqv *cmdqv = vintf->cmdqv;
+ struct tegra241_vcmdq *vcmdq;
+ int ret;
+
+ vcmdq = kzalloc(sizeof(*vcmdq), GFP_KERNEL);
+ if (!vcmdq)
+ return ERR_PTR(-ENOMEM);
+
+ ret = tegra241_vintf_init_lvcmdq(vintf, lidx, vcmdq);
+ if (ret)
+ goto free_vcmdq;
+
+ /* Build an arm_smmu_cmdq for each LVCMDQ */
+ ret = tegra241_vcmdq_alloc_smmu_cmdq(vcmdq);
+ if (ret)
+ goto deinit_lvcmdq;
+
+ dev_dbg(cmdqv->smmu->dev, "%sallocated\n", lvcmdq_error_header(vcmdq));
+ return vcmdq;
+
+deinit_lvcmdq:
+ tegra241_vintf_deinit_lvcmdq(vintf, lidx);
+free_vcmdq:
+ kfree(vcmdq);
+ return ERR_PTR(ret);
+}
+
+static void tegra241_vintf_free_lvcmdq(struct tegra241_vintf *vintf, u16 lidx)
+{
+ struct tegra241_vcmdq *vcmdq = vintf->lvcmdqs[lidx];
+
+ tegra241_vcmdq_free_smmu_cmdq(vcmdq);
+ tegra241_vintf_deinit_lvcmdq(vintf, lidx);
+
+ dev_dbg(vintf->cmdqv->smmu->dev,
+ "%sdeallocated\n", lvcmdq_error_header(vcmdq));
+ kfree(vcmdq);
+}
+
+static int tegra241_cmdqv_init_vintf(struct tegra241_cmdqv *cmdqv, u16 max_idx,
+ struct tegra241_vintf *vintf)
+{
+
+ u16 idx;
+ int ret;
+
+ ret = ida_alloc_max(&cmdqv->vintf_ids, max_idx, GFP_KERNEL);
+ if (ret < 0)
+ return ret;
+ idx = ret;
+
+ vintf->idx = idx;
+ vintf->cmdqv = cmdqv;
+ vintf->base = cmdqv->base + TEGRA241_VINTF(idx);
+
+ vintf->lvcmdqs = kcalloc(cmdqv->num_lvcmdqs_per_vintf,
+ sizeof(*vintf->lvcmdqs), GFP_KERNEL);
+ if (!vintf->lvcmdqs) {
+ ida_free(&cmdqv->vintf_ids, idx);
+ return -ENOMEM;
+ }
+
+ cmdqv->vintfs[idx] = vintf;
+ return 0;
+}
+
+static void tegra241_cmdqv_deinit_vintf(struct tegra241_cmdqv *cmdqv, u16 idx)
+{
+ kfree(cmdqv->vintfs[idx]->lvcmdqs);
+ ida_free(&cmdqv->vintf_ids, idx);
+ cmdqv->vintfs[idx] = NULL;
+}
+
+struct dentry *cmdqv_debugfs_dir;
+
+static int tegra241_cmdqv_probe(struct tegra241_cmdqv *cmdqv)
+{
+ struct tegra241_vintf *vintf;
+ u32 regval;
+ u16 lidx;
+ int ret;
+
+ regval = cmdqv_readl(cmdqv, CONFIG);
+ if (disable_cmdqv) {
+ dev_info(cmdqv->smmu->dev,
+ "disable_cmdqv=true. Falling back to SMMU CMDQ\n");
+ cmdqv_write_config(cmdqv, regval & ~CMDQV_EN);
+ return -ENODEV;
+ }
+
+ ret = cmdqv_write_config(cmdqv, regval | CMDQV_EN);
+ if (ret)
+ return ret;
+
+ regval = cmdqv_readl_relaxed(cmdqv, PARAM);
+ cmdqv->num_vintfs = 1 << FIELD_GET(CMDQV_NUM_VINTF_LOG2, regval);
+ cmdqv->num_vcmdqs = 1 << FIELD_GET(CMDQV_NUM_VCMDQ_LOG2, regval);
+ cmdqv->num_lvcmdqs_per_vintf = cmdqv->num_vcmdqs / cmdqv->num_vintfs;
+
+ cmdqv->vintfs = kcalloc(cmdqv->num_vintfs,
+ sizeof(*cmdqv->vintfs), GFP_KERNEL);
+ if (!cmdqv->vintfs)
+ return -ENOMEM;
+ ida_init(&cmdqv->vintf_ids);
+
+ vintf = kzalloc(sizeof(*vintf), GFP_KERNEL);
+ if (!vintf) {
+ ret = -ENOMEM;
+ goto destroy_ids;
+ }
+
+ /* Init VINTF0 for in-kernel use */
+ ret = tegra241_cmdqv_init_vintf(cmdqv, 0, vintf);
+ if (ret) {
+ dev_err(cmdqv->smmu->dev, "failed to init vintf0: %d\n", ret);
+ goto free_vintf;
+ }
+
+ /* Preallocate logical VCMDQs to VINTF0 */
+ for (lidx = 0; lidx < cmdqv->num_lvcmdqs_per_vintf; lidx++) {
+ struct tegra241_vcmdq *vcmdq;
+
+ vcmdq = tegra241_vintf0_alloc_lvcmdq(vintf, lidx);
+ if (IS_ERR(vcmdq)) {
+ ret = PTR_ERR(vcmdq);
+ goto free_lvcmdq;
+ }
+ }
+
+#ifdef CONFIG_IOMMU_DEBUGFS
+ if (!cmdqv_debugfs_dir) {
+ cmdqv_debugfs_dir =
+ debugfs_create_dir("tegra241_cmdqv", iommu_debugfs_dir);
+ debugfs_create_bool("bypass_vcmdq", 0644,
+ cmdqv_debugfs_dir, &bypass_vcmdq);
+ }
+#endif
+
+ return 0;
+
+free_lvcmdq:
+ for (lidx--; lidx >= 0; lidx--)
+ tegra241_vintf_free_lvcmdq(vintf, lidx);
+ tegra241_cmdqv_deinit_vintf(cmdqv, vintf->idx);
+free_vintf:
+ kfree(vintf);
+destroy_ids:
+ ida_destroy(&cmdqv->vintf_ids);
+ kfree(cmdqv->vintfs);
+ return ret;
+}
+
+struct tegra241_cmdqv *tegra241_cmdqv_acpi_probe(struct arm_smmu_device *smmu,
+ struct acpi_iort_node *node)
+{
+ struct tegra241_cmdqv *cmdqv;
+
+ cmdqv = tegra241_cmdqv_find_resource(smmu, node);
+ if (!cmdqv)
+ return NULL;
+
+ if (tegra241_cmdqv_probe(cmdqv)) {
+ if (cmdqv->irq > 0)
+ free_irq(cmdqv->irq, cmdqv);
+ iounmap(cmdqv->base);
+ kfree(cmdqv);
+ return NULL;
+ }
+
+ return cmdqv;
+}
+
+/* Remove Functions */
+
+static void tegra241_vintf_remove_lvcmdq(struct tegra241_vintf *vintf, u16 lidx)
+{
+ tegra241_vcmdq_hw_deinit(vintf->lvcmdqs[lidx]);
+ tegra241_vintf_free_lvcmdq(vintf, lidx);
+}
+
+static void tegra241_cmdqv_remove_vintf(struct tegra241_cmdqv *cmdqv, u16 idx)
+{
+ struct tegra241_vintf *vintf = cmdqv->vintfs[idx];
+ u16 lidx;
+
+ /* Remove LVCMDQ resources */
+ for (lidx = 0; lidx < vintf->cmdqv->num_lvcmdqs_per_vintf; lidx++)
+ if (vintf->lvcmdqs[lidx])
+ tegra241_vintf_remove_lvcmdq(vintf, lidx);
+
+ /* Remove VINTF resources */
+ tegra241_vintf_hw_deinit(vintf);
+ ida_free(&cmdqv->vintf_ids, vintf->idx);
+ cmdqv->vintfs[idx] = NULL;
+
+ dev_dbg(cmdqv->smmu->dev, "VINTF%u: deallocated\n", vintf->idx);
+ kfree(vintf->lvcmdqs);
+ kfree(vintf);
+}
+
+void tegra241_cmdqv_device_remove(struct arm_smmu_device *smmu)
+{
+ struct tegra241_cmdqv *cmdqv = smmu->tegra241_cmdqv;
+ u16 idx;
+
+ /* Remove VINTF resources */
+ for (idx = 0; idx < cmdqv->num_vintfs; idx++) {
+ if (cmdqv->vintfs[idx]) {
+ /* Only vintf0 should remain at this stage */
+ WARN_ON(idx > 0);
+ tegra241_cmdqv_remove_vintf(cmdqv, idx);
+ }
+ }
+
+ /* Remove cmdqv resources */
+ ida_destroy(&cmdqv->vintf_ids);
+ smmu->tegra241_cmdqv = NULL;
+
+ if (cmdqv->irq > 0)
+ free_irq(cmdqv->irq, cmdqv);
+ iounmap(cmdqv->base);
+ kfree(cmdqv->vintfs);
+ kfree(cmdqv);
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 6/6] iommu/tegra241-cmdqv: Limit CMDs for guest owned VINTF
2024-05-08 5:56 [PATCH v7 0/6] Add Tegra241 (Grace) CMDQV Support (part 1/2) Nicolin Chen
` (4 preceding siblings ...)
2024-05-08 5:56 ` [PATCH v7 5/6] iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV Nicolin Chen
@ 2024-05-08 5:56 ` Nicolin Chen
2024-05-12 16:06 ` Jason Gunthorpe
5 siblings, 1 reply; 16+ messages in thread
From: Nicolin Chen @ 2024-05-08 5:56 UTC (permalink / raw)
To: will, robin.murphy, jgg
Cc: joro, thierry.reding, vdumpa, jonathanh, linux-kernel, iommu,
linux-arm-kernel, linux-tegra
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);
return &smmu->cmdq;
}
@@ -871,7 +872,7 @@ static int __arm_smmu_cmdq_issue_cmd(struct arm_smmu_device *smmu,
}
return arm_smmu_cmdq_issue_cmdlist(
- smmu, arm_smmu_get_cmdq(smmu), cmd, 1, sync);
+ smmu, arm_smmu_get_cmdq(smmu, ent->opcode), cmd, 1, sync);
}
static int arm_smmu_cmdq_issue_cmd(struct arm_smmu_device *smmu,
@@ -887,10 +888,11 @@ static int arm_smmu_cmdq_issue_cmd_with_sync(struct arm_smmu_device *smmu,
}
static void arm_smmu_cmdq_batch_init(struct arm_smmu_device *smmu,
- struct arm_smmu_cmdq_batch *cmds)
+ struct arm_smmu_cmdq_batch *cmds,
+ u8 opcode)
{
cmds->num = 0;
- cmds->cmdq = arm_smmu_get_cmdq(smmu);
+ cmds->cmdq = arm_smmu_get_cmdq(smmu, opcode);
}
static void arm_smmu_cmdq_batch_add(struct arm_smmu_device *smmu,
@@ -1167,7 +1169,7 @@ static void arm_smmu_sync_cd(struct arm_smmu_master *master,
},
};
- arm_smmu_cmdq_batch_init(smmu, &cmds);
+ arm_smmu_cmdq_batch_init(smmu, &cmds, cmd.opcode);
for (i = 0; i < master->num_streams; i++) {
cmd.cfgi.sid = master->streams[i].id;
arm_smmu_cmdq_batch_add(smmu, &cmds, &cmd);
@@ -2006,7 +2008,7 @@ static int arm_smmu_atc_inv_master(struct arm_smmu_master *master)
arm_smmu_atc_inv_to_cmd(IOMMU_NO_PASID, 0, 0, &cmd);
- arm_smmu_cmdq_batch_init(master->smmu, &cmds);
+ arm_smmu_cmdq_batch_init(master->smmu, &cmds, cmd.opcode);
for (i = 0; i < master->num_streams; i++) {
cmd.atc.sid = master->streams[i].id;
arm_smmu_cmdq_batch_add(master->smmu, &cmds, &cmd);
@@ -2046,7 +2048,7 @@ int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, int ssid,
arm_smmu_atc_inv_to_cmd(ssid, iova, size, &cmd);
- arm_smmu_cmdq_batch_init(smmu_domain->smmu, &cmds);
+ arm_smmu_cmdq_batch_init(smmu_domain->smmu, &cmds, cmd.opcode);
spin_lock_irqsave(&smmu_domain->devices_lock, flags);
list_for_each_entry(master, &smmu_domain->devices, domain_head) {
@@ -2123,7 +2125,7 @@ static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd,
num_pages++;
}
- arm_smmu_cmdq_batch_init(smmu_domain->smmu, &cmds);
+ arm_smmu_cmdq_batch_init(smmu_domain->smmu, &cmds, cmd->opcode);
while (iova < end) {
if (smmu->features & ARM_SMMU_FEAT_RANGE_INV) {
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 604e26a292e7..2c1fe7e129cd 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -879,7 +879,8 @@ struct tegra241_cmdqv *tegra241_cmdqv_acpi_probe(struct arm_smmu_device *smmu,
struct acpi_iort_node *node);
void tegra241_cmdqv_device_remove(struct arm_smmu_device *smmu);
int tegra241_cmdqv_device_reset(struct arm_smmu_device *smmu);
-struct arm_smmu_cmdq *tegra241_cmdqv_get_cmdq(struct arm_smmu_device *smmu);
+struct arm_smmu_cmdq *tegra241_cmdqv_get_cmdq(struct arm_smmu_device *smmu,
+ u8 opcode);
#else /* CONFIG_TEGRA241_CMDQV */
static inline bool arm_smmu_has_tegra241_cmdqv(struct arm_smmu_device *smmu)
{
@@ -903,7 +904,7 @@ static inline int tegra241_cmdqv_device_reset(struct arm_smmu_device *smmu)
}
static inline struct arm_smmu_cmdq *
-tegra241_cmdqv_get_cmdq(struct arm_smmu_device *smmu)
+tegra241_cmdqv_get_cmdq(struct arm_smmu_device *smmu, u8 opcode)
{
return NULL;
}
diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index ec4767e3859e..e7a281131e5d 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -181,6 +181,7 @@ struct tegra241_vcmdq {
* struct tegra241_vintf - Virtual Interface
* @idx: Global index in the CMDQV
* @enabled: Enable status
+ * @hyp_own: Owned by hypervisor (in-kernel)
* @cmdqv: Parent CMDQV pointer
* @lvcmdqs: List of logical VCMDQ pointers
* @base: MMIO base address
@@ -189,6 +190,7 @@ struct tegra241_vintf {
u16 idx;
bool enabled;
+ bool hyp_own;
struct tegra241_cmdqv *cmdqv;
struct tegra241_vcmdq **lvcmdqs;
@@ -326,7 +328,25 @@ static irqreturn_t tegra241_cmdqv_isr(int irq, void *devid)
/* Command Queue Selecting Function */
-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;
+ }
+}
+
+struct arm_smmu_cmdq *tegra241_cmdqv_get_cmdq(struct arm_smmu_device *smmu,
+ u8 opcode)
{
struct tegra241_cmdqv *cmdqv = smmu->tegra241_cmdqv;
struct tegra241_vintf *vintf = cmdqv->vintfs[0];
@@ -340,6 +360,10 @@ struct arm_smmu_cmdq *tegra241_cmdqv_get_cmdq(struct arm_smmu_device *smmu)
if (!READ_ONCE(vintf->enabled))
return &smmu->cmdq;
+ /* Unsupported CMD go for smmu->cmdq pathway */
+ if (!tegra241_vintf_support_cmd(vintf, opcode))
+ return &smmu->cmdq;
+
/*
* Select a LVCMDQ to use. Here we use a temporal solution to
* balance out traffic on cmdq issuing: each cmdq has its own
@@ -432,12 +456,22 @@ static int tegra241_vintf_hw_init(struct tegra241_vintf *vintf, bool hyp_own)
tegra241_vintf_hw_deinit(vintf);
/* Configure and enable VINTF */
+ /*
+ * Note that HYP_OWN bit is wired to zero when running in guest kernel,
+ * whether enabling it here or not, as !HYP_OWN cmdq HWs only support a
+ * restricted set of supported commands.
+ */
regval = FIELD_PREP(VINTF_HYP_OWN, hyp_own);
vintf_writel(vintf, regval, CONFIG);
ret = vintf_write_config(vintf, regval | VINTF_EN);
if (ret)
return ret;
+ /*
+ * As being mentioned above, HYP_OWN bit is wired to zero for a guest
+ * kernel, so read it back from HW to ensure that reflects in hyp_own
+ */
+ vintf->hyp_own = !!(VINTF_HYP_OWN & vintf_readl(vintf, CONFIG));
for (lidx = 0; lidx < vintf->cmdqv->num_lvcmdqs_per_vintf; lidx++) {
if (vintf->lvcmdqs && vintf->lvcmdqs[lidx]) {
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v7 2/6] iommu/arm-smmu-v3: Issue a batch of commands to the same cmdq
2024-05-08 5:56 ` [PATCH v7 2/6] iommu/arm-smmu-v3: Issue a batch of commands to the same cmdq Nicolin Chen
@ 2024-05-12 15:34 ` Jason Gunthorpe
0 siblings, 0 replies; 16+ messages in thread
From: Jason Gunthorpe @ 2024-05-12 15:34 UTC (permalink / raw)
To: Nicolin Chen
Cc: will, robin.murphy, joro, thierry.reding, vdumpa, jonathanh,
linux-kernel, iommu, linux-arm-kernel, linux-tegra
On Tue, May 07, 2024 at 10:56:50PM -0700, Nicolin Chen wrote:
> The driver calls in different places the arm_smmu_get_cmdq() helper, and
> it's fine to do so since the helper always returns the single SMMU CMDQ.
> However, with NVIDIA CMDQV extension or SMMU ECMDQ, there can be multiple
> cmdqs in the system to select one from. And either case requires a batch
> of commands to be issued to the same cmdq. Thus, a cmdq has to be decided
> in the higher-level callers.
>
> Add a cmdq pointer in arm_smmu_cmdq_batch structure, and decide the cmdq
> when initializing the batch. Pass it down to the bottom function. Update
> __arm_smmu_cmdq_issue_cmd() accordingly for single command issuers.
>
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 44 +++++++++++++--------
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 +
> 2 files changed, 29 insertions(+), 16 deletions(-)
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 3/6] iommu/arm-smmu-v3: Enforce arm_smmu_cmdq_build_sync_cmd
2024-05-08 5:56 ` [PATCH v7 3/6] iommu/arm-smmu-v3: Enforce arm_smmu_cmdq_build_sync_cmd Nicolin Chen
@ 2024-05-12 15:39 ` Jason Gunthorpe
2024-05-12 20:56 ` Nicolin Chen
0 siblings, 1 reply; 16+ messages in thread
From: Jason Gunthorpe @ 2024-05-12 15:39 UTC (permalink / raw)
To: Nicolin Chen
Cc: will, robin.murphy, joro, thierry.reding, vdumpa, jonathanh,
linux-kernel, iommu, linux-arm-kernel, linux-tegra
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 5/6] iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV
2024-05-08 5:56 ` [PATCH v7 5/6] iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV Nicolin Chen
@ 2024-05-12 15:54 ` Jason Gunthorpe
2024-05-12 21:00 ` Nicolin Chen
0 siblings, 1 reply; 16+ messages in thread
From: Jason Gunthorpe @ 2024-05-12 15:54 UTC (permalink / raw)
To: Nicolin Chen
Cc: will, robin.murphy, joro, thierry.reding, vdumpa, jonathanh,
linux-kernel, iommu, linux-arm-kernel, linux-tegra
On Tue, May 07, 2024 at 10:56:53PM -0700, Nicolin Chen wrote:
> +/* MMIO helpers */
> +#define cmdqv_readl(_cmdqv, _regname) \
> + readl((_cmdqv)->base + TEGRA241_CMDQV_##_regname)
> +#define cmdqv_readl_relaxed(_cmdqv, _regname) \
> + readl_relaxed((_cmdqv)->base + TEGRA241_CMDQV_##_regname)
> +#define cmdqv_readq_relaxed(_cmdqv, _regname) \
> + readq_relaxed((_cmdqv)->base + TEGRA241_CMDQV_##_regname)
> +#define cmdqv_writel(_cmdqv, val, _regname) \
> + writel((val), (_cmdqv)->base + TEGRA241_CMDQV_##_regname)
> +#define cmdqv_writel_relaxed(_cmdqv, val, _regname) \
> + writel_relaxed((val), (_cmdqv)->base + TEGRA241_CMDQV_##_regname)
> +
> +#define vintf_readl(_vintf, _regname) \
> + readl((_vintf)->base + TEGRA241_VINTF_##_regname)
> +#define vintf_readq_relaxed(_vintf, _regname) \
> + readq_relaxed((_vintf)->base + TEGRA241_VINTF_##_regname)
> +#define vintf_readl_relaxed(_vintf, _regname) \
> + readl_relaxed((_vintf)->base + TEGRA241_VINTF_##_regname)
> +#define vintf_writel(_vintf, val, _regname) \
> + writel((val), (_vintf)->base + TEGRA241_VINTF_##_regname)
> +#define vintf_writel_relaxed(_vintf, val, _regname) \
> + writel_relaxed((val), (_vintf)->base + TEGRA241_VINTF_##_regname)
> +
> +#define vcmdq_page0_readl(_vcmdq, _regname) \
> + readl((_vcmdq)->page0 + TEGRA241_VCMDQ_##_regname)
> +#define vcmdq_page0_readl_relaxed(_vcmdq, _regname) \
> + readl_relaxed((_vcmdq)->page0 + TEGRA241_VCMDQ_##_regname)
> +#define vcmdq_page0_writel(_vcmdq, val, _regname) \
> + writel((val), (_vcmdq)->page0 + TEGRA241_VCMDQ_##_regname)
> +#define vcmdq_page0_writel_relaxed(_vcmdq, val, _regname) \
> + writel_relaxed((val), (_vcmdq)->page0 + TEGRA241_VCMDQ_##_regname)
> +
> +#define vcmdq_page1_readl(_vcmdq, reg) \
> + readl((_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
> +#define vcmdq_page1_readl_relaxed(_vcmdq, reg) \
> + readl_relaxed((_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
> +#define vcmdq_page1_readq_relaxed(_vcmdq, reg) \
> + readq_relaxed((_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
> +#define vcmdq_page1_writel(_vcmdq, val, reg) \
> + writel((val), (_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
> +#define vcmdq_page1_writel_relaxed(_vcmdq, val, reg) \
> + writel_relaxed((val), (_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
> +#define vcmdq_page1_writeq(_vcmdq, val, reg) \
> + writeq((val), (_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
> +#define vcmdq_page1_writeq_relaxed(_vcmdq, val, reg) \
> + writeq_relaxed((val), (_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
These still need to be trimmed to only the ones being used. I still
think it is a bad idea, maybe a middle ground is to wrapper the
regsiter coding
writeq_relaxed(val, REG_CMDQ_PAGE1(vcmdq, XXX));
Is still short enough and safe enough without creating so much obfuscation..
I didn't notice anything else in this patch
Jason
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 6/6] iommu/tegra241-cmdqv: Limit CMDs for guest owned VINTF
2024-05-08 5:56 ` [PATCH v7 6/6] iommu/tegra241-cmdqv: Limit CMDs for guest owned VINTF Nicolin Chen
@ 2024-05-12 16:06 ` Jason Gunthorpe
2024-05-12 22:09 ` Nicolin Chen
0 siblings, 1 reply; 16+ messages in thread
From: Jason Gunthorpe @ 2024-05-12 16:06 UTC (permalink / raw)
To: Nicolin Chen
Cc: will, robin.murphy, joro, thierry.reding, vdumpa, jonathanh,
linux-kernel, iommu, linux-arm-kernel, linux-tegra
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 3/6] iommu/arm-smmu-v3: Enforce arm_smmu_cmdq_build_sync_cmd
2024-05-12 15:39 ` Jason Gunthorpe
@ 2024-05-12 20:56 ` Nicolin Chen
0 siblings, 0 replies; 16+ messages in thread
From: Nicolin Chen @ 2024-05-12 20:56 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: will, robin.murphy, joro, thierry.reding, vdumpa, jonathanh,
linux-kernel, iommu, linux-arm-kernel, linux-tegra
On Sun, May 12, 2024 at 12:39:43PM -0300, Jason Gunthorpe wrote:
> 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 ?
Yes. I will add that.
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Thanks!
Nicolin
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 5/6] iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV
2024-05-12 15:54 ` Jason Gunthorpe
@ 2024-05-12 21:00 ` Nicolin Chen
0 siblings, 0 replies; 16+ messages in thread
From: Nicolin Chen @ 2024-05-12 21:00 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: will, robin.murphy, joro, thierry.reding, vdumpa, jonathanh,
linux-kernel, iommu, linux-arm-kernel, linux-tegra
On Sun, May 12, 2024 at 12:54:49PM -0300, Jason Gunthorpe wrote:
> On Tue, May 07, 2024 at 10:56:53PM -0700, Nicolin Chen wrote:
> > +#define vcmdq_page1_readl(_vcmdq, reg) \
> > + readl((_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
> > +#define vcmdq_page1_readl_relaxed(_vcmdq, reg) \
> > + readl_relaxed((_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
> > +#define vcmdq_page1_readq_relaxed(_vcmdq, reg) \
> > + readq_relaxed((_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
> > +#define vcmdq_page1_writel(_vcmdq, val, reg) \
> > + writel((val), (_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
> > +#define vcmdq_page1_writel_relaxed(_vcmdq, val, reg) \
> > + writel_relaxed((val), (_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
> > +#define vcmdq_page1_writeq(_vcmdq, val, reg) \
> > + writeq((val), (_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
> > +#define vcmdq_page1_writeq_relaxed(_vcmdq, val, reg) \
> > + writeq_relaxed((val), (_vcmdq)->page1 + TEGRA241_VCMDQ_##reg)
>
> These still need to be trimmed to only the ones being used. I still
> think it is a bad idea, maybe a middle ground is to wrapper the
> regsiter coding
>
> writeq_relaxed(val, REG_CMDQ_PAGE1(vcmdq, XXX));
>
> Is still short enough and safe enough without creating so much obfuscation..
I think that is a much better practice! Let me try with it.
Thanks!
Nicolin
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 6/6] iommu/tegra241-cmdqv: Limit CMDs for guest owned VINTF
2024-05-12 16:06 ` Jason Gunthorpe
@ 2024-05-12 22:09 ` Nicolin Chen
2024-05-14 15:15 ` Jason Gunthorpe
0 siblings, 1 reply; 16+ messages in thread
From: Nicolin Chen @ 2024-05-12 22:09 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: will, robin.murphy, joro, thierry.reding, vdumpa, jonathanh,
linux-kernel, iommu, linux-arm-kernel, linux-tegra
On Sun, May 12, 2024 at 01:06:13PM -0300, Jason Gunthorpe wrote:
> 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.
Right, this makes the "opcode" look bad, though we know that the
"opcode" in the nesting invalidation doesn't matter because VCMDQ
in that case supports all commands with HYP_OWN=1.
> 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.
>
> ?
The "opcode" is intended to mean the opcode of a repeated commands
in an arm_smmu_cmdq_batch struct. And it is based on an assumption
that the driver doesn't and won't mix different commands into an
arm_smmu_cmdq_batch struct. Though it doesn't probably matter if a
batch mixes NH_ASID and ATC_INV..
A CMD_SYNC, on the other hand, is outside the batch struct. And
here is another assumption that CMD_SYNC is always supported by a
VCMDQ..
I could clarify the "opcode" here with these assumptions. Or maybe
we should think think of a better alternative?
> > -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?
In a nesting case, the host-level VCMDQ runs those comands, i.e.
HYP_OWN=1 meaning no command limitation.
Thanks
Nicolin
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 6/6] iommu/tegra241-cmdqv: Limit CMDs for guest owned VINTF
2024-05-12 22:09 ` Nicolin Chen
@ 2024-05-14 15:15 ` Jason Gunthorpe
2024-05-14 22:20 ` Nicolin Chen
0 siblings, 1 reply; 16+ messages in thread
From: Jason Gunthorpe @ 2024-05-14 15:15 UTC (permalink / raw)
To: Nicolin Chen
Cc: will, robin.murphy, joro, thierry.reding, vdumpa, jonathanh,
linux-kernel, iommu, linux-arm-kernel, linux-tegra
On Sun, May 12, 2024 at 03:09:25PM -0700, Nicolin Chen wrote:
> > > -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.
>
> Right, this makes the "opcode" look bad, though we know that the
> "opcode" in the nesting invalidation doesn't matter because VCMDQ
> in that case supports all commands with HYP_OWN=1.
Yeah, it isn't a real problem, it just looks a little messy and
should have a small comment someplace at least..
> A CMD_SYNC, on the other hand, is outside the batch struct. And
> here is another assumption that CMD_SYNC is always supported by a
> VCMDQ..
>
> I could clarify the "opcode" here with these assumptions. Or maybe
> we should think think of a better alternative?
I don't think it really needs to be more complex, but we should
document that invalidation is going to be special and doesn't quite
follow this rule.
Jason
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 6/6] iommu/tegra241-cmdqv: Limit CMDs for guest owned VINTF
2024-05-14 15:15 ` Jason Gunthorpe
@ 2024-05-14 22:20 ` Nicolin Chen
0 siblings, 0 replies; 16+ messages in thread
From: Nicolin Chen @ 2024-05-14 22:20 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: will, robin.murphy, joro, thierry.reding, vdumpa, jonathanh,
linux-kernel, iommu, linux-arm-kernel, linux-tegra
On Tue, May 14, 2024 at 12:15:13PM -0300, Jason Gunthorpe wrote:
> On Sun, May 12, 2024 at 03:09:25PM -0700, Nicolin Chen wrote:
> > > > -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.
> >
> > Right, this makes the "opcode" look bad, though we know that the
> > "opcode" in the nesting invalidation doesn't matter because VCMDQ
> > in that case supports all commands with HYP_OWN=1.
>
> Yeah, it isn't a real problem, it just looks a little messy and
> should have a small comment someplace at least..
>
> > A CMD_SYNC, on the other hand, is outside the batch struct. And
> > here is another assumption that CMD_SYNC is always supported by a
> > VCMDQ..
> >
> > I could clarify the "opcode" here with these assumptions. Or maybe
> > we should think think of a better alternative?
>
> I don't think it really needs to be more complex, but we should
> document that invalidation is going to be special and doesn't quite
> follow this rule.
Yea. I just added this:
@@ -333,10 +333,22 @@ 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)
{
+ /*
+ * TEGRA241 CMDQV has two modes to execute commands: host and guest.
+ * The host mode supports all the opcodes, while the guest mode only
+ * supports a few invalidation ones (check tegra241_vintf_support_cmd)
+ * and also a CMD_SYNC added by arm_smmu_cmdq_issue_cmdlist(..., true).
+ *
+ * Here pass in the representing opcode for either a single command or
+ * an arm_smmu_cmdq_batch, assuming that this SMMU driver will only add
+ * same type of commands into a batch as it does today or it will only
+ * mix supported invalidation commands in a batch.
+ */
if (arm_smmu_has_tegra241_cmdqv(smmu))
- return tegra241_cmdqv_get_cmdq(smmu);
+ return tegra241_cmdqv_get_cmdq(smmu, opcode);
return &smmu->cmdq;
}
Thanks
Nicolin
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-05-14 22:20 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-08 5:56 [PATCH v7 0/6] Add Tegra241 (Grace) CMDQV Support (part 1/2) Nicolin Chen
2024-05-08 5:56 ` [PATCH v7 1/6] iommu/arm-smmu-v3: Make symbols public for CONFIG_TEGRA241_CMDQV Nicolin Chen
2024-05-08 5:56 ` [PATCH v7 2/6] iommu/arm-smmu-v3: Issue a batch of commands to the same cmdq Nicolin Chen
2024-05-12 15:34 ` Jason Gunthorpe
2024-05-08 5:56 ` [PATCH v7 3/6] iommu/arm-smmu-v3: Enforce arm_smmu_cmdq_build_sync_cmd Nicolin Chen
2024-05-12 15:39 ` Jason Gunthorpe
2024-05-12 20:56 ` Nicolin Chen
2024-05-08 5:56 ` [PATCH v7 4/6] iommu/arm-smmu-v3: Add CS_NONE quirk for CONFIG_TEGRA241_CMDQV Nicolin Chen
2024-05-08 5:56 ` [PATCH v7 5/6] iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV Nicolin Chen
2024-05-12 15:54 ` Jason Gunthorpe
2024-05-12 21:00 ` Nicolin Chen
2024-05-08 5:56 ` [PATCH v7 6/6] iommu/tegra241-cmdqv: Limit CMDs for guest owned VINTF Nicolin Chen
2024-05-12 16:06 ` Jason Gunthorpe
2024-05-12 22:09 ` Nicolin Chen
2024-05-14 15:15 ` Jason Gunthorpe
2024-05-14 22:20 ` Nicolin Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).