[PATCH v2 0/7] ACPI: Add _OST support for ACPI hotplug

[PATCH v2 0/7] ACPI: Add _OST support for ACPI hotplug

[Toshi Kani]

This patchset supports ACPI OSPM Status Indication (_OST) method for
ACPI CPU/memory/container hotplug operations and sysfs eject. After
ACPI hotplug operation has completed, OSPM calls _OST to indicate the
status of the operation to the platform. If _OST is not present, this
patchset has no effect on the platform.

This _OST support can be enabled or disabled with a new config option
CONFIG_ACPI_HOTPLUG_OST. This option is disabled by default. When
this option is disabled, this patchset has no effect on the platform.

The _OST definition can be found in section 6.3.5 of ACPI 5.0 spec.
The HPPF spec below also describes hotplug flows with _OST.

  DIG64 Hot-Plug & Partitioning Flow (HPPF) Specification R1.0
  http://www.dig64.org/home/DIG64_HPPF_R1_0.pdf

The changes have been tested with simulated _OST methods.

v2:
 - Added CONFIG_ACPI_HOTPLUG_OST option. 
 - Added _OST support for container hotplug and sysfs eject.
 - Reordered patchset to enable _OST support bit of _OSC in the 
   last patch.

---
Toshi Kani (7):
 ACPI: Add CONFIG_HOTPLUG_OST option
 ACPI: Add an interface to evaluate _OST
 ACPI: Add _OST support for sysfs eject
 ACPI: Add _OST support for ACPI CPU hotplug
 ACPI: Add _OST support for ACPI memory hotplug
 ACPI: Add _OST support for ACPI container hotplug
 ACPI: Set hotplug _OST support bit to _OSC

 drivers/acpi/Kconfig            |   14 +++++++++
 drivers/acpi/acpi_memhotplug.c  |   43 +++++++++++++++++++++-------
 drivers/acpi/bus.c              |    4 +++
 drivers/acpi/container.c        |   43 +++++++++++++++++++----------
 drivers/acpi/processor_driver.c |   28 +++++++++++++-----
 drivers/acpi/scan.c             |   58 +++++++++++++++++++++++++++++++++------
 drivers/acpi/utils.c            |   38 +++++++++++++++++++++++++
 include/acpi/acpi_bus.h         |   12 +++++++-
 include/linux/acpi.h            |   31 ++++++++++++++++++++-
 9 files changed, 226 insertions(+), 45 deletions(-)

[PATCH v2 1/7] ACPI: Add CONFIG_HOTPLUG_OST option

[Toshi Kani]

Added CONFIG_ACPI_HOTPLUG_OPT option. When this config option is disabled,
this patchset has no effect on the platform. This option is disabled by
default. The dependency list assures consistent behavior among CPU, memory
and container hotplug operations with regarding the _OST support.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/acpi/Kconfig |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 47768ff..1e3a756 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -351,6 +351,20 @@ config ACPI_HOTPLUG_MEMORY
 	  To compile this driver as a module, choose M here:
 	  the module will be called acpi_memhotplug.
 
+config ACPI_HOTPLUG_OST
+	bool "Hotplug Status Indication Support"
+	depends on ACPI_HOTPLUG_CPU
+	depends on ACPI_HOTPLUG_MEMORY
+	depends on ACPI_CONTAINER
+	default n
+	help
+	  This option enables the use of the ACPI OSPM Status Indication
+          (_OST), if available, for the platform to determine the status
+	  of various hotplug operations. Some platforms may require this
+	  option be enabled to track the status of hotplug operations.
+	  This option has no effect when the platform does not support
+	  _OST.
+
 config ACPI_SBS
 	tristate "Smart Battery System"
 	depends on X86
-- 
1.7.7.6

Re: [PATCH v2 1/7] ACPI: Add CONFIG_HOTPLUG_OST option

[Jiang Liu]

On 05/09/2012 04:12 AM, Toshi Kani wrote:
> Added CONFIG_ACPI_HOTPLUG_OPT option. When this config option is disabled,
> this patchset has no effect on the platform. This option is disabled by
> default. The dependency list assures consistent behavior among CPU, memory
> and container hotplug operations with regarding the _OST support.
Seems also need to enhance the acpiphp driver for PCI hotplug operations too.
acpiphp driver is also based on the ACPI device hotplug model.

> 
> Signed-off-by: Toshi Kani <toshi.kani@hp.com>
> ---
>  drivers/acpi/Kconfig |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 47768ff..1e3a756 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -351,6 +351,20 @@ config ACPI_HOTPLUG_MEMORY
>  	  To compile this driver as a module, choose M here:
>  	  the module will be called acpi_memhotplug.
>  
> +config ACPI_HOTPLUG_OST
> +	bool "Hotplug Status Indication Support"
> +	depends on ACPI_HOTPLUG_CPU
> +	depends on ACPI_HOTPLUG_MEMORY
> +	depends on ACPI_CONTAINER
> +	default n
> +	help
> +	  This option enables the use of the ACPI OSPM Status Indication
> +          (_OST), if available, for the platform to determine the status
> +	  of various hotplug operations. Some platforms may require this
> +	  option be enabled to track the status of hotplug operations.
> +	  This option has no effect when the platform does not support
> +	  _OST.
> +
>  config ACPI_SBS
>  	tristate "Smart Battery System"
>  	depends on X86

Re: [PATCH v2 1/7] ACPI: Add CONFIG_HOTPLUG_OST option

[Toshi Kani]

On Fri, 2012-05-11 at 00:40 +0800, Jiang Liu wrote:
> On 05/09/2012 04:12 AM, Toshi Kani wrote:
> > Added CONFIG_ACPI_HOTPLUG_OPT option. When this config option is disabled,
> > this patchset has no effect on the platform. This option is disabled by
> > default. The dependency list assures consistent behavior among CPU, memory
> > and container hotplug operations with regarding the _OST support.
> Seems also need to enhance the acpiphp driver for PCI hotplug operations too.
> acpiphp driver is also based on the ACPI device hotplug model.

Good question.  ACPI PCI hotplug is a legacy method, which is being
replaced by PCI native hotplug.  I expect PCI native hotplug will be
used for any platforms supporting PCI-E.  Since this _OST support is
targeted for new platforms (as _OST is not supported on the current
platforms), I thought that it would be prudent to avoid changing PCI
hotplug code for this.


Thanks,
-Toshi


> > 
> > Signed-off-by: Toshi Kani <toshi.kani@hp.com>
> > ---
> >  drivers/acpi/Kconfig |   14 ++++++++++++++
> >  1 files changed, 14 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > index 47768ff..1e3a756 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -351,6 +351,20 @@ config ACPI_HOTPLUG_MEMORY
> >  	  To compile this driver as a module, choose M here:
> >  	  the module will be called acpi_memhotplug.
> >  
> > +config ACPI_HOTPLUG_OST
> > +	bool "Hotplug Status Indication Support"
> > +	depends on ACPI_HOTPLUG_CPU
> > +	depends on ACPI_HOTPLUG_MEMORY
> > +	depends on ACPI_CONTAINER
> > +	default n
> > +	help
> > +	  This option enables the use of the ACPI OSPM Status Indication
> > +          (_OST), if available, for the platform to determine the status
> > +	  of various hotplug operations. Some platforms may require this
> > +	  option be enabled to track the status of hotplug operations.
> > +	  This option has no effect when the platform does not support
> > +	  _OST.
> > +
> >  config ACPI_SBS
> >  	tristate "Smart Battery System"
> >  	depends on X86
> 

Re: [PATCH v2 1/7] ACPI: Add CONFIG_HOTPLUG_OST option

[Jiang Liu]

On 05/11/2012 01:26 AM, Toshi Kani wrote:
> On Fri, 2012-05-11 at 00:40 +0800, Jiang Liu wrote:
>> On 05/09/2012 04:12 AM, Toshi Kani wrote:
>>> Added CONFIG_ACPI_HOTPLUG_OPT option. When this config option is disabled,
>>> this patchset has no effect on the platform. This option is disabled by
>>> default. The dependency list assures consistent behavior among CPU, memory
>>> and container hotplug operations with regarding the _OST support.
>> Seems also need to enhance the acpiphp driver for PCI hotplug operations too.
>> acpiphp driver is also based on the ACPI device hotplug model.
> 
> Good question.  ACPI PCI hotplug is a legacy method, which is being
> replaced by PCI native hotplug.  I expect PCI native hotplug will be
> used for any platforms supporting PCI-E.  Since this _OST support is
> targeted for new platforms (as _OST is not supported on the current
> platforms), I thought that it would be prudent to avoid changing PCI
> hotplug code for this.
You are right, acpiphp is legacy method. But it may also be used to support
some cases which can't be supported by the pciehp driver.
For example, we have a PCIe system which needs to run acpiphp and pciehp
concurrently. The pciehp driver is used to manage hot-pluggable PCIe slot,
and the acpiphp driver is used to migrate downstream ports among virtual 
PCIe switches of an MR-IOV switch. It would be great if we could enable 
_OST for acpiphp too.

To be honest, our BIOS doesn't provide _OST for acpiphp yet, so we could
enhance acpiphp driver in future when a system really needing that feature
appears.

Thanks!
--gerry

Re: [PATCH v2 1/7] ACPI: Add CONFIG_HOTPLUG_OST option

[Toshi Kani]

On Fri, 2012-05-11 at 01:43 +0800, Jiang Liu wrote:
> On 05/11/2012 01:26 AM, Toshi Kani wrote:
> > On Fri, 2012-05-11 at 00:40 +0800, Jiang Liu wrote:
> >> On 05/09/2012 04:12 AM, Toshi Kani wrote:
> >>> Added CONFIG_ACPI_HOTPLUG_OPT option. When this config option is disabled,
> >>> this patchset has no effect on the platform. This option is disabled by
> >>> default. The dependency list assures consistent behavior among CPU, memory
> >>> and container hotplug operations with regarding the _OST support.
> >> Seems also need to enhance the acpiphp driver for PCI hotplug operations too.
> >> acpiphp driver is also based on the ACPI device hotplug model.
> > 
> > Good question.  ACPI PCI hotplug is a legacy method, which is being
> > replaced by PCI native hotplug.  I expect PCI native hotplug will be
> > used for any platforms supporting PCI-E.  Since this _OST support is
> > targeted for new platforms (as _OST is not supported on the current
> > platforms), I thought that it would be prudent to avoid changing PCI
> > hotplug code for this.
> You are right, acpiphp is legacy method. But it may also be used to support
> some cases which can't be supported by the pciehp driver.
> For example, we have a PCIe system which needs to run acpiphp and pciehp
> concurrently. The pciehp driver is used to manage hot-pluggable PCIe slot,
> and the acpiphp driver is used to migrate downstream ports among virtual 
> PCIe switches of an MR-IOV switch. It would be great if we could enable 
> _OST for acpiphp too.
> 
> To be honest, our BIOS doesn't provide _OST for acpiphp yet, so we could
> enhance acpiphp driver in future when a system really needing that feature
> appears.

Interesting.  In such case, I am willing to change acpiphp, but I am
afraid that I do not have access to platforms with ACPI PCI hotplug
support.  My test systems are all PCI-E based.  So, it would be great if
you could enhance acpiphp when you need it.


Thanks!
-Toshi

Re: [PATCH v2 1/7] ACPI: Add CONFIG_HOTPLUG_OST option

[Jiang Liu]

On 05/11/2012 02:20 AM, Toshi Kani wrote:
> On Fri, 2012-05-11 at 01:43 +0800, Jiang Liu wrote:
>> On 05/11/2012 01:26 AM, Toshi Kani wrote:
>>> On Fri, 2012-05-11 at 00:40 +0800, Jiang Liu wrote:
>>>> On 05/09/2012 04:12 AM, Toshi Kani wrote:
>>>>> Added CONFIG_ACPI_HOTPLUG_OPT option. When this config option is disabled,
>>>>> this patchset has no effect on the platform. This option is disabled by
>>>>> default. The dependency list assures consistent behavior among CPU, memory
>>>>> and container hotplug operations with regarding the _OST support.
>>>> Seems also need to enhance the acpiphp driver for PCI hotplug operations too.
>>>> acpiphp driver is also based on the ACPI device hotplug model.
>>>
>>> Good question.  ACPI PCI hotplug is a legacy method, which is being
>>> replaced by PCI native hotplug.  I expect PCI native hotplug will be
>>> used for any platforms supporting PCI-E.  Since this _OST support is
>>> targeted for new platforms (as _OST is not supported on the current
>>> platforms), I thought that it would be prudent to avoid changing PCI
>>> hotplug code for this.
>> You are right, acpiphp is legacy method. But it may also be used to support
>> some cases which can't be supported by the pciehp driver.
>> For example, we have a PCIe system which needs to run acpiphp and pciehp
>> concurrently. The pciehp driver is used to manage hot-pluggable PCIe slot,
>> and the acpiphp driver is used to migrate downstream ports among virtual 
>> PCIe switches of an MR-IOV switch. It would be great if we could enable 
>> _OST for acpiphp too.
>>
>> To be honest, our BIOS doesn't provide _OST for acpiphp yet, so we could
>> enhance acpiphp driver in future when a system really needing that feature
>> appears.
> 
> Interesting.  In such case, I am willing to change acpiphp, but I am
> afraid that I do not have access to platforms with ACPI PCI hotplug
> support.  My test systems are all PCI-E based.  So, it would be great if
> you could enhance acpiphp when you need it.
Sure, I will enhance it based on your work when needed.
Thanks!
-Gerry

> 
> 
> Thanks!
> -Toshi
> 

[PATCH v2 2/7] ACPI: Add an interface to evaluate _OST

[Toshi Kani]

Added acpi_evaluate_hotplug_opt(). This function evaluates _OST for hotplug
operations. This function is stubbed out when CONFIG_ACPI_HOTPLUG_OST is
disabled.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/acpi/utils.c    |   38 ++++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |    3 +++
 2 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index b002a47..39d7349 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -382,3 +382,41 @@ acpi_evaluate_reference(acpi_handle handle,
 }
 
 EXPORT_SYMBOL(acpi_evaluate_reference);
+
+/*
+ * acpi_evaluate_hotplug_ost: Evaluate _OST for hotplug operations
+ * @handle: ACPI device handle
+ * @source_event: source event code
+ * @status_code: status code
+ * @status_buf: optional detailed information (NULL if none)
+ */
+acpi_status
+acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
+		u32 status_code, struct acpi_buffer *status_buf)
+{
+#ifdef CONFIG_ACPI_HOTPLUG_OST
+	union acpi_object params[3] = {
+		{.type = ACPI_TYPE_INTEGER,},
+		{.type = ACPI_TYPE_INTEGER,},
+		{.type = ACPI_TYPE_BUFFER,}
+	};
+	struct acpi_object_list arg_list = {3, params};
+	acpi_status status;
+
+	params[0].integer.value = source_event;
+	params[1].integer.value = status_code;
+	if (status_buf != NULL) {
+		params[2].buffer.pointer = status_buf->pointer;
+		params[2].buffer.length = status_buf->length;
+	} else {
+		params[2].buffer.pointer = NULL;
+		params[2].buffer.length = 0;
+	}
+
+	status = acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
+	return status;
+#else
+	return AE_OK;
+#endif
+}
+EXPORT_SYMBOL(acpi_evaluate_hotplug_ost);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index f1c8ca6..7309113 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -50,6 +50,9 @@ acpi_evaluate_reference(acpi_handle handle,
 			acpi_string pathname,
 			struct acpi_object_list *arguments,
 			struct acpi_handle_list *list);
+acpi_status
+acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
+			u32 status_code, struct acpi_buffer *status_buf);
 
 #ifdef CONFIG_ACPI
 
-- 
1.7.7.6

[PATCH v2 3/7] ACPI: Add _OST support for sysfs eject

[Toshi Kani]

Changed acpi_bus_hot_remove_device() to support _OST. This function is
also changed to global so that it can be called from hotplug notify
handlers to perform hot-remove operation.

Changed acpi_eject_store(), which is the sysfs eject handler. It checks
eject_pending to see if the request was originated from ACPI eject
notification. If not, it calls _OST(0x103,84,) per Figure 6-37 in ACPI
5.0 spec.

Added eject_pending bit to acpi_device_flags. This bit is set when the
kernel has received an ACPI eject notification, but does not initiate
its hot-remove operation by itself.

Added struct acpi_eject_event. This structure is used to pass extended
information to acpi_bus_hot_remove_device(), which has a single argument
to support asynchronous call.

Added macro definitions of _OST source events and status codes.
Also renamed OSC_SB_CPUHP_OST_SUPPORT to OSC_SB_HOTPLUG_OST_SUPPORT
since this _OSC bit is not specific to CPU hotplug. This bit is
defined in Table 6-147 of ACPI 5.0 as follows.

  Bits:       3
  Field Name: Insertion / Ejection _OST Processing Support
  Definition: This bit is set if OSPM will evaluate the _OST
              object defined under a device when processing
              insertion and ejection source event codes.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/acpi/scan.c     |   58 +++++++++++++++++++++++++++++++++++++++-------
 include/acpi/acpi_bus.h |    9 ++++++-
 include/linux/acpi.h    |   31 ++++++++++++++++++++++++-
 3 files changed, 87 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 7417267..84ba717 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -83,19 +83,29 @@ acpi_device_modalias_show(struct device *dev, struct device_attribute *attr, cha
 }
 static DEVICE_ATTR(modalias, 0444, acpi_device_modalias_show, NULL);
 
-static void acpi_bus_hot_remove_device(void *context)
+/*
+ * acpi_bus_hot_remove_device: hot-remove a device and its children
+ * @context: struct acpi_eject_event pointer (freed in this func)
+ *
+ * Hot-remove a device and its children. This function frees up the
+ * memory space passed by arg context, so that the caller may call
+ * this function asynchronously through acpi_os_hotplug_execute().
+ */
+void acpi_bus_hot_remove_device(void *context)
 {
+	struct acpi_eject_event *ej_event = (struct acpi_eject_event *) context;
 	struct acpi_device *device;
-	acpi_handle handle = context;
+	acpi_handle handle = ej_event->handle;
 	struct acpi_object_list arg_list;
 	union acpi_object arg;
 	acpi_status status = AE_OK;
+	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE; /* default */
 
 	if (acpi_bus_get_device(handle, &device))
-		return;
+		goto err_out;
 
 	if (!device)
-		return;
+		goto err_out;
 
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 		"Hot-removing device %s...\n", dev_name(&device->dev)));
@@ -103,7 +113,7 @@ static void acpi_bus_hot_remove_device(void *context)
 	if (acpi_bus_trim(device, 1)) {
 		printk(KERN_ERR PREFIX
 				"Removing device failed\n");
-		return;
+		goto err_out;
 	}
 
 	/* power off device */
@@ -129,10 +139,21 @@ static void acpi_bus_hot_remove_device(void *context)
 	 * TBD: _EJD support.
 	 */
 	status = acpi_evaluate_object(handle, "_EJ0", &arg_list, NULL);
-	if (ACPI_FAILURE(status))
-		printk(KERN_WARNING PREFIX
-				"Eject device failed\n");
+	if (ACPI_FAILURE(status)) {
+		if (status != AE_NOT_FOUND)
+			printk(KERN_WARNING PREFIX
+					"Eject device failed\n");
+		goto err_out;
+	}
+
+	kfree(context);
+	return;
 
+err_out:
+	/* Inform firmware the hot-remove operation has completed w/ error */
+	(void) acpi_evaluate_hotplug_ost(handle,
+				ej_event->event, ost_code, NULL);
+	kfree(context);
 	return;
 }
 
@@ -144,6 +165,7 @@ acpi_eject_store(struct device *d, struct device_attribute *attr,
 	acpi_status status;
 	acpi_object_type type = 0;
 	struct acpi_device *acpi_device = to_acpi_device(d);
+	struct acpi_eject_event *ej_event;
 
 	if ((!count) || (buf[0] != '1')) {
 		return -EINVAL;
@@ -160,7 +182,25 @@ acpi_eject_store(struct device *d, struct device_attribute *attr,
 		goto err;
 	}
 
-	acpi_os_hotplug_execute(acpi_bus_hot_remove_device, acpi_device->handle);
+	ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
+	if (!ej_event) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	ej_event->handle = acpi_device->handle;
+	if (acpi_device->flags.eject_pending) {
+		/* event originated from ACPI eject notification */
+		ej_event->event = ACPI_NOTIFY_EJECT_REQUEST;
+		acpi_device->flags.eject_pending = 0;
+	} else {
+		/* event originated from user */
+		ej_event->event = ACPI_OST_EC_OSPM_EJECT;
+		(void) acpi_evaluate_hotplug_ost(ej_event->handle,
+			ej_event->event, ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
+	}
+
+	acpi_os_hotplug_execute(acpi_bus_hot_remove_device, (void *)ej_event);
 err:
 	return ret;
 }
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 7309113..b836800 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -151,7 +151,8 @@ struct acpi_device_flags {
 	u32 suprise_removal_ok:1;
 	u32 power_manageable:1;
 	u32 performance_manageable:1;
-	u32 reserved:24;
+	u32 eject_pending:1;
+	u32 reserved:23;
 };
 
 /* File System */
@@ -303,6 +304,11 @@ struct acpi_bus_event {
 	u32 data;
 };
 
+struct acpi_eject_event {
+	acpi_handle	handle;
+	u32		event;
+};
+
 extern struct kobject *acpi_kobj;
 extern int acpi_bus_generate_netlink_event(const char*, const char*, u8, int);
 void acpi_bus_private_data_handler(acpi_handle, void *);
@@ -340,6 +346,7 @@ int acpi_bus_register_driver(struct acpi_driver *driver);
 void acpi_bus_unregister_driver(struct acpi_driver *driver);
 int acpi_bus_add(struct acpi_device **child, struct acpi_device *parent,
 		 acpi_handle handle, int type);
+void acpi_bus_hot_remove_device(void *context);
 int acpi_bus_trim(struct acpi_device *start, int rmdevice);
 int acpi_bus_start(struct acpi_device *device);
 acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index f421dd8..a4b1d3d 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -277,7 +277,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context);
 #define OSC_SB_PAD_SUPPORT		1
 #define OSC_SB_PPC_OST_SUPPORT		2
 #define OSC_SB_PR3_SUPPORT		4
-#define OSC_SB_CPUHP_OST_SUPPORT	8
+#define OSC_SB_HOTPLUG_OST_SUPPORT	8
 #define OSC_SB_APEI_SUPPORT		16
 
 extern bool osc_sb_apei_support_acked;
@@ -309,6 +309,35 @@ extern bool osc_sb_apei_support_acked;
 
 extern acpi_status acpi_pci_osc_control_set(acpi_handle handle,
 					     u32 *mask, u32 req);
+
+/* _OST Source Event Code (OSPM Action) */
+#define ACPI_OST_EC_OSPM_SHUTDOWN		0x100
+#define ACPI_OST_EC_OSPM_EJECT			0x103
+#define ACPI_OST_EC_OSPM_INSERTION		0x200
+
+/* _OST General Processing Status Code */
+#define ACPI_OST_SC_SUCCESS			0x0
+#define ACPI_OST_SC_NON_SPECIFIC_FAILURE	0x1
+#define ACPI_OST_SC_UNRECOGNIZED_NOTIFY		0x2
+
+/* _OST OS Shutdown Processing (0x100) Status Code */
+#define ACPI_OST_SC_OS_SHUTDOWN_DENIED		0x80
+#define ACPI_OST_SC_OS_SHUTDOWN_IN_PROGRESS	0x81
+#define ACPI_OST_SC_OS_SHUTDOWN_COMPLETED	0x82
+#define ACPI_OST_SC_OS_SHUTDOWN_NOT_SUPPORTED	0x83
+
+/* _OST Ejection Request (0x3, 0x103) Status Code */
+#define ACPI_OST_SC_EJECT_NOT_SUPPORTED		0x80
+#define ACPI_OST_SC_DEVICE_IN_USE		0x81
+#define ACPI_OST_SC_DEVICE_BUSY			0x82
+#define ACPI_OST_SC_EJECT_DEPENDENCY_BUSY	0x83
+#define ACPI_OST_SC_EJECT_IN_PROGRESS		0x84
+
+/* _OST Insertion Request (0x200) Status Code */
+#define ACPI_OST_SC_INSERT_IN_PROGRESS		0x80
+#define ACPI_OST_SC_DRIVER_LOAD_FAILURE		0x81
+#define ACPI_OST_SC_INSERT_NOT_SUPPORTED	0x82
+
 extern void acpi_early_init(void);
 
 extern int acpi_nvs_register(__u64 start, __u64 size);
-- 
1.7.7.6

Re: [PATCH v2 3/7] ACPI: Add _OST support for sysfs eject

[Shuah Khan]

On Tue, 2012-05-08 at 14:12 -0600, Toshi Kani wrote:
> Changed acpi_bus_hot_remove_device() to support _OST. This function is
> also changed to global so that it can be called from hotplug notify
> handlers to perform hot-remove operation.
> 
> Changed acpi_eject_store(), which is the sysfs eject handler. It checks
> eject_pending to see if the request was originated from ACPI eject
> notification. If not, it calls _OST(0x103,84,) per Figure 6-37 in ACPI
> 5.0 spec.
> 
> Added eject_pending bit to acpi_device_flags. This bit is set when the
> kernel has received an ACPI eject notification, but does not initiate
> its hot-remove operation by itself.
> 
> Added struct acpi_eject_event. This structure is used to pass extended
> information to acpi_bus_hot_remove_device(), which has a single argument
> to support asynchronous call.
> 
> Added macro definitions of _OST source events and status codes.
> Also renamed OSC_SB_CPUHP_OST_SUPPORT to OSC_SB_HOTPLUG_OST_SUPPORT
> since this _OSC bit is not specific to CPU hotplug. This bit is
> defined in Table 6-147 of ACPI 5.0 as follows.

I am confused. This patch adds lot of new code that is for _OST handling
without CONFIG_ACPI_HOTPLUG_OST check. Is this intended? Doesn't jive
with the intent communicated in the [PATCH v2 0/7] introduction.

Could you please walk though the steps on what happens with this code on
a system that doesn't enable _OST and doesn't support _OST. That will
help me understand how this code would behave on a system that doesn't
support _OST.

> 
>   Bits:       3
>   Field Name: Insertion / Ejection _OST Processing Support
>   Definition: This bit is set if OSPM will evaluate the _OST
>               object defined under a device when processing
>               insertion and ejection source event codes.
> 
> Signed-off-by: Toshi Kani <toshi.kani@hp.com>
> ---
>  drivers/acpi/scan.c     |   58 +++++++++++++++++++++++++++++++++++++++-------
>  include/acpi/acpi_bus.h |    9 ++++++-
>  include/linux/acpi.h    |   31 ++++++++++++++++++++++++-
>  3 files changed, 87 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 7417267..84ba717 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -83,19 +83,29 @@ acpi_device_modalias_show(struct device *dev, struct device_attribute *attr, cha
>  }
>  static DEVICE_ATTR(modalias, 0444, acpi_device_modalias_show, NULL);
>  
> -static void acpi_bus_hot_remove_device(void *context)
> +/*
> + * acpi_bus_hot_remove_device: hot-remove a device and its children
> + * @context: struct acpi_eject_event pointer (freed in this func)
> + *
> + * Hot-remove a device and its children. This function frees up the
> + * memory space passed by arg context, so that the caller may call
> + * this function asynchronously through acpi_os_hotplug_execute().
> + */
> +void acpi_bus_hot_remove_device(void *context)
>  {
> +	struct acpi_eject_event *ej_event = (struct acpi_eject_event *) context;
>  	struct acpi_device *device;
> -	acpi_handle handle = context;
> +	acpi_handle handle = ej_event->handle;
>  	struct acpi_object_list arg_list;
>  	union acpi_object arg;
>  	acpi_status status = AE_OK;
> +	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE; /* default */
>  
>  	if (acpi_bus_get_device(handle, &device))
> -		return;
> +		goto err_out;
>  
>  	if (!device)
> -		return;
> +		goto err_out;
>  
>  	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
>  		"Hot-removing device %s...\n", dev_name(&device->dev)));
> @@ -103,7 +113,7 @@ static void acpi_bus_hot_remove_device(void *context)
>  	if (acpi_bus_trim(device, 1)) {
>  		printk(KERN_ERR PREFIX
>  				"Removing device failed\n");
> -		return;
> +		goto err_out;
>  	}
>  
>  	/* power off device */
> @@ -129,10 +139,21 @@ static void acpi_bus_hot_remove_device(void *context)
>  	 * TBD: _EJD support.
>  	 */
>  	status = acpi_evaluate_object(handle, "_EJ0", &arg_list, NULL);
> -	if (ACPI_FAILURE(status))
> -		printk(KERN_WARNING PREFIX
> -				"Eject device failed\n");
> +	if (ACPI_FAILURE(status)) {
> +		if (status != AE_NOT_FOUND)
> +			printk(KERN_WARNING PREFIX
> +					"Eject device failed\n");
> +		goto err_out;
> +	}
> +
> +	kfree(context);

Could please explain why this kfree is needed now. Can context be
free'ed here safely?

> +	return;
>  
> +err_out:
> +	/* Inform firmware the hot-remove operation has completed w/ error */
> +	(void) acpi_evaluate_hotplug_ost(handle,
> +				ej_event->event, ost_code, NULL);
> +	kfree(context);

Same question as above.

>  	return;
>  }
>  
> @@ -144,6 +165,7 @@ acpi_eject_store(struct device *d, struct device_attribute *attr,
>  	acpi_status status;
>  	acpi_object_type type = 0;
>  	struct acpi_device *acpi_device = to_acpi_device(d);
> +	struct acpi_eject_event *ej_event;
>  
>  	if ((!count) || (buf[0] != '1')) {
>  		return -EINVAL;
> @@ -160,7 +182,25 @@ acpi_eject_store(struct device *d, struct device_attribute *attr,
>  		goto err;
>  	}
>  
> -	acpi_os_hotplug_execute(acpi_bus_hot_remove_device, acpi_device->handle);
> +	ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
> +	if (!ej_event) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	ej_event->handle = acpi_device->handle;
> +	if (acpi_device->flags.eject_pending) {
> +		/* event originated from ACPI eject notification */
> +		ej_event->event = ACPI_NOTIFY_EJECT_REQUEST;
> +		acpi_device->flags.eject_pending = 0;
> +	} else {
> +		/* event originated from user */
> +		ej_event->event = ACPI_OST_EC_OSPM_EJECT;
> +		(void) acpi_evaluate_hotplug_ost(ej_event->handle,
> +			ej_event->event, ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
> +	}
> +
> +	acpi_os_hotplug_execute(acpi_bus_hot_remove_device, (void *)ej_event);
>  err:
>  	return ret;
>  }
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 7309113..b836800 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -151,7 +151,8 @@ struct acpi_device_flags {
>  	u32 suprise_removal_ok:1;
>  	u32 power_manageable:1;
>  	u32 performance_manageable:1;
> -	u32 reserved:24;
> +	u32 eject_pending:1;
> +	u32 reserved:23;
>  };
>  
>  /* File System */
> @@ -303,6 +304,11 @@ struct acpi_bus_event {
>  	u32 data;
>  };
>  
> +struct acpi_eject_event {
> +	acpi_handle	handle;
> +	u32		event;
> +};
> +
>  extern struct kobject *acpi_kobj;
>  extern int acpi_bus_generate_netlink_event(const char*, const char*, u8, int);
>  void acpi_bus_private_data_handler(acpi_handle, void *);
> @@ -340,6 +346,7 @@ int acpi_bus_register_driver(struct acpi_driver *driver);
>  void acpi_bus_unregister_driver(struct acpi_driver *driver);
>  int acpi_bus_add(struct acpi_device **child, struct acpi_device *parent,
>  		 acpi_handle handle, int type);
> +void acpi_bus_hot_remove_device(void *context);
>  int acpi_bus_trim(struct acpi_device *start, int rmdevice);
>  int acpi_bus_start(struct acpi_device *device);
>  acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index f421dd8..a4b1d3d 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -277,7 +277,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context);
>  #define OSC_SB_PAD_SUPPORT		1
>  #define OSC_SB_PPC_OST_SUPPORT		2
>  #define OSC_SB_PR3_SUPPORT		4
> -#define OSC_SB_CPUHP_OST_SUPPORT	8
> +#define OSC_SB_HOTPLUG_OST_SUPPORT	8
>  #define OSC_SB_APEI_SUPPORT		16
>  
>  extern bool osc_sb_apei_support_acked;
> @@ -309,6 +309,35 @@ extern bool osc_sb_apei_support_acked;
>  
>  extern acpi_status acpi_pci_osc_control_set(acpi_handle handle,
>  					     u32 *mask, u32 req);
> +
> +/* _OST Source Event Code (OSPM Action) */
> +#define ACPI_OST_EC_OSPM_SHUTDOWN		0x100
> +#define ACPI_OST_EC_OSPM_EJECT			0x103
> +#define ACPI_OST_EC_OSPM_INSERTION		0x200
> +
> +/* _OST General Processing Status Code */
> +#define ACPI_OST_SC_SUCCESS			0x0
> +#define ACPI_OST_SC_NON_SPECIFIC_FAILURE	0x1
> +#define ACPI_OST_SC_UNRECOGNIZED_NOTIFY		0x2
> +
> +/* _OST OS Shutdown Processing (0x100) Status Code */
> +#define ACPI_OST_SC_OS_SHUTDOWN_DENIED		0x80
> +#define ACPI_OST_SC_OS_SHUTDOWN_IN_PROGRESS	0x81
> +#define ACPI_OST_SC_OS_SHUTDOWN_COMPLETED	0x82
> +#define ACPI_OST_SC_OS_SHUTDOWN_NOT_SUPPORTED	0x83
> +
> +/* _OST Ejection Request (0x3, 0x103) Status Code */
> +#define ACPI_OST_SC_EJECT_NOT_SUPPORTED		0x80
> +#define ACPI_OST_SC_DEVICE_IN_USE		0x81
> +#define ACPI_OST_SC_DEVICE_BUSY			0x82
> +#define ACPI_OST_SC_EJECT_DEPENDENCY_BUSY	0x83
> +#define ACPI_OST_SC_EJECT_IN_PROGRESS		0x84
> +
> +/* _OST Insertion Request (0x200) Status Code */
> +#define ACPI_OST_SC_INSERT_IN_PROGRESS		0x80
> +#define ACPI_OST_SC_DRIVER_LOAD_FAILURE		0x81
> +#define ACPI_OST_SC_INSERT_NOT_SUPPORTED	0x82
> +
>  extern void acpi_early_init(void);
>  
>  extern int acpi_nvs_register(__u64 start, __u64 size);

Re: [PATCH v2 3/7] ACPI: Add _OST support for sysfs eject

[Toshi Kani]

On Wed, 2012-05-09 at 10:46 -0600, Shuah Khan wrote:
> On Tue, 2012-05-08 at 14:12 -0600, Toshi Kani wrote:
> > Changed acpi_bus_hot_remove_device() to support _OST. This function is
> > also changed to global so that it can be called from hotplug notify
> > handlers to perform hot-remove operation.
> > 
> > Changed acpi_eject_store(), which is the sysfs eject handler. It checks
> > eject_pending to see if the request was originated from ACPI eject
> > notification. If not, it calls _OST(0x103,84,) per Figure 6-37 in ACPI
> > 5.0 spec.
> > 
> > Added eject_pending bit to acpi_device_flags. This bit is set when the
> > kernel has received an ACPI eject notification, but does not initiate
> > its hot-remove operation by itself.
> > 
> > Added struct acpi_eject_event. This structure is used to pass extended
> > information to acpi_bus_hot_remove_device(), which has a single argument
> > to support asynchronous call.
> > 
> > Added macro definitions of _OST source events and status codes.
> > Also renamed OSC_SB_CPUHP_OST_SUPPORT to OSC_SB_HOTPLUG_OST_SUPPORT
> > since this _OSC bit is not specific to CPU hotplug. This bit is
> > defined in Table 6-147 of ACPI 5.0 as follows.
> 
> I am confused. This patch adds lot of new code that is for _OST handling
> without CONFIG_ACPI_HOTPLUG_OST check. Is this intended? Doesn't jive
> with the intent communicated in the [PATCH v2 0/7] introduction.

Yes, it is intended to minimize the use of #ifdefs in order to improve
its code readability and maintainability.  The statement in the [PATCH
v2 0/7] is correct.  When CONFIG_ACPI_HOTPLUG_OST is disabled, there is
no change in the kernel functionality nor in the OS-firmware
interaction.  


> Could you please walk though the steps on what happens with this code on
> a system that doesn't enable _OST and doesn't support _OST. That will
> help me understand how this code would behave on a system that doesn't
> support _OST.

1. CONFIG_ACPI_HOTPLUG_OST disabled
There is no change in the kernel functionality nor in the OS-firmware
interaction.  This case is same whether or not the system supports _OST.

- At boot-time, the kernel calls ACPI Operating System Capabilities
(_OSC) method, if present, with hotplug _OST bit unset. This indicates
that the OS does not support hotplug _OST.
- During a hotplug operation, the OS does not call _OST method because
acpi_evaluate_hotplug_ost() is stubbed out.

2. CONFIG_ACPI_HOTPLUG_OST enabled, but the system does not support _OST

- At boot-time, the kernel calls ACPI _OSC method, if present, with
hotplug _OST bit set. This indicates that the OS supports hotplug _OST.
Firmware ignores this bit since it does not support _OST.
- During a hotplug operation, the OS attempts to call _OST method.
Since _OST method is not present, this _OST call is a no-op
(AE_NOT_FOUND).


Thanks,
-Toshi
 

Re: [PATCH v2 3/7] ACPI: Add _OST support for sysfs eject

[Shuah Khan]

On Wed, 2012-05-09 at 12:16 -0600, Toshi Kani wrote:

> > > 
> > > Added macro definitions of _OST source events and status codes.
> > > Also renamed OSC_SB_CPUHP_OST_SUPPORT to OSC_SB_HOTPLUG_OST_SUPPORT
> > > since this _OSC bit is not specific to CPU hotplug. This bit is
> > > defined in Table 6-147 of ACPI 5.0 as follows.

Sorry. Missed that. It was in patch 7. Any reason why this feature is
split across 7 patches? Might be better to combine patches 1, 2, and 7
as it contains the infrastructure type code for _OST. Something to
consider.

There is no functional change with this patch set in the sense that _OST
doesn't get evaluated on platforms that don't support _OST, however
there is run-time change on all architectures with patches 3, 4, and 5.
There are couple of new kfree() calls introduced. Something to take a
closer to make sure it is safe in that path.

Also, what missing functionality does evaluating _OST add to the kernel?
What happens if OS continues to not evaluate _OST? It is an optional
method, looking for what is the value add?

-- Shuah

Re: [PATCH v2 3/7] ACPI: Add _OST support for sysfs eject

[Toshi Kani]

On Thu, 2012-05-10 at 09:40 -0600, Shuah Khan wrote:
> On Wed, 2012-05-09 at 12:16 -0600, Toshi Kani wrote:
> 
> > > > 
> > > > Added macro definitions of _OST source events and status codes.
> > > > Also renamed OSC_SB_CPUHP_OST_SUPPORT to OSC_SB_HOTPLUG_OST_SUPPORT
> > > > since this _OSC bit is not specific to CPU hotplug. This bit is
> > > > defined in Table 6-147 of ACPI 5.0 as follows.
> 
> Sorry. Missed that. It was in patch 7. Any reason why this feature is
> split across 7 patches? Might be better to combine patches 1, 2, and 7
> as it contains the infrastructure type code for _OST. Something to
> consider.

Bjorn suggested, which I agreed, that the OS should call _OSC with
hotplug _OST bit set when the OS is in fact capable of supporting _OST.
Hence, patch 7/7 needs to be the last patch in the patchset.


> There is no functional change with this patch set in the sense that _OST
> doesn't get evaluated on platforms that don't support _OST, however
> there is run-time change on all architectures with patches 3, 4, and 5.
> There are couple of new kfree() calls introduced. Something to take a
> closer to make sure it is safe in that path.

Yes, the changes have been verified closely as required for any code
changes.  I have also added comments to acpi_bus_hot_remove_device() to
clarify the kfree()s.


> Also, what missing functionality does evaluating _OST add to the kernel?
> What happens if OS continues to not evaluate _OST? It is an optional
> method, looking for what is the value add?

_OST is the ACPI standard method for the platform to receive the status
of hotplug operations.  Some platforms may require the OS to support
_OST in order to support ACPI hotplug operations.  For instance, if the
platform has the management console where user can request a hotplug
operation from, this _OST support would be required for the management
console to show the result of a hotplug request to user.  


Thanks,
-Toshi

Re: [PATCH v2 3/7] ACPI: Add _OST support for sysfs eject

[Shuah Khan]

On Thu, 2012-05-10 at 10:34 -0600, Toshi Kani wrote:

> _OST is the ACPI standard method for the platform to receive the status
> of hotplug operations.  Some platforms may require the OS to support
> _OST in order to support ACPI hotplug operations.  For instance, if the
> platform has the management console where user can request a hotplug
> operation from, this _OST support would be required for the management
> console to show the result of a hotplug request to user.  
> 

This will be a good information capture in the change log.

Thanks Toshi. It is interesting to have platforms require optional
methods. Guess  you can never rule out weirdness in how these Specs.
actually get implemented :)


-- Shuah

Re: [PATCH v2 3/7] ACPI: Add _OST support for sysfs eject

[Toshi Kani]

On Thu, 2012-05-10 at 10:55 -0600, Shuah Khan wrote:
> On Thu, 2012-05-10 at 10:34 -0600, Toshi Kani wrote:
> 
> > _OST is the ACPI standard method for the platform to receive the status
> > of hotplug operations.  Some platforms may require the OS to support
> > _OST in order to support ACPI hotplug operations.  For instance, if the
> > platform has the management console where user can request a hotplug
> > operation from, this _OST support would be required for the management
> > console to show the result of a hotplug request to user.  
> > 
> 
> This will be a good information capture in the change log.

Sounds good.  I will add this information to patch 0/1 and 1/1.


> Thanks Toshi. It is interesting to have platforms require optional
> methods. Guess  you can never rule out weirdness in how these Specs.
> actually get implemented :)

It is optional for platforms to implement it.  But when some platforms
start implementing it, it is necessary for the OS to support it. :) 


Thanks,
-Toshi

[PATCH v2 4/7] ACPI: Add _OST support for ACPI CPU hotplug

[Toshi Kani]

Changed acpi_processor_hotplug_notify() to call ACPI _OST method
when ACPI CPU hotplug operation has completed.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/acpi/processor_driver.c |   28 ++++++++++++++++++++--------
 1 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 0734086..971c454 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -701,9 +701,9 @@ static void acpi_processor_hotplug_notify(acpi_handle handle,
 {
 	struct acpi_processor *pr;
 	struct acpi_device *device = NULL;
+	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE; /* default */
 	int result;
 
-
 	switch (event) {
 	case ACPI_NOTIFY_BUS_CHECK:
 	case ACPI_NOTIFY_DEVICE_CHECK:
@@ -715,14 +715,18 @@ static void acpi_processor_hotplug_notify(acpi_handle handle,
 		if (!is_processor_present(handle))
 			break;
 
-		if (acpi_bus_get_device(handle, &device)) {
-			result = acpi_processor_device_add(handle, &device);
-			if (result)
-				printk(KERN_ERR PREFIX
-					    "Unable to add the device\n");
+		if (!acpi_bus_get_device(handle, &device))
+			break;
+
+		result = acpi_processor_device_add(handle, &device);
+		if (result) {
+			printk(KERN_ERR PREFIX "Unable to add the device\n");
 			break;
 		}
+
+		ost_code = ACPI_OST_SC_SUCCESS;
 		break;
+
 	case ACPI_NOTIFY_EJECT_REQUEST:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 				  "received ACPI_NOTIFY_EJECT_REQUEST\n"));
@@ -736,15 +740,23 @@ static void acpi_processor_hotplug_notify(acpi_handle handle,
 		if (!pr) {
 			printk(KERN_ERR PREFIX
 				    "Driver data is NULL, dropping EJECT\n");
-			return;
+			break;
 		}
+
+		/* REVISIT: update when eject is supported */
+		ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
 		break;
+
 	default:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 				  "Unsupported event [0x%x]\n", event));
-		break;
+
+		/* non-hotplug event; possibly handled by other handler */
+		return;
 	}
 
+	/* Inform firmware that the hotplug operation has completed */
+	(void) acpi_evaluate_hotplug_ost(handle, event, ost_code, NULL);
 	return;
 }
 
-- 
1.7.7.6

[PATCH v2 5/7] ACPI: Add _OST support for ACPI memory hotplug

[Toshi Kani]

Changed acpi_memory_device_notify() to call ACPI _OST method
when ACPI memory hotplug operation has completed.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/acpi/acpi_memhotplug.c |   43 +++++++++++++++++++++++++++++----------
 1 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index d985713..24c807f 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -341,7 +341,7 @@ static void acpi_memory_device_notify(acpi_handle handle, u32 event, void *data)
 {
 	struct acpi_memory_device *mem_device;
 	struct acpi_device *device;
-
+	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE; /* default */
 
 	switch (event) {
 	case ACPI_NOTIFY_BUS_CHECK:
@@ -354,15 +354,20 @@ static void acpi_memory_device_notify(acpi_handle handle, u32 event, void *data)
 					  "\nReceived DEVICE CHECK notification for device\n"));
 		if (acpi_memory_get_device(handle, &mem_device)) {
 			printk(KERN_ERR PREFIX "Cannot find driver data\n");
-			return;
+			break;
 		}
 
-		if (!acpi_memory_check_device(mem_device)) {
-			if (acpi_memory_enable_device(mem_device))
-				printk(KERN_ERR PREFIX
-					    "Cannot enable memory device\n");
+		if (acpi_memory_check_device(mem_device))
+			break;
+
+		if (acpi_memory_enable_device(mem_device)) {
+			printk(KERN_ERR PREFIX "Cannot enable memory device\n");
+			break;
 		}
+
+		ost_code = ACPI_OST_SC_SUCCESS;
 		break;
+
 	case ACPI_NOTIFY_EJECT_REQUEST:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 				  "\nReceived EJECT REQUEST notification for device\n"));
@@ -383,19 +388,35 @@ static void acpi_memory_device_notify(acpi_handle handle, u32 event, void *data)
 		 * TBD: Can also be disabled by Callback registration
 		 *      with generic sysfs driver
 		 */
-		if (acpi_memory_disable_device(mem_device))
-			printk(KERN_ERR PREFIX
-				    "Disable memory device\n");
+		if (acpi_memory_disable_device(mem_device)) {
+			printk(KERN_ERR PREFIX "Disable memory device\n");
+			/*
+			 * If _EJ0 was called but failed, _OST is not
+			 * necessary.
+			 */
+			if (mem_device->state == MEMORY_INVALID_STATE)
+				return;
+
+			break;
+		}
+
 		/*
 		 * TBD: Invoke acpi_bus_remove to cleanup data structures
 		 */
-		break;
+
+		/* _EJ0 succeeded; _OST is not necessary */
+		return;
+
 	default:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 				  "Unsupported event [0x%x]\n", event));
-		break;
+
+		/* non-hotplug event; possibly handled by other handler */
+		return;
 	}
 
+	/* Inform firmware that the hotplug operation has completed */
+	(void) acpi_evaluate_hotplug_ost(handle, event, ost_code, NULL);
 	return;
 }
 
-- 
1.7.7.6

[PATCH v2 6/7] ACPI: Add _OST support for ACPI container hotplug

[Toshi Kani]

Changed container_notify_cb() to call ACPI _OST method when ACPI
container hotplug operation has completed. Slightly restructured
the code with the same logic. The function sets eject_pending bit
for an eject request since it does not initiate hot-remove operation.
This bit is checked by the sysfs eject handler to determine if the
request is originated from an ACPI eject notification.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/acpi/container.c |   43 ++++++++++++++++++++++++++++---------------
 1 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
index 45cd03b..1f9f7d7 100644
--- a/drivers/acpi/container.c
+++ b/drivers/acpi/container.c
@@ -158,9 +158,7 @@ static void container_notify_cb(acpi_handle handle, u32 type, void *context)
 	int result;
 	int present;
 	acpi_status status;
-
-
-	present = is_device_present(handle);
+	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE; /* default */
 
 	switch (type) {
 	case ACPI_NOTIFY_BUS_CHECK:
@@ -169,32 +167,47 @@ static void container_notify_cb(acpi_handle handle, u32 type, void *context)
 		printk(KERN_WARNING "Container driver received %s event\n",
 		       (type == ACPI_NOTIFY_BUS_CHECK) ?
 		       "ACPI_NOTIFY_BUS_CHECK" : "ACPI_NOTIFY_DEVICE_CHECK");
+
+		present = is_device_present(handle);
 		status = acpi_bus_get_device(handle, &device);
-		if (present) {
-			if (ACPI_FAILURE(status) || !device) {
-				result = container_device_add(&device, handle);
-				if (!result)
-					kobject_uevent(&device->dev.kobj,
-						       KOBJ_ONLINE);
-				else
-					printk(KERN_WARNING
-					       "Failed to add container\n");
-			}
-		} else {
+		if (!present) {
 			if (ACPI_SUCCESS(status)) {
 				/* device exist and this is a remove request */
+				device->flags.eject_pending = 1;
 				kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
+				return;
 			}
+			break;
+		}
+
+		if (!ACPI_FAILURE(status) || device)
+			break;
+
+		result = container_device_add(&device, handle);
+		if (result) {
+			printk(KERN_WARNING "Failed to add container\n");
+			break;
 		}
+
+		kobject_uevent(&device->dev.kobj, KOBJ_ONLINE);
+		ost_code = ACPI_OST_SC_SUCCESS;
 		break;
+
 	case ACPI_NOTIFY_EJECT_REQUEST:
 		if (!acpi_bus_get_device(handle, &device) && device) {
+			device->flags.eject_pending = 1;
 			kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
+			return;
 		}
 		break;
+
 	default:
-		break;
+		/* non-hotplug event; possibly handled by other handler */
+		return;
 	}
+
+	/* Inform firmware that the hotplug operation has completed */
+	(void) acpi_evaluate_hotplug_ost(handle, type, ost_code, NULL);
 	return;
 }
 
-- 
1.7.7.6

[PATCH v2 7/7] ACPI: Set hotplug _OST support bit to _OSC

[Toshi Kani]

When CONFIG_ACPI_HOTPLUG_OST is enabled, set hotplug _OST support bit
OSC_SB_HOTPLUG_OST_SUPPORT to indicate that the OS supports hotplug
_OST by calling the platform-wide ACPI Operating System Capabilities
(_OSC).

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/acpi/bus.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 3263b68..93bf515 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -544,6 +544,10 @@ static void acpi_bus_osc_support(void)
 	capbuf[OSC_SUPPORT_TYPE] |= OSC_SB_PPC_OST_SUPPORT;
 #endif
 
+#ifdef CONFIG_ACPI_HOTPLUG_OST
+	capbuf[OSC_SUPPORT_TYPE] |= OSC_SB_HOTPLUG_OST_SUPPORT;
+#endif
+
 	if (!ghes_disable)
 		capbuf[OSC_SUPPORT_TYPE] |= OSC_SB_APEI_SUPPORT;
 	if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
-- 
1.7.7.6

Re: [PATCH v2 0/7] ACPI: Add _OST support for ACPI hotplug

[Shuah Khan]

On Tue, 2012-05-08 at 14:12 -0600, Toshi Kani wrote:
> This patchset supports ACPI OSPM Status Indication (_OST) method for
> ACPI CPU/memory/container hotplug operations and sysfs eject. After
> ACPI hotplug operation has completed, OSPM calls _OST to indicate the
> status of the operation to the platform. If _OST is not present, this
> patchset has no effect on the platform.
> 
> This _OST support can be enabled or disabled with a new config option
> CONFIG_ACPI_HOTPLUG_OST. This option is disabled by default. When
> this option is disabled, this patchset has no effect on the platform.
> 
> The _OST definition can be found in section 6.3.5 of ACPI 5.0 spec.
> The HPPF spec below also describes hotplug flows with _OST.
> 
>   DIG64 Hot-Plug & Partitioning Flow (HPPF) Specification R1.0
>   http://www.dig64.org/home/DIG64_HPPF_R1_0.pdf
> 
> The changes have been tested with simulated _OST methods.
> 
> v2:
>  - Added CONFIG_ACPI_HOTPLUG_OST option. 
>  - Added _OST support for container hotplug and sysfs eject.
>  - Reordered patchset to enable _OST support bit of _OSC in the 
>    last patch.
> 
> ---
> Toshi Kani (7):
>  ACPI: Add CONFIG_HOTPLUG_OST option
>  ACPI: Add an interface to evaluate _OST
>  ACPI: Add _OST support for sysfs eject
>  ACPI: Add _OST support for ACPI CPU hotplug
>  ACPI: Add _OST support for ACPI memory hotplug
>  ACPI: Add _OST support for ACPI container hotplug
>  ACPI: Set hotplug _OST support bit to _OSC
> 

Did you look into implementing this maybe in a generic way by adding a
new device_ops that adds the ability to return status back to firmware?
It can be left null on systems that don't implement _OST. This way it
might be cleaner and can extend to other means of returning status back
to firmware if any.

Adding _OST support the way it is done in this patch set is rather
invasive and CONFIG_HOTPLUG_OST only helps disable just the evaluation
of _OST method. Other code changes do get executed on all platforms.

-- Shuah

Re: [PATCH v2 0/7] ACPI: Add _OST support for ACPI hotplug

[Toshi Kani]

On Wed, 2012-05-09 at 11:36 -0600, Shuah Khan wrote:
> On Tue, 2012-05-08 at 14:12 -0600, Toshi Kani wrote:
> > This patchset supports ACPI OSPM Status Indication (_OST) method for
> > ACPI CPU/memory/container hotplug operations and sysfs eject. After
> > ACPI hotplug operation has completed, OSPM calls _OST to indicate the
> > status of the operation to the platform. If _OST is not present, this
> > patchset has no effect on the platform.
> > 
> > This _OST support can be enabled or disabled with a new config option
> > CONFIG_ACPI_HOTPLUG_OST. This option is disabled by default. When
> > this option is disabled, this patchset has no effect on the platform.
> > 
> > The _OST definition can be found in section 6.3.5 of ACPI 5.0 spec.
> > The HPPF spec below also describes hotplug flows with _OST.
> > 
> >   DIG64 Hot-Plug & Partitioning Flow (HPPF) Specification R1.0
> >   http://www.dig64.org/home/DIG64_HPPF_R1_0.pdf
> > 
> > The changes have been tested with simulated _OST methods.
> > 
> > v2:
> >  - Added CONFIG_ACPI_HOTPLUG_OST option. 
> >  - Added _OST support for container hotplug and sysfs eject.
> >  - Reordered patchset to enable _OST support bit of _OSC in the 
> >    last patch.
> > 
> > ---
> > Toshi Kani (7):
> >  ACPI: Add CONFIG_HOTPLUG_OST option
> >  ACPI: Add an interface to evaluate _OST
> >  ACPI: Add _OST support for sysfs eject
> >  ACPI: Add _OST support for ACPI CPU hotplug
> >  ACPI: Add _OST support for ACPI memory hotplug
> >  ACPI: Add _OST support for ACPI container hotplug
> >  ACPI: Set hotplug _OST support bit to _OSC
> > 
> 
> Did you look into implementing this maybe in a generic way by adding a
> new device_ops that adds the ability to return status back to firmware?
> It can be left null on systems that don't implement _OST. This way it
> might be cleaner and can extend to other means of returning status back
> to firmware if any.

As Bjorn pointed out in his review comments, if the OS indicates hotplug
_OST support to _OSC, it needs to evaluate _OST for any devices.
Therefore, we should not provide a way for each device to have different
ways of supporting _OST.  This patchset is designed to provide
consistent behavior among multiple devices.


> Adding _OST support the way it is done in this patch set is rather
> invasive and CONFIG_HOTPLUG_OST only helps disable just the evaluation
> of _OST method. Other code changes do get executed on all platforms.

This _OST config option, as it is defined, enables or disables _OST
support.  The _OST code changes are limited to the scope of hotplug
operation code paths, which can also be enabled / disabled with their
HOTPLUG config options.


Thanks,
-Toshi