Patchwork refactor SMI handler

login
register
about
Submitter Stefan Reinauer
Date 2009-10-24 01:35:09
Message ID <4AE259CD.5040803@coresystems.de>
Download mbox | patch
Permalink /patch/476/
State Accepted
Headers show

Comments

Stefan Reinauer - 2009-10-24 01:35:09
See patch
Myles Watson - 2009-10-24 13:57:27
2009/10/23 Stefan Reinauer <stepan@coresystems.de>

> See patch
>
> --
> coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
>      Tel.: +49 761 7668825 • Fax: +49 761 7664613
> Email: info@coresystems.de  • http://www.coresystems.de/
> Registergericht: Amtsgericht Freiburg • HRB 7656
> Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866
>
>
> * refactor x86 smi handler (put all debug stuff in an extra file smiutil.c)
> * lock other CPUs in SMI handler while one CPU is handling an SMI. Without
> this
>  various racing scenarios could happen.
>
> Signed-off-by: Stefan Reinauer <stepan@coresystems.de>
>
Acked-by: Myles Watson <mylesgw@gmail.com>
Is there some header file that could be included for these defines?  Maybe
even config.h?


> +#ifndef CONFIG_TTYS0_BASE
> +#define CONFIG_TTYS0_BASE 0x3f8
> +#endif
>

Thanks,
Myles
Stefan Reinauer - 2009-10-24 18:02:49
Myles Watson wrote:
>
>
> 2009/10/23 Stefan Reinauer <stepan@coresystems.de
> <mailto:stepan@coresystems.de>>
>
>     See patch
>
>     --
>     coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
>          Tel.: +49 761 7668825 • Fax: +49 761 7664613
>     Email: info@coresystems.de <mailto:info@coresystems.de>  •
>     http://www.coresystems.de/
>     Registergericht: Amtsgericht Freiburg • HRB 7656
>     Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866
>
>
>     * refactor x86 smi handler (put all debug stuff in an extra file
>     smiutil.c)
>     * lock other CPUs in SMI handler while one CPU is handling an SMI.
>     Without this
>      various racing scenarios could happen.
>
>     Signed-off-by: Stefan Reinauer <stepan@coresystems.de
>     <mailto:stepan@coresystems.de>>
>
> Acked-by: Myles Watson <mylesgw@gmail.com <mailto:mylesgw@gmail.com>>
Thanks, r4840.


> Is there some header file that could be included for these defines? 
> Maybe even config.h?
>  
>
>     +#ifndef CONFIG_TTYS0_BASE
>     +#define CONFIG_TTYS0_BASE 0x3f8
>     +#endif
>
>
No, but it would make sense... What's the right place for this?
include/uart8250.h ?

Right now they are defined in
./src/arch/i386/init/crt0.S.lb
./src/console/uart8250_console.c
./src/cpu/amd/model_lx/cache_as_ram.inc
./src/cpu/x86/smm/smiutil.c
./src/pc80/serial.c
Myles Watson - 2009-10-24 19:40:04
> No, but it would make sense... What's the right place for this?
> include/uart8250.h ?
maybe include/pc80/serial.h or include/pc80/uart8250.h?

Thanks,
Myles
Peter Stuge - 2009-10-25 14:02:59
Myles Watson wrote:
> > No, but it would make sense... What's the right place for this?
> > include/uart8250.h ?
> 
> maybe include/pc80/serial.h or include/pc80/uart8250.h?

If either exists, use that. If neither exists I prefer the simpler
serial.h.


//Peter
Stefan Reinauer - 2009-10-25 14:14:06
Peter Stuge wrote:
> Myles Watson wrote:
>   
>>> No, but it would make sense... What's the right place for this?
>>> include/uart8250.h ?
>>>       
>> maybe include/pc80/serial.h or include/pc80/uart8250.h?
>>     
>
> If either exists, use that. If neither exists I prefer the simpler
> serial.h.
>   

include/uart8250.h is the only one that exists, hence my suggestion.


It occurs to me that we should have pc80/serial.c and lib/uart8250.c in the same directory. Maybe even the same file. They share a lot functionality.

Stefan
ron minnich - 2009-10-25 18:02:41
On Sun, Oct 25, 2009 at 7:14 AM, Stefan Reinauer <stepan@coresystems.de> wrote:

> It occurs to me that we should have pc80/serial.c and lib/uart8250.c in the same directory. Maybe even the same file. They share a lot functionality.
>

IIRC there are two as not all platforms that had teh 8250 were PCs. We
may not care now that we have dropped heterogeneity as a goal (if you
want a different platform, then you want U-Boot :-)

ron

Patch

Index: src/cpu/x86/smm/Config.lb
===================================================================
--- src/cpu/x86/smm/Config.lb	(revision 4832)
+++ src/cpu/x86/smm/Config.lb	(working copy)
@@ -25,6 +25,7 @@ 
 
 	smmobject smmhandler.S
 	smmobject smihandler.o
+	smmobject smiutil.o
 
 	makerule smm.o
 		depends	"$(SMM-OBJECTS) src/console/printk.o src/console/vtxprintf.o $(LIBGCC_FILE_NAME)" 
Index: src/cpu/x86/smm/Makefile.inc
===================================================================
--- src/cpu/x86/smm/Makefile.inc	(revision 4832)
+++ src/cpu/x86/smm/Makefile.inc	(working copy)
@@ -23,6 +23,7 @@ 
 ##
 ##	smmobject smmhandler.S
 ##	smmobject smihandler.o
+##	smmobject smiutil.o
 ##
 ##	makerule smm.o
 ##		depends	"$(SMM-OBJECTS) $(TOP)/src/console/printk.o $(TOP)/src/console/vtxprintf.o $(LIBGCC_FILE_NAME)"
Index: src/cpu/x86/smm/smihandler.c
===================================================================
--- src/cpu/x86/smm/smihandler.c	(revision 4832)
+++ src/cpu/x86/smm/smihandler.c	(working copy)
@@ -1,7 +1,7 @@ 
 /*
  * This file is part of the coreboot project.
  *
- * Copyright (C) 2008 coresystems GmbH
+ * Copyright (C) 2008-2009 coresystems GmbH
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
@@ -27,7 +27,7 @@ 
 
 void southbridge_smi_set_eos(void);
 
-#define DEBUG_SMI
+/* To enable SMI define DEBUG_SMI in smiutil.c */
 
 typedef enum { SMI_LOCKED, SMI_UNLOCKED } smi_semaphore;
 
@@ -67,66 +67,6 @@ 
 	return (*((volatile unsigned long *)(LAPIC_ID)) >> 24);
 }
 
-/* ********************* smi_util ************************* */
-
-/* Data */
-#define UART_RBR 0x00
-#define UART_TBR 0x00
-
-/* Control */
-#define UART_IER 0x01
-#define UART_IIR 0x02
-#define UART_FCR 0x02
-#define UART_LCR 0x03
-#define UART_MCR 0x04
-#define UART_DLL 0x00
-#define UART_DLM 0x01
-
-/* Status */
-#define UART_LSR 0x05
-#define UART_MSR 0x06
-#define UART_SCR 0x07
-
-static int uart_can_tx_byte(void)
-{
-	return inb(CONFIG_TTYS0_BASE + UART_LSR) & 0x20;
-}
-
-static void uart_wait_to_tx_byte(void)
-{
-	while(!uart_can_tx_byte()) 
-	;
-}
-
-static void uart_wait_until_sent(void)
-{
-	while(!(inb(CONFIG_TTYS0_BASE + UART_LSR) & 0x40))
-	; 
-}
-
-static void uart_tx_byte(unsigned char data)
-{
-	uart_wait_to_tx_byte();
-	outb(data, CONFIG_TTYS0_BASE + UART_TBR);
-	/* Make certain the data clears the fifos */
-	uart_wait_until_sent();
-}
-
-void console_tx_flush(void)
-{
-	uart_wait_to_tx_byte();
-}
-
-void console_tx_byte(unsigned char byte)
-{
-	if (byte == '\n')
-		uart_tx_byte('\r');
-	uart_tx_byte(byte);
-}
-
-/* ********************* smi_util ************************* */
-
-
 void io_trap_handler(int smif)
 {
 	/* If a handler function handled a given IO trap, it
@@ -163,16 +103,17 @@ 
 	smm_state_save_area_t state_save;
 
 	/* Are we ok to execute the handler? */
-	if (!smi_obtain_lock())
+	if (!smi_obtain_lock()) {
+		/* For security reasons we don't release the other CPUs
+		 * until the CPU with the lock is actually done
+		 */
+		while (smi_handler_status == SMI_LOCKED) /* wait */ ;
 		return;
+	}
 
 	node=nodeid();
 
-#ifdef DEBUG_SMI
-	console_loglevel = CONFIG_DEFAULT_CONSOLE_LOGLEVEL;
-#else
-	console_loglevel = 1;
-#endif
+	console_init();
 
 	printk_spew("\nSMI# #%d\n", node);
 
Index: src/cpu/x86/smm/smiutil.c
===================================================================
--- src/cpu/x86/smm/smiutil.c	(revision 0)
+++ src/cpu/x86/smm/smiutil.c	(revision 0)
@@ -0,0 +1,130 @@ 
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2008-2009 coresystems GmbH
+ *
+ * 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; version 2 of
+ * the License.
+ *
+ * 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., 51 Franklin St, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ */
+
+#include <arch/io.h>
+#include <arch/romcc_io.h>
+#include <console/console.h>
+#include <cpu/x86/cache.h>
+#include <cpu/x86/smm.h>
+
+// #define DEBUG_SMI
+
+/* ********************* smi_util ************************* */
+
+/* Data */
+#define UART_RBR 0x00
+#define UART_TBR 0x00
+
+/* Control */
+#define UART_IER 0x01
+#define UART_IIR 0x02
+#define UART_FCR 0x02
+#define UART_LCR 0x03
+#define UART_MCR 0x04
+#define UART_DLL 0x00
+#define UART_DLM 0x01
+
+/* Status */
+#define UART_LSR 0x05
+#define UART_MSR 0x06
+#define UART_SCR 0x07
+
+#ifndef CONFIG_TTYS0_BASE
+#define CONFIG_TTYS0_BASE 0x3f8
+#endif
+
+#ifndef CONFIG_TTYS0_BAUD
+#define CONFIG_TTYS0_BAUD 115200
+#endif
+
+#ifndef CONFIG_TTYS0_DIV
+#define CONFIG_TTYS0_DIV	(115200/CONFIG_TTYS0_BAUD)
+#endif
+
+/* Line Control Settings */
+#ifndef CONFIG_TTYS0_LCS
+/* Set 8bit, 1 stop bit, no parity */
+#define CONFIG_TTYS0_LCS	0x3
+#endif
+
+#define UART_LCS	CONFIG_TTYS0_LCS
+
+static int uart_can_tx_byte(void)
+{
+	return inb(CONFIG_TTYS0_BASE + UART_LSR) & 0x20;
+}
+
+static void uart_wait_to_tx_byte(void)
+{
+	while(!uart_can_tx_byte()) 
+	;
+}
+
+static void uart_wait_until_sent(void)
+{
+	while(!(inb(CONFIG_TTYS0_BASE + UART_LSR) & 0x40))
+	; 
+}
+
+static void uart_tx_byte(unsigned char data)
+{
+	uart_wait_to_tx_byte();
+	outb(data, CONFIG_TTYS0_BASE + UART_TBR);
+	/* Make certain the data clears the fifos */
+	uart_wait_until_sent();
+}
+
+void console_tx_flush(void)
+{
+	uart_wait_to_tx_byte();
+}
+
+void console_tx_byte(unsigned char byte)
+{
+	if (byte == '\n')
+		uart_tx_byte('\r');
+	uart_tx_byte(byte);
+}
+
+void uart_init(void)
+{
+	/* disable interrupts */
+	outb(0x0, CONFIG_TTYS0_BASE + UART_IER);
+	/* enable fifo's */
+	outb(0x01, CONFIG_TTYS0_BASE + UART_FCR);
+	/* Set Baud Rate Divisor to 12 ==> 115200 Baud */
+	outb(0x80 | UART_LCS, CONFIG_TTYS0_BASE + UART_LCR);
+	outb(CONFIG_TTYS0_DIV & 0xFF,   CONFIG_TTYS0_BASE + UART_DLL);
+	outb((CONFIG_TTYS0_DIV >> 8) & 0xFF,    CONFIG_TTYS0_BASE + UART_DLM);
+	outb(UART_LCS, CONFIG_TTYS0_BASE + UART_LCR);
+}
+
+void console_init(void)
+{
+#ifdef DEBUG_SMI
+	console_loglevel = CONFIG_DEFAULT_CONSOLE_LOGLEVEL;
+	uart_init();
+#else
+	console_loglevel = 1;
+#endif
+}
+
+/* ********************* smi_util ************************* */