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


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


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