Re: [PATCH v7 23/24] iommu/arm-smmu-v3-kvm: Enable nesting

[Jason Gunthorpe]

On Wed, Aug 26, 2026 at 08:54:19AM +0000, Mostafa Saleh wrote:
> On Tue, Aug 25, 2026 at 05:59:50PM -0300, Jason Gunthorpe wrote:
> > > [ ... 129 lines skipped ... ]
> > > +	ret = smmu_attach_stage_2(&target);
> > > +	if (ret)
> > > +		return ret;
> > > +	hyp_spin_lock(&smmu->hw_lock);
> > > +	cur_valid = FIELD_GET(STRTAB_STE_0_V, le64_to_cpu(hyp_ste_ptr->data[0]));
> > > +	target_cfg = FIELD_GET(STRTAB_STE_0_CFG, le64_to_cpu(target.data[0]));
> > > +	target_valid = FIELD_GET(STRTAB_STE_0_V, le64_to_cpu(target.data[0]));
> > > +	if ((cur_valid && !target_valid) ||
> > > +	    (target_cfg == STRTAB_STE_0_CFG_ABORT)) {
> > > +		WRITE_ONCE(hyp_ste_ptr->data[0], target.data[0]);
> > > +		WARN_ON(smmu_send_cmd(smmu, &cfgi_cmd));
> > > +		for (i = 1; i < STRTAB_STE_DWORDS; i++)
> > > +			WRITE_ONCE(hyp_ste_ptr->data[i], target.data[i]);
> > > +	} else {
> > > +		for (i = 1; i < STRTAB_STE_DWORDS; i++)
> > > +			WRITE_ONCE(hyp_ste_ptr->data[i], target.data[i]);
> > > +		WARN_ON(smmu_send_cmd(smmu, &cfgi_cmd));
> > > +		WRITE_ONCE(hyp_ste_ptr->data[0], target.data[0]);
> > > +	}
> > 
> > This doesn't look good enough, a driver can't safe writely to a valid
> > STE in any order like this, and it can't make it non-valid or risk
> > breaking guests. We had this bug in linux already, the hitless STE
> > update in the hypervisor is mandatory for linux guests using PASID.
> 
> This piggy-backs on the kernel algorithm, as described in the comment
> in this patch:
> +       /*
> +        * Summary of each host emulated state vs real HW.
> +        * |    Host    |       HW      |
> +        * ==============================
> +        * |    V=0     |       V=0     |
> +        * |    Abort   |       Abort   |
> +        * |    Bypass  |       S2      |
> +        * |    S1      |       S1+S2   |
> +        *
> +        * For the host, any V=0 transition is not hitless, all other permutations of
> +        * (abort, bypass, S1) transitions are hitless.
> +        * For the HW state, any V=0 transition is not hitless, as all the S2 config is
> +        * always the same (ttbr, vtcr...), all other transitions should be hitless too.
> +        * However, the host is not trusted, which means that any V=0 <=> V=1 transitions
> +        * or any transition to an abort STE we need to enforce writing order of the STE
> +        * dword 0 and add CFGI.
> +        * Otherwise, we write the STE in the opposite order to cover cases from abort
> +        * to S2 or nested.
> +        */
> 
> So this way, the hypervisor doesn't break the STE and keeps the host
> STE transitions hitless.
> 
> Or there is an example that I missed and is broken by this?

If we are exiting S1DSS mode it looks like this clears S1DSS before
changing to S2 only mode? That would be a bug.

There are also gotchas where writing things out of order can make the
STE UNPREDICTABLE that must be avoided too.

We really don't need two versions of this stuf, just use the
programmer we know it is right at this point.

Jason