[PATCH v2] acpi: pcihp: make pending delete blocking action expire

[PATCH v2] acpi: pcihp: make pending delete blocking action expire

[Igor Mammedov]

with Q35 using ACPI PCI hotplug by default, user's request to unplug
device is ignored when it's issued before guest OS has been booted.
And any additional attempt to request device hot-unplug afterwards
results in following error:

  "Device XYZ is already in the process of unplug"

arguably it can be considered as a regression introduced by [2],
before which it was possible to issue unplug request multiple
times.

Allowing pending delete blocking expire brings ACPI PCI hotplug
on par with native PCIe unplug behavior [1] and allows user
to repeat unplug requests at propper times.
Set expire timeout to arbitrary 1msec so user won't be able to
flood guest with SCI interrupts by calling device_del in tight loop.

PS:
ACPI spec doesn't mandate what OSPM can do with GPEx.status
bits set before it's booted => it's impl. depended.
Status bits may be retained (I tested with one Windows version)
or cleared (Linux since 2.6 kernel times) during guest's ACPI
subsystem initialization.
Clearing status bits (though not wrong per se) hides the unplug
event from guest, and it's upto user to repeat device_del later
when guest is able to handle unplug requests.

1) 18416c62e3 ("pcie: expire pending delete")
2)
Fixes: cce8944cc9ef ("qdev-monitor: Forbid repeated device_del")
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
   * change timeout to 1ms
   * add comment to expire usage
   * massage commit message to be a bit more clear

CC: mst@redhat.com
CC: anisinha@redhat.com
CC: jusual@redhat.com
CC: kraxel@redhat.com
---
 hw/acpi/pcihp.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index dcfb779a7a..5daa732a33 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -357,6 +357,16 @@ void acpi_pcihp_device_unplug_request_cb(HotplugHandler *hotplug_dev,
      * acpi_pcihp_eject_slot() when the operation is completed.
      */
     pdev->qdev.pending_deleted_event = true;
+    /* if unplug was requested before OSPM is initialized,
+     * linux kernel will clear GPE0.sts[] bits during boot, which effectively
+     * hides unplug event. BAnd than followup qmp_device_del() calls remain
+     * blocked by above flag permanently.
+     * Unblock qmp_device_del() by setting expire limit, so user can
+     * repeat unplug request later when OSPM has been booted.
+     */
+    pdev->qdev.pending_deleted_expires_ms =
+        qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL); /* 1 msec */
+
     s->acpi_pcihp_pci_status[bsel].down |= (1U << slot);
     acpi_send_event(DEVICE(hotplug_dev), ACPI_PCI_HOTPLUG_STATUS);
 }
-- 
2.39.1

Re: [PATCH v2] acpi: pcihp: make pending delete blocking action expire

[Michael S. Tsirkin]

On Wed, Apr 05, 2023 at 10:34:44AM +0200, Igor Mammedov wrote:
> with Q35 using ACPI PCI hotplug by default, user's request to unplug
> device is ignored when it's issued before guest OS has been booted.
> And any additional attempt to request device hot-unplug afterwards
> results in following error:
> 
>   "Device XYZ is already in the process of unplug"
> 
> arguably it can be considered as a regression introduced by [2],
> before which it was possible to issue unplug request multiple
> times.
> 
> Allowing pending delete blocking expire brings ACPI PCI hotplug
> on par with native PCIe unplug behavior [1] and allows user
> to repeat unplug requests at propper times.
> Set expire timeout to arbitrary 1msec so user won't be able to
> flood guest with SCI interrupts by calling device_del in tight loop.
> 
> PS:
> ACPI spec doesn't mandate what OSPM can do with GPEx.status
> bits set before it's booted => it's impl. depended.
> Status bits may be retained (I tested with one Windows version)
> or cleared (Linux since 2.6 kernel times) during guest's ACPI
> subsystem initialization.
> Clearing status bits (though not wrong per se) hides the unplug
> event from guest, and it's upto user to repeat device_del later
> when guest is able to handle unplug requests.
> 
> 1) 18416c62e3 ("pcie: expire pending delete")
> 2)
> Fixes: cce8944cc9ef ("qdev-monitor: Forbid repeated device_del")
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

I feel a real solution is to detect guest handling the
event such as clearing GPE and allowing resending
the interrupt then.
A similar strategy should be possible with the attention
button.

This patch is more of a band-aid - it is possible that guest
rebooted and so user knows a new device_del is required,
and we arbitrarily reject that. Right?

This is arguably a regression but not in this release yes?
So I don't think it needs to block qemu release.


> ---
> v2:
>    * change timeout to 1ms
>    * add comment to expire usage
>    * massage commit message to be a bit more clear
> 
> CC: mst@redhat.com
> CC: anisinha@redhat.com
> CC: jusual@redhat.com
> CC: kraxel@redhat.com

It's helpful to have CC before --- so backporters know whom to CC, too.

> ---
>  hw/acpi/pcihp.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index dcfb779a7a..5daa732a33 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -357,6 +357,16 @@ void acpi_pcihp_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>       * acpi_pcihp_eject_slot() when the operation is completed.
>       */
>      pdev->qdev.pending_deleted_event = true;
> +    /* if unplug was requested before OSPM is initialized,
> +     * linux kernel will clear GPE0.sts[] bits during boot, which effectively
> +     * hides unplug event. BAnd than followup qmp_device_del() calls remain

BAnd?

> +     * blocked by above flag permanently.
> +     * Unblock qmp_device_del() by setting expire limit, so user can
> +     * repeat unplug request later when OSPM has been booted.
> +     */
> +    pdev->qdev.pending_deleted_expires_ms =
> +        qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL); /* 1 msec */
> +
>      s->acpi_pcihp_pci_status[bsel].down |= (1U << slot);
>      acpi_send_event(DEVICE(hotplug_dev), ACPI_PCI_HOTPLUG_STATUS);
>  }
> -- 
> 2.39.1

Re: [PATCH v2] acpi: pcihp: make pending delete blocking action expire

[Igor Mammedov]

On Wed, 5 Apr 2023 04:47:48 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Apr 05, 2023 at 10:34:44AM +0200, Igor Mammedov wrote:
> > with Q35 using ACPI PCI hotplug by default, user's request to unplug
> > device is ignored when it's issued before guest OS has been booted.
> > And any additional attempt to request device hot-unplug afterwards
> > results in following error:
> > 
> >   "Device XYZ is already in the process of unplug"
> > 
> > arguably it can be considered as a regression introduced by [2],
> > before which it was possible to issue unplug request multiple
> > times.
> > 
> > Allowing pending delete blocking expire brings ACPI PCI hotplug
> > on par with native PCIe unplug behavior [1] and allows user
> > to repeat unplug requests at propper times.
> > Set expire timeout to arbitrary 1msec so user won't be able to
> > flood guest with SCI interrupts by calling device_del in tight loop.
> > 
> > PS:
> > ACPI spec doesn't mandate what OSPM can do with GPEx.status
> > bits set before it's booted => it's impl. depended.
> > Status bits may be retained (I tested with one Windows version)
> > or cleared (Linux since 2.6 kernel times) during guest's ACPI
> > subsystem initialization.
> > Clearing status bits (though not wrong per se) hides the unplug
> > event from guest, and it's upto user to repeat device_del later
> > when guest is able to handle unplug requests.
> > 
> > 1) 18416c62e3 ("pcie: expire pending delete")
> > 2)
> > Fixes: cce8944cc9ef ("qdev-monitor: Forbid repeated device_del")
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>  
> 
> I feel a real solution is to detect guest handling the
> event such as clearing GPE and allowing resending
> the interrupt then.

I did consider preserving(masking clearing attempt) of GPE0.sts[]
IF en[] hasn't been enabled ever (i.e. should help with unplug at
the 1st boot). But that won't work across reboots (depends on
reboot kind) and it's twisting rules wrt spec (platform(QEMU/fw)
may set status bits, but it's upto OSPM to decide what to do with
them (when and how clear or ignore them). 

> A similar strategy should be possible with the attention
> button.
> 

> This patch is more of a band-aid - it is possible that guest
> rebooted and so user knows a new device_del is required,
> and we arbitrarily reject that. Right?
You lost me here. Can you elaborate?

> 
> This is arguably a regression but not in this release yes?
> So I don't think it needs to block qemu release.
> 
> 
> > ---
> > v2:
> >    * change timeout to 1ms
> >    * add comment to expire usage
> >    * massage commit message to be a bit more clear
> > 
> > CC: mst@redhat.com
> > CC: anisinha@redhat.com
> > CC: jusual@redhat.com
> > CC: kraxel@redhat.com  
> 
> It's helpful to have CC before --- so backporters know whom to CC, too.

ok, I'll fix it up and respin

> 
> > ---
> >  hw/acpi/pcihp.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> > index dcfb779a7a..5daa732a33 100644
> > --- a/hw/acpi/pcihp.c
> > +++ b/hw/acpi/pcihp.c
> > @@ -357,6 +357,16 @@ void acpi_pcihp_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> >       * acpi_pcihp_eject_slot() when the operation is completed.
> >       */
> >      pdev->qdev.pending_deleted_event = true;
> > +    /* if unplug was requested before OSPM is initialized,
> > +     * linux kernel will clear GPE0.sts[] bits during boot, which effectively
> > +     * hides unplug event. BAnd than followup qmp_device_del() calls remain  
> 
> BAnd?
> 
> > +     * blocked by above flag permanently.
> > +     * Unblock qmp_device_del() by setting expire limit, so user can
> > +     * repeat unplug request later when OSPM has been booted.
> > +     */
> > +    pdev->qdev.pending_deleted_expires_ms =
> > +        qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL); /* 1 msec */
> > +
> >      s->acpi_pcihp_pci_status[bsel].down |= (1U << slot);
> >      acpi_send_event(DEVICE(hotplug_dev), ACPI_PCI_HOTPLUG_STATUS);
> >  }
> > -- 
> > 2.39.1  
> 

Re: [PATCH v2] acpi: pcihp: make pending delete blocking action expire

[Michael S. Tsirkin]

On Wed, Apr 05, 2023 at 11:38:56AM +0200, Igor Mammedov wrote:
> On Wed, 5 Apr 2023 04:47:48 -0400
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Apr 05, 2023 at 10:34:44AM +0200, Igor Mammedov wrote:
> > > with Q35 using ACPI PCI hotplug by default, user's request to unplug
> > > device is ignored when it's issued before guest OS has been booted.
> > > And any additional attempt to request device hot-unplug afterwards
> > > results in following error:
> > > 
> > >   "Device XYZ is already in the process of unplug"
> > > 
> > > arguably it can be considered as a regression introduced by [2],
> > > before which it was possible to issue unplug request multiple
> > > times.
> > > 
> > > Allowing pending delete blocking expire brings ACPI PCI hotplug
> > > on par with native PCIe unplug behavior [1] and allows user
> > > to repeat unplug requests at propper times.
> > > Set expire timeout to arbitrary 1msec so user won't be able to
> > > flood guest with SCI interrupts by calling device_del in tight loop.
> > > 
> > > PS:
> > > ACPI spec doesn't mandate what OSPM can do with GPEx.status
> > > bits set before it's booted => it's impl. depended.
> > > Status bits may be retained (I tested with one Windows version)
> > > or cleared (Linux since 2.6 kernel times) during guest's ACPI
> > > subsystem initialization.
> > > Clearing status bits (though not wrong per se) hides the unplug
> > > event from guest, and it's upto user to repeat device_del later
> > > when guest is able to handle unplug requests.
> > > 
> > > 1) 18416c62e3 ("pcie: expire pending delete")
> > > 2)
> > > Fixes: cce8944cc9ef ("qdev-monitor: Forbid repeated device_del")
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>  
> > 
> > I feel a real solution is to detect guest handling the
> > event such as clearing GPE and allowing resending
> > the interrupt then.
> 
> I did consider preserving(masking clearing attempt) of GPE0.sts[]
> IF en[] hasn't been enabled ever (i.e. should help with unplug at
> the 1st boot). But that won't work across reboots (depends on
> reboot kind) and it's twisting rules wrt spec (platform(QEMU/fw)
> may set status bits, but it's upto OSPM to decide what to do with
> them (when and how clear or ignore them). 
> 
> > A similar strategy should be possible with the attention
> > button.
> > 
> 
> > This patch is more of a band-aid - it is possible that guest
> > rebooted and so user knows a new device_del is required,
> > and we arbitrarily reject that. Right?
> You lost me here. Can you elaborate?

I request device deletion but guest was rebooting losing state.
I want to request deletion again but the request will fail
until 1ms passes. 1ms is not a lot but I really don't want
management to learn weird tricks like waiting for 1ms.



> > 
> > This is arguably a regression but not in this release yes?
> > So I don't think it needs to block qemu release.
> > 
> > 
> > > ---
> > > v2:
> > >    * change timeout to 1ms
> > >    * add comment to expire usage
> > >    * massage commit message to be a bit more clear
> > > 
> > > CC: mst@redhat.com
> > > CC: anisinha@redhat.com
> > > CC: jusual@redhat.com
> > > CC: kraxel@redhat.com  
> > 
> > It's helpful to have CC before --- so backporters know whom to CC, too.
> 
> ok, I'll fix it up and respin
> 
> > 
> > > ---
> > >  hw/acpi/pcihp.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > > 
> > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> > > index dcfb779a7a..5daa732a33 100644
> > > --- a/hw/acpi/pcihp.c
> > > +++ b/hw/acpi/pcihp.c
> > > @@ -357,6 +357,16 @@ void acpi_pcihp_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> > >       * acpi_pcihp_eject_slot() when the operation is completed.
> > >       */
> > >      pdev->qdev.pending_deleted_event = true;
> > > +    /* if unplug was requested before OSPM is initialized,
> > > +     * linux kernel will clear GPE0.sts[] bits during boot, which effectively
> > > +     * hides unplug event. BAnd than followup qmp_device_del() calls remain  
> > 
> > BAnd?
> > 
> > > +     * blocked by above flag permanently.
> > > +     * Unblock qmp_device_del() by setting expire limit, so user can
> > > +     * repeat unplug request later when OSPM has been booted.
> > > +     */
> > > +    pdev->qdev.pending_deleted_expires_ms =
> > > +        qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL); /* 1 msec */
> > > +
> > >      s->acpi_pcihp_pci_status[bsel].down |= (1U << slot);
> > >      acpi_send_event(DEVICE(hotplug_dev), ACPI_PCI_HOTPLUG_STATUS);
> > >  }
> > > -- 
> > > 2.39.1  
> > 

Re: [PATCH v2] acpi: pcihp: make pending delete blocking action expire

[Igor Mammedov]

On Wed, 5 Apr 2023 06:02:29 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Apr 05, 2023 at 11:38:56AM +0200, Igor Mammedov wrote:
> > On Wed, 5 Apr 2023 04:47:48 -0400
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >   
> > > On Wed, Apr 05, 2023 at 10:34:44AM +0200, Igor Mammedov wrote:  
> > > > with Q35 using ACPI PCI hotplug by default, user's request to unplug
> > > > device is ignored when it's issued before guest OS has been booted.
> > > > And any additional attempt to request device hot-unplug afterwards
> > > > results in following error:
> > > > 
> > > >   "Device XYZ is already in the process of unplug"
> > > > 
> > > > arguably it can be considered as a regression introduced by [2],
> > > > before which it was possible to issue unplug request multiple
> > > > times.
> > > > 
> > > > Allowing pending delete blocking expire brings ACPI PCI hotplug
> > > > on par with native PCIe unplug behavior [1] and allows user
> > > > to repeat unplug requests at propper times.
> > > > Set expire timeout to arbitrary 1msec so user won't be able to
> > > > flood guest with SCI interrupts by calling device_del in tight loop.
> > > > 
> > > > PS:
> > > > ACPI spec doesn't mandate what OSPM can do with GPEx.status
> > > > bits set before it's booted => it's impl. depended.
> > > > Status bits may be retained (I tested with one Windows version)
> > > > or cleared (Linux since 2.6 kernel times) during guest's ACPI
> > > > subsystem initialization.
> > > > Clearing status bits (though not wrong per se) hides the unplug
> > > > event from guest, and it's upto user to repeat device_del later
> > > > when guest is able to handle unplug requests.
> > > > 
> > > > 1) 18416c62e3 ("pcie: expire pending delete")
> > > > 2)
> > > > Fixes: cce8944cc9ef ("qdev-monitor: Forbid repeated device_del")
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>    
> > > 
> > > I feel a real solution is to detect guest handling the
> > > event such as clearing GPE and allowing resending
> > > the interrupt then.  
> > 
> > I did consider preserving(masking clearing attempt) of GPE0.sts[]
> > IF en[] hasn't been enabled ever (i.e. should help with unplug at
> > the 1st boot). But that won't work across reboots (depends on
> > reboot kind) and it's twisting rules wrt spec (platform(QEMU/fw)
> > may set status bits, but it's upto OSPM to decide what to do with
> > them (when and how clear or ignore them). 
> >   
> > > A similar strategy should be possible with the attention
> > > button.
> > >   
> >   
> > > This patch is more of a band-aid - it is possible that guest
> > > rebooted and so user knows a new device_del is required,
> > > and we arbitrarily reject that. Right?  
> > You lost me here. Can you elaborate?  
> 
> I request device deletion but guest was rebooting losing state.
> I want to request deletion again but the request will fail
> until 1ms passes. 1ms is not a lot but I really don't want
> management to learn weird tricks like waiting for 1ms.
There is no actual need to wait 1ms or 5sec if management doesn't
want to, it can bombard QEMU with with requests however it likes
if it can't do something smarter. But it won't affect guest and
management will be getting pending error until that time expires.

The thing that mgmt shall account for is that unplug request is
just that and might not work for various reasons. And mgmt has
to deal with it (i.e. repeat requests for some time then report
transient failure up stack) (any timeouts on QEMU behalf here are
largely irrelevant).

> > > 
> > > This is arguably a regression but not in this release yes?
> > > So I don't think it needs to block qemu release.
> > > 
> > >   
> > > > ---
> > > > v2:
> > > >    * change timeout to 1ms
> > > >    * add comment to expire usage
> > > >    * massage commit message to be a bit more clear
> > > > 
> > > > CC: mst@redhat.com
> > > > CC: anisinha@redhat.com
> > > > CC: jusual@redhat.com
> > > > CC: kraxel@redhat.com    
> > > 
> > > It's helpful to have CC before --- so backporters know whom to CC, too.  
> > 
> > ok, I'll fix it up and respin
> >   
> > >   
> > > > ---
> > > >  hw/acpi/pcihp.c | 10 ++++++++++
> > > >  1 file changed, 10 insertions(+)
> > > > 
> > > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> > > > index dcfb779a7a..5daa732a33 100644
> > > > --- a/hw/acpi/pcihp.c
> > > > +++ b/hw/acpi/pcihp.c
> > > > @@ -357,6 +357,16 @@ void acpi_pcihp_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> > > >       * acpi_pcihp_eject_slot() when the operation is completed.
> > > >       */
> > > >      pdev->qdev.pending_deleted_event = true;
> > > > +    /* if unplug was requested before OSPM is initialized,
> > > > +     * linux kernel will clear GPE0.sts[] bits during boot, which effectively
> > > > +     * hides unplug event. BAnd than followup qmp_device_del() calls remain    
> > > 
> > > BAnd?
> > >   
> > > > +     * blocked by above flag permanently.
> > > > +     * Unblock qmp_device_del() by setting expire limit, so user can
> > > > +     * repeat unplug request later when OSPM has been booted.
> > > > +     */
> > > > +    pdev->qdev.pending_deleted_expires_ms =
> > > > +        qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL); /* 1 msec */
> > > > +
> > > >      s->acpi_pcihp_pci_status[bsel].down |= (1U << slot);
> > > >      acpi_send_event(DEVICE(hotplug_dev), ACPI_PCI_HOTPLUG_STATUS);
> > > >  }
> > > > -- 
> > > > 2.39.1    
> > >   
> 

Re: [PATCH v2] acpi: pcihp: make pending delete blocking action expire

[Igor Mammedov]

On Wed, 5 Apr 2023 04:47:48 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

[...]
> This is arguably a regression but not in this release yes?
> So I don't think it needs to block qemu release.

yep, it's 'old' regression introduced in earlier releases
> Fixes: cce8944cc9ef ("qdev-monitor: Forbid repeated device_del")
 

> 
> 
> > ---
> > v2:
> >    * change timeout to 1ms
> >    * add comment to expire usage
> >    * massage commit message to be a bit more clear
> > 
> > CC: mst@redhat.com
> > CC: anisinha@redhat.com
> > CC: jusual@redhat.com
> > CC: kraxel@redhat.com  
> 
> It's helpful to have CC before --- so backporters know whom to CC, too.
> 
> > ---
> >  hw/acpi/pcihp.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> > index dcfb779a7a..5daa732a33 100644
> > --- a/hw/acpi/pcihp.c
> > +++ b/hw/acpi/pcihp.c
> > @@ -357,6 +357,16 @@ void acpi_pcihp_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> >       * acpi_pcihp_eject_slot() when the operation is completed.
> >       */
> >      pdev->qdev.pending_deleted_event = true;
> > +    /* if unplug was requested before OSPM is initialized,
> > +     * linux kernel will clear GPE0.sts[] bits during boot, which effectively
> > +     * hides unplug event. BAnd than followup qmp_device_del() calls remain  
> 
> BAnd?
> 
> > +     * blocked by above flag permanently.
> > +     * Unblock qmp_device_del() by setting expire limit, so user can
> > +     * repeat unplug request later when OSPM has been booted.
> > +     */
> > +    pdev->qdev.pending_deleted_expires_ms =
> > +        qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL); /* 1 msec */
> > +
> >      s->acpi_pcihp_pci_status[bsel].down |= (1U << slot);
> >      acpi_send_event(DEVICE(hotplug_dev), ACPI_PCI_HOTPLUG_STATUS);
> >  }
> > -- 
> > 2.39.1  
> 

Re: [PATCH v2] acpi: pcihp: make pending delete blocking action expire

[Ani Sinha]

On Wed, 5 Apr 2023, Igor Mammedov wrote:

> with Q35 using ACPI PCI hotplug by default, user's request to unplug
> device is ignored when it's issued before guest OS has been booted.
> And any additional attempt to request device hot-unplug afterwards
> results in following error:
>
>   "Device XYZ is already in the process of unplug"
>
> arguably it can be considered as a regression introduced by [2],
> before which it was possible to issue unplug request multiple
> times.
>
> Allowing pending delete blocking expire brings ACPI PCI hotplug
> on par with native PCIe unplug behavior [1] and allows user
> to repeat unplug requests at propper times.
> Set expire timeout to arbitrary 1msec so user won't be able to
> flood guest with SCI interrupts by calling device_del in tight loop.
>
> PS:
> ACPI spec doesn't mandate what OSPM can do with GPEx.status
> bits set before it's booted => it's impl. depended.
> Status bits may be retained (I tested with one Windows version)
> or cleared (Linux since 2.6 kernel times) during guest's ACPI
> subsystem initialization.
> Clearing status bits (though not wrong per se) hides the unplug
> event from guest, and it's upto user to repeat device_del later
> when guest is able to handle unplug requests.
>
> 1) 18416c62e3 ("pcie: expire pending delete")
> 2)
> Fixes: cce8944cc9ef ("qdev-monitor: Forbid repeated device_del")
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Ani Sinha <anisinha@redhat.com>

> ---
> v2:
>    * change timeout to 1ms
>    * add comment to expire usage
>    * massage commit message to be a bit more clear
>
> CC: mst@redhat.com
> CC: anisinha@redhat.com
> CC: jusual@redhat.com
> CC: kraxel@redhat.com
> ---
>  hw/acpi/pcihp.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index dcfb779a7a..5daa732a33 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -357,6 +357,16 @@ void acpi_pcihp_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>       * acpi_pcihp_eject_slot() when the operation is completed.
>       */
>      pdev->qdev.pending_deleted_event = true;
> +    /* if unplug was requested before OSPM is initialized,
> +     * linux kernel will clear GPE0.sts[] bits during boot, which effectively
> +     * hides unplug event. BAnd than followup qmp_device_del() calls remain
> +     * blocked by above flag permanently.
> +     * Unblock qmp_device_del() by setting expire limit, so user can
> +     * repeat unplug request later when OSPM has been booted.
> +     */
> +    pdev->qdev.pending_deleted_expires_ms =
> +        qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL); /* 1 msec */
> +
>      s->acpi_pcihp_pci_status[bsel].down |= (1U << slot);
>      acpi_send_event(DEVICE(hotplug_dev), ACPI_PCI_HOTPLUG_STATUS);
>  }
> --
> 2.39.1
>
>