Hey Lu, > On 15 Mar 2019, at 02:19, Lu Baolu <[email protected]> wrote: > > Hi James, > > On 3/14/19 7:58 PM, James Sewart wrote: >> To support mapping ISA region via iommu_group_create_direct_mappings, >> make sure its exposed by iommu_get_resv_regions. This allows >> deduplication of reserved region mappings >> Signed-off-by: James Sewart <[email protected]> >> --- >> drivers/iommu/intel-iommu.c | 42 +++++++++++++++++++++++++++++-------- >> 1 file changed, 33 insertions(+), 9 deletions(-) >> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c >> index 8e0a4e2ff77f..2e00e8708f06 100644 >> --- a/drivers/iommu/intel-iommu.c >> +++ b/drivers/iommu/intel-iommu.c >> @@ -337,6 +337,8 @@ static LIST_HEAD(dmar_rmrr_units); >> #define for_each_rmrr_units(rmrr) \ >> list_for_each_entry(rmrr, &dmar_rmrr_units, list) >> +static struct iommu_resv_region *isa_resv_region; >> + >> /* bitmap for indexing intel_iommus */ >> static int g_num_of_iommus; >> @@ -2780,26 +2782,34 @@ static inline int iommu_prepare_rmrr_dev(struct >> dmar_rmrr_unit *rmrr, >> rmrr->end_address); >> } >> +static inline struct iommu_resv_region *iommu_get_isa_resv_region(void) >> +{ >> + if (!isa_resv_region) >> + isa_resv_region = iommu_alloc_resv_region(0, >> + 16*1024*1024, >> + 0, IOMMU_RESV_DIRECT); >> + >> + return isa_resv_region; >> +} >> + >> #ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA >> -static inline void iommu_prepare_isa(void) >> +static inline void iommu_prepare_isa(struct pci_dev *pdev) >> { >> - struct pci_dev *pdev; >> int ret; >> + struct iommu_resv_region *reg = iommu_get_isa_resv_region(); >> - pdev = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL); >> - if (!pdev) >> + if (!reg) >> return; >> pr_info("Prepare 0-16MiB unity mapping for LPC\n"); >> - ret = iommu_prepare_identity_map(&pdev->dev, 0, 16*1024*1024 - 1); >> + ret = iommu_prepare_identity_map(&pdev->dev, reg->start, >> + reg->start + reg->length - 1); >> if (ret) >> pr_err("Failed to create 0-16MiB identity map - floppy might >> not work\n"); >> - >> - pci_dev_put(pdev); >> } >> #else >> -static inline void iommu_prepare_isa(void) >> +static inline void iommu_prepare_isa(struct pci_dev *pdev) >> { >> return; >> } >> @@ -3289,6 +3299,7 @@ static int __init init_dmars(void) >> struct dmar_rmrr_unit *rmrr; >> bool copied_tables = false; >> struct device *dev; >> + struct pci_dev *pdev; >> struct intel_iommu *iommu; >> int i, ret; >> @@ -3469,7 +3480,11 @@ static int __init init_dmars(void) >> } >> } >> - iommu_prepare_isa(); >> + pdev = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL); >> + if (pdev) { >> + iommu_prepare_isa(pdev); >> + pci_dev_put(pdev); >> + } >> domains_done: >> @@ -5266,6 +5281,7 @@ static void intel_iommu_get_resv_regions(struct >> device *device, >> struct iommu_resv_region *reg; >> struct dmar_rmrr_unit *rmrr; >> struct device *i_dev; >> + struct pci_dev *pdev; >> int i; >> rcu_read_lock(); >> @@ -5280,6 +5296,14 @@ static void intel_iommu_get_resv_regions(struct >> device *device, >> } >> rcu_read_unlock(); >> + if (dev_is_pci(device)) { >> + pdev = to_pci_dev(device); >> + if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) { >> + reg = iommu_get_isa_resv_region(); >> + list_add_tail(®->list, head); >> + } >> + } >> + > > Just wondering why not just > > +#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA > + if (dev_is_pci(device)) { > + pdev = to_pci_dev(device); > + if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) { > + reg = iommu_alloc_resv_region(0, > + 16*1024*1024, > + 0, IOMMU_RESV_DIRECT); > + if (reg) > + list_add_tail(®->list, head); > + } > + } > +#endif > > and, remove all other related code?
At this point in the patchset if we remove iommu_prepare_isa then the ISA region won’t be mapped to the device. Only once the dma domain is allocable will the reserved regions be mapped by iommu_group_create_direct_mappings. Theres an issue that if we choose to alloc a new resv_region with type IOMMU_RESV_DIRECT, we will need to refactor intel_iommu_put_resv_regions to free this entry type which means refactoring the rmrr regions in get_resv_regions. Should this work be in this patchset? > > Best regards, > Lu Baolu Cheers, James. _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
