lkml.org 
[lkml]   [2019]   [Mar]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 3/7] iommu/vt-d: Expose ISA direct mapping region via iommu_get_resv_regions
From
Date
Hey Lu,

> On 15 Mar 2019, at 02:19, Lu Baolu <baolu.lu@linux.intel.com> 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 <jamessewart@arista.com>
>> ---
>> 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(&reg->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(&reg->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.

\
 
 \ /
  Last update: 2019-03-22 10:57    [W:0.086 / U:0.216 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site