> [ ... 103 lines skipped ... ]
> +/*
> + * We will not claim these PCI devices. Eg hypervisor debugger is using it
> + * for a dynamic debug session. They cannot be enumerated under static ACPI
> + * device scope.
> + */
> +static char *hv_skip_pci_devs;
> +static int __init hv_iommu_setup_skip(char *str)
> +{
> + hv_skip_pci_devs = str;
> + return 1;
> +}
> +/* Eg: hv_iommu_skip=(SSSS:BB:DD.F)(SSSS:BB:DD.F) */
> +__setup("hv_iommu_skip=", hv_iommu_setup_skip);
I don't like this in a driver. This isn't really skipping anything, it
is just leaving some devices in an identity mode.
If you have a use case for this as a general command line policy then
come with a core code enhacmenet so everyone can choose per-device
their boot time mode.
> [ ... 7 lines skipped ... ]
> +struct hv_domain {
> + struct iommu_domain iommu_dom;
> + u32 domid_num; /* as opposed to domain_id.type */
> + spinlock_t mappings_lock; /* protects mappings_tree */
> + struct rb_root_cached mappings_tree; /* iova to pa lookup tree */
This seems basically identical to what virtio-iommu is doing, can you
consider sharing its code?
> [ ... 26 lines skipped ... ]
> +static bool hv_special_domain(struct hv_domain *hvdom)
> +{
> + return hvdom == &hv_def_identity_dom || hvdom == &hv_def_blocked_dom;
> +}
This is only called by hv_iommu_domain_free() which is only linked to
hv_paging_domain_ops(), so it should be dead code
> [ ... 204 lines skipped ... ]
> +static int hv_iommu_attach_dev(struct iommu_domain *immdom, struct device
> *dev,
> + struct iommu_domain *old)
> +{
> + struct pci_dev *pdev;
> + int rc;
> + struct hv_domain *hvdom_new = to_hv_domain(immdom);
'new' is an odd variable name here
> [ ... 246 lines skipped ... ]
> +static struct iommu_group *hv_iommu_device_group(struct device *dev)
> +{
> + return pci_device_group(dev);
> +}
No need for a wrapper, use the function directly in the ops
> +
> +static void hv_iommu_get_resv_regions(struct device *dev,
> + struct list_head *head)
> +{
> + struct iommu_resv_region *reg;
> +
> + /* reserve the entire LAPIC region */
> + reg = iommu_alloc_resv_region(0xfee00000, SZ_1M, 0, IOMMU_RESV_MSI,
> + GFP_KERNEL);
There was some discussion to make a helper for this, I don't see it
merged yet..
> [ ... 65 lines skipped ... ]
> +static int __init hv_iommu_init(void)
> +{
> + int rc;
> + struct iommu_device *iommup = &hv_virt_iommu;
> + struct hv_output_get_iommu_capabilities caps;
> +
> + if (!hv_is_hyperv_initialized())
> + return -ENODEV;
> +
> + rc = hv_iommu_get_caps(&caps);
> + if (rc)
> + return rc;
> +
> + hv_iommu_max_iova = ((ulong)1 << caps.max_iova_width) - 1;
> +
> + rc = iommu_device_sysfs_add(iommup, NULL, NULL, "%s", "hyperv-iommu");
> + if (rc) {
> + pr_err("Hyper-V: iommu_device_sysfs_add failed: %d\n", rc);
> + return rc;
> + }
> +
> + /* This must come before iommu_device_register() because the latter
> + * calls into the hooks.
> + */
> + hv_initialize_special_domains();
This probably should be before doing anything with sysfs.
--
Jason
