Re: [PATCH v6 16/25] iommu/arm-smmu-v3-kvm: Add CMDQ functions
From: Vincent Donnefort <hidden>
Date: 2026-07-10 16:01:15
Also in:
kvmarm, linux-iommu, lkml
On Fri, May 01, 2026 at 11:19:18AM +0000, Mostafa Saleh wrote:
quoted hunk ↗ jump to hunk
Add functions to access the command queue, there are 2 main usage: - Hypervisor's own commands, as TLB invalidation, would use functions as smmu_send_cmd(), which creates and sends a command. - Add host commands to the shadow command queue, after being filtered, these will be added with smmu_add_cmd_raw. Signed-off-by: Mostafa Saleh <smostafa@google.com> --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 14 ++- .../iommu/arm/arm-smmu-v3/pkvm/arm-smmu-v3.c | 107 ++++++++++++++++++ 2 files changed, 115 insertions(+), 6 deletions(-)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 f904f4d19609..3fc499608d76 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h@@ -1156,19 +1156,21 @@ u32 smmu_idr5_to_oas(u32 reg); unsigned long smmu_idr5_to_pgsize(u32 reg); /* Queue functions shared between kernel and hyp. */ -static inline bool queue_has_space(struct arm_smmu_ll_queue *q, u32 n) +static inline u32 queue_space(struct arm_smmu_ll_queue *q) { - u32 space, prod, cons; + u32 prod, cons; prod = Q_IDX(q, q->prod); cons = Q_IDX(q, q->cons); if (Q_WRP(q, q->prod) == Q_WRP(q, q->cons)) - space = (1 << q->max_n_shift) - (prod - cons); - else - space = cons - prod; + return (1 << q->max_n_shift) - (prod - cons); + return cons - prod; +} - return space >= n; +static inline bool queue_has_space(struct arm_smmu_ll_queue *q, u32 n) +{ + return queue_space(q) >= n; } static inline bool queue_full(struct arm_smmu_ll_queue *q)diff --git a/drivers/iommu/arm/arm-smmu-v3/pkvm/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/pkvm/arm-smmu-v3.c index 3b77796dafc7..aac455599728 100644 --- a/drivers/iommu/arm/arm-smmu-v3/pkvm/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/pkvm/arm-smmu-v3.c@@ -6,6 +6,7 @@ */ #include <asm/kvm_hyp.h> +#include <nvhe/clock.h> #include <nvhe/iommu.h> #include <nvhe/mem_protect.h> #include <nvhe/trap_handler.h>@@ -22,6 +23,31 @@ struct hyp_arm_smmu_v3_device *kvm_hyp_arm_smmu_v3_smmus; #define cmdq_size(cmdq) ((1 << ((cmdq)->llq.max_n_shift)) * CMDQ_ENT_DWORDS * 8)
I only looked at that smmu_wait() as this is the hyp_clock_ns() caller.
+/* + * Wait until @cond is true. + * Return 0 on success, or -ETIMEDOUT
nit: Would be nice to explain use_wfe.
+ */ +#define smmu_wait(use_wfe, _cond) \
nit:, (__use_wfe, __cond), or (use_fw, cond)
+({ \
+ int __ret = 0; \
+ u64 delay = hyp_clock_ns() + ARM_SMMU_POLL_TIMEOUT_US * 1000; \Does it help to lower the risk for the 128-bits fallback if we keep the resolution to _us? Also, ARM_SMMU_POLL_TIMEOUT_US seems to be 1s. Blocking that long at EL2 is really terrible. Any chance to lower that?
+ \
+ while (!(_cond)) { \
+ if (use_wfe) { \
+ wfe(); \
+ if ((_cond)) \nit: I believe there's no need for the double () here is there?
+ break; \
+ } else { \
+ cpu_relax(); \
+ } \
+ if (hyp_clock_ns() >= delay) { \
+ __ret = -ETIMEDOUT; \
+ break; \
+ } \
+ } \
+ __ret; \
+})
+[...]