On Tue, 28 Sep 2021 11:47:26 +0100
Daniel P. Berrangé <berra...@redhat.com> wrote:

> On Tue, Sep 28, 2021 at 03:28:04PM +0530, Ani Sinha wrote:
> > 
> > 
> > On Tue, 28 Sep 2021, Daniel P. Berrangé wrote:
> >   
> > > On Tue, Sep 28, 2021 at 02:35:47PM +0530, Ani Sinha wrote:  
> > > >
> > > >
> > > > On Tue, 28 Sep 2021, Daniel P. Berrangé wrote:
> > > >  
> > > > > On Sun, Sep 12, 2021 at 08:56:29AM +0530, Ani Sinha wrote:  
> > > > > > This change introduces libvirt xml support for the following two pm 
> > > > > > options:
[...]
> > > > The <target hotplug="on/off"> switch in libvirt for pcie-root-ports
> > > > currently does not care whether native or acpi hotplug is used. It 
> > > > simply
> > > > turns on the hotplug for that particular port. Whether ACPI or native is
> > > > used is controlled by this global flag that Julia has introduced in 
> > > > 6.1.  


> > > Right so we have
> > >

*1*)
following applies to piix4/q35:
  * ACPI hotplug when enabled, affects _only_ cold-plugged 'bridges'
    since it requires 'slots' being described in DSDT table which
    in current impl. is static table built at reset time.

     (i.e. built-in or 'bridges' specified on command line,
      where 'bridges' could be PCI-PCI or PCIe-PCI or root/downstream-ports')
   for anything else ('bridges' added with device_add) native hotplug
   is in use (whether it's SHPC or PCI-E native).

ACPI hotplug wiring is done by calling qbus_set_hotplug_handler()
 * for root bus piix4_pm_realize()/ich9_pm_init()
 * for anything else acpi_pcihp_device_plug_cb()


> > >   * PIIX4
> > >
> > >       - acpi-root-pci-hotplug=bool
> > >
> > >         Whether hotplug is enabled for the root bridge or not
> > >
> > >           <target hotplug="on/off"> for pci-root controller
> > >
> > >
> > >       - acpi-pci-hotplug-with-bridge-support=bool
> > >
> > >         Toggles support for ACPI based hotplug across all bridges.
> > >   If disabled will there will be no hotplug at all for PIIX4 ?
> > >   Or does 'shpc' come into play in that scenario ?

'SHPC' hotplug kicks in if it's enabled. (defaults to 'on' except 2.9 machine 
type)

on q35/APCI side of things we always advertise -all_ hotplug methods available

build_q35_osc_method()
    /*                                                                          
 
     * Always allow native PME, AER (no dependencies)                           
 
     * Allow SHPC (PCI bridges can have SHPC controller)                        
 
     */                                                                         
 
    aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl));

bits 0, 1 are Native PCI-E hotplug and SHPC respectively 

for PIIX4 we don't have _OSC so it's up to guest OS to make up
supported methods.

In order of preference:
  * Windows supports ACPI hotplug then Native PCI-E (SHPC never worked there)
  * Linux supports ACPI hotplug, SHPC, Native PCI-E
        (SHPC worked poorly due to need to reserve IO for bridges
         io reservation hinting (impl. later by Marcel))

> > >    PIIX combinations
> > >
> > >        (1) acpi-root-pci-hotplug=yes
> > >            acpi-pci-hotplug-with-bridge-support=yes
> > >
> > >          - All bridges have hotplug
> > >
> > >        (2) acpi-root-pci-hotplug=yes
> > >            acpi-pci-hotplug-with-bridge-support=no
> > >
> > >          - No bridges have hotplug
> > >
> > >        (3) acpi-root-pci-hotplug=no
> > >            acpi-pci-hotplug-with-bridge-support=yes
> > >
> > >          - All bridges except root have hotplug

requested by Promox guys, to battle against Windows 'feature' that
lets any user to unplug sole NIC using an icon on taskbar.

(Laine mentioned we have similar per port control for PCI-E
('hotplug' property) that was requested by other users
probably for the same reason)

so acpi-root-pci-hotplug is similar to pcie-root-port.hotplug
with a difference that the former applies to whole root bus
on PIIX4, where the later could be controlled per root port.

> > >        (4) acpi-root-pci-hotplug=no
> > >            acpi-pci-hotplug-with-bridge-support=no
> > >
> > >          - No bridges have hotplug. Essentially identical to (2)  
> > 
> > no (4) is not identical to (2). In (4) no hotplug is enabled. In (2) pci
> > root bus still has hotplug enabled.  
> 
> So you're saying that acpi-root-pci-hotplug=yes overrides the
> global request acpi-pci-hotplug-with-bridge-support=no and
> turns ACPI hotplug back on for the pcie-root

historically ACPI hotplug on root bus was always supported
without any option, i.e. acpi-root-pci-hotplug=yes by default.
acpi-pci-hotplug-with-bridge-support does what its name
claims - i.e. adds hotplug for bridges (at least on PIIX4).

> > >   * Q35

 clarification [*1*] still applies

> > >
> > >
> > >       - acpi-pci-hotplug-with-bridge-support=bool
> > >
> > >         Toggles support for ACPI based hotplug. If disabled native
> > >   PCIe hotplug is activated instead
> > >
> > >
> > >   * pcie-root-port
> > >
> > >       - hotplug=bool
> > >
> > >         Toggles whether hotplug is enabled on the port. Affects
> > >   either native and ACPI based hotplug, depending on what
> > >   acpi-pci-hotplug-with-bridge-support=bool is set to ?
> > >
> > >           <target hotplug="on/off"> for pcie-root-port controller
> > >
> > >       - native_hotplug=bool
> > >
> > >         Can be used to also enable native hotplug, even when ACPI
> > >   based hotplug is globally enabled. IOW only really relevant
> > >   when acpi-pci-hotplug-with-bridge-support=true
> > >
> > >
> > >    Q35 combinations
> > >
> > >        (1) acpi-pci-hotplug-with-bridge-support=yes
name is a bit confusing depending on the point of view,
but if one thinks about root/downstream-ports as bridges
it fits just fine.

> > >            pcie-root-port.hotplug=yes
> > >            pcie-root-port.native_hotplug=yes

it probably needs to say here what defaults are in place currently.
maybe say it once above (1) for all Q35 section


> > >            ports have both ACPI + native hotplug, guest prefers native

are you sure about 'prefers'?

> > >
> > >
> > >        (2) acpi-pci-hotplug-with-bridge-support=yes

> > >            pcie-root-port.hotplug=no
> > >            pcie-root-port.native_hotplug=yes
seems as invalid config, we should error out

> > >            ports have no hotplug, despite native_hotplug=yes IIUC
> > >
> > >
> > >        (3) acpi-pci-hotplug-with-bridge-support=yes
> > >            pcie-root-port.hotplug=yes
> > >            pcie-root-port.native_hotplug=no
> > >
> > >            ports have ACPI hotplug only
> > >
> > >
> > >        (4) acpi-pci-hotplug-with-bridge-support=yes
> > >            pcie-root-port.hotplug=no
> > >            pcie-root-port.native_hotplug=no
> > >
> > >            ports have no hotplug
> > >
> > >
> > >        (5) acpi-pci-hotplug-with-bridge-support=no
> > >            pcie-root-port.hotplug=yes
> > >            pcie-root-port.native_hotplug=yes
> > >
> > >            ports have native hotplug
> > >
> > >
> > >        (6) acpi-pci-hotplug-with-bridge-support=no

> > >            pcie-root-port.hotplug=no
> > >            pcie-root-port.native_hotplug=yes
ditto

> > >
> > >            ports have no hotplug, despite native_hotplug=yes IIUC
> > >
> > >
> > >        (7) acpi-pci-hotplug-with-bridge-support=no
> > >            pcie-root-port.hotplug=yes
> > >            pcie-root-port.native_hotplug=no
> > >
> > >            ports have no hotplug, despite hotplug=yes IIUC
looks wrong too, but harder to check without picking into
global acpi-pci-hotplug-with-bridge-support from 'random'
'port' device. not sure what to do here

> > >
> > >        (8) acpi-pci-hotplug-with-bridge-support=no
> > >            pcie-root-port.hotplug=no
> > >            pcie-root-port.native_hotplug=no
> > >
> > >            ports have no hotplug

Ani,

it would be nice to have a similar doc in QEMU, lets say docs/pic_hoptplug.rst
and maybe move there relevant sections from pcie.txt/pcie_pci_bridge.txt
to have everything PCI hotplug related in one place.


> > >
> > >
> > > Regards,
> > > Daniel
> > > --
> > > |: https://berrange.com      -o-    
> > > https://www.flickr.com/photos/dberrange :|
> > > |: https://libvirt.org         -o-            
> > > https://fstop138.berrange.com :|
> > > |: https://entangle-photo.org    -o-    
> > > https://www.instagram.com/dberrange :|
> > >
> > >  
> 
> 
> Regards,
> Daniel


Reply via email to