Re: [qemu-s390x] [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 <address@hidden> wrote:

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

Thanks for checking.

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

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