Skip to contentSkip to conversation

ACPI patchese on test branch

PATCH NAK Len Brown 10 participants 52 messages Nov 7, 2008 – Nov 14, 2008

Subsystems (8)
Documentation x86 ACPI PCI +4 more subsystems
lore
Share & tools
lore

Len Brown

author reviewer
#0 lore raw

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

author reviewer
#1 lore raw

PATCH 1/10 ACPI: pci_link: remove acpi_irq_balance_set() interface

From: Bjorn Helgaas <[email protected]>

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.

---
 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(-)
arch/x86/include/asm/acpi.h +0 -1
index 8d676d8..9830681 100644
@@ -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);
arch/x86/kernel/acpi/boot.c +0 -1
index 8c1f76a..4c51a2f 100644
@@ -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;
drivers/acpi/pci_link.c +9 -2
index fcfdef7..e52ad91 100644
@@ -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);
 
Signature · 1 line
1.5.6.5
author reviewer
#2 lore raw

PATCH 2/10 ACPI: Disambiguate processor declaration type

In reply to Len Brown

From: Myron Stowe <[email protected]>

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.

---
 drivers/acpi/processor_core.c |    1 +
 drivers/acpi/scan.c           |    2 +-
 include/acpi/acpi_drivers.h   |    1 +
 3 files changed, 3 insertions(+), 1 deletions(-)
drivers/acpi/processor_core.c +1 -0
index 24a362f..0c670dd 100644
@@ -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},
 };
drivers/acpi/scan.c +1 -1
index a9dda8e..3fb6e2d 100644
@@ -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;
include/acpi/acpi_drivers.h +1 -0
index cf04c60..7469ff3 100644
@@ -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"
Signature · 1 line
1.5.6.5
author reviewer
#3 lore raw

PATCH 3/10 ACPI: Behave uniquely based on processor declaration definition type

In reply to Len Brown

From: Myron Stowe <[email protected]>

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.

---
 drivers/acpi/processor_core.c |   78 +++++++++++++++++++++++------------------
 1 files changed, 44 insertions(+), 34 deletions(-)
drivers/acpi/processor_core.c +44 -34
index 0c670dd..bc332fc 100644
@@ -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;
Signature · 1 line
1.5.6.5
author reviewer
#4 lore raw

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

In reply to Len Brown

From: Myron Stowe <[email protected]>

---
 drivers/acpi/processor_core.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
drivers/acpi/processor_core.c +4 -3
index bc332fc..b57b1f0 100644
@@ -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);
Signature · 1 line
1.5.6.5
author reviewer
#5 lore raw

PATCH 5/10 Hibernate: Call platform_begin before swsusp_shrink_memory

In reply to Len Brown

From: Zhang Rui <[email protected]>

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

---
 kernel/power/disk.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
kernel/power/disk.c +3 -3
index c9d7408..096fe48 100644
@@ -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;
 
Signature · 1 line
1.5.6.5
author reviewer
#6 lore raw

PATCH 6/10 ACPI hibernate: Add a mechanism to save/restore ACPI NVS memory

In reply to Len Brown

From: Rafael J. Wysocki <[email protected]>

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.

---
 drivers/acpi/sleep/main.c |   53 +++++++++++++++++---
 include/linux/suspend.h   |   13 +++++
 kernel/power/swsusp.c     |  122 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 180 insertions(+), 8 deletions(-)
drivers/acpi/sleep/main.c +45 -8
index 80c0868..f861b73 100644
@@ -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,
include/linux/suspend.h +13 -0
index 2ce8207..2b409c4 100644
@@ -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
kernel/power/swsusp.c +122 -0

125 lines not on this page · raw message

Signature · 1 line
1.5.6.5
author reviewer
#7 lore raw

PATCH 7/10 x86 hibernate: Mark ACPI NVS memory region at startup

In reply to Len Brown

From: Rafael J. Wysocki <[email protected]>

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.

---
 arch/x86/kernel/e820.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)
arch/x86/kernel/e820.c +21 -0
index 7aafeb5..74c6a21 100644
@@ -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.
  */
Signature · 1 line
1.5.6.5
author reviewer
#8 lore raw

PATCH 8/10 ACPI hibernate: Introduce new kernel parameter acpi_sleep=s4_nonvs

In reply to Len Brown

From: Rafael J. Wysocki <[email protected]>

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.

---
 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(-)
Documentation/kernel-parameters.txt +4 -1
index 1bbcaa8..65bc2bc 100644
@@ -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 }
arch/x86/kernel/acpi/sleep.c +2 -0
index 806b4e9..707c1f6 100644
@@ -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");
drivers/acpi/sleep/main.c +17 -2
index f861b73..63bff78 100644
@@ -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;
 	}
include/linux/acpi.h +1 -0
index fd6a452..4008f61 100644
@@ -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 */
 
Signature · 1 line
1.5.6.5
author reviewer
#9 lore raw

PATCH 9/10 compal-laptop: use rfkill switch subsystem

In reply to Len Brown

From: Cezary Jackiewicz <[email protected]>

Remove unnecessary attributes, use rfkill switch subsystem.

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

---
 drivers/misc/Kconfig         |    2 +
 drivers/misc/compal-laptop.c |  270 +++++++++++++++++++-----------------------
 2 files changed, 124 insertions(+), 148 deletions(-)
drivers/misc/Kconfig +2 -0
index 9494400..fbfd86d 100644
@@ -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:
 
drivers/misc/compal-laptop.c +122 -148

376 lines not on this page · raw message

Signature · 1 line
1.5.6.5
author reviewer
#10 lore raw

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

In reply to Len Brown

From: Len Brown <[email protected]>

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.

---
 arch/x86/kernel/reboot.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/x86/kernel/reboot.c +1 -1
index 724adfc..f72f71c 100644
@@ -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)
Signature · 1 line
1.5.6.5
#11 lore raw

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

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

Quoted 12 lines · Len Brown

From: Len Brown <[email protected]>

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.

---
 arch/x86/kernel/reboot.c |    2 +-

arch/x86/kernel/reboot.c EXPORT_SYMBOL(pm_power_off); · line 32

Quoted 2 lines
arch/x86/kernel/reboot.c +1 -1
-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 <[email protected]>
| 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

author reviewer
#12 lore raw

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

Quoted 7 lines

arch/x86/kernel/reboot.c EXPORT_SYMBOL(pm_power_off); · line 32

Quoted 4 lines
arch/x86/kernel/reboot.c +1 -1
-enum reboot_type reboot_type = BOOT_KBD;
+enum reboot_type reboot_type = BOOT_ACPI;

NAK.

Quoted 7 lines

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

#13 lore raw

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

Quoted 10 lines · Len Brown
Quoted 10 lines

arch/x86/kernel/reboot.c EXPORT_SYMBOL(pm_power_off); · line 32

Quoted 3 lines
arch/x86/kernel/reboot.c +1 -1
-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.

Quoted 6 lines
Quoted 3 lines

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

Quoted 8 lines

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

author reviewer
#14 lore raw

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

Quoted 6 lines
Quoted 4 lines

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

#15 lore raw

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

Quoted 4 lines · Len Brown
Quoted 4 lines
Quoted 3 lines

If you can direct me to the failures that ACPI reset caused in 28, that would be great.

Quoted 8 lines
Quoted 1 line

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 :)

Quoted 4 lines

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

#16 lore raw

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

Quoted 9 lines · Andrey Borzenkov
Quoted 4 lines

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

#17 lore raw

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

Quoted 3 lines · Ingo Molnar

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?

Quoted 1 line

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.

Signature · 1 line
error compiling committee.c: too many arguments to function
#18 lore raw

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

Quoted 5 lines · Avi Kivity

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

#19 lore raw

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

Quoted 11 lines · H. Peter Anvin
Quoted 5 lines · Avi Kivity

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

#20 lore raw

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

Quoted 10 lines · Ingo Molnar
Quoted 7 lines · H. Peter Anvin
Quoted 5 lines · Avi Kivity

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.

Quoted 8 lines
Quoted 2 lines

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.

Quoted 4 lines

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.

Quoted 8 lines

... 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).

Signature · 1 line
error compiling committee.c: too many arguments to function
#21 lore raw

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

Quoted 17 lines · Avi Kivity
Quoted 16 lines · Ingo Molnar
Quoted 12 lines · H. Peter Anvin
Quoted 5 lines · Avi Kivity

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.)

Quoted 7 lines
Quoted 5 lines

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.

Quoted 8 lines
Quoted 3 lines

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

#22 lore raw

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

Quoted 6 lines · Ingo Molnar

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

#23 lore raw

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

Quoted 13 lines · H. Peter Anvin
Quoted 6 lines · Ingo Molnar

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.

Quoted 5 lines

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

#24 lore raw

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

Quoted 21 lines · Eric W. Biederman
Quoted 13 lines · H. Peter Anvin
Quoted 6 lines · Ingo Molnar

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.

Quoted 5 lines

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.

Quoted 3 lines

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

#25 lore raw

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

In reply to Avi Kivity
Quoted 3 lines · Avi Kivity

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.

Signature · 1 line
Matthew Garrett | [email protected]
#26 lore raw

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

In reply to Ingo Molnar
Quoted 3 lines · Ingo Molnar

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.

Signature · 1 line
Matthew Garrett | [email protected]
#27 lore raw

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

Quoted 6 lines · Matthew Garrett
Quoted 3 lines · Ingo Molnar

so, the sequence should be:

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

Quoted 8 lines

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

#28 lore raw

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

Quoted 9 lines · Ingo Molnar
Quoted 3 lines · Matthew Garrett

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.

Signature · 1 line
Matthew Garrett | [email protected]
author reviewer
#29 lore raw

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

Quoted 6 lines · Matthew Garrett
Quoted 5 lines · Ingo Molnar
Quoted 3 lines · Matthew Garrett

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.

Quoted 7 lines
Quoted 4 lines

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

#30 lore raw

Attempt rebooting via port CF9 if it seems to be available

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

Quoted 1 line

From 14d7ca5c575853664d8fe4f225a77b8df1b7de7d Mon Sep 17 00:00:00 2001

From: H. Peter Anvin <[email protected]>
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.

---
 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(-)
arch/x86/include/asm/emergency-restart.h +3 -1
index 94826cf..cc70c1c 100644
@@ -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;
arch/x86/kernel/reboot.c +27 -7
index 34f8d37..ddc9389 100644
@@ -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;
 		}
arch/x86/pci/direct.c +3 -1
index 9915293..9a5af6c 100644
@@ -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;
 	}
 
arch/x86/pci/pci.h +1 -0
index 15b9cf6..1959018 100644
@@ -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);
Signature · 1 line
1.5.6.5
#31 lore raw

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

Quoted 8 lines · H. Peter Anvin

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.

Quoted 9 lines

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.

#32 lore raw

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

In reply to Len Brown
Quoted 5 lines · Len Brown
Quoted 2 lines · Matthew Garrett

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?

Quoted 3 lines

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?

Signature · 1 line
Matthew Garrett | [email protected]
#33 lore raw

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

In reply to Len Brown
Quoted 24 lines · Len Brown
Quoted 6 lines · Matthew Garrett
Quoted 5 lines · Ingo Molnar
Quoted 3 lines · Matthew Garrett

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.

Quoted 7 lines
Quoted 4 lines

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.

Quoted 8 lines

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?

Quoted 6 lines

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

#34 lore raw

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

Quoted 9 lines · Ingo Molnar
Quoted 9 lines

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.

Quoted 9 lines
Quoted 6 lines

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.

Signature · 1 line
error compiling committee.c: too many arguments to function
#35 lore raw

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

Quoted 18 lines · Avi Kivity
Quoted 17 lines · Ingo Molnar
Quoted 16 lines

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.

Quoted 6 lines
Quoted 2 lines

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.

Quoted 2 lines
#36 lore raw

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

Quoted 8 lines · Zhao Yakui

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.

Quoted 5 lines
    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

#37 lore raw

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

Quoted 15 lines · H. Peter Anvin
Quoted 8 lines · Zhao Yakui

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.

Quoted 5 lines
    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.

Quoted 4 lines

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.

Quoted 2 lines

-hpa

#38 lore raw

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

In reply to Zhao Yakui
Quoted 5 lines · Zhao Yakui

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.

Quoted 7 lines

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

#39 lore raw

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

Quoted 12 lines · Eric W. Biederman
Quoted 5 lines · Zhao Yakui

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.

Quoted 6 lines

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.

Quoted 24 lines
Quoted 8 lines

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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#40 lore raw

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

Quoted 8 lines · Zhao Yakui

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

#41 lore raw

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

In reply to Zhao Yakui
Quoted 5 lines · Zhao Yakui
Quoted 2 lines

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?

Quoted 5 lines

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.

Quoted 2 lines

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

#42 lore raw

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

Quoted 11 lines · Eric W. Biederman
Quoted 5 lines · Zhao Yakui
Quoted 2 lines

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.

Quoted 10 lines
Quoted 6 lines

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.
Quoted 17 lines

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.

Quoted 2 lines

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

#43 lore raw

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

Quoted 5 lines · Zhao Yakui

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);

Quoted 8 lines

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

#44 lore raw

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

Quoted 7 lines · H. Peter Anvin
Quoted 5 lines · Zhao Yakui

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.

Quoted 16 lines

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);

Quoted 8 lines

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.

Quoted 4 lines

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.

Quoted 2 lines

-hpa

#45 lore raw

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

Quoted 4 lines · Zhao Yakui

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

#46 lore raw

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

Quoted 7 lines · H. Peter Anvin
Quoted 4 lines · Zhao Yakui

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.

Quoted 2 lines

-hpa

#47 lore raw

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

In reply to Zhao Yakui
Quoted 5 lines · Zhao Yakui
Quoted 2 lines

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?

Quoted 5 lines

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.

Quoted 2 lines

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

#48 lore raw

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

In reply to Zhao Yakui
Quoted 5 lines · Zhao Yakui

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.

Quoted 7 lines

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

#49 lore raw

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

Quoted 11 lines · Eric W. Biederman

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.)

Quoted 6 lines

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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#50 lore raw

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

In reply to Ingo Molnar

Hi Len,

Quoted 13 lines · On Wed, 12 Nov 2008 12:58:06 +0100 Ingo Molnar <[email protected]> wrote:
Quoted 6 lines · Len Brown

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.

Quoted 6 lines

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.

Signature · 3 lines
Cheers,
Stephen Rothwell                    [email protected]
http://www.canb.auug.org.au/~sfr/
#51 lore raw

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

In reply to Len Brown
Quoted 10 lines · Len Brown
Quoted 10 lines

arch/x86/kernel/reboot.c EXPORT_SYMBOL(pm_power_off); · line 32

Quoted 3 lines
arch/x86/kernel/reboot.c +1 -1
-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.

Quoted 8 lines
Quoted 3 lines

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