[PATCH] powerpc: force 4k update_flash block and list sizes

STALE7284d

8 messages, 4 authors, 2006-11-08 · open the first message on its own page

[PATCH] powerpc: force 4k update_flash block and list sizes

From: John Rose <hidden>
Date: 2006-11-07 23:53:26

The enablement of 64k pages on POWER platforms exposed a quirk in the RTAS
mechanism for updating firmware.  RTAS assumes 4k for flash block and list
sizes, and use of any other sizes results in a failure.

This patch changes the rtas_flash module to force the use of 4k memory block
and list sizes when preparing and sending a firmware image to RTAS.  It also
changes rtas_flash to use a slab cache to ensure 4k alignment of flash block
data.  Such alignment is a documented requirement.

Signed-off-by: John Rose <redacted>

___

Intended for inclusion for 2.6.20, if appropriate.  Thanks!

John

---

 uf_4k-johnrose/arch/powerpc/kernel/rtas_flash.c |   52 +++++++++++++++++++-----
 1 files changed, 42 insertions(+), 10 deletions(-)

diff -puN arch/powerpc/kernel/rtas_flash.c~uf_4k_alt2 arch/powerpc/kernel/rtas_flash.c
--- uf_4k/arch/powerpc/kernel/rtas_flash.c~uf_4k_alt2	2006-11-06 16:59:27.000000000 -0600
+++ uf_4k-johnrose/arch/powerpc/kernel/rtas_flash.c	2006-11-06 17:02:31.000000000 -0600
@@ -72,6 +72,10 @@
 #define VALIDATE_BUF_SIZE 4096    
 #define RTAS_MSG_MAXLEN   64
 
+/* Quirk - RTAS requires 4k list length and block size */
+#define RTAS_BLKLIST_LENGTH 4096
+#define RTAS_BLK_SIZE 4096
+
 struct flash_block {
 	char *data;
 	unsigned long length;
@@ -83,7 +87,7 @@ struct flash_block {
  * into a version/length and translate the pointers
  * to absolute.
  */
-#define FLASH_BLOCKS_PER_NODE ((PAGE_SIZE - 16) / sizeof(struct flash_block))
+#define FLASH_BLOCKS_PER_NODE ((RTAS_BLKLIST_LENGTH - 16) / sizeof(struct flash_block))
 struct flash_block_list {
 	unsigned long num_blocks;
 	struct flash_block_list *next;
@@ -96,6 +100,9 @@ struct flash_block_list_header { /* just
 
 static struct flash_block_list_header rtas_firmware_flash_list = {0, NULL};
 
+/* Use slab cache to guarantee 4k alignment */
+static kmem_cache_t *flash_block_cache = NULL;
+
 #define FLASH_BLOCK_LIST_VERSION (1UL)
 
 /* Local copy of the flash block list.
@@ -153,7 +160,7 @@ static int flash_list_valid(struct flash
 				return FLASH_IMG_NULL_DATA;
 			}
 			block_size = f->blocks[i].length;
-			if (block_size <= 0 || block_size > PAGE_SIZE) {
+			if (block_size <= 0 || block_size > RTAS_BLK_SIZE) {
 				return FLASH_IMG_BAD_LEN;
 			}
 			image_size += block_size;
@@ -177,9 +184,9 @@ static void free_flash_list(struct flash
 
 	while (f) {
 		for (i = 0; i < f->num_blocks; i++)
-			free_page((unsigned long)(f->blocks[i].data));
+			kmem_cache_free(flash_block_cache, f->blocks[i].data);
 		next = f->next;
-		free_page((unsigned long)f);
+		kmem_cache_free(flash_block_cache, f);
 		f = next;
 	}
 }
@@ -278,6 +285,12 @@ static ssize_t rtas_flash_read(struct fi
 	return msglen;
 }
 
+/* constructor for flash_block_cache */
+void rtas_block_ctor(void *ptr, kmem_cache_t *cache, unsigned long flags)
+{
+	memset(ptr, 0, RTAS_BLK_SIZE);
+}
+
 /* We could be much more efficient here.  But to keep this function
  * simple we allocate a page to the block list no matter how small the
  * count is.  If the system is low on memory it will be just as well
@@ -302,7 +315,7 @@ static ssize_t rtas_flash_write(struct f
 	 * proc file
 	 */
 	if (uf->flist == NULL) {
-		uf->flist = (struct flash_block_list *) get_zeroed_page(GFP_KERNEL);
+		uf->flist = kmem_cache_alloc(flash_block_cache, GFP_KERNEL);
 		if (!uf->flist)
 			return -ENOMEM;
 	}
@@ -313,21 +326,21 @@ static ssize_t rtas_flash_write(struct f
 	next_free = fl->num_blocks;
 	if (next_free == FLASH_BLOCKS_PER_NODE) {
 		/* Need to allocate another block_list */
-		fl->next = (struct flash_block_list *)get_zeroed_page(GFP_KERNEL);
+		fl->next = kmem_cache_alloc(flash_block_cache, GFP_KERNEL);
 		if (!fl->next)
 			return -ENOMEM;
 		fl = fl->next;
 		next_free = 0;
 	}
 
-	if (count > PAGE_SIZE)
-		count = PAGE_SIZE;
-	p = (char *)get_zeroed_page(GFP_KERNEL);
+	if (count > RTAS_BLK_SIZE)
+		count = RTAS_BLK_SIZE;
+	p = kmem_cache_alloc(flash_block_cache, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
 	
 	if(copy_from_user(p, buffer, count)) {
-		free_page((unsigned long)p);
+		kmem_cache_free(flash_block_cache, p);
 		return -EFAULT;
 	}
 	fl->blocks[next_free].data = p;
@@ -791,6 +804,16 @@ int __init rtas_flash_init(void)
 		goto cleanup;
 
 	rtas_flash_term_hook = rtas_flash_firmware;
+
+	flash_block_cache = kmem_cache_create("rtas_flash_cache",
+				RTAS_BLK_SIZE, RTAS_BLK_SIZE, 0,
+				rtas_block_ctor, NULL);
+	if (!flash_block_cache) {
+		printk(KERN_ERR "%s: failed to create block cache\n",
+				__FUNCTION__);
+		rc = -ENOMEM;
+		goto cleanup;
+	}
 	return 0;
 
 cleanup:
@@ -804,7 +827,16 @@ cleanup:
 
 void __exit rtas_flash_cleanup(void)
 {
+	int rc;
+
 	rtas_flash_term_hook = NULL;
+
+	if (flash_block_cache) {
+		rc = kmem_cache_destroy(flash_block_cache);
+		if (rc)
+			printk(KERN_WARNING "%s: cache memory leak\n",
+					__FUNCTION__);
+	}
 	remove_flash_pde(firmware_flash_pde);
 	remove_flash_pde(firmware_update_pde);
 	remove_flash_pde(validate_pde);
_

Re: [PATCH] powerpc: force 4k update_flash block and list sizes

From: Michael Neuling <hidden>
Date: 2006-11-08 02:06:22

quoted hunk
+
+	if (flash_block_cache) {
+		rc = kmem_cache_destroy(flash_block_cache);
+		if (rc)
+			printk(KERN_WARNING "%s: cache memory leak\n",
+					__FUNCTION__);
+	}
kmem_cache_destroy doesn't return anything.  Hence we get:

arch/powerpc/kernel/rtas_flash.c: In function 'rtas_flash_cleanup':
arch/powerpc/kernel/rtas_flash.c:835: error: void value not ignored as it ought to be

Mikey

Re: [PATCH] powerpc: force 4k update_flash block and list sizes

From: John Rose <hidden>
Date: 2006-11-08 04:30:08

kmem_cache_destroy doesn't return anything.
Good call!  How's this:

The enablement of 64k pages on POWER platforms exposed a quirk in the RTAS
mechanism for updating firmware.  RTAS assumes 4k for flash block and list
sizes, and use of any other sizes results in a failure.

This patch changes the rtas_flash module to force the use of 4k memory block
and list sizes when preparing and sending a firmware image to RTAS.  It also
changes rtas_flash to use a slab cache to ensure 4k alignment of flash block
data.  Such alignment is a documented requirement.

Signed-off-by: John Rose <redacted>

---

---

 uf_4k-johnrose/arch/powerpc/kernel/rtas_flash.c |   52 +++++++++++++++++++-----
 1 files changed, 42 insertions(+), 10 deletions(-)

diff -puN arch/powerpc/kernel/rtas_flash.c~uf_4k_alt2 arch/powerpc/kernel/rtas_flash.c
--- uf_4k/arch/powerpc/kernel/rtas_flash.c~uf_4k_alt2	2006-11-06 16:59:27.000000000 -0600
+++ uf_4k-johnrose/arch/powerpc/kernel/rtas_flash.c	2006-11-06 17:02:31.000000000 -0600
@@ -72,6 +72,10 @@
 #define VALIDATE_BUF_SIZE 4096    
 #define RTAS_MSG_MAXLEN   64
 
+/* Quirk - RTAS requires 4k list length and block size */
+#define RTAS_BLKLIST_LENGTH 4096
+#define RTAS_BLK_SIZE 4096
+
 struct flash_block {
 	char *data;
 	unsigned long length;
@@ -83,7 +87,7 @@ struct flash_block {
  * into a version/length and translate the pointers
  * to absolute.
  */
-#define FLASH_BLOCKS_PER_NODE ((PAGE_SIZE - 16) / sizeof(struct flash_block))
+#define FLASH_BLOCKS_PER_NODE ((RTAS_BLKLIST_LENGTH - 16) / sizeof(struct flash_block))
 struct flash_block_list {
 	unsigned long num_blocks;
 	struct flash_block_list *next;
@@ -96,6 +100,9 @@ struct flash_block_list_header { /* just
 
 static struct flash_block_list_header rtas_firmware_flash_list = {0, NULL};
 
+/* Use slab cache to guarantee 4k alignment */
+static kmem_cache_t *flash_block_cache = NULL;
+
 #define FLASH_BLOCK_LIST_VERSION (1UL)
 
 /* Local copy of the flash block list.
@@ -153,7 +160,7 @@ static int flash_list_valid(struct flash
 				return FLASH_IMG_NULL_DATA;
 			}
 			block_size = f->blocks[i].length;
-			if (block_size <= 0 || block_size > PAGE_SIZE) {
+			if (block_size <= 0 || block_size > RTAS_BLK_SIZE) {
 				return FLASH_IMG_BAD_LEN;
 			}
 			image_size += block_size;
@@ -177,9 +184,9 @@ static void free_flash_list(struct flash
 
 	while (f) {
 		for (i = 0; i < f->num_blocks; i++)
-			free_page((unsigned long)(f->blocks[i].data));
+			kmem_cache_free(flash_block_cache, f->blocks[i].data);
 		next = f->next;
-		free_page((unsigned long)f);
+		kmem_cache_free(flash_block_cache, f);
 		f = next;
 	}
 }
@@ -278,6 +285,12 @@ static ssize_t rtas_flash_read(struct fi
 	return msglen;
 }
 
+/* constructor for flash_block_cache */
+void rtas_block_ctor(void *ptr, kmem_cache_t *cache, unsigned long flags)
+{
+	memset(ptr, 0, RTAS_BLK_SIZE);
+}
+
 /* We could be much more efficient here.  But to keep this function
  * simple we allocate a page to the block list no matter how small the
  * count is.  If the system is low on memory it will be just as well
@@ -302,7 +315,7 @@ static ssize_t rtas_flash_write(struct f
 	 * proc file
 	 */
 	if (uf->flist == NULL) {
-		uf->flist = (struct flash_block_list *) get_zeroed_page(GFP_KERNEL);
+		uf->flist = kmem_cache_alloc(flash_block_cache, GFP_KERNEL);
 		if (!uf->flist)
 			return -ENOMEM;
 	}
@@ -313,21 +326,21 @@ static ssize_t rtas_flash_write(struct f
 	next_free = fl->num_blocks;
 	if (next_free == FLASH_BLOCKS_PER_NODE) {
 		/* Need to allocate another block_list */
-		fl->next = (struct flash_block_list *)get_zeroed_page(GFP_KERNEL);
+		fl->next = kmem_cache_alloc(flash_block_cache, GFP_KERNEL);
 		if (!fl->next)
 			return -ENOMEM;
 		fl = fl->next;
 		next_free = 0;
 	}
 
-	if (count > PAGE_SIZE)
-		count = PAGE_SIZE;
-	p = (char *)get_zeroed_page(GFP_KERNEL);
+	if (count > RTAS_BLK_SIZE)
+		count = RTAS_BLK_SIZE;
+	p = kmem_cache_alloc(flash_block_cache, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
 	
 	if(copy_from_user(p, buffer, count)) {
-		free_page((unsigned long)p);
+		kmem_cache_free(flash_block_cache, p);
 		return -EFAULT;
 	}
 	fl->blocks[next_free].data = p;
@@ -791,6 +804,16 @@ int __init rtas_flash_init(void)
 		goto cleanup;
 
 	rtas_flash_term_hook = rtas_flash_firmware;
+
+	flash_block_cache = kmem_cache_create("rtas_flash_cache",
+				RTAS_BLK_SIZE, RTAS_BLK_SIZE, 0,
+				rtas_block_ctor, NULL);
+	if (!flash_block_cache) {
+		printk(KERN_ERR "%s: failed to create block cache\n",
+				__FUNCTION__);
+		rc = -ENOMEM;
+		goto cleanup;
+	}
 	return 0;
 
 cleanup:
@@ -804,7 +827,16 @@ cleanup:
 
 void __exit rtas_flash_cleanup(void)
 {
+	int rc;
+
 	rtas_flash_term_hook = NULL;
+
+	if (flash_block_cache) {
+		rc = kmem_cache_destroy(flash_block_cache);
+		if (rc)
+			printk(KERN_WARNING "%s: cache memory leak\n",
+					__FUNCTION__);
+	}
 	remove_flash_pde(firmware_flash_pde);
 	remove_flash_pde(firmware_update_pde);
 	remove_flash_pde(validate_pde);
_

Re: [PATCH] powerpc: force 4k update_flash block and list sizes

From: Michael Neuling <hidden>
Date: 2006-11-08 04:39:04

In message [off-list ref] you wrote:
quoted
kmem_cache_destroy doesn't return anything.
Good call!  How's this:
<snip>
quoted hunk
+	if (flash_block_cache) {
+		rc = kmem_cache_destroy(flash_block_cache);
+		if (rc)
+			printk(KERN_WARNING "%s: cache memory leak\n",
+					__FUNCTION__);
+	}
Arrh, did anything change?

Mikey

Re: [PATCH] powerpc: force 4k update_flash block and list sizes

From: Michael Ellerman <hidden>
Date: 2006-11-08 04:41:00

On Tue, 2006-11-07 at 22:30 -0600, John Rose wrote:
quoted
kmem_cache_destroy doesn't return anything.
Good call!  How's this:

The enablement of 64k pages on POWER platforms exposed a quirk in the RTAS
mechanism for updating firmware.  RTAS assumes 4k for flash block and list
sizes, and use of any other sizes results in a failure.

This patch changes the rtas_flash module to force the use of 4k memory block
and list sizes when preparing and sending a firmware image to RTAS.  It also
changes rtas_flash to use a slab cache to ensure 4k alignment of flash block
data.  Such alignment is a documented requirement.
Just being picky .. but why did you decide to use the slab cache? Does
it make the code neater? I would have thought you could guarantee 4k
alignment some other way, and it seems slightly odd to use the slab
cache for something you only do once ..

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

Re: [PATCH] powerpc: force 4k update_flash block and list sizes

From: Arnd Bergmann <arnd@arndb.de>
Date: 2006-11-08 09:28:45

On Wednesday 08 November 2006 05:41, Michael Ellerman wrote:
Just being picky .. but why did you decide to use the slab cache? Does
it make the code neater? I would have thought you could guarantee 4k
alignment some other way, and it seems slightly odd to use the slab
cache for something you only do once ..
There are not so many allocators available. kmalloc does not guarantee
alignment beyond a few bytes and the buddy allocator and vmalloc don't
give you allocations smaller than PAGE_SIZE.

The ehca people had the same problem, and I could imagine there are
others with similar issues. How about adding a special slab allocator
architecture-wide that gives out aligned 4k chunks independent of
page size?

	Arnd <>z

Re: [PATCH] powerpc: force 4k update_flash block and list sizes

From: John Rose <hidden>
Date: 2006-11-08 16:07:12

Arrh, did anything change?
Sorry, attached the wrong thing!  That's what I get for trying to submit
code after 10PM :)

The enablement of 64k pages on POWER platforms exposed a quirk in the RTAS
mechanism for updating firmware.  RTAS assumes 4k for flash block and list
sizes, and use of any other sizes results in a failure.

This patch changes the rtas_flash module to force the use of 4k memory block
and list sizes when preparing and sending a firmware image to RTAS.  It also
changes rtas_flash to use a slab cache to ensure 4k alignment of flash block
data.  Such alignment is a documented requirement.

Signed-off-by: John Rose <redacted>

---

 uf_4k-johnrose/arch/powerpc/kernel/rtas_flash.c |   47 ++++++++++++++++++------
 1 files changed, 37 insertions(+), 10 deletions(-)

diff -puN arch/powerpc/kernel/rtas_flash.c~uf_4k_alt2 arch/powerpc/kernel/rtas_flash.c
--- uf_4k/arch/powerpc/kernel/rtas_flash.c~uf_4k_alt2	2006-11-06 16:59:27.000000000 -0600
+++ uf_4k-johnrose/arch/powerpc/kernel/rtas_flash.c	2006-11-07 22:29:52.000000000 -0600
@@ -72,6 +72,10 @@
 #define VALIDATE_BUF_SIZE 4096    
 #define RTAS_MSG_MAXLEN   64
 
+/* Quirk - RTAS requires 4k list length and block size */
+#define RTAS_BLKLIST_LENGTH 4096
+#define RTAS_BLK_SIZE 4096
+
 struct flash_block {
 	char *data;
 	unsigned long length;
@@ -83,7 +87,7 @@ struct flash_block {
  * into a version/length and translate the pointers
  * to absolute.
  */
-#define FLASH_BLOCKS_PER_NODE ((PAGE_SIZE - 16) / sizeof(struct flash_block))
+#define FLASH_BLOCKS_PER_NODE ((RTAS_BLKLIST_LENGTH - 16) / sizeof(struct flash_block))
 struct flash_block_list {
 	unsigned long num_blocks;
 	struct flash_block_list *next;
@@ -96,6 +100,9 @@ struct flash_block_list_header { /* just
 
 static struct flash_block_list_header rtas_firmware_flash_list = {0, NULL};
 
+/* Use slab cache to guarantee 4k alignment */
+static kmem_cache_t *flash_block_cache = NULL;
+
 #define FLASH_BLOCK_LIST_VERSION (1UL)
 
 /* Local copy of the flash block list.
@@ -153,7 +160,7 @@ static int flash_list_valid(struct flash
 				return FLASH_IMG_NULL_DATA;
 			}
 			block_size = f->blocks[i].length;
-			if (block_size <= 0 || block_size > PAGE_SIZE) {
+			if (block_size <= 0 || block_size > RTAS_BLK_SIZE) {
 				return FLASH_IMG_BAD_LEN;
 			}
 			image_size += block_size;
@@ -177,9 +184,9 @@ static void free_flash_list(struct flash
 
 	while (f) {
 		for (i = 0; i < f->num_blocks; i++)
-			free_page((unsigned long)(f->blocks[i].data));
+			kmem_cache_free(flash_block_cache, f->blocks[i].data);
 		next = f->next;
-		free_page((unsigned long)f);
+		kmem_cache_free(flash_block_cache, f);
 		f = next;
 	}
 }
@@ -278,6 +285,12 @@ static ssize_t rtas_flash_read(struct fi
 	return msglen;
 }
 
+/* constructor for flash_block_cache */
+void rtas_block_ctor(void *ptr, kmem_cache_t *cache, unsigned long flags)
+{
+	memset(ptr, 0, RTAS_BLK_SIZE);
+}
+
 /* We could be much more efficient here.  But to keep this function
  * simple we allocate a page to the block list no matter how small the
  * count is.  If the system is low on memory it will be just as well
@@ -302,7 +315,7 @@ static ssize_t rtas_flash_write(struct f
 	 * proc file
 	 */
 	if (uf->flist == NULL) {
-		uf->flist = (struct flash_block_list *) get_zeroed_page(GFP_KERNEL);
+		uf->flist = kmem_cache_alloc(flash_block_cache, GFP_KERNEL);
 		if (!uf->flist)
 			return -ENOMEM;
 	}
@@ -313,21 +326,21 @@ static ssize_t rtas_flash_write(struct f
 	next_free = fl->num_blocks;
 	if (next_free == FLASH_BLOCKS_PER_NODE) {
 		/* Need to allocate another block_list */
-		fl->next = (struct flash_block_list *)get_zeroed_page(GFP_KERNEL);
+		fl->next = kmem_cache_alloc(flash_block_cache, GFP_KERNEL);
 		if (!fl->next)
 			return -ENOMEM;
 		fl = fl->next;
 		next_free = 0;
 	}
 
-	if (count > PAGE_SIZE)
-		count = PAGE_SIZE;
-	p = (char *)get_zeroed_page(GFP_KERNEL);
+	if (count > RTAS_BLK_SIZE)
+		count = RTAS_BLK_SIZE;
+	p = kmem_cache_alloc(flash_block_cache, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
 	
 	if(copy_from_user(p, buffer, count)) {
-		free_page((unsigned long)p);
+		kmem_cache_free(flash_block_cache, p);
 		return -EFAULT;
 	}
 	fl->blocks[next_free].data = p;
@@ -791,6 +804,16 @@ int __init rtas_flash_init(void)
 		goto cleanup;
 
 	rtas_flash_term_hook = rtas_flash_firmware;
+
+	flash_block_cache = kmem_cache_create("rtas_flash_cache",
+				RTAS_BLK_SIZE, RTAS_BLK_SIZE, 0,
+				rtas_block_ctor, NULL);
+	if (!flash_block_cache) {
+		printk(KERN_ERR "%s: failed to create block cache\n",
+				__FUNCTION__);
+		rc = -ENOMEM;
+		goto cleanup;
+	}
 	return 0;
 
 cleanup:
@@ -805,6 +828,10 @@ cleanup:
 void __exit rtas_flash_cleanup(void)
 {
 	rtas_flash_term_hook = NULL;
+
+	if (flash_block_cache)
+		kmem_cache_destroy(flash_block_cache);
+
 	remove_flash_pde(firmware_flash_pde);
 	remove_flash_pde(firmware_update_pde);
 	remove_flash_pde(validate_pde);
_

Re: [PATCH] powerpc: force 4k update_flash block and list sizes

From: Michael Ellerman <hidden>
Date: 2006-11-08 22:40:18

On Wed, 2006-11-08 at 10:28 +0100, Arnd Bergmann wrote:
On Wednesday 08 November 2006 05:41, Michael Ellerman wrote:
quoted
Just being picky .. but why did you decide to use the slab cache? Does
it make the code neater? I would have thought you could guarantee 4k
alignment some other way, and it seems slightly odd to use the slab
cache for something you only do once ..
There are not so many allocators available. kmalloc does not guarantee
alignment beyond a few bytes and the buddy allocator and vmalloc don't
give you allocations smaller than PAGE_SIZE.
Yeah, I assumed kmalloc aligned on the size of the allocation, but it
doesn't looking at the code. We could set ARCH_KMALLOC_MINALIGN to 4k,
but that aligns all caches which is probably not what we want.
The ehca people had the same problem, and I could imagine there are
others with similar issues. How about adding a special slab allocator
architecture-wide that gives out aligned 4k chunks independent of
page size?
Yeah that sounds like a plan, I'm sure there's other code around that
could use it.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help