| Message ID | 20230411191636.26926-8-vikram.garhwal@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | dynamic node programming using overlay dtbo | expand |
Hi Vikram, On 11/04/2023 21:16, Vikram Garhwal wrote: > > > Add remove_device callback for removing the device entry from smmu-master using > following steps: > 1. Find if SMMU master exists for the device node. > 2. Remove the SMMU master > > Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com> > Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> > --- > xen/drivers/passthrough/arm/smmu.c | 56 ++++++++++++++++++++++++++++++ > 1 file changed, 56 insertions(+) > > diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c > index 0a514821b3..14e15f1bc6 100644 > --- a/xen/drivers/passthrough/arm/smmu.c > +++ b/xen/drivers/passthrough/arm/smmu.c > @@ -816,6 +816,19 @@ static int insert_smmu_master(struct arm_smmu_device *smmu, > return 0; > } > > +static int remove_smmu_master(struct arm_smmu_device *smmu, > + struct arm_smmu_master *master) > +{ > + if (!smmu->masters.rb_node) { > + ASSERT_UNREACHABLE(); > + return -ENOENT; > + } > + > + rb_erase(&master->node, &smmu->masters); > + > + return 0; > +} > + > static int arm_smmu_dt_add_device_legacy(struct arm_smmu_device *smmu, > struct device *dev, > struct iommu_fwspec *fwspec) > @@ -853,6 +866,32 @@ static int arm_smmu_dt_add_device_legacy(struct arm_smmu_device *smmu, > return insert_smmu_master(smmu, master); > } > > +static int arm_smmu_dt_remove_device_legacy(struct arm_smmu_device *smmu, > + struct device *dev) > +{ > + struct arm_smmu_master *master; > + struct device_node *dev_node = dev_get_dev_node(dev); > + int ret; > + > + master = find_smmu_master(smmu, dev_node); > + if (master == NULL) { > + dev_err(dev, > + "No registrations found for master device %s\n", > + dev_node->name); > + return -EINVAL; > + } > + > + ret = remove_smmu_master(smmu, master); This patch looks good although I remember seeing Julien advising you that it would be beneficial for the SMMU driver itself to check if device is not currently used before you remove it (even though you have this check in iommu_remove_dt_device(). I could not find your answer to this. NIT: No need for a blank line here if the next instruction is checking the ret value. With the above things clarified: Reviewed-by: Michal Orzel <michal.orzel@amd.com> ~Michal
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c index 0a514821b3..14e15f1bc6 100644 --- a/xen/drivers/passthrough/arm/smmu.c +++ b/xen/drivers/passthrough/arm/smmu.c @@ -816,6 +816,19 @@ static int insert_smmu_master(struct arm_smmu_device *smmu, return 0; } +static int remove_smmu_master(struct arm_smmu_device *smmu, + struct arm_smmu_master *master) +{ + if (!smmu->masters.rb_node) { + ASSERT_UNREACHABLE(); + return -ENOENT; + } + + rb_erase(&master->node, &smmu->masters); + + return 0; +} + static int arm_smmu_dt_add_device_legacy(struct arm_smmu_device *smmu, struct device *dev, struct iommu_fwspec *fwspec) @@ -853,6 +866,32 @@ static int arm_smmu_dt_add_device_legacy(struct arm_smmu_device *smmu, return insert_smmu_master(smmu, master); } +static int arm_smmu_dt_remove_device_legacy(struct arm_smmu_device *smmu, + struct device *dev) +{ + struct arm_smmu_master *master; + struct device_node *dev_node = dev_get_dev_node(dev); + int ret; + + master = find_smmu_master(smmu, dev_node); + if (master == NULL) { + dev_err(dev, + "No registrations found for master device %s\n", + dev_node->name); + return -EINVAL; + } + + ret = remove_smmu_master(smmu, master); + + if (ret) + return ret; + + dev_node->is_protected = false; + + kfree(master); + return 0; +} + static int register_smmu_master(struct arm_smmu_device *smmu, struct device *dev, struct of_phandle_args *masterspec) @@ -876,6 +915,22 @@ static int register_smmu_master(struct arm_smmu_device *smmu, fwspec); } +static int arm_smmu_dt_remove_device_generic(u8 devfn, struct device *dev) +{ + struct arm_smmu_device *smmu; + struct iommu_fwspec *fwspec; + + fwspec = dev_iommu_fwspec_get(dev); + if (fwspec == NULL) + return -ENXIO; + + smmu = find_smmu(fwspec->iommu_dev); + if (smmu == NULL) + return -ENXIO; + + return arm_smmu_dt_remove_device_legacy(smmu, dev); +} + static int arm_smmu_dt_add_device_generic(u8 devfn, struct device *dev) { struct arm_smmu_device *smmu; @@ -2858,6 +2913,7 @@ static const struct iommu_ops arm_smmu_iommu_ops = { .init = arm_smmu_iommu_domain_init, .hwdom_init = arch_iommu_hwdom_init, .add_device = arm_smmu_dt_add_device_generic, + .remove_device = arm_smmu_dt_remove_device_generic, .teardown = arm_smmu_iommu_domain_teardown, .iotlb_flush = arm_smmu_iotlb_flush, .assign_device = arm_smmu_assign_dev,