ACPI patchese on test branch

ACPI patchese on test branch

[Len Brown]

These patches are currently on the acpi test branch.
If they survive, they'll get queued for 2.6.29.

If you see any problems with them, or feel that
there is a case to pull any forward to 2.6.28,
please let me know.

thanks,
-Len

[PATCH 01/10] ACPI: pci_link: remove acpi_irq_balance_set() interface

[Len Brown]

From: Bjorn Helgaas <bjorn.helgaas@hp.com>

This removes the acpi_irq_balance_set() interface from the PCI
interrupt link driver.

x86 used acpi_irq_balance_set() to tell the PCI interrupt link
driver to configure links to minimize IRQ sharing.  But the link
driver can easily figure out whether to turn on IRQ balancing
based on the IRQ model (PIC/IOAPIC/etc), so we can get rid of
that external interface.

It's better for the driver to figure this out at init-time.  If
we set it externally via the x86 code, the interface reduces
modularity, and we depend on the fact that acpi_process_madt()
happens before we process the kernel command line.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 arch/x86/include/asm/acpi.h |    1 -
 arch/x86/kernel/acpi/boot.c |    1 -
 drivers/acpi/pci_link.c     |   11 +++++++++--
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index 8d676d8..9830681 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
@@ -113,7 +113,6 @@ static inline void acpi_disable_pci(void)
 	acpi_pci_disabled = 1;
 	acpi_noirq_set();
 }
-extern int acpi_irq_balance_set(char *str);
 
 /* routines for saving/restoring kernel state */
 extern int acpi_save_state_mem(void);
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 8c1f76a..4c51a2f 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1343,7 +1343,6 @@ static void __init acpi_process_madt(void)
 			error = acpi_parse_madt_ioapic_entries();
 			if (!error) {
 				acpi_irq_model = ACPI_IRQ_MODEL_IOAPIC;
-				acpi_irq_balance_set(NULL);
 				acpi_ioapic = 1;
 
 				smp_found_config = 1;
diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index fcfdef7..e52ad91 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -531,7 +531,7 @@ int __init acpi_irq_penalty_init(void)
 	return 0;
 }
 
-static int acpi_irq_balance;	/* 0: static, 1: balance */
+static int acpi_irq_balance = -1;	/* 0: static, 1: balance */
 
 static int acpi_pci_link_allocate(struct acpi_pci_link *link)
 {
@@ -950,10 +950,17 @@ device_initcall(irqrouter_init_sysfs);
 
 static int __init acpi_pci_link_init(void)
 {
-
 	if (acpi_noirq)
 		return 0;
 
+	if (acpi_irq_balance == -1) {
+		/* no command line switch: enable balancing in IOAPIC mode */
+		if (acpi_irq_model == ACPI_IRQ_MODEL_IOAPIC)
+			acpi_irq_balance = 1;
+		else
+			acpi_irq_balance = 0;
+	}
+
 	acpi_link.count = 0;
 	INIT_LIST_HEAD(&acpi_link.entries);
 
-- 
1.5.6.5

[PATCH 02/10] ACPI: Disambiguate processor declaration type

[Len Brown]

From: Myron Stowe <myron.stowe@hp.com>

Declaring processors in ACPI namespace can be done using either a
"Processor" definition or a "Device" definition (see section 8.4 -
Declaring Processors; "Advanced Configuration and Power Interface
Specification", Revision 3.0b).  Currently the two processor
declaration types are conflated.

This patch disambiguates the processor declaration's definition type
enabling subsequent code to behave uniquely based explicitly on the
declaration's type.

Signed-off-by: Myron Stowe <myron.stowe@hp.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/processor_core.c |    1 +
 drivers/acpi/scan.c           |    2 +-
 include/acpi/acpi_drivers.h   |    1 +
 3 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 24a362f..0c670dd 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -89,6 +89,7 @@ static int acpi_processor_handle_eject(struct acpi_processor *pr);
 
 
 static const struct acpi_device_id processor_device_ids[] = {
+	{ACPI_PROCESSOR_OBJECT_HID, 0},
 	{ACPI_PROCESSOR_HID, 0},
 	{"", 0},
 };
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index a9dda8e..3fb6e2d 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1043,7 +1043,7 @@ static void acpi_device_set_id(struct acpi_device *device,
 		hid = ACPI_POWER_HID;
 		break;
 	case ACPI_BUS_TYPE_PROCESSOR:
-		hid = ACPI_PROCESSOR_HID;
+		hid = ACPI_PROCESSOR_OBJECT_HID;
 		break;
 	case ACPI_BUS_TYPE_SYSTEM:
 		hid = ACPI_SYSTEM_HID;
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index cf04c60..7469ff3 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -41,6 +41,7 @@
  */
 
 #define ACPI_POWER_HID			"LNXPOWER"
+#define ACPI_PROCESSOR_OBJECT_HID	"ACPI_CPU"
 #define ACPI_PROCESSOR_HID		"ACPI0007"
 #define ACPI_SYSTEM_HID			"LNXSYSTM"
 #define ACPI_THERMAL_HID		"LNXTHERM"
-- 
1.5.6.5

[PATCH 03/10] ACPI: Behave uniquely based on processor declaration definition type

[Len Brown]

From: Myron Stowe <myron.stowe@hp.com>

Associating a Local SAPIC with a processor object is dependent upon the
processor object's definition type.  CPUs declared as "Processor" should
use the Local SAPIC's 'processor_id', and CPUs declared as "Device"
should use the 'uid'.  Note that for "Processor" declarations, even if a
'_UID' child object exists, it has no bearing with respect to mapping
Local SAPICs (see section 5.2.11.13 - Local SAPIC Structure; "Advanced
Configuration and Power Interface Specification", Revision 3.0b).

This patch changes the lsapic mapping logic to rely on the distinction of
how the processor object was declared - the mapping can't just try both
types of matches regardless of declaration type and rely on one failing
as is currently being done.

Signed-off-by: Myron Stowe <myron.stowe@hp.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/processor_core.c |   78 +++++++++++++++++++++++------------------
 1 files changed, 44 insertions(+), 34 deletions(-)

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 0c670dd..bc332fc 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -410,7 +410,7 @@ static int acpi_processor_remove_fs(struct acpi_device *device)
 /* Use the acpiid in MADT to map cpus in case of SMP */
 
 #ifndef CONFIG_SMP
-static int get_cpu_id(acpi_handle handle, u32 acpi_id) {return -1;}
+static int get_cpu_id(acpi_handle handle, int type, u32 acpi_id) { return -1; }
 #else
 
 static struct acpi_table_madt *madt;
@@ -429,27 +429,35 @@ static int map_lapic_id(struct acpi_subtable_header *entry,
 }
 
 static int map_lsapic_id(struct acpi_subtable_header *entry,
-		  u32 acpi_id, int *apic_id)
+		int device_declaration, u32 acpi_id, int *apic_id)
 {
 	struct acpi_madt_local_sapic *lsapic =
 		(struct acpi_madt_local_sapic *)entry;
+	u32 tmp = (lsapic->id << 8) | lsapic->eid;
+
 	/* Only check enabled APICs*/
-	if (lsapic->lapic_flags & ACPI_MADT_ENABLED) {
-		/* First check against id */
-		if (lsapic->processor_id == acpi_id) {
-			*apic_id = (lsapic->id << 8) | lsapic->eid;
-			return 1;
-		/* Check against optional uid */
-		} else if (entry->length >= 16 &&
-			lsapic->uid == acpi_id) {
-			*apic_id = lsapic->uid;
-			return 1;
-		}
-	}
+	if (!(lsapic->lapic_flags & ACPI_MADT_ENABLED))
+		return 0;
+
+	/* Device statement declaration type */
+	if (device_declaration) {
+		if (entry->length < 16)
+			printk(KERN_ERR PREFIX
+			    "Invalid LSAPIC with Device type processor (SAPIC ID %#x)\n",
+			    tmp);
+		else if (lsapic->uid == acpi_id)
+			goto found;
+	/* Processor statement declaration type */
+	} else if (lsapic->processor_id == acpi_id)
+		goto found;
+
 	return 0;
+found:
+	*apic_id = tmp;
+	return 1;
 }
 
-static int map_madt_entry(u32 acpi_id)
+static int map_madt_entry(int type, u32 acpi_id)
 {
 	unsigned long madt_end, entry;
 	int apic_id = -1;
@@ -470,7 +478,7 @@ static int map_madt_entry(u32 acpi_id)
 			if (map_lapic_id(header, acpi_id, &apic_id))
 				break;
 		} else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) {
-			if (map_lsapic_id(header, acpi_id, &apic_id))
+			if (map_lsapic_id(header, type, acpi_id, &apic_id))
 				break;
 		}
 		entry += header->length;
@@ -478,7 +486,7 @@ static int map_madt_entry(u32 acpi_id)
 	return apic_id;
 }
 
-static int map_mat_entry(acpi_handle handle, u32 acpi_id)
+static int map_mat_entry(acpi_handle handle, int type, u32 acpi_id)
 {
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *obj;
@@ -501,7 +509,7 @@ static int map_mat_entry(acpi_handle handle, u32 acpi_id)
 	if (header->type == ACPI_MADT_TYPE_LOCAL_APIC) {
 		map_lapic_id(header, acpi_id, &apic_id);
 	} else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) {
-		map_lsapic_id(header, acpi_id, &apic_id);
+		map_lsapic_id(header, type, acpi_id, &apic_id);
 	}
 
 exit:
@@ -510,14 +518,14 @@ exit:
 	return apic_id;
 }
 
-static int get_cpu_id(acpi_handle handle, u32 acpi_id)
+static int get_cpu_id(acpi_handle handle, int type, u32 acpi_id)
 {
 	int i;
 	int apic_id = -1;
 
-	apic_id = map_mat_entry(handle, acpi_id);
+	apic_id = map_mat_entry(handle, type, acpi_id);
 	if (apic_id == -1)
-		apic_id = map_madt_entry(acpi_id);
+		apic_id = map_madt_entry(type, acpi_id);
 	if (apic_id == -1)
 		return apic_id;
 
@@ -533,15 +541,16 @@ static int get_cpu_id(acpi_handle handle, u32 acpi_id)
                                  Driver Interface
    -------------------------------------------------------------------------- */
 
-static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)
+static int acpi_processor_get_info(struct acpi_device *device)
 {
 	acpi_status status = 0;
 	union acpi_object object = { 0 };
 	struct acpi_buffer buffer = { sizeof(union acpi_object), &object };
-	int cpu_index;
+	struct acpi_processor *pr;
+	int cpu_index, device_declaration = 0;
 	static int cpu0_initialized;
 
-
+	pr = acpi_driver_data(device);
 	if (!pr)
 		return -EINVAL;
 
@@ -562,22 +571,23 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 				  "No bus mastering arbitration control\n"));
 
-	/* Check if it is a Device with HID and UID */
-	if (has_uid) {
+	if (!strcmp(acpi_device_hid(device), ACPI_PROCESSOR_HID)) {
+		/*
+		 * Declared with "Device" statement; match _UID.
+		 * Note that we don't handle string _UIDs yet.
+		 */
 		unsigned long long value;
 		status = acpi_evaluate_integer(pr->handle, METHOD_NAME__UID,
 						NULL, &value);
 		if (ACPI_FAILURE(status)) {
-			printk(KERN_ERR PREFIX "Evaluating processor _UID\n");
+			printk(KERN_ERR PREFIX
+			    "Evaluating processor _UID [%#x]\n", status);
 			return -ENODEV;
 		}
+		device_declaration = 1;
 		pr->acpi_id = value;
 	} else {
-		/*
-		* Evalute the processor object.  Note that it is common on SMP to
-		* have the first (boot) processor with a valid PBLK address while
-		* all others have a NULL address.
-		*/
+		/* Declared with "Processor" statement; match ProcessorID */
 		status = acpi_evaluate_object(pr->handle, NULL, NULL, &buffer);
 		if (ACPI_FAILURE(status)) {
 			printk(KERN_ERR PREFIX "Evaluating processor object\n");
@@ -590,7 +600,7 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)
 		*/
 		pr->acpi_id = object.processor.proc_id;
 	}
-	cpu_index = get_cpu_id(pr->handle, pr->acpi_id);
+	cpu_index = get_cpu_id(pr->handle, device_declaration, pr->acpi_id);
 
 	/* Handle UP system running SMP kernel, with no LAPIC in MADT */
 	if (!cpu0_initialized && (cpu_index == -1) &&
@@ -662,7 +672,7 @@ static int __cpuinit acpi_processor_start(struct acpi_device *device)
 
 	pr = acpi_driver_data(device);
 
-	result = acpi_processor_get_info(pr, device->flags.unique_id);
+	result = acpi_processor_get_info(device);
 	if (result) {
 		/* Processor is physically not present */
 		return 0;
-- 
1.5.6.5

[PATCH 04/10] ACPI: 80 column adherence and spelling fix (no functional change)

[Len Brown]

From: Myron Stowe <myron.stowe@hp.com>

Signed-off-by: Myron Stowe <myron.stowe@hp.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/processor_core.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index bc332fc..b57b1f0 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -595,9 +595,10 @@ static int acpi_processor_get_info(struct acpi_device *device)
 		}
 
 		/*
-		* TBD: Synch processor ID (via LAPIC/LSAPIC structures) on SMP.
-		*      >>> 'acpi_get_processor_id(acpi_id, &id)' in arch/xxx/acpi.c
-		*/
+		 * TBD: Synch processor ID (via LAPIC/LSAPIC structures) on SMP.
+		 *      >>> 'acpi_get_processor_id(acpi_id, &id)' in
+		 *      arch/xxx/acpi.c
+		 */
 		pr->acpi_id = object.processor.proc_id;
 	}
 	cpu_index = get_cpu_id(pr->handle, device_declaration, pr->acpi_id);
-- 
1.5.6.5

[PATCH 05/10] Hibernate: Call platform_begin before swsusp_shrink_memory

[Len Brown]

From: Zhang Rui <rui.zhang@intel.com>

Call platform_begin() before swsusp_shrink_memory() so that we can
always allocate enough memory to save the ACPI NVS region from
platform_begin().

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Acked-by: Nigel Cunningham <nigel@tuxonice.net>
Acked-by: Pavel Machek <pavel@suse.cz>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 kernel/power/disk.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/power/disk.c b/kernel/power/disk.c
index c9d7408..096fe48 100644
--- a/kernel/power/disk.c
+++ b/kernel/power/disk.c
@@ -259,12 +259,12 @@ int hibernation_snapshot(int platform_mode)
 {
 	int error, ftrace_save;
 
-	/* Free memory before shutting down devices. */
-	error = swsusp_shrink_memory();
+	error = platform_begin(platform_mode);
 	if (error)
 		return error;
 
-	error = platform_begin(platform_mode);
+	/* Free memory before shutting down devices. */
+	error = swsusp_shrink_memory();
 	if (error)
 		goto Close;
 
-- 
1.5.6.5

[PATCH 06/10] ACPI hibernate: Add a mechanism to save/restore ACPI NVS memory

[Len Brown]

From: Rafael J. Wysocki <rjw@sisk.pl>

According to the ACPI Specification 3.0b, Section 15.3.2,
"OSPM will call the _PTS control method some time before entering a
sleeping state, to allow the platform's AML code to update this
memory image before entering the sleeping state. After the system
awakes from an S4 state, OSPM will restore this memory area and call
the _WAK control method to enable the BIOS to reclaim its memory
image."  For this reason, implement a mechanism allowing us to save
the NVS memory during hibernation and to restore it during the
subsequent resume.

Based on a patch by Zhang Rui.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Nigel Cunningham <nigel@tuxonice.net>
Cc: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/sleep/main.c |   53 +++++++++++++++++---
 include/linux/suspend.h   |   13 +++++
 kernel/power/swsusp.c     |  122 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 180 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c
index 80c0868..f861b73 100644
--- a/drivers/acpi/sleep/main.c
+++ b/drivers/acpi/sleep/main.c
@@ -356,9 +356,25 @@ void __init acpi_no_s4_hw_signature(void)
 
 static int acpi_hibernation_begin(void)
 {
-	acpi_target_sleep_state = ACPI_STATE_S4;
-	acpi_sleep_tts_switch(acpi_target_sleep_state);
-	return 0;
+	int error;
+
+	error = hibernate_nvs_alloc();
+	if (!error) {
+		acpi_target_sleep_state = ACPI_STATE_S4;
+		acpi_sleep_tts_switch(acpi_target_sleep_state);
+	}
+
+	return error;
+}
+
+static int acpi_hibernation_pre_snapshot(void)
+{
+	int error = acpi_pm_prepare();
+
+	if (!error)
+		hibernate_nvs_save();
+
+	return error;
 }
 
 static int acpi_hibernation_enter(void)
@@ -379,6 +395,12 @@ static int acpi_hibernation_enter(void)
 	return ACPI_SUCCESS(status) ? 0 : -EFAULT;
 }
 
+static void acpi_hibernation_finish(void)
+{
+	hibernate_nvs_free();
+	acpi_pm_finish();
+}
+
 static void acpi_hibernation_leave(void)
 {
 	/*
@@ -394,6 +416,8 @@ static void acpi_hibernation_leave(void)
 			"cannot resume!\n");
 		panic("ACPI S4 hardware signature mismatch");
 	}
+	/* Restore the NVS memory area */
+	hibernate_nvs_restore();
 }
 
 static void acpi_pm_enable_gpes(void)
@@ -404,8 +428,8 @@ static void acpi_pm_enable_gpes(void)
 static struct platform_hibernation_ops acpi_hibernation_ops = {
 	.begin = acpi_hibernation_begin,
 	.end = acpi_pm_end,
-	.pre_snapshot = acpi_pm_prepare,
-	.finish = acpi_pm_finish,
+	.pre_snapshot = acpi_hibernation_pre_snapshot,
+	.finish = acpi_hibernation_finish,
 	.prepare = acpi_pm_prepare,
 	.enter = acpi_hibernation_enter,
 	.leave = acpi_hibernation_leave,
@@ -431,8 +455,21 @@ static int acpi_hibernation_begin_old(void)
 
 	error = acpi_sleep_prepare(ACPI_STATE_S4);
 
+	if (!error) {
+		error = hibernate_nvs_alloc();
+		if (!error)
+			acpi_target_sleep_state = ACPI_STATE_S4;
+	}
+	return error;
+}
+
+static int acpi_hibernation_pre_snapshot_old(void)
+{
+	int error = acpi_pm_disable_gpes();
+
 	if (!error)
-		acpi_target_sleep_state = ACPI_STATE_S4;
+		hibernate_nvs_save();
+
 	return error;
 }
 
@@ -443,8 +480,8 @@ static int acpi_hibernation_begin_old(void)
 static struct platform_hibernation_ops acpi_hibernation_ops_old = {
 	.begin = acpi_hibernation_begin_old,
 	.end = acpi_pm_end,
-	.pre_snapshot = acpi_pm_disable_gpes,
-	.finish = acpi_pm_finish,
+	.pre_snapshot = acpi_hibernation_pre_snapshot_old,
+	.finish = acpi_hibernation_finish,
 	.prepare = acpi_pm_disable_gpes,
 	.enter = acpi_hibernation_enter,
 	.leave = acpi_hibernation_leave,
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 2ce8207..2b409c4 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -232,6 +232,11 @@ extern unsigned long get_safe_page(gfp_t gfp_mask);
 
 extern void hibernation_set_ops(struct platform_hibernation_ops *ops);
 extern int hibernate(void);
+extern int hibernate_nvs_register(unsigned long start, unsigned long size);
+extern int hibernate_nvs_alloc(void);
+extern void hibernate_nvs_free(void);
+extern void hibernate_nvs_save(void);
+extern void hibernate_nvs_restore(void);
 #else /* CONFIG_HIBERNATION */
 static inline int swsusp_page_is_forbidden(struct page *p) { return 0; }
 static inline void swsusp_set_page_free(struct page *p) {}
@@ -239,6 +244,14 @@ static inline void swsusp_unset_page_free(struct page *p) {}
 
 static inline void hibernation_set_ops(struct platform_hibernation_ops *ops) {}
 static inline int hibernate(void) { return -ENOSYS; }
+static inline int hibernate_nvs_register(unsigned long a, unsigned long b)
+{
+	return 0;
+}
+static inline int hibernate_nvs_alloc(void) { return 0; }
+static inline void hibernate_nvs_free(void) {}
+static inline void hibernate_nvs_save(void) {}
+static inline void hibernate_nvs_restore(void) {}
 #endif /* CONFIG_HIBERNATION */
 
 #ifdef CONFIG_PM_SLEEP
diff --git a/kernel/power/swsusp.c b/kernel/power/swsusp.c
index 023ff2a..a92c914 100644
--- a/kernel/power/swsusp.c
+++ b/kernel/power/swsusp.c
@@ -262,3 +262,125 @@ int swsusp_shrink_memory(void)
 
 	return 0;
 }
+
+/*
+ * Platforms, like ACPI, may want us to save some memory used by them during
+ * hibernation and to restore the contents of this memory during the subsequent
+ * resume.  The code below implements a mechanism allowing us to do that.
+ */
+
+struct nvs_page {
+	unsigned long phys_start;
+	unsigned int size;
+	void *kaddr;
+	void *data;
+	struct list_head node;
+};
+
+static LIST_HEAD(nvs_list);
+
+/**
+ *	hibernate_nvs_register - register platform NVS memory region to save
+ *	@start - physical address of the region
+ *	@size - size of the region
+ *
+ *	The NVS region need not be page-aligned (both ends) and we arrange
+ *	things so that the data from page-aligned addresses in this region will
+ *	be copied into separate RAM pages.
+ */
+int hibernate_nvs_register(unsigned long start, unsigned long size)
+{
+	struct nvs_page *entry, *next;
+
+	while (size > 0) {
+		unsigned int nr_bytes;
+
+		entry = kzalloc(sizeof(struct nvs_page), GFP_KERNEL);
+		if (!entry)
+			goto Error;
+
+		list_add_tail(&entry->node, &nvs_list);
+		entry->phys_start = start;
+		nr_bytes = PAGE_SIZE - (start & ~PAGE_MASK);
+		entry->size = (size < nr_bytes) ? size : nr_bytes;
+
+		start += entry->size;
+		size -= entry->size;
+	}
+	return 0;
+
+ Error:
+	list_for_each_entry_safe(entry, next, &nvs_list, node) {
+		list_del(&entry->node);
+		kfree(entry);
+	}
+	return -ENOMEM;
+}
+
+/**
+ *	hibernate_nvs_free - free data pages allocated for saving NVS regions
+ */
+void hibernate_nvs_free(void)
+{
+	struct nvs_page *entry;
+
+	list_for_each_entry(entry, &nvs_list, node)
+		if (entry->data) {
+			free_page((unsigned long)entry->data);
+			entry->data = NULL;
+			if (entry->kaddr) {
+				iounmap(entry->kaddr);
+				entry->kaddr = NULL;
+			}
+		}
+}
+
+/**
+ *	hibernate_nvs_alloc - allocate memory necessary for saving NVS regions
+ */
+int hibernate_nvs_alloc(void)
+{
+	struct nvs_page *entry;
+
+	list_for_each_entry(entry, &nvs_list, node) {
+		entry->data = (void *)__get_free_page(GFP_KERNEL);
+		if (!entry->data) {
+			hibernate_nvs_free();
+			return -ENOMEM;
+		}
+	}
+	return 0;
+}
+
+/**
+ *	hibernate_nvs_save - save NVS memory regions
+ */
+void hibernate_nvs_save(void)
+{
+	struct nvs_page *entry;
+
+	printk(KERN_INFO "PM: Saving platform NVS memory\n");
+
+	list_for_each_entry(entry, &nvs_list, node)
+		if (entry->data) {
+			entry->kaddr = ioremap(entry->phys_start, entry->size);
+			memcpy(entry->data, entry->kaddr, entry->size);
+		}
+}
+
+/**
+ *	hibernate_nvs_restore - restore NVS memory regions
+ *
+ *	This function is going to be called with interrupts disabled, so it
+ *	cannot iounmap the virtual addresses used to access the NVS region.
+ */
+void hibernate_nvs_restore(void)
+{
+	struct nvs_page *entry;
+
+	printk(KERN_INFO "PM: Restoring platform NVS memory\n");
+
+	list_for_each_entry(entry, &nvs_list, node)
+		if (entry->data)
+			memcpy(entry->kaddr, entry->data, entry->size);
+}
-- 
1.5.6.5

[PATCH 07/10] x86 hibernate: Mark ACPI NVS memory region at startup

[Len Brown]

From: Rafael J. Wysocki <rjw@sisk.pl>

Introduce new initcall for marking the ACPI NVS memory at startup, so
that it can be saved/restored during hibernation/resume.

Based on a patch by Zhang Rui.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 arch/x86/kernel/e820.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 7aafeb5..74c6a21 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -665,6 +665,27 @@ void __init e820_mark_nosave_regions(unsigned long limit_pfn)
 }
 #endif
 
+#ifdef CONFIG_HIBERNATION
+/**
+ * Mark ACPI NVS memory region, so that we can save/restore it during
+ * hibernation and the subsequent resume.
+ */
+static int __init e820_mark_nvs_memory(void)
+{
+	int i;
+
+	for (i = 0; i < e820.nr_map; i++) {
+		struct e820entry *ei = &e820.map[i];
+
+		if (ei->type == E820_NVS)
+			hibernate_nvs_register(ei->addr, ei->size);
+	}
+
+	return 0;
+}
+core_initcall(e820_mark_nvs_memory);
+#endif
+
 /*
  * Early reserved memory areas.
  */
-- 
1.5.6.5

[PATCH 08/10] ACPI hibernate: Introduce new kernel parameter acpi_sleep=s4_nonvs

[Len Brown]

From: Rafael J. Wysocki <rjw@sisk.pl>

On some machines it may be necessary to disable the saving/restoring
of the ACPI NVS memory region during hibernation/resume.  For this
purpose, introduce new ACPI kernel command line option
acpi_sleep=s4_nonvs.

Based on a patch by Zhang Rui.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Nigel Cunningham <nigel@tuxonice.net>
Acked-by: Pavel Machek <pavel@suse.cz>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 Documentation/kernel-parameters.txt |    5 ++++-
 arch/x86/kernel/acpi/sleep.c        |    2 ++
 drivers/acpi/sleep/main.c           |   19 +++++++++++++++++--
 include/linux/acpi.h                |    1 +
 4 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 1bbcaa8..65bc2bc 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -149,7 +149,8 @@ and is between 256 and 4096 characters. It is defined in the file
 			default: 0
 
 	acpi_sleep=	[HW,ACPI] Sleep options
-			Format: { s3_bios, s3_mode, s3_beep, s4_nohwsig, old_ordering }
+			Format: { s3_bios, s3_mode, s3_beep, s4_nohwsig,
+				  old_ordering, s4_nonvs }
 			See Documentation/power/video.txt for s3_bios and s3_mode.
 			s3_beep is for debugging; it makes the PC's speaker beep
 			as soon as the kernel's real-mode entry point is called.
@@ -159,6 +160,8 @@ and is between 256 and 4096 characters. It is defined in the file
 			control method, wrt putting devices into low power
 			states, to be enforced (the ACPI 2.0 ordering of _PTS is
 			used by default).
+			s4_nonvs prevents the kernel from saving/restoring the
+			ACPI NVS memory during hibernation.
 
 	acpi_sci=	[HW,ACPI] ACPI System Control Interrupt trigger mode
 			Format: { level | edge | high | low }
diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index 806b4e9..707c1f6 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -159,6 +159,8 @@ static int __init acpi_sleep_setup(char *str)
 #endif
 		if (strncmp(str, "old_ordering", 12) == 0)
 			acpi_old_suspend_ordering();
+		if (strncmp(str, "s4_nonvs", 8) == 0)
+			acpi_s4_no_nvs();
 		str = strchr(str, ',');
 		if (str != NULL)
 			str += strspn(str, ", \t");
diff --git a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c
index f861b73..63bff78 100644
--- a/drivers/acpi/sleep/main.c
+++ b/drivers/acpi/sleep/main.c
@@ -90,6 +90,20 @@ void __init acpi_old_suspend_ordering(void)
 	old_suspend_ordering = true;
 }
 
+/*
+ * The ACPI specification wants us to save NVS memory regions during hibernation
+ * and to restore them during the subsequent resume.  However, it is not certain
+ * if this mechanism is going to work on all machines, so we allow the user to
+ * disable this mechanism using the 'acpi_sleep=s4_nonvs' kernel command line
+ * option.
+ */
+static bool s4_no_nvs;
+
+void __init acpi_s4_no_nvs(void)
+{
+	s4_no_nvs = true;
+}
+
 /**
  *	acpi_pm_disable_gpes - Disable the GPEs.
  */
@@ -358,7 +372,7 @@ static int acpi_hibernation_begin(void)
 {
 	int error;
 
-	error = hibernate_nvs_alloc();
+	error = s4_no_nvs ? 0 : hibernate_nvs_alloc();
 	if (!error) {
 		acpi_target_sleep_state = ACPI_STATE_S4;
 		acpi_sleep_tts_switch(acpi_target_sleep_state);
@@ -456,7 +470,8 @@ static int acpi_hibernation_begin_old(void)
 	error = acpi_sleep_prepare(ACPI_STATE_S4);
 
 	if (!error) {
-		error = hibernate_nvs_alloc();
+		if (!s4_no_nvs)
+			error = hibernate_nvs_alloc();
 		if (!error)
 			acpi_target_sleep_state = ACPI_STATE_S4;
 	}
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index fd6a452..4008f61 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -230,6 +230,7 @@ int acpi_check_mem_region(resource_size_t start, resource_size_t n,
 #ifdef CONFIG_PM_SLEEP
 void __init acpi_no_s4_hw_signature(void);
 void __init acpi_old_suspend_ordering(void);
+void __init acpi_s4_no_nvs(void);
 #endif /* CONFIG_PM_SLEEP */
 #else	/* CONFIG_ACPI */
 
-- 
1.5.6.5

[PATCH 09/10] compal-laptop: use rfkill switch subsystem

[Len Brown]

From: Cezary Jackiewicz <cezary.jackiewicz@gmail.com>

Remove unnecessary attributes, use rfkill switch subsystem.

It depends on the rfkill changes in net-next-2.6.

[randy.dunlap@oracle.com: compal-laptop: depends on RKFILL]
Signed-off-by: Cezary Jackiewicz <cezary.jackiewicz@gmail.com>
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/misc/Kconfig         |    2 +
 drivers/misc/compal-laptop.c |  270 +++++++++++++++++++-----------------------
 2 files changed, 124 insertions(+), 148 deletions(-)

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 9494400..fbfd86d 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -232,6 +232,7 @@ config MSI_LAPTOP
         depends on X86
         depends on ACPI_EC
         depends on BACKLIGHT_CLASS_DEVICE
+	depends on RFKILL
         ---help---
 	  This is a driver for laptops built by MSI (MICRO-STAR
 	  INTERNATIONAL):
@@ -262,6 +263,7 @@ config COMPAL_LAPTOP
 	depends on X86
 	depends on ACPI_EC
 	depends on BACKLIGHT_CLASS_DEVICE
+	depends on RFKILL
 	---help---
 	  This is a driver for laptops built by Compal:
 
diff --git a/drivers/misc/compal-laptop.c b/drivers/misc/compal-laptop.c
index 344b790..ae886cb 100644
--- a/drivers/misc/compal-laptop.c
+++ b/drivers/misc/compal-laptop.c
@@ -24,19 +24,10 @@
  */
 
 /*
- * comapl-laptop.c - Compal laptop support.
+ * compal-laptop.c - Compal laptop support.
  *
- * This driver exports a few files in /sys/devices/platform/compal-laptop/:
- *
- *   wlan - wlan subsystem state: contains 0 or 1 (rw)
- *
- *   bluetooth - Bluetooth subsystem state: contains 0 or 1 (rw)
- *
- *   raw - raw value taken from embedded controller register (ro)
- *
- * In addition to these platform device attributes the driver
- * registers itself in the Linux backlight control subsystem and is
- * available to userspace under /sys/class/backlight/compal-laptop/.
+ * This driver registers itself in the Linux backlight control subsystem
+ * and rfkill switch subsystem.
  *
  * This driver might work on other laptops produced by Compal. If you
  * want to try it you can pass force=1 as argument to the module which
@@ -52,8 +43,12 @@
 #include <linux/backlight.h>
 #include <linux/platform_device.h>
 #include <linux/autoconf.h>
+#include <linux/rfkill.h>
 
-#define COMPAL_DRIVER_VERSION "0.2.6"
+#define COMPAL_DRIVER_VERSION "0.3.0"
+#define COMPAL_DRIVER_NAME "compal-laptop"
+#define COMPAL_ERR KERN_ERR COMPAL_DRIVER_NAME ": "
+#define COMPAL_INFO KERN_INFO COMPAL_DRIVER_NAME ": "
 
 #define COMPAL_LCD_LEVEL_MAX 8
 
@@ -64,6 +59,10 @@
 #define WLAN_MASK	0x01
 #define BT_MASK 	0x02
 
+/* rfkill switches */
+static struct rfkill *bluetooth_rfkill;
+static struct rfkill *wlan_rfkill;
+
 static int force;
 module_param(force, bool, 0);
 MODULE_PARM_DESC(force, "Force driver load, ignore DMI data");
@@ -89,67 +88,6 @@ static int get_lcd_level(void)
 	return (int) result;
 }
 
-static int set_wlan_state(int state)
-{
-	u8 result, value;
-
-	ec_read(COMPAL_EC_COMMAND_WIRELESS, &result);
-
-	if ((result & KILLSWITCH_MASK) == 0)
-		return -EINVAL;
-	else {
-		if (state)
-			value = (u8) (result | WLAN_MASK);
-		else
-			value = (u8) (result & ~WLAN_MASK);
-		ec_write(COMPAL_EC_COMMAND_WIRELESS, value);
-	}
-
-	return 0;
-}
-
-static int set_bluetooth_state(int state)
-{
-	u8 result, value;
-
-	ec_read(COMPAL_EC_COMMAND_WIRELESS, &result);
-
-	if ((result & KILLSWITCH_MASK) == 0)
-		return -EINVAL;
-	else {
-		if (state)
-			value = (u8) (result | BT_MASK);
-		else
-			value = (u8) (result & ~BT_MASK);
-		ec_write(COMPAL_EC_COMMAND_WIRELESS, value);
-	}
-
-	return 0;
-}
-
-static int get_wireless_state(int *wlan, int *bluetooth)
-{
-	u8 result;
-
-	ec_read(COMPAL_EC_COMMAND_WIRELESS, &result);
-
-	if (wlan) {
-		if ((result & KILLSWITCH_MASK) == 0)
-			*wlan = 0;
-		else
-			*wlan = result & WLAN_MASK;
-	}
-
-	if (bluetooth) {
-		if ((result & KILLSWITCH_MASK) == 0)
-			*bluetooth = 0;
-		else
-			*bluetooth = (result & BT_MASK) >> 1;
-	}
-
-	return 0;
-}
-
 /* Backlight device stuff */
 
 static int bl_get_brightness(struct backlight_device *b)
@@ -172,99 +110,121 @@ static struct backlight_device *compalbl_device;
 
 /* Platform device */
 
-static ssize_t show_wlan(struct device *dev,
-	struct device_attribute *attr, char *buf)
-{
-	int ret, enabled;
+static struct platform_driver compal_driver = {
+	.driver = {
+		.name = COMPAL_DRIVER_NAME,
+		.owner = THIS_MODULE,
+	}
+};
 
-	ret = get_wireless_state(&enabled, NULL);
-	if (ret < 0)
-		return ret;
+static struct platform_device *compal_device;
 
-	return sprintf(buf, "%i\n", enabled);
-}
+/* rfkill stuff */
 
-static ssize_t show_raw(struct device *dev,
-	struct device_attribute *attr, char *buf)
+static int wlan_rfk_set(void *data, enum rfkill_state state)
 {
-	u8 result;
+	u8 result, value;
 
 	ec_read(COMPAL_EC_COMMAND_WIRELESS, &result);
 
-	return sprintf(buf, "%i\n", result);
+	if ((result & KILLSWITCH_MASK) == 0)
+		return 0;
+	else {
+		if (state == RFKILL_STATE_ON)
+			value = (u8) (result | WLAN_MASK);
+		else
+			value = (u8) (result & ~WLAN_MASK);
+		ec_write(COMPAL_EC_COMMAND_WIRELESS, value);
+	}
+
+	return 0;
 }
 
-static ssize_t show_bluetooth(struct device *dev,
-	struct device_attribute *attr, char *buf)
+static int wlan_rfk_get(void *data, enum rfkill_state *state)
 {
-	int ret, enabled;
+	u8 result;
 
-	ret = get_wireless_state(NULL, &enabled);
-	if (ret < 0)
-		return ret;
+	ec_read(COMPAL_EC_COMMAND_WIRELESS, &result);
 
-	return sprintf(buf, "%i\n", enabled);
+	if ((result & KILLSWITCH_MASK) == 0)
+		*state = RFKILL_STATE_OFF;
+	else
+		*state = result & WLAN_MASK;
+
+	return 0;
 }
 
-static ssize_t store_wlan_state(struct device *dev,
-	struct device_attribute *attr, const char *buf, size_t count)
+static int bluetooth_rfk_set(void *data, enum rfkill_state state)
 {
-	int state, ret;
+	u8 result, value;
 
-	if (sscanf(buf, "%i", &state) != 1 || (state < 0 || state > 1))
-		return -EINVAL;
+	ec_read(COMPAL_EC_COMMAND_WIRELESS, &result);
 
-	ret = set_wlan_state(state);
-	if (ret < 0)
-		return ret;
+	if ((result & KILLSWITCH_MASK) == 0)
+		return 0;
+	else {
+		if (state == RFKILL_STATE_ON)
+			value = (u8) (result | BT_MASK);
+		else
+			value = (u8) (result & ~BT_MASK);
+		ec_write(COMPAL_EC_COMMAND_WIRELESS, value);
+	}
 
-	return count;
+	return 0;
 }
 
-static ssize_t store_bluetooth_state(struct device *dev,
-	struct device_attribute *attr, const char *buf, size_t count)
+static int bluetooth_rfk_get(void *data, enum rfkill_state *state)
 {
-	int state, ret;
+	u8 result;
 
-	if (sscanf(buf, "%i", &state) != 1 || (state < 0 || state > 1))
-		return -EINVAL;
+	ec_read(COMPAL_EC_COMMAND_WIRELESS, &result);
 
-	ret = set_bluetooth_state(state);
-	if (ret < 0)
-		return ret;
+	if ((result & KILLSWITCH_MASK) == 0)
+		*state = RFKILL_STATE_OFF;
+	else
+		*state = (result & BT_MASK) >> 1;
 
-	return count;
+	return 0;
 }
 
-static DEVICE_ATTR(bluetooth, 0644, show_bluetooth, store_bluetooth_state);
-static DEVICE_ATTR(wlan, 0644, show_wlan, store_wlan_state);
-static DEVICE_ATTR(raw, 0444, show_raw, NULL);
-
-static struct attribute *compal_attributes[] = {
-	&dev_attr_bluetooth.attr,
-	&dev_attr_wlan.attr,
-	&dev_attr_raw.attr,
-	NULL
-};
+static int __init compal_rfkill(struct rfkill **rfk,
+				const enum rfkill_type rfktype,
+				const char *name,
+				int (*toggle_radio)(void *, enum rfkill_state),
+				int (*get_state)(void *, enum rfkill_state *))
+{
+	int res;
 
-static struct attribute_group compal_attribute_group = {
-	.attrs = compal_attributes
-};
+	(*rfk) = rfkill_allocate(&compal_device->dev, rfktype);
+	if (!*rfk) {
+		printk(COMPAL_ERR
+				"failed to allocate memory for rfkill class\n");
+		return -ENOMEM;
+	}
 
-static struct platform_driver compal_driver = {
-	.driver = {
-		.name = "compal-laptop",
-		.owner = THIS_MODULE,
+	(*rfk)->name = name;
+	(*rfk)->get_state = get_state;
+	(*rfk)->toggle_radio = toggle_radio;
+	(*rfk)->user_claim_unsupported = 1;
+
+	res = rfkill_register(*rfk);
+	if (res < 0) {
+		printk(COMPAL_ERR
+				"failed to register %s rfkill switch: %d\n",
+				name, res);
+		rfkill_free(*rfk);
+		*rfk = NULL;
+		return res;
 	}
-};
 
-static struct platform_device *compal_device;
+	return 0;
+}
 
 /* Initialization */
 
 static int dmi_check_cb(const struct dmi_system_id *id)
 {
-	printk(KERN_INFO "compal-laptop: Identified laptop model '%s'.\n",
+	printk(COMPAL_INFO "Identified laptop model '%s'.\n",
 		id->ident);
 
 	return 0;
@@ -326,12 +286,13 @@ static int __init compal_init(void)
 
 	/* Register backlight stuff */
 
-	compalbl_device = backlight_device_register("compal-laptop", NULL, NULL,
-						&compalbl_ops);
+	compalbl_device = backlight_device_register(COMPAL_DRIVER_NAME, NULL,
+						NULL, &compalbl_ops);
 	if (IS_ERR(compalbl_device))
 		return PTR_ERR(compalbl_device);
 
 	compalbl_device->props.max_brightness = COMPAL_LCD_LEVEL_MAX-1;
+	compalbl_device->props.brightness = get_lcd_level();
 
 	ret = platform_driver_register(&compal_driver);
 	if (ret)
@@ -339,7 +300,7 @@ static int __init compal_init(void)
 
 	/* Register platform stuff */
 
-	compal_device = platform_device_alloc("compal-laptop", -1);
+	compal_device = platform_device_alloc(COMPAL_DRIVER_NAME, -1);
 	if (!compal_device) {
 		ret = -ENOMEM;
 		goto fail_platform_driver;
@@ -347,23 +308,28 @@ static int __init compal_init(void)
 
 	ret = platform_device_add(compal_device);
 	if (ret)
-		goto fail_platform_device1;
+		goto fail_platform_device;
 
-	ret = sysfs_create_group(&compal_device->dev.kobj,
-		&compal_attribute_group);
-	if (ret)
-		goto fail_platform_device2;
+	/* Register rfkill stuff */
 
-	printk(KERN_INFO "compal-laptop: driver "COMPAL_DRIVER_VERSION
-		" successfully loaded.\n");
+	compal_rfkill(&wlan_rfkill,
+					RFKILL_TYPE_WLAN,
+					"compal_laptop_wlan_sw",
+					wlan_rfk_set,
+					wlan_rfk_get);
 
-	return 0;
+	compal_rfkill(&bluetooth_rfkill,
+					RFKILL_TYPE_BLUETOOTH,
+					"compal_laptop_bluetooth_sw",
+					bluetooth_rfk_set,
+					bluetooth_rfk_get);
 
-fail_platform_device2:
+	printk(COMPAL_INFO "driver "COMPAL_DRIVER_VERSION
+			" successfully loaded.\n");
 
-	platform_device_del(compal_device);
+	return 0;
 
-fail_platform_device1:
+fail_platform_device:
 
 	platform_device_put(compal_device);
 
@@ -380,13 +346,21 @@ fail_backlight:
 
 static void __exit compal_cleanup(void)
 {
+	if (bluetooth_rfkill) {
+		rfkill_unregister(bluetooth_rfkill);
+		bluetooth_rfkill = NULL;
+	}
+
+	if (wlan_rfkill) {
+		rfkill_unregister(wlan_rfkill);
+		wlan_rfkill = NULL;
+	}
 
-	sysfs_remove_group(&compal_device->dev.kobj, &compal_attribute_group);
 	platform_device_unregister(compal_device);
 	platform_driver_unregister(&compal_driver);
 	backlight_device_unregister(compalbl_device);
 
-	printk(KERN_INFO "compal-laptop: driver unloaded.\n");
+	printk(COMPAL_INFO "driver unloaded.\n");
 }
 
 module_init(compal_init);
-- 
1.5.6.5

[PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Len Brown]

From: Len Brown <len.brown@intel.com>

We've run into systems which do not reboot properly
without using the ACPI reset mechanism.  So lets
try this in linux-next for a while and see
how many existing machines stop rebooting
because they can't handle ACPI reboot.

Signed-off-by: Len Brown <len.brown@intel.com>
---
 arch/x86/kernel/reboot.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 724adfc..f72f71c 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -29,7 +29,7 @@ EXPORT_SYMBOL(pm_power_off);
 
 static const struct desc_ptr no_idt = {};
 static int reboot_mode;
-enum reboot_type reboot_type = BOOT_KBD;
+enum reboot_type reboot_type = BOOT_ACPI;
 int reboot_force;
 
 #if defined(CONFIG_X86_32) && defined(CONFIG_SMP)
-- 
1.5.6.5

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Ingo Molnar]

(I've Cc:-ed the folks who worked on this problem area.)

* Len Brown <lenb@kernel.org> wrote:

> From: Len Brown <len.brown@intel.com>
> 
> We've run into systems which do not reboot properly
> without using the ACPI reset mechanism.  So lets
> try this in linux-next for a while and see
> how many existing machines stop rebooting
> because they can't handle ACPI reboot.
> 
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
>  arch/x86/kernel/reboot.c |    2 +-

> -enum reboot_type reboot_type = BOOT_KBD;
> +enum reboot_type reboot_type = BOOT_ACPI;

NAK.

Your point that the set of systems where KBD-reboot is broken is 
larger (and growing) than the set of systems where ACPI-reboot is 
broken (which set probably has a constant size) is true.

But still this change caused non-trivial regressions in v28 _worse_ 
than the inability to reboot cleanly, so this patch does not fly as-is 
as we reverted it upstream for a good reason.

| commit 8d00450d296dedec9ada38d43b83e79cca6fd5a3
| Author: Eduardo Habkost <ehabkost@redhat.com>
| Date:   Tue Nov 4 12:52:44 2008 -0200
|
|     Revert "x86: default to reboot via ACPI"

I still dont oppose ACPI-reboot on new systems, but please add a more 
intelligent and regression-free method of turning it on by default: a 
flag year ACPI-date of 2008/2007/2006 or something like that, and 
default the reboot mode to KBD reboot before that date.

Such a solution would have the desired characteristics: we'd default 
to ACPI-reboot on all new systems, but we'd default to KBD-reboot on 
all older systems. The set of systems where KBD-reboot is broken will 
thus be limited, and we have the chance to eventually DMI-map them 
all.

Also, note that the main reason why we wanted ACPI-reboot originally 
(Avi's patch) was to get the KVM-is-active reboots right. But the 
KVM/VMX reboot state will be much more standard in v2.6.29 (we'll exit 
VMX mode on all CPUs/cores), so that particular problem category is 
eliminated.

Also, the default reboot mode is an x86 patch so this should not be 
within the ACPI tree, please submit it to us.

Hm?

	Ingo

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Len Brown]

> > -enum reboot_type reboot_type = BOOT_KBD;
> > +enum reboot_type reboot_type = BOOT_ACPI;

> NAK.
> 
> Your point that the set of systems where KBD-reboot is broken is 
> larger (and growing) than the set of systems where ACPI-reboot is 
> broken (which set probably has a constant size) is true.
> 
> But still this change caused non-trivial regressions in v28 _worse_ 
> than the inability to reboot cleanly, so this patch does not fly as-is 
> as we reverted it upstream for a good reason.

If you can direct me to the failures that ACPI reset caused
in 28, that would be great.  Obviously they are not on
the linux-acpi archives because the x86 tree enabled
ACPI reset and then disabled it w/o telling us...

thanks,
-Len

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 917 bytes --]

On Saturday 08 November 2008, Len Brown wrote:
> 
> > > -enum reboot_type reboot_type = BOOT_KBD;
> > > +enum reboot_type reboot_type = BOOT_ACPI;
> 
> > NAK.
> > 
> > Your point that the set of systems where KBD-reboot is broken is 
> > larger (and growing) than the set of systems where ACPI-reboot is 
> > broken (which set probably has a constant size) is true.
> > 
> > But still this change caused non-trivial regressions in v28 _worse_ 
> > than the inability to reboot cleanly, so this patch does not fly as-is 
> > as we reverted it upstream for a good reason.
> 
> If you can direct me to the failures that ACPI reset caused
> in 28, that would be great. 

http://marc.info/?l=linux-kernel&m=122547719810921&w=2

> Obviously they are not on 
> the linux-acpi archives because the x86 tree enabled
> ACPI reset and then disabled it w/o telling us...
> 
> thanks,
> -Len
> 
> 



[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Len Brown]

> > If you can direct me to the failures that ACPI reset caused
> > in 28, that would be great. 
> 
> http://marc.info/?l=linux-kernel&m=122547719810921&w=2

Andrey,
Thanks for the quick reply.

Hmm, google on Toshiba Portege 4000 suggests it shipped with Windows 2000,
does it have a windows 2000 or a Windows XP sticker on it?

Please open up a bug report
http://bugzilla.kernel.org/enter_bug.cgi?product=ACPI
and attach the dmesg, plus the output from acpidump and dmidecode.

thanks,
-Len

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 888 bytes --]

On Saturday 08 November 2008, Len Brown wrote:
> 
> > > If you can direct me to the failures that ACPI reset caused
> > > in 28, that would be great. 
> > 
> > http://marc.info/?l=linux-kernel&m=122547719810921&w=2
> 
> Andrey,
> Thanks for the quick reply.
> 
> Hmm, google on Toshiba Portege 4000 suggests it shipped with Windows 2000,
> does it have a windows 2000 or a Windows XP sticker on it?
> 

Designed for Microsoft® Windows® 2000 Profesional (I am surprised sticker
still sticks after all thee years :)

> Please open up a bug report
> http://bugzilla.kernel.org/enter_bug.cgi?product=ACPI
> and attach the dmesg, plus the output from acpidump and dmidecode.
> 

there is already bug report to track this regression; I added files there
but I am not able to change product (bug was opened by Rafael):

http://bugzilla.kernel.org/show_bug.cgi?id=11895

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Ingo Molnar]

* Andrey Borzenkov <arvidjaar@mail.ru> wrote:

> > Please open up a bug report
> > http://bugzilla.kernel.org/enter_bug.cgi?product=ACPI
> > and attach the dmesg, plus the output from acpidump and dmidecode.
> > 
> 
> there is already bug report to track this regression; I added files there
> but I am not able to change product (bug was opened by Rafael):
> 
> http://bugzilla.kernel.org/show_bug.cgi?id=11895

Len, please consider my Lenovo T60 laptop "possibly affected" too. I 
was seeing weird sporadic reboot hangs which went away roughly since 
around that revert.

So the negative scope of the change, even after such short amount of 
testing, is non-trivial, and we simply have to go via a flag day date 
approach. Or, if these bugs are debuggable, make the ACPI reboot 
sequence more reliable.

	Ingo

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Rafael J. Wysocki]

On Saturday, 8 of November 2008, Len Brown wrote:
> 
> > > -enum reboot_type reboot_type = BOOT_KBD;
> > > +enum reboot_type reboot_type = BOOT_ACPI;
> 
> > NAK.
> > 
> > Your point that the set of systems where KBD-reboot is broken is 
> > larger (and growing) than the set of systems where ACPI-reboot is 
> > broken (which set probably has a constant size) is true.
> > 
> > But still this change caused non-trivial regressions in v28 _worse_ 
> > than the inability to reboot cleanly, so this patch does not fly as-is 
> > as we reverted it upstream for a good reason.
> 
> If you can direct me to the failures that ACPI reset caused
> in 28, that would be great.  Obviously they are not on
> the linux-acpi archives because the x86 tree enabled
> ACPI reset and then disabled it w/o telling us...

I think I have one machine with this problem at the Uni (it's an NForce4-based
desktop with Athlon64 X2).  I won't be there before Wednesday, though.

Thanks,
Rafael

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Avi Kivity]

Ingo Molnar wrote:
> Len, please consider my Lenovo T60 laptop "possibly affected" too. I 
> was seeing weird sporadic reboot hangs which went away roughly since 
> around that revert.
>
>   

Given that Windows uses ACPI reboot, I find it unlikely that it is so 
unreliable.  Maybe some other problem in the tree got fixed?

> So the negative scope of the change, even after such short amount of 
> testing, is non-trivial, and we simply have to go via a flag day date 
> approach. Or, if these bugs are debuggable, make the ACPI reboot 
> sequence more reliable.

I think the sequence should be acpi -> kbd -> triple fault.  Given that 
Windows uses ACPI, the number of machines that support it is much larger 
(and growing daily) than the number of machines that do not.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[H. Peter Anvin]

Avi Kivity wrote:
> 
> I think the sequence should be acpi -> kbd -> triple fault.  Given that
> Windows uses ACPI, the number of machines that support it is much larger
> (and growing daily) than the number of machines that do not.
> 

Like with many other things ACPI, there probably should be an ACPI date
cutoff for using it by default.  There is also port CF9 reboot (often
incorrectly described as "PCI reboot", but it has nothing to do with the
PCI standard.)

	-hpa

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Ingo Molnar]

* H. Peter Anvin <hpa@zytor.com> wrote:

> Avi Kivity wrote:
> > 
> > I think the sequence should be acpi -> kbd -> triple fault.  Given that
> > Windows uses ACPI, the number of machines that support it is much larger
> > (and growing daily) than the number of machines that do not.
> > 
> 
> Like with many other things ACPI, there probably should be an ACPI 
> date cutoff for using it by default.  There is also port CF9 reboot 
> (often incorrectly described as "PCI reboot", but it has nothing to 
> do with the PCI standard.)

so, the sequence should be:

 [ acpi if date > 2007 ] -> kbd -> triple fault

Where in this sequence should we insert port-CF9 reboot? We have no 
discovery of it, etc. The KGDB reboot will do _something_ on most 
boxes, so inserting it like this:

 [ acpi	if date	> 2007 ] -> kbd -> port-CF9 -> triple fault

... will likely have no practical impact as we rarely get to the 
triple fault method to begin with. So the reboot chain we'd like to 
have is:

 [ acpi	if date	> 2007 ] -> safe-port-CF9 -> kbd -> triple fault

... where safe-port-CF9 is something that can be done safely on all 
x86 boxes.

Anyway, safe-port-CF9 aside, the ACPI sequence should definitely be 
cutoff based, so the plain re-introduction of the patch that changes 
the default is not acceptable.

	Ingo

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Avi Kivity]

Ingo Molnar wrote:
> * H. Peter Anvin <hpa@zytor.com> wrote:
>
>   
>> Avi Kivity wrote:
>>     
>>> I think the sequence should be acpi -> kbd -> triple fault.  Given that
>>> Windows uses ACPI, the number of machines that support it is much larger
>>> (and growing daily) than the number of machines that do not.
>>>
>>>       
>> Like with many other things ACPI, there probably should be an ACPI 
>> date cutoff for using it by default.  There is also port CF9 reboot 
>> (often incorrectly described as "PCI reboot", but it has nothing to 
>> do with the PCI standard.)
>>     
>
> so, the sequence should be:
>
>  [ acpi if date > 2007 ] -> kbd -> triple fault
>
>   

2007?  Maybe 2002, a year after Windows XP was launched?

Windows XP uses ACPI by default.  Not sure about reboot, but I wouldn't 
be surprised if it did, since it's such a simple feature, not involving 
AML etc.

> Where in this sequence should we insert port-CF9 reboot? We have no 
> discovery of it, etc. The KGDB reboot will do _something_ on most 
> boxes, so inserting it like this:
>
>  [ acpi	if date	> 2007 ] -> kbd -> port-CF9 -> triple fault
>   

Most likely ACPI uses port CF9 if it's available.

> ... will likely have no practical impact as we rarely get to the 
> triple fault method to begin with. So the reboot chain we'd like to 
> have is:
>
>  [ acpi	if date	> 2007 ] -> safe-port-CF9 -> kbd -> triple fault
>
> ... where safe-port-CF9 is something that can be done safely on all 
> x86 boxes.
>
> Anyway, safe-port-CF9 aside, the ACPI sequence should definitely be 
> cutoff based, so the plain re-introduction of the patch that changes 
> the default is not acceptable.
>   

What the vmx issues showed us is that keyboard reset is unreliable on 
some machines, so reset was actually done by triple-fault, which doesn't 
work well when vmx is enabled (if it's connected to INIT; note it won't 
reset peripherals in that case).

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Ingo Molnar]

* Avi Kivity <avi@redhat.com> wrote:

> Ingo Molnar wrote:
>> * H. Peter Anvin <hpa@zytor.com> wrote:
>>
>>   
>>> Avi Kivity wrote:
>>>     
>>>> I think the sequence should be acpi -> kbd -> triple fault.  Given that
>>>> Windows uses ACPI, the number of machines that support it is much larger
>>>> (and growing daily) than the number of machines that do not.
>>>>
>>>>       
>>> Like with many other things ACPI, there probably should be an ACPI  
>>> date cutoff for using it by default.  There is also port CF9 reboot  
>>> (often incorrectly described as "PCI reboot", but it has nothing to  
>>> do with the PCI standard.)
>>>     
>>
>> so, the sequence should be:
>>
>>  [ acpi if date > 2007 ] -> kbd -> triple fault
>>
>>   
>
> 2007?  Maybe 2002, a year after Windows XP was launched?

the _first_ flag year should generally be close to the current status 
quo - otherwise we risk breaking a lot more boxes in the interim. 
Then, once the whole approach has proven out to work fine for new 
boxes, can we lower the flag year.

there's no need to argue about this much. We had our chance with ACPI 
reboot, it didnt work, now we simply _have_ to be careful about it. No 
ifs and when.

>> Anyway, safe-port-CF9 aside, the ACPI sequence should definitely be 
>> cutoff based, so the plain re-introduction of the patch that 
>> changes the default is not acceptable.
>
> What the vmx issues showed us is that keyboard reset is unreliable 
> on some machines, so reset was actually done by triple-fault, which 
> doesn't work well when vmx is enabled (if it's connected to INIT; 
> note it won't reset peripherals in that case).

well then we could insert CF9 to before the triple fault, and solve 
some of the problems as well, without unnecessary risks.

This is a separate patch from ACPI reboot itself, naturally.

	Ingo

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Matthew Garrett]

On Mon, Nov 10, 2008 at 09:39:38AM +0100, Ingo Molnar wrote:

> so, the sequence should be:
> 
>  [ acpi if date > 2007 ] -> kbd -> triple fault

No. We should find out which versions of Windows use the ACPI boot 
mechanism and then

[ acpi if firmware indicates Vista support ] -> whatever

or something. Microsoft appear to have moved away from using date 
cutoffs for anything other than whether or not to enable ACPI in the 
first place, and we ought to attempt compatibility with them.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Matthew Garrett]

On Mon, Nov 10, 2008 at 10:54:01AM +0200, Avi Kivity wrote:

> Windows XP uses ACPI by default.  Not sure about reboot, but I wouldn't 
> be surprised if it did, since it's such a simple feature, not involving 
> AML etc.

2000 uses ACPI by default under various circumstances, but it'd be 
better to actually test whether this specific part of the spec is used. 
Microsoft's support for various ACPI features has been spotty.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Ingo Molnar]

* Matthew Garrett <mjg@redhat.com> wrote:

> On Mon, Nov 10, 2008 at 09:39:38AM +0100, Ingo Molnar wrote:
> 
> > so, the sequence should be:
> > 
> >  [ acpi if date > 2007 ] -> kbd -> triple fault
> 
> No. We should find out which versions of Windows use the ACPI boot 
> mechanism and then
> 
> [ acpi if firmware indicates Vista support ] -> whatever
> 
> or something. Microsoft appear to have moved away from using date 
> cutoffs for anything other than whether or not to enable ACPI in the 
> first place, and we ought to attempt compatibility with them.

okay, that's fine to me too. My main point is that we need something 
nuanced this time around (be it a string check or a cutoff) - not the 
"enable again" patch that i saw in the ACPI tree and which i had to 
NAK.

	Ingo

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Matthew Garrett]

On Mon, Nov 10, 2008 at 01:56:30PM +0100, Ingo Molnar wrote:
> * Matthew Garrett <mjg@redhat.com> wrote:
> > or something. Microsoft appear to have moved away from using date 
> > cutoffs for anything other than whether or not to enable ACPI in the 
> > first place, and we ought to attempt compatibility with them.
> 
> okay, that's fine to me too. My main point is that we need something 
> nuanced this time around (be it a string check or a cutoff) - not the 
> "enable again" patch that i saw in the ACPI tree and which i had to 
> NAK.

All we need now is confirmation as to which versions of Windows use this 
behaviour.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[H. Peter Anvin]

Ingo Molnar wrote:
> 
> well then we could insert CF9 to before the triple fault, and solve 
> some of the problems as well, without unnecessary risks.
> 
> This is a separate patch from ACPI reboot itself, naturally.
> 

This is a pretty good point.  There is extremely low risk of being
something at port CF9 that is something other than the reset register.
It may not be there, but if it isn't, the worst thing that happened is
we confused a device right before reset.

Even better if we condition the port CF9 write on existence a PCI bus
(or even more specifically PCI Configuration Method #1 or #2).  Port CF9
is extremely unlikely to exist on a machine without a PCI bus, and
extremely unlikely to conflict with anything else on a machine with a
PCI bus.

	-hpa

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Eric W. Biederman]

"H. Peter Anvin" <hpa@zytor.com> writes:

> Ingo Molnar wrote:
>> 
>> well then we could insert CF9 to before the triple fault, and solve 
>> some of the problems as well, without unnecessary risks.
>> 
>> This is a separate patch from ACPI reboot itself, naturally.
>> 
>
> This is a pretty good point.  There is extremely low risk of being
> something at port CF9 that is something other than the reset register.
> It may not be there, but if it isn't, the worst thing that happened is
> we confused a device right before reset.
>
> Even better if we condition the port CF9 write on existence a PCI bus
> (or even more specifically PCI Configuration Method #1 or #2).  Port CF9
> is extremely unlikely to exist on a machine without a PCI bus, and
> extremely unlikely to conflict with anything else on a machine with a
> PCI bus.

Yes.  What we are guarding against in practice is a write that causes
the system to hang.  Anything else and we will make it to the next
reset method.

Eric

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Ingo Molnar]

* Eric W. Biederman <ebiederm@xmission.com> wrote:

> "H. Peter Anvin" <hpa@zytor.com> writes:
> 
> > Ingo Molnar wrote:
> >> 
> >> well then we could insert CF9 to before the triple fault, and solve 
> >> some of the problems as well, without unnecessary risks.
> >> 
> >> This is a separate patch from ACPI reboot itself, naturally.
> >> 
> >
> > This is a pretty good point.  There is extremely low risk of being 
> > something at port CF9 that is something other than the reset 
> > register. It may not be there, but if it isn't, the worst thing 
> > that happened is we confused a device right before reset.
> >
> > Even better if we condition the port CF9 write on existence a PCI 
> > bus (or even more specifically PCI Configuration Method #1 or #2).  
> > Port CF9 is extremely unlikely to exist on a machine without a PCI 
> > bus, and extremely unlikely to conflict with anything else on a 
> > machine with a PCI bus.
> 
> Yes.  What we are guarding against in practice is a write that 
> causes the system to hang.  Anything else and we will make it to the 
> next reset method.

yeah. Patches are welcome :)

	Ingo

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Len Brown]

On Mon, 10 Nov 2008, Matthew Garrett wrote:

> On Mon, Nov 10, 2008 at 01:56:30PM +0100, Ingo Molnar wrote:
> > * Matthew Garrett <mjg@redhat.com> wrote:
> > > or something. Microsoft appear to have moved away from using date 
> > > cutoffs for anything other than whether or not to enable ACPI in the 
> > > first place, and we ought to attempt compatibility with them.
> > 
> > okay, that's fine to me too. My main point is that we need something 
> > nuanced this time around (be it a string check or a cutoff) - not the 
> > "enable again" patch that i saw in the ACPI tree and which i had to 
> > NAK.
> 
> All we need now is confirmation as to which versions of Windows use this 
> behaviour.

We knows XP and Vista do it.

But upstream doesn't currently check the FADT.flags.reset-reg-supported bit
due to a recent bad guess on my part on how to be bug compatible with 
windows.

The (revert) patch to add that check is in my tree, along with
the trivial patch to flip the default to acpi-reset.

Technically, that is the only "unanced" thing we should need
to check.  However, it will not fix Avi's box, where it appears
that flag is present, the reset works, but for some reason the
keyboard fails after reset.  More likely that is a device driver
issue specific to Linux interacting with "unexpected" BIOS behavior.

Ingo,
If you don't mind, I'd like to continue to keep a version
of the acpi-reset-default patch in my test tree so that
it is seen by linux-next.  Once I have something that
I think merits upstream inclusion, I'll send a request
to you.  Will that work?

thanks,
-Len

Attempt rebooting via port CF9 if it seems to be available

[H. Peter Anvin]

[-- Attachment #1: Type: text/plain, Size: 855 bytes --]

Looking at doing the port CF9 fallback, I stumbled onto something fishy.

I wonder if anyone here happens to have any idea why we turn off
caching in machine_real_restart()?  Jumping to the BIOS is *not* a
reset; we jump to the decompressed BIOS on low memory which is usually
shadowed, not to the BIOS entry point.  In that way, it's more of an
INIT than a reset, and disabling caching seems broken.

Either way, here is a preliminary patch to do the CF9 if safe, and then
falling back to keyboard reboot.  I'm a bit concerned about how to test
it, of course; this stuff is sensitive and just about impossible to test
except on millions of machines at once...

If you have any machines (especially problematic ones) and find that
this patch either helps or hurts or do nothing, please do let me know so
I have any idea of the extent of coverage.

	-hpa


[-- Attachment #2: 0001-x86-attempt-reboot-via-port-CF9-if-we-have-standard.patch --]
[-- Type: text/x-patch, Size: 5294 bytes --]

>From 14d7ca5c575853664d8fe4f225a77b8df1b7de7d Mon Sep 17 00:00:00 2001
From: H. Peter Anvin <hpa@zytor.com>
Date: Tue, 11 Nov 2008 16:19:48 -0800
Subject: [PATCH] x86: attempt reboot via port CF9 if we have standard PCI ports

Impact: Changes reboot behavior.

If port CF9 seems to be safe to touch, attempt it before trying the
keyboard controller.  Port CF9 is not available on all chipsets (a
significant but decreasing number of modern chipsets don't implement
it), but port CF9 itself should in general be safe to poke (no ill
effects if unimplemented) on any system which has PCI Configuration
Method #1 or #2, as it falls inside the PCI configuration port range
in both cases.  No chipset without PCI is known to have port CF9,
either, although an explicit "pci=bios" would mean we miss this and
therefore don't use port CF9.  An explicit "reboot=pci" can be used to
force the use of port CF9.

Signed-off-by: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/include/asm/emergency-restart.h |    4 ++-
 arch/x86/kernel/reboot.c                 |   34 +++++++++++++++++++++++------
 arch/x86/pci/direct.c                    |    4 ++-
 arch/x86/pci/pci.h                       |    1 +
 4 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/emergency-restart.h b/arch/x86/include/asm/emergency-restart.h
index 94826cf..cc70c1c 100644
--- a/arch/x86/include/asm/emergency-restart.h
+++ b/arch/x86/include/asm/emergency-restart.h
@@ -8,7 +8,9 @@ enum reboot_type {
 	BOOT_BIOS = 'b',
 #endif
 	BOOT_ACPI = 'a',
-	BOOT_EFI = 'e'
+	BOOT_EFI = 'e',
+	BOOT_CF9 = 'p',
+	BOOT_CF9_COND = 'q',
 };
 
 extern enum reboot_type reboot_type;
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 34f8d37..ddc9389 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -29,14 +29,17 @@ EXPORT_SYMBOL(pm_power_off);
 
 static const struct desc_ptr no_idt = {};
 static int reboot_mode;
-enum reboot_type reboot_type = BOOT_KBD;
+enum reboot_type reboot_type = BOOT_CF9_COND;
 int reboot_force;
 
 #if defined(CONFIG_X86_32) && defined(CONFIG_SMP)
 static int reboot_cpu = -1;
 #endif
 
-/* reboot=b[ios] | s[mp] | t[riple] | k[bd] | e[fi] [, [w]arm | [c]old]
+/* This is set by the PCI code if either type 1 or type 2 PCI is detected */
+bool port_cf9_safe = false;
+
+/* reboot=b[ios] | s[mp] | t[riple] | k[bd] | e[fi] [, [w]arm | [c]old] | p[ci]
    warm   Don't set the cold reboot flag
    cold   Set the cold reboot flag
    bios   Reboot by jumping through the BIOS (only for X86_32)
@@ -45,6 +48,7 @@ static int reboot_cpu = -1;
    kbd    Use the keyboard controller. cold reset (default)
    acpi   Use the RESET_REG in the FADT
    efi    Use efi reset_system runtime service
+   pci    Use the so-called "PCI reset register", CF9
    force  Avoid anything that could hang.
  */
 static int __init reboot_setup(char *str)
@@ -79,6 +83,7 @@ static int __init reboot_setup(char *str)
 		case 'k':
 		case 't':
 		case 'e':
+		case 'p':
 			reboot_type = *str;
 			break;
 
@@ -379,28 +384,43 @@ static void native_machine_emergency_restart(void)
 			load_idt(&no_idt);
 			__asm__ __volatile__("int3");
 
-			reboot_type = BOOT_KBD;
+			reboot_type = BOOT_CF9_COND;
 			break;
 
 #ifdef CONFIG_X86_32
 		case BOOT_BIOS:
 			machine_real_restart(jump_to_bios, sizeof(jump_to_bios));
 
-			reboot_type = BOOT_KBD;
+			reboot_type = BOOT_CF9_COND;
 			break;
 #endif
 
 		case BOOT_ACPI:
 			acpi_reboot();
-			reboot_type = BOOT_KBD;
+			reboot_type = BOOT_CF9_COND;
 			break;
 
-
 		case BOOT_EFI:
 			if (efi_enabled)
-				efi.reset_system(reboot_mode ? EFI_RESET_WARM : EFI_RESET_COLD,
+				efi.reset_system(reboot_mode ?
+						 EFI_RESET_WARM :
+						 EFI_RESET_COLD,
 						 EFI_SUCCESS, 0, NULL);
+			reboot_type = BOOT_CF9_COND;
+			break;
+
+		case BOOT_CF9:
+			port_cf9_safe = true;
+			/* fall through */
 
+		case BOOT_CF9_COND:
+			if (port_cf9_safe) {
+				u8 cf9 = inb(0xcf9) & ~6;
+				outb(cf9|2, 0xcf9); /* Request hard reset */
+				udelay(50);
+				outb(cf9|6, 0xcf9); /* Actually do the reset */
+				udelay(50);
+			}
 			reboot_type = BOOT_KBD;
 			break;
 		}
diff --git a/arch/x86/pci/direct.c b/arch/x86/pci/direct.c
index 9915293..9a5af6c 100644
--- a/arch/x86/pci/direct.c
+++ b/arch/x86/pci/direct.c
@@ -173,7 +173,7 @@ static int pci_conf2_write(unsigned int seg, unsigned int bus,
 
 #undef PCI_CONF2_ADDRESS
 
-static struct pci_raw_ops pci_direct_conf2 = {
+struct pci_raw_ops pci_direct_conf2 = {
 	.read =		pci_conf2_read,
 	.write =	pci_conf2_write,
 };
@@ -289,6 +289,7 @@ int __init pci_direct_probe(void)
 
 	if (pci_check_type1()) {
 		raw_pci_ops = &pci_direct_conf1;
+		port_cf9_safe = true;
 		return 1;
 	}
 	release_resource(region);
@@ -305,6 +306,7 @@ int __init pci_direct_probe(void)
 
 	if (pci_check_type2()) {
 		raw_pci_ops = &pci_direct_conf2;
+		port_cf9_safe = true;
 		return 2;
 	}
 
diff --git a/arch/x86/pci/pci.h b/arch/x86/pci/pci.h
index 15b9cf6..1959018 100644
--- a/arch/x86/pci/pci.h
+++ b/arch/x86/pci/pci.h
@@ -96,6 +96,7 @@ extern struct pci_raw_ops *raw_pci_ops;
 extern struct pci_raw_ops *raw_pci_ext_ops;
 
 extern struct pci_raw_ops pci_direct_conf1;
+extern bool port_cf9_safe;
 
 /* arch_initcall level */
 extern int pci_direct_probe(void);
-- 
1.5.6.5

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Matthew Garrett]

On Tue, Nov 11, 2008 at 06:14:31PM -0500, Len Brown wrote:
> On Mon, 10 Nov 2008, Matthew Garrett wrote:
> > All we need now is confirmation as to which versions of Windows use this 
> > behaviour.
> 
> We knows XP and Vista do it.

How would you feel about having a global variable bitfield containing 
the operating systems the firmware queries, and then keying decisions 
like this off that?

> But upstream doesn't currently check the FADT.flags.reset-reg-supported bit
> due to a recent bad guess on my part on how to be bug compatible with 
> windows.

Mm. Getting that right is probably important, as is trying to figure out 
why we break on certain machines in that case. Is it possible that going 
via the keyboad controller tends to cause an implicit reset in it that's 
not mimiced by the i8042 shutdown code?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Ingo Molnar]

* Len Brown <lenb@kernel.org> wrote:

> 
> 
> On Mon, 10 Nov 2008, Matthew Garrett wrote:
> 
> > On Mon, Nov 10, 2008 at 01:56:30PM +0100, Ingo Molnar wrote:
> > > * Matthew Garrett <mjg@redhat.com> wrote:
> > > > or something. Microsoft appear to have moved away from using date 
> > > > cutoffs for anything other than whether or not to enable ACPI in the 
> > > > first place, and we ought to attempt compatibility with them.
> > > 
> > > okay, that's fine to me too. My main point is that we need something 
> > > nuanced this time around (be it a string check or a cutoff) - not the 
> > > "enable again" patch that i saw in the ACPI tree and which i had to 
> > > NAK.
> > 
> > All we need now is confirmation as to which versions of Windows use this 
> > behaviour.
> 
> We knows XP and Vista do it.
> 
> But upstream doesn't currently check the FADT.flags.reset-reg-supported bit
> due to a recent bad guess on my part on how to be bug compatible with 
> windows.
> 
> The (revert) patch to add that check is in my tree, along with
> the trivial patch to flip the default to acpi-reset.
> 
> Technically, that is the only "unanced" thing we should need
> to check.  However, it will not fix Avi's box, where it appears
> that flag is present, the reset works, but for some reason the
> keyboard fails after reset.  More likely that is a device driver
> issue specific to Linux interacting with "unexpected" BIOS behavior.

hm, will that also fix Andrey's box?

> Ingo,
> If you don't mind, I'd like to continue to keep a version
> of the acpi-reset-default patch in my test tree so that
> it is seen by linux-next.  Once I have something that
> I think merits upstream inclusion, I'll send a request
> to you.  Will that work?

It's fine to me - although i'm a bit uncomfortable about keeping a 
known breakage in linux-next.

linux-next is not really there to experiment around, it's there to 
push the known stable stuff to. linux-next has enough trouble with 
_unintended_ breakages.

At least that's how i push patches to linux-next - i've Cc:-ed Stephen 
and Andrew if there's a clarification needed.

But i _think_ we should be fine even with the KVM related reboot 
problems if we insert the CF9 sequence right before the triple fault.

	Ingo

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Avi Kivity]

Ingo Molnar wrote:
>>
>> We knows XP and Vista do it.
>>
>> But upstream doesn't currently check the FADT.flags.reset-reg-supported bit
>> due to a recent bad guess on my part on how to be bug compatible with 
>> windows.
>>
>> The (revert) patch to add that check is in my tree, along with
>> the trivial patch to flip the default to acpi-reset.
>>
>> Technically, that is the only "unanced" thing we should need
>> to check.  However, it will not fix Avi's box, where it appears
>> that flag is present, the reset works, but for some reason the
>> keyboard fails after reset.  More likely that is a device driver
>> issue specific to Linux interacting with "unexpected" BIOS behavior.
>>     
>
> hm, will that also fix Andrey's box?
>   

This describes Andrey's box, not mine.  Mine (and others) have working 
acpi reset and non-working keyboard reset; so reset was done using 
triple-fault which fails when virtualization is enabled.


-- 
error compiling committee.c: too many arguments to function

Re: Attempt rebooting via port CF9 if it seems to be available

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 999 bytes --]

On Wednesday 12 November 2008, H. Peter Anvin wrote:
> Looking at doing the port CF9 fallback, I stumbled onto something fishy.
> 
> I wonder if anyone here happens to have any idea why we turn off
> caching in machine_real_restart()?  Jumping to the BIOS is *not* a
> reset; we jump to the decompressed BIOS on low memory which is usually
> shadowed, not to the BIOS entry point.  In that way, it's more of an
> INIT than a reset, and disabling caching seems broken.
> 
> Either way, here is a preliminary patch to do the CF9 if safe, and then
> falling back to keyboard reboot.  I'm a bit concerned about how to test
> it, of course; this stuff is sensitive and just about impossible to test
> except on millions of machines at once...
> 
> If you have any machines (especially problematic ones) and find that
> this patch either helps or hurts or do nothing, please do let me know so
> I have any idea of the extent of coverage.
> 

Works here both with default and reboot=p.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[H. Peter Anvin]

Zhao Yakui wrote:
> 
> Peter Anvin suggests that OS attempt reboot via 0xCF9 port if avaiable.
> This is not very good. 0xCF9 I/O port is the reset control register
> defined in Intel ICH6/7/8/9 chipset. In theory it is effective for Intel
> chipset. Maybe it is not applied for other chipset(For example: Nvidia,
> ALI, VIA). There exists a laptop in bug 11942, in which the 0xCF9 I/O
> port is defined as the RESET_REG. But unfortunately it can't be rebooted
> Via 0xCF9 I/O port. 
>     In fact although the 0xCF9 is defined for Intel ICH chipset, it
> doesn't mean that all the box based on ICH chipset can be rebooted via
> 0xCF9 I/O port. For example: we have a laptop based on intel ICH6
> chipset that can't be rebooted by writing the 0x06 to 0xCF9 I/O port.
> 

Port CF9 isn't universal, but other vendors are increasingly
implementing it -- I know ALi are in their more recent chipsets.  Either
way, I agree that we need to figure out how to get ACPI to work, but CF9
is a low risk fallback before going to the KBC.

	-hpa

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Zhao Yakui]

On Wed, 2008-11-12 at 20:23 +0800, Avi Kivity wrote:
> Ingo Molnar wrote:
> >>
> >> We knows XP and Vista do it.
> >>
> >> But upstream doesn't currently check the FADT.flags.reset-reg-supported bit
> >> due to a recent bad guess on my part on how to be bug compatible with 
> >> windows.
> >>
> >> The (revert) patch to add that check is in my tree, along with
> >> the trivial patch to flip the default to acpi-reset.
> >>
> >> Technically, that is the only "unanced" thing we should need
> >> to check.  However, it will not fix Avi's box, where it appears
> >> that flag is present, the reset works, but for some reason the
> >> keyboard fails after reset.  More likely that is a device driver
> >> issue specific to Linux interacting with "unexpected" BIOS behavior.
> >>     
> >
> > hm, will that also fix Andrey's box?
> >   
> 
> This describes Andrey's box, not mine.  Mine (and others) have working 
> acpi reset and non-working keyboard reset; so reset was done using 
> triple-fault which fails when virtualization is enabled.
I agree with what Len has said about the Andrey's box. (Andrey attached
the acpidump/dmesg/dmidecode to the bug 11895).
     From the acpidump it seems that the acpi reset flag bit is set and
reset_reg is also defined. It indicates that the ACPI reset mechanism is
supported. But unfortunately the keyboard can't work well after acpi
reboot is used.
     
With the help of KVM now it is tested that the ACPI reboot is used on
windows XP/Vista if the acpi reboot is supported.(The flag bit is set
and RESET_REG is also defined). The only exception is that acpi reboot
is not used on windows XP/Vista even when it is supported if the
revision of FADT is 1(It indicates that the BIOS follows the ACPI 1.0
spec). And in my test the acpi reboot is also used on windows XP/vista
if the FADT revision is 2. 
    Maybe it is better that Andrey can confirm whether the keyboard
still work well after reboot on windows. If the windows can work well,
maybe the issue is related with the device driver. 

As Avi said, the system can't be rebooted fully by tripple fault/KBD
reset after VT is enabled. And the system can be rebooted fully by ACPI
reboot. Maybe the ACPI reboot should be tried firstly. If acpi reboot is
not supported, then fall backs to the other reboot type(KBD). 
   Of course the strict check should be added for the ACPI reboot. The
flag and reset_reg should be checked. Maybe the revision of FADT should
also be checked. If the FADT revision is below 3, the acpi reboot is
ignored even when it is supported on the box.
   

Peter Anvin suggests that OS attempt reboot via 0xCF9 port if avaiable.
This is not very good. 0xCF9 I/O port is the reset control register
defined in Intel ICH6/7/8/9 chipset. In theory it is effective for Intel
chipset. Maybe it is not applied for other chipset(For example: Nvidia,
ALI, VIA). There exists a laptop in bug 11942, in which the 0xCF9 I/O
port is defined as the RESET_REG. But unfortunately it can't be rebooted
Via 0xCF9 I/O port. 
    In fact although the 0xCF9 is defined for Intel ICH chipset, it
doesn't mean that all the box based on ICH chipset can be rebooted via
0xCF9 I/O port. For example: we have a laptop based on intel ICH6
chipset that can't be rebooted by writing the 0x06 to 0xCF9 I/O port.

Thanks.
> 
> 

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

Hi Len,

On Wed, 12 Nov 2008 12:58:06 +0100 Ingo Molnar <mingo@elte.hu> wrote:
>
> * Len Brown <lenb@kernel.org> wrote:
> 
> > Ingo,
> > If you don't mind, I'd like to continue to keep a version
> > of the acpi-reset-default patch in my test tree so that
> > it is seen by linux-next.  Once I have something that
> > I think merits upstream inclusion, I'll send a request
> > to you.  Will that work?
> 
> It's fine to me - although i'm a bit uncomfortable about keeping a 
> known breakage in linux-next.
> 
> linux-next is not really there to experiment around, it's there to 
> push the known stable stuff to. linux-next has enough trouble with 
> _unintended_ breakages.
> 
> At least that's how i push patches to linux-next - i've Cc:-ed Stephen 
> and Andrew if there's a clarification needed.

See my posting yesterday about procedures for linux-next.  It is really
only for integration testing.  If your code does not "merit upstream
inclusion", then it should not be in linux-next.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Zhao Yakui]

On Thu, 2008-11-13 at 11:18 +0800, H. Peter Anvin wrote:
> Zhao Yakui wrote:
> > 
> > Peter Anvin suggests that OS attempt reboot via 0xCF9 port if avaiable.
> > This is not very good. 0xCF9 I/O port is the reset control register
> > defined in Intel ICH6/7/8/9 chipset. In theory it is effective for Intel
> > chipset. Maybe it is not applied for other chipset(For example: Nvidia,
> > ALI, VIA). There exists a laptop in bug 11942, in which the 0xCF9 I/O
> > port is defined as the RESET_REG. But unfortunately it can't be rebooted
> > Via 0xCF9 I/O port. 
> >     In fact although the 0xCF9 is defined for Intel ICH chipset, it
> > doesn't mean that all the box based on ICH chipset can be rebooted via
> > 0xCF9 I/O port. For example: we have a laptop based on intel ICH6
> > chipset that can't be rebooted by writing the 0x06 to 0xCF9 I/O port.
> > 
> 
> Port CF9 isn't universal, but other vendors are increasingly
> implementing it -- I know ALi are in their more recent chipsets.  Either
> way, I agree that we need to figure out how to get ACPI to work, but CF9
> is a low risk fallback before going to the KBC.
   0xCF9 I/O port is applied for most laptops based on intel ICH6/7/8/9
chipset as it is a reset control register defined in the ICH chipset.
But it is not applied for some laptops. After the 0xCF9 I/O port is
tried, maybe the system already hangs and it can't fall back to
KBD/tripple fault mode.
   
In fact the 0xCF9 I/O port is defined as RESET_REG port on some laptops
on which the acpi reboot mechanism is supported. If acpi reboot is not
supported on the box, we had better not try the reboot via 0xCF9 I/O
port as we can't know whether the 0xCF9 I/O is supported on the box. At
the same time we don't know the proper value that should be written into
0xCF9 I/O port although the 0xCF9 I/O port is supported. 
    For example on intel chipset:      
        Sometimes 0x04 is enough.
        Sometimes 0x06 is enough.
        Sometimes 0x0e is enough.

Thanks.
> 
> 	-hpa

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Eric W. Biederman]

Zhao Yakui <yakui.zhao@intel.com> writes:


> Peter Anvin suggests that OS attempt reboot via 0xCF9 port if avaiable.
> This is not very good. 0xCF9 I/O port is the reset control register
> defined in Intel ICH6/7/8/9 chipset. In theory it is effective for Intel
> chipset. Maybe it is not applied for other chipset(For example: Nvidia,
> ALI, VIA).

Looking at the coreboot source.  It is indeed defined for Nvidia, SIS,
AMD and several others.

A lot of boards use:
outb(0x02, 0xcf9);
outb(0x06, 0xcf9);

Instead of just writing a plain 6.   I think at least on some machines
there is a requirement for a low to hight transition.

> There exists a laptop in bug 11942, in which the 0xCF9 I/O
> port is defined as the RESET_REG. But unfortunately it can't be rebooted
> Via 0xCF9 I/O port. 
>     In fact although the 0xCF9 is defined for Intel ICH chipset, it
> doesn't mean that all the box based on ICH chipset can be rebooted via
> 0xCF9 I/O port. For example: we have a laptop based on intel ICH6
> chipset that can't be rebooted by writing the 0x06 to 0xCF9 I/O port.

Right.  The only scary question is will a motherboard hang if you write
to 0xcf9.  If we don't know of an example where writing to 0xcf9 will prevent
us from getting to the next reset method then adding a generic 0xcf9 is
safe.

If we are totally paranoid we can build of a whitelist of motherboards
where writing to 0xcf9 works.  By reading and checkup on the chipset
docs, but don't make the mistake of thinking 0xcf9 is an Intel only
thing.

Eric

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Eric W. Biederman]

Zhao Yakui <yakui.zhao@intel.com> writes:


> Peter Anvin suggests that OS attempt reboot via 0xCF9 port if avaiable.
> This is not very good. 0xCF9 I/O port is the reset control register
> defined in Intel ICH6/7/8/9 chipset. In theory it is effective for Intel
> chipset. Maybe it is not applied for other chipset(For example: Nvidia,
> ALI, VIA).

Looking at the coreboot source.  It is indeed defined for Nvidia, SIS,
AMD and several others.

A lot of boards use:
outb(0x02, 0xcf9);
outb(0x06, 0xcf9);

Instead of just writing a plain 6.   I think at least on some machines
there is a requirement for a low to hight transition.

> There exists a laptop in bug 11942, in which the 0xCF9 I/O
> port is defined as the RESET_REG. But unfortunately it can't be rebooted
> Via 0xCF9 I/O port. 
>     In fact although the 0xCF9 is defined for Intel ICH chipset, it
> doesn't mean that all the box based on ICH chipset can be rebooted via
> 0xCF9 I/O port. For example: we have a laptop based on intel ICH6
> chipset that can't be rebooted by writing the 0x06 to 0xCF9 I/O port.

Right.  The only scary question is will a motherboard hang if you write
to 0xcf9.  If we don't know of an example where writing to 0xcf9 will prevent
us from getting to the next reset method then adding a generic 0xcf9 is
safe.

If we are totally paranoid we can build of a whitelist of motherboards
where writing to 0xcf9 works.  By reading and checkup on the chipset
docs, but don't make the mistake of thinking 0xcf9 is an Intel only
thing.

Eric

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[H. Peter Anvin]

Eric W. Biederman wrote:
> 
> Looking at the coreboot source.  It is indeed defined for Nvidia, SIS,
> AMD and several others.
> 
> A lot of boards use:
> outb(0x02, 0xcf9);
> outb(0x06, 0xcf9);
> 
> Instead of just writing a plain 6.   I think at least on some machines
> there is a requirement for a low to hight transition.
> 

That is correct; that is what my patch has, with a 50 µs delay in
between (I also don't touch the ~6 bits.)

> 
> Right.  The only scary question is will a motherboard hang if you write
> to 0xcf9.  If we don't know of an example where writing to 0xcf9 will prevent
> us from getting to the next reset method then adding a generic 0xcf9 is
> safe.
> 

It would be highly surprising, except possibly on pre-PCI systems, which
is why my patch skips CF9 if the standard PCI ports are not detected.
Although CF9 isn't defined in the PCI spec, it falls in the PCI
configuration port range.

	-hpa
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[H. Peter Anvin]

Zhao Yakui wrote:
> Maybe the 0xCF9 port is defined for NVidia, SiS and Several others
> vendor. But we can't say that it is appropriate for most boxes. 
> Of course IMO it is not a generic solution for reboot.
> At the same time the value written to the 0xCF9 I/O port should not be a
> fixed value. Maybe it varies on different vendors/chipset. Even the
> value will vary on the boxes from different BIOS vendors although the
> boxes are based on the same chipset. 
>     And the value should be provided by BIOS. If BIOS doesn't provide
> the above info, it is not safe to use them. 

We all agree that we should try to get the ACPI stuff to work as the 
first instance.  **THAT IS ALL THE HELP WE WILL GET FROM THE BIOS**.
Now, it isn't working.  Now what do you do?

In short, unless you have anything concrete to add to this discussion, 
please stop pontificating.

	-hpa

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Zhao Yakui]

On Thu, 2008-11-13 at 12:14 +0800, Eric W. Biederman wrote:
> Zhao Yakui <yakui.zhao@intel.com> writes:
> 
> 
> > Peter Anvin suggests that OS attempt reboot via 0xCF9 port if avaiable.
> > This is not very good. 0xCF9 I/O port is the reset control register
> > defined in Intel ICH6/7/8/9 chipset. In theory it is effective for Intel
> > chipset. Maybe it is not applied for other chipset(For example: Nvidia,
> > ALI, VIA).
> 
> Looking at the coreboot source.  It is indeed defined for Nvidia, SIS,
> AMD and several others.
> 
> A lot of boards use:
> outb(0x02, 0xcf9);
> outb(0x06, 0xcf9);
> 
> Instead of just writing a plain 6.   I think at least on some machines
> there is a requirement for a low to hight transition.
Maybe the 0xCF9 port is defined for NVidia, SiS and Several others
vendor. But we can't say that it is appropriate for most boxes. 
Of course IMO it is not a generic solution for reboot.
At the same time the value written to the 0xCF9 I/O port should not be a
fixed value. Maybe it varies on different vendors/chipset. Even the
value will vary on the boxes from different BIOS vendors although the
boxes are based on the same chipset. 
    And the value should be provided by BIOS. If BIOS doesn't provide
the above info, it is not safe to use them. 
> 
> > There exists a laptop in bug 11942, in which the 0xCF9 I/O
> > port is defined as the RESET_REG. But unfortunately it can't be rebooted
> > Via 0xCF9 I/O port. 
> >     In fact although the 0xCF9 is defined for Intel ICH chipset, it
> > doesn't mean that all the box based on ICH chipset can be rebooted via
> > 0xCF9 I/O port. For example: we have a laptop based on intel ICH6
> > chipset that can't be rebooted by writing the 0x06 to 0xCF9 I/O port.
> 
> Right.  The only scary question is will a motherboard hang if you write
> to 0xcf9.  If we don't know of an example where writing to 0xcf9 will prevent
> us from getting to the next reset method then adding a generic 0xcf9 is
> safe.
> 
> If we are totally paranoid we can build of a whitelist of motherboards
> where writing to 0xcf9 works.  By reading and checkup on the chipset
> docs, but don't make the mistake of thinking 0xcf9 is an Intel only
> thing.
> 
> Eric
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Eric W. Biederman]

Zhao Yakui <yakui.zhao@intel.com> writes:

>> Instead of just writing a plain 6.   I think at least on some machines
>> there is a requirement for a low to hight transition.

> Maybe the 0xCF9 port is defined for NVidia, SiS and Several others
> vendor. But we can't say that it is appropriate for most boxes. 

Let me get this straight.  We survey all of the major chipsets in existence.
We discover a register that is implemented the same way in all of them.
You conclude that using that register is not appropriate for most boxes?

> Of course IMO it is not a generic solution for reboot.
> At the same time the value written to the 0xCF9 I/O port should not be a
> fixed value. Maybe it varies on different vendors/chipset. Even the
> value will vary on the boxes from different BIOS vendors although the
> boxes are based on the same chipset. 

Reading in between the lines.  Did you just say someone implemented
a nasty hack on some laptop and attempting to reboot by writing to 0xcf9
will cause nasty problems?

If you know of such a case please describe the failure mode.  If there
is a case where writing to 0xcf9 will makes things worse than we need
to tread carefully, and we will happily deal with reality.  If you can
only tell us that we are being rude and circumventing a vendors vision
of the then your input is not especially useful.

>     And the value should be provided by BIOS. If BIOS doesn't provide
> the above info, it is not safe to use them. 

Balderdash.  There are other ways to learn things than by talking to
the BIOS.  Usually BIOS's are a mediocre information source at best.
The job of a BIOS is to boot and if it can do that solidly you can ship it,
when the schedule gets tight.  Everything else is a nice to have.

Eric

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Eric W. Biederman]

Zhao Yakui <yakui.zhao@intel.com> writes:

>> Instead of just writing a plain 6.   I think at least on some machines
>> there is a requirement for a low to hight transition.

> Maybe the 0xCF9 port is defined for NVidia, SiS and Several others
> vendor. But we can't say that it is appropriate for most boxes. 

Let me get this straight.  We survey all of the major chipsets in existence.
We discover a register that is implemented the same way in all of them.
You conclude that using that register is not appropriate for most boxes?

> Of course IMO it is not a generic solution for reboot.
> At the same time the value written to the 0xCF9 I/O port should not be a
> fixed value. Maybe it varies on different vendors/chipset. Even the
> value will vary on the boxes from different BIOS vendors although the
> boxes are based on the same chipset. 

Reading in between the lines.  Did you just say someone implemented
a nasty hack on some laptop and attempting to reboot by writing to 0xcf9
will cause nasty problems?

If you know of such a case please describe the failure mode.  If there
is a case where writing to 0xcf9 will makes things worse than we need
to tread carefully, and we will happily deal with reality.  If you can
only tell us that we are being rude and circumventing a vendors vision
of the then your input is not especially useful.

>     And the value should be provided by BIOS. If BIOS doesn't provide
> the above info, it is not safe to use them. 

Balderdash.  There are other ways to learn things than by talking to
the BIOS.  Usually BIOS's are a mediocre information source at best.
The job of a BIOS is to boot and if it can do that solidly you can ship it,
when the schedule gets tight.  Everything else is a nice to have.

Eric

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Zhao Yakui]

On Thu, 2008-11-13 at 14:58 +0800, Eric W. Biederman wrote:
> Zhao Yakui <yakui.zhao@intel.com> writes:
> 
> >> Instead of just writing a plain 6.   I think at least on some machines
> >> there is a requirement for a low to hight transition.
> 
> > Maybe the 0xCF9 port is defined for NVidia, SiS and Several others
> > vendor. But we can't say that it is appropriate for most boxes. 
> 
> Let me get this straight.  We survey all of the major chipsets in existence.
> We discover a register that is implemented the same way in all of them.
> You conclude that using that register is not appropriate for most boxes
I am sorry that I don't describe it very clearly. The value written into
0xCF9 I/O port maybe vary on different chipset vendors. In such case it
is difficult to write the generic 0xCF9 reboot mechanism. If the same
standard about 0xCF9 I/O port is followed by most chipset vendors, it
will be OK.
> 
> > Of course IMO it is not a generic solution for reboot.
> > At the same time the value written to the 0xCF9 I/O port should not be a
> > fixed value. Maybe it varies on different vendors/chipset. Even the
> > value will vary on the boxes from different BIOS vendors although the
> > boxes are based on the same chipset. 
> 
> Reading in between the lines.  Did you just say someone implemented
> a nasty hack on some laptop and attempting to reboot by writing to 0xcf9
> will cause nasty problems?
I have a laptop on which the ACPI reboot mechanism is not supported.(The
reset flag is not present). But there exists the definition of RESET_REG
& RESET_VALUE. The RESET_REG is 0xCF9 I/O port. (The box is based on
Intel i915 chipsets)
    When writing 0x06 to 0xCF9 I/O port the box will hang and can't be
rebooted. (Hardware reset will fail)
    When writing 0x04 to 0xCF9 I/O port the box can be
rebooted.(Software reset is OK)

On the laptop of bug 11942(kernel bugzilla) the acpi reboot is also
unsupported. But there exists the definition of 0xCF9 I/O port. (The box
is based on AMD platform. The CPU is Phenom 9500 Quad-core.)
     After writing 0x06 to 0xCF9 I/O port, the box can't be rebooted.
> 
> If you know of such a case please describe the failure mode.  If there
> is a case where writing to 0xcf9 will makes things worse than we need
> to tread carefully, and we will happily deal with reality.  If you can
> only tell us that we are being rude and circumventing a vendors vision
> of the then your input is not especially useful.
> 
> >     And the value should be provided by BIOS. If BIOS doesn't provide
> > the above info, it is not safe to use them. 
> 
> Balderdash.  There are other ways to learn things than by talking to
> the BIOS.  Usually BIOS's are a mediocre information source at best.
> The job of a BIOS is to boot and if it can do that solidly you can ship it,
> when the schedule gets tight.  Everything else is a nice to have.
> 
> Eric
> 

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[H. Peter Anvin]

Zhao Yakui wrote:
> I am sorry that I don't describe it very clearly. The value written into
> 0xCF9 I/O port maybe vary on different chipset vendors. In such case it
> is difficult to write the generic 0xCF9 reboot mechanism. If the same
> standard about 0xCF9 I/O port is followed by most chipset vendors, it
> will be OK.

What my patch does is:

u8 cf9 = inb(0xcf9) & ~6;
outb(cf9|2, 0xcf9); /* Request hard reset */
udelay(50);
outb(cf9|6, 0xcf9); /* Actually do the reset */
udelay(50);

> I have a laptop on which the ACPI reboot mechanism is not supported.(The
> reset flag is not present). But there exists the definition of RESET_REG
> & RESET_VALUE. The RESET_REG is 0xCF9 I/O port. (The box is based on
> Intel i915 chipsets)
>     When writing 0x06 to 0xCF9 I/O port the box will hang and can't be
> rebooted. (Hardware reset will fail)
>     When writing 0x04 to 0xCF9 I/O port the box can be
> rebooted.(Software reset is OK)

What happens if you write 0x02 first, and *then* write 0x06?

More details about this laptop, please?  Also, what is RESET_REG and
RESET_VALUE defined as?

	-hpa

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[H. Peter Anvin]

Zhao Yakui wrote:
> I do the test as required. First the 0x02 is written to 0xCF9 I/O port.
> The return value by reading the 0xCF9 I/O port is 0x02. Then the 0x06 is
> written to the 0xCF9 I/O port. The screen becomes black but the box
> can't be rebooted.

What is the value of the CF9 port before you do anything?

	-hpa

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Zhao Yakui]

On Fri, 2008-11-14 at 01:42 +0800, H. Peter Anvin wrote:
> Zhao Yakui wrote:
> > I am sorry that I don't describe it very clearly. The value written into
> > 0xCF9 I/O port maybe vary on different chipset vendors. In such case it
> > is difficult to write the generic 0xCF9 reboot mechanism. If the same
> > standard about 0xCF9 I/O port is followed by most chipset vendors, it
> > will be OK.
> 
> What my patch does is:
> 
> u8 cf9 = inb(0xcf9) & ~6;
> outb(cf9|2, 0xcf9); /* Request hard reset */
> udelay(50);
> outb(cf9|6, 0xcf9); /* Actually do the reset */
> udelay(50);
> 
> > I have a laptop on which the ACPI reboot mechanism is not supported.(The
> > reset flag is not present). But there exists the definition of RESET_REG
> > & RESET_VALUE. The RESET_REG is 0xCF9 I/O port. (The box is based on
> > Intel i915 chipsets)
> >     When writing 0x06 to 0xCF9 I/O port the box will hang and can't be
> > rebooted. (Hardware reset will fail)
> >     When writing 0x04 to 0xCF9 I/O port the box can be
> > rebooted.(Software reset is OK)
I do the test as required. First the 0x02 is written to 0xCF9 I/O port.
The return value by reading the 0xCF9 I/O port is 0x02. Then the 0x06 is
written to the 0xCF9 I/O port. The screen becomes black but the box
can't be rebooted.
> What happens if you write 0x02 first, and *then* write 0x06?
> 
> More details about this laptop, please?  Also, what is RESET_REG and
> RESET_VALUE defined as?
The RESET_REG is 0xCF9 I/O port.
The RESET_VALUE is 0x06. The above value is obtained from the FADT. This
laptop is the Thinkpad R52 based on Intel i915 chipset. 
Thanks.
> 
> 	-hpa

Re: [PATCH 10/10] x86, ACPI: default to reboot via ACPI (again)

[Zhao Yakui]

On Fri, 2008-11-14 at 09:22 +0800, H. Peter Anvin wrote:
> Zhao Yakui wrote:
> > I do the test as required. First the 0x02 is written to 0xCF9 I/O port.
> > The return value by reading the 0xCF9 I/O port is 0x02. Then the 0x06 is
> > written to the 0xCF9 I/O port. The screen becomes black but the box
> > can't be rebooted.
> 
> What is the value of the CF9 port before you do anything?
The value is 0x0 before writing 0xCF9 I/O port.
Thanks.
> 
> 	-hpa