[Qemu-devel] Re: [PATCH V5 27/29] pci/bridge: don't update bar mapping when bar2-5 is changed.

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Oct 09, 2009 at 03:29:00PM +0900, Isaku Yamahata wrote:
> header type of 01 has only BAR 0, 1 and ROM.
> So when bar2-5 of bridge is updated (In fact, they are used for
> different purpose), it isn't necessary to call pci_update_mappings().
> 
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

What is the motivation for all these changes?

Is there a good reason to optimize config writes into bridge?  I think
we might want to go in another direction and remove the orig mask in
default write config as well, just update mappings on any write in the
range 0x04 to 0x40.

I also think just calling pci_default_write_config from bridge
is a cleaner API than what is proposed with this
and previous patches.


> ---
>  hw/pci.c |   22 +++++++++++++++++++++-
>  hw/pci.h |    1 +
>  2 files changed, 22 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 37c2eed..3a17cd8 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -983,10 +983,30 @@ static void pci_bridge_write_config(PCIDevice *d,
>                               uint32_t address, uint32_t val, int len)
>  {
>      PCIBridge *s = (PCIBridge *)d;
> +    struct pci_config_update update;
>  
> -    pci_default_write_config(d, address, val, len);
> +    pci_write_config_init(&update, d, address, val, len);
> +    pci_write_config_update(&update);
> +    if (pci_config_changed(&update,
> +                           PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_2 + 4) ||
> +        pci_config_changed_with_size(&update, PCI_ROM_ADDRESS1, 4) ||
> +        pci_config_changed_with_size(&update, PCI_COMMAND, 1)) {
> +        pci_update_mappings(d);
> +    }
>      s->bus.bus_num = d->config[PCI_SECONDARY_BUS];
>      s->bus.sub_bus = d->config[PCI_SUBORDINATE_BUS];
> +
> +    /*
> +     * TODO:
> +     * Bridge IO/memory filter isn't emulated. (yet)
> +     * At least Linux doesn't depend on bridge filter at the moment,
> +     * so it boots happily without bridge filter emulation.
> +     * On the other hand, it needs to read/wrote to base/limit registers
> +     * accurately.
> +     *
> +     * If OS depending on bridge filter is found,
> +     * filtering will be implemented.
> +     */
>  }
>  
>  PCIBus *pci_find_bus(PCIBus *bus, int bus_num)
> diff --git a/hw/pci.h b/hw/pci.h
> index 59c65fc..3cd93e9 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -112,6 +112,7 @@ typedef struct PCIIORegion {
>  #define  PCI_HEADER_TYPE_CARDBUS	2
>  #define  PCI_HEADER_TYPE_MULTI_FUNCTION 0x80
>  #define PCI_BASE_ADDRESS_0	0x10	/* 32 bits */
> +#define PCI_BASE_ADDRESS_2	0x18	/* 32 bits */
>  #define PCI_BASE_ADDRESS_5	0x24	/* 32 bits */
>  #define  PCI_BASE_ADDRESS_SPACE_IO	0x01
>  #define  PCI_BASE_ADDRESS_SPACE_MEMORY	0x00
> -- 
> 1.6.0.2