Re: [uClinux-dev] [PATCH 1/3] MPU support

2010-08-24 Thread Steve Longerbeam

On 08/23/2010 01:18 PM, Mike Frysinger wrote:

On Monday, August 23, 2010 14:16:30 Steve Longerbeam wrote:
   

On 08/23/2010 10:47 AM, Steve Longerbeam wrote:
 

On 08/22/2010 05:20 PM, Mike Frysinger wrote:
   

the common code changes will need justification as to why they exist
   

at all.
we're doing MPU on Blackfin/nommu today without any of these.  we
support
pretty much all the same features of a MMU system short of virtual
memory --
4k pages, RWX granularity, process to process protection, process to
kernel
protection (include kernel modules), kernel XIP, and userspace XIP.

further, why did you go with CONFIG_CPU_CP15_MPU ?  there is already a
CONFIG_MPU option that is used in common nommu code.
 

which tree has CONFIG_MPU, and the MPU support for blackfin? There is
no such thing in the 888 uclinux release tree.
   

sorry, I see CONFIG_MPU under blackfin in the 888 release.

I'm not familiar with the blackfin arch, but my patches of course are
specific to ARM MPU's.
 

i dont see how the processor matters.  you're running Linux without virtual
memory support (CONFIG_MMU=n) and you want to do memory protection
(CONFIG_MPU=y).  there is no need to stick a specific cpu name in there.
after all, the option is CONFIG_MPU and not CONFIG_BFIN_MPU because all the
changes we made (which were few) to common code were processor independent
(exactly like all changes to common code should be).  we specifically left the
door open for other processors to support MMU=n MPU=y without an ifdef mess.
-mike
   


Hi Mike, I don't disagree with what you're saying, but I was a bit 
confused because in the 888 kernel I was looking at, the common MPU code 
didn't exist yet.


But looking at the 20100628 release, I can see mm/nommu:protect_vma(), 
that calls the common MPU API's protect_page() and update_protections().


Apparently the ARM MPU's are not nearly as capable as the blackfin MPU. 
The ARM MPU deals with whole regions, and typically only up to 8 memory 
regions can be controlled by the MPU at any one time, each region having 
one protection setting (r/w/x for kernel mode, r/w/x for user mode). Not 
nearly as fine grained as per-page.


So ARM could use something higher-level than protect_page(), something 
like protect_region(start, end, flags), or just all of protect_vma() 
could be moved to include/asm/mmu_context.h. That way ARM can operate on 
the whole region, while blackfin would add protection for every page in 
the VMA as it is doing now.


I'll work on another patch that better merges my original ARM MPU work 
into the blackfin work, and resubmit.


Btw, I probably should be working in whatever git tree people are 
submitting patches against, rather than the 20100628 release. Which git 
tree should I submit against?


Steve


___
uClinux-dev mailing list
uClinux-dev@uclinux.org
http://mailman.uclinux.org/mailman/listinfo/uclinux-dev
This message was resent by uclinux-dev@uclinux.org
To unsubscribe see:
http://mailman.uclinux.org/mailman/options/uclinux-dev


[uClinux-dev] [PATCH] NOMMU: Stub out vm_get_page_prot() if there's no MMU

2010-08-24 Thread David Howells
Stub out vm_get_page_prot() if there's no MMU.

This was added by commit:

commit 804af2cf6e7af31d2e664b54e6579b531dbd
Author: Hugh Dickins h...@veritas.com
Date:   Wed Jul 26 21:39:49 2006 +0100
Subject: [AGPGART] remove private page protection map

and is used in commit:

commit c07fbfd17e614a76b194f371c5331e21e6cffb54
Author: Daniel De Graaf dgde...@tycho.nsa.gov
Date:   Tue Aug 10 18:02:45 2010 -0700
Subject: fbmem: VM_IO set, but not propagated

in the fbmem video driver, but the function doesn't exist on NOMMU, resulting
in an undefined symbol at link time.

Signed-off-by: David Howells dhowe...@redhat.com
---

 include/linux/mm.h |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 709f672..6b258c5 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1357,7 +1357,15 @@ static inline unsigned long vma_pages(struct 
vm_area_struct *vma)
return (vma-vm_end - vma-vm_start)  PAGE_SHIFT;
 }
 
+#ifdef CONFIG_MMU
 pgprot_t vm_get_page_prot(unsigned long vm_flags);
+#else
+static inline pgprot_t vm_get_page_prot(unsigned long vm_flags)
+{
+   return __pgprot(0);
+}
+#endif
+
 struct vm_area_struct *find_extend_vma(struct mm_struct *, unsigned long addr);
 int remap_pfn_range(struct vm_area_struct *, unsigned long addr,
unsigned long pfn, unsigned long size, pgprot_t);

___
uClinux-dev mailing list
uClinux-dev@uclinux.org
http://mailman.uclinux.org/mailman/listinfo/uclinux-dev
This message was resent by uclinux-dev@uclinux.org
To unsubscribe see:
http://mailman.uclinux.org/mailman/options/uclinux-dev


Re: [uClinux-dev] [PATCH 1/3] MPU support

2010-08-24 Thread Mike Frysinger
On Tuesday, August 24, 2010 18:06:14 Steve Longerbeam wrote:
 On 08/23/2010 01:18 PM, Mike Frysinger wrote:
  On Monday, August 23, 2010 14:16:30 Steve Longerbeam wrote:
  sorry, I see CONFIG_MPU under blackfin in the 888 release.
  
  I'm not familiar with the blackfin arch, but my patches of course are
  specific to ARM MPU's.
  
  i dont see how the processor matters.  you're running Linux without
  virtual memory support (CONFIG_MMU=n) and you want to do memory
  protection (CONFIG_MPU=y).  there is no need to stick a specific cpu
  name in there. after all, the option is CONFIG_MPU and not
  CONFIG_BFIN_MPU because all the changes we made (which were few) to
  common code were processor independent (exactly like all changes to
  common code should be).  we specifically left the door open for other
  processors to support MMU=n MPU=y without an ifdef mess. -mike
 
 Hi Mike, I don't disagree with what you're saying, but I was a bit
 confused because in the 888 kernel I was looking at, the common MPU code
 didn't exist yet.

i focus on the mainline kernels, so my comments might be more up-to-date than 
some of the uclinux patchsets.  cant say ive ever used any of them.

 Apparently the ARM MPU's are not nearly as capable as the blackfin MPU.
 The ARM MPU deals with whole regions, and typically only up to 8 memory
 regions can be controlled by the MPU at any one time, each region having
 one protection setting (r/w/x for kernel mode, r/w/x for user mode). Not
 nearly as fine grained as per-page.

i dont quite understand what you mean by whole region.  if you define a 
region as 4KiB, dont you get the granularity expected ?  could you describe 
the flexibility/restrictions of this a little more (i'm not an ARM core guy) ?

the Blackfin MPU has separate insn/data TLBs, and each TLB has 16 entries 
(PTEs i believe is the common naming).  each PTE has supervisor rwx and 
usermode rwx permissions.  further, each PTE has a size field which may be 
1KiB, 4KiB, 1MiB, or 4MiB.

i guess we cheat a little and we lock a PTE for the kernel itself so that 
it'll always be covered so we can process PTE misses without triggering a miss 
(nested exceptions).  i'm not entirely familiar with the exact gory details of 
other arches, so i cant say how unique we are in this regard.

 So ARM could use something higher-level than protect_page(), something
 like protect_region(start, end, flags), or just all of protect_vma()
 could be moved to include/asm/mmu_context.h. That way ARM can operate on
 the whole region, while blackfin would add protection for every page in
 the VMA as it is doing now.

i think you could use the existing framework, and perhaps optionally extend 
it.  maybe if i knew a little more about your regions, i could suggest 
something else.

 I'll work on another patch that better merges my original ARM MPU work
 into the blackfin work, and resubmit.

great, thanks

 Btw, I probably should be working in whatever git tree people are
 submitting patches against, rather than the 20100628 release. Which git
 tree should I submit against?

that's hard to say.  if current mainline (2.6.36-rc2) has everything you need 
to boot a working system, then that is probably the place to base your work.  
i understand though that the arm/nommu work is taking a while to get into 
mainline, so that might not be feasible.  in which case, you should find the 
very latest uclinux tree and use that.

i know people like to base their work off a release, but in order to get 
merged, the focus has to be on the latest development tree.
-mike


signature.asc
Description: This is a digitally signed message part.
___
uClinux-dev mailing list
uClinux-dev@uclinux.org
http://mailman.uclinux.org/mailman/listinfo/uclinux-dev
This message was resent by uclinux-dev@uclinux.org
To unsubscribe see:
http://mailman.uclinux.org/mailman/options/uclinux-dev

RE: [uClinux-dev] [PATCH] NOMMU: Stub out vm_get_page_prot() if there's no MMU

2010-08-24 Thread Gavin Lambert
Quoth David Howells:
 Stub out vm_get_page_prot() if there's no MMU.
[...]
 in the fbmem video driver, but the function doesn't exist on NOMMU,
 resulting in an undefined symbol at link time.
[...]
 +#ifdef CONFIG_MMU
  pgprot_t vm_get_page_prot(unsigned long vm_flags);
 +#else
 +static inline pgprot_t vm_get_page_prot(unsigned long vm_flags)
 +{
 + return __pgprot(0);
 +}
 +#endif

Wouldn't it be better to define this in the .c for !CONFIG_MMU, rather than 
stub it out in the headers?  After all, it's possible that some arches (eg. 
Blackfin, since it has an MPU) might want to actually implement it.



___
uClinux-dev mailing list
uClinux-dev@uclinux.org
http://mailman.uclinux.org/mailman/listinfo/uclinux-dev
This message was resent by uclinux-dev@uclinux.org
To unsubscribe see:
http://mailman.uclinux.org/mailman/options/uclinux-dev


Re: [uClinux-dev] [PATCH] NOMMU: Stub out vm_get_page_prot() if there's no MMU

2010-08-24 Thread Mike Frysinger
On Tuesday, August 24, 2010 19:09:43 Gavin Lambert wrote:
 Quoth David Howells:
  Stub out vm_get_page_prot() if there's no MMU.
 
 [...]
 
  in the fbmem video driver, but the function doesn't exist on NOMMU,
  resulting in an undefined symbol at link time.
 
 [...]
 
  +#ifdef CONFIG_MMU
  
   pgprot_t vm_get_page_prot(unsigned long vm_flags);
  
  +#else
  +static inline pgprot_t vm_get_page_prot(unsigned long vm_flags)
  +{
  +   return __pgprot(0);
  +}
  +#endif
 
 Wouldn't it be better to define this in the .c for !CONFIG_MMU, rather than
 stub it out in the headers?  After all, it's possible that some arches
 (eg. Blackfin, since it has an MPU) might want to actually implement it.

even in the MPU case, i think we'd still want a header change.

#if defined(CONFIG_MMU) || defined(CONFIG_MPU)
pgprot_t vm_get_page_prot(unsigned long vm_flags);
#else
/* stub */
#endif

none of the MPU code today uses this vm field, so there'd be a bit more work 
before we could handle it and worry about the function.  which is why i think 
we should go with this change today, and worry about the rest tomorrow.
-mike


signature.asc
Description: This is a digitally signed message part.
___
uClinux-dev mailing list
uClinux-dev@uclinux.org
http://mailman.uclinux.org/mailman/listinfo/uclinux-dev
This message was resent by uclinux-dev@uclinux.org
To unsubscribe see:
http://mailman.uclinux.org/mailman/options/uclinux-dev