Skip to contentSkip to conversation

pci: set memory type for memory behind the bridge

PATCH Applied Michael S. Tsirkin 4 participants 4 messages Feb 21, 2012 – Feb 22, 2012

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 pci: set memory type for memory behind the bridge

As we make upper bits in IO and prefetcheable memory registers writeable, we should declare support for 64 bit prefetcheable memory and 32 bit io in the bridge.

This changes the default for apb, dec, but I'm guessing they got the defaults wrong by accident. Alternatively, we could let bridges declare lack of 64 bit support and make the upper bits read-only zero.

With this applied, we can drop these bits
from express code.

Could someone familiar with apb,dec ack this please?

---
 hw/pci.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)
hw/pci.c +13 -2
index 31d6a5f..3ca5f4c 100644
@@ -611,7 +611,7 @@ static void pci_init_w1cmask(PCIDevice *dev)
                  PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_DETECTED_PARITY);
 }
 
-static void pci_init_wmask_bridge(PCIDevice *d)
+static void pci_init_mask_bridge(PCIDevice *d)
 {
     /* PCI_PRIMARY_BUS, PCI_SECONDARY_BUS, PCI_SUBORDINATE_BUS and
        PCI_SEC_LETENCY_TIMER */
@@ -632,6 +632,14 @@ static void pci_init_wmask_bridge(PCIDevice *d)
     /* PCI_PREF_BASE_UPPER32 and PCI_PREF_LIMIT_UPPER32 */
     memset(d->wmask + PCI_PREF_BASE_UPPER32, 0xff, 8);
 
+    /* Supported memory and i/o types */
+    d->config[PCI_IO_BASE] |= PCI_IO_RANGE_TYPE_32;
+    d->config[PCI_IO_LIMIT] |= PCI_IO_RANGE_TYPE_32;
+    pci_word_test_and_set_mask(d->config + PCI_PREF_MEMORY_BASE,
+                               PCI_PREF_RANGE_TYPE_64);
+    pci_word_test_and_set_mask(d->config + PCI_PREF_MEMORY_LIMIT,
+                               PCI_PREF_RANGE_TYPE_64);
+
 /* TODO: add this define to pci_regs.h in linux and then in qemu. */
 #define  PCI_BRIDGE_CTL_VGA_16BIT	0x10	/* VGA 16-bit decode */
 #define  PCI_BRIDGE_CTL_DISCARD		0x100	/* Primary discard timer */
@@ -654,6 +662,9 @@ static void pci_init_wmask_bridge(PCIDevice *d)
      * completeness. */
     pci_set_word(d->w1cmask + PCI_BRIDGE_CONTROL,
                  PCI_BRIDGE_CTL_DISCARD_STATUS);
+    d->cmask[PCI_IO_BASE] |= PCI_IO_RANGE_TYPE_MASK;
+    pci_word_test_and_set_mask(d->cmask + PCI_PREF_MEMORY_BASE,
+                               PCI_PREF_RANGE_TYPE_MASK);
 }
 
 static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
@@ -775,7 +786,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
     pci_init_wmask(pci_dev);
     pci_init_w1cmask(pci_dev);
     if (pc->is_bridge) {
-        pci_init_wmask_bridge(pci_dev);
+        pci_init_mask_bridge(pci_dev);
     }
     if (pci_init_multifunction(bus, pci_dev)) {
         pci_config_free(pci_dev);
Signature · 1 line
1.7.9.111.gf3fb0
#1 lore raw
Quoted 5 lines · Michael S. Tsirkin

As we make upper bits in IO and prefetcheable memory registers writeable, we should declare support for 64 bit prefetcheable memory and 32 bit io in the bridge.

Quoted 7 lines

This changes the default for apb, dec, but I'm guessing they got the defaults wrong by accident. Alternatively, we could let bridges declare lack of 64 bit support and make the upper bits read-only zero.

With this applied, we can drop these bits
from express code.

Ben, you know these devices better than us. Any comments? :)

Alex

Quoted 56 lines

Could someone familiar with apb,dec ack this please?

---
  hw/pci.c |   15 +++++++++++++--
  1 files changed, 13 insertions(+), 2 deletions(-)
hw/pci.c +13 -2
index 31d6a5f..3ca5f4c 100644
@@ -611,7 +611,7 @@ static void pci_init_w1cmask(PCIDevice *dev)
                   PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_DETECTED_PARITY);
  }
 
-static void pci_init_wmask_bridge(PCIDevice *d)
+static void pci_init_mask_bridge(PCIDevice *d)
  {
      /* PCI_PRIMARY_BUS, PCI_SECONDARY_BUS, PCI_SUBORDINATE_BUS and
         PCI_SEC_LETENCY_TIMER */
@@ -632,6 +632,14 @@ static void pci_init_wmask_bridge(PCIDevice *d)
      /* PCI_PREF_BASE_UPPER32 and PCI_PREF_LIMIT_UPPER32 */
      memset(d->wmask + PCI_PREF_BASE_UPPER32, 0xff, 8);
 
+    /* Supported memory and i/o types */
+    d->config[PCI_IO_BASE] |= PCI_IO_RANGE_TYPE_32;
+    d->config[PCI_IO_LIMIT] |= PCI_IO_RANGE_TYPE_32;
+    pci_word_test_and_set_mask(d->config + PCI_PREF_MEMORY_BASE,
+                               PCI_PREF_RANGE_TYPE_64);
+    pci_word_test_and_set_mask(d->config + PCI_PREF_MEMORY_LIMIT,
+                               PCI_PREF_RANGE_TYPE_64);
+
  /* TODO: add this define to pci_regs.h in linux and then in qemu. */
  #define  PCI_BRIDGE_CTL_VGA_16BIT	0x10	/* VGA 16-bit decode */
  #define  PCI_BRIDGE_CTL_DISCARD		0x100	/* Primary discard timer */
@@ -654,6 +662,9 @@ static void pci_init_wmask_bridge(PCIDevice *d)
       * completeness. */
      pci_set_word(d->w1cmask + PCI_BRIDGE_CONTROL,
                   PCI_BRIDGE_CTL_DISCARD_STATUS);
+    d->cmask[PCI_IO_BASE] |= PCI_IO_RANGE_TYPE_MASK;
+    pci_word_test_and_set_mask(d->cmask + PCI_PREF_MEMORY_BASE,
+                               PCI_PREF_RANGE_TYPE_MASK);
  }
 
  static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
@@ -775,7 +786,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
      pci_init_wmask(pci_dev);
      pci_init_w1cmask(pci_dev);
      if (pc->is_bridge) {
-        pci_init_wmask_bridge(pci_dev);
+        pci_init_mask_bridge(pci_dev);
      }
      if (pci_init_multifunction(bus, pci_dev)) {
          pci_config_free(pci_dev);
#2 lore raw
Quoted 11 lines · Alexander Graf
Quoted 10 lines · Michael S. Tsirkin

As we make upper bits in IO and prefetcheable memory registers writeable, we should declare support for 64 bit prefetcheable memory and 32 bit io in the bridge.

This changes the default for apb, dec, but I'm guessing they got the defaults wrong by accident. Alternatively, we could let bridges declare lack of 64 bit support and make the upper bits read-only zero.

Quoted 4 lines
Quoted 2 lines

With this applied, we can drop these bits
from express code.

Ben, you know these devices better than us. Any comments? :)

I don't know that code very well, but Michael's explanation at least makes sense to me :-)

Cheers,
Ben.

Quoted 58 lines

Alex

Quoted 56 lines

Could someone familiar with apb,dec ack this please?

---
  hw/pci.c |   15 +++++++++++++--
  1 files changed, 13 insertions(+), 2 deletions(-)
hw/pci.c +13 -2
index 31d6a5f..3ca5f4c 100644
@@ -611,7 +611,7 @@ static void pci_init_w1cmask(PCIDevice *dev)
                   PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_DETECTED_PARITY);
  }
 
-static void pci_init_wmask_bridge(PCIDevice *d)
+static void pci_init_mask_bridge(PCIDevice *d)
  {
      /* PCI_PRIMARY_BUS, PCI_SECONDARY_BUS, PCI_SUBORDINATE_BUS and
         PCI_SEC_LETENCY_TIMER */
@@ -632,6 +632,14 @@ static void pci_init_wmask_bridge(PCIDevice *d)
      /* PCI_PREF_BASE_UPPER32 and PCI_PREF_LIMIT_UPPER32 */
      memset(d->wmask + PCI_PREF_BASE_UPPER32, 0xff, 8);
 
+    /* Supported memory and i/o types */
+    d->config[PCI_IO_BASE] |= PCI_IO_RANGE_TYPE_32;
+    d->config[PCI_IO_LIMIT] |= PCI_IO_RANGE_TYPE_32;
+    pci_word_test_and_set_mask(d->config + PCI_PREF_MEMORY_BASE,
+                               PCI_PREF_RANGE_TYPE_64);
+    pci_word_test_and_set_mask(d->config + PCI_PREF_MEMORY_LIMIT,
+                               PCI_PREF_RANGE_TYPE_64);
+
  /* TODO: add this define to pci_regs.h in linux and then in qemu. */
  #define  PCI_BRIDGE_CTL_VGA_16BIT	0x10	/* VGA 16-bit decode */
  #define  PCI_BRIDGE_CTL_DISCARD		0x100	/* Primary discard timer */
@@ -654,6 +662,9 @@ static void pci_init_wmask_bridge(PCIDevice *d)
       * completeness. */
      pci_set_word(d->w1cmask + PCI_BRIDGE_CONTROL,
                   PCI_BRIDGE_CTL_DISCARD_STATUS);
+    d->cmask[PCI_IO_BASE] |= PCI_IO_RANGE_TYPE_MASK;
+    pci_word_test_and_set_mask(d->cmask + PCI_PREF_MEMORY_BASE,
+                               PCI_PREF_RANGE_TYPE_MASK);
  }
 
  static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
@@ -775,7 +786,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
      pci_init_wmask(pci_dev);
      pci_init_w1cmask(pci_dev);
      if (pc->is_bridge) {
-        pci_init_wmask_bridge(pci_dev);
+        pci_init_mask_bridge(pci_dev);
      }
      if (pci_init_multifunction(bus, pci_dev)) {
          pci_config_free(pci_dev);
#3 lore raw
In reply to Michael S. Tsirkin
Quoted 10 lines · Michael S. Tsirkin

As we make upper bits in IO and prefetcheable memory registers writeable, we should declare support for 64 bit prefetcheable memory and 32 bit io in the bridge.

This changes the default for apb, dec, but I'm guessing they got the defaults wrong by accident. Alternatively, we could let bridges declare lack of 64 bit support and make the upper bits read-only zero.

Quoted 7 lines

With this applied, we can drop these bits
from express code.

Could someone familiar with apb,dec ack this please?

Applied. Thanks.

Regards,

Anthony Liguori

Quoted 52 lines
---
  hw/pci.c |   15 +++++++++++++--
  1 files changed, 13 insertions(+), 2 deletions(-)
hw/pci.c +13 -2
index 31d6a5f..3ca5f4c 100644
@@ -611,7 +611,7 @@ static void pci_init_w1cmask(PCIDevice *dev)
                   PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_DETECTED_PARITY);
  }
 
-static void pci_init_wmask_bridge(PCIDevice *d)
+static void pci_init_mask_bridge(PCIDevice *d)
  {
      /* PCI_PRIMARY_BUS, PCI_SECONDARY_BUS, PCI_SUBORDINATE_BUS and
         PCI_SEC_LETENCY_TIMER */
@@ -632,6 +632,14 @@ static void pci_init_wmask_bridge(PCIDevice *d)
      /* PCI_PREF_BASE_UPPER32 and PCI_PREF_LIMIT_UPPER32 */
      memset(d->wmask + PCI_PREF_BASE_UPPER32, 0xff, 8);
 
+    /* Supported memory and i/o types */
+    d->config[PCI_IO_BASE] |= PCI_IO_RANGE_TYPE_32;
+    d->config[PCI_IO_LIMIT] |= PCI_IO_RANGE_TYPE_32;
+    pci_word_test_and_set_mask(d->config + PCI_PREF_MEMORY_BASE,
+                               PCI_PREF_RANGE_TYPE_64);
+    pci_word_test_and_set_mask(d->config + PCI_PREF_MEMORY_LIMIT,
+                               PCI_PREF_RANGE_TYPE_64);
+
  /* TODO: add this define to pci_regs.h in linux and then in qemu. */
  #define  PCI_BRIDGE_CTL_VGA_16BIT	0x10	/* VGA 16-bit decode */
  #define  PCI_BRIDGE_CTL_DISCARD		0x100	/* Primary discard timer */
@@ -654,6 +662,9 @@ static void pci_init_wmask_bridge(PCIDevice *d)
       * completeness. */
      pci_set_word(d->w1cmask + PCI_BRIDGE_CONTROL,
                   PCI_BRIDGE_CTL_DISCARD_STATUS);
+    d->cmask[PCI_IO_BASE] |= PCI_IO_RANGE_TYPE_MASK;
+    pci_word_test_and_set_mask(d->cmask + PCI_PREF_MEMORY_BASE,
+                               PCI_PREF_RANGE_TYPE_MASK);
  }
 
  static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
@@ -775,7 +786,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
      pci_init_wmask(pci_dev);
      pci_init_w1cmask(pci_dev);
      if (pc->is_bridge) {
-        pci_init_wmask_bridge(pci_dev);
+        pci_init_mask_bridge(pci_dev);
      }
      if (pci_init_multifunction(bus, pci_dev)) {
          pci_config_free(pci_dev);