| Message ID | 1254514577-11896-22-git-send-email-yamahata@valinux.co.jp |
|---|---|
| State | Superseded |
| Headers | show |
On Sat, Oct 03, 2009 at 05:16:13AM +0900, Isaku Yamahata wrote: > header type of 01 has differenct BAR to type 00. > It has only BAR0,1 and expantion rom whose offset address > is different from type 00 one. There are some typos in the comment > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> > --- > hw/pci.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++---------- > hw/pci.h | 1 + > 2 files changed, 56 insertions(+), 11 deletions(-) > > diff --git a/hw/pci.c b/hw/pci.c > index ed9b785..b708d71 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -498,6 +498,35 @@ int pci_unregister_device(PCIDevice *pci_dev) > return 0; > } > > +#define PCI_BAR_INVALID UINT32_MAX Just use -1 to report error. > +static uint32_t pci_bar_config_offset(PCIDevice *d, int region_num) Why uint32_t? I think it should be just int. Please add small comment documenting what function does. > +{ This functions does too much sanity checking, which is where all the complexity comes from. It's not the best place to do sanity checking: do it when adding region, if you must. So this should be simply: static int pci_bar_config_offset(PCIDevice *d, int region_num) { if (region_num != PCI_ROM_ADDRESS) return PCI_BASE_ADDRESS_0 + region_num * 4; return (d->config[PCI_HEADER_TYPE] & PCI_HEADER_TYPE_BRIDGE) ? PCI_ROM_ADDRESS : PCI_BRIDGE_ROM_ADDRESS; } > + switch (d->config[PCI_HEADER_TYPE] & ~PCI_HEADER_TYPE_MULTI_FUNCTION) { > + case PCI_HEADER_TYPE_NORMAL: > + /* BAR 0-5 and Expantion ROM*/ Nit: space before */ > + if (region_num < PCI_ROM_SLOT) { > + return PCI_BASE_ADDRESS_0 + region_num * 4; > + } else if (region_num == PCI_ROM_SLOT) { > + return PCI_ROM_ADDRESS; > + } > + break; > + case PCI_HEADER_TYPE_BRIDGE: > + /* BAR 0-1 and Expantion ROM */ typo > + if (region_num < 2) { what's 2? > + return PCI_BASE_ADDRESS_0 + region_num * 4; > + } else if (region_num == PCI_ROM_SLOT) { > + return PCI_ROM_ADDRESS1; > + } > + break; > + case PCI_HEADER_TYPE_CARDBUS: the last line is useless > + default: > + break; > + } > + fprintf(stderr, "ERROR: %s: unknow PCI config header type %d or bar %d\n", > + __func__, d->config[PCI_HEADER_TYPE], region_num); > + return PCI_BAR_INVALID; Caller does not seem to check PCI_BAR_INVALID. > +} > + > void pci_register_bar(PCIDevice *pci_dev, int region_num, > pcibus_t size, int type, > PCIMapIORegionFunc *map_func) > @@ -521,13 +550,11 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, > r->type = type; > r->map_func = map_func; > > + addr = pci_bar_config_offset(pci_dev, region_num); > wmask = ~(size - 1); > if (region_num == PCI_ROM_SLOT) { > - addr = 0x30; > /* ROM enable bit is writeable */ > wmask |= PCI_ROM_ADDRESS_ENABLE; > - } else { > - addr = 0x10 + region_num * 4; > } > pci_set_long(pci_dev->config + addr, type); > if (pci_bar_is_64bit(r)) { > @@ -549,11 +576,9 @@ static void pci_update_mappings(PCIDevice *d) > cmd = pci_get_word(d->config + PCI_COMMAND); > for(i = 0; i < PCI_NUM_REGIONS; i++) { > r = &d->io_regions[i]; > - if (i == PCI_ROM_SLOT) { > - config_ofs = 0x30; > - } else { > - config_ofs = 0x10 + i * 4; > - } > + kill extra empty line > + config_ofs = pci_bar_config_offset(d, i); > + > if (r->size != 0) { > if (r->type & PCI_ADDRESS_SPACE_IO) { > if (cmd & PCI_COMMAND_IO) { > @@ -1133,10 +1158,29 @@ static void pci_bridge_write_config(PCIDevice *d, > uint32_t address, uint32_t val, int len) > { > PCIBridge *s = (PCIBridge *)d; > + PCIBus *bus = s->bus; > + uint8_t *orig = pci_write_config_init(d, address, len); > + > + pci_default_write_config_common(d, address, val, len); > > - pci_default_write_config(d, address, val, len); > - s->bus->bus_num = d->config[PCI_SECONDARY_BUS]; > - s->bus->sub_bus = d->config[PCI_SUBORDINATE_BUS]; > + if (pci_config_changed(orig, d->config, address, len, > + PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_2 + 4) || > + pci_config_changed_with_size(orig, d->config, address, len, > + PCI_ROM_ADDRESS1, 4) || > + pci_config_changed_with_size(orig, d->config, address, len, > + PCI_COMMAND, 1)) { > + pci_update_mappings(d); > + } > + if (pci_config_changed_with_size(orig, d->config, address, len, > + PCI_SECONDARY_BUS, 1)) { > + bus->bus_num = d->config[PCI_SECONDARY_BUS]; > + } > + if (pci_config_changed_with_size(orig, d->config, address, len, > + PCI_SECONDARY_BUS, 1)) { > + bus->sub_bus = d->config[PCI_SUBORDINATE_BUS]; > + } So simply d->config[PCI_SUBORDINATE_BUS] != orig[PCI_SUBORDINATE_BUS] would be enough if we keep orig and config in sync as I suggested in my previous post. > + > + pci_write_config_done(orig); > } > > PCIBus *pci_find_bus(PCIBus *bus, int bus_num) > diff --git a/hw/pci.h b/hw/pci.h > index f19b7a8..645dacd 100644 > --- a/hw/pci.h > +++ b/hw/pci.h > @@ -174,6 +174,7 @@ static inline int pci_bar_is_64bit(const PCIIORegion *r) > > /* Header type 1 (PCI-to-PCI bridges) */ > #define PCI_SUBORDINATE_BUS 0x1a /* Highest bus number behind the bridge */ > +#define PCI_ROM_ADDRESS1 0x38 /* Same as PCI_ROM_ADDRESS, but for htype 1 */ PCI_BRIDGE_ROM_ADDRESS would be clearer. Also don't mix your own macros and ones imported from pci_regs.h > > /* Size of the standard PCI config header */ > #define PCI_CONFIG_HEADER_SIZE 0x40 > -- > 1.6.0.2 > >
On Sun, Oct 04, 2009 at 12:53:45PM +0200, Michael S. Tsirkin wrote: > On Sat, Oct 03, 2009 at 05:16:13AM +0900, Isaku Yamahata wrote: > > header type of 01 has differenct BAR to type 00. > > It has only BAR0,1 and expantion rom whose offset address > > is different from type 00 one. > > There are some typos in the comment > > > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> > > --- > > hw/pci.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++---------- > > hw/pci.h | 1 + > > 2 files changed, 56 insertions(+), 11 deletions(-) > > > > diff --git a/hw/pci.c b/hw/pci.c > > index ed9b785..b708d71 100644 > > --- a/hw/pci.c > > +++ b/hw/pci.c > > @@ -498,6 +498,35 @@ int pci_unregister_device(PCIDevice *pci_dev) > > return 0; > > } > > > > +#define PCI_BAR_INVALID UINT32_MAX > > Just use -1 to report error. > > > +static uint32_t pci_bar_config_offset(PCIDevice *d, int region_num) > > Why uint32_t? I think it should be just int. > Please add small comment documenting what function does. > > > +{ > > This functions does too much sanity checking, which is where > all the complexity comes from. It's not the best place to do > sanity checking: do it when adding region, if you must. > > So this should be simply: > > static int pci_bar_config_offset(PCIDevice *d, int region_num) > { > if (region_num != PCI_ROM_ADDRESS) > return PCI_BASE_ADDRESS_0 + region_num * 4; > > return (d->config[PCI_HEADER_TYPE] & PCI_HEADER_TYPE_BRIDGE) ? > PCI_ROM_ADDRESS : PCI_BRIDGE_ROM_ADDRESS; > } > > > > + switch (d->config[PCI_HEADER_TYPE] & ~PCI_HEADER_TYPE_MULTI_FUNCTION) { > > + case PCI_HEADER_TYPE_NORMAL: > > + /* BAR 0-5 and Expantion ROM*/ > > Nit: space before */ > > > + if (region_num < PCI_ROM_SLOT) { > > + return PCI_BASE_ADDRESS_0 + region_num * 4; > > + } else if (region_num == PCI_ROM_SLOT) { > > + return PCI_ROM_ADDRESS; > > + } > > + break; > > + case PCI_HEADER_TYPE_BRIDGE: > > + /* BAR 0-1 and Expantion ROM */ > > typo > > > + if (region_num < 2) { > > what's 2? > > > + return PCI_BASE_ADDRESS_0 + region_num * 4; > > + } else if (region_num == PCI_ROM_SLOT) { > > + return PCI_ROM_ADDRESS1; > > + } > > + break; > > + case PCI_HEADER_TYPE_CARDBUS: > > the last line is useless > > > + default: > > + break; > > + } > > + fprintf(stderr, "ERROR: %s: unknow PCI config header type %d or bar %d\n", > > + __func__, d->config[PCI_HEADER_TYPE], region_num); > > + return PCI_BAR_INVALID; > > Caller does not seem to check PCI_BAR_INVALID. > > > +} > > + > > void pci_register_bar(PCIDevice *pci_dev, int region_num, > > pcibus_t size, int type, > > PCIMapIORegionFunc *map_func) > > @@ -521,13 +550,11 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, > > r->type = type; > > r->map_func = map_func; > > > > + addr = pci_bar_config_offset(pci_dev, region_num); > > wmask = ~(size - 1); > > if (region_num == PCI_ROM_SLOT) { > > - addr = 0x30; > > /* ROM enable bit is writeable */ > > wmask |= PCI_ROM_ADDRESS_ENABLE; > > - } else { > > - addr = 0x10 + region_num * 4; > > } > > pci_set_long(pci_dev->config + addr, type); > > if (pci_bar_is_64bit(r)) { > > @@ -549,11 +576,9 @@ static void pci_update_mappings(PCIDevice *d) > > cmd = pci_get_word(d->config + PCI_COMMAND); > > for(i = 0; i < PCI_NUM_REGIONS; i++) { > > r = &d->io_regions[i]; > > - if (i == PCI_ROM_SLOT) { > > - config_ofs = 0x30; > > - } else { > > - config_ofs = 0x10 + i * 4; > > - } > > + > > kill extra empty line > > > + config_ofs = pci_bar_config_offset(d, i); > > + > > if (r->size != 0) { > > if (r->type & PCI_ADDRESS_SPACE_IO) { > > if (cmd & PCI_COMMAND_IO) { > > @@ -1133,10 +1158,29 @@ static void pci_bridge_write_config(PCIDevice *d, > > uint32_t address, uint32_t val, int len) > > { > > PCIBridge *s = (PCIBridge *)d; > > + PCIBus *bus = s->bus; > > + uint8_t *orig = pci_write_config_init(d, address, len); > > + > > + pci_default_write_config_common(d, address, val, len); > > > > - pci_default_write_config(d, address, val, len); > > - s->bus->bus_num = d->config[PCI_SECONDARY_BUS]; > > - s->bus->sub_bus = d->config[PCI_SUBORDINATE_BUS]; > > + if (pci_config_changed(orig, d->config, address, len, > > + PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_2 + 4) || > > + pci_config_changed_with_size(orig, d->config, address, len, > > + PCI_ROM_ADDRESS1, 4) || > > + pci_config_changed_with_size(orig, d->config, address, len, > > + PCI_COMMAND, 1)) { > > + pci_update_mappings(d); > > + } > > + if (pci_config_changed_with_size(orig, d->config, address, len, > > + PCI_SECONDARY_BUS, 1)) { > > + bus->bus_num = d->config[PCI_SECONDARY_BUS]; > > + } > > + if (pci_config_changed_with_size(orig, d->config, address, len, > > + PCI_SECONDARY_BUS, 1)) { > > + bus->sub_bus = d->config[PCI_SUBORDINATE_BUS]; > > + } > > So simply > d->config[PCI_SUBORDINATE_BUS] != orig[PCI_SUBORDINATE_BUS] > would be enough if we keep orig and config in sync > as I suggested in my previous post. > > > + > > + pci_write_config_done(orig); > > } > > > > PCIBus *pci_find_bus(PCIBus *bus, int bus_num) > > diff --git a/hw/pci.h b/hw/pci.h > > index f19b7a8..645dacd 100644 > > --- a/hw/pci.h > > +++ b/hw/pci.h > > @@ -174,6 +174,7 @@ static inline int pci_bar_is_64bit(const PCIIORegion *r) > > > > /* Header type 1 (PCI-to-PCI bridges) */ > > #define PCI_SUBORDINATE_BUS 0x1a /* Highest bus number behind the bridge */ > > +#define PCI_ROM_ADDRESS1 0x38 /* Same as PCI_ROM_ADDRESS, but for htype 1 */ > > PCI_BRIDGE_ROM_ADDRESS would be clearer. > Also don't mix your own macros and ones imported from pci_regs.h Unfortunately PCI_ROM_ADDRESS1 is the one defined in pci_regs.h. So PCI_ROM_ADDRESS1 is reasonable. > > /* Size of the standard PCI config header */ > > #define PCI_CONFIG_HEADER_SIZE 0x40 >
On Mon, Oct 05, 2009 at 06:47:39PM +0900, Isaku Yamahata wrote: > On Sun, Oct 04, 2009 at 12:53:45PM +0200, Michael S. Tsirkin wrote: > > On Sat, Oct 03, 2009 at 05:16:13AM +0900, Isaku Yamahata wrote: > > > header type of 01 has differenct BAR to type 00. > > > It has only BAR0,1 and expantion rom whose offset address > > > is different from type 00 one. > > > > There are some typos in the comment > > > > > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> > > > --- > > > hw/pci.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++---------- > > > hw/pci.h | 1 + > > > 2 files changed, 56 insertions(+), 11 deletions(-) > > > > > > diff --git a/hw/pci.c b/hw/pci.c > > > index ed9b785..b708d71 100644 > > > --- a/hw/pci.c > > > +++ b/hw/pci.c > > > @@ -498,6 +498,35 @@ int pci_unregister_device(PCIDevice *pci_dev) > > > return 0; > > > } > > > > > > +#define PCI_BAR_INVALID UINT32_MAX > > > > Just use -1 to report error. > > > > > +static uint32_t pci_bar_config_offset(PCIDevice *d, int region_num) > > > > Why uint32_t? I think it should be just int. > > Please add small comment documenting what function does. > > > > > +{ > > > > This functions does too much sanity checking, which is where > > all the complexity comes from. It's not the best place to do > > sanity checking: do it when adding region, if you must. > > > > So this should be simply: > > > > static int pci_bar_config_offset(PCIDevice *d, int region_num) > > { > > if (region_num != PCI_ROM_ADDRESS) > > return PCI_BASE_ADDRESS_0 + region_num * 4; > > > > return (d->config[PCI_HEADER_TYPE] & PCI_HEADER_TYPE_BRIDGE) ? > > PCI_ROM_ADDRESS : PCI_BRIDGE_ROM_ADDRESS; > > } > > > > > > > + switch (d->config[PCI_HEADER_TYPE] & ~PCI_HEADER_TYPE_MULTI_FUNCTION) { > > > + case PCI_HEADER_TYPE_NORMAL: > > > + /* BAR 0-5 and Expantion ROM*/ > > > > Nit: space before */ > > > > > + if (region_num < PCI_ROM_SLOT) { > > > + return PCI_BASE_ADDRESS_0 + region_num * 4; > > > + } else if (region_num == PCI_ROM_SLOT) { > > > + return PCI_ROM_ADDRESS; > > > + } > > > + break; > > > + case PCI_HEADER_TYPE_BRIDGE: > > > + /* BAR 0-1 and Expantion ROM */ > > > > typo > > > > > + if (region_num < 2) { > > > > what's 2? > > > > > + return PCI_BASE_ADDRESS_0 + region_num * 4; > > > + } else if (region_num == PCI_ROM_SLOT) { > > > + return PCI_ROM_ADDRESS1; > > > + } > > > + break; > > > + case PCI_HEADER_TYPE_CARDBUS: > > > > the last line is useless > > > > > + default: > > > + break; > > > + } > > > + fprintf(stderr, "ERROR: %s: unknow PCI config header type %d or bar %d\n", > > > + __func__, d->config[PCI_HEADER_TYPE], region_num); > > > + return PCI_BAR_INVALID; > > > > Caller does not seem to check PCI_BAR_INVALID. > > > > > +} > > > + > > > void pci_register_bar(PCIDevice *pci_dev, int region_num, > > > pcibus_t size, int type, > > > PCIMapIORegionFunc *map_func) > > > @@ -521,13 +550,11 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, > > > r->type = type; > > > r->map_func = map_func; > > > > > > + addr = pci_bar_config_offset(pci_dev, region_num); > > > wmask = ~(size - 1); > > > if (region_num == PCI_ROM_SLOT) { > > > - addr = 0x30; > > > /* ROM enable bit is writeable */ > > > wmask |= PCI_ROM_ADDRESS_ENABLE; > > > - } else { > > > - addr = 0x10 + region_num * 4; > > > } > > > pci_set_long(pci_dev->config + addr, type); > > > if (pci_bar_is_64bit(r)) { > > > @@ -549,11 +576,9 @@ static void pci_update_mappings(PCIDevice *d) > > > cmd = pci_get_word(d->config + PCI_COMMAND); > > > for(i = 0; i < PCI_NUM_REGIONS; i++) { > > > r = &d->io_regions[i]; > > > - if (i == PCI_ROM_SLOT) { > > > - config_ofs = 0x30; > > > - } else { > > > - config_ofs = 0x10 + i * 4; > > > - } > > > + > > > > kill extra empty line > > > > > + config_ofs = pci_bar_config_offset(d, i); > > > + > > > if (r->size != 0) { > > > if (r->type & PCI_ADDRESS_SPACE_IO) { > > > if (cmd & PCI_COMMAND_IO) { > > > @@ -1133,10 +1158,29 @@ static void pci_bridge_write_config(PCIDevice *d, > > > uint32_t address, uint32_t val, int len) > > > { > > > PCIBridge *s = (PCIBridge *)d; > > > + PCIBus *bus = s->bus; > > > + uint8_t *orig = pci_write_config_init(d, address, len); > > > + > > > + pci_default_write_config_common(d, address, val, len); > > > > > > - pci_default_write_config(d, address, val, len); > > > - s->bus->bus_num = d->config[PCI_SECONDARY_BUS]; > > > - s->bus->sub_bus = d->config[PCI_SUBORDINATE_BUS]; > > > + if (pci_config_changed(orig, d->config, address, len, > > > + PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_2 + 4) || > > > + pci_config_changed_with_size(orig, d->config, address, len, > > > + PCI_ROM_ADDRESS1, 4) || > > > + pci_config_changed_with_size(orig, d->config, address, len, > > > + PCI_COMMAND, 1)) { > > > + pci_update_mappings(d); > > > + } > > > + if (pci_config_changed_with_size(orig, d->config, address, len, > > > + PCI_SECONDARY_BUS, 1)) { > > > + bus->bus_num = d->config[PCI_SECONDARY_BUS]; > > > + } > > > + if (pci_config_changed_with_size(orig, d->config, address, len, > > > + PCI_SECONDARY_BUS, 1)) { > > > + bus->sub_bus = d->config[PCI_SUBORDINATE_BUS]; > > > + } > > > > So simply > > d->config[PCI_SUBORDINATE_BUS] != orig[PCI_SUBORDINATE_BUS] > > would be enough if we keep orig and config in sync > > as I suggested in my previous post. > > > > > + > > > + pci_write_config_done(orig); > > > } > > > > > > PCIBus *pci_find_bus(PCIBus *bus, int bus_num) > > > diff --git a/hw/pci.h b/hw/pci.h > > > index f19b7a8..645dacd 100644 > > > --- a/hw/pci.h > > > +++ b/hw/pci.h > > > @@ -174,6 +174,7 @@ static inline int pci_bar_is_64bit(const PCIIORegion *r) > > > > > > /* Header type 1 (PCI-to-PCI bridges) */ > > > #define PCI_SUBORDINATE_BUS 0x1a /* Highest bus number behind the bridge */ > > > +#define PCI_ROM_ADDRESS1 0x38 /* Same as PCI_ROM_ADDRESS, but for htype 1 */ > > > > PCI_BRIDGE_ROM_ADDRESS would be clearer. > > Also don't mix your own macros and ones imported from pci_regs.h > > Unfortunately PCI_ROM_ADDRESS1 is the one defined in pci_regs.h. > So PCI_ROM_ADDRESS1 is reasonable. You are right. Sorry about the noise. > > > > /* Size of the standard PCI config header */ > > > #define PCI_CONFIG_HEADER_SIZE 0x40 > > > > -- > yamahata
diff --git a/hw/pci.c b/hw/pci.c index ed9b785..b708d71 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -498,6 +498,35 @@ int pci_unregister_device(PCIDevice *pci_dev) return 0; } +#define PCI_BAR_INVALID UINT32_MAX +static uint32_t pci_bar_config_offset(PCIDevice *d, int region_num) +{ + switch (d->config[PCI_HEADER_TYPE] & ~PCI_HEADER_TYPE_MULTI_FUNCTION) { + case PCI_HEADER_TYPE_NORMAL: + /* BAR 0-5 and Expantion ROM*/ + if (region_num < PCI_ROM_SLOT) { + return PCI_BASE_ADDRESS_0 + region_num * 4; + } else if (region_num == PCI_ROM_SLOT) { + return PCI_ROM_ADDRESS; + } + break; + case PCI_HEADER_TYPE_BRIDGE: + /* BAR 0-1 and Expantion ROM */ + if (region_num < 2) { + return PCI_BASE_ADDRESS_0 + region_num * 4; + } else if (region_num == PCI_ROM_SLOT) { + return PCI_ROM_ADDRESS1; + } + break; + case PCI_HEADER_TYPE_CARDBUS: + default: + break; + } + fprintf(stderr, "ERROR: %s: unknow PCI config header type %d or bar %d\n", + __func__, d->config[PCI_HEADER_TYPE], region_num); + return PCI_BAR_INVALID; +} + void pci_register_bar(PCIDevice *pci_dev, int region_num, pcibus_t size, int type, PCIMapIORegionFunc *map_func) @@ -521,13 +550,11 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, r->type = type; r->map_func = map_func; + addr = pci_bar_config_offset(pci_dev, region_num); wmask = ~(size - 1); if (region_num == PCI_ROM_SLOT) { - addr = 0x30; /* ROM enable bit is writeable */ wmask |= PCI_ROM_ADDRESS_ENABLE; - } else { - addr = 0x10 + region_num * 4; } pci_set_long(pci_dev->config + addr, type); if (pci_bar_is_64bit(r)) { @@ -549,11 +576,9 @@ static void pci_update_mappings(PCIDevice *d) cmd = pci_get_word(d->config + PCI_COMMAND); for(i = 0; i < PCI_NUM_REGIONS; i++) { r = &d->io_regions[i]; - if (i == PCI_ROM_SLOT) { - config_ofs = 0x30; - } else { - config_ofs = 0x10 + i * 4; - } + + config_ofs = pci_bar_config_offset(d, i); + if (r->size != 0) { if (r->type & PCI_ADDRESS_SPACE_IO) { if (cmd & PCI_COMMAND_IO) { @@ -1133,10 +1158,29 @@ static void pci_bridge_write_config(PCIDevice *d, uint32_t address, uint32_t val, int len) { PCIBridge *s = (PCIBridge *)d; + PCIBus *bus = s->bus; + uint8_t *orig = pci_write_config_init(d, address, len); + + pci_default_write_config_common(d, address, val, len); - pci_default_write_config(d, address, val, len); - s->bus->bus_num = d->config[PCI_SECONDARY_BUS]; - s->bus->sub_bus = d->config[PCI_SUBORDINATE_BUS]; + if (pci_config_changed(orig, d->config, address, len, + PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_2 + 4) || + pci_config_changed_with_size(orig, d->config, address, len, + PCI_ROM_ADDRESS1, 4) || + pci_config_changed_with_size(orig, d->config, address, len, + PCI_COMMAND, 1)) { + pci_update_mappings(d); + } + if (pci_config_changed_with_size(orig, d->config, address, len, + PCI_SECONDARY_BUS, 1)) { + bus->bus_num = d->config[PCI_SECONDARY_BUS]; + } + if (pci_config_changed_with_size(orig, d->config, address, len, + PCI_SECONDARY_BUS, 1)) { + bus->sub_bus = d->config[PCI_SUBORDINATE_BUS]; + } + + pci_write_config_done(orig); } PCIBus *pci_find_bus(PCIBus *bus, int bus_num) diff --git a/hw/pci.h b/hw/pci.h index f19b7a8..645dacd 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -174,6 +174,7 @@ static inline int pci_bar_is_64bit(const PCIIORegion *r) /* Header type 1 (PCI-to-PCI bridges) */ #define PCI_SUBORDINATE_BUS 0x1a /* Highest bus number behind the bridge */ +#define PCI_ROM_ADDRESS1 0x38 /* Same as PCI_ROM_ADDRESS, but for htype 1 */ /* Size of the standard PCI config header */ #define PCI_CONFIG_HEADER_SIZE 0x40
header type of 01 has differenct BAR to type 00. It has only BAR0,1 and expantion rom whose offset address is different from type 00 one. Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> --- hw/pci.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++---------- hw/pci.h | 1 + 2 files changed, 56 insertions(+), 11 deletions(-)