Re: [PATCH] acpi: pcihp: make pending delete expire in 5sec

[Gerd Hoffmann]

On Thu, Apr 06, 2023 at 01:46:48PM +0200, Igor Mammedov wrote:
> On Tue, 4 Apr 2023 12:46:45 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> > On Tue, Apr 04, 2023 at 10:30:55AM +0200, Igor Mammedov wrote:
> > > On Tue, 4 Apr 2023 09:03:59 +0200
> > > Gerd Hoffmann <kraxel@redhat.com> wrote:
> > >   
> > > >   Hi,
> > > >   
> > > > > > Allowing pending delete expire brings ACPI PCI hotplug on par
> > > > > > with native PCIe unplug behavior [1] which in its turn refers
> > > > > > back to ACPI PCI hotplug ability to repeat unplug requests.    
> > > >   
> > > > > A bit concerned about how this interacts with failover,
> > > > > and 5sec is a lot of time that I hoped we'd avoid with acpi.
> > > > > Any better ideas of catching such misbehaving guests?    
> > > > 
> > > > The 5sec are coming from the pcie spec: The hot-unplug request can be
> > > > canceled within 5 seconds by pressing the button again. The problem here
> > > > is that both hotplug and hot-unplug use the same signaling path, so we
> > > > really have to wait the 5 seconds to avoid the OS mis-interpreting the
> > > > button press as 'cancel' event.  
> > > 
> > > Any pointer to spec?  
> > 
> > pcie base spec, section 6.7.1.5. Attention Button
> > 
> > > Does it apply to SHPC too?  
> > 
> > Yes (section 2.2.5. Attention Button).
> 
> shouldn't we set pending_deleted_expires_ms to 5sec for SHPC
> as we do with PCIe?

I suspect it is not *that* simple.

For pcie there is one more detail.  The code also looks at the
indicator led.  When it is set to 'blink' (used by the guest to signal
hotplug is in progress) the code will block unplug too, i.e. the rules
are (IIRC, didn't check the code):

  (1) if less than 5 secs passed, reject, else ...
  (2) if the indicator blinks, reject, else ...
  (3) allow request (and send virtual attention button press to guest).

SHPC should probably do that too.  Sending a attention button press
while the guest clearly signals a hotplug is in progress has a high
chance to be interpreted as cancel.

take care,
  Gerd