Re: [PATCH v9 14/25] iommu/arm-smmu-v3-kvm: Shadow the command queue

[Mostafa Saleh]

On Thu, Nov 05, 2026 at 03:54:12PM -0800, Nicolin Chen wrote:
> On Thu, Oct 15, 2026 at 08:08:45PM +0000, Mostafa Saleh wrote:
> > At boot, allocate a command queue per SMMU which is used as a shadow
> > by the hypervisor.
> > 
> > The command queue size is 64K which is more than enough, as the
> > hypervisor would consume all the entries per a command queue prod
> > write, which means it can handle up to 4096 at a time.
> > 
> > Then, the host command queue needs to be pinned in a shared state, so
> > it can't be donated to VMs, and avoid tricking the hypervisor into
> > accessing them. This is done each time the command queue is enabled,
> > and undone each time the command queue is disabled.
> > The hypervisor won't access the host command queue when it is disabled
> > from the host.
> > 
> > Signed-off-by: Mostafa Saleh <smostafa@xxxxxxxxxx>
> 
> Reviewed-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
> 
> With some nits:
> 
> > @@ -23,6 +25,15 @@ static size_t kvm_arm_smmu_cur;
> >  static void __init kvm_arm_smmu_array_free(void)
> >  {
> >  	int order;
> > +	int i;
> > +
> > +	for (i = 0 ; i < kvm_arm_smmu_cur ; ++i) {
> 
> s/ ;/;/g

Will do.

> 
> > @@ -158,6 +268,14 @@ static bool smmu_dabt_device(struct hyp_arm_smmu_v3_device *smmu,
> >  		break;
> >  	/* Passthrough the register access for bisectability, handled later */
> >  	case ARM_SMMU_CMDQ_BASE:
> > +		if (is_write) {
> > +			/* Not allowed by the architecture */
> > +			if (is_cmdq_enabled(smmu))
> > +				break;
> 
> Sashiko reported the same WARN_ON issue. You might want to confirm.
> 

I replied to sashiko about that, this is by design as I also mentioned
in patch #13.

> > @@ -170,9 +288,16 @@ static bool smmu_dabt_device(struct hyp_arm_smmu_v3_device *smmu,
> >  			break;
> >  
> >  		if (is_write) {
> > +			bool last_cmdq_en = is_cmdq_enabled(smmu);
> 
> Maybe cmdq_was_enabled (local) and cmdq_is_enabled (helper)

Sure.

Thanks,
Mostafa


> > 
> Nicolin