[Qemu-devel] [PATCHv2] pcbios: enable io/memory unconditionally

[Qemu-devel] [PATCHv2] pcbios: enable io/memory unconditionally

[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.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Reposting for qemu tree.

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

diff --git a/rombios32.c b/rombios32.c
index 624f39f..d01b61d 100644
--- a/rombios32.c
+++ b/rombios32.c
@@ -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) {
-- 
1.6.5.rc2

[Qemu-devel] [PATCHv2-repost] pcbios: enable io/memory unconditionally

[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.

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 <mst@redhat.com>
---

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

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

diff --git a/rombios32.c b/rombios32.c
index 624f39f..d01b61d 100644
--- a/rombios32.c
+++ b/rombios32.c
@@ -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) {
-- 
1.6.5.rc2

[Qemu-devel] Re: [PATCHv2-repost] pcbios: enable io/memory unconditionally

[Michael S. Tsirkin]

On Mon, Nov 30, 2009 at 04:17:22PM +0200, Michael S. Tsirkin wrote:
> 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 <mst@redhat.com>
> ---
> 
> This patch is needed for qemu-kvm as that is still using pcbios.

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

-- 
MST

[Qemu-devel] Re: [PATCHv2-repost] pcbios: enable io/memory unconditionally

[Anthony Liguori]

Michael S. Tsirkin wrote:
> 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 <mst@redhat.com>
>   

This is for stable?

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

Why hasn't qemu-kvm switched to seabios?

-- 
Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCHv2-repost] pcbios: enable io/memory unconditionally

[Avi Kivity]

On 11/30/2009 06:00 PM, Anthony Liguori wrote:
> Michael S. Tsirkin wrote:
>> 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 <mst@redhat.com>
>
> This is for stable?
>

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

>> ---
>>
>> 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.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCHv2-repost] pcbios: enable io/memory unconditionally

[Avi Kivity]

On 11/30/2009 06:04 PM, Avi Kivity wrote:
>> 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.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCHv2-repost] pcbios: enable io/memory unconditionally

[Michael S. Tsirkin]

On Mon, Nov 30, 2009 at 10:00:23AM -0600, Anthony Liguori wrote:
> Michael S. Tsirkin wrote:
>> 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 <mst@redhat.com>
>>   
>
> This is for stable?

This is for qemu-kvm as the description says.

>> ---
>>
>> 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.


> -- 
> Regards,
>
> Anthony Liguori

[Qemu-devel] Re: [PATCHv2-repost] pcbios: enable io/memory unconditionally

[Avi Kivity]

On 11/30/2009 04:17 PM, Michael S. Tsirkin wrote:
> 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.
>
>    

Applied, thanks.

-- 
error compiling committee.c: too many arguments to function