[libvirt] [PATCH 2/3] conf: don't allow connecting upstream-port directly to pce-expander-bus

[Andrea Bolognani]

On Fri, 2016-08-05 at 23:01 -0400, Laine Stump wrote:
> I apparently misunderstood Marcel's description of what could and
> couldn't be plugged into qemu's pxb-pcie controller (known as
> pcie-expander-bus in libvirt) - I specifically allowed directly
> connecting a pcie-switch-upstream-port, and it turns out that causes
> the guest kernel to crash.
> 
> This patch forbids such a connection, and updates the xml docs
> appropriately.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1361172
> ---
>  docs/formatdomain.html.in | 24 +++++++++++++-----------
>  src/conf/domain_addr.c    |  9 +++------
>  2 files changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 5acb3b9..eddb8dd 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3346,8 +3346,8 @@
>          this expander bus, and bus numbers less than the specified
>          value will be available to the next lower expander-bus (or the
>          root-bus if there are no lower expander buses). If you do not
> -        specify a busNumber, libvirt will find the lowest existing
> -        busNumber in all other expander buses (or use 256 if there are
> +        specify a busNr, libvirt will find the lowest existing
> +        busNr in all other expander buses (or use 256 if there are
>          no others) and auto-assign the busNr of that found bus - 2,
>          which provides one bus number for the pci-expander-bus and one
>          for the pci-bridge that is automatically attached to it (if

This hunk is unrelated to the fix, please split it into a
separate patch (which you can definitely then push as trivial).

> @@ -3360,15 +3360,17 @@
>            2nd bus-number is just being reserved for the pcie-root-port
>            that must necessarily be connected to the bus in order to
>            actually plug in an endpoint device. If you intend to plug
> -          multiple devices into a pcie-expander-bus, you must instead
> -          connect a pcie-switch-upstream-port to the
> -          pcie-expander-bus, and multiple pcie-switch-downstream-ports
> -          to the pcie-switch-downstream-port, and of course for this
> -          to work properly, you will need to decrease the
> -          pcie-expander-bus' busNr accordingly so that there are
> -          enough unused bus numbers above it to accomodate giving out
> -          one bus number for the upstream-port and one for each
> -          downstream-port).
> +          multiple devices into a pcie-expander-bus, you must connect
> +          a pcie-switch-upstream-port to the pcie-root-bus that is

s/pcie-root-bus/pcie-root-port/

> +          plugged into the pcie-expander-bus, and multiple
> +          pcie-switch-downstream-ports to the
> +          pcie-switch-upstream-port, and of course for this to work
> +          properly, you will need to decrease the pcie-expander-bus'
> +          busNr accordingly so that there are enough unused bus
> +          numbers above it to accomodate giving out one bus number for
> +          the upstream-port and one for each downstream-port (in
> +          addition to the pcie-root-port and the pcie-expander-bus
> +          itself).
>          </p>
>        </dd>
>        <dt><code>node</code></dt>
> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> index 98176e2..faf5a5a 100644
> --- a/src/conf/domain_addr.c
> +++ b/src/conf/domain_addr.c
> @@ -161,7 +161,7 @@ virDomainPCIAddressFlagsCompatible(virPCIDeviceAddressPtr addr,
>          }
>          return false;
>      }
> -    return true;
> +   return true;
>  }
> 
> 

How did this hunk even get here? :)

> @@ -291,11 +291,8 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus,
>          bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST;
>          break;
>      case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
> -        /* single slot, no hotplug, only accepts pcie-root-port or
> -         * pcie-switch-upstream-port.
> -         */
> -        bus->flags = (VIR_PCI_CONNECT_TYPE_PCIE_ROOT_PORT
> -                      | VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_UPSTREAM_PORT);
> +        /* single slot, no hotplug, only accepts pcie-root-port */
> +        bus->flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT_PORT;
>          bus->minSlot = 0;
>          bus->maxSlot = 0;
>          break;

ACK with the comments addressed.

-- 
Andrea Bolognani / Red Hat / Virtualization