Skip to contentSkip to conversation

seabios: enable io/memory unconditionally

PATCH Superseded Michael S. Tsirkin 4 participants 5 messages Oct 8, 2009 – Oct 9, 2009 KVM

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 seabios: enable io/memory unconditionally

VGA adapters need to claim memory and i/o
transactions even if they do not have any
i/o or memory bars. E.g. PCI spec, page 297,
gives an example of such a device:

Programming interface 0000 0000b
VGA-compatible controller. Memory
addresses 0A 0000h through 0B
FFFFh. I/O addresses 3B0h to 3BBh
and 3C0h to 3DFh and all aliases of
these addresses.

While we could check for these devices and special-case them, it is easier to fix this by enabling i/o and memory space unconditionally:
devices that do not support it will just ignore this setting.

---

Reposting for qemu tree.

 src/pciinit.c |   17 ++++++-----------
 1 files changed, 6 insertions(+), 11 deletions(-)
src/pciinit.c +6 -11
index 0d558a9..eab082a 100644
@@ -28,7 +28,6 @@ static u8 pci_irqs[4] = {
 
 static void pci_set_io_region_addr(u16 bdf, int region_num, u32 addr)
 {
-    u16 cmd;
     u32 ofs, old_addr;
 
     if (region_num == PCI_ROM_SLOT) {
@@ -41,16 +40,6 @@ static void pci_set_io_region_addr(u16 bdf, int region_num, u32 addr)
 
     pci_config_writel(bdf, ofs, addr);
     dprintf(1, "region %d: 0x%08x\n", region_num, addr);
-
-    /* enable memory mappings */
-    cmd = pci_config_readw(bdf, PCI_COMMAND);
-    if (region_num == PCI_ROM_SLOT)
-        cmd |= PCI_COMMAND_MEMORY;
-    else if (old_addr & PCI_BASE_ADDRESS_SPACE_IO)
-        cmd |= PCI_COMMAND_IO;
-    else
-        cmd |= PCI_COMMAND_MEMORY;
-    pci_config_writew(bdf, PCI_COMMAND, cmd);
 }
 
 /* return the global irq number corresponding to a given device irq
@@ -95,6 +84,7 @@ static void pci_bios_init_device(u16 bdf)
 {
     int class;
     u32 *paddr;
+    uint16_t cmd;
     int i, pin, pic_irq, vendor_id, device_id;
 
     class = pci_config_readw(bdf, PCI_CLASS_DEVICE);
@@ -165,6 +155,11 @@ static void pci_bios_init_device(u16 bdf)
         break;
     }
 
+    /* enable memory mappings */
+    cmd = pci_config_readw(d, PCI_COMMAND);
+    cmd |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY;
+    pci_config_writew(d, PCI_COMMAND, cmd);
+
     /* map the interrupt */
     pin = pci_config_readb(bdf, PCI_INTERRUPT_PIN);
     if (pin != 0) {
Signature · 1 line
1.6.5.rc2
#1 lore raw

Add seabios maintainer to CC.

Quoted 79 lines · Michael S. Tsirkin

VGA adapters need to claim memory and i/o
transactions even if they do not have any
i/o or memory bars. E.g. PCI spec, page 297,
gives an example of such a device:

Programming interface 0000 0000b
VGA-compatible controller. Memory
addresses 0A 0000h through 0B
FFFFh. I/O addresses 3B0h to 3BBh
and 3C0h to 3DFh and all aliases of
these addresses.

While we could check for these devices and special-case them, it is easier to fix this by enabling i/o and memory space unconditionally:
devices that do not support it will just ignore this setting.

---

Reposting for qemu tree.

 src/pciinit.c |   17 ++++++-----------
 1 files changed, 6 insertions(+), 11 deletions(-)
src/pciinit.c +6 -11
index 0d558a9..eab082a 100644
@@ -28,7 +28,6 @@ static u8 pci_irqs[4] = {
 
 static void pci_set_io_region_addr(u16 bdf, int region_num, u32 addr)
 {
-    u16 cmd;
     u32 ofs, old_addr;
 
     if (region_num == PCI_ROM_SLOT) {
@@ -41,16 +40,6 @@ static void pci_set_io_region_addr(u16 bdf, int region_num, u32 addr)
 
     pci_config_writel(bdf, ofs, addr);
     dprintf(1, "region %d: 0x%08x\n", region_num, addr);
-
-    /* enable memory mappings */
-    cmd = pci_config_readw(bdf, PCI_COMMAND);
-    if (region_num == PCI_ROM_SLOT)
-        cmd |= PCI_COMMAND_MEMORY;
-    else if (old_addr & PCI_BASE_ADDRESS_SPACE_IO)
-        cmd |= PCI_COMMAND_IO;
-    else
-        cmd |= PCI_COMMAND_MEMORY;
-    pci_config_writew(bdf, PCI_COMMAND, cmd);
 }
 
 /* return the global irq number corresponding to a given device irq
@@ -95,6 +84,7 @@ static void pci_bios_init_device(u16 bdf)
 {
     int class;
     u32 *paddr;
+    uint16_t cmd;
     int i, pin, pic_irq, vendor_id, device_id;
 
     class = pci_config_readw(bdf, PCI_CLASS_DEVICE);
@@ -165,6 +155,11 @@ static void pci_bios_init_device(u16 bdf)
         break;
     }
 
+    /* enable memory mappings */
+    cmd = pci_config_readw(d, PCI_COMMAND);
+    cmd |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY;
+    pci_config_writew(d, PCI_COMMAND, cmd);
+
     /* map the interrupt */
     pin = pci_config_readb(bdf, PCI_INTERRUPT_PIN);
     if (pin != 0) {
Signature · 5 lines
1.6.5.rc2
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Gleb.

#2 lore raw
In reply to Michael S. Tsirkin

Hi Michael,

Quoted 12 lines · Michael S. Tsirkin

VGA adapters need to claim memory and i/o
transactions even if they do not have any
i/o or memory bars. E.g. PCI spec, page 297,
gives an example of such a device:

Programming interface 0000 0000b
VGA-compatible controller. Memory
addresses 0A 0000h through 0B
FFFFh. I/O addresses 3B0h to 3BBh
and 3C0h to 3DFh and all aliases of
these addresses.

Quoted 3 lines

While we could check for these devices and special-case them, it is easier to fix this by enabling i/o and memory space unconditionally:
devices that do not support it will just ignore this setting.

This doesn't sound correct to me - I would think the vga option rom should enable the memory and io bars. I don't have enough knowledge to say for sure though - can someone else with knowledge in this area confirm this approach?

-Kevin

#3 lore raw
Quoted 16 lines · Kevin O'Connor

Hi Michael,

Quoted 12 lines · Michael S. Tsirkin

VGA adapters need to claim memory and i/o
transactions even if they do not have any
i/o or memory bars. E.g. PCI spec, page 297,
gives an example of such a device:

Programming interface 0000 0000b
VGA-compatible controller. Memory
addresses 0A 0000h through 0B
FFFFh. I/O addresses 3B0h to 3BBh
and 3C0h to 3DFh and all aliases of
these addresses.

Quoted 10 lines
Quoted 4 lines

While we could check for these devices and special-case them, it is easier to fix this by enabling i/o and memory space unconditionally:
devices that do not support it will just ignore this setting.

This doesn't sound correct to me - I would think the vga option rom should enable the memory and io bars. I don't have enough knowledge to say for sure though - can someone else with knowledge in this area confirm this approach?

The vga option rom is often itself in a BAR, so it cannot enable memory.

Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
#4 lore raw
In reply to Kevin O'Connor
Quoted 15 lines · Kevin O'Connor

Hi Michael,

Quoted 12 lines · Michael S. Tsirkin

VGA adapters need to claim memory and i/o
transactions even if they do not have any
i/o or memory bars. E.g. PCI spec, page 297,
gives an example of such a device:

Programming interface 0000 0000b
VGA-compatible controller. Memory
addresses 0A 0000h through 0B
FFFFh. I/O addresses 3B0h to 3BBh
and 3C0h to 3DFh and all aliases of
these addresses.

Quoted 6 lines
Quoted 3 lines

While we could check for these devices and special-case them, it is easier to fix this by enabling i/o and memory space unconditionally:
devices that do not support it will just ignore this setting.

This doesn't sound correct to me - I would think the vga option rom should enable the memory and io bars.

You can do this, but in real systems, BIOS enables memory on all devices. Just take a real system and do lspci -v on a device that has no driver loaded.
And note how seabios does this *already* - it just doesn't enable I/O for VGA device unless it has I/O bar, which is wrong.

Quoted 5 lines

I don't have enough knowledge
to say for sure though - can someone else with knowledge in this area confirm this approach?

-Kevin