From: Jie Zhang <[email protected]>
The mmu code uses the copy_*_user_page() variants in access_process_vm()
rather than copy_*_user() as the former includes an icache flush. This is
important when doing things like setting software breakpoints with gdb.
So switch the nommu code over to do the same.
Signed-off-by: Jie Zhang <[email protected]>
Signed-off-by: Mike Frysinger <[email protected]>
---
mm/nommu.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/mm/nommu.c b/mm/nommu.c
index 9876fa0..51ae9be 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -1889,9 +1889,11 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, in
/* only read or write mappings where it is permitted */
if (write && vma->vm_flags & VM_MAYWRITE)
- len -= copy_to_user((void *) addr, buf, len);
+ copy_to_user_page(vma, NULL, NULL,
+ (void *) addr, buf, len);
else if (!write && vma->vm_flags & VM_MAYREAD)
- len -= copy_from_user(buf, (void *) addr, len);
+ copy_from_user_page(vma, NULL, NULL,
+ buf, (void *) addr, len);
else
len = 0;
} else {
--
1.6.5.3
Mike Frysinger wrote:
> From: Jie Zhang <[email protected]>
>
> The mmu code uses the copy_*_user_page() variants in access_process_vm()
> rather than copy_*_user() as the former includes an icache flush. This is
> important when doing things like setting software breakpoints with gdb.
> So switch the nommu code over to do the same.
Reasonable, but it's a bit subtle don't you think?
How about a one-line comment saying why it's using copy_*_user_page()?
(If it was called copy_*_user_flush_icache() I wouldn't say anything,
but it isn't).
> Signed-off-by: Jie Zhang <[email protected]>
> Signed-off-by: Mike Frysinger <[email protected]>
> ---
> mm/nommu.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 9876fa0..51ae9be 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -1889,9 +1889,11 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, in
>
> /* only read or write mappings where it is permitted */
> if (write && vma->vm_flags & VM_MAYWRITE)
> - len -= copy_to_user((void *) addr, buf, len);
> + copy_to_user_page(vma, NULL, NULL,
> + (void *) addr, buf, len);
> else if (!write && vma->vm_flags & VM_MAYREAD)
> - len -= copy_from_user(buf, (void *) addr, len);
> + copy_from_user_page(vma, NULL, NULL,
> + buf, (void *) addr, len);
> else
> len = 0;
> } else {
Jivin Mike Frysinger lays it down ...
> From: Jie Zhang <[email protected]>
>
> The mmu code uses the copy_*_user_page() variants in access_process_vm()
> rather than copy_*_user() as the former includes an icache flush. This is
> important when doing things like setting software breakpoints with gdb.
> So switch the nommu code over to do the same.
>
> Signed-off-by: Jie Zhang <[email protected]>
> Signed-off-by: Mike Frysinger <[email protected]>
Acked-by: David McCullough <[email protected]>
Cheers,
Davidm
> ---
> mm/nommu.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 9876fa0..51ae9be 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -1889,9 +1889,11 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, in
>
> /* only read or write mappings where it is permitted */
> if (write && vma->vm_flags & VM_MAYWRITE)
> - len -= copy_to_user((void *) addr, buf, len);
> + copy_to_user_page(vma, NULL, NULL,
> + (void *) addr, buf, len);
> else if (!write && vma->vm_flags & VM_MAYREAD)
> - len -= copy_from_user(buf, (void *) addr, len);
> + copy_from_user_page(vma, NULL, NULL,
> + buf, (void *) addr, len);
> else
> len = 0;
> } else {
> --
> 1.6.5.3
>
> _______________________________________________
> uClinux-dev mailing list
> [email protected]
> http://mailman.uclinux.org/mailman/listinfo/uclinux-dev
> This message was resent by [email protected]
> To unsubscribe see:
> http://mailman.uclinux.org/mailman/options/uclinux-dev
>
>
--
David McCullough, [email protected], Ph:+61 734352815
McAfee - SnapGear http://www.snapgear.com http://www.uCdot.org
On 11/25/2009 02:16 PM, Jamie Lokier wrote:
> Mike Frysinger wrote:
>> From: Jie Zhang<[email protected]>
>>
>> The mmu code uses the copy_*_user_page() variants in access_process_vm()
>> rather than copy_*_user() as the former includes an icache flush. This is
>> important when doing things like setting software breakpoints with gdb.
>> So switch the nommu code over to do the same.
>
> Reasonable, but it's a bit subtle don't you think?
> How about a one-line comment saying why it's using copy_*_user_page()?
>
> (If it was called copy_*_user_flush_icache() I wouldn't say anything,
> but it isn't).
>
But I think it's well known in Linux kernel developers that
copy_to_user_page and copy_from_user_page should do cache flushing. It's
documented in Documentation/cachetlb.txt. I don't think it's necessary
to replicate it here.
Jie
On Wed, Nov 25, 2009 at 02:27:22PM +0800, Jie Zhang wrote:
> On 11/25/2009 02:16 PM, Jamie Lokier wrote:
> >Mike Frysinger wrote:
> >>From: Jie Zhang<[email protected]>
> >>
> >>The mmu code uses the copy_*_user_page() variants in access_process_vm()
> >>rather than copy_*_user() as the former includes an icache flush. This is
> >>important when doing things like setting software breakpoints with gdb.
> >>So switch the nommu code over to do the same.
> >
> >Reasonable, but it's a bit subtle don't you think?
> >How about a one-line comment saying why it's using copy_*_user_page()?
> >
> >(If it was called copy_*_user_flush_icache() I wouldn't say anything,
> >but it isn't).
> >
> But I think it's well known in Linux kernel developers that
> copy_to_user_page and copy_from_user_page should do cache flushing. It's
> documented in Documentation/cachetlb.txt. I don't think it's necessary
> to replicate it here.
>
Documenting it in the changelog is sufficient I think. Platforms that
need the I-cache flush can deal with it there, and those that don't
aren't going to notice any difference regardless. The change in semantics
is subtle, but as it's bringing it in line with MMU behaviour it's not
really worth noting the fact that NOMMU behaviour used to be different
for no particular reason.
Acked-by: Paul Mundt <[email protected]>
Jie Zhang wrote:
> On 11/25/2009 02:16 PM, Jamie Lokier wrote:
> >Mike Frysinger wrote:
> >>From: Jie Zhang<[email protected]>
> >>
> >>The mmu code uses the copy_*_user_page() variants in access_process_vm()
> >>rather than copy_*_user() as the former includes an icache flush. This is
> >>important when doing things like setting software breakpoints with gdb.
> >>So switch the nommu code over to do the same.
> >
> >Reasonable, but it's a bit subtle don't you think?
> >How about a one-line comment saying why it's using copy_*_user_page()?
> >
> >(If it was called copy_*_user_flush_icache() I wouldn't say anything,
> >but it isn't).
> >
> But I think it's well known in Linux kernel developers that
> copy_to_user_page and copy_from_user_page should do cache flushing. It's
> documented in Documentation/cachetlb.txt. I don't think it's necessary
> to replicate it here.
You're right, however I now think the commit message is misleading.
Since this is the *only place in the entire kernel* where these
functions are used (plus the mmu equivalent), I'm not sure I'd agree
about well known, and the name could be better (copy_*_user_ptrace()),
but I agree now, it doesn't need a comment.
It was the talk of icache flush which bothered me, as I (wrongly)
assumed copy_*_user_page() was used elsewhere, without knowledge of
icache vs non-icache differences - which are often the responsibility
of userspace to get right, so often the kernel does not care.
In fact, it's not just icache. copy_*_user_page() has to do some
*data* cache flushing too, on some architecures. For example, see
arch/sparc/include/asm/cacheflush_64.h:
#define copy_to_user_page(vma, page, vaddr, dst, src, len) \
do { \
flush_cache_page(vma, vaddr, page_to_pfn(page)); \
memcpy(dst, src, len); \
flush_ptrace_access(vma, page, vaddr, src, len, 0); \
} while (0)
#define copy_from_user_page(vma, page, vaddr, dst, src, len) \
do { \
flush_cache_page(vma, vaddr, page_to_pfn(page)); \
memcpy(dst, src, len); \
flush_ptrace_access(vma, page, vaddr, dst, len, 1); \
} while (0)
I'm not sure why I don't see the same dcache flushing on ARM, so I
wonder if the ARM implementation of these buggy.
Anyway, that means the commit message is a little misleading, saying
it's for the icache flush. It is for whatever icache and dcache
flushes are needed for a ptrace access.
Which is why, given they are only used for ptrace (have just grepped),
I'm inclined to think it'd be clearer to rename the functions to
copy_*_user_ptrace(). And make your no-mmu change of course :-)
Any thoughts on the rename?
-- Jamie
On 11/25/2009 07:49 PM, Jamie Lokier wrote:
> Jie Zhang wrote:
>> On 11/25/2009 02:16 PM, Jamie Lokier wrote:
>>> Mike Frysinger wrote:
>>>> From: Jie Zhang<[email protected]>
>>>>
>>>> The mmu code uses the copy_*_user_page() variants in access_process_vm()
>>>> rather than copy_*_user() as the former includes an icache flush. This is
>>>> important when doing things like setting software breakpoints with gdb.
>>>> So switch the nommu code over to do the same.
>>>
>>> Reasonable, but it's a bit subtle don't you think?
>>> How about a one-line comment saying why it's using copy_*_user_page()?
>>>
>>> (If it was called copy_*_user_flush_icache() I wouldn't say anything,
>>> but it isn't).
>>>
>> But I think it's well known in Linux kernel developers that
>> copy_to_user_page and copy_from_user_page should do cache flushing. It's
>> documented in Documentation/cachetlb.txt. I don't think it's necessary
>> to replicate it here.
>
> You're right, however I now think the commit message is misleading.
>
> Since this is the *only place in the entire kernel* where these
> functions are used (plus the mmu equivalent), I'm not sure I'd agree
> about well known, and the name could be better (copy_*_user_ptrace()),
> but I agree now, it doesn't need a comment.
>
> It was the talk of icache flush which bothered me, as I (wrongly)
> assumed copy_*_user_page() was used elsewhere, without knowledge of
> icache vs non-icache differences - which are often the responsibility
> of userspace to get right, so often the kernel does not care.
>
> In fact, it's not just icache. copy_*_user_page() has to do some
> *data* cache flushing too, on some architecures. For example, see
You are right. We needs dcache flushing here, too. However, for harvard
architecture, flushing icache implies you have to flush dcache. So in
implementation, there is dcache flushing in flush_icache_range, as we do
for Blackfin, as well as they do for ARM. So when we say icache
flushing, dcache flushing is implied.
> I'm not sure why I don't see the same dcache flushing on ARM, so I
> wonder if the ARM implementation of these buggy.
>
I'm not familiar with ARM. But I believe they do dcache flushing after
some grepping in arch/arm/*.
> Which is why, given they are only used for ptrace (have just grepped),
> I'm inclined to think it'd be clearer to rename the functions to
> copy_*_user_ptrace(). And make your no-mmu change of course :-)
> Any thoughts on the rename?
>
I have no opinion on renaming things. I can live with the current naming.
Jie
On Wed, Nov 25, 2009 at 06:49, Jamie Lokier wrote:
> Jie Zhang wrote:
>> On 11/25/2009 02:16 PM, Jamie Lokier wrote:
>> >Mike Frysinger wrote:
>> >>From: Jie Zhang<[email protected]>
>> >>
>> >>The mmu code uses the copy_*_user_page() variants in access_process_vm()
>> >>rather than copy_*_user() as the former includes an icache flush. This is
>> >>important when doing things like setting software breakpoints with gdb.
>> >>So switch the nommu code over to do the same.
>> >
>> >Reasonable, but it's a bit subtle don't you think?
>> >How about a one-line comment saying why it's using copy_*_user_page()?
>> >
>> >(If it was called copy_*_user_flush_icache() I wouldn't say anything,
>> >but it isn't).
>> >
>> But I think it's well known in Linux kernel developers that
>> copy_to_user_page and copy_from_user_page should do cache flushing. It's
>> documented in Documentation/cachetlb.txt. I don't think it's necessary
>> to replicate it here.
>
> You're right, however I now think the commit message is misleading.
>
> Since this is the *only place in the entire kernel* where these
> functions are used (plus the mmu equivalent), I'm not sure I'd agree
> about well known, and the name could be better (copy_*_user_ptrace()),
> but I agree now, it doesn't need a comment.
>
> It was the talk of icache flush which bothered me, as I (wrongly)
> assumed copy_*_user_page() was used elsewhere, without knowledge of
> icache vs non-icache differences - which are often the responsibility
> of userspace to get right, so often the kernel does not care.
>
> In fact, it's not just icache. copy_*_user_page() has to do some
> *data* cache flushing too, on some architecures. For example, see
> arch/sparc/include/asm/cacheflush_64.h:
>
> #define copy_to_user_page(vma, page, vaddr, dst, src, len) \
> do { \
> flush_cache_page(vma, vaddr, page_to_pfn(page)); \
> memcpy(dst, src, len); \
> flush_ptrace_access(vma, page, vaddr, src, len, 0); \
> } while (0)
>
> #define copy_from_user_page(vma, page, vaddr, dst, src, len) \
> do { \
> flush_cache_page(vma, vaddr, page_to_pfn(page)); \
> memcpy(dst, src, len); \
> flush_ptrace_access(vma, page, vaddr, dst, len, 1); \
> } while (0)
>
> I'm not sure why I don't see the same dcache flushing on ARM, so I
> wonder if the ARM implementation of these buggy.
>
> Anyway, that means the commit message is a little misleading, saying
> it's for the icache flush. It is for whatever icache and dcache
> flushes are needed for a ptrace access.
>
> Which is why, given they are only used for ptrace (have just grepped),
> I'm inclined to think it'd be clearer to rename the functions to
> copy_*_user_ptrace(). And make your no-mmu change of course :-)
> Any thoughts on the rename?
these are all good points, but i think unrelated to the patch in
question ;). they can be done as a follow up.
-mike
Mike Frysinger wrote:
> From: Jie Zhang <[email protected]>
>
> The mmu code uses the copy_*_user_page() variants in access_process_vm()
> rather than copy_*_user() as the former includes an icache flush. This is
> important when doing things like setting software breakpoints with gdb.
> So switch the nommu code over to do the same.
>
> Signed-off-by: Jie Zhang <[email protected]>
> Signed-off-by: Mike Frysinger <[email protected]>
Acked-by: Greg Ungerer <[email protected]>
> mm/nommu.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 9876fa0..51ae9be 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -1889,9 +1889,11 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, in
>
> /* only read or write mappings where it is permitted */
> if (write && vma->vm_flags & VM_MAYWRITE)
> - len -= copy_to_user((void *) addr, buf, len);
> + copy_to_user_page(vma, NULL, NULL,
> + (void *) addr, buf, len);
> else if (!write && vma->vm_flags & VM_MAYREAD)
> - len -= copy_from_user(buf, (void *) addr, len);
> + copy_from_user_page(vma, NULL, NULL,
> + buf, (void *) addr, len);
> else
> len = 0;
> } else {
--
------------------------------------------------------------------------
Greg Ungerer -- Principal Engineer EMAIL: [email protected]
SnapGear Group, McAfee PHONE: +61 7 3435 2888
8 Gardner Close FAX: +61 7 3217 5323
Milton, QLD, 4064, Australia WEB: http://www.SnapGear.com
Mike Frysinger <[email protected]> wrote:
> - len -= copy_to_user((void *) addr, buf, len);
> + copy_to_user_page(vma, NULL, NULL,
> + (void *) addr, buf, len);
> else if (!write && vma->vm_flags & VM_MAYREAD)
> - len -= copy_from_user(buf, (void *) addr, len);
> + copy_from_user_page(vma, NULL, NULL,
> + buf, (void *) addr, len);
Hmmm... With this, len isn't updated anymore, and so it alters the return
value of access_process_vm(), and means ptrace_readdata() won't now return
-EIO under some circumstances where it used to. I'm not sure that matters,
though.
David
Mike Frysinger <[email protected]> wrote:
> The mmu code uses the copy_*_user_page() variants in access_process_vm()
> rather than copy_*_user() as the former includes an icache flush. This is
> important when doing things like setting software breakpoints with gdb.
> So switch the nommu code over to do the same.
Note that we may only really want to do an icache flush if the target region
is mapped executable somewhere. On the other hand, for debugging stuff on an
embedded board, it probably doesn't matter.
David
On 12/02/2009 10:36 PM, David Howells wrote:
> Mike Frysinger<[email protected]> wrote:
>
>> - len -= copy_to_user((void *) addr, buf, len);
>> + copy_to_user_page(vma, NULL, NULL,
>> + (void *) addr, buf, len);
>> else if (!write&& vma->vm_flags& VM_MAYREAD)
>> - len -= copy_from_user(buf, (void *) addr, len);
>> + copy_from_user_page(vma, NULL, NULL,
>> + buf, (void *) addr, len);
>
> Hmmm... With this, len isn't updated anymore, and so it alters the return
> value of access_process_vm(), and means ptrace_readdata() won't now return
> -EIO under some circumstances where it used to. I'm not sure that matters,
> though.
>
This keeps access_process_vm() in nommu.c align with the one in
memory.c. If this does really matter, someone or me can write another
patch to take care of it for both MMU and !MMU later.
Jie
On 12/02/2009 10:45 PM, David Howells wrote:
> Mike Frysinger<[email protected]> wrote:
>
>> The mmu code uses the copy_*_user_page() variants in access_process_vm()
>> rather than copy_*_user() as the former includes an icache flush. This is
>> important when doing things like setting software breakpoints with gdb.
>> So switch the nommu code over to do the same.
>
> Note that we may only really want to do an icache flush if the target region
> is mapped executable somewhere. On the other hand, for debugging stuff on an
> embedded board, it probably doesn't matter.
>
It is not checked before icache flush even for MMU. This is a place we
can do some improvement. But it's not important for debugging.
Jie
Mike Frysinger <[email protected]> wrote:
> + copy_to_user_page(vma, NULL, NULL,
> + (void *) addr, buf, len);
> ...
> + copy_from_user_page(vma, NULL, NULL,
> + buf, (void *) addr, len);
I think this is not correct. The third parameter in both cases (vaddr) is of
unsigned long type (so should be 0 not NULL), and should not be left zero in
any case. I think it should be passed addr. In fact, we should really pass
the second parameter too (page), though for now, I'm happy to leave that NULL.
See attached revision of the patch.
David
---
From: Jie Zhang <[email protected]>
Subject: [PATCH] NOMMU: Use copy_*_user_page() in access_process_vm()
The MMU code uses the copy_*_user_page() variants in access_process_vm()
rather than copy_*_user() as the former includes an icache flush. This is
important when doing things like setting software breakpoints with gdb.
So switch the NOMMU code over to do the same.
This patch makes the reasonable assumption that copy_from_user_page() won't
fail - which is probably fine, as we've checked the VMA from which we're
copying is usable, and the copy is not allowed to cross VMAs. The one case
where it might go wrong is if the VMA is a device rather than RAM, and that
device returns an error which - in which case rubbish will be returned rather
than EIO.
Signed-off-by: Jie Zhang <[email protected]>
Signed-off-by: Mike Frysinger <[email protected]>
Signed-off-by: David Howells <[email protected]>
---
mm/nommu.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/mm/nommu.c b/mm/nommu.c
index af12270..953800f 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -1896,9 +1896,11 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, in
/* only read or write mappings where it is permitted */
if (write && vma->vm_flags & VM_MAYWRITE)
- len -= copy_to_user((void *) addr, buf, len);
+ copy_to_user_page(vma, NULL, addr,
+ (void *) addr, buf, len);
else if (!write && vma->vm_flags & VM_MAYREAD)
- len -= copy_from_user(buf, (void *) addr, len);
+ copy_from_user_page(vma, NULL, addr,
+ buf, (void *) addr, len);
else
len = 0;
} else {
On 12/08/2009 06:57 PM, David Howells wrote:
> Mike Frysinger<[email protected]> wrote:
>
>> + copy_to_user_page(vma, NULL, NULL,
>> + (void *) addr, buf, len);
>> ...
>> + copy_from_user_page(vma, NULL, NULL,
>> + buf, (void *) addr, len);
>
> I think this is not correct. The third parameter in both cases (vaddr) is of
> unsigned long type (so should be 0 not NULL), and should not be left zero in
> any case. I think it should be passed addr. In fact, we should really pass
> the second parameter too (page), though for now, I'm happy to leave that NULL.
>
> See attached revision of the patch.
>
I agree on using 0 instead of NULL. But for !MMU, I think, vaddr is
always as same as addr. So we don't need to pass it?
Jie
> David
> ---
> From: Jie Zhang<[email protected]>
> Subject: [PATCH] NOMMU: Use copy_*_user_page() in access_process_vm()
>
> The MMU code uses the copy_*_user_page() variants in access_process_vm()
> rather than copy_*_user() as the former includes an icache flush. This is
> important when doing things like setting software breakpoints with gdb.
> So switch the NOMMU code over to do the same.
>
> This patch makes the reasonable assumption that copy_from_user_page() won't
> fail - which is probably fine, as we've checked the VMA from which we're
> copying is usable, and the copy is not allowed to cross VMAs. The one case
> where it might go wrong is if the VMA is a device rather than RAM, and that
> device returns an error which - in which case rubbish will be returned rather
> than EIO.
>
> Signed-off-by: Jie Zhang<[email protected]>
> Signed-off-by: Mike Frysinger<[email protected]>
> Signed-off-by: David Howells<[email protected]>
> ---
>
> mm/nommu.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
>
> diff --git a/mm/nommu.c b/mm/nommu.c
> index af12270..953800f 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -1896,9 +1896,11 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, in
>
> /* only read or write mappings where it is permitted */
> if (write&& vma->vm_flags& VM_MAYWRITE)
> - len -= copy_to_user((void *) addr, buf, len);
> + copy_to_user_page(vma, NULL, addr,
> + (void *) addr, buf, len);
> else if (!write&& vma->vm_flags& VM_MAYREAD)
> - len -= copy_from_user(buf, (void *) addr, len);
> + copy_from_user_page(vma, NULL, addr,
> + buf, (void *) addr, len);
> else
> len = 0;
> } else {
Jie Zhang <[email protected]> wrote:
> I agree on using 0 instead of NULL. But for !MMU, I think, vaddr is always as
> same as addr. So we don't need to pass it?
FRV flushes the vaddr because in MMU mode the cache flush instructions take
virtual addresses, so if we pass addr as vaddr, I can use the same cache flush
code for both modes. I suspect it makes little difference to the amount of
code if we pass that rather than 0, as the value is already computed, and
either way, it's going to take one instruction to set up the argument.
Note that Blackfin assumes that it may use the dst address for flushing - an
assumption that isn't valid in MMU mode with a VIVT cache (which I presume
Blackfin isn't, but other CPUs are).
David
On 12/08/2009 10:19 PM, David Howells wrote:
> Jie Zhang<[email protected]> wrote:
>
>> I agree on using 0 instead of NULL. But for !MMU, I think, vaddr is always as
>> same as addr. So we don't need to pass it?
>
> FRV flushes the vaddr because in MMU mode the cache flush instructions take
> virtual addresses, so if we pass addr as vaddr, I can use the same cache flush
> code for both modes. I suspect it makes little difference to the amount of
> code if we pass that rather than 0, as the value is already computed, and
> either way, it's going to take one instruction to set up the argument.
>
> Note that Blackfin assumes that it may use the dst address for flushing - an
> assumption that isn't valid in MMU mode with a VIVT cache (which I presume
> Blackfin isn't, but other CPUs are).
>
Thanks for your explanation. Now I understand why passing add as vaddr
is better.
Jie
On Tuesday 08 December 2009 09:19:27 David Howells wrote:
> Jie Zhang <[email protected]> wrote:
> > I agree on using 0 instead of NULL. But for !MMU, I think, vaddr is
> > always as same as addr. So we don't need to pass it?
>
> FRV flushes the vaddr because in MMU mode the cache flush instructions take
> virtual addresses, so if we pass addr as vaddr, I can use the same cache
> flush code for both modes. I suspect it makes little difference to the
> amount of code if we pass that rather than 0, as the value is already
> computed, and either way, it's going to take one instruction to set up the
> argument.
>
> Note that Blackfin assumes that it may use the dst address for flushing -
> an assumption that isn't valid in MMU mode with a VIVT cache (which I
> presume Blackfin isn't, but other CPUs are).
the Blackfin cpu currently does not have virtual memory support at all, so i
imagine there's a bunch of things that'll need updating when that day comes
the new patch is of course fine; thanks
-mike