Patchwork Improve YABEL compatibility

login
register
about
Submitter Patrick Georgi
Date 2011-01-13 10:05:55
Message ID <1294913155.2411.18.camel@linux-0a8x.site>
Download mbox | patch
Permalink /patch/2513/
State Accepted
Headers show

Comments

Patrick Georgi - 2011-01-13 10:05:55
Hi,

attached patch improves compatibility of YABEL with realmode execution
of the VGA BIOS by passing through access to IVT, BDA and option ROM
area directly, instead of copying them from the emulated memory to real
memory after initialization is done.

Some VGA BIOSes seem to map hardware registers into the option ROM area,
so routing accesses to these into emulated RAM will lead to all kinds of
unexpected behaviour.

The intent of YABEL in coreboot is to provide improved security by
isolating hardware access - this patch doesn't improve or degrade
security as the memory regions in question were already copied back
after emulation before this patch.


Signed-off-by: Patrick Georgi <patrick.georgi@secunet.com>
Joseph Smith - 2011-01-13 11:00:27
On Thu, 13 Jan 2011 11:05:55 +0100, Patrick Georgi
<Patrick.Georgi@secunet.com> wrote:
> Hi,
> 
> attached patch improves compatibility of YABEL with realmode execution
> of the VGA BIOS by passing through access to IVT, BDA and option ROM
> area directly, instead of copying them from the emulated memory to real
> memory after initialization is done.
> 
> Some VGA BIOSes seem to map hardware registers into the option ROM area,
> so routing accesses to these into emulated RAM will lead to all kinds of
> unexpected behaviour.
> 
> The intent of YABEL in coreboot is to provide improved security by
> isolating hardware access - this patch doesn't improve or degrade
> security as the memory regions in question were already copied back
> after emulation before this patch.
> 
> 
> Signed-off-by: Patrick Georgi <patrick.georgi@secunet.com>

Nice work!

Acked-by: Joseph Smith <joe@settoplinux.org>
Pattrick Hueper - 2011-01-14 10:13:07
Looks great to me...

Acked-by: Pattrick Hueper <phueper@hueper.net>

On Thu, Jan 13, 2011 at 12:00 PM, Joseph Smith <joe@settoplinux.org> wrote:
>
>
> On Thu, 13 Jan 2011 11:05:55 +0100, Patrick Georgi
> <Patrick.Georgi@secunet.com> wrote:
>> Hi,
>>
>> attached patch improves compatibility of YABEL with realmode execution
>> of the VGA BIOS by passing through access to IVT, BDA and option ROM
>> area directly, instead of copying them from the emulated memory to real
>> memory after initialization is done.
>>
>> Some VGA BIOSes seem to map hardware registers into the option ROM area,
>> so routing accesses to these into emulated RAM will lead to all kinds of
>> unexpected behaviour.
>>
>> The intent of YABEL in coreboot is to provide improved security by
>> isolating hardware access - this patch doesn't improve or degrade
>> security as the memory regions in question were already copied back
>> after emulation before this patch.
>>
>>
>> Signed-off-by: Patrick Georgi <patrick.georgi@secunet.com>
>
> Nice work!
>
> Acked-by: Joseph Smith <joe@settoplinux.org>
>
> --
> Thanks,
> Joseph Smith
> Set-Top-Linux
> www.settoplinux.org
>
>
> --
> coreboot mailing list: coreboot@coreboot.org
> http://www.coreboot.org/mailman/listinfo/coreboot
>

Patch

Index: coreboot-staging/src/devices/oprom/yabel/biosemu.c

===================================================================
--- coreboot-staging.orig/src/devices/oprom/yabel/biosemu.c

+++ coreboot-staging/src/devices/oprom/yabel/biosemu.c

@@ -106,6 +106,9 @@  biosemu(u8 *biosmem, u32 biosmem_size, s

 		printf("Error: Device Expansion ROM invalid!\n");
 		return -1;
 	}
+	biosemu_add_special_memory(0, 0x500); // IVT + BDA

+	biosemu_add_special_memory(OPTION_ROM_CODE_SEGMENT << 4, 0x10000); // option ROM

+

 	rom_image = (u8 *) bios_device.img_addr;
 	DEBUG_PRINTF("executing rom_image from %p\n", rom_image);
 	DEBUG_PRINTF("biosmem at %p\n", biosmem);
@@ -129,7 +132,7 @@  biosemu(u8 *biosmem, u32 biosmem_size, s

 	// copy expansion ROM image to segment OPTION_ROM_CODE_SEGMENT
 	// NOTE: this sometimes fails, some bytes are 0x00... so we compare
 	// after copying and do some retries...
-	u8 *mem_img = biosmem + (OPTION_ROM_CODE_SEGMENT << 4);

+	u8 *mem_img = OPTION_ROM_CODE_SEGMENT << 4;

 	u8 copy_count = 0;
 	u8 cmp_result = 0;
 	do {
@@ -152,7 +155,7 @@  biosemu(u8 *biosmem, u32 biosmem_size, s

 				break;
 			}
 			clr_ci();
-			*(mem_img + i) = c;

+			my_wrb(mem_img + i, c);

 		}
 #endif
 		copy_count++;
Index: coreboot-staging/src/devices/oprom/yabel/compat/functions.c

===================================================================
--- coreboot-staging.orig/src/devices/oprom/yabel/compat/functions.c

+++ coreboot-staging/src/devices/oprom/yabel/compat/functions.c

@@ -41,13 +41,6 @@  void run_bios(struct device * dev, unsig

 #if CONFIG_BOOTSPLASH
 	vbe_set_graphics();
 #endif
-

-	if (vmem != NULL) {

-		printf("Copying legacy memory from %p to the lower 1MB\n", vmem);

-		memcpy((void *)0x00000, vmem + 0x00000, 0x400);         // IVT

-		memcpy((void *)0x00400, vmem + 0x00400, 0x100);         // BDA

-		memcpy((void *)0xc0000, vmem + 0xc0000, 0x10000);       // VGA OPROM

-	}

 }
 
 unsigned long tb_freq = 0;
Index: coreboot-staging/src/devices/oprom/yabel/device.c

===================================================================
--- coreboot-staging.orig/src/devices/oprom/yabel/device.c

+++ coreboot-staging/src/devices/oprom/yabel/device.c

@@ -24,8 +24,8 @@ 

 
 /* the device we are working with... */
 biosemu_device_t bios_device;
-//max. 6 BARs and 1 Exp.ROM plus CfgSpace and 3 legacy ranges

-translate_address_t translate_address_array[11];

+//max. 6 BARs and 1 Exp.ROM plus CfgSpace and 3 legacy ranges, plus 2 "special" memory ranges

+translate_address_t translate_address_array[13];

 u8 taa_last_entry;
 
 typedef struct {
@@ -209,6 +209,23 @@  biosemu_dev_get_addr_info(void)

 }
 #endif
 
+// "special memory" is a hack to make some parts of memory fall through to real memory

+// (ie. no translation). Necessary if option ROMs attempt DMA there, map registers or

+// do similarily crazy things.

+void

+biosemu_add_special_memory(u32 start, u32 size)

+{

+	int taa_index = ++taa_last_entry;

+	translate_address_array[taa_index].info = IORESOURCE_FIXED | IORESOURCE_MEM;

+	translate_address_array[taa_index].bus = 0;

+	translate_address_array[taa_index].devfn = 0;

+	translate_address_array[taa_index].cfg_space_offset = 0;

+	translate_address_array[taa_index].address = start;

+	translate_address_array[taa_index].size = size;

+	/* dont translate addresses... all addresses are 1:1 */

+	translate_address_array[taa_index].address_offset = 0;

+}

+

 #ifndef CONFIG_PCI_OPTION_ROM_RUN_YABEL
 // to simulate accesses to legacy VGA Memory (0xA0000-0xBFFFF)
 // we look for the first prefetchable memory BAR, if no prefetchable BAR found,
@@ -419,7 +436,7 @@  biosemu_dev_init(struct device * device)

 // and accessing client interface for every translation...
 // returns: 0 if addr not found in translate_address_array, 1 if found.
 u8
-biosemu_dev_translate_address(unsigned long * addr)

+biosemu_dev_translate_address(int type, unsigned long * addr)

 {
 	int i = 0;
 	translate_address_t ta;
@@ -443,7 +460,7 @@  biosemu_dev_translate_address(unsigned l

 #endif
 	for (i = 0; i <= taa_last_entry; i++) {
 		ta = translate_address_array[i];
-		if ((*addr >= ta.address) && (*addr <= (ta.address + ta.size))) {

+		if ((*addr >= ta.address) && (*addr <= (ta.address + ta.size)) && (ta.info & type)) {

 			*addr += ta.address_offset;
 			return 1;
 		}
Index: coreboot-staging/src/devices/oprom/yabel/device.h

===================================================================
--- coreboot-staging.orig/src/devices/oprom/yabel/device.h

+++ coreboot-staging/src/devices/oprom/yabel/device.h

@@ -101,15 +101,18 @@  typedef struct {

 // device. Needed for faster address translation, so
 // not every I/O or Memory Access needs to call translate_address_dev
 // and access the device tree
-// 6 BARs, 1 Exp. ROM, 1 Cfg.Space, and 3 Legacy

+// 6 BARs, 1 Exp. ROM, 1 Cfg.Space, and 3 Legacy, plus 2 "special"

 // translations are supported... this should be enough for
 // most devices... for VGA it is enough anyways...
-extern translate_address_t translate_address_array[11];

+extern translate_address_t translate_address_array[13];

 
 // index of last translate_address_array entry
 // set by get_dev_addr_info function
 extern u8 taa_last_entry;
 
+// add 1:1 mapped memory regions to translation table

+void biosemu_add_special_memory(u32 start, u32 size);

+

 /* the device we are working with... */
 extern biosemu_device_t bios_device;
 
@@ -117,7 +120,7 @@  u8 biosemu_dev_init(struct device * devi

 // NOTE: for dev_check_exprom to work, biosemu_dev_init MUST be called first!
 u8 biosemu_dev_check_exprom(unsigned long rom_base_addr);
 
-u8 biosemu_dev_translate_address(unsigned long * addr);

+u8 biosemu_dev_translate_address(int type, unsigned long * addr);

 
 /* endianness swap functions for 16 and 32 bit words
  * copied from axon_pciconfig.c
Index: coreboot-staging/src/devices/oprom/yabel/io.c

===================================================================
--- coreboot-staging.orig/src/devices/oprom/yabel/io.c

+++ coreboot-staging/src/devices/oprom/yabel/io.c

@@ -22,6 +22,7 @@ 

 #ifdef CONFIG_PCI_OPTION_ROM_RUN_YABEL
 #include <device/pci.h>
 #include <device/pci_ops.h>
+#include <device/resource.h>

 #endif
 
 #ifdef CONFIG_ARCH_X86
@@ -181,7 +182,7 @@  my_inb(X86EMU_pioAddr addr)

 {
 	u8 rval = 0xFF;
 	unsigned long translated_addr = addr;
-	u8 translated = biosemu_dev_translate_address(&translated_addr);

+	u8 translated = biosemu_dev_translate_address(IORESOURCE_IO, &translated_addr);

 	if (translated != 0) {
 		//translation successfull, access Device I/O (BAR or Legacy...)
 		DEBUG_PRINTF_IO("%s(%x): access to Device I/O\n", __func__,
@@ -231,7 +232,7 @@  u16

 my_inw(X86EMU_pioAddr addr)
 {
 	unsigned long translated_addr = addr;
-	u8 translated = biosemu_dev_translate_address(&translated_addr);

+	u8 translated = biosemu_dev_translate_address(IORESOURCE_IO, &translated_addr);

 	if (translated != 0) {
 		//translation successfull, access Device I/O (BAR or Legacy...)
 		DEBUG_PRINTF_IO("%s(%x): access to Device I/O\n", __func__,
@@ -276,7 +277,7 @@  u32

 my_inl(X86EMU_pioAddr addr)
 {
 	unsigned long translated_addr = addr;
-	u8 translated = biosemu_dev_translate_address(&translated_addr);

+	u8 translated = biosemu_dev_translate_address(IORESOURCE_IO, &translated_addr);

 	if (translated != 0) {
 		//translation successfull, access Device I/O (BAR or Legacy...)
 		DEBUG_PRINTF_IO("%s(%x): access to Device I/O\n", __func__,
@@ -322,7 +323,7 @@  void

 my_outb(X86EMU_pioAddr addr, u8 val)
 {
 	unsigned long translated_addr = addr;
-	u8 translated = biosemu_dev_translate_address(&translated_addr);

+	u8 translated = biosemu_dev_translate_address(IORESOURCE_IO, &translated_addr);

 	if (translated != 0) {
 		//translation successfull, access Device I/O (BAR or Legacy...)
 		DEBUG_PRINTF_IO("%s(%x, %x): access to Device I/O\n",
@@ -354,7 +355,7 @@  void

 my_outw(X86EMU_pioAddr addr, u16 val)
 {
 	unsigned long translated_addr = addr;
-	u8 translated = biosemu_dev_translate_address(&translated_addr);

+	u8 translated = biosemu_dev_translate_address(IORESOURCE_IO, &translated_addr);

 	if (translated != 0) {
 		//translation successfull, access Device I/O (BAR or Legacy...)
 		DEBUG_PRINTF_IO("%s(%x, %x): access to Device I/O\n",
@@ -395,7 +396,7 @@  void

 my_outl(X86EMU_pioAddr addr, u32 val)
 {
 	unsigned long translated_addr = addr;
-	u8 translated = biosemu_dev_translate_address(&translated_addr);

+	u8 translated = biosemu_dev_translate_address(IORESOURCE_IO, &translated_addr);

 	if (translated != 0) {
 		//translation successfull, access Device I/O (BAR or Legacy...)
 		DEBUG_PRINTF_IO("%s(%x, %x): access to Device I/O\n",
Index: coreboot-staging/src/devices/oprom/yabel/mem.c

===================================================================
--- coreboot-staging.orig/src/devices/oprom/yabel/mem.c

+++ coreboot-staging/src/devices/oprom/yabel/mem.c

@@ -21,6 +21,10 @@ 

 
 #if !defined(CONFIG_YABEL_DIRECTHW) || (!CONFIG_YABEL_DIRECTHW)
 
+#ifdef CONFIG_PCI_OPTION_ROM_RUN_YABEL

+#include <device/resource.h>

+#endif

+

 // define a check for access to certain (virtual) memory regions (interrupt handlers, BIOS Data Area, ...)
 #if CONFIG_X86EMU_DEBUG
 static u8 in_check = 0;	// to avoid recursion...
@@ -195,7 +199,7 @@  u8

 my_rdb(u32 addr)
 {
 	unsigned long translated_addr = addr;
-	u8 translated = biosemu_dev_translate_address(&translated_addr);

+	u8 translated = biosemu_dev_translate_address(IORESOURCE_MEM, &translated_addr);

 	u8 rval;
 	if (translated != 0) {
 		//translation successfull, access VGA Memory (BAR or Legacy...)
@@ -227,7 +231,7 @@  u16

 my_rdw(u32 addr)
 {
 	unsigned long translated_addr = addr;
-	u8 translated = biosemu_dev_translate_address(&translated_addr);

+	u8 translated = biosemu_dev_translate_address(IORESOURCE_MEM, &translated_addr);

 	u16 rval;
 	if (translated != 0) {
 		//translation successfull, access VGA Memory (BAR or Legacy...)
@@ -278,7 +282,7 @@  u32

 my_rdl(u32 addr)
 {
 	unsigned long translated_addr = addr;
-	u8 translated = biosemu_dev_translate_address(&translated_addr);

+	u8 translated = biosemu_dev_translate_address(IORESOURCE_MEM, &translated_addr);

 	u32 rval;
 	if (translated != 0) {
 		//translation successfull, access VGA Memory (BAR or Legacy...)
@@ -341,7 +345,7 @@  void

 my_wrb(u32 addr, u8 val)
 {
 	unsigned long translated_addr = addr;
-	u8 translated = biosemu_dev_translate_address(&translated_addr);

+	u8 translated = biosemu_dev_translate_address(IORESOURCE_MEM, &translated_addr);

 	if (translated != 0) {
 		//translation successfull, access VGA Memory (BAR or Legacy...)
 		DEBUG_PRINTF_MEM("%s(%x, %x): access to VGA Memory\n",
@@ -366,7 +370,7 @@  void

 my_wrw(u32 addr, u16 val)
 {
 	unsigned long translated_addr = addr;
-	u8 translated = biosemu_dev_translate_address(&translated_addr);

+	u8 translated = biosemu_dev_translate_address(IORESOURCE_MEM, &translated_addr);

 	if (translated != 0) {
 		//translation successfull, access VGA Memory (BAR or Legacy...)
 		DEBUG_PRINTF_MEM("%s(%x, %x): access to VGA Memory\n",
@@ -411,7 +415,7 @@  void

 my_wrl(u32 addr, u32 val)
 {
 	unsigned long translated_addr = addr;
-	u8 translated = biosemu_dev_translate_address(&translated_addr);

+	u8 translated = biosemu_dev_translate_address(IORESOURCE_MEM, &translated_addr);

 	if (translated != 0) {
 		//translation successfull, access VGA Memory (BAR or Legacy...)
 		DEBUG_PRINTF_MEM("%s(%x, %x): access to VGA Memory\n",