On Thu, Jan 26, 2012 at 12:16:39AM +0000, Cihula, Joseph wrote: > > From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@xxxxxxxxxx] > > Sent: Thursday, January 12, 2012 12:23 PM > eturn values for tboot_sleep > > > > On Thu, Jan 12, 2012 at 09:49:58AM -0800, Konrad Rzeszutek Wilk wrote: > > > On Tue, Jan 10, 2012 at 08:10:53PM +0000, Cihula, Joseph wrote: > > > > ACK, but tboot_sleep() calls tboot_shutdown() and there are error conditions in that which you > > are not reflecting upward--i.e. you should make tboot_shutdown() return an 'int' and propagate its > > errors through tboot_sleep(). > > > > > > Hey Joe, > > > > > > Thanks for looking at the patches. > > > > > > Right now [with this patch applied] the code looks as so: > > > > > > 297 > > > 298 tboot_shutdown(acpi_shutdown_map[sleep_state]); > > > 299 return 0; > > > 300 } > > > > > > > > > If we do make tboot_shutdown() return an int, there will be a need to > > > modify other callers: native_machine_emergency_restart, > > > native_machine_halt, native_machine_power_off, and native_play_dead as well. > > > > > > Perhaps that could be done in another patch and leave this one as is > > > where the 'tboot_sleep' would just return 0 instead of 'return > > > tboot_shutdown(...)' ? > > > > > > Perhaps another way to do this is to extract the common code of > > > tboot_sleep and tboot_shutdown? > > I'd be happier to see the callers above changed to handle a tboot_shutdown() that returned an error than splitting this up and only checking the error in one path. OK, so something like this then: >From da60a441fd62aee83763758455c08c281893ae93 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> Date: Thu, 8 Dec 2011 17:14:08 +0800 Subject: [PATCH] tboot: Add return values for tboot_sleep and consequently tboot_shutdown .. as appropiately. As tboot_sleep now returns values. remove tboot_sleep_wrapper. Suggested-by: "Rafael J. Wysocki" <rjw@xxxxxxx> Acked-by: Joseph Cihula <joseph.cihula@xxxxxxxxx> [v1: Return -1/0/+1 instead of ACPI_xx values] [v2: Also make tboot_shutdown return error code] Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> --- arch/x86/kernel/reboot.c | 6 +++--- arch/x86/kernel/smpboot.c | 2 +- arch/x86/kernel/tboot.c | 33 +++++++++++++++------------------ include/linux/tboot.h | 4 ++-- 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c index 37a458b..b29326a 100644 --- a/arch/x86/kernel/reboot.c +++ b/arch/x86/kernel/reboot.c @@ -549,7 +549,7 @@ static void native_machine_emergency_restart(void) if (reboot_emergency) emergency_vmx_disable_all(); - tboot_shutdown(TB_SHUTDOWN_REBOOT); + WARN_ON(tboot_shutdown(TB_SHUTDOWN_REBOOT)); /* Tell the BIOS if we want cold or warm reboot */ *((unsigned short *)__va(0x472)) = reboot_mode; @@ -684,7 +684,7 @@ static void native_machine_halt(void) /* stop other cpus and apics */ machine_shutdown(); - tboot_shutdown(TB_SHUTDOWN_HALT); + WARN_ON(tboot_shutdown(TB_SHUTDOWN_HALT)); /* stop this cpu */ stop_this_cpu(NULL); @@ -698,7 +698,7 @@ static void native_machine_power_off(void) pm_power_off(); } /* a fallback in case there is no PM info available */ - tboot_shutdown(TB_SHUTDOWN_HALT); + WARN_ON(tboot_shutdown(TB_SHUTDOWN_HALT)); } struct machine_ops machine_ops = { diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c index 66d250c..9aa903a 100644 --- a/arch/x86/kernel/smpboot.c +++ b/arch/x86/kernel/smpboot.c @@ -1419,7 +1419,7 @@ static inline void hlt_play_dead(void) void native_play_dead(void) { play_dead_common(); - tboot_shutdown(TB_SHUTDOWN_WFS); + WARN_ON(tboot_shutdown(TB_SHUTDOWN_WFS)); mwait_play_dead(); /* Only returns on failure */ hlt_play_dead(); diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index 1a4ab7d..9504a7a 100644 --- a/arch/x86/kernel/tboot.c +++ b/arch/x86/kernel/tboot.c @@ -217,12 +217,12 @@ static int tboot_setup_sleep(void) #endif -void tboot_shutdown(u32 shutdown_type) +int tboot_shutdown(u32 shutdown_type) { void (*shutdown)(void); if (!tboot_enabled()) - return; + return 0; /* * if we're being called before the 1:1 mapping is set up then just @@ -230,13 +230,14 @@ void tboot_shutdown(u32 shutdown_type) * due to very early panic() */ if (!tboot_pg_dir) - return; + return 0; /* if this is S3 then set regions to MAC */ - if (shutdown_type == TB_SHUTDOWN_S3) - if (tboot_setup_sleep()) - return; - + if (shutdown_type == TB_SHUTDOWN_S3) { + int rc = tboot_setup_sleep(); + if (rc) + return rc; + } tboot->shutdown_type = shutdown_type; switch_to_tboot_pt(); @@ -247,6 +248,8 @@ void tboot_shutdown(u32 shutdown_type) /* should not reach here */ while (1) halt(); + + return 0; } static void tboot_copy_fadt(const struct acpi_table_fadt *fadt) @@ -272,7 +275,7 @@ static void tboot_copy_fadt(const struct acpi_table_fadt *fadt) offsetof(struct acpi_table_facs, firmware_waking_vector); } -void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control) +static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control) { static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = { /* S0,1,2: */ -1, -1, -1, @@ -281,7 +284,7 @@ void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control) /* S5: */ TB_SHUTDOWN_S5 }; if (!tboot_enabled()) - return; + return 0; tboot_copy_fadt(&acpi_gbl_FADT); tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control; @@ -292,16 +295,10 @@ void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control) if (sleep_state >= ACPI_S_STATE_COUNT || acpi_shutdown_map[sleep_state] == -1) { pr_warning("unsupported sleep state 0x%x\n", sleep_state); - return; + return -1; } - tboot_shutdown(acpi_shutdown_map[sleep_state]); -} -static int tboot_sleep_wrapper(u8 sleep_state, u32 pm1a_control, - u32 pm1b_control) -{ - tboot_sleep(sleep_state, pm1a_control, pm1b_control); - return 0; + return tboot_shutdown(acpi_shutdown_map[sleep_state]); } static atomic_t ap_wfs_count; @@ -352,7 +349,7 @@ static __init int tboot_late_init(void) atomic_set(&ap_wfs_count, 0); register_hotcpu_notifier(&tboot_cpu_notifier); - acpi_os_set_prepare_sleep(&tboot_sleep_wrapper); + acpi_os_set_prepare_sleep(&tboot_sleep); return 0; } diff --git a/include/linux/tboot.h b/include/linux/tboot.h index c75128b..8f14fe0 100644 --- a/include/linux/tboot.h +++ b/include/linux/tboot.h @@ -142,7 +142,7 @@ static inline int tboot_enabled(void) } extern void tboot_probe(void); -extern void tboot_shutdown(u32 shutdown_type); +extern int tboot_shutdown(u32 shutdown_type); extern struct acpi_table_header *tboot_get_dmar_table( struct acpi_table_header *dmar_tbl); extern int tboot_force_iommu(void); @@ -151,7 +151,7 @@ extern int tboot_force_iommu(void); #define tboot_enabled() 0 #define tboot_probe() do { } while (0) -#define tboot_shutdown(shutdown_type) do { } while (0) +#define tboot_shutdown(shutdown_type) 0 #define tboot_sleep(sleep_state, pm1a_control, pm1b_control) \ do { } while (0) #define tboot_get_dmar_table(dmar_tbl) (dmar_tbl) -- 1.7.7.5 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
