[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Jeff Ohlstein]

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 <johlstei@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.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Jeff Ohlstein]

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 <johlstei@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.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Jeff Ohlstein]

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 <johlstei@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.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Russell King - ARM Linux]

On Thu, Dec 02, 2010 at 02:11:18PM -0800, Jeff Ohlstein wrote:
> 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.

I'd like to see the rest of the code to indicate why you need DMA
coherent memory for SMP boot.  It seems to me quite unnecessary.
DMA coherent memory is meant for talking to devices, not to other
CPUs which will be part of the symmetric part of the system.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Russell King - ARM Linux]

On Thu, Dec 02, 2010 at 02:11:18PM -0800, Jeff Ohlstein wrote:
> 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.

I'd like to see the rest of the code to indicate why you need DMA
coherent memory for SMP boot.  It seems to me quite unnecessary.
DMA coherent memory is meant for talking to devices, not to other
CPUs which will be part of the symmetric part of the system.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

Russell King - ARM Linux wrote:
> On Thu, Dec 02, 2010 at 02:11:18PM -0800, Jeff Ohlstein wrote:
>> 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.
> 
> I'd like to see the rest of the code to indicate why you need DMA
> coherent memory for SMP boot.  It seems to me quite unnecessary.
> DMA coherent memory is meant for talking to devices, not to other
> CPUs which will be part of the symmetric part of the system.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

The code that needs this change will be submitted soon (hopefully in a 
day or two).

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.

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.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

Russell King - ARM Linux wrote:
> On Thu, Dec 02, 2010 at 02:11:18PM -0800, Jeff Ohlstein wrote:
>> 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.
> 
> I'd like to see the rest of the code to indicate why you need DMA
> coherent memory for SMP boot.  It seems to me quite unnecessary.
> DMA coherent memory is meant for talking to devices, not to other
> CPUs which will be part of the symmetric part of the system.
> --
> 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

The code that needs this change will be submitted soon (hopefully in a 
day or two).

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.

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.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Russell King - ARM Linux]

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.

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.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Russell King - ARM Linux]

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.

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.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Jamie Iles]

On Fri, Dec 03, 2010 at 08:36:53PM +0000, 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.
Is the secure world running with the MMU enabled (or is there enough onchip 
memory for page tables in the secure world)? If so, could you recreate the 
direct-mapping in the secure world with the same attributes and the nonsecure 
bit set in the page table descriptors?  The cache should be coherent between 
both worlds in this case.

Alternatively could you not pass the address to the monitor mode in a 
register?

Jamie

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Jamie Iles]

On Fri, Dec 03, 2010 at 08:36:53PM +0000, 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.
Is the secure world running with the MMU enabled (or is there enough onchip 
memory for page tables in the secure world)? If so, could you recreate the 
direct-mapping in the secure world with the same attributes and the nonsecure 
bit set in the page table descriptors?  The cache should be coherent between 
both worlds in this case.

Alternatively could you not pass the address to the monitor mode in a 
register?

Jamie

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

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.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

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.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[skannan]

> 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.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[skannan]

> 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.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[skannan at codeaurora.org]

> 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.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Russell King - ARM Linux]

On Thu, Dec 09, 2010 at 01:23:24AM -0800, skannan@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.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Russell King - ARM Linux]

On Thu, Dec 09, 2010 at 01:23:24AM -0800, skannan at 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.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

Russell King - ARM Linux wrote:
> On Thu, Dec 09, 2010 at 01:23:24AM -0800, skannan@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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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?

-Saravana

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

Russell King - ARM Linux wrote:
> On Thu, Dec 09, 2010 at 01:23:24AM -0800, skannan at 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 majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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?

-Saravana

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Catalin Marinas]

On 10 December 2010 00:58, Saravana Kannan <skannan@codeaurora.org> wrote:
> Russell King - ARM Linux wrote:
>>
>> On Thu, Dec 09, 2010 at 01:23:24AM -0800, skannan@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

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Catalin Marinas]

On 10 December 2010 00:58, Saravana Kannan <skannan@codeaurora.org> wrote:
> Russell King - ARM Linux wrote:
>>
>> On Thu, Dec 09, 2010 at 01:23:24AM -0800, skannan at 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

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

On 12/10/2010 02:00 AM, Catalin Marinas wrote:
> On 10 December 2010 00:58, Saravana Kannan<skannan@codeaurora.org>  wrote:
>> Russell King - ARM Linux wrote:
>>>
>>> On Thu, Dec 09, 2010 at 01:23:24AM -0800, skannan@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.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

On 12/10/2010 02:00 AM, Catalin Marinas wrote:
> On 10 December 2010 00:58, Saravana Kannan<skannan@codeaurora.org>  wrote:
>> Russell King - ARM Linux wrote:
>>>
>>> On Thu, Dec 09, 2010 at 01:23:24AM -0800, skannan at 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.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Catalin Marinas]

On 12 December 2010 04:58, Saravana Kannan <skannan@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

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Catalin Marinas]

On 12 December 2010 04:58, Saravana Kannan <skannan@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

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

> On 12 December 2010 04:58, Saravana Kannan <skannan@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.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

> On 12 December 2010 04:58, Saravana Kannan <skannan@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.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

> On 12 December 2010 04:58, Saravana Kannan <skannan@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.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Russell King - ARM Linux]

On Thu, Dec 16, 2010 at 06:55:06PM -0800, Saravana Kannan wrote:
> 
> > On 12 December 2010 04:58, Saravana Kannan <skannan@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.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Russell King - ARM Linux]

On Thu, Dec 16, 2010 at 06:55:06PM -0800, Saravana Kannan wrote:
> 
> > On 12 December 2010 04:58, Saravana Kannan <skannan@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.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

>> 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.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

>> 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.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

>> 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.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Russell King - ARM Linux]

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.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Russell King - ARM Linux]

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.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

> 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.

I will wait to hear Catalin's clarification before I proceed further with
this point.

>
> 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.
>

Yes, I saw them and even pointed out a few issues during internal-prelim
review. The new patches get around it with what we hope is a temporary
solution -- implement cache invalidate in assembly and call it over a
kmalloc'ed and page aligned memory.

-Saravana
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

> 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.

I will wait to hear Catalin's clarification before I proceed further with
this point.

>
> 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.
>

Yes, I saw them and even pointed out a few issues during internal-prelim
review. The new patches get around it with what we hope is a temporary
solution -- implement cache invalidate in assembly and call it over a
kmalloc'ed and page aligned memory.

-Saravana
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

> 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.

I will wait to hear Catalin's clarification before I proceed further with
this point.

>
> 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.
>

Yes, I saw them and even pointed out a few issues during internal-prelim
review. The new patches get around it with what we hope is a temporary
solution -- implement cache invalidate in assembly and call it over a
kmalloc'ed and page aligned memory.

-Saravana
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Catalin Marinas]

On 17 December 2010 10:26, Saravana Kannan <skannan@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

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Catalin Marinas]

On 17 December 2010 10:26, Saravana Kannan <skannan@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.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

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?

-Saravana
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

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?

-Saravana
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

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?

-Saravana
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

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.

[PATCH] arm: dma-mapping: move consistent_init to early_initcall

[Saravana Kannan]

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.