From: Robin Murphy <robin.murphy@arm.com> To: Auger Eric <eric.auger@redhat.com>, eric.auger.pro@gmail.com, christoffer.dall@linaro.org, marc.zyngier@arm.com, alex.williamson@redhat.com, will.deacon@arm.com, joro@8bytes.org, tglx@linutronix.de, jason@lakedaemon.net, linux-arm-kernel@lists.infradead.org Cc: drjones@redhat.com, kvm@vger.kernel.org, punit.agrawal@arm.com, linux-kernel@vger.kernel.org, diana.craciun@nxp.com, iommu@lists.linux-foundation.org, pranav.sawargaonkar@gmail.com Subject: Re: [RFC v2 4/8] iommu: Add a list of iommu_reserved_region in iommu_domain Date: Thu, 10 Nov 2016 11:54:39 +0000 [thread overview] Message-ID: <6851a74a-775a-bd20-cdea-4cf06f5f0289@arm.com> (raw) In-Reply-To: <fd35f7e3-6d97-7e67-a5c5-bb633ce0e8fd@redhat.com> Hi Eric, On 10/11/16 11:22, Auger Eric wrote: > Hi Robin, > > On 04/11/2016 15:00, Robin Murphy wrote: >> Hi Eric, >> >> Thanks for posting this new series - the bottom-up approach is a lot >> easier to reason about :) >> >> On 04/11/16 11:24, Eric Auger wrote: >>> Introduce a new iommu_reserved_region struct. This embodies >>> an IOVA reserved region that cannot be used along with the IOMMU >>> API. The list is protected by a dedicated mutex. >> >> In the light of these patches, I think I'm settling into agreement that >> the iommu_domain is the sweet spot for accessing this information - the >> underlying magic address ranges might be properties of various bits of >> hardware many of which aren't the IOMMU itself, but they only start to >> matter at the point you start wanting to use an IOMMU domain at the >> higher level. Therefore, having a callback in the domain ops to pull >> everything together fits rather neatly. > Using get_dm_regions could have make sense but this approach now is > ruled out by sysfs API approach. If attribute file is bound to be used > before iommu domains are created, we cannot rely on any iommu_domain > based callback. Back to square 1? I think it's still OK. The thing about these reserved regions is that as a property of the underlying hardware they must be common to any domain for a given group, therefore without loss of generality we can simply query group->domain->ops->get_dm_regions(), and can expect the reserved ones will be the same regardless of what domain that points to (identity-mapped IVMD/RMRR/etc. regions may not be, but we'd be filtering those out anyway). The default DMA domains need this information too, and since those are allocated at group creation, group->domain should always be non-NULL and interrogable. Plus, the groups are already there in sysfs, and, being representative of device topology, would seem to be an ideal place to expose the addressing limitations relevant to the devices within them. This really feels like it's all falling into place (on the kernel end, at least, I'm sticking to the sidelines on the userspace discussion ;)). Robin. > > Thanks > > Eric >> >>> >>> An iommu domain now owns a list of those. >>> >>> Signed-off-by: Eric Auger <eric.auger@redhat.com> >>> >>> --- >>> --- >>> drivers/iommu/iommu.c | 2 ++ >>> include/linux/iommu.h | 17 +++++++++++++++++ >>> 2 files changed, 19 insertions(+) >>> >>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >>> index 9a2f196..0af07492 100644 >>> --- a/drivers/iommu/iommu.c >>> +++ b/drivers/iommu/iommu.c >>> @@ -1061,6 +1061,8 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus, >>> >>> domain->ops = bus->iommu_ops; >>> domain->type = type; >>> + INIT_LIST_HEAD(&domain->reserved_regions); >>> + mutex_init(&domain->resv_mutex); >>> /* Assume all sizes by default; the driver may override this later */ >>> domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap; >>> >>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h >>> index 436dc21..0f2eb64 100644 >>> --- a/include/linux/iommu.h >>> +++ b/include/linux/iommu.h >>> @@ -84,6 +84,8 @@ struct iommu_domain { >>> void *handler_token; >>> struct iommu_domain_geometry geometry; >>> void *iova_cookie; >>> + struct list_head reserved_regions; >>> + struct mutex resv_mutex; /* protects the reserved region list */ >>> }; >>> >>> enum iommu_cap { >>> @@ -131,6 +133,21 @@ struct iommu_dm_region { >>> int prot; >>> }; >>> >>> +/** >>> + * struct iommu_reserved_region - descriptor for a reserved iova region >>> + * @list: Linked list pointers >>> + * @start: IOVA base address of the region >>> + * @length: Length of the region in bytes >>> + */ >>> +struct iommu_reserved_region { >>> + struct list_head list; >>> + dma_addr_t start; >>> + size_t length; >>> +}; >> >> Looking at this in context with the dm_region above, though, I come to >> the surprising realisation that these *are* dm_regions, even at the >> fundamental level - on the one hand you've got physical addresses which >> can't be remapped (because something is already using them), while on >> the other you've got physical addresses which can't be remapped (because >> the IOMMU is incapable). In fact for reserved regions *other* than our >> faked-up MSI region there's no harm if the IOMMU were to actually >> identity-map them. >> >> Let's just add this to the existing infrastructure, either with some >> kind of IOMMU_NOMAP flag or simply prot = 0. That way it automatically >> gets shared between the VFIO and DMA cases for free! >> >> Robin. >> >>> + >>> +#define iommu_reserved_region_for_each(resv, d) \ >>> + list_for_each_entry(resv, &(d)->reserved_regions, list) >>> + >>> #ifdef CONFIG_IOMMU_API >>> >>> /** >>> >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >>
WARNING: multiple messages have this Message-ID (diff)
From: robin.murphy@arm.com (Robin Murphy) To: linux-arm-kernel@lists.infradead.org Subject: [RFC v2 4/8] iommu: Add a list of iommu_reserved_region in iommu_domain Date: Thu, 10 Nov 2016 11:54:39 +0000 [thread overview] Message-ID: <6851a74a-775a-bd20-cdea-4cf06f5f0289@arm.com> (raw) In-Reply-To: <fd35f7e3-6d97-7e67-a5c5-bb633ce0e8fd@redhat.com> Hi Eric, On 10/11/16 11:22, Auger Eric wrote: > Hi Robin, > > On 04/11/2016 15:00, Robin Murphy wrote: >> Hi Eric, >> >> Thanks for posting this new series - the bottom-up approach is a lot >> easier to reason about :) >> >> On 04/11/16 11:24, Eric Auger wrote: >>> Introduce a new iommu_reserved_region struct. This embodies >>> an IOVA reserved region that cannot be used along with the IOMMU >>> API. The list is protected by a dedicated mutex. >> >> In the light of these patches, I think I'm settling into agreement that >> the iommu_domain is the sweet spot for accessing this information - the >> underlying magic address ranges might be properties of various bits of >> hardware many of which aren't the IOMMU itself, but they only start to >> matter at the point you start wanting to use an IOMMU domain at the >> higher level. Therefore, having a callback in the domain ops to pull >> everything together fits rather neatly. > Using get_dm_regions could have make sense but this approach now is > ruled out by sysfs API approach. If attribute file is bound to be used > before iommu domains are created, we cannot rely on any iommu_domain > based callback. Back to square 1? I think it's still OK. The thing about these reserved regions is that as a property of the underlying hardware they must be common to any domain for a given group, therefore without loss of generality we can simply query group->domain->ops->get_dm_regions(), and can expect the reserved ones will be the same regardless of what domain that points to (identity-mapped IVMD/RMRR/etc. regions may not be, but we'd be filtering those out anyway). The default DMA domains need this information too, and since those are allocated at group creation, group->domain should always be non-NULL and interrogable. Plus, the groups are already there in sysfs, and, being representative of device topology, would seem to be an ideal place to expose the addressing limitations relevant to the devices within them. This really feels like it's all falling into place (on the kernel end, at least, I'm sticking to the sidelines on the userspace discussion ;)). Robin. > > Thanks > > Eric >> >>> >>> An iommu domain now owns a list of those. >>> >>> Signed-off-by: Eric Auger <eric.auger@redhat.com> >>> >>> --- >>> --- >>> drivers/iommu/iommu.c | 2 ++ >>> include/linux/iommu.h | 17 +++++++++++++++++ >>> 2 files changed, 19 insertions(+) >>> >>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >>> index 9a2f196..0af07492 100644 >>> --- a/drivers/iommu/iommu.c >>> +++ b/drivers/iommu/iommu.c >>> @@ -1061,6 +1061,8 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus, >>> >>> domain->ops = bus->iommu_ops; >>> domain->type = type; >>> + INIT_LIST_HEAD(&domain->reserved_regions); >>> + mutex_init(&domain->resv_mutex); >>> /* Assume all sizes by default; the driver may override this later */ >>> domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap; >>> >>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h >>> index 436dc21..0f2eb64 100644 >>> --- a/include/linux/iommu.h >>> +++ b/include/linux/iommu.h >>> @@ -84,6 +84,8 @@ struct iommu_domain { >>> void *handler_token; >>> struct iommu_domain_geometry geometry; >>> void *iova_cookie; >>> + struct list_head reserved_regions; >>> + struct mutex resv_mutex; /* protects the reserved region list */ >>> }; >>> >>> enum iommu_cap { >>> @@ -131,6 +133,21 @@ struct iommu_dm_region { >>> int prot; >>> }; >>> >>> +/** >>> + * struct iommu_reserved_region - descriptor for a reserved iova region >>> + * @list: Linked list pointers >>> + * @start: IOVA base address of the region >>> + * @length: Length of the region in bytes >>> + */ >>> +struct iommu_reserved_region { >>> + struct list_head list; >>> + dma_addr_t start; >>> + size_t length; >>> +}; >> >> Looking at this in context with the dm_region above, though, I come to >> the surprising realisation that these *are* dm_regions, even at the >> fundamental level - on the one hand you've got physical addresses which >> can't be remapped (because something is already using them), while on >> the other you've got physical addresses which can't be remapped (because >> the IOMMU is incapable). In fact for reserved regions *other* than our >> faked-up MSI region there's no harm if the IOMMU were to actually >> identity-map them. >> >> Let's just add this to the existing infrastructure, either with some >> kind of IOMMU_NOMAP flag or simply prot = 0. That way it automatically >> gets shared between the VFIO and DMA cases for free! >> >> Robin. >> >>> + >>> +#define iommu_reserved_region_for_each(resv, d) \ >>> + list_for_each_entry(resv, &(d)->reserved_regions, list) >>> + >>> #ifdef CONFIG_IOMMU_API >>> >>> /** >>> >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel at lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >>
next prev parent reply other threads:[~2016-11-10 11:54 UTC|newest] Thread overview: 91+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-11-04 11:23 [RFC v2 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 - Alt II Eric Auger 2016-11-04 11:23 ` Eric Auger 2016-11-04 11:23 ` Eric Auger 2016-11-04 11:23 ` [RFC v2 1/8] vfio: fix vfio_info_cap_add/shift Eric Auger 2016-11-04 11:23 ` Eric Auger 2016-11-04 11:24 ` [RFC v2 2/8] iommu/iova: fix __alloc_and_insert_iova_range Eric Auger 2016-11-04 11:24 ` Eric Auger 2016-11-10 15:22 ` Joerg Roedel 2016-11-10 15:22 ` Joerg Roedel 2016-11-10 15:22 ` Joerg Roedel 2016-11-10 15:41 ` Auger Eric 2016-11-10 15:41 ` Auger Eric 2016-11-04 11:24 ` [RFC v2 3/8] iommu/dma: Allow MSI-only cookies Eric Auger 2016-11-14 12:36 ` Robin Murphy 2016-11-14 12:36 ` Robin Murphy 2016-11-14 12:36 ` Robin Murphy 2016-11-14 23:23 ` Auger Eric 2016-11-14 23:23 ` Auger Eric 2016-11-14 23:23 ` Auger Eric 2016-11-15 14:52 ` Robin Murphy 2016-11-15 14:52 ` Robin Murphy 2016-11-04 11:24 ` [RFC v2 4/8] iommu: Add a list of iommu_reserved_region in iommu_domain Eric Auger 2016-11-04 11:24 ` Eric Auger 2016-11-04 14:00 ` Robin Murphy 2016-11-04 14:00 ` Robin Murphy 2016-11-04 14:00 ` Robin Murphy 2016-11-10 11:22 ` Auger Eric 2016-11-10 11:22 ` Auger Eric 2016-11-10 11:54 ` Robin Murphy [this message] 2016-11-10 11:54 ` Robin Murphy 2016-11-10 12:14 ` Auger Eric 2016-11-10 12:14 ` Auger Eric 2016-11-10 12:48 ` Robin Murphy 2016-11-10 12:48 ` Robin Murphy 2016-11-10 12:48 ` Robin Murphy 2016-11-10 15:37 ` Joerg Roedel 2016-11-10 15:37 ` Joerg Roedel 2016-11-10 15:37 ` Joerg Roedel 2016-11-10 15:42 ` Auger Eric 2016-11-10 15:42 ` Auger Eric 2016-11-04 11:24 ` [RFC v2 5/8] vfio/type1: Introduce RESV_IOVA_RANGE capability Eric Auger 2016-11-04 11:24 ` Eric Auger 2016-11-04 11:24 ` [RFC v2 6/8] iommu: Handle the list of reserved regions Eric Auger 2016-11-04 11:24 ` [RFC v2 7/8] iommu/vt-d: Implement add_reserved_regions callback Eric Auger 2016-11-04 11:24 ` Eric Auger 2016-11-04 11:24 ` [RFC v2 8/8] iommu/arm-smmu: implement " Eric Auger 2016-11-04 11:24 ` Eric Auger 2016-11-04 14:16 ` Robin Murphy 2016-11-04 14:16 ` Robin Murphy 2016-11-10 15:46 ` Joerg Roedel 2016-11-10 15:46 ` Joerg Roedel 2016-11-10 15:46 ` Joerg Roedel 2016-11-10 15:57 ` Auger Eric 2016-11-10 15:57 ` Auger Eric 2016-11-10 15:57 ` Auger Eric 2016-11-10 16:13 ` Joerg Roedel 2016-11-10 16:13 ` Joerg Roedel 2016-11-10 18:00 ` Auger Eric 2016-11-10 18:00 ` Auger Eric 2016-11-10 18:00 ` Auger Eric 2016-11-11 11:42 ` Joerg Roedel 2016-11-11 11:42 ` Joerg Roedel 2016-11-11 11:42 ` Joerg Roedel 2016-11-11 15:47 ` Auger Eric 2016-11-11 15:47 ` Auger Eric 2016-11-11 16:22 ` Joerg Roedel 2016-11-11 16:22 ` Joerg Roedel 2016-11-11 16:45 ` Auger Eric 2016-11-11 16:45 ` Auger Eric 2016-11-11 16:45 ` Auger Eric 2016-11-14 15:31 ` Joerg Roedel 2016-11-14 15:31 ` Joerg Roedel 2016-11-14 16:08 ` Auger Eric 2016-11-14 16:08 ` Auger Eric 2016-11-14 16:20 ` Joerg Roedel 2016-11-14 16:20 ` Joerg Roedel 2016-11-14 16:20 ` Joerg Roedel 2016-11-14 16:57 ` Auger Eric 2016-11-14 16:57 ` Auger Eric 2016-11-10 16:07 ` Robin Murphy 2016-11-10 16:07 ` Robin Murphy 2016-11-10 16:07 ` Robin Murphy 2016-11-10 16:16 ` Joerg Roedel 2016-11-10 16:16 ` Joerg Roedel 2016-11-10 16:16 ` Joerg Roedel 2016-11-11 14:34 ` Robin Murphy 2016-11-11 14:34 ` Robin Murphy 2016-11-11 14:34 ` Robin Murphy 2016-11-11 15:03 ` Joerg Roedel 2016-11-11 15:03 ` Joerg Roedel 2016-11-11 15:03 ` Joerg Roedel
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=6851a74a-775a-bd20-cdea-4cf06f5f0289@arm.com \ --to=robin.murphy@arm.com \ --cc=alex.williamson@redhat.com \ --cc=christoffer.dall@linaro.org \ --cc=diana.craciun@nxp.com \ --cc=drjones@redhat.com \ --cc=eric.auger.pro@gmail.com \ --cc=eric.auger@redhat.com \ --cc=iommu@lists.linux-foundation.org \ --cc=jason@lakedaemon.net \ --cc=joro@8bytes.org \ --cc=kvm@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=marc.zyngier@arm.com \ --cc=pranav.sawargaonkar@gmail.com \ --cc=punit.agrawal@arm.com \ --cc=tglx@linutronix.de \ --cc=will.deacon@arm.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.