[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH v5 04/14] pci/bridge: fix pci_bridge_reset()
From: |
Michael S. Tsirkin |
Subject: |
[Qemu-devel] Re: [PATCH v5 04/14] pci/bridge: fix pci_bridge_reset() |
Date: |
Tue, 19 Oct 2010 14:07:28 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Oct 19, 2010 at 06:06:31PM +0900, Isaku Yamahata wrote:
> The default value of base/limit registers aren't specified in the spec.
> So pci_bridge_reset() shouldn't touch them.
> Instead, introduced two functions to reset those registers in a way
> of typical implementation. zero base/limit registers or disable forwarding.
> They will be used later.
>
> Signed-off-by: Isaku Yamahata <address@hidden>
> ---
I have some second thoughts:
1. pci_bridge_reset is used in several devices and I have no idea what
the reset change will do there, how do real devices behave or whether
guests depend on a specific behaviour. It seems harmless to leave the
current implementation in place, and simply add
pci_bridge_disable_base_limit which devices can call after
pci_bridge_reset.
2. _zero and _disable describe what function does already,
so we should drop _reset from function name.
Thus we only get 1 new function, pci_bridge_disable_base_limit.
> Changes v4 -> v5:
> - drop the lines in pci_bridge_reset()
> - introduced two functions to reset base/limit registers.
> ---
> hw/pci_bridge.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++-------
> hw/pci_bridge.h | 2 +
> 2 files changed, 51 insertions(+), 8 deletions(-)
>
> diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
> index 638e3b3..de75e6a 100644
> --- a/hw/pci_bridge.c
> +++ b/hw/pci_bridge.c
> @@ -151,6 +151,46 @@ void pci_bridge_write_config(PCIDevice *d,
> }
> }
>
> +void pci_bridge_reset_zero_base_limit(PCIDevice *dev)
> +{
> + uint8_t *conf = dev->config;
> +
> + pci_byte_test_and_clear_mask(conf + PCI_IO_BASE,
> + PCI_IO_RANGE_MASK & 0xff);
> + pci_byte_test_and_clear_mask(conf + PCI_IO_LIMIT,
> + PCI_IO_RANGE_MASK & 0xff);
> + pci_word_test_and_clear_mask(conf + PCI_MEMORY_BASE,
> + PCI_MEMORY_RANGE_MASK & 0xffff);
> + pci_word_test_and_clear_mask(conf + PCI_MEMORY_LIMIT,
> + PCI_MEMORY_RANGE_MASK & 0xffff);
> + pci_word_test_and_clear_mask(conf + PCI_PREF_MEMORY_BASE,
> + PCI_PREF_RANGE_MASK & 0xffff);
> + pci_word_test_and_clear_mask(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);
> +}
> +
> +void pci_bridge_reset_disable_base_limit(PCIDevice *dev)
> +{
> + uint8_t *conf = dev->config;
> +
> + pci_byte_test_and_set_mask(conf + PCI_IO_BASE,
> + PCI_IO_RANGE_MASK & 0xff);
> + pci_byte_test_and_clear_mask(conf + PCI_IO_LIMIT,
> + PCI_IO_RANGE_MASK & 0xff);
> + pci_word_test_and_set_mask(conf + PCI_MEMORY_BASE,
> + PCI_MEMORY_RANGE_MASK & 0xffff);
> + pci_word_test_and_clear_mask(conf + PCI_MEMORY_LIMIT,
> + PCI_MEMORY_RANGE_MASK & 0xffff);
> + pci_word_test_and_set_mask(conf + PCI_PREF_MEMORY_BASE,
> + PCI_PREF_RANGE_MASK & 0xffff);
> + pci_word_test_and_clear_mask(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);
> +}
> +
> /* reset bridge specific configuration registers */
> void pci_bridge_reset_reg(PCIDevice *dev)
> {
> @@ -161,14 +201,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_set_word(conf + PCI_PREF_BASE_UPPER32, 0);
> - pci_set_word(conf + PCI_PREF_LIMIT_UPPER32, 0);
> + /*
> + * the default values for base/limit registers aren't specified
> + * in the PCI-to-PCI-bridge spec. So we don't thouch them here.
> + * Each implementation can override it.
> + * typical implementation does
> + * - zero registers: pci_bridge_reset_zer_base_limit()
> + * or
> + * - disable forwarding: pci_bridge_reset_disable_base_limit()
> + */
>
> pci_set_word(conf + PCI_BRIDGE_CONTROL, 0);
> }
> diff --git a/hw/pci_bridge.h b/hw/pci_bridge.h
> index f6fade0..2359684 100644
> --- a/hw/pci_bridge.h
> +++ b/hw/pci_bridge.h
> @@ -39,6 +39,8 @@ pcibus_t pci_bridge_get_limit(const PCIDevice *bridge,
> uint8_t type);
>
> void pci_bridge_write_config(PCIDevice *d,
> uint32_t address, uint32_t val, int len);
> +void pci_bridge_reset_zero_base_limit(PCIDevice *dev);
> +void pci_bridge_reset_disable_base_limit(PCIDevice *dev);
> void pci_bridge_reset_reg(PCIDevice *dev);
> void pci_bridge_reset(DeviceState *qdev);
>
> --
> 1.7.1.1
- [Qemu-devel] [PATCH v5 00/14] pcie port switch emulators, Isaku Yamahata, 2010/10/19
- [Qemu-devel] [PATCH v5 04/14] pci/bridge: fix pci_bridge_reset(), Isaku Yamahata, 2010/10/19
- [Qemu-devel] Re: [PATCH v5 04/14] pci/bridge: fix pci_bridge_reset(),
Michael S. Tsirkin <=
- [Qemu-devel] [PATCH v5 13/14] pcie/hotplug: introduce pushing attention button command, Isaku Yamahata, 2010/10/19
- [Qemu-devel] [PATCH v5 03/14] pci: use pci_word_test_and_clear_mask() in pci_device_reset(), Isaku Yamahata, 2010/10/19
- [Qemu-devel] [PATCH v5 02/14] pci: introduce helper function to handle msi-x and msi., Isaku Yamahata, 2010/10/19
- [Qemu-devel] [PATCH v5 14/14] pcie/aer: glue aer error injection into qemu monitor, Isaku Yamahata, 2010/10/19
- [Qemu-devel] [PATCH v5 01/14] pci: introduce helper functions to test-and-{clear, set} mask in configuration space, Isaku Yamahata, 2010/10/19
- [Qemu-devel] [PATCH v5 06/14] pcie: add pcie constants to pcie_regs.h, Isaku Yamahata, 2010/10/19
- [Qemu-devel] [PATCH v5 12/14] x3130: pcie downstream port, Isaku Yamahata, 2010/10/19
- [Qemu-devel] [PATCH v5 07/14] pcie: helper functions for pcie capability and extended capability, Isaku Yamahata, 2010/10/19
- [Qemu-devel] [PATCH v5 08/14] pcie/aer: helper functions for pcie aer capability, Isaku Yamahata, 2010/10/19