[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [RFC PATCH] spapr_pci: make index property mandatory
From: |
David Gibson |
Subject: |
Re: [Qemu-ppc] [RFC PATCH] spapr_pci: make index property mandatory |
Date: |
Wed, 13 Sep 2017 23:17:48 +1000 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
On Mon, Sep 11, 2017 at 01:35:03PM +0200, Greg Kurz wrote:
> Creating several PHBs without index property confuses the DRC code
> and causes issues:
> - only the first index-less PHB is functional, the other ones will
> silently ignore hotplugging of PCI devices
> - QEMU will even terminate if these PHBs have cold-plugged devices
>
> qemu-system-ppc64: -device virtio-net,bus=pci2.0: an attached device
> is still awaiting release
>
> This happens because DR connectors for child PCI devices are created
> with a DRC index that is derived from the PHB's index property. If the
> PHBs are created without index, then the same value of -1 is used to
> compute the DRC indexes for both PHBs, hence causing the collision.
>
> Also, the index property is used to compute the placement of the PHB's
> memory regions. It is limited to 31 or 255, depending on the machine
> type version. This fits well with the requirements of DRC indexes, which
> need the PHB index to be a 16-bit value.
>
> This patch hence makes the index property mandatory. As a consequence,
> the PHB's memory regions and BUID are now always configured according
> to the index, and it is no longer possible to set them from the command
> line. This is a reasonable trade-off, as it is very unlikely that people
> create PHBs without index (at least libvirt doesn't do it FWIW).
>
> We have to introduce a PHB instance init function to initialize the
> 64-bit window address to -1 because pseries-2.7 and older machines
> don't set it.
>
> Signed-off-by: Greg Kurz <address@hidden>
> ---
>
> Hi,
>
> This is a proposal to address the issue uncovered during the review of the
> PHB hotplug patches:
>
> https://lists.nongnu.org/archive/html/qemu-ppc/2017-07/msg00557.html
>
> I'd like to address this properly before resuming work on PHB hotplug
> itself. Please comment.
Yes, I think this looks good. Commit message should explicitly
mention that this does break backwards compat - but we don't think the
non-index PHB feature was used in practice and the simplification is
worth it.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature