LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [SCSI] ipr: Fix stack overflow in ipr_format_resource_path
From: Anton Blanchard @ 2010-06-02 11:16 UTC (permalink / raw)
  To: brking, wayneb, betabandido; +Cc: linuxppc-dev, linux-scsi


Victor reported an oops during boot with 2.6.34 on a POWER6 JS22:

https://bugzilla.kernel.org/show_bug.cgi?id=16089

Checking ipr microcode levels
Unable to handle kernel paging request for instruction fetch
Faulting instruction address: 0x322d30312d31302c
...
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=128 NUMA pSeries
last sysfs file:
/sys/devices/pci0000:00/0000:00:01.0/host0/target0:255:255/0:255:255:255/resource_path
Modules linked in:
NIP: 322d30312d31302c LR: 322d30312d31302d CTR: c000000000375bec
REGS: c0000003d360f8f0 TRAP: 0400   Not tainted  (2.6.34-vjj)
MSR: 8000000040009432 <EE,ME,IR,DR>  CR: 28002484  XER: 20000020
TASK = c0000003d587d010[5163] 'iprupdate' THREAD: c0000003d360c000 CPU: 7
GPR00: 322d30312d31302d c0000003d360fb70 c000000000ad07c0 00000000000185a0 
GPR04: 0000000000000001 c0000003d360fb10 04000affffffffff c0000000006a6700 
GPR08: c000000000823383 0000000000000000 0000000000000020 0000000000000000 
GPR12: 000000000000f032 c00000000f622e00 00000000000000ed 0000000000000000 
GPR16: 00000000100b8808 0000000010020000 0000000010020000 0000000010010000 
GPR20: 0000000010010000 0000000000000001 0000000000001000 000000001045eef8 
GPR24: c0000003d360fdf8 302d31342d31302d 43302d30302d3030 2d30332d44352d34 
GPR28: 422d33382d30302d 30302d30302d3030 2d30302d30302d30 302d30302d30302d 
NIP [322d30312d31302c] 0x322d30312d31302c
LR [322d30312d31302d] 0x322d30312d31302d

A stack overflow. It turns out ipr_format_resource_path writes to a
passed in buffer using sprintf which is dangerous and in this case looks
to be the cause of the overflow.

The following patch passes in the length of the buffer and uses snprintf,
which fixes the fail for me. It doesn't fix the other issue, which is
why the loop in ipr_format_resource_path didn't terminate in the first place.
That can be fixed in a follow up patch.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: linux.trees.git/drivers/scsi/ipr.c
===================================================================
--- linux.trees.git.orig/drivers/scsi/ipr.c	2010-05-31 08:51:20.000000000 +1000
+++ linux.trees.git/drivers/scsi/ipr.c	2010-06-02 21:15:41.000000000 +1000
@@ -1132,17 +1132,20 @@ static int ipr_is_same_device(struct ipr
  * ipr_format_resource_path - Format the resource path for printing.
  * @res_path:	resource path
  * @buf:	buffer
+ * @len:	length of the buffer
  *
  * Return value:
  * 	pointer to buffer
  **/
-static char *ipr_format_resource_path(u8 *res_path, char *buffer)
+static char *ipr_format_resource_path(u8 *res_path, char *buffer,
+				      unsigned int len)
 {
 	int i;
+	char *p = buffer;
 
-	sprintf(buffer, "%02X", res_path[0]);
+	p += snprintf(p, buffer + len - p, "%02X", res_path[0]);
 	for (i=1; res_path[i] != 0xff; i++)
-		sprintf(buffer, "%s-%02X", buffer, res_path[i]);
+		p += snprintf(p, buffer + len - p, "-%02X", res_path[i]);
 
 	return buffer;
 }
@@ -1187,7 +1190,8 @@ static void ipr_update_res_entry(struct 
 
 		if (res->sdev && new_path)
 			sdev_printk(KERN_INFO, res->sdev, "Resource path: %s\n",
-				    ipr_format_resource_path(&res->res_path[0], &buffer[0]));
+				    ipr_format_resource_path(&res->res_path[0],
+				    &buffer[0], sizeof(buffer)));
 	} else {
 		res->flags = cfgtew->u.cfgte->flags;
 		if (res->flags & IPR_IS_IOA_RESOURCE)
@@ -1573,7 +1577,8 @@ static void ipr_log_sis64_config_error(s
 		ipr_err_separator;
 
 		ipr_err("Device %d : %s", i + 1,
-			 ipr_format_resource_path(&dev_entry->res_path[0], &buffer[0]));
+			 ipr_format_resource_path(&dev_entry->res_path[0],
+			 &buffer[0], sizeof(buffer)));
 		ipr_log_ext_vpd(&dev_entry->vpd);
 
 		ipr_err("-----New Device Information-----\n");
@@ -1919,13 +1924,15 @@ static void ipr_log64_fabric_path(struct
 
 			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s\n",
 				     path_active_desc[i].desc, path_state_desc[j].desc,
-				     ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
+				     ipr_format_resource_path(&fabric->res_path[0],
+				     &buffer[0], sizeof(buffer)));
 			return;
 		}
 	}
 
 	ipr_err("Path state=%02X Resource Path=%s\n", path_state,
-		ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
+		ipr_format_resource_path(&fabric->res_path[0], &buffer[0],
+		sizeof(buffer)));
 }
 
 static const struct {
@@ -2066,7 +2073,9 @@ static void ipr_log64_path_elem(struct i
 
 			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s, Link rate=%s, WWN=%08X%08X\n",
 				     path_status_desc[j].desc, path_type_desc[i].desc,
-				     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
+				     ipr_format_resource_path(&cfg->res_path[0],
+							      &buffer[0],
+							      sizeof(buffer)),
 				     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
 				     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
 			return;
@@ -2074,7 +2083,8 @@ static void ipr_log64_path_elem(struct i
 	}
 	ipr_hcam_err(hostrcb, "Path element=%02X: Resource Path=%s, Link rate=%s "
 		     "WWN=%08X%08X\n", cfg->type_status,
-		     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
+		     ipr_format_resource_path(&cfg->res_path[0], &buffer[0],
+		     sizeof(buffer)),
 		     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
 		     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
 }
@@ -2139,7 +2149,8 @@ static void ipr_log_sis64_array_error(st
 
 	ipr_err("RAID %s Array Configuration: %s\n",
 		error->protection_level,
-		ipr_format_resource_path(&error->last_res_path[0], &buffer[0]));
+		ipr_format_resource_path(&error->last_res_path[0], &buffer[0],
+					 sizeof(buffer)));
 
 	ipr_err_separator;
 
@@ -2160,9 +2171,12 @@ static void ipr_log_sis64_array_error(st
 		ipr_err("Array Member %d:\n", i);
 		ipr_log_ext_vpd(&array_entry->vpd);
 		ipr_err("Current Location: %s",
-			 ipr_format_resource_path(&array_entry->res_path[0], &buffer[0]));
+			 ipr_format_resource_path(&array_entry->res_path[0],
+						  &buffer[0],
+						  sizeof(buffer)));
 		ipr_err("Expected Location: %s",
-			 ipr_format_resource_path(&array_entry->expected_res_path[0], &buffer[0]));
+			 ipr_format_resource_path(&array_entry->expected_res_path[0],
+						  &buffer[0], sizeof(buffer)));
 
 		ipr_err_separator;
 	}
@@ -4099,7 +4113,9 @@ static ssize_t ipr_show_resource_path(st
 	res = (struct ipr_resource_entry *)sdev->hostdata;
 	if (res)
 		len = snprintf(buf, PAGE_SIZE, "%s\n",
-			       ipr_format_resource_path(&res->res_path[0], &buffer[0]));
+			       ipr_format_resource_path(&res->res_path[0],
+							&buffer[0],
+							sizeof(buffer)));
 	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
 	return len;
 }
@@ -4351,7 +4367,9 @@ static int ipr_slave_configure(struct sc
 			scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun);
 		if (ioa_cfg->sis64)
 			sdev_printk(KERN_INFO, sdev, "Resource path: %s\n",
-			            ipr_format_resource_path(&res->res_path[0], &buffer[0]));
+			            ipr_format_resource_path(&res->res_path[0],
+							     &buffer[0],
+							     sizeof(buffer)));
 		return 0;
 	}
 	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
Index: linux.trees.git/drivers/scsi/ipr.h
===================================================================
--- linux.trees.git.orig/drivers/scsi/ipr.h	2010-05-31 08:51:20.000000000 +1000
+++ linux.trees.git/drivers/scsi/ipr.h	2010-06-02 21:15:41.000000000 +1000
@@ -1685,7 +1685,8 @@ struct ipr_ucode_image_header {
 		if ((hostrcb)->ioa_cfg->sis64) {			\
 			printk(KERN_ERR IPR_NAME ": %s: " fmt, 		\
 				ipr_format_resource_path(&hostrcb->hcam.u.error64.fd_res_path[0], \
-					&hostrcb->rp_buffer[0]),	\
+					&hostrcb->rp_buffer[0],		\
+					sizeof(hostrcb->rp_buffer)),	\
 				__VA_ARGS__);				\
 		} else {						\
 			ipr_ra_err((hostrcb)->ioa_cfg,			\

^ permalink raw reply

* Re: [PATCH] net/mpc52xx_phy: Various code cleanups
From: David Miller @ 2010-06-02 10:45 UTC (permalink / raw)
  To: w.sang; +Cc: linuxppc-dev, netdev
In-Reply-To: <1274264470-2905-1-git-send-email-w.sang@pengutronix.de>

From: Wolfram Sang <w.sang@pengutronix.de>
Date: Wed, 19 May 2010 12:21:10 +0200

> - don't free bus->irq (obsoleted by ca816d98170942371535b3e862813b0aba9b7d90)
> - don't dispose irqs (should be done in of_mdiobus_register())
> - use fec-pointer consistently in transfer()
> - use resource_size()
> - cosmetic fixes
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] PowerPC: Remove hardcoded BAT configuration of IMMR in CPM early debug console
From: Martyn Welch @ 2010-06-02  8:06 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <4C053008.50306@freescale.com>

Scott Wood wrote:
> On 06/01/2010 08:43 AM, Martyn Welch wrote:
>>>>> diff --git a/arch/powerpc/kernel/head_32.S
>>>>> b/arch/powerpc/kernel/head_32.S
>>>>> index e025e89..861cace 100644
>>>>> --- a/arch/powerpc/kernel/head_32.S
>>>>> +++ b/arch/powerpc/kernel/head_32.S
>>>>> @@ -1194,12 +1194,13 @@ setup_disp_bat:
>>>>>    #endif /* CONFIG_BOOTX_TEXT */
>>>>>
>>>>>    #ifdef CONFIG_PPC_EARLY_DEBUG_CPM
>>>>> +#define PPC_EARLY_DEBUG_CPM_ADDR
>>>>> ASM_CONST(CONFIG_PPC_EARLY_DEBUG_CPM_ADDR)
>>>>>    setup_cpm_bat:
>>>>> -    lis    r8, 0xf000
>>>>> +    lis    r8, PPC_EARLY_DEBUG_CPM_ADDR@ha
>>>>>        ori    r8, r8,    0x002a
>>>>>        mtspr    SPRN_DBAT1L, r8
>>>>>
>>>>> -    lis    r11, 0xf000
>>>>> +    lis    r11, PPC_EARLY_DEBUG_CPM_ADDR@ha
>>>>>        ori    r11, r11, (BL_1M<<  2) | 2
>>>>>        mtspr    SPRN_DBAT1U, r11
>>>>>
>>>> Only the physical address should depend on where IMMR is.  We should
>>>> use fixmap instead of an arbitrary address for the effective address.
>>>> There's a existing FIX_EARLY_DEBUG_BASE, but it's only 128 KiB so
>>>> we'll have to either grow it, or map only a subset of IMMR.
>>>>
>>>>
>>>
>>> I think that's a more fundamental change to CPM early debug than I can
>>> handle right now.
>
> Is IMMRBASE on your board at some address that has a low likelihood of
> conflicting when treated as a kernel effective address?

It's at 0x0f000000, is seems ok, but then I'm not sure I fully
understand kernel effective addresses.

Martyn

-- 
Martyn Welch (Principal Software Engineer)   |   Registered in England and
GE Intelligent Platforms                     |   Wales (3828642) at 100
T +44(0)127322748                            |   Barbirolli Square, Manchester,
E martyn.welch@ge.com                        |   M2 3AB  VAT:GB 927559189

^ permalink raw reply

* [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2010-06-02  8:05 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linuxppc-dev list, Andrew Morton, Linux Kernel list

Hi Linus !

Here's a reasonably urgent few bug fixes on top of -rc1. Grant OF stuff had
a few issues that broke pretty much all PowerMacs, so this fixes it, along
with a couple of misc fixes and a MAINTAINERS update.

Cheers,
Ben. 

The following changes since commit aef4b9aaae1decc775778903922bd0075cce7a88:
  Linus Torvalds (1):
        Merge branch 'next' of git://git.kernel.org/.../benh/powerpc

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge

Ananth N Mavinakayanahalli (1):
      powerpc/kprobes: Remove resume_execution() in kprobes

Andreas Schwab (1):
      powerpc/macio: Don't dereference pointer before null check

Benjamin Herrenschmidt (1):
      powerpc/macio: Fix probing of macio devices by using the right of match table

Denis Kirjanov (1):
      powerpc/cell: Fix integer constant warning

Olof Johansson (1):
      powerpc/pasemi: Update MAINTAINERS file

Paul Mackerras (1):
      agp/uninorth: Fix oops caused by flushing too much

 MAINTAINERS                            |    3 +--
 arch/powerpc/include/asm/macio.h       |    4 ----
 arch/powerpc/kernel/kprobes.c          |   14 ++------------
 arch/powerpc/platforms/cell/iommu.c    |    2 +-
 drivers/ata/pata_macio.c               |   10 +++++-----
 drivers/block/swim3.c                  |    6 ++++--
 drivers/char/agp/uninorth-agp.c        |    2 +-
 drivers/ide/pmac.c                     |    7 +++++--
 drivers/macintosh/macio_asic.c         |    8 ++++----
 drivers/macintosh/mediabay.c           |    6 ++++--
 drivers/macintosh/rack-meter.c         |    8 +++++---
 drivers/net/bmac.c                     |    7 +++++--
 drivers/net/mace.c                     |    7 +++++--
 drivers/net/wireless/orinoco/airport.c |    7 +++++--
 drivers/scsi/mac53c94.c                |    7 +++++--
 drivers/scsi/mesh.c                    |    7 +++++--
 drivers/serial/pmac_zilog.c            |    7 +++++--
 sound/aoa/soundbus/i2sbus/core.c       |    8 +++++---
 18 files changed, 67 insertions(+), 53 deletions(-)

^ permalink raw reply

* Re: [PATCH v3] powerpc: Add i8042 keyboard and mouse irq parsing
From: Benjamin Herrenschmidt @ 2010-06-02  7:25 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Martyn Welch, linux-input, linuxppc-dev
In-Reply-To: <20100602062634.GA3713@core.coreip.homeip.net>

On Tue, 2010-06-01 at 23:26 -0700, Dmitry Torokhov wrote:
> 
> FYI: i8042 core expects I8042_{KBD|AUX}_IRQ to be integers, however it
> is certainly fixable...

Let's not bother.

Cheers,
Ben.

^ permalink raw reply

* [PATCH] powerpc/macio: Fix probing of macio devices by using the right of match table
From: Benjamin Herrenschmidt @ 2010-06-02  7:15 UTC (permalink / raw)
  To: linuxppc-dev

Grant patches added an of mach table to struct device_driver. However,
while he changed the macio device code to use that, he left the match
table pointer in struct macio_driver and didn't update drivers to use
the "new" one, thus breaking the probing.

This completes the change by moving all drivers to setup the "new"
one, removing all traces of the old one, and while at it (since it
changes the exact same locations), I also remove two other duplicates
from struct driver which are the name and owner fields.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/include/asm/macio.h       |    4 ----
 drivers/ata/pata_macio.c               |   10 +++++-----
 drivers/block/swim3.c                  |    6 ++++--
 drivers/ide/pmac.c                     |    7 +++++--
 drivers/macintosh/macio_asic.c         |    1 -
 drivers/macintosh/mediabay.c           |    6 ++++--
 drivers/macintosh/rack-meter.c         |    8 +++++---
 drivers/net/bmac.c                     |    7 +++++--
 drivers/net/mace.c                     |    7 +++++--
 drivers/net/wireless/orinoco/airport.c |    7 +++++--
 drivers/scsi/mac53c94.c                |    7 +++++--
 drivers/scsi/mesh.c                    |    7 +++++--
 drivers/serial/pmac_zilog.c            |    7 +++++--
 sound/aoa/soundbus/i2sbus/core.c       |    8 +++++---
 14 files changed, 58 insertions(+), 34 deletions(-)

I'm going to send that to Linus today btw...

diff --git a/arch/powerpc/include/asm/macio.h b/arch/powerpc/include/asm/macio.h
index 19a661b..675e159 100644
--- a/arch/powerpc/include/asm/macio.h
+++ b/arch/powerpc/include/asm/macio.h
@@ -123,10 +123,6 @@ static inline struct pci_dev *macio_get_pci_dev(struct macio_dev *mdev)
  */
 struct macio_driver
 {
-	char			*name;
-	struct of_device_id	*match_table;
-	struct module		*owner;
-
 	int	(*probe)(struct macio_dev* dev, const struct of_device_id *match);
 	int	(*remove)(struct macio_dev* dev);
 
diff --git a/drivers/ata/pata_macio.c b/drivers/ata/pata_macio.c
index 76640ac..75b49d0 100644
--- a/drivers/ata/pata_macio.c
+++ b/drivers/ata/pata_macio.c
@@ -1355,8 +1355,11 @@ static struct of_device_id pata_macio_match[] =
 
 static struct macio_driver pata_macio_driver =
 {
-	.name 		= "pata-macio",
-	.match_table	= pata_macio_match,
+	.driver = {
+		.name 		= "pata-macio",
+		.owner		= THIS_MODULE,
+		.of_match_table	= pata_macio_match,
+	},
 	.probe		= pata_macio_attach,
 	.remove		= pata_macio_detach,
 #ifdef CONFIG_PM
@@ -1366,9 +1369,6 @@ static struct macio_driver pata_macio_driver =
 #ifdef CONFIG_PMAC_MEDIABAY
 	.mediabay_event	= pata_macio_mb_event,
 #endif
-	.driver = {
-		.owner		= THIS_MODULE,
-	},
 };
 
 static const struct pci_device_id pata_macio_pci_match[] = {
diff --git a/drivers/block/swim3.c b/drivers/block/swim3.c
index 52f2d11..ed6fb91 100644
--- a/drivers/block/swim3.c
+++ b/drivers/block/swim3.c
@@ -1159,8 +1159,10 @@ static struct of_device_id swim3_match[] =
 
 static struct macio_driver swim3_driver =
 {
-	.name 		= "swim3",
-	.match_table	= swim3_match,
+	.driver = {
+		.name 		= "swim3",
+		.of_match_table	= swim3_match,
+	},
 	.probe		= swim3_attach,
 #if 0
 	.suspend	= swim3_suspend,
diff --git a/drivers/ide/pmac.c b/drivers/ide/pmac.c
index 183fa38..ebcf8e4 100644
--- a/drivers/ide/pmac.c
+++ b/drivers/ide/pmac.c
@@ -1400,8 +1400,11 @@ static struct of_device_id pmac_ide_macio_match[] =
 
 static struct macio_driver pmac_ide_macio_driver = 
 {
-	.name 		= "ide-pmac",
-	.match_table	= pmac_ide_macio_match,
+	.driver = {
+		.name 		= "ide-pmac",
+		.owner		= THIS_MODULE,
+		.of_match_table	= pmac_ide_macio_match,
+	},
 	.probe		= pmac_ide_macio_attach,
 	.suspend	= pmac_ide_macio_suspend,
 	.resume		= pmac_ide_macio_resume,
diff --git a/drivers/macintosh/macio_asic.c b/drivers/macintosh/macio_asic.c
index 40390ac..b6e7ddc 100644
--- a/drivers/macintosh/macio_asic.c
+++ b/drivers/macintosh/macio_asic.c
@@ -526,7 +526,6 @@ static void macio_pci_add_devices(struct macio_chip *chip)
 int macio_register_driver(struct macio_driver *drv)
 {
 	/* initialize common driver fields */
-	drv->driver.name = drv->name;
 	drv->driver.bus = &macio_bus_type;
 
 	/* register with core */
diff --git a/drivers/macintosh/mediabay.c b/drivers/macintosh/mediabay.c
index 288acce..2fd435b 100644
--- a/drivers/macintosh/mediabay.c
+++ b/drivers/macintosh/mediabay.c
@@ -728,8 +728,10 @@ static struct of_device_id media_bay_match[] =
 
 static struct macio_driver media_bay_driver =
 {
-	.name		= "media-bay",
-	.match_table	= media_bay_match,
+	.driver = {
+		.name		= "media-bay",
+		.of_match_table	= media_bay_match,
+	},
 	.probe		= media_bay_attach,
 	.suspend	= media_bay_suspend,
 	.resume		= media_bay_resume
diff --git a/drivers/macintosh/rack-meter.c b/drivers/macintosh/rack-meter.c
index 12946c5..53cce3a 100644
--- a/drivers/macintosh/rack-meter.c
+++ b/drivers/macintosh/rack-meter.c
@@ -584,9 +584,11 @@ static struct of_device_id rackmeter_match[] = {
 };
 
 static struct macio_driver rackmeter_driver = {
-	.name = "rackmeter",
-	.owner = THIS_MODULE,
-	.match_table = rackmeter_match,
+	.driver = {
+		.name = "rackmeter",
+		.owner = THIS_MODULE,
+		.of_match_table = rackmeter_match,
+	},
 	.probe = rackmeter_probe,
 	.remove = __devexit_p(rackmeter_remove),
 	.shutdown = rackmeter_shutdown,
diff --git a/drivers/net/bmac.c b/drivers/net/bmac.c
index 39250b2..959add2 100644
--- a/drivers/net/bmac.c
+++ b/drivers/net/bmac.c
@@ -1654,8 +1654,11 @@ MODULE_DEVICE_TABLE (of, bmac_match);
 
 static struct macio_driver bmac_driver =
 {
-	.name 		= "bmac",
-	.match_table	= bmac_match,
+	.driver = {
+		.name 		= "bmac",
+		.owner		= THIS_MODULE,
+		.of_match_table	= bmac_match,
+	},
 	.probe		= bmac_probe,
 	.remove		= bmac_remove,
 #ifdef CONFIG_PM
diff --git a/drivers/net/mace.c b/drivers/net/mace.c
index b6855a6..1c5221f 100644
--- a/drivers/net/mace.c
+++ b/drivers/net/mace.c
@@ -997,8 +997,11 @@ MODULE_DEVICE_TABLE (of, mace_match);
 
 static struct macio_driver mace_driver =
 {
-	.name 		= "mace",
-	.match_table	= mace_match,
+	.driver = {
+		.name 		= "mace",
+		.owner		= THIS_MODULE,
+		.of_match_table	= mace_match,
+	},
 	.probe		= mace_probe,
 	.remove		= mace_remove,
 };
diff --git a/drivers/net/wireless/orinoco/airport.c b/drivers/net/wireless/orinoco/airport.c
index 9bcee10..4a0a0e5 100644
--- a/drivers/net/wireless/orinoco/airport.c
+++ b/drivers/net/wireless/orinoco/airport.c
@@ -239,8 +239,11 @@ static struct of_device_id airport_match[] =
 MODULE_DEVICE_TABLE(of, airport_match);
 
 static struct macio_driver airport_driver = {
-	.name 		= DRIVER_NAME,
-	.match_table	= airport_match,
+	.driver = {
+		.name 		= DRIVER_NAME,
+		.owner		= THIS_MODULE,
+		.of_match_table	= airport_match,
+	},
 	.probe		= airport_attach,
 	.remove		= airport_detach,
 	.suspend	= airport_suspend,
diff --git a/drivers/scsi/mac53c94.c b/drivers/scsi/mac53c94.c
index 18735b3..3ddb4dc 100644
--- a/drivers/scsi/mac53c94.c
+++ b/drivers/scsi/mac53c94.c
@@ -542,8 +542,11 @@ MODULE_DEVICE_TABLE (of, mac53c94_match);
 
 static struct macio_driver mac53c94_driver = 
 {
-	.name 		= "mac53c94",
-	.match_table	= mac53c94_match,
+	.driver = {
+		.name 		= "mac53c94",
+		.owner		= THIS_MODULE,
+		.of_match_table	= mac53c94_match,
+	},
 	.probe		= mac53c94_probe,
 	.remove		= mac53c94_remove,
 };
diff --git a/drivers/scsi/mesh.c b/drivers/scsi/mesh.c
index a1c97e8..1f784fd 100644
--- a/drivers/scsi/mesh.c
+++ b/drivers/scsi/mesh.c
@@ -2036,8 +2036,11 @@ MODULE_DEVICE_TABLE (of, mesh_match);
 
 static struct macio_driver mesh_driver = 
 {
-	.name 		= "mesh",
-	.match_table	= mesh_match,
+	.driver = {
+		.name 		= "mesh",
+		.owner		= THIS_MODULE,
+		.of_match_table	= mesh_match,
+	},
 	.probe		= mesh_probe,
 	.remove		= mesh_remove,
 	.shutdown	= mesh_shutdown,
diff --git a/drivers/serial/pmac_zilog.c b/drivers/serial/pmac_zilog.c
index cabbdc7..5b9cde7 100644
--- a/drivers/serial/pmac_zilog.c
+++ b/drivers/serial/pmac_zilog.c
@@ -2005,8 +2005,11 @@ static struct of_device_id pmz_match[] =
 MODULE_DEVICE_TABLE (of, pmz_match);
 
 static struct macio_driver pmz_driver = {
-	.name 		= "pmac_zilog",
-	.match_table	= pmz_match,
+	.driver = {
+		.name 		= "pmac_zilog",
+		.owner		= THIS_MODULE,
+		.of_match_table	= pmz_match,
+	},
 	.probe		= pmz_attach,
 	.remove		= pmz_detach,
 	.suspend	= pmz_suspend,
diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c
index 6789337..3ff8cc5 100644
--- a/sound/aoa/soundbus/i2sbus/core.c
+++ b/sound/aoa/soundbus/i2sbus/core.c
@@ -437,9 +437,11 @@ static int i2sbus_shutdown(struct macio_dev* dev)
 }
 
 static struct macio_driver i2sbus_drv = {
-	.name = "soundbus-i2s",
-	.owner = THIS_MODULE,
-	.match_table = i2sbus_match,
+	.driver = {
+		.name = "soundbus-i2s",
+		.owner = THIS_MODULE,
+		.of_match_table = i2sbus_match,
+	},
 	.probe = i2sbus_probe,
 	.remove = i2sbus_remove,
 #ifdef CONFIG_PM

^ permalink raw reply related

* Re: [PATCH] powerpc: Don't export cvt_fd & _df when CONFIG_PPC_FPU is not set
From: Benjamin Herrenschmidt @ 2010-06-02  7:11 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: linuxppc-dev
In-Reply-To: <m2wruiy3zg.fsf@igel.home>


> > It's crashing in macio_pci_add_devices.
> 
> And snd-aoa is broken, too.

Right, all macio_driver's are. I have a fix, testing now.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH v3] powerpc: Add i8042 keyboard and mouse irq parsing
From: Dmitry Torokhov @ 2010-06-02  6:26 UTC (permalink / raw)
  To: Martyn Welch; +Cc: linux-input, linuxppc-dev
In-Reply-To: <4C050017.10905@ge.com>

On Tue, Jun 01, 2010 at 01:41:59PM +0100, Martyn Welch wrote:
> Benjamin Herrenschmidt wrote:
> > O
> >   
> >> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> >> index 48f0a00..3d169bb 100644
> >> --- a/arch/powerpc/kernel/setup-common.c
> >> +++ b/arch/powerpc/kernel/setup-common.c
> >> @@ -94,6 +94,10 @@ struct screen_info screen_info = {
> >>  	.orig_video_points = 16
> >>  };
> >>  
> >> +/* Variables required to store legacy IO irq routing */
> >> +int of_i8042_kbd_irq;
> >> +int of_i8042_aux_irq;
> >>     
> >
> > Is there a reasonable ifdef to use for the above or we don't care ?
> >
> >   
> >>  #ifdef __DO_IRQ_CANON
> >>  /* XXX should go elsewhere eventually */
> >>  int ppc_do_canonicalize_irqs;
> >> @@ -567,6 +571,15 @@ int check_legacy_ioport(unsigned long base_port)
> >>  			np = of_find_compatible_node(NULL, NULL, "pnpPNP,f03");
> >>  		if (np) {
> >>  			parent = of_get_parent(np);
> >> +
> >> +			of_i8042_kbd_irq = irq_of_parse_and_map(parent, 0);
> >> +			if (!of_i8042_kbd_irq)
> >> +				of_i8042_kbd_irq = 1;
> >> +
> >> +			of_i8042_aux_irq = irq_of_parse_and_map(parent, 1);
> >> +			if (!of_i8042_aux_irq)
> >> +				of_i8042_aux_irq = 12;
> >> +
> >>  			of_node_put(np);
> >>  			np = parent;
> >>  			break;
> >> diff --git a/drivers/input/serio/i8042-io.h b/drivers/input/serio/i8042-io.h
> >> index 847f4aa..5d48bb6 100644
> >> --- a/drivers/input/serio/i8042-io.h
> >> +++ b/drivers/input/serio/i8042-io.h
> >> @@ -27,6 +27,11 @@
> >>  #include <asm/irq.h>
> >>  #elif defined(CONFIG_SH_CAYMAN)
> >>  #include <asm/irq.h>
> >> +#elif defined(CONFIG_PPC)
> >> +extern int of_i8042_kbd_irq;
> >> +extern int of_i8042_aux_irq;
> >> +# define I8042_KBD_IRQ  of_i8042_kbd_irq
> >> +# define I8042_AUX_IRQ  of_i8042_aux_irq
> >>  #else
> >>  # define I8042_KBD_IRQ	1
> >>  # define I8042_AUX_IRQ	12
> >>     
> >
> > Now while that works, I do tend to dislike global variables like that.
> >
> > _Maybe_ a better approach would be to have those #define resolve to
> > functions:
> >
> > #define I8042_KBD_IRQ	of_find_i8042_kbd_irq()
> >
> > Or something like that ?
> >
> > That means ending up having 2 functions which more/less reproduce the
> > loop to find the driver in the device-tree but that's a minor
> > inconvenience.
> >
> > Now, maybe the variables are less bloat here. What do you think ? Which
> > way do you prefer ?
> >
> >   
> 
> Personally, I'm happy either way. If you'd prefer I implement these as
> functions, I can't quickly see why that wouldn't work. I thought using
> variables would be the least disruptive.

FYI: i8042 core expects I8042_{KBD|AUX}_IRQ to be integers, however it
is certainly fixable...

-- 
Dmitry

^ permalink raw reply

* Re: [RFC PATCH] powerpc: Emulate most load and store instructions in emulate_step()
From: Paul Mackerras @ 2010-06-02  6:00 UTC (permalink / raw)
  To: K.Prasad; +Cc: linuxppc-dev
In-Reply-To: <20100602052502.GA2451@in.ibm.com>

On Wed, Jun 02, 2010 at 10:55:02AM +0530, K.Prasad wrote:
> On Thu, May 20, 2010 at 10:49:55PM +1000, Paul Mackerras wrote:
> > This extends the emulate_step() function to handle most of the load
> > and store instructions implemented on current 64-bit server processors.
> > The aim is to handle all the load and store instructions used in the
> > kernel, so this handles the Altivec/VMX lvx and stvx and the VSX
> > lxv2dx and stxv2dx instructions (implemented in POWER7).
> > 
> 
> Can the emulate_step() function be used on BookIII E processors as well
> (arch/powerpc/kernel/kprobes.c invokes it irrespective of the host
> processor though)?

I expect it can.  I haven't checked what extra things I'd need to add
for Book 3E though.  I also want to use it on classic 32-bit as well.

Paul.

^ permalink raw reply

* [PATCH] agp/uninorth: Fix oops caused by flushing too much
From: Paul Mackerras @ 2010-06-02  5:33 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Michel Dänzer

This fixes a sporadic oops at boot on G5 Power Macs.  The table_end
variable has the address of the last byte of the table.  Adding on
PAGE_SIZE means we flush too much, and if the page after the table
is not mapped for any reason, the kernel will oops.  Instead we add
on 1 because flush_dcache_range() interprets its second argument as
the first byte past the range to be flushed.

Signed-off-by: Paul Mackerras <paulus@samba.org>
---

diff --git a/drivers/char/agp/uninorth-agp.c b/drivers/char/agp/uninorth-agp.c
index 6f48931..9aaa0eb 100644
--- a/drivers/char/agp/uninorth-agp.c
+++ b/drivers/char/agp/uninorth-agp.c
@@ -413,7 +413,7 @@ static int uninorth_create_gatt_table(struct agp_bridge_data *bridge)
 	bridge->gatt_table_real = (u32 *) table;
 	/* Need to clear out any dirty data still sitting in caches */
 	flush_dcache_range((unsigned long)table,
-			   (unsigned long)(table_end + PAGE_SIZE));
+			   (unsigned long)table_end + 1);
 	bridge->gatt_table = vmap(pages, (1 << page_order), 0, PAGE_KERNEL_NCG);
 
 	if (bridge->gatt_table == NULL)

^ permalink raw reply related

* Re: [RFC PATCH] powerpc: Emulate most load and store instructions in emulate_step()
From: K.Prasad @ 2010-06-02  5:25 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <20100520124955.GA29903@brick.ozlabs.ibm.com>

On Thu, May 20, 2010 at 10:49:55PM +1000, Paul Mackerras wrote:
> This extends the emulate_step() function to handle most of the load
> and store instructions implemented on current 64-bit server processors.
> The aim is to handle all the load and store instructions used in the
> kernel, so this handles the Altivec/VMX lvx and stvx and the VSX
> lxv2dx and stxv2dx instructions (implemented in POWER7).
> 

Can the emulate_step() function be used on BookIII E processors as well
(arch/powerpc/kernel/kprobes.c invokes it irrespective of the host
processor though)?

If yes, we can use it with hw_breakpoint_handler() of BookE
processors (like what is done on the PPC64 counterpart).

> The new code can emulate user mode instructions, and checks the
> effective address for a load or store if the saved state is for
> user mode.  It doesn't handle little-endian mode at present.
> 
> For floating-point, Altivec/VMX and VSX instructions, it checks
> that the saved MSR has the enable bit for the relevant facility
> set, and if so, assumes that the FP/VMX/VSX registers contain
> valid state, and does loads or stores directly to/from the
> FP/VMX/VSX registers, using assembly helpers in ldstfp.S.
> 

Thanks,
K.Prasad

^ permalink raw reply

* Re: [PATCH v21 011/100] eclone (11/11): Document sys_eclone
From: Sukadev Bhattiprolu @ 2010-06-02  1:38 UTC (permalink / raw)
  To: Albert Cahalan; +Cc: randy.dunlap, linuxppc-dev, linux-kernel
In-Reply-To: <AANLkTinxCKl7RVe62UhIpNe5EriCufkgaih2D2mLLmC4@mail.gmail.com>

| Come on, seriously, you know it's ia64 and hppa that
| have issues. Maybe the nommu ports also have issues.
| 
| The only portable way to specify the stack is base and offset,
| with flags or magic values for "share" and "kernel managed".

Ah, ok, we have not yet ported to IA64 and I see now where the #ifdef
comes in.

But are you saying that we should force x86 and other architectures to
specify base and offset for eclone() even though they currently specify
just the stack pointer to clone() ?

That would remove the ifdef, but could be a big change to applications
on x86 and other architectures.

| 
| > | There is no reason to have field usage vary by architecture. The
| >
| > The field usage does not vary by architecture. Some architectures
| > don't use some fields and those fields must be 0.
| 
| It looks like you contradict yourself. Please explain how
| those two sentences are compatible.
| 
| > | original clone syscall was not designed with ia64 and hppa
| > | in mind, and has been causing trouble ever since. Let's not
| > | perpetuate the problem.
| >
| > and lot of folks contributed to this new API to try and make sure
| > it is portable and meets the forseeable requirements.
| 
| Right, and some folks were ignored.

I don't think your comment was ignored. The ->child_stack_size field was
added specifically for IA64 and my understanding was that ->clone_flags_high
could be used to specify the "kernel managed" or "shared" mode you mention
above.

| >
| > I don't understand how "making up some numbers (pids) that will work"
| > is more portable/cleaner than the proposed eclone().
| 
| It isolates the cross-platform problems to an obscure tool
| instead of polluting the kernel interface that everybody uses.

Sure, there was talk about using an approach like /proc/<pid>/next_pid
where you write your target pid into the file and the next time you
fork() you get that target pid. But it was considered racy and ugly.

Sukadev

^ permalink raw reply

* Re: [PATCH] powerpc: Optimise per cpu accesses on 64bit
From: Benjamin Herrenschmidt @ 2010-06-01 23:50 UTC (permalink / raw)
  To: Alan Modra; +Cc: linuxppc-dev, Anton Blanchard
In-Reply-To: <20100601224026.GG5631@bubble.grove.modra.org>

On Wed, 2010-06-02 at 08:10 +0930, Alan Modra wrote:
> Not r12.  It is used in function prologue and epilogue code.  If you
> want a dedicated gpr I think you'll need to use (and lose) one of the
> non-volatile regs. 

Which would probably not be a big deal... 

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH 1/5] sched: fix capacity calculations for SMT4
From: Vaidyanathan Srinivasan @ 2010-06-01 22:52 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Michael Neuling, Suresh Siddha, Gautham R Shenoy, linux-kernel,
	linuxppc-dev, Ingo Molnar
In-Reply-To: <1275294796.27810.21554.camel@twins>

* Peter Zijlstra <peterz@infradead.org> [2010-05-31 10:33:16]:

> On Fri, 2010-04-16 at 15:58 +0200, Peter Zijlstra wrote:
> > 
> > 
> > Hrmm, my brain seems muddled but I might have another solution, let me
> > ponder this for a bit..
> > 
> 
> Right, so the thing I was thinking about is taking the group capacity
> into account when determining the capacity for a single cpu.
> 
> Say the group contains all the SMT siblings, then use the group capacity
> (usually larger than 1024) and then distribute the capacity over the
> group members, preferring CPUs with higher individual cpu_power over
> those with less.
> 
> So suppose you've got 4 siblings with cpu_power=294 each, then we assign
> capacity 1 to the first member, and the remaining 153 is insufficient,
> and thus we stop and the rest lives with 0 capacity.
> 
> Now take the example that the first sibling would be running a heavy RT
> load, and its cpu_power would be reduced to say, 50, then we still got
> nearly 933 left over the others, which is still sufficient for one
> capacity, but because the first sibling is low, we'll assign it 0 and
> instead assign 1 to the second, again, leaving the third and fourth 0.

Hi Peter,

Thanks for the suggestion.

> If the group were a core group, the total would be much higher and we'd
> likely end up assigning 1 to each before we'd run out of capacity.

This is a tricky case because we are depending upon the
DIV_ROUND_CLOSEST to decide whether to flag capacity to 0 or 1.  We
will not have any task movement until capacity is depleted to quite
low value due to RT task.  Having a threshold to flag 0/1 instead of
DIV_ROUND_CLOSEST just like you have suggested in the power savings
case may help here as well to move tasks to other idle cores.

> For power savings, we can lower the threshold and maybe use the maximal
> individual cpu_power in the group to base 1 capacity from.
> 
> So, suppose the second example, where sibling0 has 50 and the others
> have 294, you'd end up with a capacity distribution of: {0,1,1,1}.

One challenge here is that if RT tasks run on more that one thread in
this group, we will have slightly different cpu powers.  Arranging
them from max to min and having a cutoff threshold should work.

Should we keep the RT scaling as a separate entity along with
cpu_power to simplify these thresholds.  Whenever we need to scale
group load with cpu power can take the product of cpu_power and
scale_rt_power but in these cases where we compute capacity, we can
mark a 0 or 1 just based on whether scale_rt_power was less than
SCHED_LOAD_SCALE or not.  Alternatively we can keep cpu_power as
a product of all scaling factors as it is today but save the component
scale factors also like scale_rt_power() and arch_scale_freq_power()
so that it can be used in load balance decisions.

Basically in power save balance we would give all threads a capacity
'1' unless the cpu_power was reduced due to RT task.  Similarly in
the non-power save case, we can have flag 1,0,0,0 unless first thread
had a RT scaling during the last interval.

I am suggesting to distinguish the reduction is cpu_power due to
architectural (hardware DVFS) reasons from RT tasks so that it is easy
to decide if moving tasks to sibling thread or core can help or not.

--Vaidy

^ permalink raw reply

* Re: [PATCH] powerpc: Optimise per cpu accesses on 64bit
From: Alan Modra @ 2010-06-01 22:40 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Anton Blanchard
In-Reply-To: <1275375920.1931.699.camel@pasglop>

On Tue, Jun 01, 2010 at 05:05:20PM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2010-06-01 at 14:45 +1000, Anton Blanchard wrote:
> > Now we dynamically allocate the paca array, it takes an extra load
> > whenever we want to access another cpu's paca. One place we do that a lot
> > is per cpu variables. A simple example:
> 
> Can't we dedicate a GPR instead ? Or it isn't worth it ? Something we
> almost never use in the kernel like r12 ?

Not r12.  It is used in function prologue and epilogue code.  If you
want a dedicated gpr I think you'll need to use (and lose) one of the
non-volatile regs.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply

* Re: [PATCH v21 011/100] eclone (11/11): Document sys_eclone
From: Albert Cahalan @ 2010-06-01 19:59 UTC (permalink / raw)
  To: Sukadev Bhattiprolu; +Cc: randy.dunlap, linuxppc-dev, linux-kernel
In-Reply-To: <20100601193230.GA17579@us.ibm.com>

On Tue, Jun 1, 2010 at 3:32 PM, Sukadev Bhattiprolu
<sukadev@linux.vnet.ibm.com> wrote:
> Albert Cahalan [acahalan@gmail.com] wrote:
> | Sukadev Bhattiprolu writes:
> | > Randy Dunlap [randy.dunlap at oracle.com] wrote:

> | >>> base of the region allocated for stack. These architectures
> | >>> must pass in the size of the stack-region in ->child_stack_size.
> | >>
> | >>                               stack region
> | >>
> | >> Seems unfortunate that different architectures use
> | >> the fields differently.
> | >
> | > Yes and no. The field still has a single purpose, just that
> | > some architectures may not need it. We enforce that if unused
> | > on an architecture, the field must be 0. It looked like
> | > the easiest way to keep the API common across architectures.
> |
> | Yuck. You're forcing userspace to have #ifdef messes or,
> | more likely, just not work on all architectures.
>
> There is going to be #ifdef code in the library interface to eclone().
> But applications should not need any #ifdefs. Please see the test cases
> for eclone in
>
>        git://git.sr71.net/~hallyn/cr_tests.git
>
> There is no #ifdef and the tests work on x86, x86_64, ppc, s390.

Come on, seriously, you know it's ia64 and hppa that
have issues. Maybe the nommu ports also have issues.

The only portable way to specify the stack is base and offset,
with flags or magic values for "share" and "kernel managed".

> | There is no reason to have field usage vary by architecture. The
>
> The field usage does not vary by architecture. Some architectures
> don't use some fields and those fields must be 0.

It looks like you contradict yourself. Please explain how
those two sentences are compatible.

> | original clone syscall was not designed with ia64 and hppa
> | in mind, and has been causing trouble ever since. Let's not
> | perpetuate the problem.
>
> and lot of folks contributed to this new API to try and make sure
> it is portable and meets the forseeable requirements.

Right, and some folks were ignored.

> | Given code like this:   stack_base = malloc(stack_size);
> | stack_base and stack_size are what the kernel needs.
> |
> | I suspect that you chose the defective method for some reason
> | related to restarting processes that were created with the
> | older system calls. I can't say most of us even care, but in
> | that broken-already case your process restarter can make up
> | some numbers that will work. (for i386, the base could be the
> | lowest address in the vma in which %esp lies, or even address 0)
>
> I don't understand how "making up some numbers (pids) that will work"
> is more portable/cleaner than the proposed eclone().

It isolates the cross-platform problems to an obscure tool
instead of polluting the kernel interface that everybody uses.

^ permalink raw reply

* [PATCH] powerpc/oprofile: fix potential buffer overrun in op_model_cell.c
From: Denis Kirjanov @ 2010-06-01 19:43 UTC (permalink / raw)
  To: arnd, benh, paulus, jkosina
  Cc: robert.richter, linuxppc-dev, oprofile-list, cel

Fix potential initial_lfsr buffer overrun.
Writing past the end of the buffer could happen when index == ENTRIES

Signed-off-by: Denis Kirjanov <dkirjanov@kernel.org>
---
 arch/powerpc/oprofile/op_model_cell.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/oprofile/op_model_cell.c b/arch/powerpc/oprofile/op_model_cell.c
index 2c9e522..7fd90d0 100644
--- a/arch/powerpc/oprofile/op_model_cell.c
+++ b/arch/powerpc/oprofile/op_model_cell.c
@@ -1077,7 +1077,7 @@ static int calculate_lfsr(int n)
 		index = ENTRIES-1;
 
 	/* make sure index is valid */
-	if ((index > ENTRIES) || (index < 0))
+	if ((index >= ENTRIES) || (index < 0))
 		index = ENTRIES-1;
 
 	return initial_lfsr[index];

^ permalink raw reply related

* Re: [PATCH v21 011/100] eclone (11/11): Document sys_eclone
From: Sukadev Bhattiprolu @ 2010-06-01 19:32 UTC (permalink / raw)
  To: Albert Cahalan; +Cc: randy.dunlap, linuxppc-dev, linux-kernel
In-Reply-To: <AANLkTimDfbJkAuoK1-asvk8Ub9C9QBkK5RkzLR3ilJUY@mail.gmail.com>

Albert Cahalan [acahalan@gmail.com] wrote:
| Sukadev Bhattiprolu writes:
| 
| > Randy Dunlap [randy.dunlap at oracle.com] wrote:
| >>> base of the region allocated for stack. These architectures
| >>> must pass in the size of the stack-region in ->child_stack_size.
| >>
| >>                               stack region
| >>
| >> Seems unfortunate that different architectures use
| >> the fields differently.
| >
| > Yes and no. The field still has a single purpose, just that
| > some architectures may not need it. We enforce that if unused
| > on an architecture, the field must be 0. It looked like
| > the easiest way to keep the API common across architectures.
| 
| Yuck. You're forcing userspace to have #ifdef messes or,
| more likely, just not work on all architectures.

There is going to be #ifdef code in the library interface to eclone().
But applications should not need any #ifdefs. Please see the test cases
for eclone in

	git://git.sr71.net/~hallyn/cr_tests.git

There is no #ifdef and the tests work on x86, x86_64, ppc, s390.

These use the libeclone.a built from following git-tree, which has the
arch-dependent user space code.

	git://git.ncl.cs.columbia.edu/pub/git/user-cr.git

Is that the #ifdef mess you are talking about ? I don't see that as a
consequence of the API. So maybe you can elaborate.

| There is no reason to have field usage vary by architecture. The

The field usage does not vary by architecture. Some architectures
don't use some fields and those fields must be 0. A simple 

	memset(&clone_args, 0, sizeof(clone_args))

before initializing fields is all that is required.

| original clone syscall was not designed with ia64 and hppa
| in mind, and has been causing trouble ever since. Let's not
| perpetuate the problem.

and lot of folks contributed to this new API to try and make sure
it is portable and meets the forseeable requirements.

| 
| Given code like this:   stack_base = malloc(stack_size);
| stack_base and stack_size are what the kernel needs.
| 
| I suspect that you chose the defective method for some reason
| related to restarting processes that were created with the
| older system calls. I can't say most of us even care, but in
| that broken-already case your process restarter can make up
| some numbers that will work. (for i386, the base could be the
| lowest address in the vma in which %esp lies, or even address 0)

I don't understand how "making up some numbers (pids) that will work"
is more portable/cleaner than the proposed eclone(). 

Sukadev

^ permalink raw reply

* Re: [PATCH] powerpc: Don't export cvt_fd & _df when CONFIG_PPC_FPU is not set
From: Andreas Schwab @ 2010-06-01 18:58 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <m3mxvfrt5n.fsf__49359.5060579993$1275385036$gmane$org@hase.home>

Andreas Schwab <schwab@redhat.com> writes:

> Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
>
>> On Mon, 2010-05-31 at 20:06 +0200, Andreas Schwab wrote:
>>> Alexander Graf <agraf@suse.de> writes:
>>> 
>>> > Mind to send it over so I can take a look at it :)? Getting rid of the
>>> > task_struct structs lying in the stack is certainly a good idea.
>>> 
>>> Still untested, because rc1 does not boot.
>>
>> Ouch ! What machine ?
>
> It's crashing in macio_pci_add_devices.

And snd-aoa is broken, too.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: [PATCH] PowerPC: Remove hardcoded BAT configuration of IMMR in CPM early debug console
From: Scott Wood @ 2010-06-01 16:06 UTC (permalink / raw)
  To: Martyn Welch; +Cc: linuxppc-dev
In-Reply-To: <4C050E84.2010602@ge.com>

On 06/01/2010 08:43 AM, Martyn Welch wrote:
>>>> diff --git a/arch/powerpc/kernel/head_32.S
>>>> b/arch/powerpc/kernel/head_32.S
>>>> index e025e89..861cace 100644
>>>> --- a/arch/powerpc/kernel/head_32.S
>>>> +++ b/arch/powerpc/kernel/head_32.S
>>>> @@ -1194,12 +1194,13 @@ setup_disp_bat:
>>>>    #endif /* CONFIG_BOOTX_TEXT */
>>>>
>>>>    #ifdef CONFIG_PPC_EARLY_DEBUG_CPM
>>>> +#define PPC_EARLY_DEBUG_CPM_ADDR
>>>> ASM_CONST(CONFIG_PPC_EARLY_DEBUG_CPM_ADDR)
>>>>    setup_cpm_bat:
>>>> -    lis    r8, 0xf000
>>>> +    lis    r8, PPC_EARLY_DEBUG_CPM_ADDR@ha
>>>>        ori    r8, r8,    0x002a
>>>>        mtspr    SPRN_DBAT1L, r8
>>>>
>>>> -    lis    r11, 0xf000
>>>> +    lis    r11, PPC_EARLY_DEBUG_CPM_ADDR@ha
>>>>        ori    r11, r11, (BL_1M<<  2) | 2
>>>>        mtspr    SPRN_DBAT1U, r11
>>>>
>>> Only the physical address should depend on where IMMR is.  We should
>>> use fixmap instead of an arbitrary address for the effective address.
>>> There's a existing FIX_EARLY_DEBUG_BASE, but it's only 128 KiB so
>>> we'll have to either grow it, or map only a subset of IMMR.
>>>
>>>
>>
>> I think that's a more fundamental change to CPM early debug than I can
>> handle right now.

Is IMMRBASE on your board at some address that has a low likelihood of 
conflicting when treated as a kernel effective address?

>>> Plus, CONFIG_PPC_EARLY_DEBUG_CPM_ADDR points to the TX descriptor, not
>>> to the beginning of IMMR, so you should mask off the lower 20 bits
>>> (the offset is probably less than 64K, and the BAT might just ignore
>>> the extra bits anyway, but why take chances?).
>>>
>>>
>>
>> I assume that an extra instruction "andi   r8, r8, 0xfff0" after each
>> "lis" instruction  would be what you are looking for?
>>
>>
> "andis" even.

"lis r8, (PPC_EARLY_DEBUG_CPM_ADDR & 0xfff00000)@h" should work too.

-Scott

^ permalink raw reply

* [PATCH] macio: don't dereference pointer before null check
From: Andreas Schwab @ 2010-06-01 15:58 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1275385607.1931.712.camel__30394.2679115154$1275385651$gmane$org@pasglop>

Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
---
 drivers/macintosh/macio_asic.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/macintosh/macio_asic.c b/drivers/macintosh/macio_asic.c
index 9714780..40390ac 100644
--- a/drivers/macintosh/macio_asic.c
+++ b/drivers/macintosh/macio_asic.c
@@ -492,8 +492,8 @@ static void macio_pci_add_devices(struct macio_chip *chip)
 	}
 
 	/* Add media bay devices if any */
-	pnode = mbdev->ofdev.dev.of_node;
-	if (mbdev)
+	if (mbdev) {
+		pnode = mbdev->ofdev.dev.of_node;
 		for (np = NULL; (np = of_get_next_child(pnode, np)) != NULL;) {
 			if (macio_skip_device(np))
 				continue;
@@ -502,10 +502,11 @@ static void macio_pci_add_devices(struct macio_chip *chip)
 						 mbdev,  root_res) == NULL)
 				of_node_put(np);
 		}
+	}
 
 	/* Add serial ports if any */
-	pnode = sdev->ofdev.dev.of_node;
 	if (sdev) {
+		pnode = sdev->ofdev.dev.of_node;
 		for (np = NULL; (np = of_get_next_child(pnode, np)) != NULL;) {
 			if (macio_skip_device(np))
 				continue;
-- 
1.7.1

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply related

* Re: [PATCH] drivers: remove all i2c_set_clientdata(client, NULL)
From: Jean Delvare @ 2010-06-01 13:53 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Dmitry Torokhov, linuxppc-dev, linux-mtd, linux-i2c, devel,
	Samuel Ortiz, lm-sensors, linux-input, linux-media,
	Paul Gortmaker, Riku Voipio, rtc-linux, Alessandro Rubini,
	Mauro Carvalho Chehab, Ben Dooks (embedded platforms),
	Greg Kroah-Hartman, Liam Girdwood, Alessandro Zummo,
	Guillaume Ligneul, George Joseph, Mark Brown, linux-kernel,
	Richard Purdie, Colin Leroy, David Woodhouse
In-Reply-To: <1275310552-14685-1-git-send-email-w.sang@pengutronix.de>

On Mon, 31 May 2010 14:55:48 +0200, Wolfram Sang wrote:
> I2C-drivers can use the clientdata-pointer to point to private data. As I2C
> devices are not really unregistered, but merely detached from their driver, it
> used to be the drivers obligation to clear this pointer during remove() or a
> failed probe(). As a couple of drivers forgot to do this, it was agreed that it
> was cleaner if the i2c-core does this clearance when appropriate, as there is
> no guarantee for the lifetime of the clientdata-pointer after remove() anyhow.
> This feature was added to the core with commit
> e4a7b9b04de15f6b63da5ccdd373ffa3057a3681 to fix the faulty drivers.
> 
> As there is no need anymore to clear the clientdata-pointer, remove all current
> occurrences in the drivers to simplify the code and prevent confusion.
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Jean Delvare <khali@linux-fr.org>
> (...)

Applied, thanks. Will go to Linus in the next few days.

-- 
Jean Delvare

^ permalink raw reply

* Re: [PATCH] PowerPC: Remove hardcoded BAT configuration of IMMR in CPM early debug console
From: Martyn Welch @ 2010-06-01 13:43 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <4C050295.408@ge.com>

Martyn Welch wrote:
> Scott Wood wrote:
>   
>> On 05/28/2010 10:18 AM, Martyn Welch wrote:
>>     
>>> The CPM early debug console hardcodes the BAT to cover the IMMR at
>>> 0xf0000000. The IMMR (on the mpc8270 at the very least) can be set to a
>>> number of locations with bootstrap configuration, which are outside the
>>> hardcoded BAT configuration.
>>>
>>> This patch determines the correct location at which to configure a BAT
>>> during the boot process from the supplied PPC_EARLY_DEBUG_CPM_ADDR.
>>>
>>> Signed-off-by: Martyn Welch<martyn.welch@ge.com>
>>> ---
>>>
>>>   arch/powerpc/kernel/head_32.S    |    5 +++--
>>>   arch/powerpc/sysdev/cpm_common.c |    4 +++-
>>>   2 files changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/head_32.S
>>> b/arch/powerpc/kernel/head_32.S
>>> index e025e89..861cace 100644
>>> --- a/arch/powerpc/kernel/head_32.S
>>> +++ b/arch/powerpc/kernel/head_32.S
>>> @@ -1194,12 +1194,13 @@ setup_disp_bat:
>>>   #endif /* CONFIG_BOOTX_TEXT */
>>>
>>>   #ifdef CONFIG_PPC_EARLY_DEBUG_CPM
>>> +#define PPC_EARLY_DEBUG_CPM_ADDR
>>> ASM_CONST(CONFIG_PPC_EARLY_DEBUG_CPM_ADDR)
>>>   setup_cpm_bat:
>>> -    lis    r8, 0xf000
>>> +    lis    r8, PPC_EARLY_DEBUG_CPM_ADDR@ha
>>>       ori    r8, r8,    0x002a
>>>       mtspr    SPRN_DBAT1L, r8
>>>
>>> -    lis    r11, 0xf000
>>> +    lis    r11, PPC_EARLY_DEBUG_CPM_ADDR@ha
>>>       ori    r11, r11, (BL_1M << 2) | 2
>>>       mtspr    SPRN_DBAT1U, r11
>>>       
>> Only the physical address should depend on where IMMR is.  We should
>> use fixmap instead of an arbitrary address for the effective address.
>> There's a existing FIX_EARLY_DEBUG_BASE, but it's only 128 KiB so
>> we'll have to either grow it, or map only a subset of IMMR.
>>
>>     
>
> I think that's a more fundamental change to CPM early debug than I can
> handle right now.
>
>   
>> Plus, CONFIG_PPC_EARLY_DEBUG_CPM_ADDR points to the TX descriptor, not
>> to the beginning of IMMR, so you should mask off the lower 20 bits
>> (the offset is probably less than 64K, and the BAT might just ignore
>> the extra bits anyway, but why take chances?).
>>
>>     
>
> I assume that an extra instruction "andi   r8, r8, 0xfff0" after each
> "lis" instruction  would be what you are looking for?
>
>   
"andis" even.

Martyn

> Martyn
>
>   
>> -Scott
>>     
>
>
>   


-- 
Martyn Welch (Principal Software Engineer)   |   Registered in England and
GE Intelligent Platforms                     |   Wales (3828642) at 100
T +44(0)127322748                            |   Barbirolli Square, Manchester,
E martyn.welch@ge.com                        |   M2 3AB  VAT:GB 927559189

^ permalink raw reply

* Re: [PATCH] PowerPC: Remove hardcoded BAT configuration of IMMR in CPM early debug console
From: Martyn Welch @ 2010-06-01 12:52 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <4BFFECF1.9060809@freescale.com>

Scott Wood wrote:
> On 05/28/2010 10:18 AM, Martyn Welch wrote:
>> The CPM early debug console hardcodes the BAT to cover the IMMR at
>> 0xf0000000. The IMMR (on the mpc8270 at the very least) can be set to a
>> number of locations with bootstrap configuration, which are outside the
>> hardcoded BAT configuration.
>>
>> This patch determines the correct location at which to configure a BAT
>> during the boot process from the supplied PPC_EARLY_DEBUG_CPM_ADDR.
>>
>> Signed-off-by: Martyn Welch<martyn.welch@ge.com>
>> ---
>>
>>   arch/powerpc/kernel/head_32.S    |    5 +++--
>>   arch/powerpc/sysdev/cpm_common.c |    4 +++-
>>   2 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/head_32.S
>> b/arch/powerpc/kernel/head_32.S
>> index e025e89..861cace 100644
>> --- a/arch/powerpc/kernel/head_32.S
>> +++ b/arch/powerpc/kernel/head_32.S
>> @@ -1194,12 +1194,13 @@ setup_disp_bat:
>>   #endif /* CONFIG_BOOTX_TEXT */
>>
>>   #ifdef CONFIG_PPC_EARLY_DEBUG_CPM
>> +#define PPC_EARLY_DEBUG_CPM_ADDR
>> ASM_CONST(CONFIG_PPC_EARLY_DEBUG_CPM_ADDR)
>>   setup_cpm_bat:
>> -    lis    r8, 0xf000
>> +    lis    r8, PPC_EARLY_DEBUG_CPM_ADDR@ha
>>       ori    r8, r8,    0x002a
>>       mtspr    SPRN_DBAT1L, r8
>>
>> -    lis    r11, 0xf000
>> +    lis    r11, PPC_EARLY_DEBUG_CPM_ADDR@ha
>>       ori    r11, r11, (BL_1M << 2) | 2
>>       mtspr    SPRN_DBAT1U, r11
>
> Only the physical address should depend on where IMMR is.  We should
> use fixmap instead of an arbitrary address for the effective address.
> There's a existing FIX_EARLY_DEBUG_BASE, but it's only 128 KiB so
> we'll have to either grow it, or map only a subset of IMMR.
>

I think that's a more fundamental change to CPM early debug than I can
handle right now.

> Plus, CONFIG_PPC_EARLY_DEBUG_CPM_ADDR points to the TX descriptor, not
> to the beginning of IMMR, so you should mask off the lower 20 bits
> (the offset is probably less than 64K, and the BAT might just ignore
> the extra bits anyway, but why take chances?).
>

I assume that an extra instruction "andi   r8, r8, 0xfff0" after each
"lis" instruction  would be what you are looking for?

Martyn

> -Scott


-- 
Martyn Welch (Principal Software Engineer)   |   Registered in England and
GE Intelligent Platforms                     |   Wales (3828642) at 100
T +44(0)127322748                            |   Barbirolli Square, Manchester,
E martyn.welch@ge.com                        |   M2 3AB  VAT:GB 927559189

^ permalink raw reply

* Re: [PATCH v3] powerpc: Add i8042 keyboard and mouse irq parsing
From: Martyn Welch @ 2010-06-01 12:41 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Dmitry Torokhov, linux-input
In-Reply-To: <1275104126.1931.521.camel@pasglop>

Benjamin Herrenschmidt wrote:
> O
>   
>> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
>> index 48f0a00..3d169bb 100644
>> --- a/arch/powerpc/kernel/setup-common.c
>> +++ b/arch/powerpc/kernel/setup-common.c
>> @@ -94,6 +94,10 @@ struct screen_info screen_info = {
>>  	.orig_video_points = 16
>>  };
>>  
>> +/* Variables required to store legacy IO irq routing */
>> +int of_i8042_kbd_irq;
>> +int of_i8042_aux_irq;
>>     
>
> Is there a reasonable ifdef to use for the above or we don't care ?
>
>   
>>  #ifdef __DO_IRQ_CANON
>>  /* XXX should go elsewhere eventually */
>>  int ppc_do_canonicalize_irqs;
>> @@ -567,6 +571,15 @@ int check_legacy_ioport(unsigned long base_port)
>>  			np = of_find_compatible_node(NULL, NULL, "pnpPNP,f03");
>>  		if (np) {
>>  			parent = of_get_parent(np);
>> +
>> +			of_i8042_kbd_irq = irq_of_parse_and_map(parent, 0);
>> +			if (!of_i8042_kbd_irq)
>> +				of_i8042_kbd_irq = 1;
>> +
>> +			of_i8042_aux_irq = irq_of_parse_and_map(parent, 1);
>> +			if (!of_i8042_aux_irq)
>> +				of_i8042_aux_irq = 12;
>> +
>>  			of_node_put(np);
>>  			np = parent;
>>  			break;
>> diff --git a/drivers/input/serio/i8042-io.h b/drivers/input/serio/i8042-io.h
>> index 847f4aa..5d48bb6 100644
>> --- a/drivers/input/serio/i8042-io.h
>> +++ b/drivers/input/serio/i8042-io.h
>> @@ -27,6 +27,11 @@
>>  #include <asm/irq.h>
>>  #elif defined(CONFIG_SH_CAYMAN)
>>  #include <asm/irq.h>
>> +#elif defined(CONFIG_PPC)
>> +extern int of_i8042_kbd_irq;
>> +extern int of_i8042_aux_irq;
>> +# define I8042_KBD_IRQ  of_i8042_kbd_irq
>> +# define I8042_AUX_IRQ  of_i8042_aux_irq
>>  #else
>>  # define I8042_KBD_IRQ	1
>>  # define I8042_AUX_IRQ	12
>>     
>
> Now while that works, I do tend to dislike global variables like that.
>
> _Maybe_ a better approach would be to have those #define resolve to
> functions:
>
> #define I8042_KBD_IRQ	of_find_i8042_kbd_irq()
>
> Or something like that ?
>
> That means ending up having 2 functions which more/less reproduce the
> loop to find the driver in the device-tree but that's a minor
> inconvenience.
>
> Now, maybe the variables are less bloat here. What do you think ? Which
> way do you prefer ?
>
>   

Personally, I'm happy either way. If you'd prefer I implement these as
functions, I can't quickly see why that wouldn't work. I thought using
variables would be the least disruptive.

I'm also happy to change it so that the irqs are specified in the kbd
and aux nodes (I agree it would make much more sense and was how the
first revision of this patch worked).

Either way, my preferred solution is that contained in this patch,
unless those with more experience agree otherwise... ;-)

Martyn

> Cheers,
> Ben.
>
>
>   


-- 
Martyn Welch (Principal Software Engineer)   |   Registered in England and
GE Intelligent Platforms                     |   Wales (3828642) at 100
T +44(0)127322748                            |   Barbirolli Square, Manchester,
E martyn.welch@ge.com                        |   M2 3AB  VAT:GB 927559189

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox