Re: [PATCH v6 16/25] iommu/arm-smmu-v3-kvm: Add CMDQ functions

[Mostafa Saleh]

On Fri, Jul 10, 2026 at 05:01:07PM +0100, Vincent Donnefort wrote:
> On Fri, May 01, 2026 at 11:19:18AM +0000, Mostafa Saleh wrote:
> > 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@xxxxxxxxxx>
> > ---
> >  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(-)
> > 
[...]

> 
> 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. 

I will add it.
> 
> > + */
> > +#define smmu_wait(use_wfe, _cond)					\
> 
> nit:, (__use_wfe, __cond), or (use_fw, cond)

Will do.

> 
> > +({									\
> > +	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?

I will remove the tracing clock support as mentioned and I will handle
the overflow in smmu_wait()

> 
> Also, ARM_SMMU_POLL_TIMEOUT_US seems to be 1s. Blocking that long at EL2 is
> really terrible. Any chance to lower that? 
> 

That should never really happen, I am re-using the kernel max, but I can
change this for the hypervisor.

> > +									\
> > +	while (!(_cond)) {						\
> > +		if (use_wfe) {						\
> > +			wfe();						\
> > +			if ((_cond))					\
> 
> nit: I believe there's no need for the double () here is there?
> 

Will do.

Thanks,
Mostafa