Re: [PATCH v5 10/19] iommu/arm-smmu: Keep track of S2CR state

[Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>]

On Tue, Aug 23, 2016 at 08:05:21PM +0100, Robin Murphy wrote:
> Making S2CRs first-class citizens within the driver with a high-level
> representation of their state offers a neat solution to a few problems:
> 
> Firstly, the information about which context a device's stream IDs are
> associated with is already present by necessity in the S2CR. With that
> state easily accessible we can refer directly to it and obviate the need
> to track an IOMMU domain in each device's archdata (its earlier purpose
> of enforcing correct attachment of multi-device groups now being handled
> by the IOMMU core itself).
> 
> Secondly, the core API now deprecates explicit domain detach and expects
> domain attach to move devices smoothly from one domain to another; for
> SMMUv2, this notion maps directly to simply rewriting the S2CRs assigned
> to the device. By giving the driver a suitable abstraction of those
> S2CRs to work with, we can massively reduce the overhead of the current
> heavy-handed "detach, free resources, reallocate resources, attach"
> approach.
> 
> Thirdly, making the software state hardware-shaped and attached to the
> SMMU instance once again makes suspend/resume of this register group
> that much simpler to implement in future.
> 
> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> ---
>  drivers/iommu/arm-smmu.c | 159 +++++++++++++++++++++++++++--------------------
>  1 file changed, 93 insertions(+), 66 deletions(-)

[...]

> @@ -1145,9 +1198,16 @@ static void arm_smmu_master_free_smes(struct arm_smmu_device *smmu,
>  static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
>  				      struct arm_smmu_master_cfg *cfg)
>  {
> -	int i, ret;
> +	int i, ret = 0;
>  	struct arm_smmu_device *smmu = smmu_domain->smmu;
> -	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
> +	struct arm_smmu_s2cr *s2cr = smmu->s2crs;
> +	enum arm_smmu_s2cr_type type = S2CR_TYPE_TRANS;
> +	u8 cbndx = smmu_domain->cfg.cbndx;
> +
> +	if (cfg->smendx[0] < 0)

Shouldn't that be INVALID_SMENDX?

Will