[Qemu-devel] [PATCH v1] s390x/pci: Send correct event on hotplug.

[Qemu-devel] [PATCH v1] s390x/pci: Send correct event on hotplug.

[David Hildenbrand]

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 <cohuck@redhat.com>
Signed-off-by: David Hildenbrand <david@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)) {
-- 
2.17.2

Re: [Qemu-devel] [PATCH v1] s390x/pci: Send correct event on hotplug.

[David Hildenbrand]

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 <cohuck@redhat.com>

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)

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 :)

> Signed-off-by: David Hildenbrand <david@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)) {
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v1] s390x/pci: Send correct event on hotplug.

[Cornelia Huck]

On Fri, 11 Jan 2019 08:16:41 +0100
David Hildenbrand <david@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 <cohuck@redhat.com>  

Cool, works for me as well.

Tested-by: Cornelia Huck <cohuck@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.

> 
> > Signed-off-by: David Hildenbrand <david@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)) {
> >   
> 
> 

Re: [Qemu-devel] [PATCH v1] s390x/pci: Send correct event on hotplug.

[Pierre Morel]

On 11/01/2019 10:38, Cornelia Huck wrote:
> On Fri, 11 Jan 2019 08:16:41 +0100
> David Hildenbrand <david@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 <cohuck@redhat.com>
> 
> Cool, works for me as well.
> 
> Tested-by: Cornelia Huck <cohuck@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<pmorel@linux.ibm.com>


>>
>>> Signed-off-by: David Hildenbrand <david@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)) {
>>>    
>>
>>
> 
> 


-- 
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany

Re: [Qemu-devel] [PATCH v1] s390x/pci: Send correct event on hotplug.

[Cornelia Huck]

[restored cc:s]

On Mon, 14 Jan 2019 11:06:19 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

> On 11/01/2019 10:38, Cornelia Huck wrote:
> > On Fri, 11 Jan 2019 08:16:41 +0100
> > David Hildenbrand <david@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 <cohuck@redhat.com>  
> > 
> > Cool, works for me as well.
> > 
> > Tested-by: Cornelia Huck <cohuck@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<pmorel@linux.ibm.com>

Thanks for checking.

I'd like to queue this, but I'd like an ack from Collin as well.

> 
> 
> >>  
> >>> Signed-off-by: David Hildenbrand <david@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)) {
> >>>      
> >>
> >>  
> > 
> >   
> 
> 

Re: [Qemu-devel] [qemu-s390x] [PATCH v1] s390x/pci: Send correct event on hotplug.

[Collin Walling]

On 1/14/19 12:44 PM, Cornelia Huck wrote:
> [restored cc:s]
> 
> On Mon, 14 Jan 2019 11:06:19 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> On 11/01/2019 10:38, Cornelia Huck wrote:
>>> On Fri, 11 Jan 2019 08:16:41 +0100
>>> David Hildenbrand <david@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 <cohuck@redhat.com>  
>>>
>>> Cool, works for me as well.
>>>
>>> Tested-by: Cornelia Huck <cohuck@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<pmorel@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 <walling@linux.ibm.com>

>>
>>
>>>>  
>>>>> Signed-off-by: David Hildenbrand <david@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

Re: [Qemu-devel] [qemu-s390x] [PATCH v1] s390x/pci: Send correct event on hotplug.

[David Hildenbrand]

On 14.01.19 21:00, Collin Walling wrote:
> On 1/14/19 12:44 PM, Cornelia Huck wrote:
>> [restored cc:s]
>>
>> On Mon, 14 Jan 2019 11:06:19 +0100
>> Pierre Morel <pmorel@linux.ibm.com> wrote:
>>
>>> On 11/01/2019 10:38, Cornelia Huck wrote:
>>>> On Fri, 11 Jan 2019 08:16:41 +0100
>>>> David Hildenbrand <david@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 <cohuck@redhat.com>  
>>>>
>>>> Cool, works for me as well.
>>>>
>>>> Tested-by: Cornelia Huck <cohuck@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<pmorel@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)

This patch relies on Christians patch, where the general concept was
explained. As we changed the initial state, we have to send a
corresponding hotplug event. But still we can add a comment to shine
some light on the general concept.

@Conny, can you add after the first paragraph: (or let me know if you
want a respin)

"On real HW, a PCI device always pops up in the STANDBY state. In QEMU,
we decided to let it show up directly in the configured state (as
configuring it is otherwise just an extra burden for the admin). We can
safely bypass the STANDBY state when hotplugging PCI devices to a guest."

> 
> Otherwise,
> 
> Reviewed-by: Collin Walling <walling@linux.ibm.com>

Thanks!

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [qemu-s390x] [PATCH v1] s390x/pci: Send correct event on hotplug.

[Cornelia Huck]

On Mon, 14 Jan 2019 21:59:58 +0100
David Hildenbrand <david@redhat.com> wrote:

> On 14.01.19 21:00, Collin Walling wrote:
> > On 1/14/19 12:44 PM, Cornelia Huck wrote:  
> >> [restored cc:s]
> >>
> >> On Mon, 14 Jan 2019 11:06:19 +0100
> >> Pierre Morel <pmorel@linux.ibm.com> wrote:
> >>  
> >>> On 11/01/2019 10:38, Cornelia Huck wrote:  
> >>>> On Fri, 11 Jan 2019 08:16:41 +0100
> >>>> David Hildenbrand <david@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 <cohuck@redhat.com>    
> >>>>
> >>>> Cool, works for me as well.
> >>>>
> >>>> Tested-by: Cornelia Huck <cohuck@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<pmorel@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)  
> 
> This patch relies on Christians patch, where the general concept was
> explained. As we changed the initial state, we have to send a
> corresponding hotplug event. But still we can add a comment to shine
> some light on the general concept.
> 
> @Conny, can you add after the first paragraph: (or let me know if you
> want a respin)
> 
> "On real HW, a PCI device always pops up in the STANDBY state. In QEMU,
> we decided to let it show up directly in the configured state (as
> configuring it is otherwise just an extra burden for the admin). We can
> safely bypass the STANDBY state when hotplugging PCI devices to a guest."

I'll just add that text.

> 
> > 
> > Otherwise,
> > 
> > Reviewed-by: Collin Walling <walling@linux.ibm.com>  
> 
> Thanks!
> 

Thanks!

Re: [Qemu-devel] [PATCH v1] s390x/pci: Send correct event on hotplug.

[Cornelia Huck]

On Thu, 10 Jan 2019 22:03:58 +0100
David Hildenbrand <david@redhat.com> 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 <cohuck@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/s390-pci-bus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, applied.