Re: [PATCH v7 7/9] iommu/arm-smmu-v3: Move the CD generation for SVA into a function

From: Nicolin Chen <nicolinc@nvidia.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: <iommu@lists.linux.dev>, Joerg Roedel <joro@8bytes.org>,
	<linux-arm-kernel@lists.infradead.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Will Deacon <will@kernel.org>, Eric Auger <eric.auger@redhat.com>,
	"Moritz Fischer" <mdf@kernel.org>,
	Moritz Fischer <moritzf@google.com>,
	Michael Shavit <mshavit@google.com>, <patches@lists.linux.dev>,
	Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>,
	Mostafa Saleh <smostafa@google.com>
Subject: Re: [PATCH v7 7/9] iommu/arm-smmu-v3: Move the CD generation for SVA into a function
Date: Wed, 17 Apr 2024 00:37:19 -0700	[thread overview]
Message-ID: <Zh98L/vNPugu6JbV@Asurada-Nvidia> (raw)
In-Reply-To: <7-v7-cb149db3a320+3b5-smmuv3_newapi_p2_jgg@nvidia.com>

On Tue, Apr 16, 2024 at 04:28:18PM -0300, Jason Gunthorpe wrote:
> Pull all the calculations for building the CD table entry for a mmu_struct
> into arm_smmu_make_sva_cd().
> 
> Call it in the two places installing the SVA CD table entry.
> 
> Open code the last caller of arm_smmu_update_ctx_desc_devices() and remove
> the function.
> 
> Remove arm_smmu_write_ctx_desc() since all callers are gone. Add the
> locking assertions to arm_smmu_alloc_cd_ptr() since
> arm_smmu_update_ctx_desc_devices() was the last problematic caller.
> 
> Remove quiet_cd since all users are gone, arm_smmu_make_sva_cd() creates
> the same value.
> 
> The behavior of quiet_cd changes slightly, the old implementation edited
> the CD in place to set CTXDESC_CD_0_TCR_EPD0 assuming it was a SVA CD
> entry. This version generates a full CD entry with a 0 TTB0 and relies on
> arm_smmu_write_cd_entry() to install it hitlessly.
> 
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

> +static void arm_smmu_make_sva_cd(struct arm_smmu_cd *target,
> +				 struct arm_smmu_master *master,
> +				 struct mm_struct *mm, u16 asid)
> +{
> +	u64 par;
> +
> +	memset(target, 0, sizeof(*target));
> +
> +	par = cpuid_feature_extract_unsigned_field(
> +		read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1),
> +		ID_AA64MMFR0_EL1_PARANGE_SHIFT);
> +
> +	target->data[0] = cpu_to_le64(
> +		CTXDESC_CD_0_TCR_EPD1 |
> +#ifdef __BIG_ENDIAN
> +		CTXDESC_CD_0_ENDI |
> +#endif
> +		CTXDESC_CD_0_V |
> +		FIELD_PREP(CTXDESC_CD_0_TCR_IPS, par) |
> +		CTXDESC_CD_0_AA64 |
> +		(master->stall_enabled ? CTXDESC_CD_0_S : 0) |
> +		CTXDESC_CD_0_R |
> +		CTXDESC_CD_0_A |
> +		CTXDESC_CD_0_ASET |
> +		FIELD_PREP(CTXDESC_CD_0_ASID, asid));

This is set for the new "quiet_cd" case too. IIUIC, it is used to
ease the switching back to a normal CD, i.e. mm != NULL case?

Thanks
Nicolin