On Tue, Jul 25, 2017 at 06:32:40PM +0100, Robin Murphy wrote: > On 25/07/17 18:11, Lorenzo Pieralisi wrote: > > On Tue, Jul 25, 2017 at 12:17:31PM +0100, Shameer Kolothum wrote: > >> The helper function retrieves ITS address regions through IORT > >> device <-> ITS mappings and reserves it so that these regions > >> will not be translated by IOMMU and will be excluded from IOVA > >> allocations. IOMMU drivers can use this to implement their > >> .get_resv_regions callback. > >> > >> Signed-off-by: Shameer Kolothum <[email protected]> > >> --- > >> drivers/acpi/arm64/iort.c | 91 > >> ++++++++++++++++++++++++++++++++++++++-- > >> drivers/irqchip/irq-gic-v3-its.c | 3 +- > >> include/linux/acpi_iort.h | 8 +++- > >> 3 files changed, 97 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c > >> index a3215ee..e28f30c 100644 > >> --- a/drivers/acpi/arm64/iort.c > >> +++ b/drivers/acpi/arm64/iort.c > >> @@ -39,6 +39,7 @@ > >> struct iort_its_msi_chip { > >> struct list_head list; > >> struct fwnode_handle *fw_node; > >> + phys_addr_t base_addr; > >> u32 translation_id; > >> }; > >> > >> @@ -136,14 +137,16 @@ typedef acpi_status (*iort_find_node_callback) > >> static DEFINE_SPINLOCK(iort_msi_chip_lock); > >> > >> /** > >> - * iort_register_domain_token() - register domain token and related ITS ID > >> - * to the list from where we can get it back later on. > >> + * iort_register_domain_token() - register domain token along with related > >> + * ITS ID and base address to the list from where we can get it back > >> later on. > >> * @trans_id: ITS ID. > >> + * @base: ITS base address. > >> * @fw_node: Domain token. > >> * > >> * Returns: 0 on success, -ENOMEM if no memory when allocating list > >> element > >> */ > >> -int iort_register_domain_token(int trans_id, struct fwnode_handle > >> *fw_node) > >> +int iort_register_domain_token(int trans_id, phys_addr_t base, > >> + struct fwnode_handle *fw_node) > >> { > >> struct iort_its_msi_chip *its_msi_chip; > >> > >> @@ -153,6 +156,7 @@ int iort_register_domain_token(int trans_id, struct > >> fwnode_handle *fw_node) > >> > >> its_msi_chip->fw_node = fw_node; > >> its_msi_chip->translation_id = trans_id; > >> + its_msi_chip->base_addr = base; > >> > >> spin_lock(&iort_msi_chip_lock); > >> list_add(&its_msi_chip->list, &iort_msi_chip_list); > >> @@ -481,6 +485,24 @@ int iort_pmsi_get_dev_id(struct device *dev, u32 > >> *dev_id) > >> return -ENODEV; > >> } > >> > >> +static int iort_find_its_base(u32 its_id, phys_addr_t *base) > > > > You have to tag it as __maybe_unused for the !IOMMU_API case. > > > >> +{ > >> + struct iort_its_msi_chip *its_msi_chip; > >> + bool match = false; > >> + > >> + spin_lock(&iort_msi_chip_lock); > >> + list_for_each_entry(its_msi_chip, &iort_msi_chip_list, list) { > >> + if (its_msi_chip->translation_id == its_id) { > >> + *base = its_msi_chip->base_addr; > >> + match = true; > >> + break; > >> + } > >> + } > >> + spin_unlock(&iort_msi_chip_lock); > >> + > >> + return match ? 0 : -ENODEV; > >> +} > >> + > >> /** > >> * iort_dev_find_its_id() - Find the ITS identifier for a device > >> * @dev: The device. > >> @@ -639,6 +661,67 @@ int iort_add_device_replay(const struct iommu_ops > >> *ops, struct device *dev) > >> > >> return err; > >> } > >> + > >> +/** > >> + * iort_iommu_its_get_resv_regions - Reserved region driver helper > >> + * @dev: Device from iommu_get_resv_regions() > >> + * @list: Reserved region list from iommu_get_resv_regions() > >> + * > >> + * Returns: Number of reserved regions on success(0 if no associated ITS), > >> + * appropriate error value otherwise. > >> + * > >> + * IOMMU drivers can use this to implement their .get_resv_regions > >> callback > >> + * for reserving the HW ITS address regions. > > > > Stale comment. > > > >> + */ > >> +int iort_iommu_its_get_resv_regions(struct device *dev, struct list_head > >> *head) > >> +{ > >> + int i; > >> + struct acpi_iort_its_group *its; > >> + struct acpi_iort_node *node, *its_node = NULL; > >> + int resv = 0; > > > > Nit: int i, resv = 0; > > > > I can make these changes but I suspect this series will go via IOMMU > > tree, let me know how you want to handle it. > > > > Lorenzo > > > >> + node = iort_find_dev_node(dev); > >> + if (!node) > >> + return -ENODEV; > >> + > > I'd suggest we also want a comment here to clarify that we're currently > assuming straightforward topologies where all mappings for a given root > complex/named component target the same ITS group. Otherwise we're going > to need somewhat more logic to iterate the its_node processing over > every mapping (or every alias in the PCI case), but avoid creating > duplicate entries.
You have a point and we have time to update the code. Short of reserving all ITS regions for every device that maps to one at least, we could (even pre-compute instead of looking it up on the fly) create a list of ITS identifiers a given IORT node may map to and use that to reserve the regions. Thoughts ? Lorenzo _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
