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