[Qemu-devel] [PATCH v1 0/7] pci: hotplug handler reworks

David Hildenbrand posted 7 patches 7 years ago
Failed in applying to current master (apply log)
There is a newer version of this series
hw/acpi/pcihp.c                 | 40 +++++++++++++++++++++++-----
hw/acpi/piix4.c                 | 39 ++++++++++++++-------------
hw/pci-bridge/pci_bridge_dev.c  | 23 +++++++++++++++-
hw/pci-bridge/pcie_pci_bridge.c | 23 +++++++++++++++-
hw/pci/pcie.c                   | 10 ++++++-
hw/pci/pcie_port.c              |  1 +
hw/pci/shpc.c                   | 47 ++++++++++++++++++---------------
hw/ppc/spapr_pci.c              | 33 ++++++++++++++---------
include/hw/acpi/pcihp.h         |  5 ++++
include/hw/pci/pcie.h           |  2 ++
include/hw/pci/shpc.h           |  4 +++
11 files changed, 165 insertions(+), 62 deletions(-)
[Qemu-devel] [PATCH v1 0/7] pci: hotplug handler reworks
Posted by David Hildenbrand 7 years ago
This series reworks some pci hotplug handlers (except for s390, that will
require more work but is not required for now).

1. Route all unplug calls via the hotplug handler when called from the
   unplug_request handler. This will be required to get multi-stage
   hotplug handlers running, but also makes sense on its own (just like we
   already did for some CPU/memory hotplug handlers).

2. Introduce some pre_plug handlers where it makes sense already.

3. Call the plug/pre_plug handler also for coldplugged devices. Especially
   pcihp is special as it overwrites hotplug handlers.

This series will not yet factor out pre_plug/plug/unplug from pci device
realize/unrealize functions, this will require more work but this
series is also required first to get it running.

David Hildenbrand (7):
  pcihp: perform check for bus capability in pre_plug handler
  pcihp: overwrite hotplug handler recursively from the start
  pcihp: route unplug via the hotplug handler
  pci/pcie: route unplug via the hotplug handler
  pci/shpc: move hotplug checks to preplug handler
  pci/shpc: route unplug via the hotplug handler
  spapr_pci: route unplug via the hotplug handler

 hw/acpi/pcihp.c                 | 40 +++++++++++++++++++++++-----
 hw/acpi/piix4.c                 | 39 ++++++++++++++-------------
 hw/pci-bridge/pci_bridge_dev.c  | 23 +++++++++++++++-
 hw/pci-bridge/pcie_pci_bridge.c | 23 +++++++++++++++-
 hw/pci/pcie.c                   | 10 ++++++-
 hw/pci/pcie_port.c              |  1 +
 hw/pci/shpc.c                   | 47 ++++++++++++++++++---------------
 hw/ppc/spapr_pci.c              | 33 ++++++++++++++---------
 include/hw/acpi/pcihp.h         |  5 ++++
 include/hw/pci/pcie.h           |  2 ++
 include/hw/pci/shpc.h           |  4 +++
 11 files changed, 165 insertions(+), 62 deletions(-)

-- 
2.17.2


Re: [Qemu-devel] [PATCH v1 0/7] pci: hotplug handler reworks
Posted by David Hildenbrand 7 years ago
On 24.10.18 12:19, David Hildenbrand wrote:
> This series reworks some pci hotplug handlers (except for s390, that will
> require more work but is not required for now).
> 
> 1. Route all unplug calls via the hotplug handler when called from the
>    unplug_request handler. This will be required to get multi-stage
>    hotplug handlers running, but also makes sense on its own (just like we
>    already did for some CPU/memory hotplug handlers).
> 
> 2. Introduce some pre_plug handlers where it makes sense already.
> 
> 3. Call the plug/pre_plug handler also for coldplugged devices. Especially
>    pcihp is special as it overwrites hotplug handlers.
> 
> This series will not yet factor out pre_plug/plug/unplug from pci device
> realize/unrealize functions, this will require more work but this
> series is also required first to get it running.
> 
> David Hildenbrand (7):
>   pcihp: perform check for bus capability in pre_plug handler
>   pcihp: overwrite hotplug handler recursively from the start
>   pcihp: route unplug via the hotplug handler
>   pci/pcie: route unplug via the hotplug handler
>   pci/shpc: move hotplug checks to preplug handler
>   pci/shpc: route unplug via the hotplug handler
>   spapr_pci: route unplug via the hotplug handler
> 
>  hw/acpi/pcihp.c                 | 40 +++++++++++++++++++++++-----
>  hw/acpi/piix4.c                 | 39 ++++++++++++++-------------
>  hw/pci-bridge/pci_bridge_dev.c  | 23 +++++++++++++++-
>  hw/pci-bridge/pcie_pci_bridge.c | 23 +++++++++++++++-
>  hw/pci/pcie.c                   | 10 ++++++-
>  hw/pci/pcie_port.c              |  1 +
>  hw/pci/shpc.c                   | 47 ++++++++++++++++++---------------
>  hw/ppc/spapr_pci.c              | 33 ++++++++++++++---------
>  include/hw/acpi/pcihp.h         |  5 ++++
>  include/hw/pci/pcie.h           |  2 ++
>  include/hw/pci/shpc.h           |  4 +++
>  11 files changed, 165 insertions(+), 62 deletions(-)
> 

Did some more testing. Can somebody have a look/pick up? Thanks!

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v1 0/7] pci: hotplug handler reworks
Posted by Igor Mammedov 7 years ago
On Wed, 31 Oct 2018 18:31:30 +0100
David Hildenbrand <david@redhat.com> wrote:

> On 24.10.18 12:19, David Hildenbrand wrote:
> > This series reworks some pci hotplug handlers (except for s390, that will
> > require more work but is not required for now).
> > 
> > 1. Route all unplug calls via the hotplug handler when called from the
> >    unplug_request handler. This will be required to get multi-stage
> >    hotplug handlers running, but also makes sense on its own (just like we
> >    already did for some CPU/memory hotplug handlers).
> > 
> > 2. Introduce some pre_plug handlers where it makes sense already.
> > 
> > 3. Call the plug/pre_plug handler also for coldplugged devices. Especially
> >    pcihp is special as it overwrites hotplug handlers.
> > 
> > This series will not yet factor out pre_plug/plug/unplug from pci device
> > realize/unrealize functions, this will require more work but this
> > series is also required first to get it running.
> > 
> > David Hildenbrand (7):
> >   pcihp: perform check for bus capability in pre_plug handler
> >   pcihp: overwrite hotplug handler recursively from the start
> >   pcihp: route unplug via the hotplug handler
> >   pci/pcie: route unplug via the hotplug handler
> >   pci/shpc: move hotplug checks to preplug handler
> >   pci/shpc: route unplug via the hotplug handler
> >   spapr_pci: route unplug via the hotplug handler
> > 
> >  hw/acpi/pcihp.c                 | 40 +++++++++++++++++++++++-----
> >  hw/acpi/piix4.c                 | 39 ++++++++++++++-------------
> >  hw/pci-bridge/pci_bridge_dev.c  | 23 +++++++++++++++-
> >  hw/pci-bridge/pcie_pci_bridge.c | 23 +++++++++++++++-
> >  hw/pci/pcie.c                   | 10 ++++++-
> >  hw/pci/pcie_port.c              |  1 +
> >  hw/pci/shpc.c                   | 47 ++++++++++++++++++---------------
> >  hw/ppc/spapr_pci.c              | 33 ++++++++++++++---------
> >  include/hw/acpi/pcihp.h         |  5 ++++
> >  include/hw/pci/pcie.h           |  2 ++
> >  include/hw/pci/shpc.h           |  4 +++
> >  11 files changed, 165 insertions(+), 62 deletions(-)
> >   
> 
> Did some more testing. Can somebody have a look/pick up? Thanks!
Did a quick pass over series, patches overall looks good here is some other nits
that apply to series:
 * make more descriptive commit messages (important for history and for whoever comes later to read it)
 * I don't really like a mix of style in callbacks naming
    for ex: there is hotplug and then for unplug you add hot_unplug instead of hotunplug
    I'd prefer consistent approach in naming.
    (either use underscores or drop them or maybe drop 'hot' part as it's not only for hotplug anymore)


Re: [Qemu-devel] [PATCH v1 0/7] pci: hotplug handler reworks
Posted by David Hildenbrand 7 years ago
On 01.11.18 15:55, Igor Mammedov wrote:
> On Wed, 31 Oct 2018 18:31:30 +0100
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 24.10.18 12:19, David Hildenbrand wrote:
>>> This series reworks some pci hotplug handlers (except for s390, that will
>>> require more work but is not required for now).
>>>
>>> 1. Route all unplug calls via the hotplug handler when called from the
>>>    unplug_request handler. This will be required to get multi-stage
>>>    hotplug handlers running, but also makes sense on its own (just like we
>>>    already did for some CPU/memory hotplug handlers).
>>>
>>> 2. Introduce some pre_plug handlers where it makes sense already.
>>>
>>> 3. Call the plug/pre_plug handler also for coldplugged devices. Especially
>>>    pcihp is special as it overwrites hotplug handlers.
>>>
>>> This series will not yet factor out pre_plug/plug/unplug from pci device
>>> realize/unrealize functions, this will require more work but this
>>> series is also required first to get it running.
>>>
>>> David Hildenbrand (7):
>>>   pcihp: perform check for bus capability in pre_plug handler
>>>   pcihp: overwrite hotplug handler recursively from the start
>>>   pcihp: route unplug via the hotplug handler
>>>   pci/pcie: route unplug via the hotplug handler
>>>   pci/shpc: move hotplug checks to preplug handler
>>>   pci/shpc: route unplug via the hotplug handler
>>>   spapr_pci: route unplug via the hotplug handler
>>>
>>>  hw/acpi/pcihp.c                 | 40 +++++++++++++++++++++++-----
>>>  hw/acpi/piix4.c                 | 39 ++++++++++++++-------------
>>>  hw/pci-bridge/pci_bridge_dev.c  | 23 +++++++++++++++-
>>>  hw/pci-bridge/pcie_pci_bridge.c | 23 +++++++++++++++-
>>>  hw/pci/pcie.c                   | 10 ++++++-
>>>  hw/pci/pcie_port.c              |  1 +
>>>  hw/pci/shpc.c                   | 47 ++++++++++++++++++---------------
>>>  hw/ppc/spapr_pci.c              | 33 ++++++++++++++---------
>>>  include/hw/acpi/pcihp.h         |  5 ++++
>>>  include/hw/pci/pcie.h           |  2 ++
>>>  include/hw/pci/shpc.h           |  4 +++
>>>  11 files changed, 165 insertions(+), 62 deletions(-)
>>>   
>>
>> Did some more testing. Can somebody have a look/pick up? Thanks!
> Did a quick pass over series, patches overall looks good here is some other nits
> that apply to series:

Thanks for the review, will address the comments tomorrow!

>  * make more descriptive commit messages (important for history and for whoever comes later to read it)

Yes, I'll add more details. I agree that people without context might
have a harder time figuring out why this is done.

>  * I don't really like a mix of style in callbacks naming
>     for ex: there is hotplug and then for unplug you add hot_unplug instead of hotunplug
>     I'd prefer consistent approach in naming.
>     (either use underscores or drop them or maybe drop 'hot' part as it's not only for hotplug anymore)

Yes, me too. And I guess I will add some cleanup patches first, that
will introduce a common naming scheme, (especially dropping the "hot")


-- 

Thanks,

David / dhildenb