[PATCH v5 16/27] iommu/arm-smmu-v3: Keep track of valid CD entries in the cd_table
Michael Shavit
mshavit at google.com
Tue Mar 19 06:55:17 PDT 2024
On Tue, Mar 5, 2024 at 7:44 AM Jason Gunthorpe <jgg at nvidia.com> wrote:
>
> We no longer need a master->sva_enable to control what attaches are
> allowed.
>
> Instead keep track inside the cd_table how many valid CD entries exist,
> and if the RID has a valid entry.
>
> Replace all the attach focused master->sva_enabled tests with a check if
> the CD has valid entries (or not). If there are any valid entries then the
> CD table must be currently programmed to the STE.
>
> Tested-by: Nicolin Chen <nicolinc at nvidia.com>
> Signed-off-by: Jason Gunthorpe <jgg at nvidia.com>
> ---
> .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 5 +---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 26 ++++++++++---------
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 10 +++++++
> 3 files changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> index ab9de8e36c45f5..82b9c4d4061c3d 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> @@ -433,9 +433,6 @@ static int __arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm,
> if (smmu_domain->stage != ARM_SMMU_DOMAIN_S1)
> return -ENODEV;
>
I assume this doesn't matter because of subsequent patches, but the
check above could also be removed since used_sid precisely means that
the attached domain is an ARM_SMMU_DOMAIN_S1 domain.
> - if (!master || !master->sva_enabled)
> - return -ENODEV;
> -
> bond = kzalloc(sizeof(*bond), GFP_KERNEL);
> if (!bond)
> return -ENOMEM;
> @@ -638,7 +635,7 @@ static int arm_smmu_sva_set_dev_pasid(struct iommu_domain *domain,
> struct mm_struct *mm = domain->mm;
> struct arm_smmu_cd target;
>
> - if (mm_get_enqcmd_pasid(mm) != id)
> + if (mm_get_enqcmd_pasid(mm) != id || !master->cd_table.used_sid)
> return -EINVAL;
>
> if (!arm_smmu_get_cd_ptr(master, id))
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 26c6b9f6f34fd3..fc19585bf192c6 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1285,6 +1285,8 @@ void arm_smmu_write_cd_entry(struct arm_smmu_master *master, int ssid,
> struct arm_smmu_cd *cdptr,
> const struct arm_smmu_cd *target)
> {
> + bool target_valid = target->data[0] & cpu_to_le64(CTXDESC_CD_0_V);
> + bool cur_valid = cdptr->data[0] & cpu_to_le64(CTXDESC_CD_0_V);
> struct arm_smmu_cd_writer cd_writer = {
> .writer = {
> .ops = &arm_smmu_cd_writer_ops,
> @@ -1293,6 +1295,15 @@ void arm_smmu_write_cd_entry(struct arm_smmu_master *master, int ssid,
> .ssid = ssid,
> };
>
> + if (cur_valid != target_valid) {
> + if (cur_valid)
> + master->cd_table.used_ssids--;
> + else
> + master->cd_table.used_ssids++;
> + }
> + if (ssid == IOMMU_NO_PASID)
> + master->cd_table.used_sid = target_valid;
> +
> arm_smmu_write_entry(&cd_writer.writer, cdptr->data, target->data);
> }
>
> @@ -2725,16 +2736,6 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> master = dev_iommu_priv_get(dev);
> smmu = master->smmu;
>
> - /*
> - * Checking that SVA is disabled ensures that this device isn't bound to
> - * any mm, and can be safely detached from its old domain. Bonds cannot
> - * be removed concurrently since we're holding the group mutex.
> - */
> - if (arm_smmu_master_sva_enabled(master)) {
> - dev_err(dev, "cannot attach - SVA enabled\n");
> - return -EBUSY;
> - }
> -
> mutex_lock(&smmu_domain->init_mutex);
>
> if (!smmu_domain->smmu) {
> @@ -2750,7 +2751,8 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> cdptr = arm_smmu_get_cd_ptr(master, IOMMU_NO_PASID);
> if (!cdptr)
> return -ENOMEM;
> - }
> + } else if (arm_smmu_ssids_in_use(&master->cd_table))
> + return -EBUSY;
>
> /*
> * Prevent arm_smmu_share_asid() from trying to change the ASID
> @@ -2825,7 +2827,7 @@ static int arm_smmu_attach_dev_ste(struct iommu_domain *domain,
> struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> struct attach_state state = {};
>
> - if (arm_smmu_master_sva_enabled(master))
> + if (arm_smmu_ssids_in_use(&master->cd_table))
> return -EBUSY;
>
> /*
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index 98dc5885c48655..7e1f6af4ce4e79 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -602,11 +602,21 @@ struct arm_smmu_ctx_desc_cfg {
> dma_addr_t cdtab_dma;
> struct arm_smmu_l1_ctx_desc *l1_desc;
> unsigned int num_l1_ents;
> + unsigned int used_ssids;
> + bool used_sid;
This probably deserves a comment. There's plenty of places where the
"rid" domain is handled as the CD with ssid 0; but we don't count it
as a used_ssid here.
I also don't find the meaning of used_sid obvious, especially if I
didn't have the context from the commit description.
> u8 s1fmt;
> /* log2 of the maximum number of CDs supported by this table */
> u8 s1cdmax;
> };
>
> +/* True if the cd table has SSIDS > 0 in use. */
> +static inline bool arm_smmu_ssids_in_use(struct arm_smmu_ctx_desc_cfg *cd_table)
> +{
> + if (cd_table->used_sid)
> + return cd_table->used_ssids > 1;
> + return cd_table->used_ssids;
> +}
> +
> struct arm_smmu_s2_cfg {
> u16 vmid;
> };
> --
> 2.43.2
>
More information about the linux-arm-kernel
mailing list