Patchwork [1/2] mtd: physmap: add physmap_unmapped_area() for no-mmu XIP

login
register
mail settings
Submitter Mike Frysinger
Date May 23, 2010, 7:30 a.m.
Message ID <1274599824-17530-1-git-send-email-vapier@gentoo.org>
Download mbox | patch
Permalink /patch/53302/
State New
Headers show

Comments

Mike Frysinger - May 23, 2010, 7:30 a.m.
From: Bernd Schmidt <bernds_cb1@t-online.de>

Currently, romfs XIP doesn't work in flash memory (the kernel crashes
with a null pointer dereference).  The problem is that the mtd physmap
driver isn't setting up a get_unmapped_area pointer for the mtd
partitions it creates.

Signed-off-by: Bernd Schmidt <bernds_cb1@t-online.de>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/mtd/maps/physmap.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)
Artem Bityutskiy - June 12, 2010, 2:26 p.m.
On Sun, 2010-05-23 at 03:30 -0400, Mike Frysinger wrote:
> From: Bernd Schmidt <bernds_cb1@t-online.de>
> 
> Currently, romfs XIP doesn't work in flash memory (the kernel crashes
> with a null pointer dereference).  The problem is that the mtd physmap
> driver isn't setting up a get_unmapped_area pointer for the mtd
> partitions it creates.
> 
> Signed-off-by: Bernd Schmidt <bernds_cb1@t-online.de>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

Pushed both to my l2-mtd-2.6.git / dunno.
David Woodhouse - Aug. 4, 2010, 9:33 a.m.
On Sat, 2010-06-12 at 17:26 +0300, Artem Bityutskiy wrote:
> On Sun, 2010-05-23 at 03:30 -0400, Mike Frysinger wrote:
> > From: Bernd Schmidt <bernds_cb1@t-online.de>
> > 
> > Currently, romfs XIP doesn't work in flash memory (the kernel crashes
> > with a null pointer dereference).  The problem is that the mtd physmap
> > driver isn't setting up a get_unmapped_area pointer for the mtd
> > partitions it creates.
> > 
> > Signed-off-by: Bernd Schmidt <bernds_cb1@t-online.de>
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> 
> Pushed both to my l2-mtd-2.6.git / dunno.

I'm confused by the second patch -- isn't this done in add_mtd_device()
already, according to the device type?

You're setting mtd_bdi_ro_mappable indiscriminately for all types of
devices. For flash devices, that can be an issue -- if you schedule a
userspace process while the kernel is writing to the flash, for example.

If you want to allow XIP of writable flash devices, there's more to it
than this.
Mike Frysinger - Aug. 7, 2010, 11:26 p.m.
On Wed, Aug 4, 2010 at 05:33, David Woodhouse wrote:
> On Sat, 2010-06-12 at 17:26 +0300, Artem Bityutskiy wrote:
>> On Sun, 2010-05-23 at 03:30 -0400, Mike Frysinger wrote:
>> > Currently, romfs XIP doesn't work in flash memory (the kernel crashes
>> > with a null pointer dereference).  The problem is that the mtd physmap
>> > driver isn't setting up a get_unmapped_area pointer for the mtd
>> > partitions it creates.
>>
>> Pushed both to my l2-mtd-2.6.git / dunno.
>
> I'm confused by the second patch -- isn't this done in add_mtd_device()
> already, according to the device type?
>
> You're setting mtd_bdi_ro_mappable indiscriminately for all types of
> devices. For flash devices, that can be an issue -- if you schedule a
> userspace process while the kernel is writing to the flash, for example.
>
> If you want to allow XIP of writable flash devices, there's more to it
> than this.

can you can safely do XIP on writable flash devices ?  with nommu, you
certainly cant.  plus, it is only possible with certain filesystems
right ?  like ROMFS ?  and that's already r/o ...

if we delve a bit, what you're referring to is only possible i think
if this change is merged (we posted this back in Jan w/no feedback):
  mtd-physmap: add support users can assign the probe type in board files

otherwise, there's no way of declaring a physmap of a parallel nor
flash and having it end up with read-only capabilities ...
-mike
David Woodhouse - Aug. 8, 2010, 11:33 a.m.
On Sat, 2010-08-07 at 19:26 -0400, Mike Frysinger wrote:
> can you can safely do XIP on writable flash devices ?  with nommu, you
> certainly cant.  plus, it is only possible with certain filesystems
> right ?  like ROMFS ?  and that's already r/o ...

As long as you don't allow any process to run that has part of the flash
chip mapped, you ought to be able to do it.

You might not want to use XIP for *everything* in that case -- but you
could imagine a situation where your critical processes are in RAM but
optional extra apps are XIP.

It's hard to implement though, because you'd have to keep track of which
processes need to be blocked and have some way to ensure it happens.

> if we delve a bit, what you're referring to is only possible i think
> if this change is merged (we posted this back in Jan w/no feedback):
>   mtd-physmap: add support users can assign the probe type in board files
> 
> otherwise, there's no way of declaring a physmap of a parallel nor
> flash and having it end up with read-only capabilities ...

Hm, yeah -- that one looks sane enough. I can merge that, although part
1 of that 2-part series I still object to because I don't *want*
different architectures to use different config symbols for the same
thing. Driver code shouldn't have to cope with that.
Mike Frysinger - Aug. 9, 2010, 7:36 a.m.
On Sun, Aug 8, 2010 at 07:33, David Woodhouse wrote:
> On Sat, 2010-08-07 at 19:26 -0400, Mike Frysinger wrote:
>> if we delve a bit, what you're referring to is only possible i think
>> if this change is merged (we posted this back in Jan w/no feedback):
>>   mtd-physmap: add support users can assign the probe type in board files
>>
>> otherwise, there's no way of declaring a physmap of a parallel nor
>> flash and having it end up with read-only capabilities ...
>
> Hm, yeah -- that one looks sane enough. I can merge that, although part
> 1 of that 2-part series I still object to because I don't *want*
> different architectures to use different config symbols for the same
> thing. Driver code shouldn't have to cope with that.

the two arent really dependent on each other.  i'll probably just
change the Blackfin code to also enable the XIP_KERNEL symbol and
forget about the whole damned mess.
-mike
Artem Bityutskiy - Aug. 20, 2010, 6:17 a.m.
On Sat, 2010-06-12 at 17:26 +0300, Artem Bityutskiy wrote:
> On Sun, 2010-05-23 at 03:30 -0400, Mike Frysinger wrote:
> > From: Bernd Schmidt <bernds_cb1@t-online.de>
> > 
> > Currently, romfs XIP doesn't work in flash memory (the kernel crashes
> > with a null pointer dereference).  The problem is that the mtd physmap
> > driver isn't setting up a get_unmapped_area pointer for the mtd
> > partitions it creates.
> > 
> > Signed-off-by: Bernd Schmidt <bernds_cb1@t-online.de>
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> 
> Pushed both to my l2-mtd-2.6.git / dunno.

dwmw2 did not take these 2 patches, so I'm dropping them from my l2 tree
as well.

Artem.

Patch

diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c
index 09d89ff..370da18 100644
--- a/drivers/mtd/maps/physmap.c
+++ b/drivers/mtd/maps/physmap.c
@@ -72,6 +72,20 @@  static int physmap_flash_remove(struct platform_device *dev)
 	return 0;
 }
 
+/*
+ * Allow NOMMU mmap() to directly map the device (if not NULL)
+ * - return the address to which the offset maps
+ * - return -ENOSYS to indicate refusal to do the mapping
+ */
+static unsigned long physmap_unmapped_area(struct mtd_info *mtd,
+					   unsigned long len,
+					   unsigned long offset,
+					   unsigned long flags)
+{
+	struct map_info *map = mtd->priv;
+	return (unsigned long) map->virt + offset;
+}
+
 static const char *rom_probe_types[] = {
 					"cfi_probe",
 					"jedec_probe",
@@ -149,6 +163,8 @@  static int physmap_flash_probe(struct platform_device *dev)
 		} else {
 			devices_found++;
 		}
+		if (info->mtd[i]->get_unmapped_area == NULL)
+			info->mtd[i]->get_unmapped_area = physmap_unmapped_area;
 		info->mtd[i]->owner = THIS_MODULE;
 		info->mtd[i]->dev.parent = &dev->dev;
 	}