On 10/30/2017 06:15 PM, Alex Williamson wrote: > On Mon, 30 Oct 2017 16:39:31 -0400 > Vishwanath Pai via iommu <[email protected]> wrote: > >> This patch adds a new sysfile for intel-iommu driver which prints out >> all allocated iommu domains: >> >> $> cat /sys/class/iommu/dmar0/intel-iommu/domains >> did | pci_device_name >> 1 | 0000:00:14.0 >> 2 | 0000:00:1d.0 >> 3 | 0000:00:1f.0 >> 4 | 0000:02:00.0 >> 5 | 0000:02:00.1 >> 6 | 0000:02:00.2 >> 7 | 0000:02:00.3 >> 8 | 0000:00:1f.2 >> 9 | 0000:04:00.0 >> 10 | 0000:03:00.0 >> 11 | 0000:03:00.1 > > If we want this feature (you haven't said why we want this feature), > this is the wrong model to do it. It's heavy weight, it has built in > truncation if we have too many devices, it only supports PCI, it kind of > violates the sysfs one value per file model, it's not very machine > readable, and it's potentially racy. Why not instead create a directory > per domain id and within that directory link to the devices attached to > the domain? Evaluating the domains is then free at the cost of a > little extra when we add/remove domains or attach/detach devices. > Thanks, > > Alex > > >> Signed-off-by: Vishwanath Pai <[email protected]> >> --- >> drivers/iommu/intel-iommu.c | 53 >> +++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 53 insertions(+) >> >> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c >> index 6784a05..9428cb9 100644 >> --- a/drivers/iommu/intel-iommu.c >> +++ b/drivers/iommu/intel-iommu.c >> @@ -4704,6 +4704,58 @@ static ssize_t intel_iommu_show_ndoms_used(struct >> device *dev, >> } >> static DEVICE_ATTR(domains_used, S_IRUGO, intel_iommu_show_ndoms_used, >> NULL); >> >> +/* >> + * This can return a maximum of PAGE_SIZE bytes, so we are forced to chop >> off >> + * any characters beyond that >> + */ >> +static ssize_t intel_iommu_show_domains(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct intel_iommu *iommu = dev_to_intel_iommu(dev); >> + struct device_domain_info *info; >> + struct dmar_domain *domain; >> + bool found = false; >> + ssize_t len, count = 0; >> + char *str; >> + int did; >> + >> + str = (char *) kmalloc(PAGE_SIZE, GFP_KERNEL); >> + if (!str) >> + return sprintf(buf, "ERROR: ENOMEM\n"); >> + >> + for (did = 0; did < cap_ndoms(iommu->cap); did++) { >> + domain = get_iommu_domain(iommu, (u16)did); >> + >> + if (!domain) >> + continue; >> + >> + list_for_each_entry(info, &domain->devices, link) { >> + if (!info->dev) >> + continue; >> + >> + if (!dev_is_pci(info->dev)) >> + continue; >> + >> + if (!found) >> + count += sprintf(buf, "did | >> pci_device_name\n"); >> + found = true; >> + >> + len = sprintf(str, "%3d | %s\n", did, >> dev_name(info->dev)); >> + if (count + len < PAGE_SIZE) >> + count += sprintf(buf+count, str); >> + } >> + } >> + >> + kfree(str); >> + >> + if (!found) >> + return sprintf(buf, "No domains found, IOMMU disabled?\n"); >> + >> + return count; >> +} >> +static DEVICE_ATTR(domains, S_IRUGO, intel_iommu_show_domains, NULL); >> + >> static struct attribute *intel_iommu_attrs[] = { >> &dev_attr_version.attr, >> &dev_attr_address.attr, >> @@ -4711,6 +4763,7 @@ static ssize_t intel_iommu_show_ndoms_used(struct >> device *dev, >> &dev_attr_ecap.attr, >> &dev_attr_domains_supported.attr, >> &dev_attr_domains_used.attr, >> + &dev_attr_domains.attr, >> NULL, >> }; >> >
I agree that this is not the best way to achieve this, and yes it does truncate once we hit PAGE_SIZE on the buffer. But this was mainly for informational purposes only. I wanted a way to tell what devices are doing DMA and what domains they are allocated to and I could not find an easy way to do that. There are probably tools in various hypervisors to do this but we do not use any of that. Having a directory per domain_id and linking to devices from there is definitely a better approach. I will submit another patch to do it. Thanks, Vishwanath _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
