On Mon, Oct 18, 2010 at 04:10:17PM +0900, Isaku Yamahata wrote: > On Mon, Oct 18, 2010 at 08:22:24AM +0200, Michael S. Tsirkin wrote: > > On Mon, Oct 18, 2010 at 12:17:46PM +0900, Isaku Yamahata wrote: > > > lower 4bits of base/limit register is RO, and > > > should not be modified on reset. > > > > > > Signed-off-by: Isaku Yamahata <yamah...@valinux.co.jp> > > > --- > > > hw/pci_bridge.c | 15 +++++++++------ > > > 1 files changed, 9 insertions(+), 6 deletions(-) > > > > > > diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c > > > index 638e3b3..7229610 100644 > > > --- a/hw/pci_bridge.c > > > +++ b/hw/pci_bridge.c > > > @@ -161,12 +161,15 @@ void pci_bridge_reset_reg(PCIDevice *dev) > > > conf[PCI_SUBORDINATE_BUS] = 0; > > > conf[PCI_SEC_LATENCY_TIMER] = 0; > > > > > > - conf[PCI_IO_BASE] = 0; > > > - conf[PCI_IO_LIMIT] = 0; > > > - pci_set_word(conf + PCI_MEMORY_BASE, 0); > > > - pci_set_word(conf + PCI_MEMORY_LIMIT, 0); > > > - pci_set_word(conf + PCI_PREF_MEMORY_BASE, 0); > > > - pci_set_word(conf + PCI_PREF_MEMORY_LIMIT, 0); > > > + pci_clear_bit_byte(conf + PCI_IO_BASE, PCI_IO_RANGE_MASK & 0xff); > > > + pci_clear_bit_byte(conf + PCI_IO_LIMIT, PCI_IO_RANGE_MASK & 0xff); > > > > No need for & 0xff and & 0xfffff here and below. > > gcc complains like this without them. > hw/pci_bridge.c:165: error: large integer implicitly truncated to unsigned > type
I see. > > > also, PCI spec also says that these registers' value > > is undefined after reset, so no need to clear them? > > If there's a reason, pls put it in comment. > > The spec says the lower bits are read-only and they have its meaning. Yes. but my question is why even touch io base/io limit at all in this function? It looks like guest can not rely on these being 0 after reset. > Anyway I'll add the reference as comment. > > For example. > > > 3.2.5.6 I/O Base Register and I/O Limit Register > > > > If the low four bits of the I/O Base and I/O Limit registers are 01h, > > then the bridge supports 32-bit I/O address decoding, and the I/O Base > > Upper 16 Bits and the I/O Limit Upper 16 Bits hold the upper 16 bits, > > corresponding to AD[31::16], of the 32-bit I/O Base and I/O Limit > > addresses respectively. In this case, system configuration software is > > permitted to locate the I/O address range supported by the bridge > > anywhere in the 4-GB I/O Space. Note that the 4-KB alignment and > > granularity restrictions still apply when the bridge supports 32-bit > > I/O addressing. > > table 3-7 I don't think this is required but up to you. > > > > > > > + pci_clear_bit_word(conf + PCI_MEMORY_BASE, PCI_MEMORY_RANGE_MASK & > > > 0xffff); > > > + pci_clear_bit_word(conf + PCI_MEMORY_LIMIT, > > > + PCI_MEMORY_RANGE_MASK & 0xffff); > > > + pci_clear_bit_word(conf + PCI_PREF_MEMORY_BASE, > > > + PCI_PREF_RANGE_MASK & 0xffff); > > > + pci_clear_bit_word(conf + PCI_PREF_MEMORY_LIMIT, > > > + PCI_PREF_RANGE_MASK & 0xffff); > > > pci_set_word(conf + PCI_PREF_BASE_UPPER32, 0); > > > pci_set_word(conf + PCI_PREF_LIMIT_UPPER32, 0); > > > > > > -- > > > 1.7.1.1 > > > > -- > yamahata
