* Re: [PATCH 2/7] x86, acpi, tboot: Have a ACPI sleep override instead of calling tboot_sleep.
From: Cihula, Joseph @ 2011-09-07 4:20 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, x86@kernel.org, tglx@linutronix.de,
tboot-devel
Cc: xen-devel@lists.xensource.com
In-Reply-To: <1314815484-4668-3-git-send-email-konrad.wilk@oracle.com>
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Wednesday, August 31, 2011 11:31 AM
>
> The ACPI suspend path makes a call to tboot_sleep right before it writes the PM1A, PM1B values. We
> replace the direct call to tboot via an registration callback similar to __acpi_register_gsi.
>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: x86@kernel.org
> CC: Len Brown <len.brown@intel.com>
> CC: Joseph Cihula <joseph.cihula@intel.com>
> CC: Shane Wang <shane.wang@intel.com>
> CC: xen-devel@lists.xensource.com
> CC: linux-pm@lists.linux-foundation.org
> CC: tboot-devel@lists.sourceforge.net
> CC: linux-acpi@vger.kernel.org
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> arch/x86/include/asm/acpi.h | 3 +++
> arch/x86/kernel/acpi/boot.c | 3 +++
> arch/x86/kernel/tboot.c | 13 +++++++++----
> drivers/acpi/acpica/hwsleep.c | 12 ++++++++++--
> include/linux/tboot.h | 3 ++-
> 5 files changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h index 610001d..49864a1
> 100644
> --- a/arch/x86/include/asm/acpi.h
> +++ b/arch/x86/include/asm/acpi.h
> @@ -98,6 +98,9 @@ void acpi_pic_sci_set_trigger(unsigned int, u16); extern int
> (*__acpi_register_gsi)(struct device *dev, u32 gsi,
> int trigger, int polarity);
>
> +extern int (*__acpi_override_sleep)(u8 sleep_state, u32 pm1a_ctrl,
> + u32 pm1b_ctrl, bool *skip_rest);
> +
> static inline void disable_acpi(void)
> {
> acpi_disabled = 1;
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index 4558f0d..d191b4c
> 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -552,6 +552,9 @@ static int acpi_register_gsi_ioapic(struct device *dev, u32 gsi, int
> (*__acpi_register_gsi)(struct device *dev, u32 gsi,
> int trigger, int polarity) = acpi_register_gsi_pic;
>
> +int (*__acpi_override_sleep)(u8 sleep_state, u32 pm1a_ctrl,
> + u32 pm1b_ctrl, bool *skip_rest) = NULL;
> +
> /*
> * success: return IRQ number (>=0)
> * failure: return < 0
> diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index 30ac65d..a18070c 100644
> --- a/arch/x86/kernel/tboot.c
> +++ b/arch/x86/kernel/tboot.c
> @@ -41,7 +41,7 @@
> #include <asm/setup.h>
> #include <asm/e820.h>
> #include <asm/io.h>
> -
> +#include <linux/acpi.h>
> #include "acpi/realmode/wakeup.h"
>
> /* Global pointer to shared data; NULL means no measured launch. */ @@ -270,7 +270,8 @@ 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)
> +int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
> + bool *skip_rest)
Don't you need to use the 'unused' attrib on skip_rest in order to prevent compiler warnings?
> {
> static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
> /* S0,1,2: */ -1, -1, -1,
> @@ -279,7 +280,7 @@ void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
> /* S5: */ TB_SHUTDOWN_S5 };
>
> if (!tboot_enabled())
> - return;
> + return AE_OK;
>
> tboot_copy_fadt(&acpi_gbl_FADT);
> tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control; @@ -290,10 +291,12 @@ 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 AE_ERROR;
> }
>
> tboot_shutdown(acpi_shutdown_map[sleep_state]);
> +
> + return AE_OK;
> }
>
> static atomic_t ap_wfs_count;
> @@ -343,6 +346,8 @@ static __init int tboot_late_init(void)
>
> atomic_set(&ap_wfs_count, 0);
> register_hotcpu_notifier(&tboot_cpu_notifier);
> +
> + __acpi_override_sleep = tboot_sleep;
> return 0;
> }
>
> diff --git a/drivers/acpi/acpica/hwsleep.c b/drivers/acpi/acpica/hwsleep.c index 2ac28bb..31d1198
> 100644
> --- a/drivers/acpi/acpica/hwsleep.c
> +++ b/drivers/acpi/acpica/hwsleep.c
> @@ -45,7 +45,6 @@
> #include <acpi/acpi.h>
> #include "accommon.h"
> #include "actables.h"
> -#include <linux/tboot.h>
>
> #define _COMPONENT ACPI_HARDWARE
> ACPI_MODULE_NAME("hwsleep")
> @@ -343,8 +342,17 @@ acpi_status asmlinkage acpi_enter_sleep_state(u8 sleep_state)
>
> ACPI_FLUSH_CPU_CACHE();
>
> - tboot_sleep(sleep_state, pm1a_control, pm1b_control);
> + if (__acpi_override_sleep) {
> + bool skip_rest = false;
>
> + status = __acpi_override_sleep(sleep_state, pm1a_control,
> + pm1b_control, &skip_rest);
> +
> + if (ACPI_FAILURE(status))
> + return_ACPI_STATUS(status);
> + if (skip_rest)
> + return_ACPI_STATUS(AE_OK);
> + }
> /* Write #2: Write both SLP_TYP + SLP_EN */
>
> status = acpi_hw_write_pm1_control(pm1a_control, pm1b_control); diff --git
> a/include/linux/tboot.h b/include/linux/tboot.h index 1dba6ee..19badbd 100644
> --- a/include/linux/tboot.h
> +++ b/include/linux/tboot.h
> @@ -143,7 +143,8 @@ static inline int tboot_enabled(void)
>
> extern void tboot_probe(void);
> extern void tboot_shutdown(u32 shutdown_type); -extern void tboot_sleep(u8 sleep_state, u32
> pm1a_control, u32 pm1b_control);
> +extern int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
> + bool *skip);
> extern struct acpi_table_header *tboot_get_dmar_table(
> struct acpi_table_header *dmar_tbl); extern int
> tboot_force_iommu(void);
> --
> 1.7.4.1
^ permalink raw reply
* Re: [PATCH 6/7] xen/acpi/sleep: Enable ACPI sleep via the __acpi_override_sleep
From: Cihula, Joseph @ 2011-09-07 4:36 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, x86@kernel.org, tglx@linutronix.de,
tboot-devel
Cc: xen-devel@lists.xensource.com
In-Reply-To: <1314815484-4668-7-git-send-email-konrad.wilk@oracle.com>
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Wednesday, August 31, 2011 11:31 AM
>
> Provide the registration callback to call in the Xen's ACPI sleep functionality. This means that
> during S3/S5 we make a hypercall XENPF_enter_acpi_sleep with the proper PM1A/PM1B registers.
>
> Based of Ke Yu's <ke.yu@intel.com> initial idea.
> [ From http://xenbits.xensource.com/linux-2.6.18-xen.hg
> change c68699484a65 ]
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> arch/x86/include/asm/xen/hypercall.h | 8 ++++++++
> arch/x86/xen/enlighten.c | 3 +++
> drivers/xen/Makefile | 2 +-
> drivers/xen/acpi.c | 25 +++++++++++++++++++++++++
> include/xen/acpi.h | 26 ++++++++++++++++++++++++++
> 5 files changed, 63 insertions(+), 1 deletions(-) create mode 100644 drivers/xen/acpi.c create
> mode 100644 include/xen/acpi.h
>
> diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h
> index d240ea9..0c9894e 100644
> --- a/arch/x86/include/asm/xen/hypercall.h
> +++ b/arch/x86/include/asm/xen/hypercall.h
> @@ -45,6 +45,7 @@
> #include <xen/interface/xen.h>
> #include <xen/interface/sched.h>
> #include <xen/interface/physdev.h>
> +#include <xen/interface/platform.h>
>
> /*
> * The hypercall asms have to meet several constraints:
> @@ -299,6 +300,13 @@ HYPERVISOR_set_timer_op(u64 timeout) }
>
> static inline int
> +HYPERVISOR_dom0_op(struct xen_platform_op *platform_op) {
> + platform_op->interface_version = XENPF_INTERFACE_VERSION;
> + return _hypercall1(int, dom0_op, platform_op); }
> +
> +static inline int
> HYPERVISOR_set_debugreg(int reg, unsigned long value) {
> return _hypercall2(int, set_debugreg, reg, value); diff --git a/arch/x86/xen/enlighten.c
> b/arch/x86/xen/enlighten.c index 5525163..6962653 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -42,6 +42,7 @@
> #include <xen/page.h>
> #include <xen/hvm.h>
> #include <xen/hvc-console.h>
> +#include <xen/acpi.h>
>
> #include <asm/paravirt.h>
> #include <asm/apic.h>
> @@ -1250,6 +1251,8 @@ asmlinkage void __init xen_start_kernel(void)
> } else {
> /* Make sure ACS will be enabled */
> pci_request_acs();
> +
> + xen_acpi_sleep_register();
> }
>
>
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile index bbc1825..370552d 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -16,7 +16,7 @@ obj-$(CONFIG_XENFS) += xenfs/
> obj-$(CONFIG_XEN_SYS_HYPERVISOR) += sys-hypervisor.o
> obj-$(CONFIG_XEN_PLATFORM_PCI) += xen-platform-pci.o
> obj-$(CONFIG_SWIOTLB_XEN) += swiotlb-xen.o
> -obj-$(CONFIG_XEN_DOM0) += pci.o
> +obj-$(CONFIG_XEN_DOM0) += pci.o acpi.o
>
> xen-evtchn-y := evtchn.o
> xen-gntdev-y := gntdev.o
> diff --git a/drivers/xen/acpi.c b/drivers/xen/acpi.c new file mode 100644 index 0000000..c0f829f
> --- /dev/null
> +++ b/drivers/xen/acpi.c
> @@ -0,0 +1,25 @@
Copyright and license?
> +#include <xen/acpi.h>
> +#include <xen/interface/platform.h>
> +#include <asm/xen/hypercall.h>
> +#include <asm/xen/hypervisor.h>
> +
> +int xen_acpi_notify_hypervisor_state(u8 sleep_state,
> + u32 pm1a_cnt, u32 pm1b_cnt,
> + bool *skip_rest)
> +{
> + struct xen_platform_op op = {
> + .cmd = XENPF_enter_acpi_sleep,
> + .interface_version = XENPF_INTERFACE_VERSION,
> + .u = {
> + .enter_acpi_sleep = {
> + .pm1a_cnt_val = (u16)pm1a_cnt,
> + .pm1b_cnt_val = (u16)pm1b_cnt,
> + .sleep_state = sleep_state,
> + },
> + },
> + };
> + if (skip_rest)
> + *skip_rest = true;
> +
> + return HYPERVISOR_dom0_op(&op);
> +}
> diff --git a/include/xen/acpi.h b/include/xen/acpi.h new file mode 100644 index 0000000..e414f14
> --- /dev/null
> +++ b/include/xen/acpi.h
> @@ -0,0 +1,26 @@
Copyright and license?
> +#ifndef _XEN_ACPI_H
> +#define _XEN_ACPI_H
> +
> +#include <linux/types.h>
> +
> +#ifdef CONFIG_XEN_DOM0
> +#include <asm/xen/hypervisor.h>
> +#include <xen/xen.h>
> +#include <linux/acpi.h>
> +
> +int xen_acpi_notify_hypervisor_state(u8 sleep_state,
> + u32 pm1a_cnt, u32 pm1b_cnd,
> + bool *skip_rest);
> +
> +static inline void xen_acpi_sleep_register(void) {
> + if (xen_initial_domain())
> + __acpi_override_sleep = xen_acpi_notify_hypervisor_state; } #else
> +static inline void xen_acpi_sleep_register(void) { } #endif
> +
> +#endif /* _XEN_ACPI_H */
> --
> 1.7.4.1
^ permalink raw reply
* Re: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall
From: Cihula, Joseph @ 2011-09-07 5:50 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, x86@kernel.org, tglx@linutronix.de,
tboot-devel
Cc: xen-devel@lists.xensource.com, Jeremy Fitzhardinge
In-Reply-To: <1314815484-4668-6-git-send-email-konrad.wilk@oracle.com>
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Wednesday, August 31, 2011 11:31 AM
>
> From: Yu Ke <ke.yu@intel.com>
>
> This patches implements the xen_platform_op hypercall, to pass the parsed ACPI info to hypervisor.
>
> Signed-off-by: Yu Ke <ke.yu@intel.com>
> Signed-off-by: Tian Kevin <kevin.tian@intel.com>
> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> [v1: Added DEFINE_GUEST.. in appropiate headers]
> [v2: Ripped out typedefs]
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> arch/ia64/include/asm/xen/interface.h | 1 +
> arch/x86/include/asm/xen/interface.h | 1 +
> include/xen/interface/platform.h | 320 +++++++++++++++++++++++++++++++++
> include/xen/interface/xen.h | 1 +
> 4 files changed, 323 insertions(+), 0 deletions(-) create mode 100644
> include/xen/interface/platform.h
>
> diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
> index e951e74..1d2427d 100644
> --- a/arch/ia64/include/asm/xen/interface.h
> +++ b/arch/ia64/include/asm/xen/interface.h
> @@ -76,6 +76,7 @@ DEFINE_GUEST_HANDLE(char); DEFINE_GUEST_HANDLE(int);
> DEFINE_GUEST_HANDLE(long); DEFINE_GUEST_HANDLE(void);
> +DEFINE_GUEST_HANDLE(uint64_t);
>
> typedef unsigned long xen_pfn_t;
> DEFINE_GUEST_HANDLE(xen_pfn_t);
> diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
> index 5d4922a..a1f2db5 100644
> --- a/arch/x86/include/asm/xen/interface.h
> +++ b/arch/x86/include/asm/xen/interface.h
> @@ -55,6 +55,7 @@ DEFINE_GUEST_HANDLE(char); DEFINE_GUEST_HANDLE(int);
> DEFINE_GUEST_HANDLE(long); DEFINE_GUEST_HANDLE(void);
> +DEFINE_GUEST_HANDLE(uint64_t);
> #endif
>
> #ifndef HYPERVISOR_VIRT_START
> diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
> new file mode 100644
> index 0000000..c168468
> --- /dev/null
> +++ b/include/xen/interface/platform.h
Why are you adding so many new hypercalls that aren't being used in this patchset?
> @@ -0,0 +1,320 @@
> +/**********************************************************************
> +********
> + * platform.h
> + *
> + * Hardware platform operations. Intended for use by domain-0 kernel.
> + *
> + * Permission is hereby granted, free of charge, to any person
> +obtaining a copy
> + * of this software and associated documentation files (the
> +"Software"), to
> + * deal in the Software without restriction, including without
> +limitation the
> + * rights to use, copy, modify, merge, publish, distribute, sublicense,
> +and/or
> + * sell copies of the Software, and to permit persons to whom the
> +Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> +included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> +SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> +OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> +ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * Copyright (c) 2002-2006, K Fraser
> + */
> +
> +#ifndef __XEN_PUBLIC_PLATFORM_H__
> +#define __XEN_PUBLIC_PLATFORM_H__
> +
> +#include "xen.h"
> +
> +#define XENPF_INTERFACE_VERSION 0x03000001
> +
> +/*
> + * Set clock such that it would read <secs,nsecs> after 00:00:00 UTC,
> + * 1 January, 1970 if the current system time was <system_time>.
> + */
> +#define XENPF_settime 17
> +struct xenpf_settime {
> + /* IN variables. */
> + uint32_t secs;
> + uint32_t nsecs;
> + uint64_t system_time;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime_t);
> +
> +/*
> + * Request memory range (@mfn, @mfn+@nr_mfns-1) to have type @type.
> + * On x86, @type is an architecture-defined MTRR memory type.
> + * On success, returns the MTRR that was used (@reg) and a handle that
> +can
> + * be passed to XENPF_DEL_MEMTYPE to accurately tear down the new setting.
> + * (x86-specific).
> + */
> +#define XENPF_add_memtype 31
> +struct xenpf_add_memtype {
> + /* IN variables. */
> + unsigned long mfn;
> + uint64_t nr_mfns;
> + uint32_t type;
> + /* OUT variables. */
> + uint32_t handle;
> + uint32_t reg;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_add_memtype_t);
> +
> +/*
> + * Tear down an existing memory-range type. If @handle is remembered
> +then it
> + * should be passed in to accurately tear down the correct setting (in
> +case
> + * of overlapping memory regions with differing types). If it is not
> +known
> + * then @handle should be set to zero. In all cases @reg must be set.
> + * (x86-specific).
> + */
> +#define XENPF_del_memtype 32
> +struct xenpf_del_memtype {
> + /* IN variables. */
> + uint32_t handle;
> + uint32_t reg;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_del_memtype_t);
> +
> +/* Read current type of an MTRR (x86-specific). */
> +#define XENPF_read_memtype 33
> +struct xenpf_read_memtype {
> + /* IN variables. */
> + uint32_t reg;
> + /* OUT variables. */
> + unsigned long mfn;
> + uint64_t nr_mfns;
> + uint32_t type;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_read_memtype_t);
> +
> +#define XENPF_microcode_update 35
> +struct xenpf_microcode_update {
> + /* IN variables. */
> + GUEST_HANDLE(void) data; /* Pointer to microcode data */
> + uint32_t length; /* Length of microcode data. */
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_microcode_update_t);
> +
> +#define XENPF_platform_quirk 39
> +#define QUIRK_NOIRQBALANCING 1 /* Do not restrict IO-APIC RTE targets */
> +#define QUIRK_IOAPIC_BAD_REGSEL 2 /* IO-APIC REGSEL forgets its value */
> +#define QUIRK_IOAPIC_GOOD_REGSEL 3 /* IO-APIC REGSEL behaves properly */
> +struct xenpf_platform_quirk {
> + /* IN variables. */
> + uint32_t quirk_id;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_platform_quirk_t);
> +
> +#define XENPF_firmware_info 50
> +#define XEN_FW_DISK_INFO 1 /* from int 13 AH=08/41/48 */
> +#define XEN_FW_DISK_MBR_SIGNATURE 2 /* from MBR offset 0x1b8 */
> +#define XEN_FW_VBEDDC_INFO 3 /* from int 10 AX=4f15 */
> +struct xenpf_firmware_info {
> + /* IN variables. */
> + uint32_t type;
> + uint32_t index;
> + /* OUT variables. */
> + union {
> + struct {
> + /* Int13, Fn48: Check Extensions Present. */
> + uint8_t device; /* %dl: bios device number */
> + uint8_t version; /* %ah: major version */
> + uint16_t interface_support; /* %cx: support bitmap */
> + /* Int13, Fn08: Legacy Get Device Parameters. */
> + uint16_t legacy_max_cylinder; /* %cl[7:6]:%ch: max cyl # */
> + uint8_t legacy_max_head; /* %dh: max head # */
> + uint8_t legacy_sectors_per_track; /* %cl[5:0]: max sector # */
> + /* Int13, Fn41: Get Device Parameters (as filled into %ds:%esi). */
> + /* NB. First uint16_t of buffer must be set to buffer size. */
> + GUEST_HANDLE(void) edd_params;
> + } disk_info; /* XEN_FW_DISK_INFO */
> + struct {
> + uint8_t device; /* bios device number */
> + uint32_t mbr_signature; /* offset 0x1b8 in mbr */
> + } disk_mbr_signature; /* XEN_FW_DISK_MBR_SIGNATURE */
> + struct {
> + /* Int10, AX=4F15: Get EDID info. */
> + uint8_t capabilities;
> + uint8_t edid_transfer_time;
> + /* must refer to 128-byte buffer */
> + GUEST_HANDLE(uchar) edid;
> + } vbeddc_info; /* XEN_FW_VBEDDC_INFO */
> + } u;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmware_info_t);
> +
> +#define XENPF_enter_acpi_sleep 51
> +struct xenpf_enter_acpi_sleep {
> + /* IN variables */
> + uint16_t pm1a_cnt_val; /* PM1a control value. */
> + uint16_t pm1b_cnt_val; /* PM1b control value. */
These are uint32_t in native Linux--why truncate in the API and not at use?
> + uint32_t sleep_state; /* Which state to enter (Sn). */
> + uint32_t flags; /* Must be zero. */
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_enter_acpi_sleep_t);
> +
> +#define XENPF_change_freq 52
> +struct xenpf_change_freq {
> + /* IN variables */
> + uint32_t flags; /* Must be zero. */
> + uint32_t cpu; /* Physical cpu. */
> + uint64_t freq; /* New frequency (Hz). */ };
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_change_freq_t);
> +
> +/*
> + * Get idle times (nanoseconds since boot) for physical CPUs specified
> +in the
> + * @cpumap_bitmap with range [0..@cpumap_nr_cpus-1]. The @idletime
> +array is
> + * indexed by CPU number; only entries with the corresponding
> +@cpumap_bitmap
> + * bit set are written to. On return, @cpumap_bitmap is modified so
> +that any
> + * non-existent CPUs are cleared. Such CPUs have their @idletime array
> +entry
> + * cleared.
> + */
> +#define XENPF_getidletime 53
> +struct xenpf_getidletime {
> + /* IN/OUT variables */
> + /* IN: CPUs to interrogate; OUT: subset of IN which are present */
> + GUEST_HANDLE(uchar) cpumap_bitmap;
> + /* IN variables */
> + /* Size of cpumap bitmap. */
> + uint32_t cpumap_nr_cpus;
> + /* Must be indexable for every cpu in cpumap_bitmap. */
> + GUEST_HANDLE(uint64_t) idletime;
> + /* OUT variables */
> + /* System time when the idletime snapshots were taken. */
> + uint64_t now;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_getidletime_t);
> +
> +#define XENPF_set_processor_pminfo 54
> +
> +/* ability bits */
> +#define XEN_PROCESSOR_PM_CX 1
> +#define XEN_PROCESSOR_PM_PX 2
> +#define XEN_PROCESSOR_PM_TX 4
> +
> +/* cmd type */
> +#define XEN_PM_CX 0
> +#define XEN_PM_PX 1
> +#define XEN_PM_TX 2
> +
> +/* Px sub info type */
> +#define XEN_PX_PCT 1
> +#define XEN_PX_PSS 2
> +#define XEN_PX_PPC 4
> +#define XEN_PX_PSD 8
> +
> +struct xen_power_register {
> + uint32_t space_id;
> + uint32_t bit_width;
> + uint32_t bit_offset;
> + uint32_t access_size;
> + uint64_t address;
> +};
> +
> +struct xen_processor_csd {
> + uint32_t domain; /* domain number of one dependent group */
> + uint32_t coord_type; /* coordination type */
> + uint32_t num; /* number of processors in same domain */
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xen_processor_csd);
> +
> +struct xen_processor_cx {
> + struct xen_power_register reg; /* GAS for Cx trigger register */
> + uint8_t type; /* cstate value, c0: 0, c1: 1, ... */
> + uint32_t latency; /* worst latency (ms) to enter/exit this cstate */
> + uint32_t power; /* average power consumption(mW) */
> + uint32_t dpcnt; /* number of dependency entries */
> + GUEST_HANDLE(xen_processor_csd) dp; /* NULL if no dependency */ };
> +DEFINE_GUEST_HANDLE_STRUCT(xen_processor_cx);
> +
> +struct xen_processor_flags {
> + uint32_t bm_control:1;
> + uint32_t bm_check:1;
> + uint32_t has_cst:1;
> + uint32_t power_setup_done:1;
> + uint32_t bm_rld_set:1;
> +};
> +
> +struct xen_processor_power {
> + uint32_t count; /* number of C state entries in array below */
> + struct xen_processor_flags flags; /* global flags of this processor */
> + GUEST_HANDLE(xen_processor_cx) states; /* supported c states */ };
> +
> +struct xen_pct_register {
> + uint8_t descriptor;
> + uint16_t length;
> + uint8_t space_id;
> + uint8_t bit_width;
> + uint8_t bit_offset;
> + uint8_t reserved;
> + uint64_t address;
> +};
> +
> +struct xen_processor_px {
> + uint64_t core_frequency; /* megahertz */
> + uint64_t power; /* milliWatts */
> + uint64_t transition_latency; /* microseconds */
> + uint64_t bus_master_latency; /* microseconds */
> + uint64_t control; /* control value */
> + uint64_t status; /* success indicator */
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xen_processor_px);
> +
> +struct xen_psd_package {
> + uint64_t num_entries;
> + uint64_t revision;
> + uint64_t domain;
> + uint64_t coord_type;
> + uint64_t num_processors;
> +};
> +
> +struct xen_processor_performance {
> + uint32_t flags; /* flag for Px sub info type */
> + uint32_t platform_limit; /* Platform limitation on freq usage */
> + struct xen_pct_register control_register;
> + struct xen_pct_register status_register;
> + uint32_t state_count; /* total available performance states */
> + GUEST_HANDLE(xen_processor_px) states;
> + struct xen_psd_package domain_info;
> + uint32_t shared_type; /* coordination type of this processor */
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xen_processor_performance);
> +
> +struct xenpf_set_processor_pminfo {
> + /* IN variables */
> + uint32_t id; /* ACPI CPU ID */
> + uint32_t type; /* {XEN_PM_CX, XEN_PM_PX} */
> + union {
> + struct xen_processor_power power;/* Cx: _CST/_CSD */
> + struct xen_processor_performance perf; /* Px: _PPC/_PCT/_PSS/_PSD */
> + };
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_set_processor_pminfo);
> +
> +struct xen_platform_op {
> + uint32_t cmd;
> + uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
> + union {
> + struct xenpf_settime settime;
> + struct xenpf_add_memtype add_memtype;
> + struct xenpf_del_memtype del_memtype;
> + struct xenpf_read_memtype read_memtype;
> + struct xenpf_microcode_update microcode;
> + struct xenpf_platform_quirk platform_quirk;
> + struct xenpf_firmware_info firmware_info;
> + struct xenpf_enter_acpi_sleep enter_acpi_sleep;
> + struct xenpf_change_freq change_freq;
> + struct xenpf_getidletime getidletime;
> + struct xenpf_set_processor_pminfo set_pminfo;
> + uint8_t pad[128];
> + } u;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xen_platform_op_t);
> +
> +#endif /* __XEN_PUBLIC_PLATFORM_H__ */
> diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h index 70213b4..d83cc08
> 100644
> --- a/include/xen/interface/xen.h
> +++ b/include/xen/interface/xen.h
> @@ -453,6 +453,7 @@ struct start_info {
> /* These flags are passed in the 'flags' field of start_info_t. */
> #define SIF_PRIVILEGED (1<<0) /* Is the domain privileged? */
> #define SIF_INITDOMAIN (1<<1) /* Is this the initial control domain? */
> +#define SIF_PM_MASK (0xFF<<8) /* reserve 1 byte for xen-pm options */
>
> typedef uint64_t cpumap_t;
>
> --
> 1.7.4.1
^ permalink raw reply
* Re: Issue: Runtime API usage in wake-up device irq_handler during wakeup from system-wide-suspend.
From: Tero Kristo @ 2011-09-07 15:48 UTC (permalink / raw)
To: Alan Stern
Cc: Balbi, Felipe, Basak, Partha, R, Govindraj, Munegowda, Keshava,
linux-pm, linux-omap
In-Reply-To: <Pine.LNX.4.44L0.1108271537300.21403-100000@netrider.rowland.org>
Hi,
On Sat, 2011-08-27 at 21:42 +0200, Alan Stern wrote:
> On Sat, 27 Aug 2011, Santosh wrote:
>
> > On Saturday 27 August 2011 07:31 PM, Alan Stern wrote:
> > > On Sat, 27 Aug 2011, Santosh wrote:
> > >
> > >> I might be wrong here, but after discussion with Govindraj on this
> > >> issue, it seems there is a flaw in the way OMAP chain handler
> > >> handling the child interrupts.
> > >>
> > >> On OMAP, we have special interrupt wakeup source at PRCM level and
> > >> many devices can trigger wakeup from low power via this common
> > >> interrupt source. The common interrupt source upon wakeup from low
> > >> power state, decodes the source of interrupt and based on that
> > >> source, calls the respective device ISR directly.
> > >>
> > >> The issue I see here is, the ISR on _a_ device (UART in this case)
> > >> is happening even before UART resume and DPM resume has been completed.
> > >> If this is the case, then it is surely asking for trouble. Because not
> > >> just clocks, but even driver state machine is already in suspend state
> > >> when the ISR is called.
> > >
> > > If the driver state machine is in the suspend state when the ISR is
> > > called, then the ISR should realize it is handling a wakeup event
> > > instead of a normal I/O event. All it needs to do is turn off the
> > > interrupt source; the event itself will be taken care of during the
> > > device's resume callback.
> > >
> > Good point but the ISR is called as a function call and not real
> > hardware event so no need to turn-off the source in the child
> > ISR. Parent ISR will clear the source anyways.
> >
> > But the intention here is to record the event for the child.
>
> In that case the ISR only has to record the event.
>
> > I mean for UART wakeup, the received character should be
> > extracted. If not done, UART will loose that character because
> > the event is lost. So not sure how the event will be taken
> > care during resume callback. Could you elaborate bit more on
> > last comment please?
>
> The resume callback routine must check to see if an event was recorded.
> If one was, the routine must see whether a character was received while
> the system was asleep and extract the character from the UART. (It
> probably won't hurt to do this even if an event wasn't recorded.)
>
> Alan Stern
>
After thinking about this problem and looking at possible ways to fix
it, I am planning to change the PRCM chain handler to be a driver, which
gets suspended along with the rest of the system. This means the PRCM
interrupt would get disabled also during this time, and it would be
enabled in the driver->complete() call, which should happen after rest
of the drivers have been able to enable their PM runtime in the
driver->resume() call chain. Do you see any problems with this approach?
The only issue I am seeing myself is if some driver decides to do
resume() in the complete() pm-op and potentially screwing the ordering
here...
-Tero
Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. Kotipaikka: Helsinki
^ permalink raw reply
* Re: [PATCH] vt/suspend: cleanup #if defined uglyness and fix compile error
From: Arnd Bergmann @ 2011-09-07 16:49 UTC (permalink / raw)
To: H Hartley Sweeten
Cc: len.brown, art, gregkh, Linux Kernel, linux-pm, jslaby, akpm
In-Reply-To: <201109061404.10301.hartleys@visionengravers.com>
On Tuesday 06 September 2011, H Hartley Sweeten wrote:
> rder to cleanup
> the #if defined ugliness for the vt suspend support functions. Note that
> CONFIG_VT_CONSOLE is already dependant on CONFIG_VT.
>
> The function pm_set_vt_switch is actually dependant on CONFIG_VT and not
> CONFIG_PM_SLEEP. This fixes a compile error when CONFIG_PM_SLEEP is
> not set:
>
> drivers/tty/vt/vt_ioctl.c:1794: error: redefinition of 'pm_set_vt_switch'
> include/linux/suspend.h:17: error: previous definition of 'pm_set_vt_switch' was here
>
> Also, remove the incorrect path from the comment in console.c.
>
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: Arnd Bergmann <arnd@arnd.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Arthur Taylor <art@ified.ca>
> Cc: Jiri Slaby <jslaby@suse.cz>
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply
* Re: [Xen-devel] RE: [PATCH 2/7] x86, acpi, tboot: Have a ACPI sleep override instead of calling tboot_sleep.
From: Jeremy Fitzhardinge @ 2011-09-07 17:27 UTC (permalink / raw)
To: Cihula, Joseph
Cc: Brown, Len, xen-devel@lists.xensource.com, keir@xen.org,
Konrad Rzeszutek Wilk, Wang, Shane, x86@kernel.org,
linux-acpi@vger.kernel.org, tboot-devel@lists.sourceforge.net,
liang.tang@oracle.com, hpa@zytor.com, tglx@linutronix.de,
linux-pm@lists.linux-foundation.org
In-Reply-To: <9F57BF860713DF4BA3EFA4F8C6DFEDAC16F3F524@ORSMSX101.amr.corp.intel.com>
On 09/06/2011 09:20 PM, Cihula, Joseph wrote:
>> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
>> Sent: Wednesday, August 31, 2011 11:31 AM
>>
>> The ACPI suspend path makes a call to tboot_sleep right before it writes the PM1A, PM1B values. We
>> replace the direct call to tboot via an registration callback similar to __acpi_register_gsi.
>>
>> CC: Thomas Gleixner <tglx@linutronix.de>
>> CC: "H. Peter Anvin" <hpa@zytor.com>
>> CC: x86@kernel.org
>> CC: Len Brown <len.brown@intel.com>
>> CC: Joseph Cihula <joseph.cihula@intel.com>
>> CC: Shane Wang <shane.wang@intel.com>
>> CC: xen-devel@lists.xensource.com
>> CC: linux-pm@lists.linux-foundation.org
>> CC: tboot-devel@lists.sourceforge.net
>> CC: linux-acpi@vger.kernel.org
>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> ---
>> arch/x86/include/asm/acpi.h | 3 +++
>> arch/x86/kernel/acpi/boot.c | 3 +++
>> arch/x86/kernel/tboot.c | 13 +++++++++----
>> drivers/acpi/acpica/hwsleep.c | 12 ++++++++++--
>> include/linux/tboot.h | 3 ++-
>> 5 files changed, 27 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h index 610001d..49864a1
>> 100644
>> --- a/arch/x86/include/asm/acpi.h
>> +++ b/arch/x86/include/asm/acpi.h
>> @@ -98,6 +98,9 @@ void acpi_pic_sci_set_trigger(unsigned int, u16); extern int
>> (*__acpi_register_gsi)(struct device *dev, u32 gsi,
>> int trigger, int polarity);
>>
>> +extern int (*__acpi_override_sleep)(u8 sleep_state, u32 pm1a_ctrl,
>> + u32 pm1b_ctrl, bool *skip_rest);
>> +
>> static inline void disable_acpi(void)
>> {
>> acpi_disabled = 1;
>> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index 4558f0d..d191b4c
>> 100644
>> --- a/arch/x86/kernel/acpi/boot.c
>> +++ b/arch/x86/kernel/acpi/boot.c
>> @@ -552,6 +552,9 @@ static int acpi_register_gsi_ioapic(struct device *dev, u32 gsi, int
>> (*__acpi_register_gsi)(struct device *dev, u32 gsi,
>> int trigger, int polarity) = acpi_register_gsi_pic;
>>
>> +int (*__acpi_override_sleep)(u8 sleep_state, u32 pm1a_ctrl,
>> + u32 pm1b_ctrl, bool *skip_rest) = NULL;
>> +
>> /*
>> * success: return IRQ number (>=0)
>> * failure: return < 0
>> diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index 30ac65d..a18070c 100644
>> --- a/arch/x86/kernel/tboot.c
>> +++ b/arch/x86/kernel/tboot.c
>> @@ -41,7 +41,7 @@
>> #include <asm/setup.h>
>> #include <asm/e820.h>
>> #include <asm/io.h>
>> -
>> +#include <linux/acpi.h>
>> #include "acpi/realmode/wakeup.h"
>>
>> /* Global pointer to shared data; NULL means no measured launch. */ @@ -270,7 +270,8 @@ 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)
>> +int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
>> + bool *skip_rest)
> Don't you need to use the 'unused' attrib on skip_rest in order to prevent compiler warnings?
No, gcc doesn't warn about unused parameters.
J
^ permalink raw reply
* Re: [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall
From: Jeremy Fitzhardinge @ 2011-09-07 17:29 UTC (permalink / raw)
To: Cihula, Joseph
Cc: Brown, Len, xen-devel@lists.xensource.com, keir@xen.org,
Fitzhardinge, Konrad Rzeszutek Wilk, Wang, Shane, x86@kernel.org,
Jeremy, linux-acpi@vger.kernel.org,
tboot-devel@lists.sourceforge.net, liang.tang@oracle.com,
hpa@zytor.com, tglx@linutronix.de,
linux-pm@lists.linux-foundation.org
In-Reply-To: <9F57BF860713DF4BA3EFA4F8C6DFEDAC16F3F665@ORSMSX101.amr.corp.intel.com>
On 09/06/2011 10:50 PM, Cihula, Joseph wrote:
>> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
>> Sent: Wednesday, August 31, 2011 11:31 AM
>>
>> From: Yu Ke <ke.yu@intel.com>
>>
>> This patches implements the xen_platform_op hypercall, to pass the parsed ACPI info to hypervisor.
>>
>> Signed-off-by: Yu Ke <ke.yu@intel.com>
>> Signed-off-by: Tian Kevin <kevin.tian@intel.com>
>> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>> [v1: Added DEFINE_GUEST.. in appropiate headers]
>> [v2: Ripped out typedefs]
>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> ---
>> arch/ia64/include/asm/xen/interface.h | 1 +
>> arch/x86/include/asm/xen/interface.h | 1 +
>> include/xen/interface/platform.h | 320 +++++++++++++++++++++++++++++++++
>> include/xen/interface/xen.h | 1 +
>> 4 files changed, 323 insertions(+), 0 deletions(-) create mode 100644
>> include/xen/interface/platform.h
>>
>> diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
>> index e951e74..1d2427d 100644
>> --- a/arch/ia64/include/asm/xen/interface.h
>> +++ b/arch/ia64/include/asm/xen/interface.h
>> @@ -76,6 +76,7 @@ DEFINE_GUEST_HANDLE(char); DEFINE_GUEST_HANDLE(int);
>> DEFINE_GUEST_HANDLE(long); DEFINE_GUEST_HANDLE(void);
>> +DEFINE_GUEST_HANDLE(uint64_t);
>>
>> typedef unsigned long xen_pfn_t;
>> DEFINE_GUEST_HANDLE(xen_pfn_t);
>> diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
>> index 5d4922a..a1f2db5 100644
>> --- a/arch/x86/include/asm/xen/interface.h
>> +++ b/arch/x86/include/asm/xen/interface.h
>> @@ -55,6 +55,7 @@ DEFINE_GUEST_HANDLE(char); DEFINE_GUEST_HANDLE(int);
>> DEFINE_GUEST_HANDLE(long); DEFINE_GUEST_HANDLE(void);
>> +DEFINE_GUEST_HANDLE(uint64_t);
>> #endif
>>
>> #ifndef HYPERVISOR_VIRT_START
>> diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
>> new file mode 100644
>> index 0000000..c168468
>> --- /dev/null
>> +++ b/include/xen/interface/platform.h
> Why are you adding so many new hypercalls that aren't being used in this patchset?
May as well bring in all the platform-related hypercall definitions at
once rather than be piecemeal.
>> @@ -0,0 +1,320 @@
>> +/**********************************************************************
>> +********
>> + * platform.h
>> + *
>> + * Hardware platform operations. Intended for use by domain-0 kernel.
>> + *
>> + * Permission is hereby granted, free of charge, to any person
>> +obtaining a copy
>> + * of this software and associated documentation files (the
>> +"Software"), to
>> + * deal in the Software without restriction, including without
>> +limitation the
>> + * rights to use, copy, modify, merge, publish, distribute, sublicense,
>> +and/or
>> + * sell copies of the Software, and to permit persons to whom the
>> +Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be
>> +included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> +EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> +MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>> +SHALL THE
>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> +OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> +ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> + * DEALINGS IN THE SOFTWARE.
>> + *
>> + * Copyright (c) 2002-2006, K Fraser
>> + */
>> +
>> +#ifndef __XEN_PUBLIC_PLATFORM_H__
>> +#define __XEN_PUBLIC_PLATFORM_H__
>> +
>> +#include "xen.h"
>> +
>> +#define XENPF_INTERFACE_VERSION 0x03000001
>> +
>> +/*
>> + * Set clock such that it would read <secs,nsecs> after 00:00:00 UTC,
>> + * 1 January, 1970 if the current system time was <system_time>.
>> + */
>> +#define XENPF_settime 17
>> +struct xenpf_settime {
>> + /* IN variables. */
>> + uint32_t secs;
>> + uint32_t nsecs;
>> + uint64_t system_time;
>> +};
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime_t);
>> +
>> +/*
>> + * Request memory range (@mfn, @mfn+@nr_mfns-1) to have type @type.
>> + * On x86, @type is an architecture-defined MTRR memory type.
>> + * On success, returns the MTRR that was used (@reg) and a handle that
>> +can
>> + * be passed to XENPF_DEL_MEMTYPE to accurately tear down the new setting.
>> + * (x86-specific).
>> + */
>> +#define XENPF_add_memtype 31
>> +struct xenpf_add_memtype {
>> + /* IN variables. */
>> + unsigned long mfn;
>> + uint64_t nr_mfns;
>> + uint32_t type;
>> + /* OUT variables. */
>> + uint32_t handle;
>> + uint32_t reg;
>> +};
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_add_memtype_t);
>> +
>> +/*
>> + * Tear down an existing memory-range type. If @handle is remembered
>> +then it
>> + * should be passed in to accurately tear down the correct setting (in
>> +case
>> + * of overlapping memory regions with differing types). If it is not
>> +known
>> + * then @handle should be set to zero. In all cases @reg must be set.
>> + * (x86-specific).
>> + */
>> +#define XENPF_del_memtype 32
>> +struct xenpf_del_memtype {
>> + /* IN variables. */
>> + uint32_t handle;
>> + uint32_t reg;
>> +};
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_del_memtype_t);
>> +
>> +/* Read current type of an MTRR (x86-specific). */
>> +#define XENPF_read_memtype 33
>> +struct xenpf_read_memtype {
>> + /* IN variables. */
>> + uint32_t reg;
>> + /* OUT variables. */
>> + unsigned long mfn;
>> + uint64_t nr_mfns;
>> + uint32_t type;
>> +};
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_read_memtype_t);
>> +
>> +#define XENPF_microcode_update 35
>> +struct xenpf_microcode_update {
>> + /* IN variables. */
>> + GUEST_HANDLE(void) data; /* Pointer to microcode data */
>> + uint32_t length; /* Length of microcode data. */
>> +};
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_microcode_update_t);
>> +
>> +#define XENPF_platform_quirk 39
>> +#define QUIRK_NOIRQBALANCING 1 /* Do not restrict IO-APIC RTE targets */
>> +#define QUIRK_IOAPIC_BAD_REGSEL 2 /* IO-APIC REGSEL forgets its value */
>> +#define QUIRK_IOAPIC_GOOD_REGSEL 3 /* IO-APIC REGSEL behaves properly */
>> +struct xenpf_platform_quirk {
>> + /* IN variables. */
>> + uint32_t quirk_id;
>> +};
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_platform_quirk_t);
>> +
>> +#define XENPF_firmware_info 50
>> +#define XEN_FW_DISK_INFO 1 /* from int 13 AH=08/41/48 */
>> +#define XEN_FW_DISK_MBR_SIGNATURE 2 /* from MBR offset 0x1b8 */
>> +#define XEN_FW_VBEDDC_INFO 3 /* from int 10 AX=4f15 */
>> +struct xenpf_firmware_info {
>> + /* IN variables. */
>> + uint32_t type;
>> + uint32_t index;
>> + /* OUT variables. */
>> + union {
>> + struct {
>> + /* Int13, Fn48: Check Extensions Present. */
>> + uint8_t device; /* %dl: bios device number */
>> + uint8_t version; /* %ah: major version */
>> + uint16_t interface_support; /* %cx: support bitmap */
>> + /* Int13, Fn08: Legacy Get Device Parameters. */
>> + uint16_t legacy_max_cylinder; /* %cl[7:6]:%ch: max cyl # */
>> + uint8_t legacy_max_head; /* %dh: max head # */
>> + uint8_t legacy_sectors_per_track; /* %cl[5:0]: max sector # */
>> + /* Int13, Fn41: Get Device Parameters (as filled into %ds:%esi). */
>> + /* NB. First uint16_t of buffer must be set to buffer size. */
>> + GUEST_HANDLE(void) edd_params;
>> + } disk_info; /* XEN_FW_DISK_INFO */
>> + struct {
>> + uint8_t device; /* bios device number */
>> + uint32_t mbr_signature; /* offset 0x1b8 in mbr */
>> + } disk_mbr_signature; /* XEN_FW_DISK_MBR_SIGNATURE */
>> + struct {
>> + /* Int10, AX=4F15: Get EDID info. */
>> + uint8_t capabilities;
>> + uint8_t edid_transfer_time;
>> + /* must refer to 128-byte buffer */
>> + GUEST_HANDLE(uchar) edid;
>> + } vbeddc_info; /* XEN_FW_VBEDDC_INFO */
>> + } u;
>> +};
>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmware_info_t);
>> +
>> +#define XENPF_enter_acpi_sleep 51
>> +struct xenpf_enter_acpi_sleep {
>> + /* IN variables */
>> + uint16_t pm1a_cnt_val; /* PM1a control value. */
>> + uint16_t pm1b_cnt_val; /* PM1b control value. */
> These are uint32_t in native Linux--why truncate in the API and not at use?
Does ACPI define them as 32 or 16 bit?
J
^ permalink raw reply
* Re: [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall
From: Cihula, Joseph @ 2011-09-07 17:43 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Brown, Len, xen-devel@lists.xensource.com, hpa@zytor.com,
Konrad Rzeszutek Wilk, x86@kernel.org, Fitzhardinge,
linux-acpi@vger.kernel.org, tboot-devel@lists.sourceforge.net,
liang.tang@oracle.com, linux-pm@lists.linux-foundation.org,
keir@xen.org, tglx@linutronix.de
In-Reply-To: <4E67A9E7.2020802@goop.org>
> From: Jeremy Fitzhardinge [mailto:jeremy@goop.org]
> Sent: Wednesday, September 07, 2011 10:29 AM
>
> On 09/06/2011 10:50 PM, Cihula, Joseph wrote:
> >> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> >> Sent: Wednesday, August 31, 2011 11:31 AM
> >>
> >> From: Yu Ke <ke.yu@intel.com>
> >>
> >> This patches implements the xen_platform_op hypercall, to pass the parsed ACPI info to
> hypervisor.
> >>
> >> Signed-off-by: Yu Ke <ke.yu@intel.com>
> >> Signed-off-by: Tian Kevin <kevin.tian@intel.com>
> >> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> >> [v1: Added DEFINE_GUEST.. in appropiate headers]
> >> [v2: Ripped out typedefs]
> >> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >> ---
> >> arch/ia64/include/asm/xen/interface.h | 1 +
> >> arch/x86/include/asm/xen/interface.h | 1 +
> >> include/xen/interface/platform.h | 320 +++++++++++++++++++++++++++++++++
> >> include/xen/interface/xen.h | 1 +
> >> 4 files changed, 323 insertions(+), 0 deletions(-) create mode
> >> 100644 include/xen/interface/platform.h
> >>
> >> diff --git a/arch/ia64/include/asm/xen/interface.h
> >> b/arch/ia64/include/asm/xen/interface.h
> >> index e951e74..1d2427d 100644
> >> --- a/arch/ia64/include/asm/xen/interface.h
> >> +++ b/arch/ia64/include/asm/xen/interface.h
> >> @@ -76,6 +76,7 @@ DEFINE_GUEST_HANDLE(char);
> >> DEFINE_GUEST_HANDLE(int); DEFINE_GUEST_HANDLE(long);
> >> DEFINE_GUEST_HANDLE(void);
> >> +DEFINE_GUEST_HANDLE(uint64_t);
> >>
> >> typedef unsigned long xen_pfn_t;
> >> DEFINE_GUEST_HANDLE(xen_pfn_t);
> >> diff --git a/arch/x86/include/asm/xen/interface.h
> >> b/arch/x86/include/asm/xen/interface.h
> >> index 5d4922a..a1f2db5 100644
> >> --- a/arch/x86/include/asm/xen/interface.h
> >> +++ b/arch/x86/include/asm/xen/interface.h
> >> @@ -55,6 +55,7 @@ DEFINE_GUEST_HANDLE(char);
> >> DEFINE_GUEST_HANDLE(int); DEFINE_GUEST_HANDLE(long);
> >> DEFINE_GUEST_HANDLE(void);
> >> +DEFINE_GUEST_HANDLE(uint64_t);
> >> #endif
> >>
> >> #ifndef HYPERVISOR_VIRT_START
> >> diff --git a/include/xen/interface/platform.h
> >> b/include/xen/interface/platform.h
> >> new file mode 100644
> >> index 0000000..c168468
> >> --- /dev/null
> >> +++ b/include/xen/interface/platform.h
> > Why are you adding so many new hypercalls that aren't being used in this patchset?
>
> May as well bring in all the platform-related hypercall definitions at once rather than be
> piecemeal.
>
> >> @@ -0,0 +1,320 @@
> >> +/*******************************************************************
> >> +***
> >> +********
> >> + * platform.h
> >> + *
> >> + * Hardware platform operations. Intended for use by domain-0 kernel.
> >> + *
> >> + * Permission is hereby granted, free of charge, to any person
> >> +obtaining a copy
> >> + * of this software and associated documentation files (the
> >> +"Software"), to
> >> + * deal in the Software without restriction, including without
> >> +limitation the
> >> + * rights to use, copy, modify, merge, publish, distribute,
> >> +sublicense, and/or
> >> + * sell copies of the Software, and to permit persons to whom the
> >> +Software is
> >> + * furnished to do so, subject to the following conditions:
> >> + *
> >> + * The above copyright notice and this permission notice shall be
> >> +included in
> >> + * all copies or substantial portions of the Software.
> >> + *
> >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >> +EXPRESS OR
> >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> >> +MERCHANTABILITY,
> >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> >> +SHALL THE
> >> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> >> +OTHER
> >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> >> +ARISING
> >> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> >> +OTHER
> >> + * DEALINGS IN THE SOFTWARE.
> >> + *
> >> + * Copyright (c) 2002-2006, K Fraser */
> >> +
> >> +#ifndef __XEN_PUBLIC_PLATFORM_H__
> >> +#define __XEN_PUBLIC_PLATFORM_H__
> >> +
> >> +#include "xen.h"
> >> +
> >> +#define XENPF_INTERFACE_VERSION 0x03000001
> >> +
> >> +/*
> >> + * Set clock such that it would read <secs,nsecs> after 00:00:00
> >> +UTC,
> >> + * 1 January, 1970 if the current system time was <system_time>.
> >> + */
> >> +#define XENPF_settime 17
> >> +struct xenpf_settime {
> >> + /* IN variables. */
> >> + uint32_t secs;
> >> + uint32_t nsecs;
> >> + uint64_t system_time;
> >> +};
> >> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime_t);
> >> +
> >> +/*
> >> + * Request memory range (@mfn, @mfn+@nr_mfns-1) to have type @type.
> >> + * On x86, @type is an architecture-defined MTRR memory type.
> >> + * On success, returns the MTRR that was used (@reg) and a handle
> >> +that can
> >> + * be passed to XENPF_DEL_MEMTYPE to accurately tear down the new setting.
> >> + * (x86-specific).
> >> + */
> >> +#define XENPF_add_memtype 31
> >> +struct xenpf_add_memtype {
> >> + /* IN variables. */
> >> + unsigned long mfn;
> >> + uint64_t nr_mfns;
> >> + uint32_t type;
> >> + /* OUT variables. */
> >> + uint32_t handle;
> >> + uint32_t reg;
> >> +};
> >> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_add_memtype_t);
> >> +
> >> +/*
> >> + * Tear down an existing memory-range type. If @handle is remembered
> >> +then it
> >> + * should be passed in to accurately tear down the correct setting
> >> +(in case
> >> + * of overlapping memory regions with differing types). If it is not
> >> +known
> >> + * then @handle should be set to zero. In all cases @reg must be set.
> >> + * (x86-specific).
> >> + */
> >> +#define XENPF_del_memtype 32
> >> +struct xenpf_del_memtype {
> >> + /* IN variables. */
> >> + uint32_t handle;
> >> + uint32_t reg;
> >> +};
> >> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_del_memtype_t);
> >> +
> >> +/* Read current type of an MTRR (x86-specific). */
> >> +#define XENPF_read_memtype 33
> >> +struct xenpf_read_memtype {
> >> + /* IN variables. */
> >> + uint32_t reg;
> >> + /* OUT variables. */
> >> + unsigned long mfn;
> >> + uint64_t nr_mfns;
> >> + uint32_t type;
> >> +};
> >> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_read_memtype_t);
> >> +
> >> +#define XENPF_microcode_update 35
> >> +struct xenpf_microcode_update {
> >> + /* IN variables. */
> >> + GUEST_HANDLE(void) data; /* Pointer to microcode data */
> >> + uint32_t length; /* Length of microcode data. */
> >> +};
> >> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_microcode_update_t);
> >> +
> >> +#define XENPF_platform_quirk 39
> >> +#define QUIRK_NOIRQBALANCING 1 /* Do not restrict IO-APIC RTE targets */
> >> +#define QUIRK_IOAPIC_BAD_REGSEL 2 /* IO-APIC REGSEL forgets its value */
> >> +#define QUIRK_IOAPIC_GOOD_REGSEL 3 /* IO-APIC REGSEL behaves properly */
> >> +struct xenpf_platform_quirk {
> >> + /* IN variables. */
> >> + uint32_t quirk_id;
> >> +};
> >> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_platform_quirk_t);
> >> +
> >> +#define XENPF_firmware_info 50
> >> +#define XEN_FW_DISK_INFO 1 /* from int 13 AH=08/41/48 */
> >> +#define XEN_FW_DISK_MBR_SIGNATURE 2 /* from MBR offset 0x1b8 */
> >> +#define XEN_FW_VBEDDC_INFO 3 /* from int 10 AX=4f15 */
> >> +struct xenpf_firmware_info {
> >> + /* IN variables. */
> >> + uint32_t type;
> >> + uint32_t index;
> >> + /* OUT variables. */
> >> + union {
> >> + struct {
> >> + /* Int13, Fn48: Check Extensions Present. */
> >> + uint8_t device; /* %dl: bios device number */
> >> + uint8_t version; /* %ah: major version */
> >> + uint16_t interface_support; /* %cx: support bitmap */
> >> + /* Int13, Fn08: Legacy Get Device Parameters. */
> >> + uint16_t legacy_max_cylinder; /* %cl[7:6]:%ch: max cyl # */
> >> + uint8_t legacy_max_head; /* %dh: max head # */
> >> + uint8_t legacy_sectors_per_track; /* %cl[5:0]: max sector # */
> >> + /* Int13, Fn41: Get Device Parameters (as filled into %ds:%esi). */
> >> + /* NB. First uint16_t of buffer must be set to buffer size. */
> >> + GUEST_HANDLE(void) edd_params;
> >> + } disk_info; /* XEN_FW_DISK_INFO */
> >> + struct {
> >> + uint8_t device; /* bios device number */
> >> + uint32_t mbr_signature; /* offset 0x1b8 in mbr */
> >> + } disk_mbr_signature; /* XEN_FW_DISK_MBR_SIGNATURE */
> >> + struct {
> >> + /* Int10, AX=4F15: Get EDID info. */
> >> + uint8_t capabilities;
> >> + uint8_t edid_transfer_time;
> >> + /* must refer to 128-byte buffer */
> >> + GUEST_HANDLE(uchar) edid;
> >> + } vbeddc_info; /* XEN_FW_VBEDDC_INFO */
> >> + } u;
> >> +};
> >> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmware_info_t);
> >> +
> >> +#define XENPF_enter_acpi_sleep 51
> >> +struct xenpf_enter_acpi_sleep {
> >> + /* IN variables */
> >> + uint16_t pm1a_cnt_val; /* PM1a control value. */
> >> + uint16_t pm1b_cnt_val; /* PM1b control value. */
> > These are uint32_t in native Linux--why truncate in the API and not at use?
>
> Does ACPI define them as 32 or 16 bit?
The spec indicates that the length is variable and could be up to 32 bits (AFAICT). And Linux uses 32b, which your other patch is truncating for this call.
Joe
^ permalink raw reply
* Re: [Xen-devel] RE: [PATCH 2/7] x86, acpi, tboot: Have a ACPI sleep override instead of calling tboot_sleep.
From: Cihula, Joseph @ 2011-09-07 17:55 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Brown, Len, xen-devel@lists.xensource.com, keir@xen.org,
Konrad Rzeszutek Wilk, Wang, Shane, x86@kernel.org,
linux-acpi@vger.kernel.org, tboot-devel@lists.sourceforge.net,
liang.tang@oracle.com, hpa@zytor.com, tglx@linutronix.de,
linux-pm@lists.linux-foundation.org
In-Reply-To: <4E67A972.6030909@goop.org>
> From: Jeremy Fitzhardinge [mailto:jeremy@goop.org]
> Sent: Wednesday, September 07, 2011 10:27 AM
>
> On 09/06/2011 09:20 PM, Cihula, Joseph wrote:
> >> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> >> Sent: Wednesday, August 31, 2011 11:31 AM
> >>
> >> The ACPI suspend path makes a call to tboot_sleep right before it
> >> writes the PM1A, PM1B values. We replace the direct call to tboot via an registration callback
> similar to __acpi_register_gsi.
> >>
> >> CC: Thomas Gleixner <tglx@linutronix.de>
> >> CC: "H. Peter Anvin" <hpa@zytor.com>
> >> CC: x86@kernel.org
> >> CC: Len Brown <len.brown@intel.com>
> >> CC: Joseph Cihula <joseph.cihula@intel.com>
> >> CC: Shane Wang <shane.wang@intel.com>
> >> CC: xen-devel@lists.xensource.com
> >> CC: linux-pm@lists.linux-foundation.org
> >> CC: tboot-devel@lists.sourceforge.net
> >> CC: linux-acpi@vger.kernel.org
> >> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >> ---
> >> arch/x86/include/asm/acpi.h | 3 +++
> >> arch/x86/kernel/acpi/boot.c | 3 +++
> >> arch/x86/kernel/tboot.c | 13 +++++++++----
> >> drivers/acpi/acpica/hwsleep.c | 12 ++++++++++--
> >> include/linux/tboot.h | 3 ++-
> >> 5 files changed, 27 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/arch/x86/include/asm/acpi.h
> >> b/arch/x86/include/asm/acpi.h index 610001d..49864a1
> >> 100644
> >> --- a/arch/x86/include/asm/acpi.h
> >> +++ b/arch/x86/include/asm/acpi.h
> >> @@ -98,6 +98,9 @@ void acpi_pic_sci_set_trigger(unsigned int, u16);
> >> extern int (*__acpi_register_gsi)(struct device *dev, u32 gsi,
> >> int trigger, int polarity);
> >>
> >> +extern int (*__acpi_override_sleep)(u8 sleep_state, u32 pm1a_ctrl,
> >> + u32 pm1b_ctrl, bool *skip_rest);
> >> +
> >> static inline void disable_acpi(void) {
> >> acpi_disabled = 1;
> >> diff --git a/arch/x86/kernel/acpi/boot.c
> >> b/arch/x86/kernel/acpi/boot.c index 4558f0d..d191b4c
> >> 100644
> >> --- a/arch/x86/kernel/acpi/boot.c
> >> +++ b/arch/x86/kernel/acpi/boot.c
> >> @@ -552,6 +552,9 @@ static int acpi_register_gsi_ioapic(struct device
> >> *dev, u32 gsi, int (*__acpi_register_gsi)(struct device *dev, u32 gsi,
> >> int trigger, int polarity) = acpi_register_gsi_pic;
> >>
> >> +int (*__acpi_override_sleep)(u8 sleep_state, u32 pm1a_ctrl,
> >> + u32 pm1b_ctrl, bool *skip_rest) = NULL;
> >> +
> >> /*
> >> * success: return IRQ number (>=0)
> >> * failure: return < 0
> >> diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index
> >> 30ac65d..a18070c 100644
> >> --- a/arch/x86/kernel/tboot.c
> >> +++ b/arch/x86/kernel/tboot.c
> >> @@ -41,7 +41,7 @@
> >> #include <asm/setup.h>
> >> #include <asm/e820.h>
> >> #include <asm/io.h>
> >> -
> >> +#include <linux/acpi.h>
> >> #include "acpi/realmode/wakeup.h"
> >>
> >> /* Global pointer to shared data; NULL means no measured launch. */
> >> @@ -270,7 +270,8 @@ 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)
> >> +int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
> >> + bool *skip_rest)
> > Don't you need to use the 'unused' attrib on skip_rest in order to prevent compiler warnings?
>
> No, gcc doesn't warn about unused parameters.
-Wunused-parameter
While the kernel may not be compiled with this flag, it wouldn't hurt to specify it anyway; but it's not a big issue.
Joe
^ permalink raw reply
* Re: Issue: Runtime API usage in wake-up device irq_handler during wakeup from system-wide-suspend.
From: Kevin Hilman @ 2011-09-07 17:59 UTC (permalink / raw)
To: t-kristo
Cc: Balbi, Felipe, Basak, Partha, R, Govindraj, Munegowda, Keshava,
linux-pm, linux-omap
In-Reply-To: <1315410513.2679.9.camel@sokoban>
Tero Kristo <t-kristo@ti.com> writes:
> Hi,
>
> On Sat, 2011-08-27 at 21:42 +0200, Alan Stern wrote:
>> On Sat, 27 Aug 2011, Santosh wrote:
>>
>> > On Saturday 27 August 2011 07:31 PM, Alan Stern wrote:
>> > > On Sat, 27 Aug 2011, Santosh wrote:
>> > >
>> > >> I might be wrong here, but after discussion with Govindraj on this
>> > >> issue, it seems there is a flaw in the way OMAP chain handler
>> > >> handling the child interrupts.
>> > >>
>> > >> On OMAP, we have special interrupt wakeup source at PRCM level and
>> > >> many devices can trigger wakeup from low power via this common
>> > >> interrupt source. The common interrupt source upon wakeup from low
>> > >> power state, decodes the source of interrupt and based on that
>> > >> source, calls the respective device ISR directly.
>> > >>
>> > >> The issue I see here is, the ISR on _a_ device (UART in this case)
>> > >> is happening even before UART resume and DPM resume has been completed.
>> > >> If this is the case, then it is surely asking for trouble. Because not
>> > >> just clocks, but even driver state machine is already in suspend state
>> > >> when the ISR is called.
>> > >
>> > > If the driver state machine is in the suspend state when the ISR is
>> > > called, then the ISR should realize it is handling a wakeup event
>> > > instead of a normal I/O event. All it needs to do is turn off the
>> > > interrupt source; the event itself will be taken care of during the
>> > > device's resume callback.
>> > >
>> > Good point but the ISR is called as a function call and not real
>> > hardware event so no need to turn-off the source in the child
>> > ISR. Parent ISR will clear the source anyways.
>> >
>> > But the intention here is to record the event for the child.
>>
>> In that case the ISR only has to record the event.
>>
>> > I mean for UART wakeup, the received character should be
>> > extracted. If not done, UART will loose that character because
>> > the event is lost. So not sure how the event will be taken
>> > care during resume callback. Could you elaborate bit more on
>> > last comment please?
>>
>> The resume callback routine must check to see if an event was recorded.
>> If one was, the routine must see whether a character was received while
>> the system was asleep and extract the character from the UART. (It
>> probably won't hurt to do this even if an event wasn't recorded.)
>>
>> Alan Stern
>>
>
> After thinking about this problem and looking at possible ways to fix
> it, I am planning to change the PRCM chain handler to be a driver, which
> gets suspended along with the rest of the system. This means the PRCM
> interrupt would get disabled also during this time, and it would be
> enabled in the driver->complete() call, which should happen after rest
> of the drivers have been able to enable their PM runtime in the
> driver->resume() call chain. Do you see any problems with this approach?
How will the system wakeup from retention or off-mode if the PRCM IRQ is
disabled?
Besides that, I don't like this approach because it leaves a rather long
window during which no PRCM-triggered wakeup events can happen. This is
not good since we also want potential wakeup events that happen *during*
system suspend to be able to cancel the suspend.
> The only issue I am seeing myself is if some driver decides to do
> resume() in the complete() pm-op and potentially screwing the ordering
> here...
Personally, I think Alan's approach is the only scalable approach. This
has to be handled by the drivers.
If the driver's ISR does a pm_runtime_get_sync() and it fails (in this
case, with -EACCESS since runtime PM is disabled), the driver should
handle that handle as Alan described above.
Kevin
^ permalink raw reply
* [PATCH 0/1] freezer: fix wait_event_freezable/__thaw_task races
From: Oleg Nesterov @ 2011-09-07 18:21 UTC (permalink / raw)
To: Tejun Heo, Rafael J. Wysocki; +Cc: containers, paul, linux-kernel, linux-pm
In-Reply-To: <20110906155332.GF18425@mtj.dyndns.org>
Hi,
On 09/07, Tejun Heo wrote:
> >
> > I meant, unless the caller plays with allow_signal(), it has all rights to do
> >
> > if (wait_event_freezable(...))
> > BUG();
> >
> > This becomes correct with the code above.
>
> Yeap, sure, w/ freezable_with_signal gone, the above should work fine.
> Care to create a patch?
Sure.
But. Tejun, Rafael, I have to rely on your review. I have no idea how
can I test this change. Hopfully it is simple enough, though.
And. wait_event_freezable_timeout() obviously has another problem,
although I hope this is fine. The caller can spend a lot of time in
refrigerator, shouldn't we update "timeout" in this case? I hope we
shouldn't.
Oleg.
^ permalink raw reply
* [PATCH 1/1] freezer: fix wait_event_freezable/__thaw_task races
From: Oleg Nesterov @ 2011-09-07 18:22 UTC (permalink / raw)
To: Tejun Heo, Rafael J. Wysocki; +Cc: containers, paul, linux-kernel, linux-pm
In-Reply-To: <20110907182156.GA13909@redhat.com>
wait_event_freezable() and wait_event_freezable_timeout() stop
the waiting if try_to_freeze() fails. This is not right, we can
race with __thaw_task() and in this case
- wait_event_freezable() returns the wrong ERESTARTSYS
- wait_event_freezable_timeout() can return the positive
value while condition == F
Change the code to always check __retval/condition before return.
Note: with or without this patch the timeout logic looks strange,
probably we should recalc timeout if try_to_freeze() returns T.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
include/linux/freezer.h | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
--- 3.1/include/linux/freezer.h~w_e_f 2011-09-04 20:23:30.000000000 +0200
+++ 3.1/include/linux/freezer.h 2011-09-07 20:00:27.000000000 +0200
@@ -107,32 +107,33 @@ static inline int freezer_should_skip(st
* Freezer-friendly wrappers around wait_event_interruptible() and
* wait_event_interruptible_timeout(), originally defined in <linux/wait.h>
*/
-
#define wait_event_freezable(wq, condition) \
({ \
int __retval; \
- do { \
+ for (;;) { \
__retval = wait_event_interruptible(wq, \
(condition) || freezing(current)); \
- if (__retval && !freezing(current)) \
+ if (__retval || (condition)) \
break; \
- else if (!(condition)) \
- __retval = -ERESTARTSYS; \
- } while (try_to_freeze()); \
+ try_to_freeze(); \
+ } \
__retval; \
})
-
#define wait_event_freezable_timeout(wq, condition, timeout) \
({ \
long __retval = timeout; \
- do { \
+ for (;;) { \
__retval = wait_event_interruptible_timeout(wq, \
(condition) || freezing(current), \
__retval); \
- } while (try_to_freeze()); \
+ if (__retval <= 0 || (condition)) \
+ break; \
+ try_to_freeze(); \
+ } \
__retval; \
})
+
#else /* !CONFIG_FREEZER */
static inline bool frozen(struct task_struct *p) { return false; }
static inline bool freezing(struct task_struct *p) { return false; }
^ permalink raw reply
* Re: [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall
From: Konrad Rzeszutek Wilk @ 2011-09-07 19:06 UTC (permalink / raw)
To: Cihula, Joseph
Cc: Brown, Len, Jeremy Fitzhardinge, xen-devel@lists.xensource.com,
keir@xen.org, x86@kernel.org, linux-acpi@vger.kernel.org,
tboot-devel@lists.sourceforge.net, Fitzhardinge,
tglx@linutronix.de, hpa@zytor.com, liang.tang@oracle.com,
linux-pm@lists.linux-foundation.org
In-Reply-To: <9F57BF860713DF4BA3EFA4F8C6DFEDAC16F40F0B@ORSMSX101.amr.corp.intel.com>
> > >> +#define XENPF_enter_acpi_sleep 51
> > >> +struct xenpf_enter_acpi_sleep {
> > >> + /* IN variables */
> > >> + uint16_t pm1a_cnt_val; /* PM1a control value. */
> > >> + uint16_t pm1b_cnt_val; /* PM1b control value. */
> > > These are uint32_t in native Linux--why truncate in the API and not at use?
> >
> > Does ACPI define them as 32 or 16 bit?
>
> The spec indicates that the length is variable and could be up to 32 bits (AFAICT). And Linux uses 32b, which your other patch is truncating for this call.
Yikes! Well, looks like we need to fix the Xen ABI too. Lets get that fixed
and also address all the other comments (thanks for looking at it) you pointed
out.
^ permalink raw reply
* Re: [PATCH 1/1] freezer: fix wait_event_freezable/__thaw_task races
From: Tejun Heo @ 2011-09-08 1:05 UTC (permalink / raw)
To: Oleg Nesterov; +Cc: containers, linux-kernel, linux-pm, paul
In-Reply-To: <20110907182217.GB13909@redhat.com>
Hello,
On Wed, Sep 07, 2011 at 08:22:17PM +0200, Oleg Nesterov wrote:
> wait_event_freezable() and wait_event_freezable_timeout() stop
> the waiting if try_to_freeze() fails. This is not right, we can
> race with __thaw_task() and in this case
>
> - wait_event_freezable() returns the wrong ERESTARTSYS
>
> - wait_event_freezable_timeout() can return the positive
> value while condition == F
Indeed, nice catch. This one actually is pretty dangerous. We
probably want to make a separate fix for this and backport it to
-stable?
> Change the code to always check __retval/condition before return.
>
> Note: with or without this patch the timeout logic looks strange,
> probably we should recalc timeout if try_to_freeze() returns T.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Yeap, with freezable_with_signal gone, this looks correct & simpler to
me but it would be nice if you can sprinkle some documentation while
at it. :)
Thanks.
--
tejun
^ permalink raw reply
* Re: Issue: Runtime API usage in wake-up device irq_handler during wakeup from system-wide-suspend.
From: Tero Kristo @ 2011-09-08 4:58 UTC (permalink / raw)
To: Hilman, Kevin
Cc: Balbi, Felipe, Basak, Partha, R, Govindraj, Munegowda, Keshava,
linux-pm, linux-omap
In-Reply-To: <87fwk8qllx.fsf@ti.com>
On Wed, 2011-09-07 at 19:59 +0200, Hilman, Kevin wrote:
> Tero Kristo <t-kristo@ti.com> writes:
>
> > Hi,
> >
> > On Sat, 2011-08-27 at 21:42 +0200, Alan Stern wrote:
> >> On Sat, 27 Aug 2011, Santosh wrote:
> >>
> >> > On Saturday 27 August 2011 07:31 PM, Alan Stern wrote:
> >> > > On Sat, 27 Aug 2011, Santosh wrote:
> >> > >
> >> > >> I might be wrong here, but after discussion with Govindraj on this
> >> > >> issue, it seems there is a flaw in the way OMAP chain handler
> >> > >> handling the child interrupts.
> >> > >>
> >> > >> On OMAP, we have special interrupt wakeup source at PRCM level and
> >> > >> many devices can trigger wakeup from low power via this common
> >> > >> interrupt source. The common interrupt source upon wakeup from low
> >> > >> power state, decodes the source of interrupt and based on that
> >> > >> source, calls the respective device ISR directly.
> >> > >>
> >> > >> The issue I see here is, the ISR on _a_ device (UART in this case)
> >> > >> is happening even before UART resume and DPM resume has been completed.
> >> > >> If this is the case, then it is surely asking for trouble. Because not
> >> > >> just clocks, but even driver state machine is already in suspend state
> >> > >> when the ISR is called.
> >> > >
> >> > > If the driver state machine is in the suspend state when the ISR is
> >> > > called, then the ISR should realize it is handling a wakeup event
> >> > > instead of a normal I/O event. All it needs to do is turn off the
> >> > > interrupt source; the event itself will be taken care of during the
> >> > > device's resume callback.
> >> > >
> >> > Good point but the ISR is called as a function call and not real
> >> > hardware event so no need to turn-off the source in the child
> >> > ISR. Parent ISR will clear the source anyways.
> >> >
> >> > But the intention here is to record the event for the child.
> >>
> >> In that case the ISR only has to record the event.
> >>
> >> > I mean for UART wakeup, the received character should be
> >> > extracted. If not done, UART will loose that character because
> >> > the event is lost. So not sure how the event will be taken
> >> > care during resume callback. Could you elaborate bit more on
> >> > last comment please?
> >>
> >> The resume callback routine must check to see if an event was recorded.
> >> If one was, the routine must see whether a character was received while
> >> the system was asleep and extract the character from the UART. (It
> >> probably won't hurt to do this even if an event wasn't recorded.)
> >>
> >> Alan Stern
> >>
> >
> > After thinking about this problem and looking at possible ways to fix
> > it, I am planning to change the PRCM chain handler to be a driver, which
> > gets suspended along with the rest of the system. This means the PRCM
> > interrupt would get disabled also during this time, and it would be
> > enabled in the driver->complete() call, which should happen after rest
> > of the drivers have been able to enable their PM runtime in the
> > driver->resume() call chain. Do you see any problems with this approach?
>
> How will the system wakeup from retention or off-mode if the PRCM IRQ is
> disabled?
This is actually some sort of an issue with retention if we disable PRCM
irq completely, off works purely with wakeup signals as we come out of
reset. Anyway, I did some experimentation with this, and OMAP3 is able
to wake up if we leave WKUP irq up, but disable IO chain irq. IO chain
events seem to trigger a WKUP event also always, we just postpone the
processing of IO chain until later. I had to also split the wakeup
clearing for OMAP3 into 2 parts, one part handles wakeups and another IO
chain. Currently both IO chain and WKUP are acked by the same handler.
> Besides that, I don't like this approach because it leaves a rather long
> window during which no PRCM-triggered wakeup events can happen. This is
> not good since we also want potential wakeup events that happen *during*
> system suspend to be able to cancel the suspend.
This should also be taken care of by the approach described above.
>
> > The only issue I am seeing myself is if some driver decides to do
> > resume() in the complete() pm-op and potentially screwing the ordering
> > here...
>
> Personally, I think Alan's approach is the only scalable approach. This
> has to be handled by the drivers.
>
> If the driver's ISR does a pm_runtime_get_sync() and it fails (in this
> case, with -EACCESS since runtime PM is disabled), the driver should
> handle that handle as Alan described above.
Yea I think this probably needs to be done anyway, at least on some
cases. The PRCM chain handler approach might be able to solve most of
the problems though. I think I will post what I have anyway for comments
hopefully later today, so you can have a look and say what you think.
-Tero
Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. Kotipaikka: Helsinki
^ permalink raw reply
* Re: [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall
From: Konrad Rzeszutek Wilk @ 2011-09-08 13:38 UTC (permalink / raw)
To: Cihula, Joseph
Cc: Brown, Len, Jeremy Fitzhardinge, xen-devel@lists.xensource.com,
keir@xen.org, x86@kernel.org, linux-acpi@vger.kernel.org,
tboot-devel@lists.sourceforge.net, Fitzhardinge,
tglx@linutronix.de, hpa@zytor.com, liang.tang@oracle.com,
linux-pm@lists.linux-foundation.org
In-Reply-To: <20110907190659.GH7074@dumpdata.com>
On Wed, Sep 07, 2011 at 03:06:59PM -0400, Konrad Rzeszutek Wilk wrote:
> > > >> +#define XENPF_enter_acpi_sleep 51
> > > >> +struct xenpf_enter_acpi_sleep {
> > > >> + /* IN variables */
> > > >> + uint16_t pm1a_cnt_val; /* PM1a control value. */
> > > >> + uint16_t pm1b_cnt_val; /* PM1b control value. */
> > > > These are uint32_t in native Linux--why truncate in the API and not at use?
> > >
> > > Does ACPI define them as 32 or 16 bit?
> >
> > The spec indicates that the length is variable and could be up to 32 bits (AFAICT). And Linux uses 32b, which your other patch is truncating for this call.
>
> Yikes! Well, looks like we need to fix the Xen ABI too. Lets get that fixed
> and also address all the other comments (thanks for looking at it) you pointed
> out.
So read up the ACPI spec and it says that the minimum is 2 bytes and does not
say anything about the maximum. The list of what the bits do stops at 16-bits
(the last two are reserved) so I think we are actually OK.
Albeit if the spec starts using more of them - then yes we will need to revist
this Xen ABI and potentially add a new call.
^ permalink raw reply
* Re: Issue: Runtime API usage in wake-up device irq_handler during wakeup from system-wide-suspend.
From: Kevin Hilman @ 2011-09-08 13:51 UTC (permalink / raw)
To: t-kristo
Cc: Balbi, Felipe, Basak, Partha, R, Govindraj, Munegowda, Keshava,
linux-pm, linux-omap
In-Reply-To: <1315457911.2679.19.camel@sokoban>
Hi Tero,
Tero Kristo <t-kristo@ti.com> writes:
> On Wed, 2011-09-07 at 19:59 +0200, Hilman, Kevin wrote:
>> Tero Kristo <t-kristo@ti.com> writes:
[...]
>> > After thinking about this problem and looking at possible ways to fix
>> > it, I am planning to change the PRCM chain handler to be a driver, which
>> > gets suspended along with the rest of the system. This means the PRCM
>> > interrupt would get disabled also during this time, and it would be
>> > enabled in the driver->complete() call, which should happen after rest
>> > of the drivers have been able to enable their PM runtime in the
>> > driver->resume() call chain. Do you see any problems with this approach?
>>
>> How will the system wakeup from retention or off-mode if the PRCM IRQ is
>> disabled?
>
> This is actually some sort of an issue with retention if we disable PRCM
> irq completely, off works purely with wakeup signals as we come out of
> reset. Anyway, I did some experimentation with this, and OMAP3 is able
> to wake up if we leave WKUP irq up, but disable IO chain irq. IO chain
> events seem to trigger a WKUP event also always, we just postpone the
> processing of IO chain until later. I had to also split the wakeup
> clearing for OMAP3 into 2 parts, one part handles wakeups and another IO
> chain. Currently both IO chain and WKUP are acked by the same handler.
Here's another option, which is kind of a hybrid of what's been
discussed so far.
The PRCM driver would leave the IRQs enabled during suspend, but would
just delay delivering them to the drivers. IOW, handle/clear the PRCM
IRQ normally, but delay delivery of the *device* IRQ until the
->complete callback of the PRCM driver.
Doing this ensures all the drivers are resumed before any device IRQ is
delivered, and doesn't require any additional queuing of events in the
drivers.
Kevin
^ permalink raw reply
* [PATCH] PM / Runtime: pm_runtime_idle can be called in atomic context
From: tom.leiming @ 2011-09-08 16:06 UTC (permalink / raw)
To: rjw, stern; +Cc: linux-pm
From: Ming Lei <tom.leiming@gmail.com>
Add to pm_runtime_idle the list of functions that can be called
in atomic context if pm_runtime_irq_safe() has been called for the
device.
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
Documentation/power/runtime_pm.txt | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/Documentation/power/runtime_pm.txt b/Documentation/power/runtime_pm.txt
index 08d70e4..1f05404 100644
--- a/Documentation/power/runtime_pm.txt
+++ b/Documentation/power/runtime_pm.txt
@@ -477,6 +477,7 @@ pm_runtime_autosuspend_expiration()
If pm_runtime_irq_safe() has been called for a device then the following helper
functions may also be used in interrupt context:
+pm_runtime_idle()
pm_runtime_suspend()
pm_runtime_autosuspend()
pm_runtime_resume()
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH 1/1] freezer: fix wait_event_freezable/__thaw_task races
From: Oleg Nesterov @ 2011-09-08 17:59 UTC (permalink / raw)
To: Tejun Heo; +Cc: containers, linux-kernel, linux-pm, paul
In-Reply-To: <20110908010530.GD3987@mtj.dyndns.org>
Hi,
On 09/08, Tejun Heo wrote:
>
> Hello,
>
> On Wed, Sep 07, 2011 at 08:22:17PM +0200, Oleg Nesterov wrote:
> > wait_event_freezable() and wait_event_freezable_timeout() stop
> > the waiting if try_to_freeze() fails. This is not right, we can
> > race with __thaw_task() and in this case
> >
> > - wait_event_freezable() returns the wrong ERESTARTSYS
> >
> > - wait_event_freezable_timeout() can return the positive
> > value while condition == F
>
> Indeed, nice catch. This one actually is pretty dangerous. We
> probably want to make a separate fix for this and backport it to
> -stable?
And I forgot to mention that wait_event_freezable_timeout() doesn't
handle -ERESTARTSYS at all.
But I don't think -stable needs this fix. According to grep, nobody
check the returned value, and none of the callers plays with signals.
> > Change the code to always check __retval/condition before return.
> >
> > Note: with or without this patch the timeout logic looks strange,
> > probably we should recalc timeout if try_to_freeze() returns T.
> >
> > Signed-off-by: Oleg Nesterov <oleg@redhat.com>
>
> Yeap, with freezable_with_signal gone, this looks correct & simpler to
> me
I don't really understand this... set_freezable_with_signal() has a
lot of problems, yes... But even if it wasn't removed this fix makes
sense anyway, afaics.
If freezable_with_signal caller does wait_event_freezable_timeout(),
__retval becomes -ERESTARTSYS after freeze_task(). The next iteration
will return 0 with the KERN_ERR message from schedule_timeout().
> but it would be nice if you can sprinkle some documentation while
> at it. :)
But they already have the comment ;) What can I add?
Oleg.
^ permalink raw reply
* Re: [PATCH 6/6] freezer: kill unused set_freezable_with_signal()
From: Matt Helsley @ 2011-09-08 18:01 UTC (permalink / raw)
To: Tejun Heo; +Cc: containers, Oleg Nesterov, linux-kernel, linux-pm, paul
In-Reply-To: <20110906032846.GA18425@mtj.dyndns.org>
On Tue, Sep 06, 2011 at 12:28:55PM +0900, Tejun Heo wrote:
> Hello,
>
> On Mon, Sep 05, 2011 at 06:20:12PM +0200, Oleg Nesterov wrote:
> > Perhaps it is correct... Just I do not understand what it should do.
> > I thought it is "wait_for_event && do_not_block_freezer". And at first
> > glance the code looks as if it tries to do this. Say, in the "likely"
> > case we restart wait_event_interruptible() after refrigerator().
> >
> > But this looks racy. Suppose that freezing() is already false when
> > try_to_freeze() or __refrigerator() is called. Say, cgroup_freezer does
> > freeze_task() + __thaw_task(). Why it returns -ERESTARTSYS in this case?
>
> It may return -ERESTARTSYS when not strictly necessary but given that
> it's supposed to trigger restart anyway I don't think it's actually
> broken (it doesn't break the contract of the wait). Another thing to
> note is that kthread freezing via cgroup is almost fundamentally
> broken. With the removal of freezable_with_signal, this shouldn't be
> an issue anymore although cgroup_freezer still needs to be fixed to
> account for kthreads for xstate transitions (it currently triggers
> BUG_ON).
Looking at the code and docs I realize I didn't explicitly mention that
kthreads could not be frozen by the cgroup freezer. However, the code did
not allow it. When freezing tasks the cgroup freezer always did:
freeze_task(task, true)
which would only freeze tasks without PF_FREEZER_NOSIG due to the second
"sig_only" parameter. I believe this means it could not be used to
freeze kthreads.
My feeling is kthreads should not be "managed" directly by userspace.
Especially if that includes the ability to arbitrarily stop or freeze them.
So it seems more appropriate to explicitly disallow freezing of kthreads
via the cgroup freezer.
Cheers,
-Matt Helsley
^ permalink raw reply
* LPC 2011 Power Management Session notes
From: Paul Walmsley @ 2011-09-09 0:28 UTC (permalink / raw)
To: linux-pm
Available at
http://pad.ubuntu.com/plumbers-2011-pwr-mgt
in real time if anyone's interested
- Paul
^ permalink raw reply
* Re: LPC 2011 Power Management Session notes
From: Jean Pihet @ 2011-09-09 8:09 UTC (permalink / raw)
To: Paul Walmsley; +Cc: linux-pm
In-Reply-To: <alpine.DEB.2.00.1109081827360.17022@utopia.booyaka.com>
Hi,
On Fri, Sep 9, 2011 at 2:28 AM, Paul Walmsley <paul@pwsan.com> wrote:
>
> Available at
>
> http://pad.ubuntu.com/plumbers-2011-pwr-mgt
>
> in real time if anyone's interested
Thanks for the doc!
About the Device PM QoS user space interface:
1. Proposal: For every device, there will be a sysfs wakeup latency attribute
2. Proposal: Userspace processes should communicate their needs via sysfs
I assume there can be only one user app that can open a given sysfs
entry at a time, is that correct?
Let me think about it and propose a patch next week.
Regards,
Jean
>
>
> - Paul
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/linux-pm
>
^ permalink raw reply
* Re: LPC 2011 Power Management Session notes
From: Mark Brown @ 2011-09-09 16:50 UTC (permalink / raw)
To: Jean Pihet; +Cc: linux-pm
In-Reply-To: <CAORVsuWKd9uvTTNCr_k9W6DeA+z3ZEaD99bqit++4xUy9zqKSQ@mail.gmail.com>
On Fri, Sep 09, 2011 at 10:09:48AM +0200, Jean Pihet wrote:
> On Fri, Sep 9, 2011 at 2:28 AM, Paul Walmsley <paul@pwsan.com> wrote:
> About the Device PM QoS user space interface:
> 1. Proposal: For every device, there will be a sysfs wakeup latency attribute
> 2. Proposal: Userspace processes should communicate their needs via sysfs
> I assume there can be only one user app that can open a given sysfs
> entry at a time, is that correct?
No, that's not the case. The file implementation doesn't get to see
opens and closes at all, it just gets passed buffers to fill or read
from.
^ permalink raw reply
* Re: LPC 2011 Power Management Session notes
From: mark gross @ 2011-09-09 22:50 UTC (permalink / raw)
To: Jean Pihet; +Cc: linux-pm
In-Reply-To: <CAORVsuWKd9uvTTNCr_k9W6DeA+z3ZEaD99bqit++4xUy9zqKSQ@mail.gmail.com>
On Fri, Sep 09, 2011 at 10:09:48AM +0200, Jean Pihet wrote:
> Hi,
>
> On Fri, Sep 9, 2011 at 2:28 AM, Paul Walmsley <paul@pwsan.com> wrote:
> >
> > Available at
> >
> > http://pad.ubuntu.com/plumbers-2011-pwr-mgt
> >
> > in real time if anyone's interested
> Thanks for the doc!
>
>
> About the Device PM QoS user space interface:
> 1. Proposal: For every device, there will be a sysfs wakeup latency attribute
> 2. Proposal: Userspace processes should communicate their needs via sysfs
>
> I assume there can be only one user app that can open a given sysfs
> entry at a time, is that correct?
>
> Let me think about it and propose a patch next week.
My recollection of the evening was that the exposing of per device qos
latencies was not considered a good idea without better articulation on
how it would be used. (or misused)
FWIW a few folks (ok maybe just me) are not comfortable with exposing
raw values without a good enough abstraction for them to user mode such
that the values used through that user mode ABI would be somewhat
portable between ISA's or even versions of the same chips / platforms.
--mark
> Regards,
> Jean
>
> >
> >
> > - Paul
> > _______________________________________________
> > linux-pm mailing list
> > linux-pm@lists.linux-foundation.org
> > https://lists.linux-foundation.org/mailman/listinfo/linux-pm
> >
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/linux-pm
^ permalink raw reply
* [PATCH 1/2] PM/runtime: update document about callbacks
From: tom.leiming @ 2011-09-10 14:37 UTC (permalink / raw)
To: rjw, stern; +Cc: linux-pm, linux-kernel
From: Ming Lei <tom.leiming@gmail.com>
Support for device power domains has been introduced in
commit 9659cc0678b954f187290c6e8b247a673c5d37e1 (PM: Make
system-wide PM and runtime PM treat subsystems consistently),
also power domain callbacks will take precedence over subsystem ones
from commit 4d27e9dcff00a6425d779b065ec8892e4f391661(PM: Make
power domain callbacks take precedence over subsystem ones).
So update part of "Device Runtime PM Callbacks" in
Documentation/power/runtime_pm.txt.
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
Documentation/power/runtime_pm.txt | 19 ++++++++++++-------
1 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/Documentation/power/runtime_pm.txt b/Documentation/power/runtime_pm.txt
index 1f05404..0e85608 100644
--- a/Documentation/power/runtime_pm.txt
+++ b/Documentation/power/runtime_pm.txt
@@ -43,13 +43,18 @@ struct dev_pm_ops {
...
};
-The ->runtime_suspend(), ->runtime_resume() and ->runtime_idle() callbacks are
-executed by the PM core for either the device type, or the class (if the device
-type's struct dev_pm_ops object does not exist), or the bus type (if the
-device type's and class' struct dev_pm_ops objects do not exist) of the given
-device (this allows device types to override callbacks provided by bus types or
-classes if necessary). The bus type, device type and class callbacks are
-referred to as subsystem-level callbacks in what follows.
+The ->runtime_suspend(), ->runtime_resume() and ->runtime_idle() callbacks
+are executed by the PM core for either the power domain, or the device type
+(if the device power domain's struct dev_pm_ops does not exist), or the class
+(if the device power domain's and type's struct dev_pm_ops object does not
+exist), or the bus type (if the device power domain's, type's and class'
+struct dev_pm_ops objects do not exist) of the given device, so the priority
+order of callbacks from high to low is that power domain callbacks, device
+type callbacks, class callbacks and bus type callbacks, and the high priority
+one will take precedence over low priority one. The bus type, device type and
+class callbacks are referred to as subsystem-level callbacks in what follows,
+and generally speaking, the power domain callbacks are used for representing
+power domains within a SoC.
By default, the callbacks are always invoked in process context with interrupts
enabled. However, subsystems can use the pm_runtime_irq_safe() helper function
--
1.7.4.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox