From: Lu Baolu <baolu.lu@linux.intel.com>
To: James Sewart <jamessewart@arista.com>
Cc: baolu.lu@linux.intel.com, iommu@lists.linux-foundation.org,
Tom Murphy <tmurphy@arista.com>, Dmitry Safonov <dima@arista.com>,
Jacob Pan <jacob.jun.pan@linux.intel.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 3/7] iommu/vt-d: Expose ISA direct mapping region via iommu_get_resv_regions
Date: Mon, 25 Mar 2019 10:03:18 +0800 [thread overview]
Message-ID: <fb1565dc-d991-acf9-e275-c0090f69f47e@linux.intel.com> (raw)
In-Reply-To: <A489539F-4DD4-438A-AD9A-1A2F50404DB9@arista.com>
Hi James,
On 3/22/19 5:57 PM, James Sewart wrote:
> 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(®->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.
Yes. So if we put the allocation code here, it won't impact anything and
will take effect as soon as the dma domain is allocatable.
>
> 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?
Do you mean the rmrr regions are not allocated in get_resv_regions, but
are freed in put_resv_regions? I think we should fix this in this patch
set since this might impact the device passthrough if we don't do it.
Best regards,
Lu Baolu
next prev parent reply other threads:[~2019-03-25 2:09 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-04 15:41 [PATCH 0/4] iommu/vt-d: Fix-up device-domain relationship by refactoring to use iommu group default domain James Sewart
2019-03-04 15:45 ` [PATCH 1/4] iommu: Move iommu_group_create_direct_mappings to after device_attach James Sewart
2019-03-04 15:46 ` [PATCH 2/4] iommu/vt-d: Implement apply_resv_region for reserving IOVA ranges James Sewart
2019-03-04 15:46 ` [PATCH 3/4] iommu/vt-d: Allow IOMMU_DOMAIN_DMA and IOMMU_DOMAIN_IDENTITY to be allocated James Sewart
2019-03-04 15:47 ` [PATCH 4/4] iommu/vt-d: Remove lazy allocation of domains James Sewart
2019-03-05 6:59 ` Lu Baolu
2019-03-05 11:46 ` James Sewart
2019-03-06 7:00 ` Lu Baolu
2019-03-06 18:08 ` James Sewart
2019-03-07 6:31 ` Lu Baolu
2019-03-07 10:21 ` James Sewart
2019-03-08 1:09 ` Lu Baolu
2019-03-08 3:09 ` Lu Baolu
2019-03-08 16:57 ` James Sewart
2019-03-09 1:53 ` Lu Baolu
2019-03-09 11:49 ` James Sewart
2019-03-10 2:51 ` Lu Baolu
2019-03-05 6:46 ` [PATCH 3/4] iommu/vt-d: Allow IOMMU_DOMAIN_DMA and IOMMU_DOMAIN_IDENTITY to be allocated Lu Baolu
2019-03-05 11:34 ` James Sewart
2019-03-08 1:20 ` Dmitry Safonov
2019-03-09 11:57 ` James Sewart
2019-03-05 6:05 ` [PATCH 0/4] iommu/vt-d: Fix-up device-domain relationship by refactoring to use iommu group default domain Lu Baolu
2019-03-05 11:14 ` James Sewart
2019-03-06 6:27 ` Lu Baolu
2019-03-14 11:56 ` [PATCH v2 0/7] " James Sewart
2019-03-14 11:57 ` [PATCH v2 1/7] iommu: Move iommu_group_create_direct_mappings to after device_attach James Sewart
2019-03-14 11:58 ` [PATCH v2 2/7] iommu/vt-d: Implement apply_resv_region for reserving IOVA ranges James Sewart
2019-03-14 11:58 ` [PATCH v2 3/7] iommu/vt-d: Expose ISA direct mapping region via iommu_get_resv_regions James Sewart
2019-03-15 2:19 ` Lu Baolu
2019-03-22 9:57 ` James Sewart
2019-03-25 2:03 ` Lu Baolu [this message]
2019-03-25 12:57 ` James Sewart
2019-03-26 1:10 ` Lu Baolu
2019-03-26 1:24 ` Lu Baolu
2019-03-28 18:37 ` James Sewart
2019-03-29 15:26 ` James Sewart
2019-04-04 6:49 ` Lu Baolu
2019-04-05 18:02 ` James Sewart
2019-04-08 2:43 ` Lu Baolu
2019-04-10 5:22 ` Lu Baolu
2019-04-15 14:16 ` James Sewart
2019-04-16 2:18 ` Lu Baolu
2019-04-24 23:47 ` Tom Murphy
2019-04-25 1:15 ` Lu Baolu
2019-03-14 11:58 ` [PATCH v2 4/7] iommu/vt-d: Ignore domain parameter in attach_device if device requires identity map James Sewart
2019-03-15 2:30 ` Lu Baolu
2019-03-14 11:58 ` [PATCH v2 5/7] iommu/vt-d: Enable DMA remapping after rmrr mapped James Sewart
2019-03-14 11:59 ` [PATCH v2 6/7] iommu/vt-d: Allow IOMMU_DOMAIN_DMA to be allocated by iommu_ops James Sewart
2019-03-14 11:59 ` [PATCH v2 7/7] iommu/vt-d: Remove lazy allocation of domains James Sewart
2019-03-14 23:35 ` Jacob Pan
2019-03-22 10:07 ` James Sewart
2019-03-15 3:13 ` [PATCH v2 0/7] iommu/vt-d: Fix-up device-domain relationship by refactoring to use iommu group default domain Lu Baolu
2019-03-19 13:35 ` James Sewart
2019-03-20 1:26 ` Lu Baolu
2019-03-22 10:05 ` James Sewart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fb1565dc-d991-acf9-e275-c0090f69f47e@linux.intel.com \
--to=baolu.lu@linux.intel.com \
--cc=dima@arista.com \
--cc=iommu@lists.linux-foundation.org \
--cc=jacob.jun.pan@linux.intel.com \
--cc=jamessewart@arista.com \
--cc=linux-kernel@vger.kernel.org \
--cc=tmurphy@arista.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).