[Qemu-devel] [PATCH 0/4] pci: clean up of command/status register

[Qemu-devel] [PATCH 0/4] pci: clean up of command/status register

[Isaku Yamahata]

This patch set cleans up of initialization of pci command/status register.

Isaku Yamahata (4):
  pci: revise pci command register initialization
  pci: clean up pci command register io/memory bit initialization
  pci: fix accesses to pci status register
  pci: clean up of pci status register

 hw/pci.c |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 108 insertions(+), 11 deletions(-)

[Qemu-devel] [PATCH 1/4] pci: revise pci command register initialization

[Isaku Yamahata]

This patch cleans up command register initialization with
comments.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/pci.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 8b79ad6..86900a2 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -640,8 +640,50 @@ static void pci_init_wmask(PCIDevice *dev)
 
     dev->wmask[PCI_CACHE_LINE_SIZE] = 0xff;
     dev->wmask[PCI_INTERRUPT_LINE] = 0xff;
+
+    /*
+     * bit 0: PCI_COMMAND_IO
+     *        type 0: if IO BAR is used, RW
+     *        type 1: RW
+     * bit 1: PCI_COMMAND_MEMORY
+     *        type 0: if IO BAR is used, RW
+     *        type 1: RW
+     * bit 2: PCI_COMMAND_MASTER
+     *        type 0: RW if bus master
+     *        type 1: RW
+     * bit 3: PCI_COMMAND_SPECIAL
+     *        RO=0, optionally RW: Such device should set this bit itself
+     * bit 4: PCI_COMMAND_INVALIDATE
+     *        RO=0, optionally RW: Such device should set this bit itself
+     * bit 5: PCI_COMMAND_VGA_PALETTE
+     *        RO=0, optionally RW: Such device should set this bit itself
+     * bit 6: PCI_COMMAND_PARITY
+     *        RW with exceptions: Such device should clear this bit itself
+     *        Given that qemu doesn't emulate pci bus cycles, so that there
+     *        is no place to generate parity error. So just make this
+     *        register RW is safe because there is no place which refers
+     *        this bit.
+     *        TODO: When device assignment tried to inject PERR# into qemu,
+     *              some extra work would be needed.
+     * bit 7: PCI_COMMAND_WAIT: reserved (PCI 3.0)
+     *        RO=0
+     * bit 8: PCI_COMMAND_SERR
+     *        RW with exceptions: Such device should clear this bit itself
+     *        Given that qemu doesn't emulate pci bus cycles, so that there
+     *        is no place to generate system error. So just make this
+     *        register RW is safe because there is no place which refers
+     *        this bit.
+     *        TODO: When device assignment tried to inject SERR# into qemu,
+     *              some extra work would be needed.
+     * bit 9: PCI_COMMAND_FAST_BACK
+     *        RO=0, optionally RW: Such device should set this bit itself
+     * bit 10: PCI_COMMAND_INTX_DISABLE
+     *         RW
+     * bit 11-15: reserved
+     */
     pci_set_word(dev->wmask + PCI_COMMAND,
                  PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER |
+                 PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
                  PCI_COMMAND_INTX_DISABLE);
 
     memset(dev->wmask + PCI_CONFIG_HEADER_SIZE, 0xff,
-- 
1.7.1.1

[Qemu-devel] [PATCH 2/4] pci: clean up pci command register io/memory bit initialization

[Isaku Yamahata]

This patch fixes the initialization of io/memory bit of command register.
Those bits for type 1 device is RW.
Those bits for type 0 device is
  RO = 0 if it has no io/memory BAR
  RW if it has io/memory BAR

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/pci.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 86900a2..a734e14 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -644,10 +644,14 @@ static void pci_init_wmask(PCIDevice *dev)
     /*
      * bit 0: PCI_COMMAND_IO
      *        type 0: if IO BAR is used, RW
-     *        type 1: RW
+     *                This is handled by pci_register_bar()
+     *        type 1: RW:
+     *                This is fixed by pci_init_wmask_bridge()
      * bit 1: PCI_COMMAND_MEMORY
      *        type 0: if IO BAR is used, RW
+     *                This is handled by pci_register_bar()
      *        type 1: RW
+     *                This is fixed by pci_init_wmask_bridge()
      * bit 2: PCI_COMMAND_MASTER
      *        type 0: RW if bus master
      *        type 1: RW
@@ -682,8 +686,7 @@ static void pci_init_wmask(PCIDevice *dev)
      * bit 11-15: reserved
      */
     pci_set_word(dev->wmask + PCI_COMMAND,
-                 PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER |
-                 PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
+                 PCI_COMMAND_MASTER | PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
                  PCI_COMMAND_INTX_DISABLE);
 
     memset(dev->wmask + PCI_CONFIG_HEADER_SIZE, 0xff,
@@ -692,6 +695,9 @@ static void pci_init_wmask(PCIDevice *dev)
 
 static void pci_init_wmask_bridge(PCIDevice *d)
 {
+    pci_word_test_and_set_mask(d->wmask + PCI_COMMAND,
+                               PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
+
     /* PCI_PRIMARY_BUS, PCI_SECONDARY_BUS, PCI_SUBORDINATE_BUS and
        PCI_SEC_LETENCY_TIMER */
     memset(d->wmask + PCI_PRIMARY_BUS, 0xff, 4);
@@ -929,6 +935,14 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
     if (region_num == PCI_ROM_SLOT) {
         /* ROM enable bit is writeable */
         wmask |= PCI_ROM_ADDRESS_ENABLE;
+    } else {
+        if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
+            pci_word_test_and_set_mask(pci_dev->wmask + PCI_COMMAND,
+                                       PCI_COMMAND_IO);
+        } else {
+            pci_word_test_and_set_mask(pci_dev->wmask + PCI_COMMAND,
+                                       PCI_COMMAND_MEMORY);
+        }
     }
     pci_set_long(pci_dev->config + addr, type);
     if (!(r->type & PCI_BASE_ADDRESS_SPACE_IO) &&
-- 
1.7.1.1

[Qemu-devel] [PATCH 3/4] pci: fix accesses to pci status register

[Isaku Yamahata]

pci status register is 16 bit, not 8 bit.
So use helper function to manipulate status register.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/pci.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index a734e14..a16e763 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -127,9 +127,11 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int irq_num, int change)
 static void pci_update_irq_status(PCIDevice *dev)
 {
     if (dev->irq_state) {
-        dev->config[PCI_STATUS] |= PCI_STATUS_INTERRUPT;
+        pci_word_test_and_set_mask(dev->config + PCI_STATUS,
+                                   PCI_STATUS_INTERRUPT);
     } else {
-        dev->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
+        pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
+                                     PCI_STATUS_INTERRUPT);
     }
 }
 
@@ -413,7 +415,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
      * in irq_state which we are saving.
      * This makes us compatible with old devices
      * which never set or clear this bit. */
-    s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
+    pci_word_test_and_clear_mask(s->config + PCI_STATUS, PCI_STATUS_INTERRUPT);
     vmstate_save_state(f, pci_get_vmstate(s), s);
     /* Restore the interrupt status bit. */
     pci_update_irq_status(s);
@@ -626,7 +628,7 @@ static void pci_init_cmask(PCIDevice *dev)
 {
     pci_set_word(dev->cmask + PCI_VENDOR_ID, 0xffff);
     pci_set_word(dev->cmask + PCI_DEVICE_ID, 0xffff);
-    dev->cmask[PCI_STATUS] = PCI_STATUS_CAP_LIST;
+    pci_set_word(dev->cmask + PCI_STATUS, PCI_STATUS_CAP_LIST);
     dev->cmask[PCI_REVISION_ID] = 0xff;
     dev->cmask[PCI_CLASS_PROG] = 0xff;
     pci_set_word(dev->cmask + PCI_CLASS_DEVICE, 0xffff);
@@ -1793,8 +1795,9 @@ static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id,
 {
     uint8_t next, prev;
 
-    if (!(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST))
+    if (!(pci_get_word(pdev->config + PCI_STATUS) & PCI_STATUS_CAP_LIST)) {
         return 0;
+    }
 
     for (prev = PCI_CAPABILITY_LIST; (next = pdev->config[prev]);
          prev = next + PCI_CAP_LIST_NEXT)
@@ -1900,7 +1903,7 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
     config[PCI_CAP_LIST_ID] = cap_id;
     config[PCI_CAP_LIST_NEXT] = pdev->config[PCI_CAPABILITY_LIST];
     pdev->config[PCI_CAPABILITY_LIST] = offset;
-    pdev->config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
+    pci_word_test_and_set_mask(pdev->config + PCI_STATUS, PCI_STATUS_CAP_LIST);
     memset(pdev->used + offset, 0xFF, size);
     /* Make capability read-only by default */
     memset(pdev->wmask + offset, 0, size);
@@ -1923,8 +1926,10 @@ void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size)
     memset(pdev->cmask + offset, 0, size);
     memset(pdev->used + offset, 0, size);
 
-    if (!pdev->config[PCI_CAPABILITY_LIST])
-        pdev->config[PCI_STATUS] &= ~PCI_STATUS_CAP_LIST;
+    if (!pdev->config[PCI_CAPABILITY_LIST]) {
+        pci_word_test_and_clear_mask(pdev->config + PCI_STATUS,
+                                     PCI_STATUS_CAP_LIST);
+    }
 }
 
 /* Reserve space for capability at a known offset (to call after load). */
-- 
1.7.1.1

[Qemu-devel] [PATCH 4/4] pci: clean up of pci status register

[Isaku Yamahata]

This patch refine the initialization/reset of
pci status registers.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/pci.c |   40 ++++++++++++++++++++++++++++++++++++++--
 1 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index a16e763..184db6c 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -154,6 +154,9 @@ static void pci_device_reset(PCIDevice *dev)
     pci_word_test_and_clear_mask(dev->config + PCI_COMMAND,
                                  pci_get_word(dev->wmask + PCI_COMMAND) |
                                  pci_get_word(dev->w1cmask + PCI_COMMAND));
+    pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
+                                 pci_get_word(dev->wmask + PCI_STATUS) |
+                                 pci_get_word(dev->w1cmask + PCI_STATUS));
     dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
     dev->config[PCI_INTERRUPT_LINE] = 0x0;
     for (r = 0; r < PCI_NUM_REGIONS; ++r) {
@@ -636,7 +639,7 @@ static void pci_init_cmask(PCIDevice *dev)
     dev->cmask[PCI_CAPABILITY_LIST] = 0xff;
 }
 
-static void pci_init_wmask(PCIDevice *dev)
+static void pci_init_wmask_w1cmask(PCIDevice *dev)
 {
     int config_size = pci_config_size(dev);
 
@@ -691,6 +694,39 @@ static void pci_init_wmask(PCIDevice *dev)
                  PCI_COMMAND_MASTER | PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
                  PCI_COMMAND_INTX_DISABLE);
 
+    /*
+     * bit 0-2: reserved
+     * bit 3: PCI_STATUS_INTERRUPT: RO
+     * bit 4: PCI_STATUS_CAP_LIST: RO
+     * bit 5: PCI_STATUS_66MHZ: RO
+     * bit 6: PCI_STATUS_UDF: reserved (PCI 2.2-)
+     * bit 7: PCI_STATUS_FAST_BACK: RO
+     * bit 8: PCI_STATUS_PARITY
+     *        type 0: RW for bus master
+     *        type 1: RW1C
+     * bit 9-10: PCI_STATUS_DEVSEL: RO
+     * bit 11: PCI_STATUS_SIG_TARGET_ABORT
+     *         type 0: RW1C for targets that is capable of terminating
+     *                 a transaction.
+     *         type 1: RW1C
+     * bit 12: PCI_STATUS_REC_TARGET_ABORT
+     *         type 0: RW1C for masters
+     *         type 1: RW1C
+     * bit 13: PCI_STATUS_REC_MASTER_ABORT
+     *         type 0: RW1C for masters
+     *         type 1: RW1C
+     * bit 14: PCI_STATUS_SIG_SYSTEM_ERROR
+     *         type 0: RW1C with execptions
+     *         type 1: RW1C
+     * bit : PCI_STATUS_DETECTED_PARITY: RW1C
+     *
+     * It's safe to set w1mask even for RO.
+     */
+    pci_set_word(dev->w1cmask + PCI_STATUS,
+                 PCI_STATUS_PARITY | PCI_STATUS_SIG_TARGET_ABORT |
+                 PCI_STATUS_REC_TARGET_ABORT | PCI_STATUS_REC_MASTER_ABORT |
+                 PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_DETECTED_PARITY);
+
     memset(dev->wmask + PCI_CONFIG_HEADER_SIZE, 0xff,
            config_size - PCI_CONFIG_HEADER_SIZE);
 }
@@ -821,7 +857,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
         pci_set_default_subsystem_id(pci_dev);
     }
     pci_init_cmask(pci_dev);
-    pci_init_wmask(pci_dev);
+    pci_init_wmask_w1cmask(pci_dev);
     if (is_bridge) {
         pci_init_wmask_bridge(pci_dev);
     }
-- 
1.7.1.1