arm: msm: Add System MMU support.
Subsystems (4)
PATCH 1/2 arm: msm: Add System MMU support.
Add support for the System MMUs found on the 8x60 and 8x72 families of Qualcomm chips. These SMMUs allow virtualization of the address space used by most of the multimedia cores on these chips.
--- arch/arm/mach-msm/include/mach/smmu_driver.h | 322 +++++ arch/arm/mach-msm/include/mach/smmu_hw-8xxx.h | 1860 +++++++++++++++++++++++++ arch/arm/mach-msm/smmu_driver.c | 834 +++++++++++ 3 files changed, 3016 insertions(+), 0 deletions(-) create mode 100644 arch/arm/mach-msm/include/mach/smmu_driver.h create mode 100644 arch/arm/mach-msm/include/mach/smmu_hw-8xxx.h create mode 100644 arch/arm/mach-msm/smmu_driver.c
arch/arm/mach-msm/include/mach/smmu_driver.h new file +322 -0
@@ -0,0 +1,322 @@ +/* Copyright (c) 2010, Code Aurora Forum. All rights reserved. +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions are +* met: +* * Redistributions of source code must retain the above copyright +* notice, this list of conditions and the following disclaimer. +* * Redistributions in binary form must reproduce the above +* copyright notice, this list of conditions and the following +* disclaimer in the documentation and/or other materials provided +* with the distribution. +* * Neither the name of Code Aurora Forum, Inc. nor the names of its +* contributors may be used to endorse or promote products derived +* from this software without specific prior written permission. +* +* THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED +* WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF +* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT +* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS +* BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR +* BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, +* WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE +* OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN +* IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +* +*/ + +#ifndef SMMU_DRIVER_H +#define SMMU_DRIVER_H + +#include <linux/list.h> + +#define FL_OFFSET (va) (((va) & 0xFFF00000) >> 20) +#define SL_OFFSET (va) (((va) & 0xFF000) >> 12) + +#define NUM_FL_PTE 4096 +#define NUM_SL_PTE 256 + +/** +* struct smmu_driver - A single SMMU hardware instance +* @base: SMMU config port base address +* @irq: Interrupt number +* @list_active: List of activated context structures on this SMMU +* +* A smmu_driver holds the global information about a single piece +* of SMMU hardware. +* +*/ +struct smmu_driver { +/* Public */ +unsigned long base; +int irq; + +/* Private */ +struct list_head list_active; +}; + +/** +* smmu_handler - The signature of the fault hook. +* @dev_id: The device id of the faulting device. +* @data: The generic data pointer. +* @fault_data: System specific common fault data. +* +* The handler should return 0 for success. This indicates that the +* fault was handled. A non-zero return value is an error and will be +* propagated up the stack. +*/ +typedef int (*smmu_handler )(size_t dev_id,void *data,void *fault_data); + +/** +* struct smmu_dev - A single SMMU context instance +* @drv: Pointer to hardware instance for this context +* @context: The context number (index) for this context +* @base: Config port base address of SMMU +* @fl_table: First-level page table VA for this mapping +* @smmu_interrupt_handler: Interrupt handler for faults in this context +* @smmu_interrupt_handler_data: Private data for smmu_interrupt_handler +* @dev_elm: List of all contexts on this SMMU instance +* +* A smmu_dev holds the mapping-specific information associated with each +* context of the SMMU. +* +*/ +struct smmu_dev { +/* Public */ +unsigned long base; +int context; + +/* Private */ +smmu_handler smmu_interrupt_handler; +void *smmu_interrupt_handler_data; +unsigned long *fl_table; +struct list_head dev_elm; +struct smmu_driver *drv; +}; + +void v7_flush_kern_cache_all (void ); + +/** +* smmu_drvdata_init() - Initialize a SMMU hardware instance +* @drv: Pointer to a newly-allocated smmu_driver structure that is to +* be populated with information regarding this HW instance +* @base: The base address of the SMMU's config port +* @irq: The interrupt number associated with this SMMU's context +* interrupt #0. +* +* Initialize an SMMU hardware instance and the handle to it. +* +* The return value is zero on success and non-zero on failure. +*/ +int smmu_drvdata_init (struct smmu_driver *drv,unsigned long base,int irq); + + +/** +* smmu_ctx_init() - Initialize an SMMU context +* @ctx: The context number (index) to initialize +* +* Create a handle to an SMMU context. The SMMU must have already been +* initialized using smmu_drvdata_init and the context number must be a valid +* context index on that particular SMMU hardware instance. +* +* The return value is a pointer to a smmu_dev structure, which represents +* a handle to this context. The return value will be NULL if the memory could +* not be allocated or if the parameters were not valid. +*/ +struct smmu_dev *smmu_ctx_init (int ctx); + + +/** +* smmu_ctx_bind() - associate a smmu_dev with a specific SMMU HW instance +* @ctx: The context instance to bind to the HW instance +* @drv: The HW instance to which the context instance is to be bound +* +* Associate the context instance structure with a particular hardware instance +* The parameters must be valid pointers to their respective structures and the +* specific context instance must not have already been bound to a DIFFERENT +* hardware instance. The context instance must have been initialized with a +* valid context number for the given hardware instance. +* +* The return value is zero on success, -EBUSY if the context has been already +* bound, or -EINVAL for bad input. +*/ +int smmu_ctx_bind (struct smmu_dev *ctx,struct smmu_driver *drv); + + +/** +* smmu_ctx_deinit() - Tear down an SMMU context +* @dev: The smmu_dev to tear down +* +* Tears down the SMMU context and frees the associated memory. The context +* mapping must have already been deactivated (if it had been activated prior) +* using smmu_deactivate(). +* +* The return value is 0 if the context was successfully deinitialized or +* nonzero if the operation could not be completed. The memory allocated for +* the smmu_dev structure will be freed and will no longer be valid after this +* function returns 0. Error return values are -EINVAL for bad input or -EBUSY +* if mappings still exist in that context. +*/ +int smmu_ctx_deinit (struct smmu_dev *dev); + + + +/** +* smmu_activate() - Activates a context mapping +* @dev: The context to activate +* +* Enables the mapping associated with this context. After this function returns +* successfully, the hardware requests associated with this SMMU instance and +* this context will be translated according to the mapping specified for this +* context. +* +* Returns 0 if the activation was successful or -EINVAL for bad input. +*/ +int smmu_activate (struct smmu_dev *dev); + + +/** +* smmu_deactivate() - Deactivates a context mapping +* @dev: The context to deactivate +* +* Disables the mapping associated with this context. Hawrdware requests +* associated with this SMMU instance and this context will effectively +* bypass the SMMU. +* +* Returns 0 if the deactivation was successful, -ENODEV for a device that +* has not been activated, or -EINVAL for bad input. +*/ +int smmu_deactivate (struct smmu_dev *dev); + + +/** +* smmu_is_active() - Tests if a context mapping is activated +* @dev: The smmu_dev structure to check +* +* Returns >0 if the mapping is activated, 0 if it is not activated, and +* -EINVAL if the supplied smmu_dev is not a valid context instance. +*/ +int smmu_is_active (struct smmu_dev *dev); + + +/** +* smmu_update_start() - Prepare to update mappings +* @dev: The context instance whose mappings will be updated +* +* This function must be called prior to making any smmu_map or smmu_unmap calls +* It is an error to allow a SMMU client to issue bus requests while mappings +* are being updated (that is, if there is an outstanding smmu_update_start +* call without a corresponding smmu_update_done call). The translation result +* in this case is UNPREDICTABLE due to potentially stale TLB entries. +* It is an error to call smmu_map or smmu_unmap unless a new call to +* smmu_update_start (not followed by smmu_update_done) has been made. +* The return value is zero on success and -EINVAL for bad input. +*/ +int smmu_update_start (struct smmu_dev *dev); + + +/** +* smmu_update_done() - Finalize mapping updates +* @dev: The context instance whose mappings have been updated +* +* This function must be called after making any smmu_map or smmu_unmap calls. +* It must correspond to a previous outstanding smmu_update_start call. Note +* that the mapping changes MIGHT take effect before smmu_update_done is called. +* It is a programming error to allow an SMMU client to issue translation +* requests if there is an outstanding call to smmu_update_start without being +* followed by smmu_update_done. +* +* The return value is zero on success or -EINVAL for bad input. +*/ +int smmu_update_done (struct smmu_dev *dev); + + +/** +* smmu_map() - Maps a virtual page into physical space +* @dev: The context to in which to map the page +* @pa: The physical address for the mapping +* @va: The virtual address to map +* @len: The page size (4KB, 64KB, 1MB, or 16MB only). +* @attr: Mapping attributes (cacheability / shareability) +* +* Maps the specified virtual page to the specified physical page. The page +* size must be one of 4KB, 64KB, 1MB, or 16MB. The physical and virtual +* addresses must be aligned to the page size specified. Care must be taken so +* that no hardware requests are made within this context while this function +* runs if the context is active. The function will return an error if the given +* virtual address is already part of a different mapping. +* +* This function may be called multiple times using the same physical and +* virtual addresses (without first calling smmu_unmap), as long as the +* length is UNCHANGED across all such invocations. The attributes may be +* different, in which case the new attributes replace the previous ones. +* +* Returns 0 if mapping was successfully made, -ENOMEM if page tables could +* not be allocated, or -EINVAL for bad input. +*/ +int smmu_map (struct smmu_dev *dev,phys_addr_t pa,unsigned long va,size_t len, +u32 attr); + + +/** +* __smmu_map() - smmu_map() without locking +*/ +int __smmu_map (struct smmu_dev *dev,phys_addr_t pa,unsigned long va, +size_t len,u32 attr); + + +/** +* smmu_unmap() - Removes a page mapping +* @dev: The context to in which to unmap the page +* @va: The virtual address to unmap +* @len: The page size (4KB, 64KB, 1MB, or 16MB only). +* +* Removes the specified page mapping. The virtual address and the length must +* be the same as the va and the length that were passed to smmu_map when this +* mapping had been set up. Care must be taken so that no hardware requests are +* made within this context while this function runs if the context is active. +* +* Returns 0 if mapping was successfully removed or -EINVAL if the parameters +* were not valid, or if the specified mapping had been previously removed. +*/ +int smmu_unmap (struct smmu_dev *dev,unsigned long va,size_t len); + + +/** +* smmu_translate() - Performs a VA to PA translation on a single address +* @dev: The context to in which to perform the translation +* @va: The virtual address to translate +* +* Translates the virtual address to a physical address using the SMMU hardware +* and the mappings that have been configured for this context. The context +* must have been activated before calling this function. If the specified +* virtual address has not been mapped, a user-supplied interrupt handler will +* be called. +* +* Returns the physical address associated with the given virtual address. If +* no relevant mapping exists, an interrupt is raised 0 is immediately +* returned to indicate the error. -ENODEV is returned if the device had not +* been activated, and -EINVAL is returned for bad input. +*/ +phys_addr_t smmu_translate (struct smmu_dev *dev,unsigned long va); + + +/** +* smmu_hook_irpt() - Registers an interrupt handler for this context +* @dev: The context for which to register the interrupt +* @handler: Pointer to the user-supplied interrupt handler +* @data: Data to be supplied to the interrupt handler +* +* Registers an interrupt handler to be called when a fault occurs within the +* current context. Registering a different interrupt handler for the same +* context will result in the new handler replacing the old one. Registering a +* NULL handler will disable the user interrupt handler for this context. +* +* Returns 0 if the interrupt handler was successfully registered or -EINVAL if +* the smmu_dev passed in was not valid. +*/ +int smmu_hook_irpt (struct smmu_dev *dev,smmu_handler handler,void *data); + +#endif /* SMMU_DRIVER_H */
arch/arm/mach-msm/include/mach/smmu_hw-8xxx.h new file +1860 -0
1,860 lines not on this page · raw message
arch/arm/mach-msm/smmu_driver.c new file +834 -0
818 lines not on this page · raw message
The rest of this diff is not shown. View the raw message.
--
1.7.0.2
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
Part of the diff is not shown here. View the raw message.
Quoted 8 lines · Stepan Moskovchenko
+/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above
Quoted 6 lines
+ * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution.+ * * Neither the name of Code Aurora Forum, Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
This should be GPLv2 ..
Daniel
Signature · 2 lines
Sent by an consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
Arnd Bergmann
maintainerQuoted 6 lines · On Wednesday 28 July 2010 00:41:06 Stepan Moskovchenko wrote:
Add support for the System MMUs found on the 8x60 and 8x72 families of Qualcomm chips. These SMMUs allow virtualization of the address space used by most of the multimedia cores on these chips.
How is this different from an IOMMU?
Quoted 1 line
From a very brief look, it seems that you should be using the
existing dma-mapping APIs here instead of making up your own.
Arnd
Quoted 6 lines · On Wednesday 28 July 2010 00:41:06 Stepan Moskovchenko wrote:
Quoted 4 lines
Add support for the System MMUs found on the 8x60 and 8x72 families of Qualcomm chips. These SMMUs allow virtualization of the address space used by most of the multimedia cores on these chips.
Quoted 6 lines
How is this different from an IOMMU?
From a very brief look, it seems that you should be using the existing dma-mapping APIs here instead of making up your own.
Arnd
These are just SMMU APIs, and the DMA-mapping API is one layer above this.
We have our own SMMU API for the MSM SoCs because we have muliple IOMMUs, each one having multiple contexts, or even having multiple instances of the same context. Our usage model is also quite a bit different from how the DMA APIs are set up. I believe only two IOMMU drivers actually make use of the DMA API (Intel and AMD) and the other ones (OMAP and other SoCs) have their own APIs for their specific use cases.
Steve
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
Arnd Bergmann
maintainerQuoted 14 lines · stepanm at codeaurora.org
Quoted 6 lines · On Wednesday 28 July 2010 00:41:06 Stepan Moskovchenko wrote:
Quoted 4 lines
Add support for the System MMUs found on the 8x60 and 8x72 families of Qualcomm chips. These SMMUs allow virtualization of the address space used by most of the multimedia cores on these chips.
Quoted 6 lines
How is this different from an IOMMU?
From a very brief look, it seems that you should be using the existing dma-mapping APIs here instead of making up your own.
Arnd
Quoted 8 lines
These are just SMMU APIs, and the DMA-mapping API is one layer above this.
We have our own SMMU API for the MSM SoCs because we have muliple IOMMUs, each one having multiple contexts, or even having multiple instances of the same context. Our usage model is also quite a bit different from how the DMA APIs are set up. I believe only two IOMMU drivers actually make use of the DMA API (Intel and AMD) and the other ones (OMAP and other SoCs) have their own APIs for their specific use cases.
The DMA API is extremely flexible, it works just fine with all the IOMMUs that I've seen so far. Please take a look at include/asm-generic/dma-mapping-common.h and its users to see how to use multiple IOMMUs depending on the device.
If the OMAP developers got this wrong, that's not your problem :-)
Arnd
Quoted 4 lines · Arnd Bergmann
The DMA API is extremely flexible, it works just fine with all the IOMMUs that I've seen so far. Please take a look at
include/asm-generic/dma-mapping-common.hand its users to see how to use multiple IOMMUs depending on the device.
We don't yet use those DMA API interface extensions because we haven't had the need. If someone who has the need wants to put the effort in though...
One of the problems with it though is the abstraction of the sync* operations is the wrong way around for stuff like dmabounce - we want to be passed the base address of the buffer (so we can look this up), plus offset and length. We don't want to know just the region which is affected.
Quoted 10 lines · On Wed, 28 Jul 2010 22:21:56 +0100 Russell King - ARM Linux…
Quoted 4 lines · Arnd Bergmann
The DMA API is extremely flexible, it works just fine with all the IOMMUs that I've seen so far. Please take a look at
include/asm-generic/dma-mapping-common.hand its users to see how to use multiple IOMMUs depending on the device.We don't yet use those DMA API interface extensions because we haven't had the need. If someone who has the need wants to put the effort in though...
Quoted 5 lines
One of the problems with it though is the abstraction of the sync* operations is the wrong way around for stuff like dmabounce - we want to be passed the base address of the buffer (so we can look this up), plus offset and length. We don't want to know just the region which is affected.
We can't pass the base address because the DMA API callers don't pass the base address for dma_sync_single_for_{device|cpu}.
dma_sync_single_range_for_* requires the base address but they are obsolete.
So you need to fix dmabounce. Actually, I send you a patch to fix dmabounce long ago (looks like not applied yet):
http://kerneltrap.org/mailarchive/linux-netdev/2010/4/5/6274046
Arnd Bergmann
maintainerQuoted 9 lines · On Wednesday 28 July 2010 23:21:56 Russell King - ARM Linux wrote:
Quoted 4 lines · Arnd Bergmann
The DMA API is extremely flexible, it works just fine with all the IOMMUs that I've seen so far. Please take a look at
include/asm-generic/dma-mapping-common.hand its users to see how to use multiple IOMMUs depending on the device.We don't yet use those DMA API interface extensions because we haven't had the need. If someone who has the need wants to put the effort in though...
Right, it shouldn't be hard now that the groundwork for that is done. Also, it's only really needed if you have IOMMUs of different types in the same system. If msm doesn't have any swiotlb or dmabounce devices, it could always use the same implementation for all devices.
Quoted 5 lines
One of the problems with it though is the abstraction of the sync* operations is the wrong way around for stuff like dmabounce - we want to be passed the base address of the buffer (so we can look this up), plus offset and length. We don't want to know just the region which is affected.
Yes, but that is an unrelated (dmabounce specific) problem that seems to be fixed by an existing patch.
The driver posted by Stepan doesn't even support the dma_sync_single_* style operations, and I don't think it can run into that specific problem. Are there even (hardware) IOMMUs that are connected to noncoherent buses? AFAICT, anything that needs to flush a dcache range in dma_sync_* has a trivial mapping between bus and phys addresses.
Arnd
Quoted 16 lines · Arnd Bergmann
Quoted 9 lines · On Wednesday 28 July 2010 23:21:56 Russell King - ARM Linux wrote:
Quoted 4 lines · Arnd Bergmann
The DMA API is extremely flexible, it works just fine with all the IOMMUs that I've seen so far. Please take a look at
include/asm-generic/dma-mapping-common.hand its users to see how to use multiple IOMMUs depending on the device.We don't yet use those DMA API interface extensions because we haven't had the need. If someone who has the need wants to put the effort in though...
Right, it shouldn't be hard now that the groundwork for that is done. Also, it's only really needed if you have IOMMUs of different types in the same system. If msm doesn't have any swiotlb or dmabounce devices, it could always use the same implementation for all devices.
Quoted 8 lines
Quoted 5 lines
One of the problems with it though is the abstraction of the sync* operations is the wrong way around for stuff like dmabounce - we want to be passed the base address of the buffer (so we can look this up), plus offset and length. We don't want to know just the region which is affected.
Yes, but that is an unrelated (dmabounce specific) problem that seems to be fixed by an existing patch.
It's not unrelated because it stands in the way of using that interface. The patch also seems to be buggy in that it doesn't fix the for_device case - it leaves 'off' as zero.
I'm also not sold on this idea that the sync_range API is being obsoleted. It seems to me to be a step in the wrong direction. The range API is a natural subset of the 'normal' sync API, yet people are trying to shoehorn the range API into the 'norma' API. If anything it's the 'normal' API which should be obsoleted as it provides reduced information to implementations, which then have to start fuzzy-matching the passed address.
If we're going to start fuzzy-matching the passed address, then I think we also need to add detection of overlapping mappings and BUG() on such cases - otherwise we risk the possibility of having multiple overlapping mappings and hitting the wrong mapping with this reduced-information sync API.
Quoted 4 lines
The driver posted by Stepan doesn't even support the dma_sync_single_* style operations, and I don't think it can run into that specific problem. Are there even (hardware) IOMMUs that are connected to noncoherent buses?
Yes. Virtually all ARM systems have non-cache coherent DMA. Doesn't matter if there's an IOMMU or not.
Quoted 23 lines · On Thu, 29 Jul 2010 12:47:26 +0100 Russell King - ARM Linux…
Quoted 22 lines · Arnd Bergmann
Quoted 9 lines · On Wednesday 28 July 2010 23:21:56 Russell King - ARM Linux wrote:
Quoted 4 lines · Arnd Bergmann
The DMA API is extremely flexible, it works just fine with all the IOMMUs that I've seen so far. Please take a look at
include/asm-generic/dma-mapping-common.hand its users to see how to use multiple IOMMUs depending on the device.We don't yet use those DMA API interface extensions because we haven't had the need. If someone who has the need wants to put the effort in though...
Right, it shouldn't be hard now that the groundwork for that is done. Also, it's only really needed if you have IOMMUs of different types in the same system. If msm doesn't have any swiotlb or dmabounce devices, it could always use the same implementation for all devices.
Quoted 5 lines
One of the problems with it though is the abstraction of the sync* operations is the wrong way around for stuff like dmabounce - we want to be passed the base address of the buffer (so we can look this up), plus offset and length. We don't want to know just the region which is affected.
Quoted 6 lines
Quoted 2 lines
Yes, but that is an unrelated (dmabounce specific) problem that seems to be fixed by an existing patch.
It's not unrelated because it stands in the way of using that interface. The patch also seems to be buggy in that it doesn't fix the
for_devicecase - it leaves 'off' as zero.
Ah, sorry about the bug. Surely, the for_device needs to do the same as the for_cpu. I've attached the updated patch.
We need to fix dmabounce.c anyway (even if we keep the sync_range API) because drivers use the sync API to do a partial sync.
Quoted 7 lines
I'm also not sold on this idea that the
sync_rangeAPI is being obsoleted. It seems to me to be a step in the wrong direction. The range API is a natural subset of the 'normal' sync API, yet people are trying to shoehorn the range API into the 'norma' API. If anything it's the 'normal' API which should be obsoleted as it provides reduced information to implementations, which then have to start fuzzy-matching the passed address.
It would have been nice if you had opposed when this issue was discussed...
commit 8127bfc5645db0e050468e0ff971b4081f73ddcf
Author: FUJITA Tomonori <[email protected]> Date: Wed Mar 10 15:23:18 2010 -0800
DMA-API.txt: remove dma_sync_single_range description
As you said, the range API might be safer (since it requires more information). However, there were already drivers using the dma_sync_single_for API to do a partial sync (i.e. do a sync on range).
Inspecting all the usage of the dma_sync_single_for API to see which drivers to do a partial sync looks unrealistic. So keeping the dma_sync_single_range_for API is pointless since drivers keep using dma_sync_single_for API.
And the majority of implementations doesn't use 'range' information, i.e., the implementation of dma_sync_single_for and dma_sync_single_range_for API is identical.
Quoted 5 lines
If we're going to start fuzzy-matching the passed address, then I think we also need to add detection of overlapping mappings and
BUG()on such cases - otherwise we risk the possibility of having multiple overlapping mappings and hitting the wrong mapping with this reduced-information sync API.
Strict checking would be nice. If architectures can do such easily, we had better to do so.
However, I'm not sure we need to take special care for the dma_sync_single_for API. In general, misuse of the majority of the DMA functions is deadly.
=
From: FUJITA Tomonori <[email protected]>
Subject: [PATCH] ARM: dmabounce: fix partial sync in dma_sync_single_* API
Some network drivers do a partial sync with dma_sync_single_for_{device|cpu}. The dma_addr argument might not be the same as one as passed into the mapping API.
This adds some tricks to find_safe_buffer() for
dma_sync_single_for_{device|cpu}.
--- arch/arm/common/dmabounce.c | 32 +++++++++++++++++++++++--------- 1 files changed, 23 insertions(+), 9 deletions(-)
arch/arm/common/dmabounce.c +23 -9
@@ -163,7 +163,8 @@ alloc_safe_buffer(struct dmabounce_device_info *device_info, void *ptr,/* determine if a buffer is from our "safe" pool */ static inline struct safe_buffer * -find_safe_buffer (struct dmabounce_device_info *device_info,dma_addr_t safe_dma_addr) +find_safe_buffer (struct dmabounce_device_info *device_info,dma_addr_t safe_dma_addr, +int for_sync) {struct safe_buffer *b, *rb =NULL ;unsigned long flags;
@@ -171,9 +172,17 @@ find_safe_buffer(struct dmabounce_device_info *device_info, dma_addr_t safe_dma_read_lock_irqsave (&device_info->lock, flags);list_for_each_entry (b, &device_info->safe_buffers, node) -if (b->safe_dma_addr == safe_dma_addr) { - rb = b; -break ; +if (for_sync) { +if (b->safe_dma_addr <= safe_dma_addr && + safe_dma_addr < b->safe_dma_addr + b->size) { + rb = b; +break ; + } + }else { +if (b->safe_dma_addr == safe_dma_addr) { + rb = b; +break ; + } }read_unlock_irqrestore (&device_info->lock, flags);
@@ -205,7 +214,8 @@ free_safe_buffer(struct dmabounce_device_info *device_info, struct safe_buffer */* ************************************************** */ static struct safe_buffer *find_safe_buffer_dev (struct device *dev, -dma_addr_t dma_addr,const char *where) +dma_addr_t dma_addr,const char *where, +int for_sync) {if (!dev || !dev->archdata.dmabounce)return NULL ;
@@ -216,7 +226,7 @@ static struct safe_buffer *find_safe_buffer_dev(struct device *dev,pr_err ("unknown device: Trying to %s invalid mapping\n" , where);return NULL ; } -return find_safe_buffer (dev->archdata.dmabounce, dma_addr); +return find_safe_buffer (dev->archdata.dmabounce, dma_addr, for_sync); } static inline dma_addr_t map_single(struct device *dev, void *ptr, size_t size,
@@ -286,7 +296,7 @@ static inline dma_addr_t map_single(struct device *dev, void *ptr, size_t size,static inline void unmap_single (struct device *dev,dma_addr_t dma_addr,size_t size,enum dma_data_direction dir) { -struct safe_buffer *buf =find_safe_buffer_dev (dev, dma_addr,"unmap" ); +struct safe_buffer *buf =find_safe_buffer_dev (dev, dma_addr,"unmap" ,0 );if (buf) {BUG_ON (buf->size != size);
@@ -398,7 +408,7 @@ int dmabounce_sync_for_cpu(struct device *dev, dma_addr_t addr,dev_dbg (dev,"%s(dma=%#x,off=%#lx,sz=%zx,dir=%x)\n" , __func__, addr, off, sz, dir); - buf =find_safe_buffer_dev (dev, addr, __func__); + buf =find_safe_buffer_dev (dev, addr, __func__,1 );if (!buf)return 1 ;
@@ -411,6 +421,8 @@ int dmabounce_sync_for_cpu(struct device *dev, dma_addr_t addr,DO_STATS (dev->archdata.dmabounce->bounce_count++);if (dir ==DMA_FROM_DEVICE || dir ==DMA_BIDIRECTIONAL ) { +if (addr != buf->safe_dma_addr) + off = addr - buf->safe_dma_addr;dev_dbg (dev,"%s: copy back safe %p to unsafe %p size %d\n" , __func__, buf->safe + off, buf->ptr + off, sz);memcpy (buf->ptr + off, buf->safe + off, sz);
@@ -427,7 +439,7 @@ int dmabounce_sync_for_device(struct device *dev, dma_addr_t addr,dev_dbg (dev,"%s(dma=%#x,off=%#lx,sz=%zx,dir=%x)\n" , __func__, addr, off, sz, dir); - buf =find_safe_buffer_dev (dev, addr, __func__); + buf =find_safe_buffer_dev (dev, addr, __func__,1 );if (!buf)return 1 ;
@@ -440,6 +452,8 @@ int dmabounce_sync_for_device(struct device *dev, dma_addr_t addr,DO_STATS (dev->archdata.dmabounce->bounce_count++);if (dir ==DMA_TO_DEVICE || dir ==DMA_BIDIRECTIONAL ) { +if (addr != buf->safe_dma_addr) + off = addr - buf->safe_dma_addr;dev_dbg (dev,"%s: copy out unsafe %p to safe %p, size %d\n" , __func__,buf->ptr + off, buf->safe + off, sz);memcpy (buf->safe + off, buf->ptr + off, sz);
Signature · 1 line
1.6.5
Quoted 26 lines · On Wednesday 28 July 2010, [email protected] wrote:
Quoted 17 lines
Quoted 6 lines · On Wednesday 28 July 2010 00:41:06 Stepan Moskovchenko wrote:
Quoted 4 lines
Add support for the System MMUs found on the 8x60 and 8x72 families of Qualcomm chips. These SMMUs allow virtualization of the address space used by most of the multimedia cores on these chips.
Quoted 6 lines
How is this different from an IOMMU?
From a very brief look, it seems that you should be using the existing dma-mapping APIs here instead of making up your own.
Arnd
These are just SMMU APIs, and the DMA-mapping API is one layer above this.
Quoted 7 lines
We have our own SMMU API for the MSM SoCs because we have muliple IOMMUs,
each one having multiple contexts, or even having multiple instances of the same context. Our usage model is also quite a bit different from how the DMA APIs are set up. I believe only two IOMMU drivers actually make use of the DMA API (Intel and AMD) and the other ones (OMAP and other SoCs) have their own APIs for their specific use cases.
Quoted 8 lines
The DMA API is extremely flexible, it works just fine with all the IOMMUs that I've seen so far. Please take a look at
include/asm-generic/dma-mapping-common.hand its users to see how to use multiple IOMMUs depending on the device.If the OMAP developers got this wrong, that's not your problem :-)
Arnd
Hi Arnd,
Quoted 1 line
From what I have been able to tell, the IOMMU interface was written by
AMD/Intel to allow the kvm code to work with a common IOMMU interface. To that end, it isn't really a generic IOMMU interface. We have chosen to use our own interface because it provides us with a lightweight way of managing mappings for more esoteric MSM-specific use cases.
These map functions also take into account the way in which we map buffers that we get from our own physical pool, because the current API was not intended to deal with prioritized allocation of things like on/off-chip memory. We are currently evaluating how to use the DMA API with our own specialized allocator, which has been undergoing some discussion on the other lists. We would like to use this allocator to maximize TLB performance, as well as to prioritize the allocation from several different memory pools.
Steve
Quoted 16 lines · On Wed, 28 Jul 2010 17:58:50 -0700 (PDT) stepanm at codeaurora.org wrote:
Quoted 16 lines
Quoted 10 lines
These are just SMMU APIs, and the DMA-mapping API is one layer above this.
We have our own SMMU API for the MSM SoCs because we have muliple IOMMUs,
each one having multiple contexts, or even having multiple instances of the same context. Our usage model is also quite a bit different from how the DMA APIs are set up. I believe only two IOMMU drivers actually make use of the DMA API (Intel and AMD) and the other ones (OMAP and other SoCs) have their own APIs for their specific use cases.The DMA API is extremely flexible, it works just fine with all the IOMMUs that I've seen so far. Please take a look at
include/asm-generic/dma-mapping-common.hand its users to see how to use multiple IOMMUs depending on the device.
Quoted 8 lines
Quoted 3 lines
If the OMAP developers got this wrong, that's not your problem :-)
Arnd
Hi Arnd,
From what I have been able to tell, the IOMMU interface was written by AMD/Intel to allow the kvm code to work with a common IOMMU interface. To
Don't confuse the IOMMU interface with the DMA API that Arnd mentioned.
They are not related at all.
The DMA API is defined in Documentation/DMA-API.txt.
Arnd told you that include/asm-generic/dma-mapping-common.h is the library to support the DMA API with multiple IOMMUs. Lots of architectures (x86, powerpc, sh, alpha, ia64, microblaze, sparc) use it.
Arnd Bergmann
maintainerQuoted 8 lines · On Thursday 29 July 2010 05:35:48 FUJITA Tomonori wrote:
Quoted 3 lines
From what I have been able to tell, the IOMMU interface was written by AMD/Intel to allow the kvm code to work with a common IOMMU interface. To
Don't confuse the IOMMU interface with the DMA API that Arnd mentioned.
They are not related at all.
Exactly, thanks for the clarification. I also didn't realize that there is now an include/linux/iommu.h file that only describes the PCI SR-IOV interfaces, unlike the generic IOMMU support that we have in your include/linux/dma-mapping.h file.
Maybe we should rename linux/iommu.h to something more specific so we can reduce this confusion in the future.
Arnd
Quoted 10 lines · On Thu, 29 Jul 2010 10:26:55 +0200 Arnd Bergmann <[email protected]> wrote:
Quoted 8 lines · On Thursday 29 July 2010 05:35:48 FUJITA Tomonori wrote:
Quoted 3 lines
From what I have been able to tell, the IOMMU interface was written by AMD/Intel to allow the kvm code to work with a common IOMMU interface. To
Don't confuse the IOMMU interface with the DMA API that Arnd mentioned.
They are not related at all.
Quoted 7 lines
Exactly, thanks for the clarification. I also didn't realize that there is now an
include/linux/iommu.hfile that only describes the PCI SR-IOV interfaces, unlike the generic IOMMU support that we have in yourinclude/linux/dma-mapping.hfile.Maybe we should rename
linux/iommu.hto something more specific so we can reduce this confusion in the future.
I also said that linux/iommu.h was a bad name when it was introduced :) Well, the author disagreed. Now the name actually confuse developers. The author might rethink.
Quoted 9 lines · Arnd Bergmann
Quoted 8 lines · On Thursday 29 July 2010 05:35:48 FUJITA Tomonori wrote:
Quoted 3 lines
From what I have been able to tell, the IOMMU interface was written by AMD/Intel to allow the kvm code to work with a common IOMMU interface. To
Don't confuse the IOMMU interface with the DMA API that Arnd mentioned.
They are not related at all.
Thats not 100% true. They are not strictly related, but they are related as they may use the same backend kernel drivers to provide their functionality. Both APIs exist for different purposes, of course.
Quoted 4 lines
Exactly, thanks for the clarification. I also didn't realize that there is now an
include/linux/iommu.hfile that only describes the PCI SR-IOV interfaces, unlike the generic IOMMU support that we have in yourinclude/linux/dma-mapping.hfile.
The IOMMU-API is not about SR-IOV. It is about the capabilities of modern IOMMU hardware that we can not provide to the kernel with the DMA-API such as the ability to choose ourself at which io-virtual address a given cpu physical address should be mapped. Also I wouldn't call the DMA-API an IOMMU interface. The API does not depend on an IOMMU which is an important difference to the IOMMU-API. The IOMMU-API is probably not generic enough to handle all kinds of IOMMUs but its closer to a generic IOMMU-API than the DMA-API.
Joerg
Signature · 5 lines
Joerg Roedel - AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Quoted 1 line · Joerg
The IOMMU-API is not about SR-IOV.
That's true. However, the point is that include/iommu.h is far from the IOMMU-API.
You could still insist that include/iommu.h is designed for the generic IOMMU-API. But the fact is that it's designed for very specific purposes. No intention to make it for generic purposes.
Since you added it two years ago, nobody has tried to extend it. Instead, we have something like arch/arm/plat-omap/include/plat/iommu.h.
Quoted 11 lines · FUJITA Tomonori
Quoted 1 line · Joerg
The IOMMU-API is not about SR-IOV.
That's true. However, the point is that
include/iommu.his far from the IOMMU-API.You could still insist that
include/iommu.his designed for the generic IOMMU-API. But the fact is that it's designed for very specific purposes. No intention to make it for generic purposes.
I have no clue about the ARM iommus on the omap-platform. From a quick look into the header file I see some similarities to the IOMMU-API. I am also very open for discussions about how the IOMMU-API could be extended to fit the needs of other platforms. Only because nobody has tried to discuss about such an effort is no reason to push the IOMMU-API back.
Quoted 4 lines
Since you added it two years ago, nobody has tried to extend it. Instead, we have something like
arch/arm/plat-omap/include/plat/iommu.h.
And I think we should try to merge this platform-specific functionality into the IOMMU-API.
Joerg
Signature · 5 lines
Joerg Roedel - AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Quoted 9 lines · Joerg
Quoted 8 lines · FUJITA Tomonori
Quoted 1 line · Joerg
The IOMMU-API is not about SR-IOV.
That's true. However, the point is that
include/iommu.his far from the IOMMU-API.
Quoted 6 lines
Quoted 3 lines
You could still insist that
include/iommu.his designed for the generic IOMMU-API. But the fact is that it's designed for very specific purposes. No intention to make it for generic purposes.I have no clue about the ARM iommus on the omap-platform. From a quick look into the header file I see some similarities to the IOMMU-API. I am
ARM's iommu stuff might be more appropriate as the IOMMU-API than include/linux/iommu.h
Quoted 3 lines
also very open for discussions about how the IOMMU-API could be extended to fit the needs of other platforms. Only because nobody has tried to discuss about such an effort is no reason to push the IOMMU-API back.
Well, the reason (nobody has tried) might be that linux/iommu.h doesn't look something intended for the generic IOMMU-API.
Quoted 7 lines · FUJITA Tomonori
Quoted 2 lines · Joerg
I have no clue about the ARM iommus on the omap-platform. From a quick look into the header file I see some similarities to the IOMMU-API. I am
ARM's iommu stuff might be more appropriate as the IOMMU-API than
include/linux/iommu.h
Then lets add this functionality to the generic IOMMU-API.
Quoted 6 lines
Quoted 3 lines
also very open for discussions about how the IOMMU-API could be extended to fit the needs of other platforms. Only because nobody has tried to discuss about such an effort is no reason to push the IOMMU-API back.
Well, the reason (nobody has tried) might be that
linux/iommu.hdoesn't look something intended for the generic IOMMU-API.
How does it not look like a generic intention? The function names are all generic and do not express that this API should only be used for KVM. If you talk about the design of the API itself, it was designed for the IOMMUs I was aware of at the time writing the API (in fact, the initial design was not my own, it was a generalization of the VT-d interfaces for KVM).
In other words it was a bottom-up approach to fit the needs of the time it was written. But its an kernel-only API so we can easily change it and extend it for other users/iommus when the need arises. I think this is the way we should go instead of letting each architecture design their own IOMMU-interfaces.
Joerg
Signature · 5 lines
Joerg Roedel - AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Quoted 2 lines · FUJITA Tomonori
Well, the reason (nobody has tried) might be that
linux/iommu.hdoesn't look something intended for the generic IOMMU-API.
Oh, and as an additional note, the reason might also be that people were not aware of its existence :-)
Signature · 5 lines
Joerg Roedel - AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Quoted 6 lines · Joerg
Quoted 2 lines · FUJITA Tomonori
Well, the reason (nobody has tried) might be that
linux/iommu.hdoesn't look something intended for the generic IOMMU-API.Oh, and as an additional note, the reason might also be that people were not aware of its existence :-)
No. People actually read it and think that it's not intended for generic purposes, i.e., it was designed for VT-d/AMD-IOMMU with KVM:
http://lkml.org/lkml/2010/7/28/470
You designed it for what you need at the time. It should have been named appropriately to avoid confusion. Later, when we actually understand what other IOMMUs need, we can evolve the specific API for generic purposes. Then we can rename the API to more generic.
Quoted 7 lines · FUJITA Tomonori
Quoted 4 lines · Joerg
Quoted 2 lines · FUJITA Tomonori
Well, the reason (nobody has tried) might be that
linux/iommu.hdoesn't look something intended for the generic IOMMU-API.
Quoted 7 lines
Quoted 2 lines
Oh, and as an additional note, the reason might also be that people were not aware of its existence :-)
No. People actually read it and think that it's not intended for generic purposes, i.e., it was designed for VT-d/AMD-IOMMU with KVM:
This states the as-is situation. There is not a single sentence that states why the iommu-api can't be extended to fit their needs. Nobody has ever written me an email about this to discuss this possibility.
Quoted 4 lines
You designed it for what you need at the time. It should have been named appropriately to avoid confusion. Later, when we actually understand what other IOMMUs need, we can evolve the specific API for generic purposes. Then we can rename the API to more generic.
At the time the iommu-api was written is was generic enough for what we had. So it was designed as an generic API. At this point in time nobody knew what the future requirements would we. So today it turns out that it is not generic enough anymore for latest hardware. The logical consequence is to fix this in the iommu-api.
Joerg
Signature · 5 lines
Joerg Roedel - AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Arnd Bergmann
maintainerQuoted 10 lines · Joerg
Quoted 4 lines
You designed it for what you need at the time. It should have been named appropriately to avoid confusion. Later, when we actually understand what other IOMMUs need, we can evolve the specific API for generic purposes. Then we can rename the API to more generic.
At the time the iommu-api was written is was generic enough for what we had. So it was designed as an generic API. At this point in time nobody knew what the future requirements would we. So today it turns out that it is not generic enough anymore for latest hardware. The logical consequence is to fix this in the iommu-api.
Well, I think the real question is why we have two APIs that both claim to work with IOMMUs in a generic way and how we could unify the two.
The Intel and AMD IOMMU drivers currently register at both the DMA API and the IOMMU API. The first one is used by everything except KVM and the second is only used by KVM.
I really think we should not extend the (KVM) IOMMU API further but just use the generic DMA mapping api for KVM and extend it as necessary. It already has the concept of cache coherency and mapping/unmapping that are in the IOMMU API and could be extended to support domains as well, through the use of dma_attrs.
Arnd
Quoted 7 lines · Arnd Bergmann
Quoted 5 lines · Joerg
Quoted 4 lines
You designed it for what you need at the time. It should have been named appropriately to avoid confusion. Later, when we actually understand what other IOMMUs need, we can evolve the specific API for generic purposes. Then we can rename the API to more generic.
Quoted 8 lines
Quoted 5 lines
At the time the iommu-api was written is was generic enough for what we had. So it was designed as an generic API. At this point in time nobody knew what the future requirements would we. So today it turns out that it is not generic enough anymore for latest hardware. The logical consequence is to fix this in the iommu-api.
Well, I think the real question is why we have two APIs that both claim to work with IOMMUs in a generic way and how we could unify the two.
The DMA-API itself does not claim to be an iommu-frontend. The purpose of the DMA-API is to convert physical memory addresses into dma handles and do all the management of these handles. Backend implementations can use hardware iommus for this task. But depending on the hardware in the system the DMA-API can very well be implemented without any hardware support. This is an important difference to the IOMMU-API which needs hardware because it exposes hardware iommu features to software.
Quoted 3 lines
The Intel and AMD IOMMU drivers currently register at both the DMA API and the IOMMU API. The first one is used by everything except KVM and the second is only used by KVM.
Right. But there is also a mode where the AMD IOMMU driver only registers for the IOMMU-API.
Quoted 5 lines
I really think we should not extend the (KVM) IOMMU API further but just use the generic DMA mapping api for KVM and extend it as necessary. It already has the concept of cache coherency and mapping/unmapping that are in the IOMMU API and could be extended to support domains as well, through the use of
dma_attrs.
If we find a nice and clean way to expose lower-level iommu functionality through the DMA-API, thats fine. We could certainly discuss ideas in this direction. I think this is going to be hard because the DMA-API today does not provide enough flexibility to let the user choose both sides of a io-virtual<->cpu-physical address mapping. Thats fine for most drivers because it makes sense for them to use the generic io-address-allocator the DMA-API provides but not for KVM which needs this flexibility.
Joerg
Signature · 5 lines
Joerg Roedel - AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Arnd Bergmann
maintainerQuoted 17 lines · Joerg
Quoted 7 lines · Arnd Bergmann
Quoted 5 lines · Joerg
Quoted 4 lines
You designed it for what you need at the time. It should have been named appropriately to avoid confusion. Later, when we actually understand what other IOMMUs need, we can evolve the specific API for generic purposes. Then we can rename the API to more generic.
Quoted 8 lines
Quoted 5 lines
At the time the iommu-api was written is was generic enough for what we had. So it was designed as an generic API. At this point in time nobody knew what the future requirements would we. So today it turns out that it is not generic enough anymore for latest hardware. The logical consequence is to fix this in the iommu-api.
Well, I think the real question is why we have two APIs that both claim to work with IOMMUs in a generic way and how we could unify the two.
Quoted 7 lines
The DMA-API itself does not claim to be an iommu-frontend. The purpose of the DMA-API is to convert physical memory addresses into dma handles and do all the management of these handles. Backend implementations can use hardware iommus for this task. But depending on the hardware in the system the DMA-API can very well be implemented without any hardware support. This is an important difference to the IOMMU-API which needs hardware because it exposes hardware iommu features to software.
Well, you could call that a limitation in the IOMMU API ;-)
The idea behind the DMA mapping API is to allow a device driver to work without knowing if the hardware can, cannot or must use an IOMMU.
Quoted 6 lines
Quoted 5 lines
I really think we should not extend the (KVM) IOMMU API further but just use the generic DMA mapping api for KVM and extend it as necessary. It already has the concept of cache coherency and mapping/unmapping that are in the IOMMU API and could be extended to support domains as well, through the use of
dma_attrs.
Quoted 8 lines
If we find a nice and clean way to expose lower-level iommu functionality through the DMA-API, thats fine. We could certainly discuss ideas in this direction. I think this is going to be hard because the DMA-API today does not provide enough flexibility to let the user choose both sides of a io-virtual<->cpu-physical address mapping. Thats fine for most drivers because it makes sense for them to use the generic io-address-allocator the DMA-API provides but not for KVM which needs this flexibility.
One way to do this would be to add a new attribute, e.g.
enum dma_attr {
DMA_ATTR_WRITE_BARRIER, DMA_ATTR_WEAK_ORDERING, DMA_ATTR_FIXED_MAPPING, /* this one is new */ DMA_ATTR_MAX,
};
struct dma_attrs {
unsigned long flags[__DMA_ATTRS_LONGS];
dma_add_t dest;
};Nothing except for KVM would need to use that attribute, and KVM would obviously need a way to check if this is supported by the underlying implementation.
Arnd
Joerg,
Thanks for the information. I have been trying to adapt the MSM IOMMU driver to use your IOMMU interface, and it looks like it might work, with one minor modification.
Unlike a more traditional system with one IOMMU between the bus and memory, MSM has multiple IOMMUs, with each one hard-wired to a dedicated device. Furthermore, each IOMMU can have more than one translation context. One of the use cases is being able to create mappings within multiple instances of one context, and arbitrarily context-switch the IOMMU on the fly.
It sounds like the domain abstraction and attach_device/detach_device can encapsulate this rather nicely and I am in the process of updating my driver to fit this framework.
My problem, however, is with iommu_domain_alloc(). This will set up a domain and call the ops function to initialize it, but I want to be able to pass it an “IOMMU id" that will tell the underlying driver which IOMMU (and which "stream id") is to be associated with that domain instance. This can be a void* parameter that gets passed through to domain_init. I feel like this change will make it easy to deal with multiple IOMMUs/translation contexts, and implementations that have only a singular IOMMU/translation context are free to ignore that parameter.
The alternative for me is to have a separate msm_iommu_domain_alloc(void *context_id) function, to which I can specify which IOMMU I want to use, but I would like to fully use your API if possible.
What are your thoughts? I can prepare a patch if you like - the domain_alloc change looks like it will be very innocuous.
Thanks
Steve
Quoted 43 lines · FUJITA Tomonori
Quoted 11 lines
Quoted 1 line · Joerg
The IOMMU-API is not about SR-IOV.
That's true. However, the point is that
include/iommu.his far from the IOMMU-API.You could still insist that
include/iommu.his designed for the generic IOMMU-API. But the fact is that it's designed for very specific purposes. No intention to make it for generic purposes.I have no clue about the ARM iommus on the omap-platform. From a quick look into the header file I see some similarities to the IOMMU-API. I am also very open for discussions about how the IOMMU-API could be extended to fit the needs of other platforms. Only because nobody has tried to discuss about such an effort is no reason to push the IOMMU-API back.
Quoted 4 lines
Since you added it two years ago, nobody has tried to extend it. Instead, we have something like
arch/arm/plat-omap/include/plat/iommu.h.And I think we should try to merge this platform-specific functionality into the IOMMU-API.
Joerg
--
Joerg Roedel - AMD Operating System Research CenterAdvanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Arnd Bergmann
maintainerQuoted 7 lines · On Friday 30 July 2010 07:19:06 stepanm at codeaurora.org wrote:
Unlike a more traditional system with one IOMMU between the bus and memory, MSM has multiple IOMMUs, with each one hard-wired to a dedicated device. Furthermore, each IOMMU can have more than one translation context. One of the use cases is being able to create mappings within multiple instances of one context, and arbitrarily context-switch the IOMMU on the fly.
Quoted 8 lines
It sounds like the domain abstraction and attach_device/detach_device can encapsulate this rather nicely and I am in the process of updating my driver to fit this framework.
My problem, however, is with
iommu_domain_alloc(). This will set up a domain and call the ops function to initialize it, but I want to be able to pass it an ?IOMMU id" that will tell the underlying driver which IOMMU (and which "stream id") is to be associated with that domain instance.
This probably best fits into the device itself, so you can assign the iommu data when probing the bus, e.g. (I don't know what bus you use)
struct msm_device {
struct msm_iommu *iommu;
struct device dev;
};This will work both for device drivers using the DMA API and for KVM with the IOMMU API.
Quoted 8 lines
This can be a void* parameter that gets passed through to
domain_init. I feel like this change will make it easy to deal with multiple IOMMUs/translation contexts, and implementations that have only a singular IOMMU/translation context are free to ignore that parameter.The alternative for me is to have a separate
msm_iommu_domain_alloc(void *context_id)function, to which I can specify which IOMMU I want to use, but I would like to fully use your API if possible.
No, that would require adding msm specific code to KVM and potential other users.
Arnd
Quoted 22 lines · On Friday 30 July 2010 07:19:06 [email protected] wrote:
Quoted 12 lines
Unlike a more traditional system with one IOMMU between the bus and memory, MSM has multiple IOMMUs, with each one hard-wired to a dedicated device. Furthermore, each IOMMU can have more than one translation context. One of the use cases is being able to create mappings within multiple instances of one context, and arbitrarily context-switch the IOMMU on the fly.
It sounds like the domain abstraction and attach_device/detach_device can
encapsulate this rather nicely and I am in the process of updating my driver to fit this framework.Quoted 5 lines
My problem, however, is with
iommu_domain_alloc(). This will set up a domain and call the ops function to initialize it, but I want to be able to pass it an “IOMMU id" that will tell the underlying driver which IOMMU
(and which "stream id") is to be associated with that domain instance.This probably best fits into the device itself, so you can assign the iommu data when probing the bus, e.g. (I don't know what bus you use)
Quoted 7 lines
struct msm_device { struct msm_iommu *iommu; struct device dev; };This will work both for device drivers using the DMA API and for KVM with the IOMMU API.
Right, this makes sense, and that is similar to how we were planning to set the iommus for the devices. But my question is, how does the IOMMU API know *which* IOMMU to talk to? It seems like this API has been designed with a singular IOMMU in mind, and it is implied that things like iommu_domain_alloc, iommu_map, etc all use "the" IOMMU. But I would like to allocate a domain and specify which IOMMU it is to be used for.
I can think of solving this in several ways.
One way would be to modify iommu_domain_alloc to take an IOMMU parameter, which gets passed into domain_init. This seems like the cleanest solution. Another way would be to have something like msm_iommu_domain_bind(domain, iommu) which would need to be called after iommu_domain_alloc to set the domain binding.
A third way that I could see is to delay the domain/iommu binding until iommu_attach_device, where the iommu could be picked up from the device that is passed in. I am not certain of this approach, since I had not been planning to pass in full devices, as in the MSM case this makes little sense (that is, if I am understanding the API correctly). On MSM, each device already has a dedicated IOMMU hard-wired to it. I had been planning to use iommu_attach_device to switch between active domains on a specific IOMMU and the given device would be of little use because that association is implicit on MSM.
Does that make sense? Am I correctly understanding the API? What do you think would be a good way to handle the multiple-iommu case?
Thanks
Steve
Arnd Bergmann
maintainerQuoted 12 lines · On Friday 30 July 2010 18:25:48 stepanm at codeaurora.org wrote:
Quoted 10 lines
This probably best fits into the device itself, so you can assign the iommu data when probing the bus, e.g. (I don't know what bus you use)
struct msm_device { struct msm_iommu *iommu; struct device dev; };This will work both for device drivers using the DMA API and for KVM with the IOMMU API.
Quoted 5 lines
Right, this makes sense, and that is similar to how we were planning to set the iommus for the devices. But my question is, how does the IOMMU API know *which* IOMMU to talk to? It seems like this API has been designed with a singular IOMMU in mind, and it is implied that things like
iommu_domain_alloc,iommu_map, etc all use "the" IOMMU.
The primary key is always the device pointer. If you look e.g. at arch/powerpc/include/asm/dma-mapping.h, you find
static inline struct dma_map_ops *get_dma_ops(struct device *dev)
{
return dev->archdata.dma_ops;
}Quoted 1 line
From there, you know the type of the iommu, each of which has its
own dma_ops pointer. The dma_ops->map_sg() referenced there is specific to one (or a fixed small number of) bus_type, e.g. PCI or in your case an MSM specific SoC bus, so it can cast the device to the bus specific data structure:
int msm_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
enum dma_data_direction dir)
{
struct msm_device *dev = container_of(dev, struct msm_device, dev);
...
}Quoted 8 lines
But I would like
to allocate a domain and specify which IOMMU it is to be used for. I can think of solving this in several ways.
One way would be to modifyiommu_domain_allocto take an IOMMU parameter, which gets passed intodomain_init. This seems like the cleanest solution. Another way would be to have something likemsm_iommu_domain_bind(domain, iommu)which would need to be called afteriommu_domain_allocto set the domain binding.
The iommu_domain is currently a concept that is only used in KVM, and there a domain currently would always span all of the IOMMUs that can host virtualized devices. I'm not sure what you want to do with domains though. Are you implementing KVM or another hypervisor, or is there another use case?
I've seen discussions about using an IOMMU to share page tables with regular processes so that user space can program a device to do DMA into its own address space, which would require an IOMMU domain per process using the device.
However, most of the time, it is better to change the programming model of those devices to do the mapping inside of a kernel device driver that allocates a physical memory area and maps it into both the BUS address space (using dma_map_{sg,single}) and the user address space (using mmap()).
Quoted 10 lines
A third way that I could see is to delay the domain/iommu binding until
iommu_attach_device, where the iommu could be picked up from the device that is passed in. I am not certain of this approach, since I had not been planning to pass in full devices, as in the MSM case this makes little sense (that is, if I am understanding the API correctly). On MSM, each device already has a dedicated IOMMU hard-wired to it. I had been planning to useiommu_attach_deviceto switch between active domains on a specific IOMMU and the given device would be of little use because that association is implicit on MSM.
Quoted 2 lines
Does that make sense? Am I correctly understanding the API? What do you think would be a good way to handle the multiple-iommu case?
My impression is that you are confusing the multi-IOMMU and the multi-domain problem, which are orthogonal. The dma-mapping API can deal with multiple IOMMUs as I described above, but has no concept of domains. KVM uses the iommu.h API to get one domain per guest OS, but as you said, it does not have a concept of multiple IOMMUs because neither Intel nor AMD require that today.
If you really need multiple domains across multiple IOMMUs, I'd suggest that we first merge the APIs and then port your code to that, but as a first step you could implement the standard dma-mapping.h API, which allows you to use the IOMMUs in kernel space.
Arnd
Quoted 99 lines · On Friday 30 July 2010 18:25:48 [email protected] wrote:
Quoted 13 lines
Quoted 11 lines
This probably best fits into the device itself, so you can assign the iommu data when probing the bus, e.g. (I don't know what bus you use)
struct msm_device { struct msm_iommu *iommu; struct device dev; };This will work both for device drivers using the DMA API and for KVM with the IOMMU API.
Quoted 6 lines
Right, this makes sense, and that is similar to how we were planning to set the iommus for the devices. But my question is, how does the IOMMU API
know *which* IOMMU to talk to? It seems like this API has been designed with a singular IOMMU in mind, and it is implied that things likeiommu_domain_alloc,iommu_map, etc all use "the" IOMMU.The primary key is always the device pointer. If you look e.g. at
arch/powerpc/include/asm/dma-mapping.h, you findstatic inline struct
dma_map_ops*get_dma_ops(struct device *dev){ return dev->archdata.dma_ops; }From there, you know the type of the iommu, each of which has its own
dma_opspointer. Thedma_ops->map_sg()referenced there is specific to one (or a fixed small number of)bus_type, e.g. PCI or in your case an MSM specific SoC bus, so it can cast the device to the bus specific data structure:int msm_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
enumdma_data_directiondir){ struct msm_device *dev = container_of(dev, struct msm_device, dev); ... }Quoted 11 lines
But I would like
to allocate a domain and specify which IOMMU it is to be used for. I can think of solving this in several ways.
One way would be to modifyiommu_domain_allocto take an IOMMU parameter,
which gets passed intodomain_init. This seems like the cleanest solution.
Another way would be to have something like msm_iommu_domain_bind(domain,
iommu) which would need to be called afteriommu_domain_allocto set the domain binding.The
iommu_domainis currently a concept that is only used in KVM, and there
a domain currently would always span all of the IOMMUs that can host virtualized devices. I'm not sure what you want to do with domains though. Are you implementing KVM or another hypervisor, or is there another use case?I've seen discussions about using an IOMMU to share page tables with regular processes so that user space can program a device to do DMA into its own address space, which would require an IOMMU domain per process using the device.
However, most of the time, it is better to change the programming model of those devices to do the mapping inside of a kernel device driver that allocates a physical memory area and maps it into both the BUS address space (using dma_map_{sg,single}) and the user address space (using
mmap()).Quoted 14 lines
A third way that I could see is to delay the domain/iommu binding until
iommu_attach_device, where the iommu could be picked up from the device that is passed in. I am not certain of this approach, since I had not been
planning to pass in full devices, as in the MSM case this makes little sense (that is, if I am understanding the API correctly). On MSM, each device already has a dedicated IOMMU hard-wired to it. I had been planning
to useiommu_attach_deviceto switch between active domains on a specific
IOMMU and the given device would be of little use because that association
is implicit on MSM.Quoted 2 lines
Does that make sense? Am I correctly understanding the API? What do you think would be a good way to handle the multiple-iommu case?
My impression is that you are confusing the multi-IOMMU and the multi-domain
problem, which are orthogonal. The dma-mapping API can deal with multiple IOMMUs as I described above, but has no concept of domains. KVM uses theiommu.hAPI to get one domain per guest OS, but as you said, it does not have a concept of multiple IOMMUs because neither Intel nor AMD require that
today.
Quoted 7 lines
If you really need multiple domains across multiple IOMMUs, I'd suggest that
we first merge the APIs and then port your code to that, but as a first step
you could implement the standarddma-mapping.hAPI, which allows you to use
the IOMMUs in kernel space.
One of our uses cases actually does involve using domains pretty much as you had described them, though only on one of the IOMMUs. That is, the domain for that IOMMU basically abstracts its page table, and it is a legitimate thing to switch out page tables for the IOMMU on the fly. I guess the difference is that you described the domain as the set of mappings made on ALL the IOMMUs, whereas I had envisioned there being one (or more) domains for each IOMMU.
Arnd Bergmann
maintainerQuoted 5 lines · On Saturday 31 July 2010 00:58:00 stepanm at codeaurora.org wrote:
Quoted 4 lines
If you really need multiple domains across multiple IOMMUs, I'd suggest that we first merge the APIs and then port your code to that, but as a first step you could implement the standard
dma-mapping.hAPI, which allows you to use the IOMMUs in kernel space.
Quoted 7 lines
One of our uses cases actually does involve using domains pretty much as you had described them, though only on one of the IOMMUs. That is, the domain for that IOMMU basically abstracts its page table, and it is a legitimate thing to switch out page tables for the IOMMU on the fly. I guess the difference is that you described the domain as the set of mappings made on ALL the IOMMUs, whereas I had envisioned there being one (or more) domains for each IOMMU.
Can you be more specific on what kind of device would use multiple domains in your case and how you intend to use them? Is this for some kind of DSP interacting with user processes?
This seems to be a scenario that we haven't dealt with before (or perhaps avoided intentionally), so if we need to make API changes, we should all understand what we need them for. It's no problem to extend the API if you have good reasons for using multiple domains, but I also want to make sure that there isn't also a way to achieve the same or better result with the current APIs.
Arnd
Hi Stephan,
Quoted 6 lines · On Fri, Jul 30, 2010 at 01:19:06AM -0400, [email protected] wrote:
Unlike a more traditional system with one IOMMU between the bus and memory, MSM has multiple IOMMUs, with each one hard-wired to a dedicated device. Furthermore, each IOMMU can have more than one translation context. One of the use cases is being able to create mappings within multiple instances of one context, and arbitrarily context-switch the IOMMU on the fly.
The IOMMU-API supports multiple IOMMUs (at least multiple AMD/Intel IOMMUs). But the face that there are more than one IOMMU is hidden in the backend driver implementation. The API itself only works with domains and devices. The IOMMU driver needs to know which IOMMU it needs to program for a given device. If I understand the concept of your hardware correctly you also have this information.
Quoted 4 lines
It sounds like the domain abstraction and attach_device/detach_device can encapsulate this rather nicely and I am in the process of updating my driver to fit this framework.
Quoted 8 lines
My problem, however, is with
iommu_domain_alloc(). This will set up a domain and call the ops function to initialize it, but I want to be able to pass it an “IOMMU id" that will tell the underlying driver which IOMMU (and which "stream id") is to be associated with that domain instance. This can be a void* parameter that gets passed through todomain_init. I feel like this change will make it easy to deal with multiple IOMMUs/translation contexts, and implementations that have only a singular IOMMU/translation context are free to ignore that parameter.
In the means of the IOMMU-API the domain is the abstraction of an address space (in other words a page table). The IOMMU(s) which this domain is later assigned to are determined by the iommu_attach_device calls. I think the right way to go here is to create the concept of a device-context in the IOMMU-API and add functions like
iommu_attach_context(struct iommu_domain *domain, struct iommu_context *ctxt); iommu_detach_context(struct iommu_context *ctxt);
This would work if you can determine in your iommu-driver which iommu you need to program for which device. What do you think?
Joerg
Signature · 5 lines
Joerg Roedel - AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Quoted 36 lines · Joerg
Hi Stephan,
Quoted 6 lines · stepanm at codeaurora.org
Unlike a more traditional system with one IOMMU between the bus and memory, MSM has multiple IOMMUs, with each one hard-wired to a dedicated device. Furthermore, each IOMMU can have more than one translation context. One of the use cases is being able to create mappings within multiple instances of one context, and arbitrarily context-switch the IOMMU on the fly.
The IOMMU-API supports multiple IOMMUs (at least multiple AMD/Intel IOMMUs). But the face that there are more than one IOMMU is hidden in the backend driver implementation. The API itself only works with domains and devices. The IOMMU driver needs to know which IOMMU it needs to program for a given device. If I understand the concept of your hardware correctly you also have this information.
Quoted 4 lines
It sounds like the domain abstraction and attach_device/detach_device can encapsulate this rather nicely and I am in the process of updating my driver to fit this framework.
Quoted 8 lines
My problem, however, is with
iommu_domain_alloc(). This will set up a domain and call the ops function to initialize it, but I want to be able to pass it an ???IOMMU id" that will tell the underlying driver which IOMMU (and which "stream id") is to be associated with that domain instance. This can be a void* parameter that gets passed through todomain_init. I feel like this change will make it easy to deal with multiple IOMMUs/translation contexts, and implementations that have only a singular IOMMU/translation context are free to ignore that parameter.In the means of the IOMMU-API the domain is the abstraction of an address space (in other words a page table). The IOMMU(s) which this domain is later assigned to are determined by the
iommu_attach_devicecalls. I think the right way to go here is to create the concept of a device-context in the IOMMU-API and add functions like
Quoted 7 lines
iommu_attach_context(struct iommu_domain *domain, struct iommu_context *ctxt); iommu_detach_context(struct iommu_context *ctxt);This would work if you can determine in your iommu-driver which iommu you need to program for which device. What do you think?
Joerg, I'd like to make sure I understand this. A domain is an address space separate from the actual page-tables that may be part of an iommu_context, correct? After I iommu_attach_context the ctxt will reflect the address space of the domain, correct?
Quoted 14 lines
Joerg
Signature · 10 lines
Joerg Roedel - AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 12 lines · Zach Pfeffer
Quoted 10 lines · Joerg
In the means of the IOMMU-API the domain is the abstraction of an address space (in other words a page table). The IOMMU(s) which this domain is later assigned to are determined by the
iommu_attach_devicecalls. I think the right way to go here is to create the concept of a device-context in the IOMMU-API and add functions likeiommu_attach_context(struct iommu_domain *domain, struct iommu_context *ctxt); iommu_detach_context(struct iommu_context *ctxt);
Quoted 8 lines
Quoted 3 lines
This would work if you can determine in your iommu-driver which iommu you need to program for which device. What do you think?
Joerg, I'd like to make sure I understand this. A domain is an address space separate from the actual page-tables that may be part of an
iommu_context, correct? After Iiommu_attach_contextthe ctxt will reflect the address space of the domain, correct?
A domain is defined by a single page-table which can be modified using the iommu_map/iommu_unmap function calls. I am not completly sure what you mean by an iommu_context. Can you describe what it means in your context?
Joerg
Signature · 5 lines
AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Quoted 19 lines · Joerg
Quoted 11 lines · Zach Pfeffer
Quoted 10 lines · Joerg
In the means of the IOMMU-API the domain is the abstraction of an address space (in other words a page table). The IOMMU(s) which this domain is later assigned to are determined by the
iommu_attach_devicecalls. I think the right way to go here is to create the concept of a device-context in the IOMMU-API and add functions likeiommu_attach_context(struct iommu_domain *domain, struct iommu_context *ctxt); iommu_detach_context(struct iommu_context *ctxt);Quoted 7 lines
Quoted 3 lines
This would work if you can determine in your iommu-driver which iommu you need to program for which device. What do you think?
Joerg, I'd like to make sure I understand this. A domain is an address space separate from the actual page-tables that may be part of an
iommu_context, correct? After Iiommu_attach_contextthe ctxt will reflect the address space of the domain, correct?
Quoted 7 lines
A domain is defined by a single page-table which can be modified using the iommu_map/iommu_unmap function calls. I am not completly sure what you mean by an
iommu_context. Can you describe what it means in your context?Joerg
Joerg,
I think with some rework, all my use cases can be handled by your existing iommu API. If the domain is treated basically a page table, there will be some changes, but I think it can be done. I will push a new version of my driver in a few days.
One thing that may be helpful for the future, however, is maybe something like adding iommu_tlb_flush to the ops. I suppose this would either have to take a device, or the domain would need to keep a list of devices it had been attached to (so that their TLBs can be invalidated). But I suppose on the other hand, iommu_map/unmap may be able to just implicitly invalidate the TLB also, since TLB invalidation often follows map/unmap. What are your thoughts?
Thanks
Steve
Quoted 12 lines · Stepan Moskovchenko
Quoted 4 lines · Joerg
A domain is defined by a single page-table which can be modified using the iommu_map/iommu_unmap function calls. I am not completly sure what you mean by an
iommu_context. Can you describe what it means in your context?Joerg,
I think with some rework, all my use cases can be handled by your existing iommu API. If the domain is treated basically a page table, there will be some changes, but I think it can be done. I will push a new version of my driver in a few days.
Quoted 7 lines
One thing that may be helpful for the future, however, is maybe something like adding
iommu_tlb_flushto the ops. I suppose this would either have to take a device, or the domain would need to keep a list of devices it had been attached to (so that their TLBs can be invalidated). But I suppose on the other hand, iommu_map/unmap may be able to just implicitly invalidate the TLB also, since TLB invalidation often follows map/unmap. What are your thoughts?
Sounds good. I am curious for your patches :-)
For the TLB-flush question, I think it would make sense to add iommu tlb flushing functions to the IOMMU-API. We currently flush the TLB implicitly in the map/unmap calls but thats very inefficient. It would be better to have a seperate function for it in the API. The right parameter for such a function is a domain. The IOMMU driver knows which devices are attached to a domain an could easily flush all TLBs.
One alternative I can think of: An iommu_domain_commit() function which syncs software changes of a domain to the hardware. The map/unmap calls have to save which parts of the tlb need to be flushed and commit does flush those parts then (or flush everything).
Joerg
Signature · 5 lines
AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Quoted 9 lines · Arnd Bergmann
Exactly, thanks for the clarification. I also didn't realize that there
is now aninclude/linux/iommu.hfile that only describes the PCI SR-IOV
interfaces, unlike the generic IOMMU support that we have in yourinclude/linux/dma-mapping.hfile.Maybe we should rename
linux/iommu.hto something more specific so we can reduce this confusion in the future.
Hrm, indeed I just noticed that. Pretty gross... it should definitly be renamed, is will caused endless confusion with unrelated iommu.h and iommu_* interfaces which represent something different.
Ben.
Quoted 8 lines · Benjamin Herrenschmidt
Quoted 7 lines · Arnd Bergmann
Exactly, thanks for the clarification. I also didn't realize that there
is now aninclude/linux/iommu.hfile that only describes the PCI SR-IOV
interfaces, unlike the generic IOMMU support that we have in yourinclude/linux/dma-mapping.hfile.
Quoted 6 lines
Quoted 2 lines
Maybe we should rename
linux/iommu.hto something more specific so we can reduce this confusion in the future.Hrm, indeed I just noticed that. Pretty gross... it should definitly be renamed, is will caused endless confusion with unrelated
iommu.hand iommu_* interfaces which represent something different.
The first direction to go should be trying to unify all the different iommu* interfaces into the iommu-api. The generic api will definitly need to be extended for that, but since it is an in-kernel interface thats no problem.
Signature · 5 lines
Joerg Roedel - AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Quoted 9 lines · Joerg
Quoted 1 line
Hrm, indeed I just noticed that. Pretty gross... it should definitly
be
Quoted 2 lines
renamed, is will caused endless confusion with unrelated
iommu.hand iommu_* interfaces which represent something different.The first direction to go should be trying to unify all the different iommu* interfaces into the iommu-api. The generic api will definitly need to be extended for that, but since it is an in-kernel interface thats no problem.
Well, I suppose I'm the de-facto candidate to take care of the powerpc side then :-)
I don't have the bandwidth right now, but I'll try to have a look when time permits.
Cheers,
Ben.
Quoted 11 lines · Benjamin Herrenschmidt
Quoted 9 lines · Joerg
Quoted 1 line
Hrm, indeed I just noticed that. Pretty gross... it should definitly
be
Quoted 2 lines
renamed, is will caused endless confusion with unrelated
iommu.hand iommu_* interfaces which represent something different.The first direction to go should be trying to unify all the different iommu* interfaces into the iommu-api. The generic api will definitly need to be extended for that, but since it is an in-kernel interface thats no problem.
Quoted 5 lines
Well, I suppose I'm the de-facto candidate to take care of the powerpc side then :-)
I don't have the bandwidth right now, but I'll try to have a look when time permits.
Great :-)
Signature · 5 lines
Joerg Roedel - AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Quoted 6 lines · On Mon, 02 Aug 2010 18:03:02 +1000 Benjamin Herrenschmidt…
Quoted 5 lines · Joerg
Quoted 1 line
Hrm, indeed I just noticed that. Pretty gross... it should definitly
be
Quoted 2 lines
renamed, is will caused endless confusion with unrelated
iommu.hand iommu_* interfaces which represent something different.
Quoted 7 lines
Quoted 4 lines
The first direction to go should be trying to unify all the different iommu* interfaces into the iommu-api. The generic api will definitly need to be extended for that, but since it is an in-kernel interface thats no problem.
Well, I suppose I'm the de-facto candidate to take care of the powerpc side then :-)
We already agreed that what the iommu-api looks like?
ARM's iommu code (arch/plat-omap/include/plat/iommu.h) is a library to simplify the IOMMU implementations. It could be useful for all the iommu implementations.
The current iommu-api (include/linux/iommu.h) provides the common interface for specific purposes (for KVM).
I think that the current iommu-api can be a part of the former.
I also think that the IOMMU part of this new msm should be integrated into the former.
Another question is how the above can work with the DMA-API.
Quoted 3 lines · FUJITA Tomonori
ARM's iommu code (
arch/plat-omap/include/plat/iommu.h) is a library to simplify the IOMMU implementations. It could be useful for all the iommu implementations.
ITYM OMAP's iommu code.
Quoted 6 lines · On Mon, 2 Aug 2010 10:03:26 +0100 Russell King - ARM Linux…
Quoted 3 lines · FUJITA Tomonori
ARM's iommu code (
arch/plat-omap/include/plat/iommu.h) is a library to simplify the IOMMU implementations. It could be useful for all the iommu implementations.ITYM OMAP's iommu code.
Yeah, I meant that we could extend it to make it useful for other iommu implementations. At least, we could make something generic like struct iommu_functions, I think. Then we can embed a generic iommu structure into an iommu specific struct (like we do with inode).
The current iommu-api (include/linux/iommu.h) is just about domain and mapping concept. We can implement it on the top of the above infrastructure.
I'm still trying to figure out how the DMA-API can work well with them.
Quoted 15 lines · FUJITA Tomonori
Quoted 6 lines · On Mon, 2 Aug 2010 10:03:26 +0100 Russell King - ARM Linux…
Quoted 3 lines · FUJITA Tomonori
ARM's iommu code (
arch/plat-omap/include/plat/iommu.h) is a library to simplify the IOMMU implementations. It could be useful for all the iommu implementations.ITYM OMAP's iommu code.
Yeah, I meant that we could extend it to make it useful for other iommu implementations. At least, we could make something generic like struct
iommu_functions, I think. Then we can embed a generic iommu structure into an iommu specific struct (like we do with inode).
Quoted 6 lines
The current iommu-api (
include/linux/iommu.h) is just about domain and mapping concept. We can implement it on the top of the above infrastructure.I'm still trying to figure out how the DMA-API can work well with them.
I'm not sure it can in totality. The DMA-API solves the host CPU <-> device aspect of DMA support only.
However, there is another use case for IOMMUs, which is to allow two separate peripheral devices to communicate with each other via system memory. As the streaming DMA-API involves the idea of buffer ownership (of a singular agent), it is unsuitable for this use case.
The coherent allocation part of the DMA-API also only deals with the idea of there being a singular DMA agent accessing the allocated buffer (in conjunction with the host CPU).
Quoted 26 lines · On Mon, 2 Aug 2010 11:04:19 +0100 Russell King - ARM Linux…
Quoted 15 lines · FUJITA Tomonori
Quoted 6 lines · On Mon, 2 Aug 2010 10:03:26 +0100 Russell King - ARM Linux…
Quoted 3 lines · FUJITA Tomonori
ARM's iommu code (
arch/plat-omap/include/plat/iommu.h) is a library to simplify the IOMMU implementations. It could be useful for all the iommu implementations.ITYM OMAP's iommu code.
Yeah, I meant that we could extend it to make it useful for other iommu implementations. At least, we could make something generic like struct
iommu_functions, I think. Then we can embed a generic iommu structure into an iommu specific struct (like we do with inode).Quoted 6 lines
The current iommu-api (
include/linux/iommu.h) is just about domain and mapping concept. We can implement it on the top of the above infrastructure.I'm still trying to figure out how the DMA-API can work well with them.
I'm not sure it can in totality. The DMA-API solves the host CPU <-> device aspect of DMA support only.
Quoted 8 lines
However, there is another use case for IOMMUs, which is to allow two separate peripheral devices to communicate with each other via system memory. As the streaming DMA-API involves the idea of buffer ownership (of a singular agent), it is unsuitable for this use case.
The coherent allocation part of the DMA-API also only deals with the idea of there being a singular DMA agent accessing the allocated buffer (in conjunction with the host CPU).
I don't have a clear idea what kinda API works well in the above case yet.
But we have been talking about more bigger things? Not just about the interface, how things work together.
- OMAP's iommu code is a library to simplify OMAP implementations.
- include/linux/iommu.h is an interface to IOMMUs for specific interfaces.
- the DMA API could access to IOMMUs internally.
IOMMU library provides generic iommu interfaces (a structure including IOMMU implementation specific function pointers). KVM uses some of them. Architectures could implement the DMA API on the top of some the interfaces too.
Quoted 14 lines · FUJITA Tomonori
Quoted 11 lines · On Mon, 02 Aug 2010 18:03:02 +1000 Benjamin Herrenschmidt…
Quoted 9 lines · Joerg
Quoted 1 line
Hrm, indeed I just noticed that. Pretty gross... it should definitly
be
Quoted 2 lines
renamed, is will caused endless confusion with unrelated
iommu.hand iommu_* interfaces which represent something different.The first direction to go should be trying to unify all the different iommu* interfaces into the iommu-api. The generic api will definitly need to be extended for that, but since it is an in-kernel interface thats no problem.
Quoted 4 lines
Quoted 2 lines
Well, I suppose I'm the de-facto candidate to take care of the powerpc side then :-)
We already agreed that what the iommu-api looks like?
Well, we currently trying to figure out how to extend the IOMMU-API concepts to fit the omap-hardware in. Thats what I currently discuss with Stephan. It looks to me that we need to add the concept of device contexts to the IOMMU-API. We should also add IO-TLB management functions. The TLB management is currently handled completly in the backend driver. This needs to be changed and makes sense for AMD IOMMUs and Intel IOMMUs with QPI too.
Quoted 3 lines
ARM's iommu code (
arch/plat-omap/include/plat/iommu.h) is a library to simplify the IOMMU implementations. It could be useful for all the iommu implementations.
To me it looks like a very hardware specific library. But it should fit well in the domain/device concept the IOMMU-API provides (when we also add device-contexts to it)
Quoted 2 lines
The current iommu-api (
include/linux/iommu.h) provides the common interface for specific purposes (for KVM).
The IOMMU-API is not limited to the purposes of KVM. There is currently development effort to use the IOMMU-API for UIO stuff. So the IOMMU-API is by no means KVM specific.
Quoted 1 line
Another question is how the above can work with the DMA-API.
This would work if we handle every device-context the platform provides as 'struct device'. But does that really need to work with the DMA-API? What is the driver use-case for that?
Joerg
Signature · 5 lines
Joerg Roedel - AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Quoted 11 lines · Benjamin Herrenschmidt
Quoted 9 lines · Joerg
Quoted 1 line
Hrm, indeed I just noticed that. Pretty gross... it should definitly
be
Quoted 2 lines
renamed, is will caused endless confusion with unrelated
iommu.hand iommu_* interfaces which represent something different.The first direction to go should be trying to unify all the different iommu* interfaces into the iommu-api. The generic api will definitly need to be extended for that, but since it is an in-kernel interface thats no problem.
Quoted 5 lines
Well, I suppose I'm the de-facto candidate to take care of the powerpc side then :-)
I don't have the bandwidth right now, but I'll try to have a look when time permits.
Btw. I have some ideas to extend the IOMMU-API to also support GART-like IOMMUs. These pieces could also support (limited-size) domains (without isolation) using segmentation. Not sure if this makes sense for the use-cases in other architectures but we should not declare this impossible for now.
Signature · 5 lines
Joerg Roedel - AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Quoted 11 lines · FUJITA Tomonori
Don't confuse the IOMMU interface with the DMA API that Arnd mentioned.
They are not related at all.
The DMA API is defined in
Documentation/DMA-API.txt.Arnd told you that
include/asm-generic/dma-mapping-common.his the library to support the DMA API with multiple IOMMUs. Lots of architectures (x86, powerpc, sh, alpha, ia64, microblaze, sparc) use it.
Also, some of the iommu layer actually originates from powerpc.
Cheers,
Ben.