Hi Eric, On 2021/2/24 4:56, Eric Auger wrote: > On attach_pasid_table() we program STE S1 related info set > by the guest into the actual physical STEs. At minimum > we need to program the context descriptor GPA and compute > whether the stage1 is translated/bypassed or aborted. > > On detach, the stage 1 config is unset and the abort flag is > unset. > > Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx> > [...] > + > + /* > + * we currently support a single CD so s1fmt and s1dss > + * fields are also ignored > + */ > + if (cfg->pasid_bits) > + goto out; > + > + smmu_domain->s1_cfg.cdcfg.cdtab_dma = cfg->base_ptr; only the "cdtab_dma" field of "cdcfg" is set, we are not able to locate a specific cd using arm_smmu_get_cd_ptr(). Maybe we'd better use a specialized function to fill other fields of "cdcfg" or add a sanity check in arm_smmu_get_cd_ptr() to prevent calling it under nested mode? As now we just call arm_smmu_get_cd_ptr() during finalise_s1(), no problem found. Just a suggestion ;-) Thanks, Keqian > + smmu_domain->s1_cfg.set = true; > + smmu_domain->abort = false; > + break; > + default: > + goto out; > + } > + spin_lock_irqsave(&smmu_domain->devices_lock, flags); > + list_for_each_entry(master, &smmu_domain->devices, domain_head) > + arm_smmu_install_ste_for_dev(master); > + spin_unlock_irqrestore(&smmu_domain->devices_lock, flags); > + ret = 0; > +out: > + mutex_unlock(&smmu_domain->init_mutex); > + return ret; > +} > + > +static void arm_smmu_detach_pasid_table(struct iommu_domain *domain) > +{ > + struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); > + struct arm_smmu_master *master; > + unsigned long flags; > + > + mutex_lock(&smmu_domain->init_mutex); > + > + if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED) > + goto unlock; > + > + smmu_domain->s1_cfg.set = false; > + smmu_domain->abort = false; > + > + spin_lock_irqsave(&smmu_domain->devices_lock, flags); > + list_for_each_entry(master, &smmu_domain->devices, domain_head) > + arm_smmu_install_ste_for_dev(master); > + spin_unlock_irqrestore(&smmu_domain->devices_lock, flags); > + > +unlock: > + mutex_unlock(&smmu_domain->init_mutex); > +} > + > static bool arm_smmu_dev_has_feature(struct device *dev, > enum iommu_dev_features feat) > { > @@ -2939,6 +3026,8 @@ static struct iommu_ops arm_smmu_ops = { > .of_xlate = arm_smmu_of_xlate, > .get_resv_regions = arm_smmu_get_resv_regions, > .put_resv_regions = generic_iommu_put_resv_regions, > + .attach_pasid_table = arm_smmu_attach_pasid_table, > + .detach_pasid_table = arm_smmu_detach_pasid_table, > .dev_has_feat = arm_smmu_dev_has_feature, > .dev_feat_enabled = arm_smmu_dev_feature_enabled, > .dev_enable_feat = arm_smmu_dev_enable_feature, > _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm