On Mon, 11 Jan 2016 14:24:23 +0200
Marcel Apfelbaum <mar...@redhat.com> wrote:
> Two reasons:
> - PCI Spec indicates that while the bit is not set
> the memory sizing is not finished.
> - pci_bar_address will return PCI_BAR_UNMAPPED
> and a previous value can be accidentally overridden
> if the command register is modified (and not the BAR).
>
> Signed-off-by: Marcel Apfelbaum <mar...@redhat.com>
> ---
>
> Hi,
>
> I found this when trying to use multiple root complexes with OVMF.
>
> When trying to attach a device to the pxb-pcie device as Integrated
> Device it did not receive the IO/MEM resources.
>
> The reason is that OVMF is working like that:
> 1. It disables the Decode (I/O or memory) bit in the Command register
> 2. It configures the device BARS
> 3. Makes some tests on the Command register
> 4. ...
> 5. Enables the Decode (I/O or memory) at some point.
>
> On step 3 all the BARS are overridden to 0xffffffff by QEMU.
>
> Since QEMU uses the device BARs to compute the new host bridge resources
> it now gets garbage.
>
> Laszlo, this also solves the SHPC problem for the pci-2-pci bridge inside the
> pxb.
> Now we can enable the SHPC for it too.
What about migration case?
Shouldn't mappings be updated to match source even if bit isn't set?
>
> Thanks,
> Marcel
>
> hw/pci/pci.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 168b9cc..f9127dc 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1148,6 +1148,7 @@ static void pci_update_mappings(PCIDevice *d)
> PCIIORegion *r;
> int i;
> pcibus_t new_addr;
> + uint16_t cmd = pci_get_word(d->config + PCI_COMMAND);
>
> for(i = 0; i < PCI_NUM_REGIONS; i++) {
> r = &d->io_regions[i];
> @@ -1156,6 +1157,22 @@ static void pci_update_mappings(PCIDevice *d)
> if (!r->size)
> continue;
>
> + /*
> + * Do not update the mappings until the command register's
> + * Decode (I/O or memory) bit is not set. Two reasons:
> + * - PCI Spec indicates that while the bit is not set
> + * the memory sizing is not finished.
> + * - pci_bar_address will return PCI_BAR_UNMAPPED
> + * and a previous value can be accidentally overridden
> + * if the command register is modified (and not the BAR).
> + * */
> + if (((r->type & PCI_BASE_ADDRESS_SPACE_IO) &&
> + !(cmd & PCI_COMMAND_IO)) ||
> + ((r->type != PCI_BASE_ADDRESS_SPACE_IO) &&
> + !(cmd & PCI_COMMAND_MEMORY))) {
> + continue;
> + }
> +
> new_addr = pci_bar_address(d, i, r->type, r->size);
>
> /* This bar isn't changed */
