| Message ID | 20200722161722.51416-2-mlevitsk@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RFC: pcie: parital fix for missing unplug events | expand |
On Wed, 2020-07-22 at 19:17 +0300, Maxim Levitsky wrote: > Curently it is possible to hotplug a device and then immediatly > hotunplug it before the OS notices, and that will result > in missed unplug event since we can only send one attention button event. > > Moreover the device will stuck in unplugging state forever. > > Error out in such cases and rely on the caller (e.g libvirt) to retry > the unplug a bit later > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > hw/pci/pcie.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index 5b48bae0f6..9e836cf2f4 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -402,6 +402,17 @@ static void pcie_cap_slot_plug_common(PCIDevice *hotplug_dev, DeviceState *dev, > */ > error_setg_errno(errp, EBUSY, "slot is electromechanically locked"); > } > + > + if (sltsta & PCI_EXP_SLTSTA_ABP) { > + /* > + * Attention button is pressed, thus we can't send another > + * hotpplug event Typo here, forgot to refresh the commit. > + */ > + error_setg_errno(errp, EBUSY, > + "attention button is already pressed, can't " > + "send another hotplug event"); > + } > + > } > > void pcie_cap_slot_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
On Wed, 2020-07-22 at 19:19 +0300, Maxim Levitsky wrote: > On Wed, 2020-07-22 at 19:17 +0300, Maxim Levitsky wrote: > > Curently it is possible to hotplug a device and then immediatly > > hotunplug it before the OS notices, and that will result > > in missed unplug event since we can only send one attention button event. > > > > Moreover the device will stuck in unplugging state forever. > > > > Error out in such cases and rely on the caller (e.g libvirt) to retry > > the unplug a bit later > > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > --- > > hw/pci/pcie.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > > index 5b48bae0f6..9e836cf2f4 100644 > > --- a/hw/pci/pcie.c > > +++ b/hw/pci/pcie.c > > @@ -402,6 +402,17 @@ static void pcie_cap_slot_plug_common(PCIDevice *hotplug_dev, DeviceState *dev, > > */ > > error_setg_errno(errp, EBUSY, "slot is electromechanically locked"); > > } > > + > > + if (sltsta & PCI_EXP_SLTSTA_ABP) { > > + /* > > + * Attention button is pressed, thus we can't send another > > + * hotpplug event > Typo here, forgot to refresh the commit. > > + */ > > + error_setg_errno(errp, EBUSY, > > + "attention button is already pressed, can't " > > + "send another hotplug event"); > > + } > > + > > } > > > > void pcie_cap_slot_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, ping. Best regards, Maxim Levitsky
On Wed, 29 Jul 2020 08:09:37 +0300 Maxim Levitsky <mlevitsk@redhat.com> wrote: > On Wed, 2020-07-22 at 19:19 +0300, Maxim Levitsky wrote: > > On Wed, 2020-07-22 at 19:17 +0300, Maxim Levitsky wrote: > > > Curently it is possible to hotplug a device and then immediatly > > > hotunplug it before the OS notices, and that will result > > > in missed unplug event since we can only send one attention button event. > > > > > > Moreover the device will stuck in unplugging state forever. > > > > > > Error out in such cases and rely on the caller (e.g libvirt) to retry > > > the unplug a bit later > > > > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > > --- > > > hw/pci/pcie.c | 11 +++++++++++ > > > 1 file changed, 11 insertions(+) > > > > > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > > > index 5b48bae0f6..9e836cf2f4 100644 > > > --- a/hw/pci/pcie.c > > > +++ b/hw/pci/pcie.c > > > @@ -402,6 +402,17 @@ static void pcie_cap_slot_plug_common(PCIDevice *hotplug_dev, DeviceState *dev, > > > */ > > > error_setg_errno(errp, EBUSY, "slot is electromechanically locked"); > > > } > > > + > > > + if (sltsta & PCI_EXP_SLTSTA_ABP) { > > > + /* > > > + * Attention button is pressed, thus we can't send another > > > + * hotpplug event > > Typo here, forgot to refresh the commit. > > > + */ > > > + error_setg_errno(errp, EBUSY, > > > + "attention button is already pressed, can't " > > > + "send another hotplug event"); > > > + } > > > + > > > } > > > > > > void pcie_cap_slot_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > ping. CCing Julia since she was looking into PCI hotplug/unplug code recently. > > Best regards, > Maxim Levitsky > >
On Wed, Jul 22, 2020 at 07:17:22PM +0300, Maxim Levitsky wrote: > Curently it is possible to hotplug a device and then immediatly > hotunplug it before the OS notices, and that will result > in missed unplug event since we can only send one attention button event. > > Moreover the device will stuck in unplugging state forever. > > Error out in such cases and rely on the caller (e.g libvirt) to retry > the unplug a bit later > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > hw/pci/pcie.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index 5b48bae0f6..9e836cf2f4 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -402,6 +402,17 @@ static void pcie_cap_slot_plug_common(PCIDevice *hotplug_dev, DeviceState *dev, > */ > error_setg_errno(errp, EBUSY, "slot is electromechanically locked"); > } > + > + if (sltsta & PCI_EXP_SLTSTA_ABP) { > + /* > + * Attention button is pressed, thus we can't send another > + * hotpplug event typo > + */ > + error_setg_errno(errp, EBUSY, > + "attention button is already pressed, can't " > + "send another hotplug event"); > + } > + > } It would be neater if we could queue the event up in qemu. Alternatively - can we clean up the unhandled event so guest does not even notice the device briefly appeared? > > void pcie_cap_slot_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > -- > 2.26.2
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index 5b48bae0f6..9e836cf2f4 100644 --- a/hw/pci/pcie.c +++ b/hw/pci/pcie.c @@ -402,6 +402,17 @@ static void pcie_cap_slot_plug_common(PCIDevice *hotplug_dev, DeviceState *dev, */ error_setg_errno(errp, EBUSY, "slot is electromechanically locked"); } + + if (sltsta & PCI_EXP_SLTSTA_ABP) { + /* + * Attention button is pressed, thus we can't send another + * hotpplug event + */ + error_setg_errno(errp, EBUSY, + "attention button is already pressed, can't " + "send another hotplug event"); + } + } void pcie_cap_slot_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
Curently it is possible to hotplug a device and then immediatly hotunplug it before the OS notices, and that will result in missed unplug event since we can only send one attention button event. Moreover the device will stuck in unplugging state forever. Error out in such cases and rely on the caller (e.g libvirt) to retry the unplug a bit later Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- hw/pci/pcie.c | 11 +++++++++++ 1 file changed, 11 insertions(+)