In the original code vga_inited is set, then run_bios() is called, which does printk(), which is allowed using the VGA.
The patch puts vga_inited=1 and run_bios() in the right order.
Roman
---------------------- Index: src/devices/pci_rom.c =================================================================== --- src/devices/pci_rom.c (revision 2536) +++ src/devices/pci_rom.c (working copy) @@ -69,7 +69,7 @@ #endif #endif
-struct rom_header *pci_rom_load(struct device *dev, struct rom_header *rom_header) +struct rom_header *pci_rom_load_and_run(struct device *dev, struct rom_header *rom_header) { struct pci_data * rom_data; unsigned long rom_address; @@ -96,6 +96,7 @@ printk_debug("copying VGA ROM Image from 0x%x to 0x%x, 0x%x bytes\n", rom_header, PCI_VGA_RAM_IMAGE_START, rom_size); memcpy(PCI_VGA_RAM_IMAGE_START, rom_header, rom_size); + run_bios(dev,(unsigned long)PCI_VGA_RAM_IMAGE_START); vga_inited = 1; return (struct rom_header *) (PCI_VGA_RAM_IMAGE_START); #endif @@ -103,6 +104,7 @@ printk_debug("copying non-VGA ROM Image from 0x%x to 0x%x, 0x%x bytes\n", rom_header, pci_ram_image_start, rom_size); memcpy(pci_ram_image_start, rom_header, rom_size); + run_bios(dev,(unsigned long)pci_ram_image_start); pci_ram_image_start += rom_size; return (struct rom_header *) (pci_ram_image_start-rom_size); } Index: src/devices/pci_device.c =================================================================== --- src/devices/pci_device.c (revision 2536) +++ src/devices/pci_device.c (working copy) @@ -634,16 +634,12 @@ void pci_dev_init(struct device *dev) { #if CONFIG_PCI_ROM_RUN == 1 - struct rom_header *rom, *ram; + struct rom_header *rom;
rom = pci_rom_probe(dev); if (rom == NULL) return; - ram = pci_rom_load(dev, rom); - if (ram == NULL) - return; - - run_bios(dev, ram); + pci_rom_load_and_run(dev, rom); #endif }
Index: src/include/device/pci_rom.h =================================================================== --- src/include/device/pci_rom.h (revision 2536) +++ src/include/device/pci_rom.h (working copy) @@ -34,7 +34,8 @@ };
extern struct rom_header * pci_rom_probe(struct device *dev); -extern struct rom_header *pci_rom_load(struct device *dev, struct rom_header *rom_header); +extern struct rom_header *pci_rom_load_and_run(struct device *dev, struct rom_header *rom_header); +void run_bios(struct device *dev,unsigned long addr);
extern void pci_dev_init(struct device *dev);
On 01/17/2007 05:56 PM, Roman Kononov wrote:
In the original code vga_inited is set, then run_bios() is called, which does printk(), which is allowed using the VGA.
The patch puts vga_inited=1 and run_bios() in the right order.
For me, this bug resulted in linuxbios hanged.
When console log level is BIOS_INFO and above, linuxbios accesses the video memory of uninitialized VGA hardware, which is bad.
Roman
Roman Kononov wrote:
On 01/17/2007 05:56 PM, Roman Kononov wrote:
In the original code vga_inited is set, then run_bios() is called, which does printk(), which is allowed using the VGA.
The patch puts vga_inited=1 and run_bios() in the right order.
For me, this bug resulted in linuxbios hanged.
When console log level is BIOS_INFO and above, linuxbios accesses the video memory of uninitialized VGA hardware, which is bad.
Can you try the attached patch? It's a little less intrusive
Index: src/devices/pci_rom.c =================================================================== --- src/devices/pci_rom.c (revision 2534) +++ src/devices/pci_rom.c (working copy) @@ -63,7 +63,6 @@ static void *pci_ram_image_start = (void *)PCI_RAM_IMAGE_START;
#if CONFIG_CONSOLE_VGA == 1 -extern int vga_inited; // defined in vga_console.c #if CONFIG_CONSOLE_VGA_MULTI == 0 extern device_t vga_pri; // the primary vga device, defined in device.c #endif @@ -96,7 +95,6 @@ printk_debug("copying VGA ROM Image from 0x%x to 0x%x, 0x%x bytes\n", rom_header, PCI_VGA_RAM_IMAGE_START, rom_size); memcpy(PCI_VGA_RAM_IMAGE_START, rom_header, rom_size); - vga_inited = 1; return (struct rom_header *) (PCI_VGA_RAM_IMAGE_START); #endif } else { Index: src/devices/pci_device.c =================================================================== --- src/devices/pci_device.c (revision 2534) +++ src/devices/pci_device.c (working copy) @@ -634,7 +634,9 @@ void pci_dev_init(struct device *dev) { #if CONFIG_PCI_ROM_RUN == 1 + extern int vga_inited; struct rom_header *rom, *ram; + struct pci_data * rom_data;
rom = pci_rom_probe(dev); if (rom == NULL) @@ -644,6 +646,8 @@ return;
run_bios(dev, ram); + if (dev->class == PCI_CLASS_DISPLAY_VGA) + vga_inited = 1; #endif }
On 01/31/2007 12:48 PM, Stefan Reinauer wrote:
Can you try the attached patch? It's a little less intrusive
It will not compile when CONFIG_CONSOLE_VGA=0. Are you comfortable with more #ifdefs?
Related question:
If I say CONFIG_PCI_ROM_RUN=1 and CONFIG_CONSOLE_VGA=0, why does it skip VGA initialization?
The rational here is that I may need a lot of debugging output, but I do not want it to be directed to the VGA, because it is almost useless on the VGA and the users are scared. In the same time I need the VGA to be initialized so that payloads can use it.
I would make it so that the VGA is initialized if either flag is set. Anything else is initialized only on CONFIG_PCI_ROM_RUN.
Roman Kononov wrote:
Related question:
If I say CONFIG_PCI_ROM_RUN=1 and CONFIG_CONSOLE_VGA=0, why does it skip VGA initialization?
The rational here is that I may need a lot of debugging output, but I do not want it to be directed to the VGA, because it is almost useless on the VGA and the users are scared. In the same time I need the VGA to be initialized so that payloads can use it.
I would make it so that the VGA is initialized if either flag is set. Anything else is initialized only on CONFIG_PCI_ROM_RUN.
This sounds reasonable. Is it enough to not set vga_inited if CONFIG_CONSOLE_VGA=0 ?
On 01/31/2007 01:52 PM, Stefan Reinauer wrote:
Is it enough to not set vga_inited if CONFIG_CONSOLE_VGA=0 ?
Yes, it is. If CONFIG_CONSOLE_VGA=0, vga_inited is undefined, else vga_inited is defined and should be set.
This patch makes sure that VGA is initialized before it is used. Additionally, VGA will be initialized if either CONFIG_PCI_ROM_RUN=1 or CONFIG_CONSOLE_VGA=1.
Signed-off-by: Roman Kononov kononov195-lbl@yahoo.com ---
On Wed, Jan 31, 2007 at 05:35:21PM -0600, Roman Kononov wrote:
This patch makes sure that VGA is initialized before it is used. Additionally, VGA will be initialized if either CONFIG_PCI_ROM_RUN=1 or CONFIG_CONSOLE_VGA=1.
Index: src/devices/pci_device.c =================================================================== --- src/devices/pci_device.c (revision 2539) +++ src/devices/pci_device.c (working copy) @@ -633,7 +633,7 @@ void pci_dev_set_subsystem(device_t dev,
void pci_dev_init(struct device *dev) { -#if CONFIG_PCI_ROM_RUN == 1 +#if CONFIG_PCI_ROM_RUN == 1 || CONFIG_CONSOLE_VGA=1 struct rom_header *rom, *ram;
rom = pci_rom_probe(dev);
One or two = ? Which is it?
//Peter
On 01/31/2007 05:53 PM, Peter Stuge wrote:
+#if CONFIG_PCI_ROM_RUN == 1 || CONFIG_CONSOLE_VGA=1 One or two = ? Which is it?
None.
Roman Kononov wrote:
On 01/31/2007 05:53 PM, Peter Stuge wrote:
+#if CONFIG_PCI_ROM_RUN == 1 || CONFIG_CONSOLE_VGA=1 One or two = ? Which is it?
None.
I cleaned the patch up a bit, and and added some comments. I will apply it as soon as someone sends an Acked-by.
Stefan
When CONFIG_CONSOLE_VGA==0 and CONFIG_PCI_ROM_RUN==1 VGA will not be initialized because of pci_rom_load()
What about vgainit-v3?
Roman
-
Peter Stuge -
Roman Kononov -
Stefan Reinauer