Re: [Qemu-devel] [PATCH 0/6] PCI hotplug improvements

[Gleb Natapov]

On Wed, Mar 07, 2012 at 10:20:49AM -0700, Alex Williamson wrote:
> On Wed, 2012-03-07 at 14:43 +0200, Gleb Natapov wrote:
> > On Tue, Mar 06, 2012 at 05:13:36PM -0700, Alex Williamson wrote:
> > > Here's a re-work of the patch that added _STA for the purpose of
> > > using it as an ack from the guest.  Instead of that, add a notifier
> > > for device access.  Once the guest reads from device config space,
> > > it owns it.  Until that point, we can remove it directly.  As pointed
> > > out by MST, this passes test b) below, which the _STA method would not.
> > > As a bonus, no bios change is required for this.  Patches 5 & 6 are
> > > just cleanups that can be applied independently.  Thanks,
> > > 
> > While I agree with Michael that using _STA as ack is a hack I think
> > this approach is not less of a hack. It is unlikely that this is how it
> > work on bare metal and we should follow real HW if possible.
> 
> The test below is the only thing that proved to me it was less of a
> hack.  Introducing a _LCK method for a slot may be another way to do
> this.  Unfortunately it's not required that the OSPM call _LCK and it's
> not mentioned in the msft document referenced previously.
>  
I do not understand where this requirement, that device_del should work
if non-acpi guest is running, is coming from? Because if there is no
such requirement then the hack is not needed.

> > > Tested using Linux guest:
> > > a) without acpiphp loaded:
> > >     - device_add (nothing happens)
> > >     - device_del (device removed directly)
> > How it works on real HW? On non ACPI compliant guest hot plug unplug is
> > not suppose to work.
> 
> We're dealing with ACPI hotplug, so it's not as completely defined as
> something like SHPC.  My understanding is that add and remove can be
> initiated by either an OS defined software method or via a physical
> button on the slot.  What we do in qemu is more akin to the physical
> button.  The user places a card in a slot, presses the button, which
> will signal the OS to power up the slot, discover the device, and start
> making use of it.  An indicator LED is optional in the PCI hotplug spec,
> so the user is left to check the OS or look for device activity to
> determine if the card was inserted.  If nothing happens, I suspect the
> user would likely pull the card back out, which is effectively what
> we're allowing here.
> 
> > > b) without acpiphp loaded:
> > >     - device_add (nothing happens)
> > >     - echo 1 > /sys/bus/pci/rescan (device discovered)
> > >     - device_del (nothing happens, guest owns device)
> > So guest can block a device from being ever removed?
> 
> Yes, surprise removal is beyond the scope of this series.  We can always
> shutdown a guest to remove the device, but surprise removal risks the
> integrity of the guest.  Thanks,
Surprise removal is a guest driver issue, not ACPI AFAIK. 

> 
> Alex
> 
> > >     - modprobe acpiphp
> > >     - device_del (guest releases device)
> > > c) with acpiphp loaded:
> > >     - device_add/del behave as expected (automatic add + coordinated 
> > > removal)
> > > Tested using WinXP guest:
> > >     - device_add/del behave as expected (automatic add + coordinated 
> > > removal)
> > > 
> > > ---
> > > 
> > > Alex Williamson (6):
> > >       api_piix4: Remove PCI_RMV_BASE write code
> > >       acpi_piix4: Use pci_get/set_byte
> > >       acpi_piix4: Track PCI hotplug status and allow non-ACPI remove path
> > >       pci: Add notifier for device probing
> > >       acpi_piix4: Only allow writes to PCI hotplug eject register
> > >       acpi_piix4: Disallow write to up/down PCI hotplug registers
> > > 
> > > 
> > >  hw/acpi_piix4.c |  175 
> > > ++++++++++++++++++++++++++++---------------------------
> > >  hw/pci_host.c   |   19 ++++++
> > >  hw/pci_host.h   |    2 +
> > >  3 files changed, 111 insertions(+), 85 deletions(-)
> > 
> > --
> >                     Gleb.
> 
> 

--
                        Gleb.