Bao, Zheng | 9 Apr 05:24 2012
Picon

Re: Patch merged into coreboot/master: c35c461 Invalidate cache before first jump

Hi,
It is unstable. In most cases, it hangs at this wbinvd. Once it pass that instruction, it will hang at when AP
cores are launched.

Joe

> -----Original Message-----
> From: Marc Jones [mailto:marcj303 <at> gmail.com]
> Sent: Friday, April 06, 2012 11:27 PM
> To: Bao, Zheng
> Cc: coreboot <at> coreboot.org; Marc Jones
> Subject: Re: [coreboot] Patch merged into coreboot/master: c35c461
> Invalidate cache before first jump
> 
> Can you be more descriptive to how it fails? Does it hang on that
> instruction?
> 
> Marc
> 
> 
> On Fri, Apr 6, 2012 at 5:38 AM, Bao, Zheng <Zheng.Bao <at> amd.com> wrote:
> > Actually, it hurts my board.
> > AMD trinity fam15 + Hudson.
> >
> > Zheng
> >
> >> -----Original Message-----
> >> From: coreboot-bounces <at> coreboot.org [mailto:coreboot-
> >> bounces <at> coreboot.org] On Behalf Of gerrit <at> coreboot.org
> >> Sent: Friday, April 06, 2012 5:03 AM
(Continue reading)

coreboot tracker | 9 Apr 16:00 2012

Trac reminder: list of new ticket(s)

Ticket Owner Status Description
#186 stepan <at> coresystems.de new 3com 3c905tx / gpxe boot problem
#185 stepan <at> coresystems.de new Vtech partial success
#184 stepan <at> coresystems.de new Asus p2b with aty128 fails
#183 stepan <at> coresystems.de new SeaBIOS: test-gcc.sh executed multiple times, also during make clean
#182 uwe <at> hermann-uwe.de new superiotool installs man page with +x perms
#181 stepan <at> coresystems.de new Tyan S2885 (and other K8 boards) won't boot with TINY_BOOTBLOCK
#180 stepan <at> coresystems.de new ASRock E350M1 Gigabit Ethernet Problem
#178 stepan <at> coresystems.de new linux kernel hang while boot from SATA SSD on EPIA CN
#176 stepan <at> coresystems.de new inteltool: added PCI_DEVICE_ID_INTEL_X44 0x29e0
#174 stepan <at> coresystems.de new Unable to boot from qemu-kvm -- seems to be a cbfs problem
#168 stepan <at> coresystems.de new USBDEBUG might slow down coreboot
#162 oxygene new Move SYSTEM_TYPE to Kconfig
#160 oxygene new Build system: There's no convincing CFLAGS management for util/*
#158 ward <at> gnu.org new buildrom svn error
#156 hailfinger new Add Layout File capability to v3 and LAR tool
#154 hailfinger new Flashing BIOSes from Fujitsu/Siemens is not supported
#150 somebody new AMD DB800 dev board PLL strapping leaves CPU and GLIU in non-optimal clock
#147 somebody new Linux kernel halts when scanning the PCI bus below 0:14.4 on RS690
#145 somebody new Fix CMOS handling
#143 oxygene new unify intel car for model_6[ef]x
#135 ward new Flashrom deletes MAC addresses on Tyan Tomcat n3400B (S2925-E)
#129 stepan new etherboot payload does not work with HIGH_TABLES
#128 somebody new Improve email user interface for trac
#125 somebody new BCM5785 / HT1000 reset functions
#111 somebody new Add i18n support for translating strings e.g. for bayou / coreinfo
#110 somebody new Allow for per-device subsystem IDs
#77 somebody new hang on the "Jumping to coreboot" step on via epia-m with 4-chip 128Mbyte DDR module
#76 rminnich new coreboot messages should be accessible in dmesg
#18 oxygene new autoprobe apic cluster and application processors on K8 systems
#17 stepan new clean up coreboot table handling
#16 ollie new I2C driver and mainboard Config.lb
#11 yhlu new pirq table automation
#5 uwe new Add license header to all source files
#2 somebody new Complete tables of supported motherboards
--

-- 
coreboot mailing list: coreboot <at> coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot
Ronald G. Minnich | 9 Apr 19:32 2012

Patch set updated for coreboot: 7ebde20 Add multicore support to coreboot

Ronald G. Minnich (rminnich <at> gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/877

-gerrit

commit 7ebde200d7acc1409b952b738c41be28ddf9a4e7
Author: Ron Minnich <rminnich <at> gmail.com>
Date:   Thu Apr 5 23:51:18 2012 -0700

    Add multicore support to coreboot

    This set of changes supports the ability for multicore support in coreboot.
    To give people a chance to examine the structure, and to make bisecting
    for bugs easier, we are implementing the patch in 3 stages. This first stage
    introduces the basic mechanism but does not make any visible change in
    behavior. The APs, instead of being immediately spun down, are asked to
    run one debug print function and then spun down.

    The means by which APs are tasked was implemented in the NIX operating
    system in 2011, see: http://code.google.com/p/nix-os/
    The APs come alive and spin on a memory location, contained
    in a per-AP structure. Tasking is accomplished
    by setting parameters into an argument array and then writing a function
    pointer into the memory location. The AP indicates completion by writing
    zero to the memory location. The BSP can wait for the AP to finish
    (synchronous) or going off to do other work (asynchronous). Wait times
    are in units of microseconds.

    This way of tasking APs is incredibly cheap and fast: in the minimal
    case, one memory write suffices to launch an AP into doing work.

    This code has been tested on a sandybridge system (chromebook)
    and on another 4-core sandybridge system.

    Take the opportunity to remove the macro definition whose origins
    few people remember.
    Change-Id: I19f8587562fd499e98457aa9f11b52400c105697
    Signed-off-by: Ron Minnich <rminnich <at> gmail.com>
---
 src/arch/x86/include/arch/cpu.h    |    5 ++
 src/arch/x86/lib/cpu.c             |   30 +++++++--
 src/cpu/x86/lapic/lapic_cpu_init.c |  132 +++++++++++++++++++++++++++++++++---
 src/include/cpu/cpu.h              |   10 ++-
 4 files changed, 160 insertions(+), 17 deletions(-)

diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h
index 604abde..34d0856 100644
--- a/src/arch/x86/include/arch/cpu.h
+++ b/src/arch/x86/include/arch/cpu.h
 <at>  <at>  -157,9 +157,14  <at>  <at>  struct cpu_driver {
 struct device;
 struct cpu_driver *find_cpu_driver(struct device *cpu);

+typedef u32 (*workfunc)(u32, u32, u32);
+
 struct cpu_info {
 	device_t cpu;
 	unsigned long index;
+	workfunc work;
+	u32 params[3];
+	u32 result;
 };

 static inline struct cpu_info *cpu_info(void)
diff --git a/src/arch/x86/lib/cpu.c b/src/arch/x86/lib/cpu.c
index 98ede06..ad028e3 100644
--- a/src/arch/x86/lib/cpu.c
+++ b/src/arch/x86/lib/cpu.c
 <at>  <at>  -234,7 +234,7  <at>  <at>  static void set_cpu_ops(struct device *cpu)
 	cpu->ops = driver ? driver->ops : NULL;
 }

-void cpu_initialize(void)
+void cpu_initialize(struct cpu_info *info)
 {
 	/* Because we busy wait at the printk spinlock.
 	 * It is important to keep the number of printed messages
 <at>  <at>  -242,12 +242,9  <at>  <at>  void cpu_initialize(void)
 	 * disabled.
 	 */
 	struct device *cpu;
-	struct cpu_info *info;
 	struct cpuinfo_x86 c;

-	info = cpu_info();
-
-	printk(BIOS_INFO, "Initializing CPU #%ld\n", info->index);
+	printk(BIOS_INFO, "cpu_initialize: CPU #%ld\n", info->index);

 	cpu = info->cpu;
 	if (!cpu) {
 <at>  <at>  -289,3 +286,26  <at>  <at>  void cpu_initialize(void)
 	return;
 }

+void cpu_work(struct cpu_info *info)
+{
+	workfunc f;
+	volatile workfunc *ptr = &info->work;
+	volatile u32 *params = info->params;
+
+	printk(BIOS_INFO, "CPU #%ld ready to work\n", info->index);
+
+	while (!*ptr)
+		;
+	f = *ptr;
+
+	printk(BIOS_SPEW, "CPU #%ld is asked to do %p\n", info->index, f);
+	f(params[0], params[1], params[2]);
+
+	printk(BIOS_SPEW, "CPU #%ld finishes %p, mark %p\n", info->index, f, ptr);
+	*ptr = 0;
+
+	printk(BIOS_INFO, "CPU #%ld leaving cpu_work()\n", info->index);
+
+	return;
+}
+
diff --git a/src/cpu/x86/lapic/lapic_cpu_init.c b/src/cpu/x86/lapic/lapic_cpu_init.c
index 2ac9093..aca429a 100644
--- a/src/cpu/x86/lapic/lapic_cpu_init.c
+++ b/src/cpu/x86/lapic/lapic_cpu_init.c
 <at>  <at>  -17,6 +17,21  <at>  <at> 
 #include <cpu/intel/speedstep.h>

 #if CONFIG_SMP == 1
+
+/* we do not want this struct visible outside this file.
+ * The external interface is via functions.
+ */
+
+struct apcore {
+	u32 stack[1024 - sizeof(struct cpu_info)];
+	struct cpu_info info;
+};
+
+#define TOS(x) (&apcores[(x)].stack[ARRAY_SIZE(apcores[x].stack)-1])
+typedef struct apcore apcore_t;
+
+apcore_t apcores[CONFIG_MAX_CPUS];
+
 /* This is a lot more paranoid now, since Linux can NOT handle
  * being told there is a CPU when none exists. So any errors
  * will return 0, meaning no CPU.
 <at>  <at>  -227,7 +242,6  <at>  <at>  volatile unsigned long secondary_stack;

 int start_cpu(device_t cpu)
 {
-	extern unsigned char _estack[];
 	struct cpu_info *info;
 	unsigned long stack_end;
 	unsigned long apicid;
 <at>  <at>  -241,19 +255,25  <at>  <at>  int start_cpu(device_t cpu)
 	apicid = cpu->path.apic.apic_id;

 	/* Get an index for the new processor */
+	if (last_cpu_index >= CONFIG_MAX_CPUS){
+		printk(BIOS_ERR, "CONFIG_MAX_CPUS(%d) too small!\n", CONFIG_MAX_CPUS);
+		return 0;
+	}
+
 	index = ++last_cpu_index;

 	/* Find end of the new processors stack */
-	stack_end = ((unsigned long)_estack) - (CONFIG_STACK_SIZE*index) - sizeof(struct cpu_info);
+	stack_end = (unsigned long) TOS(index);

 	/* Record the index and which cpu structure we are using */
-	info = (struct cpu_info *)stack_end;
+	info = &apcores[index].info;
 	info->index = index;
 	info->cpu   = cpu;

 	/* Advertise the new stack to start_cpu */
 	secondary_stack = stack_end;
-
+	printk(BIOS_SPEW, "start_cpu CPU %ld secondary_stack %#lx info %p\n",
+		index, secondary_stack, info);
 	/* Until the cpu starts up report the cpu is not enabled */
 	cpu->enabled = 0;
 	cpu->initialized = 0;
 <at>  <at>  -381,8 +401,11  <at>  <at>  static __inline__ __attribute__((always_inline)) void writecr4(unsigned long Dat
 #endif

 /* C entry point of secondary cpus */
-void secondary_cpu_init(void)
+void secondary_cpu_init(u32 infoptr)
 {
+	/* note: we tried (((u32 *)&infoptr)[1]) but that failed. Compiler? */
+	struct cpu_info *info = (struct cpu_info *)(((u8*)&infoptr)+sizeof(u32));
+
 	atomic_inc(&active_cpus);
 #if CONFIG_SERIAL_CPU_INIT == 1
 	spin_lock(&start_cpu_lock);
 <at>  <at>  -398,11 +421,11  <at>  <at>  void secondary_cpu_init(void)
 	cr4_val |= (1 << 9 | 1 << 10);
 	writecr4(cr4_val);
 #endif
-	cpu_initialize();
+	cpu_initialize(info);
 #if CONFIG_SERIAL_CPU_INIT == 1
 	spin_unlock(&start_cpu_lock);
 #endif
-
+	cpu_work(info);
 	atomic_dec(&active_cpus);

 	stop_this_cpu();
 <at>  <at>  -476,8 +499,6  <at>  <at>  static void wait_other_cpus_stop(struct bus *cpu_bus)
 	printk(BIOS_DEBUG, "All AP CPUs stopped (%ld loops)\n", loopcount);
 }

-#else /* CONFIG_SMP */
-#define initialize_other_cpus(root) do {} while(0)
 #endif /* CONFIG_SMP */

 void initialize_cpus(struct bus *cpu_bus)
 <at>  <at>  -505,6 +526,7  <at>  <at>  void initialize_cpus(struct bus *cpu_bus)
 	info->cpu = alloc_find_dev(cpu_bus, &cpu_path);

 #if CONFIG_SMP == 1
+	memset(apcores, 0, sizeof(*apcores));
 	copy_secondary_start_to_1m_below(); // why here? In case some day we can start core1 in amd_sibling_init
 #endif

 <at>  <at>  -522,7 +544,7  <at>  <at>  void initialize_cpus(struct bus *cpu_bus)
 #endif

 	/* Initialize the bootstrap processor */
-	cpu_initialize();
+	cpu_initialize(info);

 #if CONFIG_SMP == 1
 	#if CONFIG_SERIAL_CPU_INIT == 1
 <at>  <at>  -534,3 +556,93  <at>  <at>  void initialize_cpus(struct bus *cpu_bus)
 #endif
 }

+#if CONFIG_SMP == 1
+/* work primitives */
+
+/* start_work is only intended to be called by the bsp. */
+/* A built in assumption of this code is that you know what 
+ * you're doing. This is firmware, not pthreads. You should 
+ * not call this function for a core if:
+ * - the core does not exist
+ * - the core is not initialized
+ * - the core is busy
+ * Any of these return a -1, else the core is started and
+ * 0 is returned. 
+ */
+int start_work(unsigned int core, workfunc f, u32 a, u32 b, u32 c)
+{
+	struct cpu_info *info;
+	volatile workfunc *ptr;
+	volatile u32 *params;
+
+	if (core >= CONFIG_MAX_CPUS){
+		printk(BIOS_EMERG, "start_work: invalid core %d\n", core);
+		return -1;
+	}
+	info = &apcores[core].info;
+	if (! info->cpu->initialized){
+		printk(BIOS_EMERG, "start_work: core not initialized %d\n", core);
+		return -1;
+	}
+	ptr = &info->work;
+	if (*ptr){
+		printk(BIOS_EMERG, "start_work: core is busy %d\n", core);
+		return -1;
+	}
+	params = (u32 *)&info->params;
+	params[0] = a;
+	params[1] = b;
+	params[2] = c;
+	printk(BIOS_INFO, "BSP starts work on core %d\n", core);
+	*ptr = f;
+	return 0;
+}
+
+/* wait for the work to finish and return the result. Wait at
+ * most maxusec microseconds. 
+ */
+int wait_work(unsigned int core, u32 *retval, unsigned int maxusec)
+{
+	unsigned int usec;
+	struct cpu_info *info;
+	volatile workfunc *ptr;
+	volatile u32 *result;
+
+	if (core >= CONFIG_MAX_CPUS){
+		printk(BIOS_EMERG, "start_work: invalid core %d\n", core);
+		return -1;
+	}
+	info = &apcores[core].info;
+	if (! info->cpu->initialized){
+		printk(BIOS_EMERG, "start_work: core not initialized %d\n", core);
+		return -1;
+	}
+	ptr = &info->work;
+	for(usec = 0; usec < maxusec && *ptr; usec++)
+		udelay(1);
+
+	/* N.B. since only the BSP starts cores, there is not 
+	 * problem checking the pointer since access to it is 
+	 * serialized by the single-threaded BSP code. 
+	 */
+	if (*ptr){
+		printk(BIOS_INFO, "core %d still running after %d microseconds\n",
+			core, usec);
+		return -1;
+	}
+	result = &info->result;
+	printk(BIOS_INFO, "info->work after result is %#lx\n", (unsigned long)*result);
+	if (retval)
+		*retval = *result;
+	return 0;
+}
+
+/* start the work and wait for it to finish */
+int run_work(unsigned int core, workfunc f, u32 a, u32 b, u32 c, u32 *retval,
+	unsigned int timeout)
+{
+	if (start_work(core, f, a, b, c))
+		return -1;
+	return wait_work(core, retval, timeout);
+}
+#endif /* CONFIG_SMP == 1 */
diff --git a/src/include/cpu/cpu.h b/src/include/cpu/cpu.h
index cca2be1..0b7a748 100644
--- a/src/include/cpu/cpu.h
+++ b/src/include/cpu/cpu.h
 <at>  <at>  -5,9 +5,15  <at>  <at>  struct device;
 struct bus;
 #include <arch/cpu.h>

-void cpu_initialize(void);
+void cpu_initialize(struct cpu_info *info);
 void initialize_cpus(struct bus *cpu_bus);
-void secondary_cpu_init(void);
+void secondary_cpu_init(u32 unused);
+int start_work(unsigned int core, workfunc f, u32 a, u32 b, u32 c);
+int wait_work(unsigned int core, u32 *retval, unsigned int maxusec);
+int run_work(unsigned int core, workfunc f, u32 a, u32 b, u32 c, u32 *retval,
+	unsigned int timeout);
+void cpu_work(struct cpu_info *info);
+

 #if !CONFIG_WAIT_BEFORE_CPUS_INIT
 	#define cpus_ready_for_init() do {} while(0)

--

-- 
coreboot mailing list: coreboot <at> coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Ronald G. Minnich | 9 Apr 20:21 2012

Patch set updated for coreboot: 83d27ab Add multicore support to coreboot

Ronald G. Minnich (rminnich <at> gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/877

-gerrit

commit 83d27ab81445e3f9cc88ed77ec52800c5c7b4321
Author: Ron Minnich <rminnich <at> gmail.com>
Date:   Thu Apr 5 23:51:18 2012 -0700

    Add multicore support to coreboot

    This set of changes supports the ability for multicore support in coreboot.
    To give people a chance to examine the structure, and to make bisecting
    for bugs easier, we are implementing the patch in 3 stages. This first stage
    introduces the basic mechanism but does not make any visible change in
    behavior. The APs, instead of being immediately spun down, are asked to
    run one debug print function and then spun down.

    The means by which APs are tasked was implemented in the NIX operating
    system in 2011, see: http://code.google.com/p/nix-os/
    The APs come alive and spin on a memory location, contained
    in a per-AP structure. Tasking is accomplished
    by setting parameters into an argument array and then writing a function
    pointer into the memory location. The AP indicates completion by writing
    zero to the memory location. The BSP can wait for the AP to finish
    (synchronous) or going off to do other work (asynchronous). Wait times
    are in units of microseconds.

    This way of tasking APs is incredibly cheap and fast: in the minimal
    case, one memory write suffices to launch an AP into doing work.

    This code has been tested on a sandybridge system (chromebook)
    and on another 4-core sandybridge system.

    Take the opportunity to remove the macro definition whose origins
    few people remember.
    Change-Id: I19f8587562fd499e98457aa9f11b52400c105697
    Signed-off-by: Ron Minnich <rminnich <at> gmail.com>
---
 src/arch/x86/include/arch/cpu.h    |    5 ++
 src/arch/x86/lib/cpu.c             |   30 +++++++--
 src/cpu/x86/lapic/lapic_cpu_init.c |  137 +++++++++++++++++++++++++++++++++---
 src/include/cpu/cpu.h              |   10 ++-
 4 files changed, 165 insertions(+), 17 deletions(-)

diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h
index 604abde..34d0856 100644
--- a/src/arch/x86/include/arch/cpu.h
+++ b/src/arch/x86/include/arch/cpu.h
 <at>  <at>  -157,9 +157,14  <at>  <at>  struct cpu_driver {
 struct device;
 struct cpu_driver *find_cpu_driver(struct device *cpu);

+typedef u32 (*workfunc)(u32, u32, u32);
+
 struct cpu_info {
 	device_t cpu;
 	unsigned long index;
+	workfunc work;
+	u32 params[3];
+	u32 result;
 };

 static inline struct cpu_info *cpu_info(void)
diff --git a/src/arch/x86/lib/cpu.c b/src/arch/x86/lib/cpu.c
index 98ede06..ad028e3 100644
--- a/src/arch/x86/lib/cpu.c
+++ b/src/arch/x86/lib/cpu.c
 <at>  <at>  -234,7 +234,7  <at>  <at>  static void set_cpu_ops(struct device *cpu)
 	cpu->ops = driver ? driver->ops : NULL;
 }

-void cpu_initialize(void)
+void cpu_initialize(struct cpu_info *info)
 {
 	/* Because we busy wait at the printk spinlock.
 	 * It is important to keep the number of printed messages
 <at>  <at>  -242,12 +242,9  <at>  <at>  void cpu_initialize(void)
 	 * disabled.
 	 */
 	struct device *cpu;
-	struct cpu_info *info;
 	struct cpuinfo_x86 c;

-	info = cpu_info();
-
-	printk(BIOS_INFO, "Initializing CPU #%ld\n", info->index);
+	printk(BIOS_INFO, "cpu_initialize: CPU #%ld\n", info->index);

 	cpu = info->cpu;
 	if (!cpu) {
 <at>  <at>  -289,3 +286,26  <at>  <at>  void cpu_initialize(void)
 	return;
 }

+void cpu_work(struct cpu_info *info)
+{
+	workfunc f;
+	volatile workfunc *ptr = &info->work;
+	volatile u32 *params = info->params;
+
+	printk(BIOS_INFO, "CPU #%ld ready to work\n", info->index);
+
+	while (!*ptr)
+		;
+	f = *ptr;
+
+	printk(BIOS_SPEW, "CPU #%ld is asked to do %p\n", info->index, f);
+	f(params[0], params[1], params[2]);
+
+	printk(BIOS_SPEW, "CPU #%ld finishes %p, mark %p\n", info->index, f, ptr);
+	*ptr = 0;
+
+	printk(BIOS_INFO, "CPU #%ld leaving cpu_work()\n", info->index);
+
+	return;
+}
+
diff --git a/src/cpu/x86/lapic/lapic_cpu_init.c b/src/cpu/x86/lapic/lapic_cpu_init.c
index 2ac9093..f54f07c 100644
--- a/src/cpu/x86/lapic/lapic_cpu_init.c
+++ b/src/cpu/x86/lapic/lapic_cpu_init.c
 <at>  <at>  -17,6 +17,21  <at>  <at> 
 #include <cpu/intel/speedstep.h>

 #if CONFIG_SMP == 1
+
+/* we do not want this struct visible outside this file.
+ * The external interface is via functions.
+ */
+
+struct apcore {
+	u32 stack[1024 - sizeof(struct cpu_info)];
+	struct cpu_info info;
+};
+
+#define TOS(x) (&apcores[(x)].stack[ARRAY_SIZE(apcores[x].stack)-1])
+typedef struct apcore apcore_t;
+
+apcore_t apcores[CONFIG_MAX_CPUS];
+
 /* This is a lot more paranoid now, since Linux can NOT handle
  * being told there is a CPU when none exists. So any errors
  * will return 0, meaning no CPU.
 <at>  <at>  -221,13 +236,16  <at>  <at>  static atomic_t active_cpus = ATOMIC_INIT(1);
  * start_cpu returns.
  */

+/* N.B. if we move to serial smp init we don't need this spin lock. 
+ * Consider for the future. 
+ */
+
 static spinlock_t start_cpu_lock = SPIN_LOCK_UNLOCKED;
 static unsigned last_cpu_index = 0;
 volatile unsigned long secondary_stack;

 int start_cpu(device_t cpu)
 {
-	extern unsigned char _estack[];
 	struct cpu_info *info;
 	unsigned long stack_end;
 	unsigned long apicid;
 <at>  <at>  -241,19 +259,26  <at>  <at>  int start_cpu(device_t cpu)
 	apicid = cpu->path.apic.apic_id;

 	/* Get an index for the new processor */
+	if ((last_cpu_index+1) >= CONFIG_MAX_CPUS){
+		printk(BIOS_ERR, "CONFIG_MAX_CPUS(%d) too small!\n", CONFIG_MAX_CPUS);
+		spin_unlock(&start_cpu_lock);
+		return 0;
+	}
+
 	index = ++last_cpu_index;

 	/* Find end of the new processors stack */
-	stack_end = ((unsigned long)_estack) - (CONFIG_STACK_SIZE*index) - sizeof(struct cpu_info);
+	stack_end = (unsigned long) TOS(index);

 	/* Record the index and which cpu structure we are using */
-	info = (struct cpu_info *)stack_end;
+	info = &apcores[index].info;
 	info->index = index;
 	info->cpu   = cpu;

 	/* Advertise the new stack to start_cpu */
 	secondary_stack = stack_end;
-
+	printk(BIOS_SPEW, "start_cpu CPU %ld secondary_stack %#lx info %p\n",
+		index, secondary_stack, info);
 	/* Until the cpu starts up report the cpu is not enabled */
 	cpu->enabled = 0;
 	cpu->initialized = 0;
 <at>  <at>  -381,8 +406,11  <at>  <at>  static __inline__ __attribute__((always_inline)) void writecr4(unsigned long Dat
 #endif

 /* C entry point of secondary cpus */
-void secondary_cpu_init(void)
+void secondary_cpu_init(u32 infoptr)
 {
+	/* note: we tried (((u32 *)&infoptr)[1]) but that failed. Compiler? */
+	struct cpu_info *info = (struct cpu_info *)(((u8*)&infoptr)+sizeof(u32));
+
 	atomic_inc(&active_cpus);
 #if CONFIG_SERIAL_CPU_INIT == 1
 	spin_lock(&start_cpu_lock);
 <at>  <at>  -398,11 +426,11  <at>  <at>  void secondary_cpu_init(void)
 	cr4_val |= (1 << 9 | 1 << 10);
 	writecr4(cr4_val);
 #endif
-	cpu_initialize();
+	cpu_initialize(info);
 #if CONFIG_SERIAL_CPU_INIT == 1
 	spin_unlock(&start_cpu_lock);
 #endif
-
+	cpu_work(info);
 	atomic_dec(&active_cpus);

 	stop_this_cpu();
 <at>  <at>  -476,8 +504,6  <at>  <at>  static void wait_other_cpus_stop(struct bus *cpu_bus)
 	printk(BIOS_DEBUG, "All AP CPUs stopped (%ld loops)\n", loopcount);
 }

-#else /* CONFIG_SMP */
-#define initialize_other_cpus(root) do {} while(0)
 #endif /* CONFIG_SMP */

 void initialize_cpus(struct bus *cpu_bus)
 <at>  <at>  -505,6 +531,7  <at>  <at>  void initialize_cpus(struct bus *cpu_bus)
 	info->cpu = alloc_find_dev(cpu_bus, &cpu_path);

 #if CONFIG_SMP == 1
+	memset(apcores, 0, sizeof(*apcores));
 	copy_secondary_start_to_1m_below(); // why here? In case some day we can start core1 in amd_sibling_init
 #endif

 <at>  <at>  -522,7 +549,7  <at>  <at>  void initialize_cpus(struct bus *cpu_bus)
 #endif

 	/* Initialize the bootstrap processor */
-	cpu_initialize();
+	cpu_initialize(info);

 #if CONFIG_SMP == 1
 	#if CONFIG_SERIAL_CPU_INIT == 1
 <at>  <at>  -534,3 +561,93  <at>  <at>  void initialize_cpus(struct bus *cpu_bus)
 #endif
 }

+#if CONFIG_SMP == 1
+/* work primitives */
+
+/* start_work is only intended to be called by the bsp. */
+/* A built in assumption of this code is that you know what 
+ * you're doing. This is firmware, not pthreads. You should 
+ * not call this function for a core if:
+ * - the core does not exist
+ * - the core is not initialized
+ * - the core is busy
+ * Any of these return a -1, else the core is started and
+ * 0 is returned. 
+ */
+int start_work(unsigned int core, workfunc f, u32 a, u32 b, u32 c)
+{
+	struct cpu_info *info;
+	volatile workfunc *ptr;
+	volatile u32 *params;
+
+	if (core >= CONFIG_MAX_CPUS){
+		printk(BIOS_EMERG, "start_work: invalid core %d\n", core);
+		return -1;
+	}
+	info = &apcores[core].info;
+	if (! info->cpu->initialized){
+		printk(BIOS_EMERG, "start_work: core not initialized %d\n", core);
+		return -1;
+	}
+	ptr = &info->work;
+	if (*ptr){
+		printk(BIOS_EMERG, "start_work: core is busy %d\n", core);
+		return -1;
+	}
+	params = (u32 *)&info->params;
+	params[0] = a;
+	params[1] = b;
+	params[2] = c;
+	printk(BIOS_INFO, "BSP starts work on core %d\n", core);
+	*ptr = f;
+	return 0;
+}
+
+/* wait for the work to finish and return the result. Wait at
+ * most maxusec microseconds. 
+ */
+int wait_work(unsigned int core, u32 *retval, unsigned int maxusec)
+{
+	unsigned int usec;
+	struct cpu_info *info;
+	volatile workfunc *ptr;
+	volatile u32 *result;
+
+	if (core >= CONFIG_MAX_CPUS){
+		printk(BIOS_EMERG, "start_work: invalid core %d\n", core);
+		return -1;
+	}
+	info = &apcores[core].info;
+	if (! info->cpu->initialized){
+		printk(BIOS_EMERG, "start_work: core not initialized %d\n", core);
+		return -1;
+	}
+	ptr = &info->work;
+	for(usec = 0; usec < maxusec && *ptr; usec++)
+		udelay(1);
+
+	/* N.B. since only the BSP starts cores, there is not 
+	 * problem checking the pointer since access to it is 
+	 * serialized by the single-threaded BSP code. 
+	 */
+	if (*ptr){
+		printk(BIOS_INFO, "core %d still running after %d microseconds\n",
+			core, usec);
+		return -1;
+	}
+	result = &info->result;
+	printk(BIOS_INFO, "info->work after result is %#lx\n", (unsigned long)*result);
+	if (retval)
+		*retval = *result;
+	return 0;
+}
+
+/* start the work and wait for it to finish */
+int run_work(unsigned int core, workfunc f, u32 a, u32 b, u32 c, u32 *retval,
+	unsigned int timeout)
+{
+	if (start_work(core, f, a, b, c))
+		return -1;
+	return wait_work(core, retval, timeout);
+}
+#endif /* CONFIG_SMP == 1 */
diff --git a/src/include/cpu/cpu.h b/src/include/cpu/cpu.h
index cca2be1..0b7a748 100644
--- a/src/include/cpu/cpu.h
+++ b/src/include/cpu/cpu.h
 <at>  <at>  -5,9 +5,15  <at>  <at>  struct device;
 struct bus;
 #include <arch/cpu.h>

-void cpu_initialize(void);
+void cpu_initialize(struct cpu_info *info);
 void initialize_cpus(struct bus *cpu_bus);
-void secondary_cpu_init(void);
+void secondary_cpu_init(u32 unused);
+int start_work(unsigned int core, workfunc f, u32 a, u32 b, u32 c);
+int wait_work(unsigned int core, u32 *retval, unsigned int maxusec);
+int run_work(unsigned int core, workfunc f, u32 a, u32 b, u32 c, u32 *retval,
+	unsigned int timeout);
+void cpu_work(struct cpu_info *info);
+

 #if !CONFIG_WAIT_BEFORE_CPUS_INIT
 	#define cpus_ready_for_init() do {} while(0)

--

-- 
coreboot mailing list: coreboot <at> coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

ron minnich | 9 Apr 20:30 2012
Picon

coccinelle project for i915 graphics driver for coreboot

I'm trying an experiment and I welcome anyone who wants to help.

It's a simple idea motivated by a very hard problem. The problem is
that we have unending problems with binary video bioses. We'd like to
get to a source based video bios on, e.g., chromebook.

The linux driver for the intel i915 series can in fact turn on
hardware without the vbios having run. This is great; one reason we
boot the chromebook so fast (as some of you saw last week) is that we
don't bother running the video bios.

This doesn't help those cases where you need graphics in coreboot,
seabios, u-boot, or whatever. For those modes, we still need the video
bios, and we have to cope with its slowness, bugs, and impenetrable
behavior.

The simple idea is to use coccinelle to extract the init code from the
linux driver to create a standalone piece of C code . I've got to the
point that I can compile the module load function and most of the
things it calls to
a standalone user program.

Now why do it this way? Why not just do a one time hack of the driver
code to a coreboot driver? The reason is that the driver keeps
changing and getting bug fixes. It most recently got some very useful
fixes to the GMBUS driver. We want those fixes but the structure of
the code changed a lot.

I want to see if there is some more automated way to track those
changes. This may not work, but I think it's worth a try.

I'm calling the user mode code i915tool. The workflow is basically like this:

cd util/i915tool
export LINUX=/path/to/kernel
sh transform # copies in linux C code and transforms it
make

I've got this building two binaries that do things to graphics. Sadly,
I'm not lighting up any devices yet as the GMBUS always fails. But
it's a start, and being able to gdb this code is nice.

Anyway, there are people on this list who are lots better at
coccinelle than I am, and who know graphics better, and so on. What
I'm thinking to do is submit a commit for this tool in its nascent
state and let people take a look. Or, share the code, which may be
better.

If interested, let me know. I could use the help :-)

Thanks!

ron

--

-- 
coreboot mailing list: coreboot <at> coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Peter Stuge | 9 Apr 20:42 2012
Picon

Re: coccinelle project for i915 graphics driver for coreboot

ron minnich wrote:
> What I'm thinking to do is submit a commit for this tool in its
> nascent state and let people take a look. Or, share the code, which
> may be better.

I think it's a good approach. I would like having separate
repositories for our tools, but for now adding one more is
not making the problem worse I guess.

//Peter

--

-- 
coreboot mailing list: coreboot <at> coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Vikram Narayanan | 9 Apr 20:47 2012

New patch to review for coreboot: a928a36 Removed repeated sections of code in header file

Vikram Narayanan (vikram186 <at> gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/881

-gerrit

commit a928a367944580ed22a27ad5ad9de1ae2a17abde
Author: Vikram Narayanan <vikram186 <at> gmail.com>
Date:   Sat Feb 11 21:44:10 2012 +0530

    Removed repeated sections of code in header file

    Change-Id: Ia25e554dc2b4885b0470d250efb8b71671e47adb
    Signed-off-by: Vikram Narayanan <vikram186 <at> gmail.com>
---
 src/include/cpu/amd/amdfam12.h |   32 ++------------------------------
 1 files changed, 2 insertions(+), 30 deletions(-)

diff --git a/src/include/cpu/amd/amdfam12.h b/src/include/cpu/amd/amdfam12.h
index cabd532..db83821 100644
--- a/src/include/cpu/amd/amdfam12.h
+++ b/src/include/cpu/amd/amdfam12.h
 <at>  <at>  -20,35 +20,7  <at>  <at> 
 #ifndef CPU_AMD_FAM12_H
 #define CPU_AMD_FAM12_H

-#include <cpu/x86/msr.h>
-
-#define HWCR_MSR			0xC0010015
-#define NB_CFG_MSR			0xC001001f
-#define LS_CFG_MSR			0xC0011020
-#define IC_CFG_MSR			0xC0011021
-#define DC_CFG_MSR			0xC0011022
-#define BU_CFG_MSR			0xC0011023
-#define BU_CFG2_MSR			0xC001102A
-
-#define CPU_ID_FEATURES_MSR		0xC0011004
-#define CPU_ID_EXT_FEATURES_MSR	0xC0011005
-
-msr_t rdmsr_amd(u32 index);
-void wrmsr_amd(u32 index, msr_t msr);
-
-//#if defined(__GNUC__)
-//// it can be used to get unitid and coreid it running only
-//struct node_core_id get_node_core_id(u32 nb_cfg_54);
-//struct node_core_id get_node_core_id_x(void);
-//#endif
-
-#if defined(__PRE_RAM__)
-void wait_all_core0_started(void);
-void wait_all_other_cores_started(u32 bsp_apicid);
-void wait_all_aps_started(u32 bsp_apicid);
-void allow_all_aps_stop(u32 bsp_apicid);
-#endif
-void get_bus_conf(void);
-u32 get_initial_apicid(void);
+/* As this file has the same contents as amdfam14.h, it is included here */
+#include <cpu/amd/amdfam14.h>

 #endif /* CPU_AMD_FAM12_H */

--

-- 
coreboot mailing list: coreboot <at> coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Paul Menzel | 9 Apr 22:21 2012
Picon
Picon

Re: coccinelle project for i915 graphics driver for coreboot

Dear Ron,

Am Montag, den 09.04.2012, 11:30 -0700 schrieb ron minnich:
> I'm trying an experiment and I welcome anyone who wants to help.
> 
> It's a simple idea motivated by a very hard problem. The problem is
> that we have unending problems with binary video bioses. We'd like to
> get to a source based video bios on, e.g., chromebook.

[…]

> The simple idea is to use coccinelle to extract the init code from the
> linux driver to create a standalone piece of C code . I've got to the
> point that I can compile the module load function and most of the
> things it calls to a standalone user program.

[…]

> Anyway, there are people on this list who are lots better at
> coccinelle than I am, and who know graphics better, and so on. What
> I'm thinking to do is submit a commit for this tool in its nascent
> state and let people take a look. Or, share the code, which may be
> better.
> 
> If interested, let me know. I could use the help :-)

I guess also contacting the Coccinelle [1] and Intel-gfx [2] folks will
get you more answers.

Sharing your code in a repository hosted somewhere or submit a patch
with current state will certainly be beneficial.

Thanks,

Paul

[1] http://coccinelle.lip6.fr/contact.php
[2] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--

-- 
coreboot mailing list: coreboot <at> coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot
ron minnich | 9 Apr 22:23 2012
Picon

Re: coccinelle project for i915 graphics driver for coreboot

On Mon, Apr 9, 2012 at 11:42 AM, Peter Stuge <peter <at> stuge.se> wrote:

> I think it's a good approach. I would like having separate
> repositories for our tools, but for now adding one more is
> not making the problem worse I guess.

OK, what I will do for now is put it on code.google.com and, when we
think it's ready, we can if we want pull it into coreboot.

I really welcome comments and improvements and 'here is a much better
way to do it' patches.

The tool is at http://code.google.com/p/i915tool

I realize it's not at all polished. I'm learning :-)

thanks
ron

--

-- 
coreboot mailing list: coreboot <at> coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot


Gmane