diff mbox

pci_bridge: factor out common code

Message ID 20130304092839.GA30441@redhat.com
State New
Headers show

Commit Message

Michael S. Tsirkin March 4, 2013, 9:28 a.m. UTC
Reuse common code in pcie_port, override the hardwired-to-0
bits per PCI Express spec.
No functional change but makes the code easier to follow.

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

Alex, I applied the following on top of your patch and
did a dump of the config space to verify nothing changed.
Could you please also confirm this does not conflict with your vga
assignment work?

 hw/pci/pci.c        | 14 ++++----------
 hw/pci/pci_bridge.h |  7 +++++++
 hw/pci/pcie_port.c  | 22 +++++++++++++---------
 3 files changed, 24 insertions(+), 19 deletions(-)

Comments

Alex Williamson March 4, 2013, 3:45 p.m. UTC | #1
On Mon, 2013-03-04 at 11:28 +0200, Michael S. Tsirkin wrote:
> Reuse common code in pcie_port, override the hardwired-to-0
> bits per PCI Express spec.
> No functional change but makes the code easier to follow.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> Alex, I applied the following on top of your patch and
> did a dump of the config space to verify nothing changed.
> Could you please also confirm this does not conflict with your vga
> assignment work?
> 
>  hw/pci/pci.c        | 14 ++++----------
>  hw/pci/pci_bridge.h |  7 +++++++
>  hw/pci/pcie_port.c  | 22 +++++++++++++---------
>  3 files changed, 24 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index a881602..8772707 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -668,16 +668,10 @@ static void pci_init_mask_bridge(PCIDevice *d)
>      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 */
> -#define  PCI_BRIDGE_CTL_SEC_DISCARD	0x200	/* Secondary discard timer */
> -#define  PCI_BRIDGE_CTL_DISCARD_STATUS	0x400	/* Discard timer status */
> -#define  PCI_BRIDGE_CTL_DISCARD_SERR	0x800	/* Discard timer SERR# enable */
> -/*
> - * TODO: Bridges default to 10-bit VGA decoding but we currently only
> - * implement 16-bit decoding (no alias support).
> - */
> +    /*
> +     * TODO: Bridges default to 10-bit VGA decoding but we currently only
> +     * implement 16-bit decoding (no alias support).
> +     */
>      pci_set_word(d->wmask + PCI_BRIDGE_CONTROL,
>                   PCI_BRIDGE_CTL_PARITY |
>                   PCI_BRIDGE_CTL_SERR |
> diff --git a/hw/pci/pci_bridge.h b/hw/pci/pci_bridge.h
> index 455cb66..9d25c1b 100644
> --- a/hw/pci/pci_bridge.h
> +++ b/hw/pci/pci_bridge.h
> @@ -55,6 +55,13 @@ void pci_bridge_exitfn(PCIDevice *pci_dev);
>  void pci_bridge_map_irq(PCIBridge *br, const char* bus_name,
>                          pci_map_irq_fn map_irq);
>  
> +/* 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 */
> +#define  PCI_BRIDGE_CTL_SEC_DISCARD	0x200	/* Secondary discard timer */
> +#define  PCI_BRIDGE_CTL_DISCARD_STATUS	0x400	/* Discard timer status */
> +#define  PCI_BRIDGE_CTL_DISCARD_SERR	0x800	/* Discard timer SERR# enable */
> +
>  #endif  /* QEMU_PCI_BRIDGE_H */
>  /*
>   * Local variables:
> diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
> index 1be107b..5d39a2d 100644
> --- a/hw/pci/pcie_port.c
> +++ b/hw/pci/pcie_port.c
> @@ -27,15 +27,17 @@ void pcie_port_init_reg(PCIDevice *d)
>      pci_set_word(d->config + PCI_STATUS, 0);
>      pci_set_word(d->config + PCI_SEC_STATUS, 0);
>  
> -    /* Unlike conventional pci bridge, some bits are hardwired to 0. */
> -#define  PCI_BRIDGE_CTL_VGA_16BIT       0x10    /* VGA 16-bit decode */
> -    pci_set_word(d->wmask + PCI_BRIDGE_CONTROL,
> -                 PCI_BRIDGE_CTL_PARITY |
> -                 PCI_BRIDGE_CTL_ISA |
> -                 PCI_BRIDGE_CTL_VGA |
> -                 PCI_BRIDGE_CTL_VGA_16BIT | /* Req, but no alias support yet */
> -                 PCI_BRIDGE_CTL_SERR |
> -                 PCI_BRIDGE_CTL_BUS_RESET);
> +    /*
> +     * Unlike conventional pci bridge, for some bits the spec states:
> +     * Does not apply to PCI Express and must be hardwired to 0.
> +     */
> +    pci_word_test_and_clear_mask(d->wmask + PCI_BRIDGE_CONTROL,
> +                                 PCI_BRIDGE_CTL_MASTER_ABORT |
> +                                 PCI_BRIDGE_CTL_FAST_BACK |
> +                                 PCI_BRIDGE_CTL_DISCARD |
> +                                 PCI_BRIDGE_CTL_SEC_DISCARD |
> +                                 PCI_BRIDGE_CTL_DISCARD_STATUS |
> +                                 PCI_BRIDGE_CTL_DISCARD_SERR);
>  }

Looks like no change, but I'll give it a test.  Clearing bits makes more
sense than writing it from scratch, especially given the comment.
Thanks,

Alex
diff mbox

Patch

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index a881602..8772707 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -668,16 +668,10 @@  static void pci_init_mask_bridge(PCIDevice *d)
     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 */
-#define  PCI_BRIDGE_CTL_SEC_DISCARD	0x200	/* Secondary discard timer */
-#define  PCI_BRIDGE_CTL_DISCARD_STATUS	0x400	/* Discard timer status */
-#define  PCI_BRIDGE_CTL_DISCARD_SERR	0x800	/* Discard timer SERR# enable */
-/*
- * TODO: Bridges default to 10-bit VGA decoding but we currently only
- * implement 16-bit decoding (no alias support).
- */
+    /*
+     * TODO: Bridges default to 10-bit VGA decoding but we currently only
+     * implement 16-bit decoding (no alias support).
+     */
     pci_set_word(d->wmask + PCI_BRIDGE_CONTROL,
                  PCI_BRIDGE_CTL_PARITY |
                  PCI_BRIDGE_CTL_SERR |
diff --git a/hw/pci/pci_bridge.h b/hw/pci/pci_bridge.h
index 455cb66..9d25c1b 100644
--- a/hw/pci/pci_bridge.h
+++ b/hw/pci/pci_bridge.h
@@ -55,6 +55,13 @@  void pci_bridge_exitfn(PCIDevice *pci_dev);
 void pci_bridge_map_irq(PCIBridge *br, const char* bus_name,
                         pci_map_irq_fn map_irq);
 
+/* 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 */
+#define  PCI_BRIDGE_CTL_SEC_DISCARD	0x200	/* Secondary discard timer */
+#define  PCI_BRIDGE_CTL_DISCARD_STATUS	0x400	/* Discard timer status */
+#define  PCI_BRIDGE_CTL_DISCARD_SERR	0x800	/* Discard timer SERR# enable */
+
 #endif  /* QEMU_PCI_BRIDGE_H */
 /*
  * Local variables:
diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
index 1be107b..5d39a2d 100644
--- a/hw/pci/pcie_port.c
+++ b/hw/pci/pcie_port.c
@@ -27,15 +27,17 @@  void pcie_port_init_reg(PCIDevice *d)
     pci_set_word(d->config + PCI_STATUS, 0);
     pci_set_word(d->config + PCI_SEC_STATUS, 0);
 
-    /* Unlike conventional pci bridge, some bits are hardwired to 0. */
-#define  PCI_BRIDGE_CTL_VGA_16BIT       0x10    /* VGA 16-bit decode */
-    pci_set_word(d->wmask + PCI_BRIDGE_CONTROL,
-                 PCI_BRIDGE_CTL_PARITY |
-                 PCI_BRIDGE_CTL_ISA |
-                 PCI_BRIDGE_CTL_VGA |
-                 PCI_BRIDGE_CTL_VGA_16BIT | /* Req, but no alias support yet */
-                 PCI_BRIDGE_CTL_SERR |
-                 PCI_BRIDGE_CTL_BUS_RESET);
+    /*
+     * Unlike conventional pci bridge, for some bits the spec states:
+     * Does not apply to PCI Express and must be hardwired to 0.
+     */
+    pci_word_test_and_clear_mask(d->wmask + PCI_BRIDGE_CONTROL,
+                                 PCI_BRIDGE_CTL_MASTER_ABORT |
+                                 PCI_BRIDGE_CTL_FAST_BACK |
+                                 PCI_BRIDGE_CTL_DISCARD |
+                                 PCI_BRIDGE_CTL_SEC_DISCARD |
+                                 PCI_BRIDGE_CTL_DISCARD_STATUS |
+                                 PCI_BRIDGE_CTL_DISCARD_SERR);
 }
 
 /**************************************************************************