diff mbox

hw/pci: disable pci-bridge's shpc by default

Message ID 1478099802-14188-1-git-send-email-marcel@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Marcel Apfelbaum Nov. 2, 2016, 3:16 p.m. UTC
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.

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/pci-bridge/pci_bridge_dev.c | 2 +-
 include/hw/compat.h            | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

Laine Stump Nov. 2, 2016, 4:01 p.m. UTC | #1
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 <marcel@redhat.com>
> ---
>  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 \
>
Michael S. Tsirkin Nov. 3, 2016, 4:18 a.m. UTC | #2
On Wed, Nov 02, 2016 at 05:16:42PM +0200, 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

at the cost of breaking all hotplug for all non-acpi users.

> and not only for hot-plug, without loosing any functionality.
> Older machines will have shpc enabled for compatibility reasons.
> 
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>

Is an extra slot such a big deal? You can always add more bridges ...

> ---
>  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 \
> -- 
> 2.5.5
Marcel Apfelbaum Nov. 3, 2016, 11:05 a.m. UTC | #3
On 11/03/2016 06:18 AM, Michael S. Tsirkin wrote:
> On Wed, Nov 02, 2016 at 05:16:42PM +0200, 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

Hi Michael

>
> at the cost of breaking all hotplug for all non-acpi users.
>

Do we have a non-acpi user that is able to use the shpc component as-is today?
I remember we need to even tweak QEMU before it can be used, but I might be wrong.

And we don't touch the current machines < 2.8 .

>> and not only for hot-plug, without loosing any functionality.
>> Older machines will have shpc enabled for compatibility reasons.
>>
>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>
> Is an extra slot such a big deal? You can always add more bridges ...
>

It is not only about the slot itself, but more about the usage model.
The PCIe Upstream ports/DMI-PCI devices are also pci-bridges,
but for them slot 0 is allowed.
And what about the hotplug? Slot 0 is not usable at boot, but then is
usable again (for ACPI users) making people wondering:
  https://bugzilla.redhat.com/show_bug.cgi?id=1175113

My point is - can shpc be used as-is today? Even so, I suspect there are much (much)
less users using SHPC than ACPI based hotplug. If this is the case, why bother the
majority of the users? And for the shpc users, they can keep the prev machines
or change the command line, I think changes like this happens over the time.

Adding Markus for his opinion on command line changes.

Thanks,
Marcel

>> ---
>>  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 \
>> --
>> 2.5.5
Marcel Apfelbaum Nov. 3, 2016, 11:08 a.m. UTC | #4
On 11/02/2016 06:01 PM, Laine Stump wrote:
> 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.
>

Hi Laine,

Please see my reply to Michael regarding the "why", anyway, can't libvirt deal with it?
Start use the shpc parameter no matter what QEMU does by default while keeping it 'on' for machines < 2.8.
(this is only a suggestion, of course...)


Thanks,
Marcel

> (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 <marcel@redhat.com>
>> ---
>>  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 \
>>
>
Laine Stump Nov. 3, 2016, 3:24 p.m. UTC | #5
On 11/03/2016 07:08 AM, Marcel Apfelbaum wrote:
> On 11/02/2016 06:01 PM, Laine Stump wrote:
>> 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.
>>
>
> Hi Laine,
>
> Please see my reply to Michael regarding the "why", anyway, can't
> libvirt deal with it?
> Start use the shpc parameter no matter what QEMU does by default while
> keeping it 'on' for machines < 2.8.
> (this is only a suggestion, of course...)


We greatly dislike coding in behavior changes to libvirt based on 
machinetype/version or qemu version since a version number is a very 
broad and inaccurate sword. The best behavior changes are those that can 
be discovered by querying something specific to that behavior, which 
can't be done in this case, as there is no way to query *anything* 
specific to a machinetype without instantiating a virtual machine of 
that machinetype (if it's even queryable then, which is anyway 
irrelevant to libvirt, since our queries to qemu are done with -machine 
none).

libvirt can of course deal with such a change in default behavior by 
qemu, but the way it will deal with it is by removing all assumptions 
about the default of shpc from the code, and replacing those assumptions 
with explicit setting of the option in the xml and on the qemu command 
line all the time.

In the end, libvirt wouldn't gain anything from this change in qemu's 
default behavior - it would be more work than just adding a setting for 
shpc to libvirt and having *libvirt* change its default setting (which 
has the same ultimate results). Of course that's a relatively selfish 
(libvirt-centric) view, and I suppose direct users of the qemu 
commandline might get some benefit from changing the qemu default, but 
anyone concerned enough about the exact commandline contents to be 
running qemu directly would probably also not have a problem adding an 
option to the commandline if they really wanted a 1/32 increase in the 
number of available slots.


>
>
> Thanks,
> Marcel
>
>> (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 <marcel@redhat.com>
>>> ---
>>>  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 \
>>>
>>
>
Marcel Apfelbaum Nov. 3, 2016, 4:43 p.m. UTC | #6
On 11/03/2016 05:24 PM, Laine Stump wrote:
> On 11/03/2016 07:08 AM, Marcel Apfelbaum wrote:
>> On 11/02/2016 06:01 PM, Laine Stump wrote:
>>> 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.
>>>
>>
>> Hi Laine,
>>
>> Please see my reply to Michael regarding the "why", anyway, can't
>> libvirt deal with it?
>> Start use the shpc parameter no matter what QEMU does by default while
>> keeping it 'on' for machines < 2.8.
>> (this is only a suggestion, of course...)
>
>
> We greatly dislike coding in behavior changes to libvirt based on machinetype/version or qemu version since a version number is a very broad and inaccurate sword. The best behavior changes are those
> that can be discovered by querying something specific to that behavior, which can't be done in this case, as there is no way to query *anything* specific to a machinetype without instantiating a
> virtual machine of that machinetype (if it's even queryable then, which is anyway irrelevant to libvirt, since our queries to qemu are done with -machine none).
>

+Eduardo

Hi Eduardo,
Can your work can help on this specific case?


> libvirt can of course deal with such a change in default behavior by qemu, but the way it will deal with it is by removing all assumptions about the default of shpc from the code, and replacing those
> assumptions with explicit setting of the option in the xml and on the qemu command line all the time.
>
> In the end, libvirt wouldn't gain anything from this change in qemu's default behavior - it would be more work than just adding a setting for shpc to libvirt and having *libvirt* change its default
> setting (which has the same ultimate results). Of course that's a relatively selfish (libvirt-centric) view, and I suppose direct users of the qemu commandline might get some benefit from changing the
> qemu default, but anyone concerned enough about the exact commandline contents to be running qemu directly would probably also not have a problem adding an option to the commandline if they really
> wanted a 1/32 increase in the number of available slots.
>

As I explained to Michael, this is not only about the extra slot, but
more about the usage model.
I do understand the libvirt point of view, on the other hand why
should we use a default QEMU value that the majority of users don't need?

Thanks,
Marcel

>
>>
>>
>> Thanks,
>> Marcel
>>
>>> (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 <marcel@redhat.com>
>>>> ---
>>>>  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 \
>>>>
>>>
>>
>
Eduardo Habkost Nov. 3, 2016, 7:09 p.m. UTC | #7
On Thu, Nov 03, 2016 at 06:43:05PM +0200, Marcel Apfelbaum wrote:
> On 11/03/2016 05:24 PM, Laine Stump wrote:
> > On 11/03/2016 07:08 AM, Marcel Apfelbaum wrote:
> > > On 11/02/2016 06:01 PM, Laine Stump wrote:
> > > > 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.
> > > > 
> > > 
> > > Hi Laine,
> > > 
> > > Please see my reply to Michael regarding the "why", anyway, can't
> > > libvirt deal with it?
> > > Start use the shpc parameter no matter what QEMU does by default while
> > > keeping it 'on' for machines < 2.8.
> > > (this is only a suggestion, of course...)
> > 
> > 
> > We greatly dislike coding in behavior changes to libvirt
> > based on machinetype/version or qemu version since a version
> > number is a very broad and inaccurate sword. The best
> > behavior changes are those that can be discovered by querying
> > something specific to that behavior, which can't be done in
> > this case, as there is no way to query *anything* specific to
> > a machinetype without instantiating a virtual machine of that
> > machinetype (if it's even queryable then, which is anyway
> > irrelevant to libvirt, since our queries to qemu are done
> > with -machine none).
> > 
> 
> +Eduardo
> 
> Hi Eduardo,
> Can your work can help on this specific case?

In this cases, the needed mechanisms already exist. We just need
to encode that info in the MachineInfo struct, so that it gets
returned by the "query-machines" QMP command. (But making QEMU
maintainers agree on how to encode stuff in QMP is always a
challenge.)

> 
> 
> > libvirt can of course deal with such a change in default behavior by qemu, but the way it will deal with it is by removing all assumptions about the default of shpc from the code, and replacing those
> > assumptions with explicit setting of the option in the xml and on the qemu command line all the time.
> > 
> > In the end, libvirt wouldn't gain anything from this change in qemu's default behavior - it would be more work than just adding a setting for shpc to libvirt and having *libvirt* change its default
> > setting (which has the same ultimate results). Of course that's a relatively selfish (libvirt-centric) view, and I suppose direct users of the qemu commandline might get some benefit from changing the
> > qemu default, but anyone concerned enough about the exact commandline contents to be running qemu directly would probably also not have a problem adding an option to the commandline if they really
> > wanted a 1/32 increase in the number of available slots.
> > 
> 
> As I explained to Michael, this is not only about the extra slot, but
> more about the usage model.
> I do understand the libvirt point of view, on the other hand why
> should we use a default QEMU value that the majority of users don't need?

This is a common problem: sometimes we want to make the default
behavior saner on new machine-types, but changing the default
behavior often doesn't help libvirt (and makes life more
difficult for everybody).

I hope this will become easier over time, once we start making
machine-type info easily discoverable by libvirt.
Michael S. Tsirkin Nov. 3, 2016, 7:40 p.m. UTC | #8
On Thu, Nov 03, 2016 at 01:05:44PM +0200, Marcel Apfelbaum wrote:
> On 11/03/2016 06:18 AM, Michael S. Tsirkin wrote:
> > On Wed, Nov 02, 2016 at 05:16:42PM +0200, 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
> 
> Hi Michael
> 
> > 
> > at the cost of breaking all hotplug for all non-acpi users.
> > 
> 
> Do we have a non-acpi user that is able to use the shpc component as-is today?

power and some arm systems I guess?

> I remember we need to even tweak QEMU before it can be used, but I might be wrong.
> 
> And we don't touch the current machines < 2.8 .
> 
> > > and not only for hot-plug, without loosing any functionality.
> > > Older machines will have shpc enabled for compatibility reasons.
> > > 
> > > Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> > 
> > Is an extra slot such a big deal? You can always add more bridges ...
> > 
> 
> It is not only about the slot itself, but more about the usage model.
> The PCIe Upstream ports/DMI-PCI devices are also pci-bridges,
> but for them slot 0 is allowed.

The reason is that these devices are not themselves
hotpluggable. Isn't there a flag that allows adding
a non hotpluggable device? Allowing these would be one solution.

> And what about the hotplug? Slot 0 is not usable at boot, but then is
> usable again (for ACPI users) making people wondering:
>  https://bugzilla.redhat.com/show_bug.cgi?id=1175113

Let's just disallow that then for consistency?


> My point is - can shpc be used as-is today? Even so, I suspect there are much (much)
> less users using SHPC than ACPI based hotplug. If this is the case, why bother the
> majority of the users? And for the shpc users, they can keep the prev machines
> or change the command line, I think changes like this happens over the time.
> 
> Adding Markus for his opinion on command line changes.
> 
> Thanks,
> Marcel
> > > ---
> > >  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 \
> > > --
> > > 2.5.5
Marcel Apfelbaum Nov. 5, 2016, 4:46 p.m. UTC | #9
On 11/03/2016 09:40 PM, Michael S. Tsirkin wrote:
> On Thu, Nov 03, 2016 at 01:05:44PM +0200, Marcel Apfelbaum wrote:
>> On 11/03/2016 06:18 AM, Michael S. Tsirkin wrote:
>>> On Wed, Nov 02, 2016 at 05:16:42PM +0200, 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
>>
>> Hi Michael
>>
>>>
>>> at the cost of breaking all hotplug for all non-acpi users.
>>>
>>
>> Do we have a non-acpi user that is able to use the shpc component as-is today?
>
> power and some arm systems I guess?
>

Adding Andrew , maybe he can give us an answer.

Anybody else can help answering this?

>> I remember we need to even tweak QEMU before it can be used, but I might be wrong.
>>
>> And we don't touch the current machines < 2.8 .
>>
>>>> and not only for hot-plug, without loosing any functionality.
>>>> Older machines will have shpc enabled for compatibility reasons.
>>>>
>>>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>>>
>>> Is an extra slot such a big deal? You can always add more bridges ...
>>>
>>
>> It is not only about the slot itself, but more about the usage model.
>> The PCIe Upstream ports/DMI-PCI devices are also pci-bridges,
>> but for them slot 0 is allowed.
>
> The reason is that these devices are not themselves
> hotpluggable. Isn't there a flag that allows adding
> a non hotpluggable device? Allowing these would be one solution.
>
>> And what about the hotplug? Slot 0 is not usable at boot, but then is
>> usable again (for ACPI users) making people wondering:
>>  https://bugzilla.redhat.com/show_bug.cgi?id=1175113
>
> Let's just disallow that then for consistency?
>

I suppose we can do that... not sure if it worth it.

Thanks,
Marcel

>
>> My point is - can shpc be used as-is today? Even so, I suspect there are much (much)
>> less users using SHPC than ACPI based hotplug. If this is the case, why bother the
>> majority of the users? And for the shpc users, they can keep the prev machines
>> or change the command line, I think changes like this happens over the time.
>>
>> Adding Markus for his opinion on command line changes.
>>
>> Thanks,
>> Marcel
>>>> ---
>>>>  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 \
>>>> --
>>>> 2.5.5
Andrew Jones Nov. 16, 2016, 4:44 p.m. UTC | #10
On Sat, Nov 05, 2016 at 06:46:34PM +0200, Marcel Apfelbaum wrote:
> On 11/03/2016 09:40 PM, Michael S. Tsirkin wrote:
> > On Thu, Nov 03, 2016 at 01:05:44PM +0200, Marcel Apfelbaum wrote:
> > > On 11/03/2016 06:18 AM, Michael S. Tsirkin wrote:
> > > > On Wed, Nov 02, 2016 at 05:16:42PM +0200, 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
> > > 
> > > Hi Michael
> > > 
> > > > 
> > > > at the cost of breaking all hotplug for all non-acpi users.
> > > > 
> > > 
> > > Do we have a non-acpi user that is able to use the shpc component as-is today?
> > 
> > power and some arm systems I guess?
> > 
> 
> Adding Andrew , maybe he can give us an answer.

Not really :-) My lack of PCI knowledge makes that difficult. I'd be happy
to help with an experiment though. Can you give me command line arguments,
qmp commands, etc. that I should use to try it out? I imagine I should
just boot an ARM guest using DT (instead of ACPI) and then attempt to
hotplug a PCI device. I'm not sure, however, what, if any, special
configuration I need in order to ensure I'm testing what you're
interested in.

Thanks,
drew


> 
> Anybody else can help answering this?
> 
> > > I remember we need to even tweak QEMU before it can be used, but I might be wrong.
> > > 
> > > And we don't touch the current machines < 2.8 .
> > > 
> > > > > and not only for hot-plug, without loosing any functionality.
> > > > > Older machines will have shpc enabled for compatibility reasons.
> > > > > 
> > > > > Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> > > > 
> > > > Is an extra slot such a big deal? You can always add more bridges ...
> > > > 
> > > 
> > > It is not only about the slot itself, but more about the usage model.
> > > The PCIe Upstream ports/DMI-PCI devices are also pci-bridges,
> > > but for them slot 0 is allowed.
> > 
> > The reason is that these devices are not themselves
> > hotpluggable. Isn't there a flag that allows adding
> > a non hotpluggable device? Allowing these would be one solution.
> > 
> > > And what about the hotplug? Slot 0 is not usable at boot, but then is
> > > usable again (for ACPI users) making people wondering:
> > >  https://bugzilla.redhat.com/show_bug.cgi?id=1175113
> > 
> > Let's just disallow that then for consistency?
> > 
> 
> I suppose we can do that... not sure if it worth it.
> 
> Thanks,
> Marcel
> 
> > 
> > > My point is - can shpc be used as-is today? Even so, I suspect there are much (much)
> > > less users using SHPC than ACPI based hotplug. If this is the case, why bother the
> > > majority of the users? And for the shpc users, they can keep the prev machines
> > > or change the command line, I think changes like this happens over the time.
> > > 
> > > Adding Markus for his opinion on command line changes.
> > > 
> > > Thanks,
> > > Marcel
> > > > > ---
> > > > >  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 \
> > > > > --
> > > > > 2.5.5
> 
>
Marcel Apfelbaum Nov. 16, 2016, 5:05 p.m. UTC | #11
On 11/16/2016 06:44 PM, Andrew Jones wrote:
> On Sat, Nov 05, 2016 at 06:46:34PM +0200, Marcel Apfelbaum wrote:
>> On 11/03/2016 09:40 PM, Michael S. Tsirkin wrote:
>>> On Thu, Nov 03, 2016 at 01:05:44PM +0200, Marcel Apfelbaum wrote:
>>>> On 11/03/2016 06:18 AM, Michael S. Tsirkin wrote:
>>>>> On Wed, Nov 02, 2016 at 05:16:42PM +0200, 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
>>>>
>>>> Hi Michael
>>>>
>>>>>
>>>>> at the cost of breaking all hotplug for all non-acpi users.
>>>>>
>>>>
>>>> Do we have a non-acpi user that is able to use the shpc component as-is today?
>>>
>>> power and some arm systems I guess?
>>>
>>
>> Adding Andrew , maybe he can give us an answer.
>
> Not really :-) My lack of PCI knowledge makes that difficult. I'd be happy
> to help with an experiment though. Can you give me command line arguments,
> qmp commands, etc. that I should use to try it out? I imagine I should
> just boot an ARM guest using DT (instead of ACPI) and then attempt to
> hotplug a PCI device. I'm not sure, however, what, if any, special
> configuration I need in order to ensure I'm testing what you're
> interested in.
>

Hi Drew,


Just run QEMU with '-device pci-bridge,chassis_nr=1,id=bridge1 -monitor stdio'
with an ARM guest using DT and wait until the guest finish booting.

Then run at hmp:
device_add virtio-net-pci,bus=bridge1,id=net2

Next run lspci in the guest to see the new device.


BTW, will an ARM guest run 'fast' enough to be usable on a x86 machine?
If yes, any pointers on how to create such a guest?


Thanks,
Marcel




> Thanks,
> drew
>


[...]
Andrew Jones Nov. 18, 2016, 3:52 p.m. UTC | #12
On Wed, Nov 16, 2016 at 07:05:25PM +0200, Marcel Apfelbaum wrote:
> On 11/16/2016 06:44 PM, Andrew Jones wrote:
> > On Sat, Nov 05, 2016 at 06:46:34PM +0200, Marcel Apfelbaum wrote:
> > > On 11/03/2016 09:40 PM, Michael S. Tsirkin wrote:
> > > > On Thu, Nov 03, 2016 at 01:05:44PM +0200, Marcel Apfelbaum wrote:
> > > > > On 11/03/2016 06:18 AM, Michael S. Tsirkin wrote:
> > > > > > On Wed, Nov 02, 2016 at 05:16:42PM +0200, 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
> > > > > 
> > > > > Hi Michael
> > > > > 
> > > > > > 
> > > > > > at the cost of breaking all hotplug for all non-acpi users.
> > > > > > 
> > > > > 
> > > > > Do we have a non-acpi user that is able to use the shpc component as-is today?
> > > > 
> > > > power and some arm systems I guess?
> > > > 
> > > 
> > > Adding Andrew , maybe he can give us an answer.
> > 
> > Not really :-) My lack of PCI knowledge makes that difficult. I'd be happy
> > to help with an experiment though. Can you give me command line arguments,
> > qmp commands, etc. that I should use to try it out? I imagine I should
> > just boot an ARM guest using DT (instead of ACPI) and then attempt to
> > hotplug a PCI device. I'm not sure, however, what, if any, special
> > configuration I need in order to ensure I'm testing what you're
> > interested in.
> > 
> 
> Hi Drew,
> 
> 
> Just run QEMU with '-device pci-bridge,chassis_nr=1,id=bridge1 -monitor stdio'
> with an ARM guest using DT and wait until the guest finish booting.
> 
> Then run at hmp:
> device_add virtio-net-pci,bus=bridge1,id=net2
> 
> Next run lspci in the guest to see the new device.

Thanks for the instructions Marcel. Here's the results

 $QEMU -machine virt,accel=$ACCEL -cpu $CPU -nographic -m 4096 -smp 8 \
       -bios /usr/share/AAVMF/AAVMF_CODE.fd \
       -device pci-bridge,chassis_nr=1,id=bridge1 \
       -drive file=$FEDORA_IMG,if=none,id=dr0,format=qcow2 \
       -device virtio-blk-pci,bus=bridge1,addr=01,drive=dr0,id=disk0 \
       -netdev user,id=hostnet0 \
       -device virtio-net-pci,bus=bridge1,addr=02,netdev=hostnet0,id=net0

 # lspci
 00:00.0 Host bridge: Red Hat, Inc. Device 0008
 00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
 01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
 01:02.0 Ethernet controller: Red Hat, Inc Virtio network device

 (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
 Unsupported PCI slot 0 for standard hotplug controller. Valid slots are
 between 1 and 31.

(Tried again giving addr=03)

 (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=03

(Seemed to work, but...)

 # lspci
 00:00.0 Host bridge: Red Hat, Inc. Device 0008
 00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
 01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
 01:02.0 Ethernet controller: Red Hat, Inc Virtio network device

(Doesn't show up in lscpi. So I guess it doesn't work)

> 
> 
> BTW, will an ARM guest run 'fast' enough to be usable on a x86 machine?
> If yes, any pointers on how to create such a guest?

You can run AArch64 guests on x86 machines. It's not super fast though...
Certainly I wouldn't want to create my guest image using TCG. So, assuming
you acquire an image somewhere (or create it on a real machine), then you
can use the above command line, just change 

ACCEL=kvm CPU=host to ACCEL=tcg CPU=cortex-a57

Thanks,
drew
Michael S. Tsirkin Nov. 18, 2016, 3:55 p.m. UTC | #13
On Fri, Nov 18, 2016 at 04:52:01PM +0100, Andrew Jones wrote:
> On Wed, Nov 16, 2016 at 07:05:25PM +0200, Marcel Apfelbaum wrote:
> > On 11/16/2016 06:44 PM, Andrew Jones wrote:
> > > On Sat, Nov 05, 2016 at 06:46:34PM +0200, Marcel Apfelbaum wrote:
> > > > On 11/03/2016 09:40 PM, Michael S. Tsirkin wrote:
> > > > > On Thu, Nov 03, 2016 at 01:05:44PM +0200, Marcel Apfelbaum wrote:
> > > > > > On 11/03/2016 06:18 AM, Michael S. Tsirkin wrote:
> > > > > > > On Wed, Nov 02, 2016 at 05:16:42PM +0200, 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
> > > > > > 
> > > > > > Hi Michael
> > > > > > 
> > > > > > > 
> > > > > > > at the cost of breaking all hotplug for all non-acpi users.
> > > > > > > 
> > > > > > 
> > > > > > Do we have a non-acpi user that is able to use the shpc component as-is today?
> > > > > 
> > > > > power and some arm systems I guess?
> > > > > 
> > > > 
> > > > Adding Andrew , maybe he can give us an answer.
> > > 
> > > Not really :-) My lack of PCI knowledge makes that difficult. I'd be happy
> > > to help with an experiment though. Can you give me command line arguments,
> > > qmp commands, etc. that I should use to try it out? I imagine I should
> > > just boot an ARM guest using DT (instead of ACPI) and then attempt to
> > > hotplug a PCI device. I'm not sure, however, what, if any, special
> > > configuration I need in order to ensure I'm testing what you're
> > > interested in.
> > > 
> > 
> > Hi Drew,
> > 
> > 
> > Just run QEMU with '-device pci-bridge,chassis_nr=1,id=bridge1 -monitor stdio'
> > with an ARM guest using DT and wait until the guest finish booting.
> > 
> > Then run at hmp:
> > device_add virtio-net-pci,bus=bridge1,id=net2
> > 
> > Next run lspci in the guest to see the new device.
> 
> Thanks for the instructions Marcel. Here's the results
> 
>  $QEMU -machine virt,accel=$ACCEL -cpu $CPU -nographic -m 4096 -smp 8 \
>        -bios /usr/share/AAVMF/AAVMF_CODE.fd \
>        -device pci-bridge,chassis_nr=1,id=bridge1 \
>        -drive file=$FEDORA_IMG,if=none,id=dr0,format=qcow2 \
>        -device virtio-blk-pci,bus=bridge1,addr=01,drive=dr0,id=disk0 \
>        -netdev user,id=hostnet0 \
>        -device virtio-net-pci,bus=bridge1,addr=02,netdev=hostnet0,id=net0
> 
>  # lspci
>  00:00.0 Host bridge: Red Hat, Inc. Device 0008
>  00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
>  01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
>  01:02.0 Ethernet controller: Red Hat, Inc Virtio network device
> 
>  (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
>  Unsupported PCI slot 0 for standard hotplug controller. Valid slots are
>  between 1 and 31.
> 
> (Tried again giving addr=03)
> 
>  (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=03
> 
> (Seemed to work, but...)
> 
>  # lspci
>  00:00.0 Host bridge: Red Hat, Inc. Device 0008
>  00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
>  01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
>  01:02.0 Ethernet controller: Red Hat, Inc Virtio network device
> 
> (Doesn't show up in lscpi. So I guess it doesn't work)
> 
> > 
> > 
> > BTW, will an ARM guest run 'fast' enough to be usable on a x86 machine?
> > If yes, any pointers on how to create such a guest?
> 
> You can run AArch64 guests on x86 machines. It's not super fast though...
> Certainly I wouldn't want to create my guest image using TCG. So, assuming
> you acquire an image somewhere (or create it on a real machine), then you
> can use the above command line, just change 
> 
> ACCEL=kvm CPU=host to ACCEL=tcg CPU=cortex-a57
> 
> Thanks,
> drew

http://wiki.qemu.org/Testing/System_Images

has some images.
If you have a better one to contribute, upload it there.
Marcel Apfelbaum Nov. 22, 2016, 5:26 p.m. UTC | #14
On 11/18/2016 05:52 PM, Andrew Jones wrote:
> On Wed, Nov 16, 2016 at 07:05:25PM +0200, Marcel Apfelbaum wrote:
>> On 11/16/2016 06:44 PM, Andrew Jones wrote:
>>> On Sat, Nov 05, 2016 at 06:46:34PM +0200, Marcel Apfelbaum wrote:
>>>> On 11/03/2016 09:40 PM, Michael S. Tsirkin wrote:
>>>>> On Thu, Nov 03, 2016 at 01:05:44PM +0200, Marcel Apfelbaum wrote:
>>>>>> On 11/03/2016 06:18 AM, Michael S. Tsirkin wrote:
>>>>>>> On Wed, Nov 02, 2016 at 05:16:42PM +0200, 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
>>>>>>
>>>>>> Hi Michael
>>>>>>
>>>>>>>
>>>>>>> at the cost of breaking all hotplug for all non-acpi users.
>>>>>>>
>>>>>>
>>>>>> Do we have a non-acpi user that is able to use the shpc component as-is today?
>>>>>
>>>>> power and some arm systems I guess?
>>>>>
>>>>
>>>> Adding Andrew , maybe he can give us an answer.
>>>
>>> Not really :-) My lack of PCI knowledge makes that difficult. I'd be happy
>>> to help with an experiment though. Can you give me command line arguments,
>>> qmp commands, etc. that I should use to try it out? I imagine I should
>>> just boot an ARM guest using DT (instead of ACPI) and then attempt to
>>> hotplug a PCI device. I'm not sure, however, what, if any, special
>>> configuration I need in order to ensure I'm testing what you're
>>> interested in.
>>>
>>
>> Hi Drew,
>>
>>
>> Just run QEMU with '-device pci-bridge,chassis_nr=1,id=bridge1 -monitor stdio'
>> with an ARM guest using DT and wait until the guest finish booting.
>>
>> Then run at hmp:
>> device_add virtio-net-pci,bus=bridge1,id=net2
>>
>> Next run lspci in the guest to see the new device.
>
> Thanks for the instructions Marcel. Here's the results
>
>  $QEMU -machine virt,accel=$ACCEL -cpu $CPU -nographic -m 4096 -smp 8 \
>        -bios /usr/share/AAVMF/AAVMF_CODE.fd \
>        -device pci-bridge,chassis_nr=1,id=bridge1 \
>        -drive file=$FEDORA_IMG,if=none,id=dr0,format=qcow2 \
>        -device virtio-blk-pci,bus=bridge1,addr=01,drive=dr0,id=disk0 \
>        -netdev user,id=hostnet0 \
>        -device virtio-net-pci,bus=bridge1,addr=02,netdev=hostnet0,id=net0
>
>  # lspci
>  00:00.0 Host bridge: Red Hat, Inc. Device 0008
>  00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
>  01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
>  01:02.0 Ethernet controller: Red Hat, Inc Virtio network device
>
>  (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
>  Unsupported PCI slot 0 for standard hotplug controller. Valid slots are
>  between 1 and 31.
>
> (Tried again giving addr=03)
>
>  (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=03
>
> (Seemed to work, but...)
>
>  # lspci
>  00:00.0 Host bridge: Red Hat, Inc. Device 0008
>  00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
>  01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
>  01:02.0 Ethernet controller: Red Hat, Inc Virtio network device
>
> (Doesn't show up in lscpi. So I guess it doesn't work)
>

Hi Drew,
Thanks for confirming that it doesn't work.

Michael asked if we can check the same for powerpc before
disabling the shpc by default.

Adding David, Thomas and Laurrent, maybe they have time
to check it for powerpc.

Your help would be very much appreciated.

Thanks,
Marcel

>>
>>
>> BTW, will an ARM guest run 'fast' enough to be usable on a x86 machine?
>> If yes, any pointers on how to create such a guest?
>
> You can run AArch64 guests on x86 machines. It's not super fast though...
> Certainly I wouldn't want to create my guest image using TCG. So, assuming
> you acquire an image somewhere (or create it on a real machine), then you
> can use the above command line, just change
>
> ACCEL=kvm CPU=host to ACCEL=tcg CPU=cortex-a57
>
> Thanks,
> drew
>
Michael S. Tsirkin Nov. 22, 2016, 5:47 p.m. UTC | #15
On Fri, Nov 18, 2016 at 04:52:01PM +0100, Andrew Jones wrote:
> On Wed, Nov 16, 2016 at 07:05:25PM +0200, Marcel Apfelbaum wrote:
> > On 11/16/2016 06:44 PM, Andrew Jones wrote:
> > > On Sat, Nov 05, 2016 at 06:46:34PM +0200, Marcel Apfelbaum wrote:
> > > > On 11/03/2016 09:40 PM, Michael S. Tsirkin wrote:
> > > > > On Thu, Nov 03, 2016 at 01:05:44PM +0200, Marcel Apfelbaum wrote:
> > > > > > On 11/03/2016 06:18 AM, Michael S. Tsirkin wrote:
> > > > > > > On Wed, Nov 02, 2016 at 05:16:42PM +0200, 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
> > > > > > 
> > > > > > Hi Michael
> > > > > > 
> > > > > > > 
> > > > > > > at the cost of breaking all hotplug for all non-acpi users.
> > > > > > > 
> > > > > > 
> > > > > > Do we have a non-acpi user that is able to use the shpc component as-is today?
> > > > > 
> > > > > power and some arm systems I guess?
> > > > > 
> > > > 
> > > > Adding Andrew , maybe he can give us an answer.
> > > 
> > > Not really :-) My lack of PCI knowledge makes that difficult. I'd be happy
> > > to help with an experiment though. Can you give me command line arguments,
> > > qmp commands, etc. that I should use to try it out? I imagine I should
> > > just boot an ARM guest using DT (instead of ACPI) and then attempt to
> > > hotplug a PCI device. I'm not sure, however, what, if any, special
> > > configuration I need in order to ensure I'm testing what you're
> > > interested in.
> > > 
> > 
> > Hi Drew,
> > 
> > 
> > Just run QEMU with '-device pci-bridge,chassis_nr=1,id=bridge1 -monitor stdio'
> > with an ARM guest using DT and wait until the guest finish booting.
> > 
> > Then run at hmp:
> > device_add virtio-net-pci,bus=bridge1,id=net2
> > 
> > Next run lspci in the guest to see the new device.
> 
> Thanks for the instructions Marcel. Here's the results
> 
>  $QEMU -machine virt,accel=$ACCEL -cpu $CPU -nographic -m 4096 -smp 8 \
>        -bios /usr/share/AAVMF/AAVMF_CODE.fd \
>        -device pci-bridge,chassis_nr=1,id=bridge1 \
>        -drive file=$FEDORA_IMG,if=none,id=dr0,format=qcow2 \
>        -device virtio-blk-pci,bus=bridge1,addr=01,drive=dr0,id=disk0 \
>        -netdev user,id=hostnet0 \
>        -device virtio-net-pci,bus=bridge1,addr=02,netdev=hostnet0,id=net0
> 
>  # lspci
>  00:00.0 Host bridge: Red Hat, Inc. Device 0008
>  00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
>  01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
>  01:02.0 Ethernet controller: Red Hat, Inc Virtio network device
> 
>  (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
>  Unsupported PCI slot 0 for standard hotplug controller. Valid slots are
>  between 1 and 31.
> 
> (Tried again giving addr=03)
> 
>  (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=03
> 
> (Seemed to work, but...)
> 
>  # lspci
>  00:00.0 Host bridge: Red Hat, Inc. Device 0008
>  00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
>  01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
>  01:02.0 Ethernet controller: Red Hat, Inc Virtio network device
> 
> (Doesn't show up in lscpi. So I guess it doesn't work)

Yes - I just realized people seem to disable CONFIG_HOTPLUG_PCI_SHPC
there. I don't know why really - want to try rebuilding the kernel
and seeing if there's any change?


> > 
> > 
> > BTW, will an ARM guest run 'fast' enough to be usable on a x86 machine?
> > If yes, any pointers on how to create such a guest?
> 
> You can run AArch64 guests on x86 machines. It's not super fast though...
> Certainly I wouldn't want to create my guest image using TCG. So, assuming
> you acquire an image somewhere (or create it on a real machine), then you
> can use the above command line, just change 
> 
> ACCEL=kvm CPU=host to ACCEL=tcg CPU=cortex-a57
> 
> Thanks,
> drew
Laurent Vivier Nov. 22, 2016, 8:25 p.m. UTC | #16
On 22/11/2016 18:26, Marcel Apfelbaum wrote:
> On 11/18/2016 05:52 PM, Andrew Jones wrote:
>> On Wed, Nov 16, 2016 at 07:05:25PM +0200, Marcel Apfelbaum wrote:
>>> On 11/16/2016 06:44 PM, Andrew Jones wrote:
>>>> On Sat, Nov 05, 2016 at 06:46:34PM +0200, Marcel Apfelbaum wrote:
>>>>> On 11/03/2016 09:40 PM, Michael S. Tsirkin wrote:
>>>>>> On Thu, Nov 03, 2016 at 01:05:44PM +0200, Marcel Apfelbaum wrote:
>>>>>>> On 11/03/2016 06:18 AM, Michael S. Tsirkin wrote:
>>>>>>>> On Wed, Nov 02, 2016 at 05:16:42PM +0200, 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
>>>>>>>
>>>>>>> Hi Michael
>>>>>>>
>>>>>>>>
>>>>>>>> at the cost of breaking all hotplug for all non-acpi users.
>>>>>>>>
>>>>>>>
>>>>>>> Do we have a non-acpi user that is able to use the shpc component
>>>>>>> as-is today?
>>>>>>
>>>>>> power and some arm systems I guess?
>>>>>>
>>>>>
>>>>> Adding Andrew , maybe he can give us an answer.
>>>>
>>>> Not really :-) My lack of PCI knowledge makes that difficult. I'd be
>>>> happy
>>>> to help with an experiment though. Can you give me command line
>>>> arguments,
>>>> qmp commands, etc. that I should use to try it out? I imagine I should
>>>> just boot an ARM guest using DT (instead of ACPI) and then attempt to
>>>> hotplug a PCI device. I'm not sure, however, what, if any, special
>>>> configuration I need in order to ensure I'm testing what you're
>>>> interested in.
>>>>
>>>
>>> Hi Drew,
>>>
>>>
>>> Just run QEMU with '-device pci-bridge,chassis_nr=1,id=bridge1
>>> -monitor stdio'
>>> with an ARM guest using DT and wait until the guest finish booting.
>>>
>>> Then run at hmp:
>>> device_add virtio-net-pci,bus=bridge1,id=net2
>>>
>>> Next run lspci in the guest to see the new device.
>>
>> Thanks for the instructions Marcel. Here's the results
>>
>>  $QEMU -machine virt,accel=$ACCEL -cpu $CPU -nographic -m 4096 -smp 8 \
>>        -bios /usr/share/AAVMF/AAVMF_CODE.fd \
>>        -device pci-bridge,chassis_nr=1,id=bridge1 \
>>        -drive file=$FEDORA_IMG,if=none,id=dr0,format=qcow2 \
>>        -device virtio-blk-pci,bus=bridge1,addr=01,drive=dr0,id=disk0 \
>>        -netdev user,id=hostnet0 \
>>        -device virtio-net-pci,bus=bridge1,addr=02,netdev=hostnet0,id=net0
>>
>>  # lspci
>>  00:00.0 Host bridge: Red Hat, Inc. Device 0008
>>  00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
>>  01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
>>  01:02.0 Ethernet controller: Red Hat, Inc Virtio network device
>>
>>  (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
>>  Unsupported PCI slot 0 for standard hotplug controller. Valid slots are
>>  between 1 and 31.
>>
>> (Tried again giving addr=03)
>>
>>  (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=03
>>
>> (Seemed to work, but...)
>>
>>  # lspci
>>  00:00.0 Host bridge: Red Hat, Inc. Device 0008
>>  00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
>>  01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
>>  01:02.0 Ethernet controller: Red Hat, Inc Virtio network device
>>
>> (Doesn't show up in lscpi. So I guess it doesn't work)
>>
> 
> Hi Drew,
> Thanks for confirming that it doesn't work.
> 
> Michael asked if we can check the same for powerpc before
> disabling the shpc by default.
> 
> Adding David, Thomas and Laurrent, maybe they have time
> to check it for powerpc.

With this patch:

# lspci
00:00.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
00:03.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
[root@dhcp6-56 ~]# QEMU 2.7.90 monitor - type 'help' for more information
(qemu) device_add virtio-net-pci,bus=bridge1,id=net2
Bus 'bridge1' does not support hotplugging
(qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=3
Bus 'bridge1' does not support hotplugging

Without this patch:

# lspci
00:00.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
00:03.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
[root@dhcp6-56 ~]# QEMU 2.7.90 monitor - type 'help' for more information
(qemu) device_add virtio-net-pci,bus=bridge1,id=net2
Unsupported PCI slot 0 for standard hotplug controller. Valid slots are
between 1 and 31.
(qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=3
(qemu) [   37.342908] shpchp 0000:00:00.0: Latch close on Slot(3)
[   37.342963] shpchp 0000:00:00.0: Button pressed on Slot(3)
[   37.343003] shpchp 0000:00:00.0: Card present on Slot(3)
[   37.344186] shpchp 0000:00:00.0: PCI slot #3 - powering on due to
button press
[   43.361827] shpchp 0000:00:00.0: No new device found
[   43.361867] shpchp 0000:00:00.0: Cannot add device at 0000:01:03
[   43.363277] shpchp 0000:00:00.0: Latch open on Slot(3)
[   43.363320] shpchp 0000:00:00.0: Button pressed on Slot(3)
[   43.363360] shpchp 0000:00:00.0: Card not present on Slot(3)
[   43.363506] shpchp 0000:00:00.0: PCI slot #3 - powering on due to
button press
[   48.371835] shpchp 0000:00:00.0: No adapter on slot(3)

Laurent
Marcel Apfelbaum Nov. 23, 2016, 11:08 a.m. UTC | #17
On 11/22/2016 10:25 PM, Laurent Vivier wrote:
>
>
> On 22/11/2016 18:26, Marcel Apfelbaum wrote:
>> On 11/18/2016 05:52 PM, Andrew Jones wrote:
>>> On Wed, Nov 16, 2016 at 07:05:25PM +0200, Marcel Apfelbaum wrote:
>>>> On 11/16/2016 06:44 PM, Andrew Jones wrote:
>>>>> On Sat, Nov 05, 2016 at 06:46:34PM +0200, Marcel Apfelbaum wrote:
>>>>>> On 11/03/2016 09:40 PM, Michael S. Tsirkin wrote:
>>>>>>> On Thu, Nov 03, 2016 at 01:05:44PM +0200, Marcel Apfelbaum wrote:
>>>>>>>> On 11/03/2016 06:18 AM, Michael S. Tsirkin wrote:
>>>>>>>>> On Wed, Nov 02, 2016 at 05:16:42PM +0200, 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
>>>>>>>>
>>>>>>>> Hi Michael
>>>>>>>>
>>>>>>>>>
>>>>>>>>> at the cost of breaking all hotplug for all non-acpi users.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Do we have a non-acpi user that is able to use the shpc component
>>>>>>>> as-is today?
>>>>>>>
>>>>>>> power and some arm systems I guess?
>>>>>>>
>>>>>>
>>>>>> Adding Andrew , maybe he can give us an answer.
>>>>>
>>>>> Not really :-) My lack of PCI knowledge makes that difficult. I'd be
>>>>> happy
>>>>> to help with an experiment though. Can you give me command line
>>>>> arguments,
>>>>> qmp commands, etc. that I should use to try it out? I imagine I should
>>>>> just boot an ARM guest using DT (instead of ACPI) and then attempt to
>>>>> hotplug a PCI device. I'm not sure, however, what, if any, special
>>>>> configuration I need in order to ensure I'm testing what you're
>>>>> interested in.
>>>>>
>>>>
>>>> Hi Drew,
>>>>
>>>>
>>>> Just run QEMU with '-device pci-bridge,chassis_nr=1,id=bridge1
>>>> -monitor stdio'
>>>> with an ARM guest using DT and wait until the guest finish booting.
>>>>
>>>> Then run at hmp:
>>>> device_add virtio-net-pci,bus=bridge1,id=net2
>>>>
>>>> Next run lspci in the guest to see the new device.
>>>
>>> Thanks for the instructions Marcel. Here's the results
>>>
>>>  $QEMU -machine virt,accel=$ACCEL -cpu $CPU -nographic -m 4096 -smp 8 \
>>>        -bios /usr/share/AAVMF/AAVMF_CODE.fd \
>>>        -device pci-bridge,chassis_nr=1,id=bridge1 \
>>>        -drive file=$FEDORA_IMG,if=none,id=dr0,format=qcow2 \
>>>        -device virtio-blk-pci,bus=bridge1,addr=01,drive=dr0,id=disk0 \
>>>        -netdev user,id=hostnet0 \
>>>        -device virtio-net-pci,bus=bridge1,addr=02,netdev=hostnet0,id=net0
>>>
>>>  # lspci
>>>  00:00.0 Host bridge: Red Hat, Inc. Device 0008
>>>  00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
>>>  01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
>>>  01:02.0 Ethernet controller: Red Hat, Inc Virtio network device
>>>
>>>  (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
>>>  Unsupported PCI slot 0 for standard hotplug controller. Valid slots are
>>>  between 1 and 31.
>>>
>>> (Tried again giving addr=03)
>>>
>>>  (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=03
>>>
>>> (Seemed to work, but...)
>>>
>>>  # lspci
>>>  00:00.0 Host bridge: Red Hat, Inc. Device 0008
>>>  00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
>>>  01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
>>>  01:02.0 Ethernet controller: Red Hat, Inc Virtio network device
>>>
>>> (Doesn't show up in lscpi. So I guess it doesn't work)
>>>
>>
>> Hi Drew,
>> Thanks for confirming that it doesn't work.
>>
>> Michael asked if we can check the same for powerpc before
>> disabling the shpc by default.
>>
>> Adding David, Thomas and Laurrent, maybe they have time
>> to check it for powerpc.
>
> With this patch:
>
> # lspci
> 00:00.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
> 00:03.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
> [root@dhcp6-56 ~]# QEMU 2.7.90 monitor - type 'help' for more information
> (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
> Bus 'bridge1' does not support hotplugging
> (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=3
> Bus 'bridge1' does not support hotplugging
>
> Without this patch:
>
> # lspci
> 00:00.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
> 00:03.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
> [root@dhcp6-56 ~]# QEMU 2.7.90 monitor - type 'help' for more information
> (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
> Unsupported PCI slot 0 for standard hotplug controller. Valid slots are
> between 1 and 31.
> (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=3
> (qemu) [   37.342908] shpchp 0000:00:00.0: Latch close on Slot(3)
> [   37.342963] shpchp 0000:00:00.0: Button pressed on Slot(3)
> [   37.343003] shpchp 0000:00:00.0: Card present on Slot(3)
> [   37.344186] shpchp 0000:00:00.0: PCI slot #3 - powering on due to
> button press
> [   43.361827] shpchp 0000:00:00.0: No new device found


Hi Laurrent,
Thank you for the fast reply!

What I see from the log is that ppc does use the shpc for hotplug,
but there is some implementation bug preventing it to work.

Drew, may I trouble one last time for the dmesg?
I think we would see exactly the same log.


Thanks,
Marcel

> [   43.361867] shpchp 0000:00:00.0: Cannot add device at 0000:01:03
> [   43.363277] shpchp 0000:00:00.0: Latch open on Slot(3)
> [   43.363320] shpchp 0000:00:00.0: Button pressed on Slot(3)
> [   43.363360] shpchp 0000:00:00.0: Card not present on Slot(3)
> [   43.363506] shpchp 0000:00:00.0: PCI slot #3 - powering on due to
> button press
> [   48.371835] shpchp 0000:00:00.0: No adapter on slot(3)
>
> Laurent
>
David Gibson Nov. 24, 2016, 4:06 a.m. UTC | #18
On Wed, Nov 23, 2016 at 01:08:46PM +0200, Marcel Apfelbaum wrote:
> On 11/22/2016 10:25 PM, Laurent Vivier wrote:
> > 
> > 
> > On 22/11/2016 18:26, Marcel Apfelbaum wrote:
> > > On 11/18/2016 05:52 PM, Andrew Jones wrote:
> > > > On Wed, Nov 16, 2016 at 07:05:25PM +0200, Marcel Apfelbaum wrote:
> > > > > On 11/16/2016 06:44 PM, Andrew Jones wrote:
> > > > > > On Sat, Nov 05, 2016 at 06:46:34PM +0200, Marcel Apfelbaum wrote:
> > > > > > > On 11/03/2016 09:40 PM, Michael S. Tsirkin wrote:
> > > > > > > > On Thu, Nov 03, 2016 at 01:05:44PM +0200, Marcel Apfelbaum wrote:
> > > > > > > > > On 11/03/2016 06:18 AM, Michael S. Tsirkin wrote:
> > > > > > > > > > On Wed, Nov 02, 2016 at 05:16:42PM +0200, 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
> > > > > > > > > 
> > > > > > > > > Hi Michael
> > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > at the cost of breaking all hotplug for all non-acpi users.
> > > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Do we have a non-acpi user that is able to use the shpc component
> > > > > > > > > as-is today?
> > > > > > > > 
> > > > > > > > power and some arm systems I guess?
> > > > > > > > 
> > > > > > > 
> > > > > > > Adding Andrew , maybe he can give us an answer.
> > > > > > 
> > > > > > Not really :-) My lack of PCI knowledge makes that difficult. I'd be
> > > > > > happy
> > > > > > to help with an experiment though. Can you give me command line
> > > > > > arguments,
> > > > > > qmp commands, etc. that I should use to try it out? I imagine I should
> > > > > > just boot an ARM guest using DT (instead of ACPI) and then attempt to
> > > > > > hotplug a PCI device. I'm not sure, however, what, if any, special
> > > > > > configuration I need in order to ensure I'm testing what you're
> > > > > > interested in.
> > > > > > 
> > > > > 
> > > > > Hi Drew,
> > > > > 
> > > > > 
> > > > > Just run QEMU with '-device pci-bridge,chassis_nr=1,id=bridge1
> > > > > -monitor stdio'
> > > > > with an ARM guest using DT and wait until the guest finish booting.
> > > > > 
> > > > > Then run at hmp:
> > > > > device_add virtio-net-pci,bus=bridge1,id=net2
> > > > > 
> > > > > Next run lspci in the guest to see the new device.
> > > > 
> > > > Thanks for the instructions Marcel. Here's the results
> > > > 
> > > >  $QEMU -machine virt,accel=$ACCEL -cpu $CPU -nographic -m 4096 -smp 8 \
> > > >        -bios /usr/share/AAVMF/AAVMF_CODE.fd \
> > > >        -device pci-bridge,chassis_nr=1,id=bridge1 \
> > > >        -drive file=$FEDORA_IMG,if=none,id=dr0,format=qcow2 \
> > > >        -device virtio-blk-pci,bus=bridge1,addr=01,drive=dr0,id=disk0 \
> > > >        -netdev user,id=hostnet0 \
> > > >        -device virtio-net-pci,bus=bridge1,addr=02,netdev=hostnet0,id=net0
> > > > 
> > > >  # lspci
> > > >  00:00.0 Host bridge: Red Hat, Inc. Device 0008
> > > >  00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
> > > >  01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
> > > >  01:02.0 Ethernet controller: Red Hat, Inc Virtio network device
> > > > 
> > > >  (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
> > > >  Unsupported PCI slot 0 for standard hotplug controller. Valid slots are
> > > >  between 1 and 31.
> > > > 
> > > > (Tried again giving addr=03)
> > > > 
> > > >  (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=03
> > > > 
> > > > (Seemed to work, but...)
> > > > 
> > > >  # lspci
> > > >  00:00.0 Host bridge: Red Hat, Inc. Device 0008
> > > >  00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
> > > >  01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
> > > >  01:02.0 Ethernet controller: Red Hat, Inc Virtio network device
> > > > 
> > > > (Doesn't show up in lscpi. So I guess it doesn't work)
> > > > 
> > > 
> > > Hi Drew,
> > > Thanks for confirming that it doesn't work.
> > > 
> > > Michael asked if we can check the same for powerpc before
> > > disabling the shpc by default.
> > > 
> > > Adding David, Thomas and Laurrent, maybe they have time
> > > to check it for powerpc.
> > 
> > With this patch:
> > 
> > # lspci
> > 00:00.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
> > 00:03.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
> > [root@dhcp6-56 ~]# QEMU 2.7.90 monitor - type 'help' for more information
> > (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
> > Bus 'bridge1' does not support hotplugging
> > (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=3
> > Bus 'bridge1' does not support hotplugging
> > 
> > Without this patch:
> > 
> > # lspci
> > 00:00.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
> > 00:03.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
> > [root@dhcp6-56 ~]# QEMU 2.7.90 monitor - type 'help' for more information
> > (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
> > Unsupported PCI slot 0 for standard hotplug controller. Valid slots are
> > between 1 and 31.
> > (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=3
> > (qemu) [   37.342908] shpchp 0000:00:00.0: Latch close on Slot(3)
> > [   37.342963] shpchp 0000:00:00.0: Button pressed on Slot(3)
> > [   37.343003] shpchp 0000:00:00.0: Card present on Slot(3)
> > [   37.344186] shpchp 0000:00:00.0: PCI slot #3 - powering on due to
> > button press
> > [   43.361827] shpchp 0000:00:00.0: No new device found
> 
> 
> Hi Laurrent,
> Thank you for the fast reply!
> 
> What I see from the log is that ppc does use the shpc for hotplug,
> but there is some implementation bug preventing it to work.

That doesn't sound right.  ppc, or more correctly the pseries machine
type, has it's own hotplug notification mechanism.  If we're trying to
invoke shpc, I think something is wrong.

> Drew, may I trouble one last time for the dmesg?
> I think we would see exactly the same log.
> 
> 
> Thanks,
> Marcel
> 
> > [   43.361867] shpchp 0000:00:00.0: Cannot add device at 0000:01:03
> > [   43.363277] shpchp 0000:00:00.0: Latch open on Slot(3)
> > [   43.363320] shpchp 0000:00:00.0: Button pressed on Slot(3)
> > [   43.363360] shpchp 0000:00:00.0: Card not present on Slot(3)
> > [   43.363506] shpchp 0000:00:00.0: PCI slot #3 - powering on due to
> > button press
> > [   48.371835] shpchp 0000:00:00.0: No adapter on slot(3)
> > 
> > Laurent
> > 
>
Marcel Apfelbaum Nov. 24, 2016, 9:39 a.m. UTC | #19
On 11/24/2016 06:06 AM, David Gibson wrote:
> On Wed, Nov 23, 2016 at 01:08:46PM +0200, Marcel Apfelbaum wrote:
>> On 11/22/2016 10:25 PM, Laurent Vivier wrote:
>>>
>>>
>>> On 22/11/2016 18:26, Marcel Apfelbaum wrote:
>>>> On 11/18/2016 05:52 PM, Andrew Jones wrote:
>>>>> On Wed, Nov 16, 2016 at 07:05:25PM +0200, Marcel Apfelbaum wrote:
>>>>>> On 11/16/2016 06:44 PM, Andrew Jones wrote:
>>>>>>> On Sat, Nov 05, 2016 at 06:46:34PM +0200, Marcel Apfelbaum wrote:
>>>>>>>> On 11/03/2016 09:40 PM, Michael S. Tsirkin wrote:
>>>>>>>>> On Thu, Nov 03, 2016 at 01:05:44PM +0200, Marcel Apfelbaum wrote:
>>>>>>>>>> On 11/03/2016 06:18 AM, Michael S. Tsirkin wrote:
>>>>>>>>>>> On Wed, Nov 02, 2016 at 05:16:42PM +0200, 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
>>>>>>>>>>
>>>>>>>>>> Hi Michael
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> at the cost of breaking all hotplug for all non-acpi users.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Do we have a non-acpi user that is able to use the shpc component
>>>>>>>>>> as-is today?
>>>>>>>>>
>>>>>>>>> power and some arm systems I guess?
>>>>>>>>>
>>>>>>>>
>>>>>>>> Adding Andrew , maybe he can give us an answer.
>>>>>>>
>>>>>>> Not really :-) My lack of PCI knowledge makes that difficult. I'd be
>>>>>>> happy
>>>>>>> to help with an experiment though. Can you give me command line
>>>>>>> arguments,
>>>>>>> qmp commands, etc. that I should use to try it out? I imagine I should
>>>>>>> just boot an ARM guest using DT (instead of ACPI) and then attempt to
>>>>>>> hotplug a PCI device. I'm not sure, however, what, if any, special
>>>>>>> configuration I need in order to ensure I'm testing what you're
>>>>>>> interested in.
>>>>>>>
>>>>>>
>>>>>> Hi Drew,
>>>>>>
>>>>>>
>>>>>> Just run QEMU with '-device pci-bridge,chassis_nr=1,id=bridge1
>>>>>> -monitor stdio'
>>>>>> with an ARM guest using DT and wait until the guest finish booting.
>>>>>>
>>>>>> Then run at hmp:
>>>>>> device_add virtio-net-pci,bus=bridge1,id=net2
>>>>>>
>>>>>> Next run lspci in the guest to see the new device.
>>>>>
>>>>> Thanks for the instructions Marcel. Here's the results
>>>>>
>>>>>  $QEMU -machine virt,accel=$ACCEL -cpu $CPU -nographic -m 4096 -smp 8 \
>>>>>        -bios /usr/share/AAVMF/AAVMF_CODE.fd \
>>>>>        -device pci-bridge,chassis_nr=1,id=bridge1 \
>>>>>        -drive file=$FEDORA_IMG,if=none,id=dr0,format=qcow2 \
>>>>>        -device virtio-blk-pci,bus=bridge1,addr=01,drive=dr0,id=disk0 \
>>>>>        -netdev user,id=hostnet0 \
>>>>>        -device virtio-net-pci,bus=bridge1,addr=02,netdev=hostnet0,id=net0
>>>>>
>>>>>  # lspci
>>>>>  00:00.0 Host bridge: Red Hat, Inc. Device 0008
>>>>>  00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
>>>>>  01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
>>>>>  01:02.0 Ethernet controller: Red Hat, Inc Virtio network device
>>>>>
>>>>>  (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
>>>>>  Unsupported PCI slot 0 for standard hotplug controller. Valid slots are
>>>>>  between 1 and 31.
>>>>>
>>>>> (Tried again giving addr=03)
>>>>>
>>>>>  (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=03
>>>>>
>>>>> (Seemed to work, but...)
>>>>>
>>>>>  # lspci
>>>>>  00:00.0 Host bridge: Red Hat, Inc. Device 0008
>>>>>  00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
>>>>>  01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
>>>>>  01:02.0 Ethernet controller: Red Hat, Inc Virtio network device
>>>>>
>>>>> (Doesn't show up in lscpi. So I guess it doesn't work)
>>>>>
>>>>
>>>> Hi Drew,
>>>> Thanks for confirming that it doesn't work.
>>>>
>>>> Michael asked if we can check the same for powerpc before
>>>> disabling the shpc by default.
>>>>
>>>> Adding David, Thomas and Laurrent, maybe they have time
>>>> to check it for powerpc.
>>>
>>> With this patch:
>>>
>>> # lspci
>>> 00:00.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
>>> 00:03.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
>>> [root@dhcp6-56 ~]# QEMU 2.7.90 monitor - type 'help' for more information
>>> (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
>>> Bus 'bridge1' does not support hotplugging
>>> (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=3
>>> Bus 'bridge1' does not support hotplugging
>>>
>>> Without this patch:
>>>
>>> # lspci
>>> 00:00.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
>>> 00:03.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
>>> [root@dhcp6-56 ~]# QEMU 2.7.90 monitor - type 'help' for more information
>>> (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
>>> Unsupported PCI slot 0 for standard hotplug controller. Valid slots are
>>> between 1 and 31.
>>> (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=3
>>> (qemu) [   37.342908] shpchp 0000:00:00.0: Latch close on Slot(3)
>>> [   37.342963] shpchp 0000:00:00.0: Button pressed on Slot(3)
>>> [   37.343003] shpchp 0000:00:00.0: Card present on Slot(3)
>>> [   37.344186] shpchp 0000:00:00.0: PCI slot #3 - powering on due to
>>> button press
>>> [   43.361827] shpchp 0000:00:00.0: No new device found
>>
>>
>> Hi Laurrent,
>> Thank you for the fast reply!
>>
>> What I see from the log is that ppc does use the shpc for hotplug,
>> but there is some implementation bug preventing it to work.
>
> That doesn't sound right.  ppc, or more correctly the pseries machine
> type, has it's own hotplug notification mechanism.  If we're trying to
> invoke shpc, I think something is wrong.
>

Hi David,

As you can see the shpc driver claims the pci-bridge slots and it responds
to the hot-plug events.

Anyway, what is the hotplug mechanism preferred by the pseries machine ?

Side note: what we want to understand is if using pci-bridges's shpc  by default
would help non x86 archs (since QEMU x86 machine uses ACPI based hot-plug).

Thanks,
Marcel

>> Drew, may I trouble one last time for the dmesg?
>> I think we would see exactly the same log.
>>
>>
>> Thanks,
>> Marcel
>>
>>> [   43.361867] shpchp 0000:00:00.0: Cannot add device at 0000:01:03
>>> [   43.363277] shpchp 0000:00:00.0: Latch open on Slot(3)
>>> [   43.363320] shpchp 0000:00:00.0: Button pressed on Slot(3)
>>> [   43.363360] shpchp 0000:00:00.0: Card not present on Slot(3)
>>> [   43.363506] shpchp 0000:00:00.0: PCI slot #3 - powering on due to
>>> button press
>>> [   48.371835] shpchp 0000:00:00.0: No adapter on slot(3)
>>>
>>> Laurent
>>>
>>
>
David Gibson Nov. 25, 2016, 4:14 a.m. UTC | #20
On Thu, Nov 24, 2016 at 11:39:25AM +0200, Marcel Apfelbaum wrote:
> On 11/24/2016 06:06 AM, David Gibson wrote:
> > On Wed, Nov 23, 2016 at 01:08:46PM +0200, Marcel Apfelbaum wrote:
> > > On 11/22/2016 10:25 PM, Laurent Vivier wrote:
> > > > 
> > > > 
> > > > On 22/11/2016 18:26, Marcel Apfelbaum wrote:
> > > > > On 11/18/2016 05:52 PM, Andrew Jones wrote:
> > > > > > On Wed, Nov 16, 2016 at 07:05:25PM +0200, Marcel Apfelbaum wrote:
> > > > > > > On 11/16/2016 06:44 PM, Andrew Jones wrote:
> > > > > > > > On Sat, Nov 05, 2016 at 06:46:34PM +0200, Marcel Apfelbaum wrote:
> > > > > > > > > On 11/03/2016 09:40 PM, Michael S. Tsirkin wrote:
> > > > > > > > > > On Thu, Nov 03, 2016 at 01:05:44PM +0200, Marcel Apfelbaum wrote:
> > > > > > > > > > > On 11/03/2016 06:18 AM, Michael S. Tsirkin wrote:
> > > > > > > > > > > > On Wed, Nov 02, 2016 at 05:16:42PM +0200, 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
> > > > > > > > > > > 
> > > > > > > > > > > Hi Michael
> > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > at the cost of breaking all hotplug for all non-acpi users.
> > > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > Do we have a non-acpi user that is able to use the shpc component
> > > > > > > > > > > as-is today?
> > > > > > > > > > 
> > > > > > > > > > power and some arm systems I guess?
> > > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Adding Andrew , maybe he can give us an answer.
> > > > > > > > 
> > > > > > > > Not really :-) My lack of PCI knowledge makes that difficult. I'd be
> > > > > > > > happy
> > > > > > > > to help with an experiment though. Can you give me command line
> > > > > > > > arguments,
> > > > > > > > qmp commands, etc. that I should use to try it out? I imagine I should
> > > > > > > > just boot an ARM guest using DT (instead of ACPI) and then attempt to
> > > > > > > > hotplug a PCI device. I'm not sure, however, what, if any, special
> > > > > > > > configuration I need in order to ensure I'm testing what you're
> > > > > > > > interested in.
> > > > > > > > 
> > > > > > > 
> > > > > > > Hi Drew,
> > > > > > > 
> > > > > > > 
> > > > > > > Just run QEMU with '-device pci-bridge,chassis_nr=1,id=bridge1
> > > > > > > -monitor stdio'
> > > > > > > with an ARM guest using DT and wait until the guest finish booting.
> > > > > > > 
> > > > > > > Then run at hmp:
> > > > > > > device_add virtio-net-pci,bus=bridge1,id=net2
> > > > > > > 
> > > > > > > Next run lspci in the guest to see the new device.
> > > > > > 
> > > > > > Thanks for the instructions Marcel. Here's the results
> > > > > > 
> > > > > >  $QEMU -machine virt,accel=$ACCEL -cpu $CPU -nographic -m 4096 -smp 8 \
> > > > > >        -bios /usr/share/AAVMF/AAVMF_CODE.fd \
> > > > > >        -device pci-bridge,chassis_nr=1,id=bridge1 \
> > > > > >        -drive file=$FEDORA_IMG,if=none,id=dr0,format=qcow2 \
> > > > > >        -device virtio-blk-pci,bus=bridge1,addr=01,drive=dr0,id=disk0 \
> > > > > >        -netdev user,id=hostnet0 \
> > > > > >        -device virtio-net-pci,bus=bridge1,addr=02,netdev=hostnet0,id=net0
> > > > > > 
> > > > > >  # lspci
> > > > > >  00:00.0 Host bridge: Red Hat, Inc. Device 0008
> > > > > >  00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
> > > > > >  01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
> > > > > >  01:02.0 Ethernet controller: Red Hat, Inc Virtio network device
> > > > > > 
> > > > > >  (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
> > > > > >  Unsupported PCI slot 0 for standard hotplug controller. Valid slots are
> > > > > >  between 1 and 31.
> > > > > > 
> > > > > > (Tried again giving addr=03)
> > > > > > 
> > > > > >  (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=03
> > > > > > 
> > > > > > (Seemed to work, but...)
> > > > > > 
> > > > > >  # lspci
> > > > > >  00:00.0 Host bridge: Red Hat, Inc. Device 0008
> > > > > >  00:01.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
> > > > > >  01:01.0 SCSI storage controller: Red Hat, Inc Virtio block device
> > > > > >  01:02.0 Ethernet controller: Red Hat, Inc Virtio network device
> > > > > > 
> > > > > > (Doesn't show up in lscpi. So I guess it doesn't work)
> > > > > > 
> > > > > 
> > > > > Hi Drew,
> > > > > Thanks for confirming that it doesn't work.
> > > > > 
> > > > > Michael asked if we can check the same for powerpc before
> > > > > disabling the shpc by default.
> > > > > 
> > > > > Adding David, Thomas and Laurrent, maybe they have time
> > > > > to check it for powerpc.
> > > > 
> > > > With this patch:
> > > > 
> > > > # lspci
> > > > 00:00.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
> > > > 00:03.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
> > > > [root@dhcp6-56 ~]# QEMU 2.7.90 monitor - type 'help' for more information
> > > > (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
> > > > Bus 'bridge1' does not support hotplugging
> > > > (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=3
> > > > Bus 'bridge1' does not support hotplugging
> > > > 
> > > > Without this patch:
> > > > 
> > > > # lspci
> > > > 00:00.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
> > > > 00:03.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
> > > > [root@dhcp6-56 ~]# QEMU 2.7.90 monitor - type 'help' for more information
> > > > (qemu) device_add virtio-net-pci,bus=bridge1,id=net2
> > > > Unsupported PCI slot 0 for standard hotplug controller. Valid slots are
> > > > between 1 and 31.
> > > > (qemu) device_add virtio-net-pci,bus=bridge1,id=net2,addr=3
> > > > (qemu) [   37.342908] shpchp 0000:00:00.0: Latch close on Slot(3)
> > > > [   37.342963] shpchp 0000:00:00.0: Button pressed on Slot(3)
> > > > [   37.343003] shpchp 0000:00:00.0: Card present on Slot(3)
> > > > [   37.344186] shpchp 0000:00:00.0: PCI slot #3 - powering on due to
> > > > button press
> > > > [   43.361827] shpchp 0000:00:00.0: No new device found
> > > 
> > > 
> > > Hi Laurrent,
> > > Thank you for the fast reply!
> > > 
> > > What I see from the log is that ppc does use the shpc for hotplug,
> > > but there is some implementation bug preventing it to work.
> > 
> > That doesn't sound right.  ppc, or more correctly the pseries machine
> > type, has it's own hotplug notification mechanism.  If we're trying to
> > invoke shpc, I think something is wrong.
> > 
> 
> Hi David,
> 
> As you can see the shpc driver claims the pci-bridge slots and it responds
> to the hot-plug events.

Yeah.. that's a bug of itself.  shpc can't work properly on PAPR
guests (or even on IBM POWER hardware hosts, for related though not
identical reasons).

In addition to PAPR having its own hotplug mechanism, the shpc stuff
apparently conflicts with the (also POWER specific) EEH error recovery
mechanism.

> Anyway, what is the hotplug mechanism preferred by the pseries machine ?

The PAPR specification which defines the firmware and hypervisor
interfaces that guests on POWER operate under, has its own hotplug
mechanism, using hypervisor events to deliver updated device tree
fragments to the guest.

> Side note: what we want to understand is if using pci-bridges's shpc  by default
> would help non x86 archs (since QEMU x86 machine uses ACPI based
> hot-plug).

Certainly not for pseries or powernv.  It might be correct for (some?)
embedded ppc machines with PCI-E

> 
> Thanks,
> Marcel
> 
> > > Drew, may I trouble one last time for the dmesg?
> > > I think we would see exactly the same log.
> > > 
> > > 
> > > Thanks,
> > > Marcel
> > > 
> > > > [   43.361867] shpchp 0000:00:00.0: Cannot add device at 0000:01:03
> > > > [   43.363277] shpchp 0000:00:00.0: Latch open on Slot(3)
> > > > [   43.363320] shpchp 0000:00:00.0: Button pressed on Slot(3)
> > > > [   43.363360] shpchp 0000:00:00.0: Card not present on Slot(3)
> > > > [   43.363506] shpchp 0000:00:00.0: PCI slot #3 - powering on due to
> > > > button press
> > > > [   48.371835] shpchp 0000:00:00.0: No adapter on slot(3)
> > > > 
> > > > Laurent
> > > > 
> > > 
> > 
>
Michael S. Tsirkin Jan. 10, 2017, 3:48 a.m. UTC | #21
On Wed, Nov 02, 2016 at 05:16:42PM +0200, 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.
> 
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>

Can you pls post a rebase since compat changed?

> ---
>  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 \
> -- 
> 2.5.5
diff mbox

Patch

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 \