On 1/14/19 12:44 PM, Cornelia Huck wrote:
> [restored cc:s]
>
> On Mon, 14 Jan 2019 11:06:19 +0100
> Pierre Morel <pmo...@linux.ibm.com> wrote:
>
>> On 11/01/2019 10:38, Cornelia Huck wrote:
>>> On Fri, 11 Jan 2019 08:16:41 +0100
>>> David Hildenbrand <da...@redhat.com> wrote:
>>>
>>>> On 10.01.19 22:03, David Hildenbrand wrote:
>>>>> Comit 2c28c490571f ("s390x/pci: let pci devices start in configured mode")
>>>>> changed the initial state of zPCI devices from ZPCI_FS_STANDBY to
>>>>> ZPCI_FS_DISABLED (a.k.a. configured). However we still only send a
>>>>> HP_EVENT_RESERVED_TO_STANDBY event to the guest, indicating a wrong
>>>>> state.
>>>>>
>>>>> Let's send a HP_EVENT_TO_CONFIGURED event instead, to match the actual
>>>>> state the device is in.
>>>>>
>>>>> This fixes hotplugged devices having to be enabled explicitly in the
>>>>> guest e.g. via echo 1 > /sys/bus/pci/slots/00000000/power.
>>>>>
>>>>> Fixes: 2c28c490571f ("s390x/pci: let pci devices start in configured
>>>>> mode")
>>>>> Report-by: Cornelia Huck <coh...@redhat.com>
>>>
>>> Cool, works for me as well.
>>>
>>> Tested-by: Cornelia Huck <coh...@redhat.com>
>>>
>>> Do we want to cc:stable? Probably not, as it's more annoying than
>>> critical, and pci hotplug does not seem to be much used on s390x.
>>>
>>>>
>>>> If this patch is the right thing to do, then
>>>>
>>>> 1. s/Report-by/Reported-by/
>>>> 2. Dropping the "." from the subject
>>>>
>>>> (yes, it was late)
>>>
>>> :) Can do while applying.
>>>
>>>>
>>>> I wonder if we should do both events sequentially, but as I don't have
>>>> access to the architecture I have to rely on that this works :)
>>>
>>> Yep, let's wait for feedback from folks with architecture access.
>>>
>>
>> Works fine on the architecture too.
>>
>> Seems the logical thing to do for me.
>>
>> Reviewed-by: Pierre Morel<pmo...@linux.ibm.com>
>
> Thanks for checking.
>
> I'd like to queue this, but I'd like an ack from Collin as well.
>
Would you mind adding a comment somewhere that states something like
"we can safely bypass the standby state when PCI hotplugging for a guest"
just to be clear that QEMU is a bit different from how we handle it on the LPAR
level?
That comment would more-or-less clarify why we set the ZPCI_FS_<STATE> directly
to disabled instead of to standby when hotplugging (which, AFAIU, is the order
how things occur at the LPAR level)
Otherwise,
Reviewed-by: Collin Walling <wall...@linux.ibm.com>
>>
>>
>>>>
>>>>> Signed-off-by: David Hildenbrand <da...@redhat.com>
>>>>> ---
>>>>> hw/s390x/s390-pci-bus.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>>>>> index 15759b6514..7f911b216a 100644
>>>>> --- a/hw/s390x/s390-pci-bus.c
>>>>> +++ b/hw/s390x/s390-pci-bus.c
>>>>> @@ -899,7 +899,7 @@ static void s390_pcihost_plug(HotplugHandler
>>>>> *hotplug_dev, DeviceState *dev,
>>>>> }
>>>>>
>>>>> if (dev->hotplugged) {
>>>>> - s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
>>>>> + s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED ,
>>>>> pbdev->fh, pbdev->fid);
>>>>> }
>>>>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) {
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
>
--
Respectfully,
- Collin Walling
