Skip to contentSkip to conversation

ppc64: Introduce Cell/BPA platform, v2

PATCH In review Arnd Bergmann 9 participants 37 messages May 13, 2005 – Jun 10, 2005

Subsystems (6)
lore
Share & tools
lore

Arnd Bergmann

author maintainer
#0 lore raw

cover ppc64: Introduce Cell/BPA platform, v2

This series of patches add support for a fifth platform type in the ppc64 architecture tree. The Broadband Processor Architecture (BPA) is what machines using the Cell processor should be following and currently only prototype hardware exists for it.

Except for the last patch, these are functionally the same as the first version but are updated for 2.6.12-rc4 and contain changes based on the feedback I got so far.

The first three patches add some infrastructure that is used by BPA machines but is not really specific to them can could be used by other new platform types as well.

The next three patches add the actual platform code, which should be usable for any BPA compatible implementation.

Patch 7 introduces a new file system to make use of the SPUs inside the processors. This patch is still in a prototype stage and not intended for merging yet. The final patch adds some user space code in the Documentation directory that clarifies how to use the file system. This one should become a separate package at a later point.

	Arnd <><
author maintainer
#1 lore raw

PATCH 1/8 ppc64: split out generic rtas code from pSeries_pci.c

BPA is using rtas for PCI but should not be confused by pSeries code. This also avoids some #ifdefs. Other platforms that want to use rtas_pci.c could create their own platform_pci.c with platform specific fixups.

arch/ppc64/kernel/Makefile +2 -1
@@ -32,13 +32,14 @@ obj-$(CONFIG_PPC_MULTIPLATFORM) += nvram
 
 obj-$(CONFIG_PPC_PSERIES) += pSeries_pci.o pSeries_lpar.o pSeries_hvCall.o \
 			     pSeries_nvram.o rtasd.o ras.o pSeries_reconfig.o \
-			     xics.o rtas.o pSeries_setup.o pSeries_iommu.o
+			     xics.o pSeries_setup.o pSeries_iommu.o
 
 obj-$(CONFIG_EEH)		+= eeh.o
 obj-$(CONFIG_PROC_FS)		+= proc_ppc64.o
 obj-$(CONFIG_RTAS_FLASH)	+= rtas_flash.o
 obj-$(CONFIG_SMP)		+= smp.o
 obj-$(CONFIG_MODULES)		+= module.o ppc_ksyms.o
+obj-$(CONFIG_PPC_RTAS)		+= rtas.o rtas_pci.o
 obj-$(CONFIG_RTAS_PROC)		+= rtas-proc.o
 obj-$(CONFIG_SCANLOG)		+= scanlog.o
 obj-$(CONFIG_VIOPATH)		+= viopath.o
arch/ppc64/kernel/mpic.h +3 -0
@@ -265,3 +265,6 @@ extern void mpic_send_ipi(unsigned int i
 extern int mpic_get_one_irq(struct mpic *mpic, struct pt_regs *regs);
 /* This one gets to the primary mpic */
 extern int mpic_get_irq(struct pt_regs *regs);
+
+/* global mpic for pSeries */
+extern struct mpic *pSeries_mpic;
arch/ppc64/kernel/pSeries_pci.c +6 -470

512 lines not on this page · raw message

arch/ppc64/kernel/rtas_pci.c +495 -0

495 lines not on this page · raw message

author maintainer
#2 lore raw

PATCH 2/8 ppc64: add a minimal nvram driver

In reply to Arnd Bergmann

The firmware provides the location and size of the nvram in the device tree, so it does not really contain any hardware specific bits and could be used on other machines as well.

From: Utz Bacher <[email protected]>

arch/ppc64/kernel/bpa_nvram.c new file +118 -0
Index: linus-2.5/arch/ppc64/kernel/bpa_nvram.c
===================================================================
@@ -0,0 +1,118 @@
+/*
+ * NVRAM for CPBW
+ *
+ * (C) Copyright IBM Corp. 2005
+ *
+ * Authors : Utz Bacher <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#include <asm/machdep.h>
+#include <asm/nvram.h>
+#include <asm/prom.h>
+
+static void __iomem *bpa_nvram_start;
+static long bpa_nvram_len;
+static spinlock_t bpa_nvram_lock = SPIN_LOCK_UNLOCKED;
+
+static ssize_t bpa_nvram_read(char *buf, size_t count, loff_t *index)
+{
+	unsigned long flags;
+
+	if (*index >= bpa_nvram_len)
+		return 0;
+	if (*index + count > bpa_nvram_len)
+		count = bpa_nvram_len - *index;
+
+	spin_lock_irqsave(&bpa_nvram_lock, flags);
+
+	memcpy_fromio(buf, bpa_nvram_start + *index, count);
+
+	spin_unlock_irqrestore(&bpa_nvram_lock, flags);
+	
+	*index += count;
+	return count;
+}
+
+static ssize_t bpa_nvram_write(char *buf, size_t count, loff_t *index)
+{
+	unsigned long flags;
+
+	if (*index >= bpa_nvram_len)
+		return 0;
+	if (*index + count > bpa_nvram_len)
+		count = bpa_nvram_len - *index;
+
+	spin_lock_irqsave(&bpa_nvram_lock, flags);
+
+	memcpy_toio(bpa_nvram_start + *index, buf, count);
+
+	spin_unlock_irqrestore(&bpa_nvram_lock, flags);
+	
+	*index += count;
+	return count;
+}
+
+static ssize_t bpa_nvram_get_size(void)
+{
+	return bpa_nvram_len;
+}
+
+int __init bpa_nvram_init(void)
+{
+	struct device_node *nvram_node;
+	unsigned long *buffer;
+	int proplen;
+	unsigned long nvram_addr;
+	int ret;
+
+	ret = -ENODEV;
+	nvram_node = of_find_node_by_type(NULL, "nvram");
+	if (!nvram_node)
+		goto out;
+
+	ret = -EIO;
+	buffer = (unsigned long *)get_property(nvram_node, "reg", &proplen);
+	if (proplen != 2*sizeof(unsigned long))
+		goto out;
+
+	ret = -ENODEV;
+	nvram_addr = buffer[0];
+	bpa_nvram_len = buffer[1];
+	if ( (!bpa_nvram_len) || (!nvram_addr) )
+		goto out;
+
+	bpa_nvram_start = ioremap(nvram_addr, bpa_nvram_len);
+	if (!bpa_nvram_start)
+		goto out;
+
+	printk(KERN_INFO "BPA NVRAM, %luk mapped to %p\n",
+	       bpa_nvram_len >> 10, bpa_nvram_start);
+
+	ppc_md.nvram_read	= bpa_nvram_read;
+	ppc_md.nvram_write	= bpa_nvram_write;
+	ppc_md.nvram_size	= bpa_nvram_get_size;
+
+out:
+	of_node_put(nvram_node);
+	return ret;
+}
include/asm-ppc64/nvram.h +1 -0
Index: linus-2.5/include/asm-ppc64/nvram.h
===================================================================
@@ -70,6 +70,7 @@
 
 extern int pSeries_nvram_init(void);
 extern int pmac_nvram_init(void);
+extern int bpa_nvram_init(void);
 
 /* PowerMac specific nvram stuffs */
 
author maintainer
#3 lore raw

PATCH 3/8 ppc64: add a watchdog driver for rtas

In reply to Arnd Bergmann

Add a watchdog using the RTAS OS surveillance service. This is provided as a simpler alternative to rtasd. The added value is that it works with standard watchdog client programs and can therefore also do user space monitoring.

On BPA, rtasd is not really useful because the hardware does not have much to report with event-scan.

The driver should also work on other platforms that support the OS surveillance rtas calls.

From: Utz Bacher <[email protected]>

drivers/char/watchdog/Kconfig +10 -0
@@ -414,6 +414,16 @@ config WATCHDOG_RIO
 	  machines.  The watchdog timeout period is normally one minute but
 	  can be changed with a boot-time parameter.
 
+# ppc64 RTAS watchdog
+config WATCHDOG_RTAS
+	tristate "RTAS watchdog"
+	depends on WATCHDOG && PPC_RTAS
+	help
+	  This driver adds watchdog support for the RTAS watchdog.
+
+          To compile this driver as a module, choose M here. The module
+	  will be called wdrtas.
+
 #
 # ISA-based Watchdog Cards
 #
drivers/char/watchdog/Makefile +1 -0
@@ -33,6 +33,7 @@ obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.
 obj-$(CONFIG_IXP4XX_WATCHDOG) += ixp4xx_wdt.o
 obj-$(CONFIG_IXP2000_WATCHDOG) += ixp2000_wdt.o
 obj-$(CONFIG_8xx_WDT) += mpc8xx_wdt.o
+obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
 
 # Only one watchdog can succeed. We probe the hardware watchdog
 # drivers first, then the softdog driver.  This means if your hardware
drivers/char/watchdog/wdrtas.c +691 -0

691 lines not on this page · raw message

#4 lore raw

Re: [PATCH 3/8] ppc64: add a watchdog driver for rtas

Quoted 1 line · Arnd Bergmann

+static volatile int wdrtas_miscdev_open = 0;

...

drivers/char/watchdog/wdrtas.c line 406

Quoted 6 lines
drivers/char/watchdog/wdrtas.c +6 -0
+static int
+wdrtas_open(struct inode *inode, struct file *file)
+{
+	/* only open once */
+	if (xchg(&wdrtas_miscdev_open,1))
+		return -EBUSY;

The volatile and xchg strike me as an obscure method for ensuring only one process at a time can open this file. Any reason a semaphore couldn't be used?

drivers/char/watchdog/wdrtas.c line 431

Quoted 9 lines
drivers/char/watchdog/wdrtas.c +9 -0
+static int
+wdrtas_close(struct inode *inode, struct file *file)
+{
+	/* only stop watchdog, if this was announced using 'V' before */
+	if (wdrtas_expect_close == WDRTAS_MAGIC_CHAR)
+		wdrtas_timer_stop();
+	else {
+		printk("wdrtas: got unexpected close. Watchdog "
+		       "not stopped.\n");

printk's need a valid log level specified. There are several in this file that lack them.

Nathan

author maintainer
#5 lore raw

Re: [PATCH 3/8] ppc64: add a watchdog driver for rtas

Quoted 13 lines · Nathan Lynch
Quoted 1 line · Arnd Bergmann

+static volatile int wdrtas_miscdev_open = 0;

...

drivers/char/watchdog/wdrtas.c line 406

Quoted 6 lines
drivers/char/watchdog/wdrtas.c +6 -0
+static int
+wdrtas_open(struct inode *inode, struct file *file)
+{
+	/* only open once */
+	if (xchg(&wdrtas_miscdev_open,1))
+		return -EBUSY;

The volatile and xchg strike me as an obscure method for ensuring only one process at a time can open this file. Any reason a semaphore couldn't be used?

A semaphore would also be the wrong approach since we don't want processes to block but instead to fail opening the watchdog twice. Other watchdog drivers use atomic_t or bitops to guard open, which imho would be the better solution.

Of course, there is also Wim's plan to do a unified watchdog driver that would solve this once and for all.

Quoted 5 lines

drivers/char/watchdog/wdrtas.c line 431

Quoted 2 lines
drivers/char/watchdog/wdrtas.c +2 -0
+		printk("wdrtas: got unexpected close. Watchdog "
+		       "not stopped.\n");

printk's need a valid log level specified. There are several in this file that lack them.

Right.

Utz, do you have time to fix up these issues? If not, I probably won't look into it this week either.

Thanks,

	Arnd <><
#6 lore raw

Re: [PATCH 3/8] ppc64: add a watchdog driver for rtas

Quoted 15 lines · Arnd Bergmann
Quoted 13 lines · Nathan Lynch
Quoted 1 line · Arnd Bergmann

+static volatile int wdrtas_miscdev_open = 0;

...

drivers/char/watchdog/wdrtas.c line 406

Quoted 6 lines
drivers/char/watchdog/wdrtas.c +6 -0
+static int
+wdrtas_open(struct inode *inode, struct file *file)
+{
+	/* only open once */
+	if (xchg(&wdrtas_miscdev_open,1))
+		return -EBUSY;

The volatile and xchg strike me as an obscure method for ensuring only one process at a time can open this file. Any reason a semaphore couldn't be used?

Quoted 2 lines

A semaphore would also be the wrong approach since we don't want processes to block but instead to fail opening the watchdog twice.

I should have been more explicit. What I had in mind was using down_trylock and returning -EBUSY if it failed.

Nathan

author maintainer
#7 lore raw

Re: [PATCH 3/8] ppc64: add a watchdog driver for rtas

Quoted 6 lines · Nathan Lynch
Quoted 3 lines

A semaphore would also be the wrong approach since we don't want processes to block but instead to fail opening the watchdog twice.

I should have been more explicit.  What I had in mind was using down_trylock and returning -EBUSY if it failed.

Well, that's also pointless. If the only operations you ever do on a semaphore are down_trylock() and up(), you end up using only the atomic variable in there while wasting a few bytes of extra memory for storing the wait queue head ;-)

	Arnd <><
author maintainer
#8 lore raw

PATCH 4/8 ppc64: add BPA platform type

In reply to Arnd Bergmann

This adds the basic support for running on BPA machines. So far, this is only the IBM workstation, and it will not run on others without a little more generalization.

It should be possible to configure a kernel for any combination of CONFIG_PPC_BPA with any of the other multiplatform targets.

MAINTAINERS +7 -0
Index: linus-2.5/MAINTAINERS
===================================================================
@@ -493,6 +493,13 @@
 W:   http://sourceforge.net/projects/bonding/
 S:   Supported
 
+BROADBAND PROCESSOR ARCHITECTURE
+P:	Arnd Bergmann
+M:	[email protected]
+L:	[email protected]
+W:	http://linuxppc64.org
+S:	Supported
+
 BTTV VIDEO4LINUX DRIVER
 P:	Gerd Knorr
 M:	[email protected]
arch/ppc64/Kconfig +5 -1
Index: linus-2.5/arch/ppc64/Kconfig
===================================================================
@@ -77,6 +77,10 @@
 	bool "  IBM pSeries & new iSeries"
 	default y
 
+config PPC_BPA
+	bool "  Broadband Processor Architecture"
+	depends on PPC_MULTIPLATFORM
+
 config PPC_PMAC
 	depends on PPC_MULTIPLATFORM
 	bool "  Apple G5 based machines"
@@ -256,7 +260,7 @@
 
 config PPC_RTAS
 	bool
-	depends on PPC_PSERIES
+	depends on PPC_PSERIES || PPC_BPA
 	default y
 
 config RTAS_PROC
arch/ppc64/Makefile +2 -0
Index: linus-2.5/arch/ppc64/Makefile
===================================================================
@@ -90,12 +90,14 @@
 boottarget-$(CONFIG_PPC_PSERIES) := zImage zImage.initrd
 boottarget-$(CONFIG_PPC_MAPLE) := zImage zImage.initrd
 boottarget-$(CONFIG_PPC_ISERIES) := vmlinux.sminitrd vmlinux.initrd vmlinux.sm
+boottarget-$(CONFIG_PPC_BPA) := zImage zImage.initrd
 $(boottarget-y): vmlinux
 	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
 
 bootimage-$(CONFIG_PPC_PSERIES) := $(boot)/zImage
 bootimage-$(CONFIG_PPC_PMAC) := vmlinux
 bootimage-$(CONFIG_PPC_MAPLE) := $(boot)/zImage
+bootimage-$(CONFIG_PPC_BPA) := zImage
 bootimage-$(CONFIG_PPC_ISERIES) := vmlinux
 BOOTIMAGE := $(bootimage-y)
 install: vmlinux
arch/ppc64/kernel/Makefile +3 -0
Index: linus-2.5/arch/ppc64/kernel/Makefile
===================================================================
@@ -34,6 +34,8 @@
 			     pSeries_nvram.o rtasd.o ras.o pSeries_reconfig.o \
 			     xics.o pSeries_setup.o pSeries_iommu.o
 
+obj-$(CONFIG_PPC_BPA) += bpa_setup.o bpa_nvram.o
+
 obj-$(CONFIG_EEH)		+= eeh.o
 obj-$(CONFIG_PROC_FS)		+= proc_ppc64.o
 obj-$(CONFIG_RTAS_FLASH)	+= rtas_flash.o
@@ -60,6 +62,7 @@
 obj-$(CONFIG_PPC_PMAC)		+= pmac_smp.o smp-tbsync.o
 obj-$(CONFIG_PPC_ISERIES)	+= iSeries_smp.o
 obj-$(CONFIG_PPC_PSERIES)	+= pSeries_smp.o
+obj-$(CONFIG_PPC_BPA)		+= pSeries_smp.o
 obj-$(CONFIG_PPC_MAPLE)		+= smp-tbsync.o
 endif
 
arch/ppc64/kernel/bpa_setup.c new file +207 -0

207 lines not on this page · raw message

arch/ppc64/kernel/cpu_setup_power4.S +15 -1
Index: linus-2.5/arch/ppc64/kernel/cpu_setup_power4.S
===================================================================
@@ -73,7 +73,21 @@
 
 _GLOBAL(__setup_cpu_power4)
 	blr
-	
+
+_GLOBAL(__setup_cpu_be)
+        /* Set large page sizes LP=0: 16MB, LP=1: 64KB */
+        addi    r3, 0,  0
+        ori     r3, r3, HID6_LB
+        sldi    r3, r3, 32
+        nor     r3, r3, r3
+        mfspr   r4, SPRN_HID6
+        and     r4, r4, r3
+        addi    r3, 0, 0x02000
+        sldi    r3, r3, 32
+        or      r4, r4, r3
+        mtspr   SPRN_HID6, r4
+	blr
+
 _GLOBAL(__setup_cpu_ppc970)
 	mfspr	r0,SPRN_HID0
 	li	r11,5			/* clear DOZE and SLEEP */
arch/ppc64/kernel/cputable.c +11 -0
Index: linus-2.5/arch/ppc64/kernel/cputable.c
===================================================================
@@ -34,6 +34,7 @@
 extern void __setup_cpu_power3(unsigned long offset, struct cpu_spec* spec);
 extern void __setup_cpu_power4(unsigned long offset, struct cpu_spec* spec);
 extern void __setup_cpu_ppc970(unsigned long offset, struct cpu_spec* spec);
+extern void __setup_cpu_be(unsigned long offset, struct cpu_spec* spec);
 
 
 /* We only set the altivec features if the kernel was compiled with altivec
@@ -162,6 +163,16 @@
 	    __setup_cpu_power4,
 	    COMMON_PPC64_FW
     },
+    {	/* BE DD1.x  */
+	    0xffff0000, 0x00700000, "Broadband Engine",
+	    CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB | CPU_FTR_HPTE_TABLE |
+		    CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_ALTIVEC_COMP |
+		    CPU_FTR_SMT,
+	    COMMON_USER_PPC64 | PPC_FEATURE_HAS_ALTIVEC_COMP,
+	    128, 128,
+	    __setup_cpu_be,
+	    COMMON_PPC64_FW
+    },
     {	/* default match */
 	    0x00000000, 0x00000000, "POWER4 (compatible)",
   	    CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB | CPU_FTR_HPTE_TABLE |
arch/ppc64/kernel/irq.c +3 -0
Index: linus-2.5/arch/ppc64/kernel/irq.c
===================================================================
@@ -395,6 +395,9 @@
 	if (ppc64_interrupt_controller == IC_OPEN_PIC)
 		return real_irq;	/* no mapping for openpic (for now) */
 
+	if (ppc64_interrupt_controller == IC_BPA_IIC)
+		return real_irq;	/* no mapping for iic either */
+
 	/* don't map interrupts < MIN_VIRT_IRQ */
 	if (real_irq < MIN_VIRT_IRQ) {
 		virt_irq_to_real_map[real_irq] = real_irq;
arch/ppc64/kernel/proc_ppc64.c +1 -1
Index: linus-2.5/arch/ppc64/kernel/proc_ppc64.c
===================================================================
@@ -53,7 +53,7 @@
 	if (!root)
 		return 1;
 
-	if (!(systemcfg->platform & PLATFORM_PSERIES))
+	if (!(systemcfg->platform & (PLATFORM_PSERIES | PLATFORM_BPA)))
 		return 0;
 
 	if (!proc_mkdir("rtas", root))
arch/ppc64/kernel/prom_init.c +2 -2
Index: linus-2.5/arch/ppc64/kernel/prom_init.c
===================================================================
@@ -1844,9 +1844,9 @@
 		     &getprop_rval, sizeof(getprop_rval));
 
 	/*
-	 * On pSeries, copy the CPU hold code
+	 * On pSeries and BPA, copy the CPU hold code
 	 */
-       	if (RELOC(of_platform) & PLATFORM_PSERIES)
+       	if (RELOC(of_platform) & (PLATFORM_PSERIES | PLATFORM_BPA))
        		copy_and_flush(0, KERNELBASE - offset, 0x100, 0);
 
 	/*
arch/ppc64/kernel/setup.c +4 -0
Index: linus-2.5/arch/ppc64/kernel/setup.c
===================================================================
@@ -348,6 +348,7 @@
 extern struct machdep_calls pSeries_md;
 extern struct machdep_calls pmac_md;
 extern struct machdep_calls maple_md;
+extern struct machdep_calls bpa_md;
 
 /* Ultimately, stuff them in an elf section like initcalls... */
 static struct machdep_calls __initdata *machines[] = {
@@ -360,6 +361,9 @@
 #ifdef CONFIG_PPC_MAPLE
 	&maple_md,
 #endif /* CONFIG_PPC_MAPLE */
+#ifdef CONFIG_PPC_BPA
+	&bpa_md,
+#endif
 	NULL
 };
 
arch/ppc64/kernel/traps.c +4 -0
Index: linus-2.5/arch/ppc64/kernel/traps.c
===================================================================
@@ -126,6 +126,10 @@
 			printk("POWERMAC ");
 			nl = 1;
 			break;
+		case PLATFORM_BPA:
+			printk("BPA ");
+			nl = 1;
+			break;
 	}
 	if (nl)
 		printk("\n");
include/asm-ppc64/mmu.h +3 -2
Index: linus-2.5/include/asm-ppc64/mmu.h
===================================================================
@@ -47,9 +47,10 @@
 #define SLB_VSID_KS		ASM_CONST(0x0000000000000800)
 #define SLB_VSID_KP		ASM_CONST(0x0000000000000400)
 #define SLB_VSID_N		ASM_CONST(0x0000000000000200) /* no-execute */
-#define SLB_VSID_L		ASM_CONST(0x0000000000000100) /* largepage 16M */
+#define SLB_VSID_L		ASM_CONST(0x0000000000000100) /* largepage */
 #define SLB_VSID_C		ASM_CONST(0x0000000000000080) /* class */
-
+#define SLB_VSID_LS		ASM_CONST(0x0000000000000070) /* size of largepage */
+ 
 #define SLB_VSID_KERNEL		(SLB_VSID_KP|SLB_VSID_C)
 #define SLB_VSID_USER		(SLB_VSID_KP|SLB_VSID_KS)
 
include/asm-ppc64/processor.h +15 -4
Index: linus-2.5/include/asm-ppc64/processor.h
===================================================================
@@ -217,14 +217,22 @@
 #define   HID0_ABE	(1<<3)		/* Address Broadcast Enable */
 #define	  HID0_BHTE	(1<<2)		/* Branch History Table Enable */
 #define	  HID0_BTCD	(1<<1)		/* Branch target cache disable */
+#define	SPRN_HID6	0x3F9	/* Hardware Implementation Register 6 */
+#define	  HID6_LB	(0x0F<<12)	/* Concurrent Large Page Modes */
+#define	  HID6_DLP	(1<<20)		/* Disable all large page modes (4K only) */
 #define	SPRN_MSRDORM	0x3F1	/* Hardware Implementation Register 1 */
 #define SPRN_HID1	0x3F1	/* Hardware Implementation Register 1 */
 #define	SPRN_IABR	0x3F2	/* Instruction Address Breakpoint Register */
 #define	SPRN_NIADORM	0x3F3	/* Hardware Implementation Register 2 */
 #define SPRN_HID4	0x3F4	/* 970 HID4 */
 #define SPRN_HID5	0x3F6	/* 970 HID5 */
-#define	SPRN_TSC 	0x3FD	/* Thread switch control */
-#define	SPRN_TST 	0x3FC	/* Thread switch timeout */
+#define	SPRN_TSCR	0x399   /* Thread switch control on BE */
+#define	SPRN_TTR	0x39A   /* Thread switch timeout on BE */
+#define	  TSCR_DEC_ENABLE	0x200000 /* Decrementer Interrupt */
+#define	  TSCR_EE_ENABLE	0x100000 /* External Interrupt */
+#define	  TSCR_EE_BOOST		0x080000 /* External Interrupt Boost */
+#define	SPRN_TSC 	0x3FD	/* Thread switch control on others */
+#define	SPRN_TST 	0x3FC	/* Thread switch timeout on others */
 #define	SPRN_IAC1	0x3F4	/* Instruction Address Compare 1 */
 #define	SPRN_IAC2	0x3F5	/* Instruction Address Compare 2 */
 #define	SPRN_ICCR	0x3FB	/* Instruction Cache Cacheability Register */
@@ -411,8 +419,9 @@
 #define	PV_POWER5	0x003A
 #define PV_POWER5p	0x003B
 #define PV_970FX	0x003C
-#define	PV_630        	0x0040
-#define	PV_630p	        0x0041
+#define	PV_630		0x0040
+#define	PV_630p		0x0041
+#define	PV_BE		0x0070
 
 /* Platforms supported by PPC64 */
 #define PLATFORM_PSERIES      0x0100
@@ -421,6 +430,7 @@
 #define PLATFORM_LPAR         0x0001
 #define PLATFORM_POWERMAC     0x0400
 #define PLATFORM_MAPLE        0x0500
+#define PLATFORM_BPA          0x1000
 
 /* Compatibility with drivers coming from PPC32 world */
 #define _machine	(systemcfg->platform)
@@ -432,6 +442,7 @@
 #define IC_INVALID    0
 #define IC_OPEN_PIC   1
 #define IC_PPC_XIC    2
+#define IC_BPA_IIC    3
 
 #define XGLUE(a,b) a##b
 #define GLUE(a,b) XGLUE(a,b)
include/asm-ppc64/smp.h +8 -0
Index: linus-2.5/include/asm-ppc64/smp.h
===================================================================
@@ -85,6 +85,14 @@
 
 extern struct smp_ops_t *smp_ops;
 
+#ifdef CONFIG_PPC_PSERIES
+void vpa_init(int cpu);
+#else
+static inline void vpa_init(int cpu)
+{
+}
+#endif /* CONFIG_PPC_PSERIES */
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* !(_PPC64_SMP_H) */
#9 lore raw

Re: [PATCH 4/8] ppc64: add BPA platform type

arch/ppc64/kernel/bpa_setup.c line 86

Quoted 6 lines · Arnd Bergmann

This adds the basic support for running on BPA machines. So far, this is only the IBM workstation, and it will not run on others without a little more generalization.

arch/ppc64/kernel/bpa_setup.c new file +2 -0
+/* FIXME: consolidate this into rtas.c or similar */
+static void __init pSeries_calibrate_decr(void)

Shouldn't this be called bpa_calibrate_decr or something similar?

include/asm-ppc64/processor.h line 423

Quoted 4 lines
include/asm-ppc64/processor.h +2 -2
-#define	PV_630        	0x0040
-#define	PV_630p	        0x0041
+#define	PV_630		0x0040
+#define	PV_630p		0x0041

Hmmm, I don't think your patch needs to clean up the whitespace here.

Regards,
Paul.

author maintainer
#10 lore raw

Re: [PATCH 4/8] ppc64: add BPA platform type

Quoted 10 lines · Paul Mackerras

arch/ppc64/kernel/bpa_setup.c line 86

Quoted 6 lines · Arnd Bergmann

This adds the basic support for running on BPA machines. So far, this is only the IBM workstation, and it will not run on others without a little more generalization.

arch/ppc64/kernel/bpa_setup.c new file +2 -0
+/* FIXME: consolidate this into rtas.c or similar */
+static void __init pSeries_calibrate_decr(void)

Shouldn't this be called bpa_calibrate_decr or something similar?

The function is identical to the one for pSeries, and I'd prefer to have only one copy of it with a more generic name. Actually, it looks like maple and perhaps pmac have a very similar *_calibrate_decr function, so I could perhaps just put this into time.c as generic_calibrate_decr().

[ Ben, can you tell if pSeries_calibrate_decr should work on
all G5 macs or if it can be changed to support them as well? ]

On a similar issue, I just remembered that I wanted to create a rtas_time.c to hold the rtc access functions for pSeries and BPA. Do you think that's a good idea?

Quoted 6 lines

include/asm-ppc64/processor.h line 423

Quoted 4 lines
include/asm-ppc64/processor.h +2 -2
-#define	PV_630        	0x0040
-#define	PV_630p	        0x0041
+#define	PV_630		0x0040
+#define	PV_630p		0x0041

Hmmm, I don't think your patch needs to clean up the whitespace here.

ok.

	Arnd <><
author maintainer
#11 lore raw

PATCH 5/8 ppc64: Add driver for BPA interrupt controllers

In reply to Arnd Bergmann

Add support for the integrated interrupt controller on BPA CPUs. There is one of those for each SMT thread.

The mapping of interrupt numbers to HW interrupt sources is described in arch/ppc64/kernel/bpa_iic.h.

This version hardcodes the 'Spider' chip as the secondary interrupt controller. That is not really generic for the architecture, but at the moment it is the only secondary PIC that exists.

A little more work will be needed on this as soon as we have boards with multiple external interrupt controllers.

arch/ppc64/Kconfig +15 -0
Index: linus-2.5/arch/ppc64/Kconfig
===================================================================
@@ -106,6 +106,21 @@
 	bool
 	default y
 
+config XICS
+	depends on PPC_PSERIES
+	bool
+	default y
+
+config MPIC
+	depends on PPC_PSERIES || PPC_PMAC || PPC_MAPLE
+	bool
+	default y
+
+config BPA_IIC
+	depends on PPC_BPA
+	bool
+	default y
+
 # VMX is pSeries only for now until somebody writes the iSeries
 # exception vectors for it
 config ALTIVEC
arch/ppc64/kernel/Makefile +5 -3
Index: linus-2.5/arch/ppc64/kernel/Makefile
===================================================================
@@ -28,13 +28,13 @@
 			     mf.o HvLpEvent.o iSeries_proc.o iSeries_htab.o \
 			     iSeries_iommu.o
 
-obj-$(CONFIG_PPC_MULTIPLATFORM) += nvram.o i8259.o prom_init.o prom.o mpic.o
+obj-$(CONFIG_PPC_MULTIPLATFORM) += nvram.o i8259.o prom_init.o prom.o
 
 obj-$(CONFIG_PPC_PSERIES) += pSeries_pci.o pSeries_lpar.o pSeries_hvCall.o \
 			     pSeries_nvram.o rtasd.o ras.o pSeries_reconfig.o \
-			     xics.o pSeries_setup.o pSeries_iommu.o
+			     pSeries_setup.o pSeries_iommu.o
 
-obj-$(CONFIG_PPC_BPA) += bpa_setup.o bpa_nvram.o
+obj-$(CONFIG_PPC_BPA) += bpa_setup.o bpa_nvram.o bpa_iic.o spider-pic.o
 
 obj-$(CONFIG_EEH)		+= eeh.o
 obj-$(CONFIG_PROC_FS)		+= proc_ppc64.o
@@ -50,6 +50,8 @@
 obj-$(CONFIG_BOOTX_TEXT)	+= btext.o
 obj-$(CONFIG_HVCS)		+= hvcserver.o
 obj-$(CONFIG_IBMVIO)		+= vio.o
+obj-$(CONFIG_XICS)		+= xics.o
+obj-$(CONFIG_MPIC)		+= mpic.o
 
 obj-$(CONFIG_PPC_PMAC)		+= pmac_setup.o pmac_feature.o pmac_pci.o \
 				   pmac_time.o pmac_nvram.o pmac_low_i2c.o
arch/ppc64/kernel/bpa_iic.c new file +270 -0

270 lines not on this page · raw message

arch/ppc64/kernel/bpa_iic.h new file +62 -0

62 lines not on this page · raw message

arch/ppc64/kernel/bpa_setup.c +5 -1
Index: linus-2.5/arch/ppc64/kernel/bpa_setup.c
===================================================================
@@ -45,6 +45,7 @@
 #include <asm/cputable.h>
 
 #include "pci.h"
+#include "bpa_iic.h"
 
 #ifdef DEBUG
 #define DBG(fmt...) udbg_printf(fmt)
@@ -143,6 +144,9 @@
 
 static void __init bpa_setup_arch(void)
 {
+	ppc_md.init_IRQ       = iic_init_IRQ;
+	ppc_md.get_irq        = iic_get_irq;
+
 #ifdef CONFIG_SMP
 	smp_init_pSeries();
 #endif
@@ -158,7 +162,7 @@
 	/* Find and initialize PCI host bridges */
 	init_pci_config_tokens();
 	find_and_init_phbs();
-
+	spider_init_IRQ();
 #ifdef CONFIG_DUMMY_CONSOLE
 	conswitchp = &dummy_con;
 #endif
arch/ppc64/kernel/pSeries_smp.c +64 -5

117 lines not on this page · raw message

arch/ppc64/kernel/smp.c +1 -1
Index: linus-2.5/arch/ppc64/kernel/smp.c
===================================================================
@@ -71,7 +71,7 @@
 
 int smt_enabled_at_boot = 1;
 
-#ifdef CONFIG_PPC_MULTIPLATFORM
+#if defined(CONFIG_PPC_PSERIES) || defined(CONFIG_PPC_PMAC) || defined(CONFIG_PPC_MAPLE)
 void smp_mpic_message_pass(int target, int msg)
 {
 	/* make sure we're sending something that translates to an IPI */
arch/ppc64/kernel/spider-pic.c new file +191 -0

191 lines not on this page · raw message

author maintainer
#12 lore raw

PATCH 6/8 ppc64: Add driver for BPA iommu

In reply to Arnd Bergmann

Implementation of software load support for the BE iommu. This is very different from other iommu code on ppc64, since we only do a static mapping. The mapping is currently hardcoded but should really be read from the firmware, but they don't set up the device nodes yet. There is a single 512MB DMA window for PCI, USB and ethernet at 0x20000000 for our RAM.

The Cell processor can put the I/O page table either in memory like the hashed page table (hardware load) or have the operating system write the entries into memory mapped CPU registers (software load).

I use the software load mechanism because I know that all I/O page table entries for the amount of installed physical memory fit into the IO TLB cache. At the point when we get machines with more than 4GB of installed memory, we can either use hardware I/O page table access like the other platforms do or dynamically update the I/O TLB entries when a page fault occurs in the I/O subsystem.

The software load can then use the macros that I have implemented for the static mapping in order to do the TLB cache updates.

arch/ppc64/kernel/Makefile +2 -1
Index: linus-2.5/arch/ppc64/kernel/Makefile
===================================================================
@@ -34,7 +34,8 @@
 			     pSeries_nvram.o rtasd.o ras.o pSeries_reconfig.o \
 			     pSeries_setup.o pSeries_iommu.o
 
-obj-$(CONFIG_PPC_BPA) += bpa_setup.o bpa_nvram.o bpa_iic.o spider-pic.o
+obj-$(CONFIG_PPC_BPA) += bpa_setup.o bpa_iommu.o bpa_nvram.o \
+			 bpa_iic.o spider-pic.o
 
 obj-$(CONFIG_EEH)		+= eeh.o
 obj-$(CONFIG_PROC_FS)		+= proc_ppc64.o
arch/ppc64/kernel/bpa_iommu.c new file +377 -0

377 lines not on this page · raw message

arch/ppc64/kernel/bpa_iommu.h new file +65 -0

65 lines not on this page · raw message

arch/ppc64/kernel/bpa_setup.c +2 -1
Index: linus-2.5/arch/ppc64/kernel/bpa_setup.c
===================================================================
@@ -46,6 +46,7 @@
 
 #include "pci.h"
 #include "bpa_iic.h"
+#include "bpa_iommu.h"
 
 #ifdef DEBUG
 #define DBG(fmt...) udbg_printf(fmt)
@@ -179,7 +180,7 @@
 
 	hpte_init_native();
 
-	pci_direct_iommu_init();
+	bpa_init_iommu();
 
 	ppc64_interrupt_controller = IC_BPA_IIC;
 
author maintainer
#13 lore raw

PATCH 7/8 ppc64: SPU file system

In reply to Arnd Bergmann

This is an early version of the SPU file system, which is used to run code on the Synergistic Processing Units of the Broadband Engine.

The file system provides a name space similar to posix shared memory or message queues. Users that have write permissions on the file system can create directories in the spufs root.

Every directory represents an SPU context, which is currently mapped to a physical SPU, but that is going to change to a virtualization scheme in future updates.

An SPU context directory contains a predefined set of files used for manipulating the state of the logical SPU. Users can change permissions on those files, but not actually add or remove files without removing the complete directory.

The current set of files is:

/mem the contents of the local store memory of the SPU.
This can be accessed like a regular shared memory file and contains both code and data in the address space of the SPU.
The implemented file operations currently are read(), write() and mmap(). We will need our own address space operations as soon as we allow the SPU context to be scheduled away from the physical SPU into page cache.

/run A stub file that lets us do ioctl. The only ioctl
method we need is the spu_run() call. spu_run suspends the current thread from the host CPU and transfers the flow of execution to the SPU.
The ioctl call return to the calling thread when a state is entered that can not be handled by the kernel, e.g. an error in the SPU code or an exit() from it. When a signal is pending for the host CPU thread, the ioctl is interrupted and the SPU stopped in order to call the signal handler.

/mbox The first SPU to CPU communication mailbox. This file
is read-only and can be read in units of 32 bits. The file can only be used in non-blocking mode and it even poll() will not block on it.
When no data is available in the mailbox, read() returns EAGAIN.

/ibox The second SPU to CPU communication mailbox. This file
is similar to the first mailbox file, but can be read in blocking I/O mode, and the poll familiy of system calls can be used to wait for it.

/wbox The CPU to SPU communation mailbox. It is write-only
can can be written in units of 32 bits. If the mailbox is full, write() will block and poll can be used to wait for it becoming empty again.

Other files are planned but currently are not implemented or not functional.

arch/ppc64/kernel/Makefile +1 -0
@@ -53,6 +53,7 @@ obj-$(CONFIG_HVCS)		+= hvcserver.o
 obj-$(CONFIG_IBMVIO)		+= vio.o
 obj-$(CONFIG_XICS)		+= xics.o
 obj-$(CONFIG_MPIC)		+= mpic.o
+obj-$(CONFIG_SPU_FS)		+= spu_base.o
 
 obj-$(CONFIG_PPC_PMAC)		+= pmac_setup.o pmac_feature.o pmac_pci.o \
 				   pmac_time.o pmac_nvram.o pmac_low_i2c.o
arch/ppc64/kernel/spu_base.c +579 -0

579 lines not on this page · raw message

arch/ppc64/mm/hash_utils.c +1 -0
@@ -354,6 +354,7 @@ int hash_page(unsigned long ea, unsigned
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(hash_page);
 
 void flush_hash_page(unsigned long context, unsigned long ea, pte_t pte,
 		     int local)
fs/Kconfig +10 -0
@@ -853,6 +853,16 @@ config HUGETLBFS
 config HUGETLB_PAGE
 	def_bool HUGETLBFS
 
+config SPU_FS
+	tristate "SPU file system"
+	default m
+	depends on PPC_BPA
+	help
+	  The SPU file system is used to access Synergistic Processing
+	  Units on machines implementing the Broadband Processor
+	  Architecture.
+	
+
 config RAMFS
 	bool
 	default y
fs/Makefile +1 -0
@@ -95,3 +95,4 @@ obj-$(CONFIG_BEFS_FS)		+= befs/
 obj-$(CONFIG_HOSTFS)		+= hostfs/
 obj-$(CONFIG_HPPFS)		+= hppfs/
 obj-$(CONFIG_DEBUG_FS)		+= debugfs/
+obj-$(CONFIG_SPU_FS)		+= spufs/
fs/spufs/Makefile +3 -0
@@ -0,0 +1,3 @@
+obj-$(CONFIG_SPU_FS) += spufs.o
+
+spufs-y += inode.o
fs/spufs/inode.c +991 -0

991 lines not on this page · raw message

include/asm-ppc64/spu.h +463 -0

463 lines not on this page · raw message

mm/memory.c +1 -0
@@ -2194,6 +2194,7 @@ unsigned long vmalloc_to_pfn(void * vmal
 {
 	return page_to_pfn(vmalloc_to_page(vmalloc_addr));
 }
+EXPORT_SYMBOL_GPL(handle_mm_fault);
 
 EXPORT_SYMBOL(vmalloc_to_pfn);
 
#14 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

Quoted 11 lines

/run A stub file that lets us do ioctl. The only ioctl
method we need is the spu_run() call. spu_run suspends the current thread from the host CPU and transfers the flow of execution to the SPU.
The ioctl call return to the calling thread when a state is entered that can not be handled by the kernel, e.g. an error in the SPU code or an exit() from it. When a signal is pending for the host CPU thread, the ioctl is interrupted and the SPU stopped in order to call the signal handler.

ioctl's are generally considered evil ... what about a write() method writing a command ?

Ben.

#15 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

Hi!

Quoted 11 lines
Quoted 10 lines

/run A stub file that lets us do ioctl. The only ioctl
method we need is the spu_run() call. spu_run suspends the current thread from the host CPU and transfers the flow of execution to the SPU.
The ioctl call return to the calling thread when a state is entered that can not be handled by the kernel, e.g. an error in the SPU code or an exit() from it. When a signal is pending for the host CPU thread, the ioctl is interrupted and the SPU stopped in order to call the signal handler.

Quoted 2 lines

ioctl's are generally considered evil ... what about a write() method writing a command ?

That's even more evil than ioctl()... Try doing 32-vs-64bit conversion on write...
Pavel

Signature · 1 line
Boycott Kodak -- for their patent abuse against Java.
#16 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

Quoted 6 lines
Quoted 3 lines

ioctl's are generally considered evil ... what about a write() method writing a command ?

That's even more evil than ioctl()... Try doing 32-vs-64bit conversion on write...

I don't see the problem ... if you are passing a structure, you have to convert it anyway, and it's bad practice. I was thinking about passing ascii so it can be controlled by shell scripts.

Ben.

author maintainer
#17 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

Quoted 7 lines · Benjamin Herrenschmidt
Quoted 3 lines

That's even more evil than ioctl()... Try doing 32-vs-64bit conversion on write...

I don't see the problem ... if you are passing a structure, you have to convert it anyway, and it's bad practice. I was thinking about passing ascii so it can be controlled by shell scripts.

Parsing multi-value ascii data is error prone. in kernel space, I would not want to do anything more complex than a simple_strtoul(), if only for the reason of not giving bad examples.

When passing binary structures, there is a significant difference between passing it through ioctl or read/write: We already have a rather complicated method of detecting if whether and how to convert them (f_op->compat_ioctl, hash lookup and the deprecated dynamic registration).

For read/write, there is no way to tell if you need to do the conversion, even if the file operation is aware of the actual data layout of both variants. Moreover, a good implementation of a read/write file operation should be able to deal with resuming partial transfers.

Regarding the shell scripting possibility, I don't really see the point. The only code that should actually use the kernel interfaces is something like an /lib/ld-spu.so interpreter and that is better implemented in C anyway because it needs to parse ELF structures and such.

	Arnd <><
#18 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

In reply to Arnd Bergmann
Quoted 3 lines · Arnd Bergmann

This is an early version of the SPU file system, which is used to run code on the Synergistic Processing Units of the Broadband Engine.

The whitespace seems a bit dammaged in places, check your tabs vs. spaces...

Quoted 1 line

/run A stub file that lets us do ioctl.

No, as Ben said, do not do this. Use write. And as you are only doing 1 type of ioctl, it shouldn't be an issue. Also it will be faster than the ioctl due to lack of BKL usage :)

And I don't quite think you do the proper permission and validate of the data in your code, you should verify this is all correct.

Quoted 8 lines

+/**** spufs attributes
+ *
+ * Attributes in spufs behave similar to those in sysfs:
+ *
+ * Writing to an attribute immediately sets a value, an open file can be
+ * written to multiple times.
+ *
+ * Reading from an attribute creates a buffer from the value that might get

Quoted 8 lines

+ * read with multiple read calls. When the attribute has been read completely,
+ * no further read calls are possible until the file is opened again.
+ *
+ * All spufs attributes contain a text representation of a numeric value that
+ * are accessed with the get() and set() functions.
+ *
+ * Perhaps these file operations could be put in debugfs or libfs instead,
+ * they are not really SPU specific.

Yes they should. I'll gladly take them for debugfs or like you state, libfs is probably the better place for them so everyone can use them.

If you make up a patch, I'll fix up debugfs to use them properly.

fs/spufs/inode.c line 536

Quoted 11 lines
fs/spufs/inode.c +11 -0
+#define spufs_attribute(name)						   \
+static int name ## _open(struct inode *inode, struct file *file)	   \
+{									   \
+	return spufs_attr_open(inode, file, &name ## _get, &name ## _set); \
+}									   \
+static struct file_operations name = {					   \
+	.open	 = name ## _open,					   \
+	.release = spufs_attr_close,					   \
+	.read	 = spufs_attr_read,					   \
+	.write	 = spufs_attr_write,					   \
+};

No module owner set? Be careful if not...

fs/spufs/inode.c line 758

Quoted 2 lines
fs/spufs/inode.c +2 -0
+static struct tree_descr spufs_dir_contents[] = {
+	{ "mem",  &spufs_mem_fops,  0644, },

Named identifiers are the better way to do this (yeah, longer code I know...)

Quoted 100 lines
fs/spufs/inode.c +100 -0
+	{ "run",  &spufs_run_fops,  0400, },
+	{ "mbox", &spufs_mbox_fops, 0400, },
+	{ "ibox", &spufs_ibox_fops, 0400, },
+	{ "wbox", &spufs_wbox_fops, 0200, },
+	{ "signal1_type", &spufs_signal1_type, 0600, },
+	{ "signal2_type", &spufs_signal1_type, 0600, },
+
+#if 1 /* debugging only */
+	{ "class0_mask", &spufs_class0_mask, 0600, },
+	{ "class1_mask", &spufs_class1_mask, 0600, },
+	{ "class2_mask", &spufs_class2_mask, 0600, },
+	{ "class0_stat", &spufs_class0_stat, 0600, },
+	{ "class1_stat", &spufs_class1_stat, 0600, },
+	{ "class2_stat", &spufs_class2_stat, 0600, },
+	{ "sr1", &spufs_mfc_sr1_RW, 0600, },
+	{ "fir", &spufs_mfc_fir_R, 0400, },
+	{ "fir_status_or", &spufs_mfc_fir_status_or_W, 0200, },
+	{ "fir_status_and", &spufs_mfc_fir_status_and_W, 0200, },
+	{ "fir_mask", &spufs_mfc_fir_mask_R, 0400, },
+	{ "fir_mask_or", &spufs_mfc_fir_mask_or_W, 0200, },
+	{ "fir_mask_and", &spufs_mfc_fir_mask_and_W, 0200, },
+	{ "fir_chkstp", &spufs_mfc_fir_chkstp_enable_RW, 0600, },
+	{ "cer", &spufs_mfc_cer_R, 0400, },
+	{ "dsisr", &spufs_mfc_dsisr_RW, 0600, },
+	{ "dsir", &spufs_mfc_dsir_R, 0200, },
+	{ "cntl", &spufs_mfc_control_RW, 0600, },
+	{ "sdr", &spufs_mfc_sdr_RW, 0600, },
+#endif
+	{},
+};
+
+static int
+spufs_fill_dir(struct dentry *dir, struct tree_descr *files,
+		int mode, struct spu_context *ctx)
+{
+	struct inode *inode;
+	struct dentry *dentry;
+	int ret;
+
+	static struct inode_operations iops = {
+		.getattr = simple_getattr,
+		.setattr = spufs_setattr,
+	};
+
+	ret = -ENOSPC;
+	while (files->name && files->name[0]) {
+		dentry = d_alloc_name(dir, files->name);
+		if (!dentry)
+			goto out;
+		inode = spufs_new_inode(dir->d_sb,
+				S_IFREG | (files->mode & mode));
+		if (!inode)
+			goto out;
+		inode->i_op = &iops;
+		inode->i_fop = files->ops;
+		inode->i_mapping->a_ops = &spufs_aops;
+		inode->i_mapping->backing_dev_info = &spufs_backing_dev_info;
+		SPUFS_I(inode)->i_ctx = get_spu_context(ctx);
+
+		d_add(dentry, inode);
+		files++;
+	}
+	return 0;
+out:
+	// FIXME: remove all files that are left
+	return ret;
+}
+
+static int
+spufs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
+{
+	int ret;
+	struct inode *inode;
+	struct spu_context *ctx;
+
+	ret = -ENOSPC;
+	inode = spufs_new_inode(dir->i_sb, mode | S_IFDIR);
+	if (!inode)
+		goto out;
+
+	if (dir->i_mode & S_ISGID) {
+		inode->i_gid = dir->i_gid;
+		inode->i_mode |= S_ISGID;
+	}
+	ctx = alloc_spu_context();
+	SPUFS_I(inode)->i_ctx = ctx;
+	if (!ctx)
+		goto out_iput;
+
+	inode->i_op = &simple_dir_inode_operations;
+	inode->i_fop = &simple_dir_operations;
+	ret = spufs_fill_dir(dentry, spufs_dir_contents, mode, ctx);
+	if (ret)
+		goto out_free_ctx;
+
+	d_instantiate(dentry, inode);
+	dget(dentry);
+	dir->i_nlink++;
+	goto out;
+

fs/spufs/inode.c line 868

Quoted 10 lines
fs/spufs/inode.c +10 -0
+out_free_ctx:
+	put_spu_context(ctx);
+out_iput:
+	iput(inode);
+out:
+	return ret;
+}
+
+/* This looks really wrong! */
+static int spufs_rmdir(struct inode *root, struct dentry *dir_dentry)

Why do you need this? Doesn't 'simple_rmdir' work for you?

The rest of your ramfs based fs code looks a bit complex. Can't it be as "simple" as the debugfs code is (only 100 lines for a fs.) Or is it doing different types of things that I'm completly misunderstanding?

And I still think that 100 lines of code to make a ramfs type fs is a bit big, need to work on that one of these days...

Quoted 4 lines
include/asm-ppc64/spu.h +4 -0
+union MFC_TagSizeClassCmd {
+	struct {
+		u16 mfc_size;
+		u16 mfc_tag;

include/asm-ppc64/spu.h line 76

Quoted 10 lines
include/asm-ppc64/spu.h +10 -0
+		u8  pad;
+		u8  mfc_rclassid;
+		u16 mfc_cmd;
+	} u;
+	struct {
+		u32 mfc_size_tag32;
+		u32 mfc_class_cmd32;
+	} by32;
+	u64 all64;
+};

Remember __u16 and friends for structures that cross the user/kernel boundry (like your ioctl that you will be rewriting...)

thanks,

greg k-h

author maintainer
#19 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

Quoted 6 lines · Greg KH
Quoted 1 line · Arnd Bergmann

/run A stub file that lets us do ioctl.

No, as Ben said, do not do this. Use write. And as you are only doing 1 type of ioctl, it shouldn't be an issue. Also it will be faster than the ioctl due to lack of BKL usage :)

I've been back and forth between a number of interfaces here and haven't found one that I'm really happy with. Using write() is probably my least favorite one, but these are the alternatives I've come up with so far:

1. ioctl:
pro:
- easy to do in a file system
- can have both input and output arguments
contra:
- ugly
- weakly typed
- unpopular

2. sys_spufs_run(int fd, __u32 pc, __u32 *new_pc, __u32 *status):
pro:
- strong types
- can have both input and output arguments
contra:
- does not fit file system semantics well
- bad for prototyping

3. read:
pro:
- fits file system semantics
- can still return a struct { __u32 new_pc; __u32 status; };
contra:
- no way to pass updated instruction pointer directly

4. write:
pro:
- fits file system semantics
- can take instruction pointer as input
contra:
- no output data

The main problem is the way that the ABI requires the main loop to work, which is roughly:

pc = initial_instruction_pointer;

do {
	set_pc(pc);
	status = enter_spu();
	if ((status & 0xff00) == SPU_WANTS_EXIT)
        	return (status & 0xff);
	if ((status & 0xffff) == SPU_LIBRARY_CALL) {
		pc = get_pc();
		do_library_call(*(unsigned int)(local_store_pointer + pc));
		pc += 4;
	}
	if ((status & 0xffff) < SPU_USER_CODE)
		do_user_defined_stuff(status);
} while (!(status & 0xffff0000) & ERROR_MASK));

Currently, I'm doing all this in user space, i.e. the kernel does not need to know about the different status codes that are reserved for exit or library calls.

Having a new system call would keep the basic concept of the ioctl and may or may not be nicer but is certainly harder to debug for now.

One thing I could do instead is have the kernel automatically increment the program counter when the spu requests a library call. This should be ok, because the SPU_LIBRARY_CALL stuff has already been defined to have a very specific operating system independent meaning and as soon as we want to do system calls from the SPU, the kernel needs to know about some status codes anyway.

In that case, I can make the SPU instruction pointer another file in the SPU context directory that only needs to be written once. The operation that starts the SPU code could be an eight byte read system call returning the new instruction pointer (needed to get the library call arguments) and the status code that lets the user determine the required action.

Using a write call instead of read makes the interface even more complicated because it would require the user to read the status from a separate file after write returns to check what needs to be done and then use lseek() or yet another file to access the instruction pointer.

Quoted 2 lines

And I don't quite think you do the proper permission and validate of the data in your code, you should verify this is all correct.

Yes, I'm sure I got that wrong. I'll put that on my todo list.

Quoted 10 lines
Quoted 5 lines

+/**** spufs attributes
+ *

+ * Perhaps these file operations could be put in debugfs or libfs instead,
+ * they are not really SPU specific.

Yes they should. I'll gladly take them for debugfs or like you state, libfs is probably the better place for them so everyone can use them.

If you make up a patch, I'll fix up debugfs to use them properly.

Ok. I'll do the patch for libfs then. I've been thinking about changing

+#define spufs_attribute(name) \
+static int name ## _open(struct inode *inode, struct file *file) \
+{ \
+ return spufs_attr_open(inode, file, &name ## _get, &name ## _set); \
+} \
+static struct file_operations name = { \
+ .open = name ## _open, \
+ .release = spufs_attr_close, \
+ .read = spufs_attr_read, \
+ .write = spufs_attr_write, \
+};

to take a format string argument as well, which is then used in the spufs_attr_read function instead of the hardcoded "%ld\n". Do you think I should do that or rather keep the current implementation?

Quoted 5 lines
Quoted 5 lines
fs/spufs/inode.c +5 -0
+#define spufs_attribute(name)						   \
+static int name ## _open(struct inode *inode, struct file *file)	   \
+{									   \
+	return spufs_attr_open(inode, file, &name ## _get, &name ## _set); \
+}									   \
Quoted 8 lines

fs/spufs/inode.c line 536

Quoted 6 lines
fs/spufs/inode.c +6 -0
+static struct file_operations name = {					   \
+	.open	 = name ## _open,					   \
+	.release = spufs_attr_close,					   \
+	.read	 = spufs_attr_read,					   \
+	.write	 = spufs_attr_write,					   \
+};

No module owner set? Be careful if not...

Right. Is there ever a reason to have file operations without owner? Maybe dentry_open() could warn about this.

Quoted 5 lines

fs/spufs/inode.c line 758

Quoted 2 lines
fs/spufs/inode.c +2 -0
+static struct tree_descr spufs_dir_contents[] = {
+	{ "mem",  &spufs_mem_fops,  0644, },

Named identifiers are the better way to do this (yeah, longer code I know...)

Ok. I took the concept from fs/nfsd/nfsctl.c, thinking that Al knows how to best do these things, but I can of course change this.

Quoted 4 lines

fs/spufs/inode.c line 868

Quoted 2 lines
fs/spufs/inode.c +2 -0
+/* This looks really wrong! */
+static int spufs_rmdir(struct inode *root, struct dentry *dir_dentry)

Why do you need this? Doesn't 'simple_rmdir' work for you?

The idea was to keep the file system contents consistant with the underlying data structures. If I allow users to unlink context directories or files in there, there is no longer a way to extract reliable information from the file system, e.g. for the debugger or for implementing something like spu_ps.

My solution was to force the dentries in each directory to be present. When the directory is created, the files are already there and unlinking a single file is impossible. To destroy the spu context, the user has to rmdir it, which will either remove all files in there as well or fail in the case that any file is still open.

Of course that is not really Posix behavior, but it avoids some other pitfalls.

Quoted 3 lines

The rest of your ramfs based fs code looks a bit complex. Can't it be as "simple" as the debugfs code is (only 100 lines for a fs.) Or is it doing different types of things that I'm completly misunderstanding?

Apart from my special directory semantics, I plan to have a rather unusual way to map the "mem" files: Each spu context can be either present on a physical SPU or stored in memory, so I can create a large number of SPU contexts despite the limitation of physical SPUs present in the machine.

When the SPU context is executing code, it obviously has to be on a physical SPU and a memory map of the "mem" file accesses the actual local store memory that is accessible in the real address space of the kernel. The context save operation copies the local store memory into a virtual file that lives only in page cache, exactly how ramfs deals with its files. Switching between these two states should be possible without breaking user space programs that have the file mapped into their address space.

This mechanism is not implemented yet, but some of my code is already prepared for it.

I also intend to split some parts out from inode.c, probably have a file.c that contains all the file operations and another context.c that deals with the interface to the low level spu code and with abstracting logical spu context from physical spus.

I suppose I should also go over my code to find unnecessary functionality.

Quoted 2 lines

Remember __u16 and friends for structures that cross the user/kernel boundry (like your ioctl that you will be rewriting...)

Yes. There are no data structures that are shared with user space except the current ioctl argument. The MFC_TagSizeClassCmd (yes, I need to remember to change the name some day, currently this still uses the identifiers from the spec) and the others are defined by the hardware interface.

Thanks for all your comments,

	Arnd <><
#20 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

Quoted 6 lines

Using a write call instead of read makes the interface even more complicated because it would require the user to read the status from a separate file after write returns to check what needs to be done and then use lseek() or yet another file to access the instruction pointer.

Why not just write(pc) to start and read back status from the same file ?

Ben.

author maintainer
#21 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

Quoted 2 lines · Benjamin Herrenschmidt

Why not just write(pc) to start and read back status from the same file ?

I suppose you are thinking of the simple_transaction_read() style interface. I've got the feeling that this is generally even less popular than ioctl because

- it is still an untyped interface (as would be a read() based one)
- you can't do 32 bit emulation (doesn't matter for me, we only have 32 bit data)
- it is non-atomic
- it doubles the system call overhead

One operation that I want to allow is to have an infinite loop running on the SPU that does a simple operation (e.g. process one MPEG macroblock) and have that called by multiple unrelated processes in turns. When my operation is not atomic, users need to have additional IPC serialization of their accesses. Most would want that anyway, but it is not a requirement with an interface that needs only a single system call.

For the extra syscall overhead, I would like to see measurements of a real world application before I change to an interface that is slower in theory. Do you have measurements for the time spent in a trivial system call on G5 or Power4?

	Arnd <><
author maintainer
#22 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

Quoted 3 lines · Arnd Bergmann
Quoted 2 lines · Benjamin Herrenschmidt

Why not just write(pc) to start and read back status from the same file ?

I just remembered the strongest reason against using write() to set the instruction pointer: It breaks signal delivery during execution of SPU code. With an ioctl or system call based interface, the kernel simply updates the instruction pointer in process memory before calling a signal handler. When/if the signal handler returns, it does the same call again with the updated argument and the SPU continues to fetch code at the point where it stopped.

If I do a read() based interface, there are no input parameters at all, so restarted system calls work as well.

How about this one:

read() starts execution and returns the status value in a four byte buffer.
Calling lseek() on the "run" file updates the instruction pointer, so the library call can work like this plus error handling:

extern char *mapped_local_store;
uint32_t status;
int runfd = open("run", O_RDONLY);
lseek(runfd, INITIAL_INSTRUCTION, SEEK_SET);

do {
	read(runfd, &status, 4);
	if (status == SPU_DO_LIBRARY_CALL) {
		size_t arg = lseek(runfd, 4, SEEK_CUR) - 4;
		do_library_call(mapped_local_store + arg);
	}
} while (status != SPU_EXIT);
	Arnd <><
#23 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

In reply to Arnd Bergmann
Quoted 21 lines · Arnd Bergmann
Quoted 6 lines · Greg KH
Quoted 1 line · Arnd Bergmann

/run A stub file that lets us do ioctl.

No, as Ben said, do not do this. Use write. And as you are only doing 1 type of ioctl, it shouldn't be an issue. Also it will be faster than the ioctl due to lack of BKL usage :)

I've been back and forth between a number of interfaces here and haven't found one that I'm really happy with. Using write() is probably my least favorite one, but these are the alternatives I've come up with so far:

1. ioctl:
pro:
- easy to do in a file system
- can have both input and output arguments
contra:
- ugly
- weakly typed
- unpopular

Quoted 7 lines

2. sys_spufs_run(int fd, __u32 pc, __u32 *new_pc, __u32 *status):
pro:
- strong types
- can have both input and output arguments
contra:
- does not fit file system semantics well
- bad for prototyping

I suggest you do this. Based on what you say you want the code to do, I agree, write() doesn't really work out well (but it might, and if you want an example of how to do it, look at the ibmasm driver, it implements write() in a way much like what you are wanting to do.)

Quoted 26 lines
Quoted 10 lines
Quoted 5 lines

+/**** spufs attributes
+ *

+ * Perhaps these file operations could be put in debugfs or libfs instead,
+ * they are not really SPU specific.

Yes they should. I'll gladly take them for debugfs or like you state, libfs is probably the better place for them so everyone can use them.

If you make up a patch, I'll fix up debugfs to use them properly.

Ok. I'll do the patch for libfs then. I've been thinking about changing

fs/spufs/inode.c +11 -0
+#define spufs_attribute(name)						   \
+static int name ## _open(struct inode *inode, struct file *file)	   \
+{									   \
+	return spufs_attr_open(inode, file, &name ## _get, &name ## _set); \
+}									   \
+static struct file_operations name = {					   \
+	.open	 = name ## _open,					   \
+	.release = spufs_attr_close,					   \
+	.read	 = spufs_attr_read,					   \
+	.write	 = spufs_attr_write,					   \
+};
Quoted 3 lines

to take a format string argument as well, which is then used in the spufs_attr_read function instead of the hardcoded "%ld\n". Do you think I should do that or rather keep the current implementation?

yeah, you probably need the format string.

Quoted 14 lines
Quoted 5 lines
Quoted 5 lines
fs/spufs/inode.c +5 -0
+#define spufs_attribute(name)						   \
+static int name ## _open(struct inode *inode, struct file *file)	   \
+{									   \
+	return spufs_attr_open(inode, file, &name ## _get, &name ## _set); \
+}									   \
Quoted 8 lines

fs/spufs/inode.c line 536

Quoted 6 lines
fs/spufs/inode.c +6 -0
+static struct file_operations name = {					   \
+	.open	 = name ## _open,					   \
+	.release = spufs_attr_close,					   \
+	.read	 = spufs_attr_read,					   \
+	.write	 = spufs_attr_write,					   \
+};

No module owner set? Be careful if not...

Quoted 1 line

Right. Is there ever a reason to have file operations without owner?

Code built into the kernel?

Quoted 1 line

Maybe dentry_open() could warn about this.

Would die a horrible death due to the above :)

Quoted 11 lines
Quoted 4 lines

fs/spufs/inode.c line 868

Quoted 2 lines
fs/spufs/inode.c +2 -0
+/* This looks really wrong! */
+static int spufs_rmdir(struct inode *root, struct dentry *dir_dentry)

Why do you need this? Doesn't 'simple_rmdir' work for you?

The idea was to keep the file system contents consistant with the underlying data structures. If I allow users to unlink context directories or files in there, there is no longer a way to extract reliable information from the file system, e.g. for the debugger or for implementing something like spu_ps.

Quoted 6 lines

My solution was to force the dentries in each directory to be present. When the directory is created, the files are already there and unlinking a single file is impossible. To destroy the spu context, the user has to rmdir it, which will either remove all files in there as well or fail in the case that any file is still open.

Ick.

Quoted 2 lines

Of course that is not really Posix behavior, but it avoids some other pitfalls.

Go with a syscall :)

Quoted 8 lines
Quoted 2 lines

Remember __u16 and friends for structures that cross the user/kernel boundry (like your ioctl that you will be rewriting...)

Yes. There are no data structures that are shared with user space except the current ioctl argument. The MFC_TagSizeClassCmd (yes, I need to remember to change the name some day, currently this still uses the identifiers from the spec) and the others are defined by the hardware interface.

Identifiers that are named as per a spec are ok to leave alone. We did that with USB, as it makes sense to do it that way for anyone who reads the spec and the code.

But if your spec is only for the Linux OS, well, that's a different issue...

thanks,

greg k-h

author maintainer
#24 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

Quoted 10 lines · Greg KH
Quoted 7 lines · Arnd Bergmann

2. sys_spufs_run(int fd, __u32 pc, __u32 *new_pc, __u32 *status):
pro:
- strong types
- can have both input and output arguments
contra:
- does not fit file system semantics well
- bad for prototyping

Quoted 2 lines

I suggest you do this. Based on what you say you want the code to do, I agree, write() doesn't really work out well

The syscall approach has another small disadvantage in that I need to do a callback registration mechanism for it if I want to have spufs as a loadable module. I could of course require spufs to be builtin, but that complicates prototype testing (as mentioned) and enlarges combined pSeries/powermac/BPA distro kernels.

I think I'll leave the ioctl for now and add a note saying that I need to replace it with a syscall or the write/read or lseek/read based approach when I arrive at a more feature complete point.

Quoted 3 lines

(but it might, and if you
want an example of how to do it, look at the ibmasm driver, it implements write() in a way much like what you are wanting to do.)

That would be the same write/read combination as Ben's second proposal and the nfsctl file system, right?

Quoted 7 lines
Quoted 6 lines

My solution was to force the dentries in each directory to be present. When the directory is created, the files are already there and unlinking a single file is impossible. To destroy the spu context, the user has to rmdir it, which will either remove all files in there as well or fail in the case that any file is still open.

Quoted 6 lines

Ick.

Quoted 2 lines

Of course that is not really Posix behavior, but it avoids some other pitfalls.

Go with a syscall :)

Sorry, I'm not following that reasoning. How does a syscall help with the problem of atomic context destruction?

	Arnd <><
#25 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

Quoted 11 lines · Arnd Bergmann
Quoted 10 lines · Greg KH
Quoted 7 lines · Arnd Bergmann

2. sys_spufs_run(int fd, __u32 pc, __u32 *new_pc, __u32 *status):
pro:
- strong types
- can have both input and output arguments
contra:
- does not fit file system semantics well
- bad for prototyping

Quoted 8 lines
Quoted 2 lines

I suggest you do this. Based on what you say you want the code to do, I agree, write() doesn't really work out well

The syscall approach has another small disadvantage in that I need to do a callback registration mechanism for it if I want to have spufs as a loadable module. I could of course require spufs to be builtin, but that complicates prototype testing (as mentioned) and enlarges combined pSeries/powermac/BPA distro kernels.

Huh? We can handle syscalls in modules these days pretty simply. Look at how nfs and others do it.

Quoted 3 lines

I think I'll leave the ioctl for now and add a note saying that I need to replace it with a syscall or the write/read or lseek/read based approach when I arrive at a more feature complete point.

Nah, make it a syscall :)

Quoted 6 lines
Quoted 3 lines

(but it might, and if you
want an example of how to do it, look at the ibmasm driver, it implements write() in a way much like what you are wanting to do.)

That would be the same write/read combination as Ben's second proposal and the nfsctl file system, right?

Yes.

Quoted 9 lines
Quoted 9 lines
Quoted 6 lines

My solution was to force the dentries in each directory to be present. When the directory is created, the files are already there and unlinking a single file is impossible. To destroy the spu context, the user has to rmdir it, which will either remove all files in there as well or fail in the case that any file is still open.

Ick.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Of course that is not really Posix behavior, but it avoids some other pitfalls.

Go with a syscall :)

Sorry, I'm not following that reasoning. How does a syscall help with the problem of atomic context destruction?

Sorry, I thought they were referring to the same issue.

greg k-h

author maintainer
#26 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

Quoted 2 lines · Greg KH

Huh?  We can handle syscalls in modules these days pretty simply.  Look at how nfs and others do it.

Well afaics, nfs works around this issue by having fs/nfsctl.o always as a builtin and abstract the calls through a file system using read/write. That would be Ben's idea again, i.e. not actually using a system call.

The only widely used module that I'm aware of ever implementing a system call was the TUX web accelerator that that used a hack in entry.S and its own dynamic registration.

	Arnd <><
#27 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

Quoted 4 lines · Arnd Bergmann
Quoted 2 lines · Greg KH

Huh? ?We can handle syscalls in modules these days pretty simply. ?Look at how nfs and others do it.

Quoted 8 lines

Well afaics, nfs works around this issue by having fs/nfsctl.o always as a builtin and abstract the calls through a file system using read/write. That would be Ben's idea again, i.e. not actually using a system call.

The only widely used module that I'm aware of ever implementing a system call was the TUX web accelerator that that used a hack in entry.S and its own dynamic registration.

Sorry, but I was thinking of the cond_syscall() stuff, to allow syscalls in modules or code that just happens to not be built into the kernel.

thanks,

greg k-h

#28 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

In reply to Greg KH
Quoted 4 lines · Greg KH

No, as Ben said, do not do this. Use write. And as you are only doing 1 type of ioctl, it shouldn't be an issue. Also it will be faster than the ioctl due to lack of BKL usage :)

The problem is that if something is wrong regarding 32bit/64bit compatibility (I am not saying Arnd will get it wrong, but for a general rule someone will get it wrong and it has happened, e.g. in ubsfs) then it is impossible to do any compat emulation on read/write.

So I would actually prefer ioctl because it is sfer.

-Andi

#29 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

In reply to Greg KH

Greg> No, as Ben said, do not do this. Use write. And as you are Greg> only doing 1 type of ioctl, it shouldn't be an issue. Also Greg> it will be faster than the ioctl due to lack of BKL usage :)

This is no longer true. ioctls don't have to take the BKL now that struct file_operations has unlocked_ioctl and compat_ioctl.

- R.

#30 lore raw

Re: [PATCH 7/8] ppc64: SPU file system

Quoted 6 lines · Roland Dreier

Greg> No, as Ben said, do not do this. Use write. And as you are Greg> only doing 1 type of ioctl, it shouldn't be an issue. Also Greg> it will be faster than the ioctl due to lack of BKL usage :)

This is no longer true. ioctls don't have to take the BKL now that struct file_operations has unlocked_ioctl and compat_ioctl.

Yes, but his patch did not use them :)

thanks,

greg k-h

author maintainer
#31 lore raw

PATCH 1/1 libfs: add simple attribute files

In reply to Greg KH

Based on the discussion about spufs attributes, this is my suggestion for a more generic attribute file support that can be used by both debugfs and spufs.

Simple attribute files behave similarly to sequential files from a kernel programmers perspective in that a standard set of file operations is provided and only an open operation needs to be written that registers file specific get() and set() functions.

These operations are defined as

void foo_set(void *data, long val); and
long foo_get(void *data);

where data is the inode->u.generic_ip pointer of the file and the operations just need to make send of that pointer. The infrastructure makes sure this works correctly with concurrent access and partial read calls.

A macro named DEFINE_SIMPLE_ATTRIBUTE is provided to further simplify using the attributes.

This patch already contains the changes for debugfs to use attributes for its internal file operations.

---
 fs/debugfs/file.c  |   74 +++++++++++++++++++----------------------
 fs/libfs.c         |   94 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h |   48 +++++++++++++++++++++++++++
 3 files changed, 177 insertions(+), 39 deletions(-)

fs/libfs.c: needs update
Index: linus-2.5/include/linux/fs.h
===================================================================

include/linux/fs.h +49 -0
@@ -1657,6 +1657,55 @@
 	ar->size = n;
 }
 
+/*
+ * simple attribute files
+ *
+ * These attributes behave similar to those in sysfs:
+ *
+ * Writing to an attribute immediately sets a value, an open file can be
+ * written to multiple times.
+ *
+ * Reading from an attribute creates a buffer from the value that might get
+ * read with multiple read calls. When the attribute has been read
+ * completely, no further read calls are possible until the file is opened
+ * again.
+ *
+ * All spufs attributes contain a text representation of a numeric value
+ * that are accessed with the get() and set() functions.
+ *
+ * Perhaps these file operations could be put in debugfs or libfs instead,
+ * they are not really SPU specific.
+ */
+#define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt)		\
+static int __fops ## _open(struct inode *inode, struct file *file)	\
+{									\
+	__simple_attr_check_format(__fmt, 0ul);				\
+	return simple_attr_open(inode, file, &__get, &__set, __fmt);	\
+}									\
+static struct file_operations __fops = {				\
+	.owner	 = THIS_MODULE,						\
+	.open	 = __fops ## _open,					\
+	.release = simple_attr_close,					\
+	.read	 = simple_attr_read,					\
+	.write	 = simple_attr_write,					\
+};
+
+static inline void __attribute__((format(printf, 1, 2)))
+__simple_attr_check_format(const char *fmt, ...)
+{
+	/* don't do anything, just let the compiler check the arguments; */
+}
+
+int simple_attr_open(struct inode *inode, struct file *file,
+		long (*get)(void *), void (*set)(void *, long),
+		const char *fmt);
+int simple_attr_close(struct inode *inode, struct file *file);
+ssize_t simple_attr_read(struct file *file, char __user *buf,
+					size_t len, loff_t *ppos);
+ssize_t simple_attr_write(struct file *file, const char __user *buf,
+					size_t len, loff_t *ppos);
+
+
 #ifdef CONFIG_SECURITY
 static inline char *alloc_secdata(void)
 {
fs/libfs.c +94 -0

100 lines not on this page · raw message

fs/debugfs/file.c +35 -39

98 lines not on this page · raw message

#32 lore raw

Re: [PATCH] libfs: add simple attribute files

Quoted 3 lines · Arnd Bergmann

Based on the discussion about spufs attributes, this is my suggestion for a more generic attribute file support that can be used by both debugfs and spufs.

Thanks for the patch. I've cleaned it up a bit (drop the spufs comments, changed the access check, and made the val be u64, and exported the symbols and cleaned up the debugfs portion) and added it to my tree. It should show up in the next -mm release. I've included the patch below so you can see my
changes.

thanks,

greg k-h

---------------

Based on the discussion about spufs attributes, this is my suggestion for a more generic attribute file support that can be used by both debugfs and spufs.

Simple attribute files behave similarly to sequential files from a kernel programmers perspective in that a standard set of file operations is provided and only an open operation needs to be written that registers file specific get() and set() functions.

These operations are defined as

void foo_set(void *data, long val); and
long foo_get(void *data);

where data is the inode->u.generic_ip pointer of the file and the operations just need to make send of that pointer. The infrastructure makes sure this works correctly with concurrent access and partial read calls.

A macro named DEFINE_SIMPLE_ATTRIBUTE is provided to further simplify using the attributes.

This patch already contains the changes for debugfs to use attributes for its internal file operations.

---
 fs/debugfs/file.c  |   67 +++++++++++++++--------------------
 fs/libfs.c         |   99 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h |   46 ++++++++++++++++++++++++
 3 files changed, 174 insertions(+), 38 deletions(-)
include/linux/fs.h +46 -0
@@ -1657,6 +1657,52 @@
 	ar->size = n;
 }
 
+/*
+ * simple attribute files
+ *
+ * These attributes behave similar to those in sysfs:
+ *
+ * Writing to an attribute immediately sets a value, an open file can be
+ * written to multiple times.
+ *
+ * Reading from an attribute creates a buffer from the value that might get
+ * read with multiple read calls. When the attribute has been read
+ * completely, no further read calls are possible until the file is opened
+ * again.
+ *
+ * All attributes contain a text representation of a numeric value
+ * that are accessed with the get() and set() functions.
+ */
+#define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt)		\
+static int __fops ## _open(struct inode *inode, struct file *file)	\
+{									\
+	__simple_attr_check_format(__fmt, 0ul);				\
+	return simple_attr_open(inode, file, &__get, &__set, __fmt);	\
+}									\
+static struct file_operations __fops = {				\
+	.owner	 = THIS_MODULE,						\
+	.open	 = __fops ## _open,					\
+	.release = simple_attr_close,					\
+	.read	 = simple_attr_read,					\
+	.write	 = simple_attr_write,					\
+};
+
+static inline void __attribute__((format(printf, 1, 2)))
+__simple_attr_check_format(const char *fmt, ...)
+{
+	/* don't do anything, just let the compiler check the arguments; */
+}
+
+int simple_attr_open(struct inode *inode, struct file *file,
+		     u64 (*get)(void *), void (*set)(void *, u64),
+		     const char *fmt);
+int simple_attr_close(struct inode *inode, struct file *file);
+ssize_t simple_attr_read(struct file *file, char __user *buf,
+			 size_t len, loff_t *ppos);
+ssize_t simple_attr_write(struct file *file, const char __user *buf,
+			  size_t len, loff_t *ppos);
+
+
 #ifdef CONFIG_SECURITY
 static inline char *alloc_secdata(void)
 {
fs/libfs.c +99 -0

108 lines not on this page · raw message

fs/debugfs/file.c +29 -38

85 lines not on this page · raw message

author maintainer
#33 lore raw

Re: [PATCH] libfs: add simple attribute files

Quoted 6 lines · Greg KH

Thanks for the patch. I've cleaned it up a bit (drop the spufs comments, changed the access check, and made the val be u64, and exported the symbols and cleaned up the debugfs portion) and added it to my tree. It should show up in the next -mm release. I've included the patch below so you can see my
changes.

Great, thanks for cleaning up those mistakes.

I noticed one small problem with the change from 'long' to 'u64', in that you did not change it in all places. In particular, using "%lu" to print a u64 value will always do the wrong thing on big-endian 32 bit platforms and maybe on some others.
Since 'u64' is '%llu' on most platforms but '%lu' on some 64 bit platforms, I'd either do explicit cast to unsigned long long in the printf or use unsigned long long throughout the code.

Quoted 1 line

void foo_set(void *data, long val); and

                           ^^     u64
Quoted 1 line

long foo_get(void *data);

^^ u64

Review

Quoted 4 lines
(quoted diff) +4 -0
+#define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt)		\
+static int __fops ## _open(struct inode *inode, struct file *file)	\
+{									\
+	__simple_attr_check_format(__fmt, 0ul);				\
                                         ^^^^    0ull

Review

Quoted 3 lines
(quoted diff) +3 -0
+	else	  /* first read */
+		size = scnprintf(attr->get_buf, sizeof(attr->get_buf),
+				 attr->fmt,  attr->get(attr->data));

^^ (unsigned long long)

Review

Quoted 3 lines
(quoted diff) +3 -0
+DEFINE_SIMPLE_ATTRIBUTE(fops_u8, debugfs_u8_get, debugfs_u8_set, "%lu\n");
+DEFINE_SIMPLE_ATTRIBUTE(fops_u16, debugfs_u16_get, debugfs_u16_set, "%lu\n");
+DEFINE_SIMPLE_ATTRIBUTE(fops_u32, debugfs_u32_get, debugfs_u32_set, "%lu\n");
                                                                 %llu   ^^^^ 

I also noticed that it is not possible to pass NULL operations to DEFINE_SIMPLE_ATTRIBUTE() unless you change

include/linux/fs.h +1 -1
@@ -1680,7 +1680,7 @@
 static int __fops ## _open(struct inode *inode, struct file *file)	\
 {									\
 	__simple_attr_check_format(__fmt, 0ul);				\
-	return simple_attr_open(inode, file, &__get, &__set, __fmt);	\
+	return simple_attr_open(inode, file, __get, __set, __fmt);	\
 }									\
 static struct file_operations __fops = {				\
 	.owner	 = THIS_MODULE,						\

I'm currently away from my test machine, so I think it's easier if you just update your patch yourself, but I could also send you an update patch later if you prefer.

	Arnd <><
#34 lore raw

2.4 Kernel threads

Hi all,
i am starting from inside a module a kernel thread*,but in some time later i want to remove that module.
What is the process while unloading a module to release a kernel thread in 2.4.x kernel series.?????

* kernel_thread(module_thread, &startup, CLONE_KERNEL);

Thanks in advance.

#35 lore raw

Re: [PATCH] libfs: add simple attribute files

In reply to Arnd Bergmann
Quoted 55 lines · Arnd Bergmann
Quoted 6 lines · Greg KH

Thanks for the patch. I've cleaned it up a bit (drop the spufs comments, changed the access check, and made the val be u64, and exported the symbols and cleaned up the debugfs portion) and added it to my tree. It should show up in the next -mm release. I've included the patch below so you can see my
changes.

Great, thanks for cleaning up those mistakes.

I noticed one small problem with the change from 'long' to 'u64', in that you did not change it in all places. In particular, using "%lu" to print a u64 value will always do the wrong thing on big-endian 32 bit platforms and maybe on some others.
Since 'u64' is '%llu' on most platforms but '%lu' on some 64 bit platforms, I'd either do explicit cast to unsigned long long in the printf or use unsigned long long throughout the code.

Quoted 1 line

void foo_set(void *data, long val); and

                           ^^     u64
Quoted 1 line

long foo_get(void *data);

^^ u64

Review

Quoted 4 lines
(quoted diff) +4 -0
+#define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt)		\
+static int __fops ## _open(struct inode *inode, struct file *file)	\
+{									\
+	__simple_attr_check_format(__fmt, 0ul);				\
                                         ^^^^    0ull

Review

Quoted 3 lines
(quoted diff) +3 -0
+	else	  /* first read */
+		size = scnprintf(attr->get_buf, sizeof(attr->get_buf),
+				 attr->fmt,  attr->get(attr->data));

^^ (unsigned long long)

Review

Quoted 3 lines
(quoted diff) +3 -0
+DEFINE_SIMPLE_ATTRIBUTE(fops_u8, debugfs_u8_get, debugfs_u8_set, "%lu\n");
+DEFINE_SIMPLE_ATTRIBUTE(fops_u16, debugfs_u16_get, debugfs_u16_set, "%lu\n");
+DEFINE_SIMPLE_ATTRIBUTE(fops_u32, debugfs_u32_get, debugfs_u32_set, "%lu\n");
                                                                 %llu   ^^^^ 

I also noticed that it is not possible to pass NULL operations to DEFINE_SIMPLE_ATTRIBUTE() unless you change

include/linux/fs.h +1 -1
@@ -1680,7 +1680,7 @@
 static int __fops ## _open(struct inode *inode, struct file *file)	\
 {									\
 	__simple_attr_check_format(__fmt, 0ul);				\
-	return simple_attr_open(inode, file, &__get, &__set, __fmt);	\
+	return simple_attr_open(inode, file, __get, __set, __fmt);	\
 }									\
 static struct file_operations __fops = {				\
 	.owner	 = THIS_MODULE,						\
Quoted 3 lines

I'm currently away from my test machine, so I think it's easier if you just update your patch yourself, but I could also send you an update patch later if you prefer.

Thanks for the updates, I've made them by hand to the patch, and will show up in the next -mm release.

thanks again,

greg k-h

author maintainer
#36 lore raw

PATCH 8/8 ppc64: add spufs user library

In reply to Arnd Bergmann

This adds a user space library as a counterpart to the kernel side spufs. Since the hardware is not available yet, this is mostly for documenting the spufs API and is not intended for merging into mainline.

As the API matures, libspu will become a separate package.

From: Dirk Herrendörfer <[email protected]>

Documentation/bpa/libspu/Makefile +41 -0
@@ -0,0 +1,41 @@
+#*
+#* libbpathread - A wrapper library to adapt the JSRE SPU usage model to SPUFS 
+#* Copyright (C) 2005 IBM Corp. 
+#*
+#* This library is free software; you can redistribute it and/or modify it
+#* under the terms of the GNU Lesser General Public License as published by 
+#* the Free Software Foundation; either version 2.1 of the License, 
+#* or (at your option) any later version.
+#*
+#*  This library is distributed in the hope that it will be useful, but 
+#*  WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY 
+#*  or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public 
+#*  License for more details.
+#*
+#*   You should have received a copy of the GNU Lesser General Public License 
+#*   along with this library; if not, write to the Free Software Foundation, 
+#*   Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+#*
+
+CC := gcc
+CTAGS = ctags
+
+CFLAGS := -O2 -m32 -Wall -I. -Iinclude -DDEBUG -g \
+
+libspu_OBJS := elf_loader.o bpathread.o
+OBJS := libbpathread.a $(libspu_OBJS)
+
+all: $(OBJS)
+
+libbpathread.a: $(libspu_OBJS)
+	ar -r $@ $(libspu_OBJS)
+
+tests: 
+	make -C test/start-stop
+
+tags:
+	$(CTAGS) -R .
+
+clean: 
+	rm -f $(OBJS) *~ tags
+	make -C test/start-stop clean
Documentation/bpa/libspu/README +7 -0
@@ -0,0 +1,7 @@
+This is an example of how to use the SPEs in applications. It is by
+no means complete or perfect - it is meant as a reference of how to
+use the SPUFS implementation in linux.
+As more and more features of SPUFS become available, this code will
+be extended too.
+
+D.Herrendoerfer <[email protected]>
Documentation/bpa/libspu/bpathread.c +314 -0

314 lines not on this page · raw message

Documentation/bpa/libspu/bpathread.h +47 -0

47 lines not on this page · raw message

Documentation/bpa/libspu/elf_loader.c +130 -0

130 lines not on this page · raw message

Documentation/bpa/libspu/elf_loader.h +40 -0
@@ -0,0 +1,40 @@
+/*
+ * libbpathread - A wrapper library to adapt the JSRE SPU usage model to SPUFS
+ * Copyright (C) 2005 IBM Corp.
+ *
+ * This library is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License,
+ * or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ *  or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public
+ *  License for more details.
+ *
+ *   You should have received a copy of the GNU Lesser General Public License
+ *   along with this library; if not, write to the Free Software Foundation,
+ *   Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#define LS_SIZE                       0x40000	/* 256K (in bytes) */
+
+#define SPE_LDR_PROG_start    (LS_SIZE - 512)	// location of spu_ld.so prog
+#define SPE_LDR_PARAMS_start  (LS_SIZE - 128)	// location of spu_ldr_params
+
+typedef union
+{
+	unsigned long long ull;
+	unsigned int ui[2];
+} addr64;
+
+struct spe_ld_info
+{
+	unsigned int entry;	/* Entry point of SPU image                 */
+};
+
+/*
+ * Global API : */
+
+int load_spe_elf (void *elf_start, void *ld_buffer,
+		  struct spe_ld_info *ld_info);
Documentation/bpa/libspu/spe_exec.h +43 -0

43 lines not on this page · raw message

Documentation/bpa/libspu/test/start-stop/Makefile +43 -0

43 lines not on this page · raw message

Documentation/bpa/libspu/test/start-stop/ppe-start-stop.c +89 -0

89 lines not on this page · raw message

Documentation/bpa/libspu/test/start-stop/spe-start-stop.c +23 -0
@@ -0,0 +1,23 @@
+/*
+ * libbpathread - A wrapper library to adapt the JSRE SPU usage model to SPUFS
+ * Copyright (C) 2005 IBM Corp.
+ *
+ * This library is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License,
+ * or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ *  or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public
+ *  License for more details.
+ *
+ *   You should have received a copy of the GNU Lesser General Public License
+ *   along with this library; if not, write to the Free Software Foundation,
+ *   Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+int main(void)
+{
+	return 0;
+}
Documentation/bpa/libspu/tools/README +8 -0
@@ -0,0 +1,8 @@
+Contents of this Directory
+
+elfspe-register:
+Script to register a SPE-ELF loading app with binfmt_misc.
+
+embedspu:
+Script to attach a SPE-ELF object to an executable.
+
Documentation/bpa/libspu/tools/elfspe-register +4 -0
@@ -0,0 +1,4 @@
+#!/bin/sh
+
+echo ':spu:M::\x7fELF\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x17::/home/uweigand/runspu:' > /proc/sys/fs/binfmt_misc/register
+
Documentation/bpa/libspu/tools/embedspu +56 -0

56 lines not on this page · raw message