Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default

[Laine Stump]

On 11/02/2016 11:16 AM, Marcel Apfelbaum wrote:
> The shpc component is optional while  ACPI hotplug is used
> for hot-plugging PCI devices into a PCI-PCI bridge.
> Disabling the shpc by default will make slot 0 usable at boot time
> and not only for hot-plug, without loosing any functionality.
> Older machines will have shpc enabled for compatibility reasons.

From libvirt's POV, changing qemu's default for shpc in qemu only 
serves to confuse; since we have no way to discover what is the default 
setting (especially problematic since it is different for different 
machinetype versions), we now either need to keep a table of what the 
setting is for various machinetypes, or we need to just always set it 
whether it's on or off.

So for us, the simplest thing is for the default to remain the same 
(since for so long we haven't set this option as we didn't even know 
what it did, or indeed even that it existed), and for libvirt to learn 
about the option, make its default in the config files "on", but begin 
setting it to "off" in all newly defined machines.

(The change to the default for virtio devices' disable-legacy depending 
on where the device was connected could also have been problematic, 
except that the change in default value of that has no direct 
consequences on the rest of the configuration - libvirt will still use 
the controllers in exactly the same way, and the same commandlines for 
the same machinetype/versions will still result in identical behavior 
(so it's not possible for libvirt to know whether or not IO is enabled 
for the virtio device, nor to know what is the device's . In the case of 
pci-bridge's shpc though, the whole point of disabling shpc is to make 
another slot available for libvirt to use for a device, so presumably 
the config *beyond the pci-bridge device* needs to change as a result of 
this change in default setting, so libvirt needs to know the setting and 
behave differently.)

> Signed-off-by: Marcel Apfelbaum <address@hidden>
> ---
>  hw/pci-bridge/pci_bridge_dev.c | 2 +-
>  include/hw/compat.h            | 4 ++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
> index 5dbd933..647ad80 100644
> --- a/hw/pci-bridge/pci_bridge_dev.c
> +++ b/hw/pci-bridge/pci_bridge_dev.c
> @@ -163,7 +163,7 @@ static Property pci_bridge_dev_properties[] = {
>      DEFINE_PROP_ON_OFF_AUTO(PCI_BRIDGE_DEV_PROP_MSI, PCIBridgeDev, msi,
>                              ON_OFF_AUTO_AUTO),
>      DEFINE_PROP_BIT(PCI_BRIDGE_DEV_PROP_SHPC, PCIBridgeDev, flags,
> -                    PCI_BRIDGE_DEV_F_SHPC_REQ, true),
> +                    PCI_BRIDGE_DEV_F_SHPC_REQ, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 0f06e11..388b7ec 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -18,6 +18,10 @@
>          .driver   = "intel-iommu",\
>          .property = "x-buggy-eim",\
>          .value    = "true",\
> +    },{\
> +        .driver   = "pci-bridge",\
> +        .property = "shpc",\
> +        .value    = "on",\
>      },
>
>  #define HW_COMPAT_2_6 \