[libvirt] [PATCH 4/8] Allow PM reset on multi-function PCI devices

[Daniel P. Berrange]

On Thu, Aug 13, 2009 at 05:44:33PM +0100, Mark McLoughlin wrote:
> It turns out that a PCI Power Management reset only affects individual
> functions, and not the whole device.
> 
> The PCI Power Management spec talks about resetting the 'device' rather
> than the 'function', but Intel's Dexuan Cui informs me that it is
> actually a per-function reset.
> 
> Also, Yu Zhao has added pci_pm_reset() to the kernel, and it doesn't
> reject multi-function devices, so it must be true! :-)
> 
> (A side issue is that we could defer the PM reset to the kernel if we
> could detect that the kernel has PM reset support, but barring version
> number checks we don't have a way to detect that support)
> 
> * src/pci.c: remove the pciDeviceContainsOtherFunctions() check from
>   pciTryPowerManagementReset() and prefer PM reset over bus reset
>   where both are available
> 
> Cc: Cui, Dexuan <dexuan.cui at intel.com>
> Cc: Yu Zhao <yu.zhao at intel.com>
> ---
>  src/pci.c |   48 +++++++++---------------------------------------
>  1 files changed, 9 insertions(+), 39 deletions(-)
> 
> diff --git a/src/pci.c b/src/pci.c
> index 2dc2e1c..11b3e8b 100644
> --- a/src/pci.c
> +++ b/src/pci.c
> @@ -402,29 +402,6 @@ pciBusContainsOtherDevices(virConnectPtr conn, pciDevice *dev)
>      return 1;
>  }
>  
> -/* Any other functions on this device ? */
> -static int
> -pciSharesDevice(pciDevice *a, pciDevice *b)
> -{
> -    return
> -        a->domain == b->domain &&
> -        a->bus == b->bus &&
> -        a->slot == b->slot &&
> -        a->function != b->function;
> -}
> -
> -static int
> -pciDeviceContainsOtherFunctions(virConnectPtr conn, pciDevice *dev)
> -{
> -    pciDevice *matched = NULL;
> -    if (pciIterDevices(conn, pciSharesDevice, dev, &matched) < 0)
> -        return 1;
> -    if (!matched)
> -        return 0;
> -    pciFreeDevice(conn, matched);
> -    return 1;
> -}
> -
>  /* Is @a the parent of @b ? */
>  static int
>  pciIsParent(pciDevice *a, pciDevice *b)
> @@ -529,7 +506,7 @@ out:
>   * above we require the device supports a full internal reset.
>   */
>  static int
> -pciTryPowerManagementReset(virConnectPtr conn, pciDevice *dev)
> +pciTryPowerManagementReset(virConnectPtr conn ATTRIBUTE_UNUSED, pciDevice *dev)
>  {
>      uint8_t config_space[PCI_CONF_LEN];
>      uint32_t ctl;
> @@ -537,16 +514,6 @@ pciTryPowerManagementReset(virConnectPtr conn, pciDevice *dev)
>      if (!dev->pci_pm_cap_pos)
>          return -1;
>  
> -    /* For now, we just refuse to do a power management reset
> -     * if there are other functions on this device.
> -     * In future, we could allow it so long as those functions
> -     * are not in use by the host or other guests.
> -     */
> -    if (pciDeviceContainsOtherFunctions(conn, dev)) {
> -        VIR_WARN("%s contains other functions, not resetting", dev->name);
> -        return -1;
> -    }
> -
>      /* Save and restore the device's config space. */
>      if (pciRead(dev, 0, &config_space[0], PCI_CONF_LEN) < 0) {
>          VIR_WARN("Failed to save PCI config space for %s", dev->name);
> @@ -604,14 +571,17 @@ pciResetDevice(virConnectPtr conn, pciDevice *dev)
>      if (dev->has_flr)
>          return 0;
>  
> +    /* If the device supports PCI power management reset,
> +     * that's the next best thing because it only resets
> +     * the function, not the whole device.
> +     */
> +    if (dev->has_pm_reset)
> +        ret = pciTryPowerManagementReset(conn, dev);
> +
>      /* Bus reset is not an option with the root bus */
> -    if (dev->bus != 0)
> +    if (ret < 0 && dev->bus != 0)
>          ret = pciTrySecondaryBusReset(conn, dev);
>  
> -    /* Next best option is a PCI power management reset */
> -    if (ret < 0 && dev->has_pm_reset)
> -        ret = pciTryPowerManagementReset(conn, dev);
> -
>      if (ret < 0)
>          pciReportError(conn, VIR_ERR_NO_SUPPORT,
>                         _("No PCI reset capability available for %s"),

ACK


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|