[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] pci_bridge: factor out common code
From: |
Alex Williamson |
Subject: |
Re: [Qemu-devel] [PATCH] pci_bridge: factor out common code |
Date: |
Mon, 04 Mar 2013 08:45:14 -0700 |
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 <address@hidden>
> ---
>
> 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