Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall
On 12/17/10 15:14, Saravana Kannan wrote: Catalin Marinas wrote: Russell, I agree with your point about using an API for purpose and not property. But I read Catalin's proposal as, let's treat secure domain as another DMA device. If we make a conscious agreement to do that, then using the DMA API for secure domain would be using it for its purpose and we will make an effort to not break it with future updates. Of course, if we don't agree on that proposal, then we can't use the DMA API for secure domain stuff. If there is no better proposal, I'm for such extension to the DMA API. From the kernel perspecitve, the secure side is just another entity that accesses the RAM directly. It's not a physically separate device indeed but from a direct memory access perspective it can be treated as any other device. In the DMA API we can fall back to the non-coherent ops when a NULL struct device is passed. I assume in your code you already pass a NULL device to dma_alloc_coherent(). Russell, Would the extension of the DMA API as described above be acceptable to you? If not, can you please suggest an alternative that's acceptable to you? Ping... -Saravana -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall
On Thu, Dec 16, 2010 at 06:55:06PM -0800, Saravana Kannan wrote: On 12 December 2010 04:58, Saravana Kannan skan...@codeaurora.org wrote: As you and James suggested, having the NS bit set by the secure world is definitely a solution that would work. But IMHO, the explicit cache flush/invalidate approach keeps the design simple and easy to maintain. That is indeed an approach to the problem. But it depends on whether we consider the DMA API appropriate for this. We can view the secure world as a non-coherent agent accessing the memory and could try to justify the use of the DMA API in Linux. At some point we'll probably have platforms supporting cacheable DMA (e.g. via the ARM coherency port) and the DMA API would no longer give you what you need. But it is also possible that platforms with ACP would only have 1 or 2 devices on that port (some HD LCD controller for example) and the rest of devices non-coherent. In this case, we need to have different DMA operations depending on the bus/device (via get_dma_ops) and thus we can allow your scenario via dedicated DMA ops. Catalin, Looks like you agree with our approach. If that's the case, would you mind Acking Jeff's initial patch that this thread is based on? I read Catalin's reply as agreeing with me. Russell, Does Catalin's proposal sound acceptable to you? Catalin's proposal for get_dma_ops doesn't work for you because you don't have a struct device for your CPUs - and as we don't have anything supporting ACP at the moment, there's little point engineering it in. The basic point here is that using the DMA API to achieve DMA coherency with something that is not DMA is going to be prone to failure, because we aren't going to guarantee that it'll do what you want. There's already a history of people abusing the DMA API, and then when we fix stuff in the DMA API, they complain that their drivers have broken. What I've been saying is never use it for its properties (for its uncached memory) as that is _not_ guaranteed - use it for its purpose instead (which is to provide coherent memory for DMA devices) which is guaranteed. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall
Catalin, Looks like you agree with our approach. If that's the case, would you mind Acking Jeff's initial patch that this thread is based on? I read Catalin's reply as agreeing with me. Catalin, Can you clarify? Russell, Does Catalin's proposal sound acceptable to you? Catalin's proposal for get_dma_ops doesn't work for you because you don't have a struct device for your CPUs - and as we don't have anything supporting ACP at the moment, there's little point engineering it in. The basic point here is that using the DMA API to achieve DMA coherency with something that is not DMA is going to be prone to failure, because we aren't going to guarantee that it'll do what you want. There's already a history of people abusing the DMA API, and then when we fix stuff in the DMA API, they complain that their drivers have broken. What I've been saying is never use it for its properties (for its uncached memory) as that is _not_ guaranteed - use it for its purpose instead (which is to provide coherent memory for DMA devices) which is guaranteed. Russell, I agree with your point about using an API for purpose and not property. But I read Catalin's proposal as, let's treat secure domain as another DMA device. If we make a conscious agreement to do that, then using the DMA API for secure domain would be using it for its purpose and we will make an effort to not break it with future updates. Of course, if we don't agree on that proposal, then we can't use the DMA API for secure domain stuff. After Catalin's response to clarify, if we still end up not treating secure domain as a DMA device, then what's the alternative? Can we get an explicit cache invalidate API that's outside of the DMA APIs? Or a general uncached pages alloc/free APIs? Thanks, Saravana -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall
On Fri, Dec 17, 2010 at 02:26:44AM -0800, Saravana Kannan wrote: After Catalin's response to clarify, if we still end up not treating secure domain as a DMA device, then what's the alternative? Can we get an explicit cache invalidate API that's outside of the DMA APIs? Or a general uncached pages alloc/free APIs? The answer _always_ is that if the existing APIs don't give you want you need, then either the APIs need extending or a new API needs to be created. In any case, have you looked at the latest set of SCM patches? They don't make use of DMA coherent memory anymore, so the problem would seem to be resolved. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall
On 17 December 2010 10:26, Saravana Kannan skan...@codeaurora.org wrote: Looks like you agree with our approach. If that's the case, would you mind Acking Jeff's initial patch that this thread is based on? I read Catalin's reply as agreeing with me. Catalin, Can you clarify? I'll try but I started my holidays and I'll only be online occasionally. Just to clarify, even if I ack Jeff's patch, it is for Russell to decide what gets merged. Now, Jeff's patch doesn't show anything about how the dma_alloc_coherent is used, just suggests something in the commit log, so I don't see it critical to this discussion. I wouldn't ack it without agreement on the extension of the DMA API (which can only have a no-op get_dma_ops at this point). I agree with Russell's points that just using the DMA API as it is may break in the future, hence a proposal to treat it slightly different. People in ARM working on a generic state save/restore mechanism face the same problem - they need some non-cacheable memory for synchronisation. I'm not sure whether they managed to find an alternative algorithm with cached memory and cache flushing and I also haven't followed the development to give more details. Russell, I agree with your point about using an API for purpose and not property. But I read Catalin's proposal as, let's treat secure domain as another DMA device. If we make a conscious agreement to do that, then using the DMA API for secure domain would be using it for its purpose and we will make an effort to not break it with future updates. Of course, if we don't agree on that proposal, then we can't use the DMA API for secure domain stuff. If there is no better proposal, I'm for such extension to the DMA API. From the kernel perspecitve, the secure side is just another entity that accesses the RAM directly. It's not a physically separate device indeed but from a direct memory access perspective it can be treated as any other device. In the DMA API we can fall back to the non-coherent ops when a NULL struct device is passed. I assume in your code you already pass a NULL device to dma_alloc_coherent(). -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall
On 12 December 2010 04:58, Saravana Kannan skan...@codeaurora.org wrote: As you and James suggested, having the NS bit set by the secure world is definitely a solution that would work. But IMHO, the explicit cache flush/invalidate approach keeps the design simple and easy to maintain. That is indeed an approach to the problem. But it depends on whether we consider the DMA API appropriate for this. We can view the secure world as a non-coherent agent accessing the memory and could try to justify the use of the DMA API in Linux. At some point we'll probably have platforms supporting cacheable DMA (e.g. via the ARM coherency port) and the DMA API would no longer give you what you need. But it is also possible that platforms with ACP would only have 1 or 2 devices on that port (some HD LCD controller for example) and the rest of devices non-coherent. In this case, we need to have different DMA operations depending on the bus/device (via get_dma_ops) and thus we can allow your scenario via dedicated DMA ops. Catalin, Looks like you agree with our approach. If that's the case, would you mind Acking Jeff's initial patch that this thread is based on? Russell, Does Catalin's proposal sound acceptable to you? Thanks, Saravana -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall
On 12 December 2010 04:58, Saravana Kannan skan...@codeaurora.org wrote: As you and James suggested, having the NS bit set by the secure world is definitely a solution that would work. But IMHO, the explicit cache flush/invalidate approach keeps the design simple and easy to maintain. That is indeed an approach to the problem. But it depends on whether we consider the DMA API appropriate for this. We can view the secure world as a non-coherent agent accessing the memory and could try to justify the use of the DMA API in Linux. At some point we'll probably have platforms supporting cacheable DMA (e.g. via the ARM coherency port) and the DMA API would no longer give you what you need. But it is also possible that platforms with ACP would only have 1 or 2 devices on that port (some HD LCD controller for example) and the rest of devices non-coherent. In this case, we need to have different DMA operations depending on the bus/device (via get_dma_ops) and thus we can allow your scenario via dedicated DMA ops. -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall
On 12/10/2010 02:00 AM, Catalin Marinas wrote: On 10 December 2010 00:58, Saravana Kannanskan...@codeaurora.org wrote: Russell King - ARM Linux wrote: On Thu, Dec 09, 2010 at 01:23:24AM -0800, skan...@codeaurora.org wrote: Russell, Have you had a chance to look at this? Any comments? How do we move ahead? I had connected the other thread with this one - it's pretty hard not to as it included this patch in that set as the first patch. Ok. So what's your take on handling the cache coherency with secure domain? Do we get to add cache invalidate APIs outside of DMA APIs? Can the secure software not create NS pages and be fully coherent? IMHO, trying to keep the cache fully coherent with the secure world without using explicit cache flush/invalidate raises the following concerns (in decreasing order of importance): 1. Since the secure world would need to enable the MMU to mark the pages as NS, it removes the possibility of secure world implementations that don't use MMU. The secure world image that's in use for MSM8660 as of today doesn't enable its MMU. 2. Even if MMU was enabled in secure world, the same binary needs to operate with different non-secure kernels. I'm certainly not a cache expert, but if I'm not mistaken, there are other page attributes that need to match between virt mem mappings for them to end up on the same cache lines (wasn't this a reason that Russell added the already mapped? check to ioremap). Trying to coordinate this between various non-secure kernels would be harder than having the various non-secure kernels do a flush/invalidate. 3. *If* a user of the SCM driver wants to pass phys addr of other memory pages to a service on the secure world (say, to avoid multiple copies), then they will also have to start coordinating on the page attributes. That would be harder to maintain than just an invalidate before reading if secure-world service will write to this memory approach. 4. Coordinating to make sure all the necessary page attributes match becomes harder if OEMs decide to make changes to the secure world implementation or drop in a new one. As you and James suggested, having the NS bit set by the secure world is definitely a solution that would work. But IMHO, the explicit cache flush/invalidate approach keeps the design simple and easy to maintain. Thanks, Saravana -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall
On 10 December 2010 00:58, Saravana Kannan skan...@codeaurora.org wrote: Russell King - ARM Linux wrote: On Thu, Dec 09, 2010 at 01:23:24AM -0800, skan...@codeaurora.org wrote: Russell, Have you had a chance to look at this? Any comments? How do we move ahead? I had connected the other thread with this one - it's pretty hard not to as it included this patch in that set as the first patch. Ok. So what's your take on handling the cache coherency with secure domain? Do we get to add cache invalidate APIs outside of DMA APIs? Can the secure software not create NS pages and be fully coherent? -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall
Russell King - ARM Linux wrote: On Fri, Dec 03, 2010 at 12:06:53PM -0800, Saravana Kannan wrote: The MSM8660 SoC uses the TrustZone technology and the Linux kernel executes in normal/non-secure domain. When the second core is brought out of reset, it starts executing a secure image which then jumps to secondary_startup. So, before bringing the second core out of reset, we need to inform the secure domain code where secondary_startup is located in memory. We do the communication with the secure code by using buffers in memory. The cache treats the NS (non secure) bit as an additional address bit when tagging memory. Hence, cache accesses are not coherent between the secure and non-secure domains. So, the secure side flushes it's cache after writing to the buffer. To properly read the response from the secure side, the kernel has to pick a buffer that isn't cacheable in the first place. We have similar issues in the reverse direction. So when ARM gets DMA-coherent caches, you of course aren't going to complain that the DMA APIs start avoiding doing the current tricks with non-cacheable memory? I view what you're doing above with the DMA API as an abuse of the API. Just like the problems we're facing with ioremap() being used on system RAM, you're asking for problems when the ARM architecture changes because you're using an API for it's current properties, not for its purpose. You are right. Thanks for catching this. So, that basically leaves us with these options: * Create another API to allow getting uncached pages. I don't think we will be the first or the last to want uncached pages. Even if ARM introduces DMA-coherent caches, it's possible for SoC vendors to have other h/w blocks that could directly operate on memory. The cache might not be coherent with these h/w blocks. * Add a cache invalidate API that's outside the DMA APIs and can be used when needed. Do one of the above two options sound reasonable to you? I've been on for years about purpose-designed APIs for cache issues, and every time someone abuses them, they eventually end up suffering breakage. Let's wait until the full set of patches is available before discussing further. Jeff Ohlstein sent out a series of patches ([PATCH 0/5] SMP support for msm). The patch that deals with talking to the secure domain code is titled [PATCH 2/5] msm: scm-boot: Support for setting cold/warm boot addresses. I see that you replied to an email on that, but it's not clear if you connected that patch with this thread. Thanks, Saravana Russell, Have you had a chance to look at this? Any comments? How do we move ahead? Thanks, Saravana -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall
On Thu, Dec 09, 2010 at 01:23:24AM -0800, skan...@codeaurora.org wrote: Russell, Have you had a chance to look at this? Any comments? How do we move ahead? I had connected the other thread with this one - it's pretty hard not to as it included this patch in that set as the first patch. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall
Russell King - ARM Linux wrote: On Fri, Dec 03, 2010 at 12:06:53PM -0800, Saravana Kannan wrote: The MSM8660 SoC uses the TrustZone technology and the Linux kernel executes in normal/non-secure domain. When the second core is brought out of reset, it starts executing a secure image which then jumps to secondary_startup. So, before bringing the second core out of reset, we need to inform the secure domain code where secondary_startup is located in memory. We do the communication with the secure code by using buffers in memory. The cache treats the NS (non secure) bit as an additional address bit when tagging memory. Hence, cache accesses are not coherent between the secure and non-secure domains. So, the secure side flushes it's cache after writing to the buffer. To properly read the response from the secure side, the kernel has to pick a buffer that isn't cacheable in the first place. We have similar issues in the reverse direction. So when ARM gets DMA-coherent caches, you of course aren't going to complain that the DMA APIs start avoiding doing the current tricks with non-cacheable memory? I view what you're doing above with the DMA API as an abuse of the API. Just like the problems we're facing with ioremap() being used on system RAM, you're asking for problems when the ARM architecture changes because you're using an API for it's current properties, not for its purpose. You are right. Thanks for catching this. So, that basically leaves us with these options: * Create another API to allow getting uncached pages. I don't think we will be the first or the last to want uncached pages. Even if ARM introduces DMA-coherent caches, it's possible for SoC vendors to have other h/w blocks that could directly operate on memory. The cache might not be coherent with these h/w blocks. * Add a cache invalidate API that's outside the DMA APIs and can be used when needed. Do one of the above two options sound reasonable to you? I've been on for years about purpose-designed APIs for cache issues, and every time someone abuses them, they eventually end up suffering breakage. Let's wait until the full set of patches is available before discussing further. Jeff Ohlstein sent out a series of patches ([PATCH 0/5] SMP support for msm). The patch that deals with talking to the secure domain code is titled [PATCH 2/5] msm: scm-boot: Support for setting cold/warm boot addresses. I see that you replied to an email on that, but it's not clear if you connected that patch with this thread. Thanks, Saravana -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] arm: dma-mapping: move consistent_init to early_initcall
Some machines require the use of coherent memory to bring up auxillary cpus, and thus want to use dma_alloc_coherent prior to smp_init completing. Signed-off-by: Jeff Ohlstein johls...@codeaurora.org --- arch/arm/mm/dma-mapping.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index ac6a361..d46ce8d 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -177,7 +177,7 @@ static int __init consistent_init(void) return ret; } -core_initcall(consistent_init); +early_initcall(consistent_init); static void * __dma_alloc_remap(struct page *page, size_t size, gfp_t gfp, pgprot_t prot) -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
