Re: [RFC PATCH v2 00/16] ACPI based system device hotplug framework

[Toshi Kani]

On Sat, 2012-08-04 at 20:13 +0800, Jiang Liu wrote:
> From: Jiang Liu <liuj97@xxxxxxxxx>
> 
> The patchset is based on v3.5-rc6 and you may pull them from:
> git://github.com/jiangliu/linux.git acpihp
> 
> Modern high-end server may support advanced hotplug features for system
> devices, including physical processor, memory board, IO extension board
> and/or computer node. The ACPI specifications have provided standard
> interfaces between firmware and OS to support device hotplug at runtime.
> This patch series provide an ACPI based hotplug framework to support system
> device hotplug at runtime, which will replace current existing ACPI device
> driver based CPU/memory/CONTAINER hotplug mechanism.
> 
> The new ACPI based hotplug framework is modelled after PCI hotplug
> architecture and target to achieve following goals:

Hi Jiang,

It is nice to see such infrastructure work!  I have some high-level
questions / comments below.  So far, I have only looked at part of the
changes briefly, so please correct me if I missed something.

> 1) Provide a mechanism to detect hotplug slots by checking ACPI _EJ0 method,
> ACPI PRCT (platform RAS capabilities table) and other platform specific
> mechanisms.

Does this mean that hot-plug device must support both hot-add &
hot-delete operations?  Some platforms may choose to only support
hot-add operations to increase the resource on-line (since it requires
less effort and Windows does not support hot-remove, either).

> 2) Unify the way to enumerate ACPI based hotplug slots. All hotplug slots
> will be enumerated by the enumeration driver, instead of by ACPI device
> drivers.

It is nice to see redundant ACPI namespace walks removed from the ACPI
drivers.  But why do you need to add a new enumerator to create the
acpihp_slot tree, in addition to the current acpi_device tree?  I'd
prefer hotplug features to be generally integrated into the current ACPI
core code and data structures, instead of adding a new layer on top of
it.

Also, acpihp_dev_get_type() in core.c relies on PNP IDs that is embedded
in the file.  This does not seem very flexible / extendable.  One should
be able to add a handler for a new or vendor-specific PNP ID without
changing the core code.  struct acpi_driver allows such extension today.

> 3) Dynamically create/destroy ACPI hotplug slots. For example, new ACPI
> hotplug slots may be created when hot-adding a computer node if the node
> contains some memory hotplug slots.

This is good, but it is necessary because you added the slots...

> 4) Unify the way to handle ACPI hotplug events. All ACPI hotplug events
> for system devices will be handled by a generic ACPI hotplug driver,
> instead of handled by ACPI device drivers.

It seems that acpihp_drv_slot_add() registers an ACPI notify handler
through .add_dev interface.  Does it mean that a device must be marked
as present prior to hot-add operation..?

> 5) Solve dependencies among hotplug slots. You need first to remove the
> memory device before removing a physical processor if a hotpluggable memory
> device is connected to a hotpluggable physical processor.

This is nice, but in your example, I'd expect that a container object
(as a node or socket) is used to generally represent such dependency
with topology.  Such container object contains processor and memory
devices.  When user needs to eject the whole, i.e. both processor and
memory, an eject request can be sent to the container object.

> 6) Provide better error handling and recover.
> 
> 7) Provide interface to cancel ongoing hotplug operations. It may take a
> very long time to remove a memory device, so provide interface to cancel
> the operation.
>
> 8) Support new RAS features, such as socket/memory migration.
> 
> 9) Provide better user interfaces to the hotplug functionalities.

I have not looked at the code yet, but these goals sound great.


Thanks,
-Toshi


> The new hotplug framework includes four logical components.
> 
> The first is an ACPI hotplug slot enumerator, which enumerates ACPI hotplug
> slots on load and provides callbacks to manage those hotplug slots. An ACPI
> hotplug slot is an abstraction of receptacles, where a group of system
> devices could be connected to.
> 
> The second is a device class for ACPI hotplug slots, named acpihp_slot_class.
> All ACPI hotplug slot devices will be associated with acpihp_slot_class.
> 
> The third is a platform independent class driver for ACPI hotplug slots,
> which registers itself onto acpihp_slot_class and manages all ACPI hotplug
> slots in system. This hotplug driver handles ACPI hotplug events, processes
> user requests and manages slot state machine accoring to user requests.
> 
> The fourth is a series of ACPI device drivers, for CPU, memory, PCI host
> bridge, IOAPIC and ACPI CONTAINER. These ACPI device drivers provide device
> specific callbacks for the hotplug driver to add/remove system devices at
> runtime.
> 
> This patch set implements the first and second parts, which enumerates
> hotplug slots and creates sysfs entries for each slot as below.
> 
> linux-drf:/sys/devices/LNXSYSTM:00/acpihp # ll
> drwxr-xr-x 4 root root 0 Jul 28 16:00 NODE00
> drwxr-xr-x 3 root root 0 Jul 28 16:00 NODE01
> drwxr-xr-x 3 root root 0 Jul 28 16:00 NODE02
> 
> linux-drf:/sys/devices/LNXSYSTM:00/acpihp/NODE00 # ll
> drwxr-xr-x 3 root root     0 Jul 28 16:00 IOX01
> -r--r--r-- 1 root root 65536 Jul 28 16:01 capabilities
> lrwxrwxrwx 1 root root     0 Jul 28 16:00 device -> ../../../LNXSYSTM:00
> -r--r--r-- 1 root root 65536 Jul 28 16:01 object
> drwxr-xr-x 2 root root     0 Jul 28 16:01 power
> -r--r--r-- 1 root root 65536 Jul 28 16:01 state
> -r--r--r-- 1 root root 65536 Jul 28 16:01 status
> lrwxrwxrwx 1 root root     0 Jul 28 16:00 subsystem -> ../../../../class/acpihp
> -r--r--r-- 1 root root 65536 Jul 28 16:01 type
> -rw-r--r-- 1 root root 65536 Jul 28 16:01 uevent
> 
> linux-drf:/sys/bus/acpi/acpihp # ls
> NODE00  NODE00.IOX01  NODE01  NODE02
> 
> linux-drf:/sys/bus/acpi/acpihp # ll
> lrwxrwxrwx 1 root root 0 Jul 28 16:03 NODE00 ->
> 		../../../devices/LNXSYSTM:00/acpihp/NODE00
> lrwxrwxrwx 1 root root 0 Jul 28 16:03 NODE00.IOX01 ->
> 		../../../devices/LNXSYSTM:00/acpihp/NODE00/IOX01
> lrwxrwxrwx 1 root root 0 Jul 28 16:03 NODE01 ->
> 		../../../devices/LNXSYSTM:00/acpihp/NODE01
> lrwxrwxrwx 1 root root 0 Jul 28 16:03 NODE02 ->
> 		../../../devices/LNXSYSTM:00/acpihp/NODE02
> 
> V1->V2 changes:
> 1) implement the ACPI hotplug driver
> 2) enhance ACPI container driver to support new hotplug framework
> TODO:
> 1) handle ACPI hotplug events in ACPI hotplug driver
> 2) handle dependencies among slots by evaluating ACPI _EDL
> 3) enhance ACPI processor, memory and PCI root drivers to support new
>    hotplug framework
> 4) develop an ACPI driver for IOAPIC
> 5) thorough tests:)
> 
> Jiang Liu (16):
>   ACPIHP: introduce a framework for ACPI based system device hotplug
>   ACPIHP: ACPI system device hotplug slot enumerator
>   ACPIHP: detect ACPI hotplug slots by checking ACPI _EJ0 method
>   ACPI: introduce interfaces to manage ACPI device reference count
>   ACPIHP: scan and walk ACPI devices connecting to an ACPI hotplug slot
>   ACPIHP: group devices connecting to a hotplug slot according to
>     device types
>   ACPIHP: add callbacks into acpi_device_ops to support new hotplug
>     framework
>   ACPIHP: provide interfaces to associate driver specific data to
>     hotplug slots
>   ACPIHP: implement utility functions to support system device hotplug
>   ACPIHP: system device hotplug driver skeleton
>   ACPIHP: analyse dependences among ACPI system device hotplug slots
>   ACPIHP: cancel inprogress ACPI system device hotplug operations
>   ACPIHP: configure/unconfigure system devices connecting to a hotplug
>     slot
>   ACPIHP: implement the state machine for ACPI hotplug slots
>   ACPIHP: implement ACPI hotplug sysfs interfaces
>   ACPIHP: enhance ACPI container driver to support new hotplug
>     framework
> 
>  drivers/acpi/Kconfig                   |   45 ++
>  drivers/acpi/Makefile                  |    2 +
>  drivers/acpi/bus.c                     |   22 +-
>  drivers/acpi/container.c               |  201 ++------
>  drivers/acpi/hotplug/Makefile          |   18 +
>  drivers/acpi/hotplug/acpihp_drv.h      |  107 ++++
>  drivers/acpi/hotplug/cancel.c          |  171 +++++++
>  drivers/acpi/hotplug/configure.c       |  349 +++++++++++++
>  drivers/acpi/hotplug/core.c            |  874 ++++++++++++++++++++++++++++++++
>  drivers/acpi/hotplug/dependency.c      |  167 ++++++
>  drivers/acpi/hotplug/device.c          |  193 +++++++
>  drivers/acpi/hotplug/drv_main.c        |  337 ++++++++++++
>  drivers/acpi/hotplug/slot_enum.c       |  469 +++++++++++++++++
>  drivers/acpi/hotplug/slot_enum_ej0.c   |  113 +++++
>  drivers/acpi/hotplug/state_machine.c   |  633 +++++++++++++++++++++++
>  drivers/acpi/hotplug/sysfs.c           |  246 +++++++++
>  drivers/acpi/internal.h                |    5 +
>  drivers/acpi/scan.c                    |   17 +-
>  drivers/gpu/drm/i915/intel_opregion.c  |    2 +
>  drivers/gpu/drm/nouveau/nouveau_acpi.c |    1 +
>  drivers/pci/hotplug/acpiphp_glue.c     |    9 +-
>  drivers/pci/hotplug/acpiphp_ibm.c      |    5 +-
>  drivers/pci/hotplug/sgi_hotplug.c      |    6 +-
>  drivers/platform/x86/thinkpad_acpi.c   |    2 +
>  drivers/pnp/pnpacpi/core.c             |   23 +-
>  include/acpi/acpi_bus.h                |   21 +-
>  include/acpi/acpi_hotplug.h            |  315 ++++++++++++
>  include/acpi/container.h               |   12 -
>  28 files changed, 4161 insertions(+), 204 deletions(-)
>  create mode 100644 drivers/acpi/hotplug/Makefile
>  create mode 100644 drivers/acpi/hotplug/acpihp_drv.h
>  create mode 100644 drivers/acpi/hotplug/cancel.c
>  create mode 100644 drivers/acpi/hotplug/configure.c
>  create mode 100644 drivers/acpi/hotplug/core.c
>  create mode 100644 drivers/acpi/hotplug/dependency.c
>  create mode 100644 drivers/acpi/hotplug/device.c
>  create mode 100644 drivers/acpi/hotplug/drv_main.c
>  create mode 100644 drivers/acpi/hotplug/slot_enum.c
>  create mode 100644 drivers/acpi/hotplug/slot_enum_ej0.c
>  create mode 100644 drivers/acpi/hotplug/state_machine.c
>  create mode 100644 drivers/acpi/hotplug/sysfs.c
>  create mode 100644 include/acpi/acpi_hotplug.h
>  delete mode 100644 include/acpi/container.h
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/