pcbios: enable io/memory unconditionally

PATCH Applied Michael S. Tsirkin 3 participants 8 messages Oct 8, 2009 – Dec 2, 2009 KVM

Share & tools

Michael S. Tsirkin

author

2009-10-08 15:55 UTC #0 lore raw

PATCH v2 1/1 pcbios: 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.

Signed-off-by Michael S. Tsirkin <[email protected]>

---

Reposting for qemu tree.

 rombios32.c |   18 +++++++-----------
 1 files changed, 7 insertions(+), 11 deletions(-)

rombios32.c +7 ~~-11~~

index 624f39f..d01b61d 100644
@@ -693,6 +693,7 @@ void smp_probe(void)
 #define PCI_COMMAND		0x04	/* 16 bits */
 #define  PCI_COMMAND_IO		0x1	/* Enable response in I/O space */
 #define  PCI_COMMAND_MEMORY	0x2	/* Enable response in Memory space */
+#define PCI_CLASS_PROG		0x09	/* Reg. Level Programming Interface */
 #define PCI_CLASS_DEVICE        0x0a    /* Device class */
 #define PCI_INTERRUPT_LINE	0x3c	/* 8 bits */
 #define PCI_INTERRUPT_PIN	0x3d	/* 8 bits */

@@ -760,7 +761,6 @@ static uint32_t pci_config_readb(PCIDevice *d, uint32_t addr)
 
 static void pci_set_io_region_addr(PCIDevice *d, int region_num, uint32_t addr)
 {
-    uint16_t cmd;
     uint32_t ofs, old_addr;
 
     if ( region_num == PCI_ROM_SLOT ) {

@@ -773,16 +773,6 @@ static void pci_set_io_region_addr(PCIDevice *d, int region_num, uint32_t addr)
 
     pci_config_writel(d, ofs, addr);
     BX_INFO("region %d: 0x%08x\n", region_num, addr);
-
-    /* enable memory mappings */
-    cmd = pci_config_readw(d, PCI_COMMAND);
-    if ( region_num == PCI_ROM_SLOT )
-        cmd |= 2;
-    else if (old_addr & PCI_ADDRESS_SPACE_IO)
-        cmd |= 1;
-    else
-        cmd |= 2;
-    pci_config_writew(d, PCI_COMMAND, cmd);
 }
 
 /* return the global irq number corresponding to a given device irq

@@ -939,6 +929,7 @@ static void pci_bios_init_device(PCIDevice *d)
 {
     int class;
     uint32_t *paddr;
+    uint16_t cmd;
     int i, pin, pic_irq, vendor_id, device_id;
 
     class = pci_config_readw(d, PCI_CLASS_DEVICE);

@@ -1016,6 +1007,11 @@ static void pci_bios_init_device(PCIDevice *d)
         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(d, PCI_INTERRUPT_PIN);
     if (pin != 0) {

Signature · 1 line

1.6.5.rc2

On Thu, Oct 08, 2009 at 05:52:56PM +0200, Michael S. Tsirkin wrote:
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.

Signed-off-by Michael S. Tsirkin <[email protected]>

---

This patch is needed for qemu-kvm as that is still using pcbios.

 rombios32.c |   18 +++++++-----------
 1 files changed, 7 insertions(+), 11 deletions(-)

rombios32.c +7 ~~-11~~

index 624f39f..d01b61d 100644
@@ -693,6 +693,7 @@ void smp_probe(void)
 #define PCI_COMMAND		0x04	/* 16 bits */
 #define  PCI_COMMAND_IO		0x1	/* Enable response in I/O space */
 #define  PCI_COMMAND_MEMORY	0x2	/* Enable response in Memory space */
+#define PCI_CLASS_PROG		0x09	/* Reg. Level Programming Interface */
 #define PCI_CLASS_DEVICE        0x0a    /* Device class */
 #define PCI_INTERRUPT_LINE	0x3c	/* 8 bits */
 #define PCI_INTERRUPT_PIN	0x3d	/* 8 bits */

@@ -760,7 +761,6 @@ static uint32_t pci_config_readb(PCIDevice *d, uint32_t addr)
 
 static void pci_set_io_region_addr(PCIDevice *d, int region_num, uint32_t addr)
 {
-    uint16_t cmd;
     uint32_t ofs, old_addr;
 
     if ( region_num == PCI_ROM_SLOT ) {

@@ -773,16 +773,6 @@ static void pci_set_io_region_addr(PCIDevice *d, int region_num, uint32_t addr)
 
     pci_config_writel(d, ofs, addr);
     BX_INFO("region %d: 0x%08x\n", region_num, addr);
-
-    /* enable memory mappings */
-    cmd = pci_config_readw(d, PCI_COMMAND);
-    if ( region_num == PCI_ROM_SLOT )
-        cmd |= 2;
-    else if (old_addr & PCI_ADDRESS_SPACE_IO)
-        cmd |= 1;
-    else
-        cmd |= 2;
-    pci_config_writew(d, PCI_COMMAND, cmd);
 }
 
 /* return the global irq number corresponding to a given device irq

@@ -939,6 +929,7 @@ static void pci_bios_init_device(PCIDevice *d)
 {
     int class;
     uint32_t *paddr;
+    uint16_t cmd;
     int i, pin, pic_irq, vendor_id, device_id;
 
     class = pci_config_readw(d, PCI_CLASS_DEVICE);

@@ -1016,6 +1007,11 @@ static void pci_bios_init_device(PCIDevice *d)
         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(d, PCI_INTERRUPT_PIN);
     if (pin != 0) {

Signature · 1 line

1.6.5.rc2

Michael S. Tsirkin

author

2009-11-30 14:21 UTC #2 lore raw

Quoted 13 lines · Michael S. Tsirkin

On Thu, Oct 08, 2009 at 05:52:56PM +0200, Michael S. Tsirkin wrote:
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 8 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.
Signed-off-by Michael S. Tsirkin <[email protected]>
---
This patch is needed for qemu-kvm as that is still using pcbios.

After applying it, f0c728d09728825f7f8b6896adcf6e4fd58dc7ef in qemu-kvm can be reverted.

Signature · 1 line

MST

Anthony Liguori

2009-11-30 16:00 UTC #3 lore raw

In reply to Michael S. Tsirkin

Quoted 13 lines · Michael S. Tsirkin

On Thu, Oct 08, 2009 at 05:52:56PM +0200, Michael S. Tsirkin wrote:
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

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.
Signed-off-by Michael S. Tsirkin <[email protected]>

This is for stable?

Quoted 4 lines

---
This patch is needed for qemu-kvm as that is still using pcbios.

Why hasn't qemu-kvm switched to seabios?

Signature · 3 lines

Regards,

Anthony Liguori

Avi Kivity

2009-11-30 16:04 UTC #4 lore raw

Quoted 14 lines · Anthony Liguori

Quoted 13 lines · Michael S. Tsirkin
On Thu, Oct 08, 2009 at 05:52:56PM +0200, Michael S. Tsirkin wrote:
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 8 lines

Quoted 5 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.
Signed-off-by Michael S. Tsirkin <[email protected]>
This is for stable?

This is for qemu-kvm.git to get pcbios consistent with seabios.

Quoted 6 lines

Quoted 3 lines
---
This patch is needed for qemu-kvm as that is still using pcbios.
Why hasn't qemu-kvm switched to seabios?

Regressions. AFAICT all have been fixed now, and the switch is imminent.

Signature · 1 line

error compiling committee.c: too many arguments to function

Avi Kivity

2009-12-02 12:56 UTC #5 lore raw

Quoted 6 lines · Avi Kivity

Quoted 2 lines
Why hasn't qemu-kvm switched to seabios?
Regressions. AFAICT all have been fixed now, and the switch is imminent.

The switch is now complete.

Signature · 1 line

error compiling committee.c: too many arguments to function

Michael S. Tsirkin

author

2009-11-30 16:07 UTC #6 lore raw

In reply to Anthony Liguori

Quoted 14 lines · Anthony Liguori

Quoted 13 lines · Michael S. Tsirkin
On Thu, Oct 08, 2009 at 05:52:56PM +0200, Michael S. Tsirkin wrote:
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 8 lines

Quoted 6 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.
Signed-off-by Michael S. Tsirkin <[email protected]>
This is for stable?

This is for qemu-kvm as the description says.

Quoted 6 lines

Quoted 4 lines
---
This patch is needed for qemu-kvm as that is still using pcbios.
Why hasn't qemu-kvm switched to seabios?

IMO it's better to fix bug then switch than the reverse. This way you have a good base to compare against.

Quoted 4 lines

Signature · 3 lines
Regards,

Anthony Liguori

Avi Kivity

Applied

2009-12-02 13:28 UTC #7 lore raw

In reply to Michael S. Tsirkin

Quoted 13 lines · Michael S. Tsirkin

On Thu, Oct 08, 2009 at 05:52:56PM +0200, Michael S. Tsirkin wrote:
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 5 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.

Applied, thanks.

Signature · 1 line

error compiling committee.c: too many arguments to function