linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ACPI: PM: Export the function acpi_sleep_state_supported()
@ 2019-06-14 18:19 Dexuan Cui
  2019-06-14 20:48 ` Michael Kelley
  0 siblings, 1 reply; 11+ messages in thread
From: Dexuan Cui @ 2019-06-14 18:19 UTC (permalink / raw)
  To: linux-acpi, rjw, lenb, robert.moore, erik.schmauss, Michael Kelley
  Cc: linux-hyperv, linux-kernel, KY Srinivasan, Stephen Hemminger,
	Haiyang Zhang, Sasha Levin, olaf, apw, jasowang, vkuznets,
	marcelo.cerri, Dexuan Cui

In a Linux VM running on Hyper-V, when ACPI S4 is enabled, the balloon
driver (drivers/hv/hv_balloon.c) needs to ask the host not to do memory
hot-add/remove.

So let's export acpi_sleep_state_supported() for the hv_balloon driver.
This might also be useful to the other drivers in the future.

Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
 drivers/acpi/sleep.c    | 3 ++-
 include/acpi/acpi_bus.h | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index a34deccd7317..69755411e008 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -79,7 +79,7 @@ static int acpi_sleep_prepare(u32 acpi_state)
 	return 0;
 }
 
-static bool acpi_sleep_state_supported(u8 sleep_state)
+bool acpi_sleep_state_supported(u8 sleep_state)
 {
 	acpi_status status;
 	u8 type_a, type_b;
@@ -89,6 +89,7 @@ static bool acpi_sleep_state_supported(u8 sleep_state)
 		|| (acpi_gbl_FADT.sleep_control.address
 			&& acpi_gbl_FADT.sleep_status.address));
 }
+EXPORT_SYMBOL_GPL(acpi_sleep_state_supported);
 
 #ifdef CONFIG_ACPI_SLEEP
 static u32 acpi_target_sleep_state = ACPI_STATE_S0;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 31b6c87d6240..5b102e7bbf25 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -651,6 +651,8 @@ static inline int acpi_pm_set_bridge_wakeup(struct device *dev, bool enable)
 }
 #endif
 
+bool acpi_sleep_state_supported(u8 sleep_state);
+
 #ifdef CONFIG_ACPI_SLEEP
 u32 acpi_target_system_state(void);
 #else
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* RE: [PATCH] ACPI: PM: Export the function acpi_sleep_state_supported()
  2019-06-14 18:19 [PATCH] ACPI: PM: Export the function acpi_sleep_state_supported() Dexuan Cui
@ 2019-06-14 20:48 ` Michael Kelley
  2019-06-14 22:19   ` Dexuan Cui
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Kelley @ 2019-06-14 20:48 UTC (permalink / raw)
  To: Dexuan Cui, linux-acpi, rjw, lenb, robert.moore, erik.schmauss
  Cc: linux-hyperv, linux-kernel, KY Srinivasan, Stephen Hemminger,
	Haiyang Zhang, Sasha Levin, olaf, apw, jasowang, vkuznets,
	marcelo.cerri

From: Dexuan Cui <decui@microsoft.com>  Sent: Friday, June 14, 2019 11:19 AM
> 
> In a Linux VM running on Hyper-V, when ACPI S4 is enabled, the balloon
> driver (drivers/hv/hv_balloon.c) needs to ask the host not to do memory
> hot-add/remove.
> 
> So let's export acpi_sleep_state_supported() for the hv_balloon driver.
> This might also be useful to the other drivers in the future.
> 
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>  drivers/acpi/sleep.c    | 3 ++-
>  include/acpi/acpi_bus.h | 2 ++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index a34deccd7317..69755411e008 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -79,7 +79,7 @@ static int acpi_sleep_prepare(u32 acpi_state)
>  	return 0;
>  }
> 
> -static bool acpi_sleep_state_supported(u8 sleep_state)
> +bool acpi_sleep_state_supported(u8 sleep_state)
>  {
>  	acpi_status status;
>  	u8 type_a, type_b;
> @@ -89,6 +89,7 @@ static bool acpi_sleep_state_supported(u8 sleep_state)
>  		|| (acpi_gbl_FADT.sleep_control.address
>  			&& acpi_gbl_FADT.sleep_status.address));
>  }
> +EXPORT_SYMBOL_GPL(acpi_sleep_state_supported);
> 
>  #ifdef CONFIG_ACPI_SLEEP
>  static u32 acpi_target_sleep_state = ACPI_STATE_S0;
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 31b6c87d6240..5b102e7bbf25 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -651,6 +651,8 @@ static inline int acpi_pm_set_bridge_wakeup(struct device *dev,
> bool enable)
>  }
>  #endif
> 
> +bool acpi_sleep_state_supported(u8 sleep_state);
> +
>  #ifdef CONFIG_ACPI_SLEEP
>  u32 acpi_target_system_state(void);
>  #else
> --
> 2.19.1

It seems that sleep.c isn't built when on the ARM64 architecture.  Using
acpi_sleep_state_supported() directly in hv_balloon.c will be problematic
since hv_balloon.c needs to be architecture independent when the
Hyper-V ARM64 support is added.  If that doesn't change, a per-architecture
wrapper will be needed to give hv_balloon.c the correct information.  This
may affect whether acpi_sleep_state_supported() needs to be exported vs.
just removing the "static".   I'm not sure what the best approach is.

Michael

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] ACPI: PM: Export the function acpi_sleep_state_supported()
  2019-06-14 20:48 ` Michael Kelley
@ 2019-06-14 22:19   ` Dexuan Cui
  2019-06-14 22:33     ` Russell King - ARM Linux admin
                       ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Dexuan Cui @ 2019-06-14 22:19 UTC (permalink / raw)
  To: Michael Kelley, linux-acpi, rjw, lenb, robert.moore,
	erik.schmauss, Russell King, Russ Dill, Sebastian Capella,
	Pavel Machek, Lorenzo Pieralisi
  Cc: linux-hyperv, linux-kernel, KY Srinivasan, Stephen Hemminger,
	Haiyang Zhang, Sasha Levin, olaf, apw, jasowang, vkuznets,
	marcelo.cerri

> -----Original Message-----
> From: Michael Kelley <mikelley@microsoft.com>
> Sent: Friday, June 14, 2019 1:48 PM
> To: Dexuan Cui <decui@microsoft.com>; linux-acpi@vger.kernel.org;
> rjw@rjwysocki.net; lenb@kernel.org; robert.moore@intel.com;
> erik.schmauss@intel.com
> Cc: linux-hyperv@vger.kernel.org; linux-kernel@vger.kernel.org; KY Srinivasan
> <kys@microsoft.com>; Stephen Hemminger <sthemmin@microsoft.com>;
> Haiyang Zhang <haiyangz@microsoft.com>; Sasha Levin
> <Alexander.Levin@microsoft.com>; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com; vkuznets <vkuznets@redhat.com>;
> marcelo.cerri@canonical.com
> Subject: RE: [PATCH] ACPI: PM: Export the function
> acpi_sleep_state_supported()
> 
> From: Dexuan Cui <decui@microsoft.com>  Sent: Friday, June 14, 2019 11:19
> AM
> >
> > In a Linux VM running on Hyper-V, when ACPI S4 is enabled, the balloon
> > driver (drivers/hv/hv_balloon.c) needs to ask the host not to do memory
> > hot-add/remove.
> >
> > So let's export acpi_sleep_state_supported() for the hv_balloon driver.
> > This might also be useful to the other drivers in the future.
> >
> > Signed-off-by: Dexuan Cui <decui@microsoft.com>
> > ---
> >  drivers/acpi/sleep.c    | 3 ++-
> >  include/acpi/acpi_bus.h | 2 ++
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> > index a34deccd7317..69755411e008 100644
> > --- a/drivers/acpi/sleep.c
> > +++ b/drivers/acpi/sleep.c
> > @@ -79,7 +79,7 @@ static int acpi_sleep_prepare(u32 acpi_state)
> >  	return 0;
> >  }
> >
> > -static bool acpi_sleep_state_supported(u8 sleep_state)
> > +bool acpi_sleep_state_supported(u8 sleep_state)
> >  {
> >  	acpi_status status;
> >  	u8 type_a, type_b;
> > @@ -89,6 +89,7 @@ static bool acpi_sleep_state_supported(u8 sleep_state)
> >  		|| (acpi_gbl_FADT.sleep_control.address
> >  			&& acpi_gbl_FADT.sleep_status.address));
> >  }
> > +EXPORT_SYMBOL_GPL(acpi_sleep_state_supported);
> >
> >  #ifdef CONFIG_ACPI_SLEEP
> >  static u32 acpi_target_sleep_state = ACPI_STATE_S0;
> > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> > index 31b6c87d6240..5b102e7bbf25 100644
> > --- a/include/acpi/acpi_bus.h
> > +++ b/include/acpi/acpi_bus.h
> > @@ -651,6 +651,8 @@ static inline int acpi_pm_set_bridge_wakeup(struct
> device *dev,
> > bool enable)
> >  }
> >  #endif
> >
> > +bool acpi_sleep_state_supported(u8 sleep_state);
> > +
> >  #ifdef CONFIG_ACPI_SLEEP
> >  u32 acpi_target_system_state(void);
> >  #else
> > --
> > 2.19.1
> 
> It seems that sleep.c isn't built when on the ARM64 architecture.  Using
> acpi_sleep_state_supported() directly in hv_balloon.c will be problematic
> since hv_balloon.c needs to be architecture independent when the
> Hyper-V ARM64 support is added.  If that doesn't change, a per-architecture
> wrapper will be needed to give hv_balloon.c the correct information.  This
> may affect whether acpi_sleep_state_supported() needs to be exported vs.
> just removing the "static".   I'm not sure what the best approach is.
> 
> Michael

+ some ARM experts who worked on arch/arm/kernel/hibernate.c.

drivers/acpi/sleep.c is only built if ACPI_SYSTEM_POWER_STATES_SUPPORT
is defined, but it looks this option is not defined on ARM.

It looks ARM does not support the ACPI S4 state, then how do we know 
if an ARM host supports hibernation or not?

Thanks,
-- Dexuan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] ACPI: PM: Export the function acpi_sleep_state_supported()
  2019-06-14 22:19   ` Dexuan Cui
@ 2019-06-14 22:33     ` Russell King - ARM Linux admin
  2019-06-14 23:34       ` Dexuan Cui
  2019-06-17 13:08     ` Pavel Machek
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Russell King - ARM Linux admin @ 2019-06-14 22:33 UTC (permalink / raw)
  To: Dexuan Cui
  Cc: Michael Kelley, linux-acpi, rjw, lenb, robert.moore,
	erik.schmauss, Russ Dill, Sebastian Capella, Pavel Machek,
	Lorenzo Pieralisi, linux-hyperv, linux-kernel, KY Srinivasan,
	Stephen Hemminger, Haiyang Zhang, Sasha Levin, olaf, apw,
	jasowang, vkuznets, marcelo.cerri

Hi,

On Fri, Jun 14, 2019 at 10:19:02PM +0000, Dexuan Cui wrote:
> > -----Original Message-----
> > From: Michael Kelley <mikelley@microsoft.com>
> > Sent: Friday, June 14, 2019 1:48 PM
> > To: Dexuan Cui <decui@microsoft.com>; linux-acpi@vger.kernel.org;
> > rjw@rjwysocki.net; lenb@kernel.org; robert.moore@intel.com;
> > erik.schmauss@intel.com
> > Cc: linux-hyperv@vger.kernel.org; linux-kernel@vger.kernel.org; KY Srinivasan
> > <kys@microsoft.com>; Stephen Hemminger <sthemmin@microsoft.com>;
> > Haiyang Zhang <haiyangz@microsoft.com>; Sasha Levin
> > <Alexander.Levin@microsoft.com>; olaf@aepfle.de; apw@canonical.com;
> > jasowang@redhat.com; vkuznets <vkuznets@redhat.com>;
> > marcelo.cerri@canonical.com
> > Subject: RE: [PATCH] ACPI: PM: Export the function
> > acpi_sleep_state_supported()
> > 
> > It seems that sleep.c isn't built when on the ARM64 architecture.  Using
> > acpi_sleep_state_supported() directly in hv_balloon.c will be problematic
> > since hv_balloon.c needs to be architecture independent when the
> > Hyper-V ARM64 support is added.  If that doesn't change, a per-architecture
> > wrapper will be needed to give hv_balloon.c the correct information.  This
> > may affect whether acpi_sleep_state_supported() needs to be exported vs.
> > just removing the "static".   I'm not sure what the best approach is.
> > 
> > Michael
> 
> + some ARM experts who worked on arch/arm/kernel/hibernate.c.
> 
> drivers/acpi/sleep.c is only built if ACPI_SYSTEM_POWER_STATES_SUPPORT
> is defined, but it looks this option is not defined on ARM.
> 
> It looks ARM does not support the ACPI S4 state, then how do we know 
> if an ARM host supports hibernation or not?

Don't forget that Linux does not support ACPI on 32-bit ARM, which is
quite different from the situation on 64-bit ARM.

arch/arm/kernel/hibernate.c is only for 32-bit ARM, and is written with
the assumption that there is no interaction required with any firmware
to save state, and later restore state upon resuming.

Or am I missing something?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] ACPI: PM: Export the function acpi_sleep_state_supported()
  2019-06-14 22:33     ` Russell King - ARM Linux admin
@ 2019-06-14 23:34       ` Dexuan Cui
  0 siblings, 0 replies; 11+ messages in thread
From: Dexuan Cui @ 2019-06-14 23:34 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Michael Kelley, linux-acpi, rjw, lenb, robert.moore,
	erik.schmauss, Russ Dill, Sebastian Capella, Pavel Machek,
	Lorenzo Pieralisi, linux-hyperv, linux-kernel, KY Srinivasan,
	Stephen Hemminger, Haiyang Zhang, Sasha Levin, olaf, apw,
	jasowang, vkuznets, marcelo.cerri

> From: linux-hyperv-owner@vger.kernel.org
> <linux-hyperv-owner@vger.kernel.org> On Behalf Of Russell King
> On Fri, Jun 14, 2019 at 10:19:02PM +0000, Dexuan Cui wrote:
> > It looks ARM does not support the ACPI S4 state, then how do we know
> > if an ARM host supports hibernation or not?
> 
> Don't forget that Linux does not support ACPI on 32-bit ARM, which is
> quite different from the situation on 64-bit ARM.
> 
> arch/arm/kernel/hibernate.c is only for 32-bit ARM, and is written with
> the assumption that there is no interaction required with any firmware
> to save state, and later restore state upon resuming.
> 
> Or am I missing something?

Hi Russell,
Thanks for your reply and please excuse me for my ignorance of ARM. 

So 32-bit ARM Linux can hibernate even if it doesn't support ACPI, but
I guess not all 32-bit ARM machines support hibernation? If my guess
is correct, is there any standard capability bit or something that can be
used to tell if an ARM machine supports hibernation? I'm purely curious. :-)

Do you imply 64-bit ARM Linux supports ACPI and the ACPI S4 state?
If not, how can we tell if a 64-bit ARM machine supports hibernation or not?

Thanks,
-- Dexuan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] ACPI: PM: Export the function acpi_sleep_state_supported()
  2019-06-14 22:19   ` Dexuan Cui
  2019-06-14 22:33     ` Russell King - ARM Linux admin
@ 2019-06-17 13:08     ` Pavel Machek
  2019-06-17 13:09     ` Pavel Machek
  2019-06-17 16:14     ` Lorenzo Pieralisi
  3 siblings, 0 replies; 11+ messages in thread
From: Pavel Machek @ 2019-06-17 13:08 UTC (permalink / raw)
  To: Dexuan Cui
  Cc: Michael Kelley, linux-acpi, rjw, lenb, robert.moore,
	erik.schmauss, Russell King, Russ Dill, Sebastian Capella,
	Lorenzo Pieralisi, linux-hyperv, linux-kernel, KY Srinivasan,
	Stephen Hemminger, Haiyang Zhang, Sasha Levin, olaf, apw,
	jasowang, vkuznets, marcelo.cerri

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

Hi!

> > > In a Linux VM running on Hyper-V, when ACPI S4 is enabled, the balloon
> > > driver (drivers/hv/hv_balloon.c) needs to ask the host not to do memory
> > > hot-add/remove.
> > >
> > > So let's export acpi_sleep_state_supported() for the hv_balloon driver.
> > > This might also be useful to the other drivers in the future.
> > >
> > > Signed-off-by: Dexuan Cui <decui@microsoft.com>

> > 
> > It seems that sleep.c isn't built when on the ARM64 architecture.  Using
> > acpi_sleep_state_supported() directly in hv_balloon.c will be problematic
> > since hv_balloon.c needs to be architecture independent when the
> > Hyper-V ARM64 support is added.  If that doesn't change, a per-architecture
> > wrapper will be needed to give hv_balloon.c the correct information.  This
> > may affect whether acpi_sleep_state_supported() needs to be exported vs.
> > just removing the "static".   I'm not sure what the best approach is.
> > 
> > Michael
> 
> + some ARM experts who worked on arch/arm/kernel/hibernate.c.
> 
> drivers/acpi/sleep.c is only built if ACPI_SYSTEM_POWER_STATES_SUPPORT
> is defined, but it looks this option is not defined on ARM.
> 
> It looks ARM does not support the ACPI S4 state, then how do we know 
> if an ARM host supports hibernation or not?

You should be able to do hibernation without ACPI S4 support. All you
need is ability to powerdown...

It is well possible that noone tested hibernation on ARM.. people
usually do suspend-to-ram there.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] ACPI: PM: Export the function acpi_sleep_state_supported()
  2019-06-14 22:19   ` Dexuan Cui
  2019-06-14 22:33     ` Russell King - ARM Linux admin
  2019-06-17 13:08     ` Pavel Machek
@ 2019-06-17 13:09     ` Pavel Machek
  2019-06-17 16:14     ` Lorenzo Pieralisi
  3 siblings, 0 replies; 11+ messages in thread
From: Pavel Machek @ 2019-06-17 13:09 UTC (permalink / raw)
  To: Dexuan Cui
  Cc: Michael Kelley, linux-acpi, rjw, lenb, robert.moore,
	erik.schmauss, Russell King, Russ Dill, Sebastian Capella,
	Lorenzo Pieralisi, linux-hyperv, linux-kernel, KY Srinivasan,
	Stephen Hemminger, Haiyang Zhang, Sasha Levin, olaf, apw,
	jasowang, vkuznets, marcelo.cerri

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


> > It seems that sleep.c isn't built when on the ARM64 architecture.  Using
> > acpi_sleep_state_supported() directly in hv_balloon.c will be problematic
> > since hv_balloon.c needs to be architecture independent when the
> > Hyper-V ARM64 support is added.  If that doesn't change, a per-architecture
> > wrapper will be needed to give hv_balloon.c the correct information.  This
> > may affect whether acpi_sleep_state_supported() needs to be exported vs.
> > just removing the "static".   I'm not sure what the best approach is.
> > 
> > Michael
> 
> + some ARM experts who worked on arch/arm/kernel/hibernate.c.
> 
> drivers/acpi/sleep.c is only built if ACPI_SYSTEM_POWER_STATES_SUPPORT
> is defined, but it looks this option is not defined on ARM.
> 
> It looks ARM does not support the ACPI S4 state, then how do we know 
> if an ARM host supports hibernation or not?

But actually... I remember ELCE talk about hibernation or ARM32. Not
sure if patches are mainline, but someone was working on that.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] ACPI: PM: Export the function acpi_sleep_state_supported()
  2019-06-14 22:19   ` Dexuan Cui
                       ` (2 preceding siblings ...)
  2019-06-17 13:09     ` Pavel Machek
@ 2019-06-17 16:14     ` Lorenzo Pieralisi
  2019-06-19 19:54       ` Dexuan Cui
  3 siblings, 1 reply; 11+ messages in thread
From: Lorenzo Pieralisi @ 2019-06-17 16:14 UTC (permalink / raw)
  To: Dexuan Cui
  Cc: Michael Kelley, linux-acpi, rjw, lenb, robert.moore,
	erik.schmauss, Russell King, Russ Dill, Sebastian Capella,
	Pavel Machek, linux-hyperv, linux-kernel, KY Srinivasan,
	Stephen Hemminger, Haiyang Zhang, Sasha Levin, olaf, apw,
	jasowang, vkuznets, marcelo.cerri

On Fri, Jun 14, 2019 at 10:19:02PM +0000, Dexuan Cui wrote:
> > -----Original Message-----
> > From: Michael Kelley <mikelley@microsoft.com>
> > Sent: Friday, June 14, 2019 1:48 PM
> > To: Dexuan Cui <decui@microsoft.com>; linux-acpi@vger.kernel.org;
> > rjw@rjwysocki.net; lenb@kernel.org; robert.moore@intel.com;
> > erik.schmauss@intel.com
> > Cc: linux-hyperv@vger.kernel.org; linux-kernel@vger.kernel.org; KY Srinivasan
> > <kys@microsoft.com>; Stephen Hemminger <sthemmin@microsoft.com>;
> > Haiyang Zhang <haiyangz@microsoft.com>; Sasha Levin
> > <Alexander.Levin@microsoft.com>; olaf@aepfle.de; apw@canonical.com;
> > jasowang@redhat.com; vkuznets <vkuznets@redhat.com>;
> > marcelo.cerri@canonical.com
> > Subject: RE: [PATCH] ACPI: PM: Export the function
> > acpi_sleep_state_supported()
> > 
> > From: Dexuan Cui <decui@microsoft.com>  Sent: Friday, June 14, 2019 11:19
> > AM
> > >
> > > In a Linux VM running on Hyper-V, when ACPI S4 is enabled, the balloon
> > > driver (drivers/hv/hv_balloon.c) needs to ask the host not to do memory
> > > hot-add/remove.
> > >
> > > So let's export acpi_sleep_state_supported() for the hv_balloon driver.
> > > This might also be useful to the other drivers in the future.
> > >
> > > Signed-off-by: Dexuan Cui <decui@microsoft.com>
> > > ---
> > >  drivers/acpi/sleep.c    | 3 ++-
> > >  include/acpi/acpi_bus.h | 2 ++
> > >  2 files changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> > > index a34deccd7317..69755411e008 100644
> > > --- a/drivers/acpi/sleep.c
> > > +++ b/drivers/acpi/sleep.c
> > > @@ -79,7 +79,7 @@ static int acpi_sleep_prepare(u32 acpi_state)
> > >  	return 0;
> > >  }
> > >
> > > -static bool acpi_sleep_state_supported(u8 sleep_state)
> > > +bool acpi_sleep_state_supported(u8 sleep_state)
> > >  {
> > >  	acpi_status status;
> > >  	u8 type_a, type_b;
> > > @@ -89,6 +89,7 @@ static bool acpi_sleep_state_supported(u8 sleep_state)
> > >  		|| (acpi_gbl_FADT.sleep_control.address
> > >  			&& acpi_gbl_FADT.sleep_status.address));
> > >  }
> > > +EXPORT_SYMBOL_GPL(acpi_sleep_state_supported);
> > >
> > >  #ifdef CONFIG_ACPI_SLEEP
> > >  static u32 acpi_target_sleep_state = ACPI_STATE_S0;
> > > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> > > index 31b6c87d6240..5b102e7bbf25 100644
> > > --- a/include/acpi/acpi_bus.h
> > > +++ b/include/acpi/acpi_bus.h
> > > @@ -651,6 +651,8 @@ static inline int acpi_pm_set_bridge_wakeup(struct
> > device *dev,
> > > bool enable)
> > >  }
> > >  #endif
> > >
> > > +bool acpi_sleep_state_supported(u8 sleep_state);
> > > +
> > >  #ifdef CONFIG_ACPI_SLEEP
> > >  u32 acpi_target_system_state(void);
> > >  #else
> > > --
> > > 2.19.1
> > 
> > It seems that sleep.c isn't built when on the ARM64 architecture.  Using
> > acpi_sleep_state_supported() directly in hv_balloon.c will be problematic
> > since hv_balloon.c needs to be architecture independent when the
> > Hyper-V ARM64 support is added.  If that doesn't change, a per-architecture
> > wrapper will be needed to give hv_balloon.c the correct information.  This
> > may affect whether acpi_sleep_state_supported() needs to be exported vs.
> > just removing the "static".   I'm not sure what the best approach is.
> > 
> > Michael
> 
> + some ARM experts who worked on arch/arm/kernel/hibernate.c.
> 
> drivers/acpi/sleep.c is only built if ACPI_SYSTEM_POWER_STATES_SUPPORT
> is defined, but it looks this option is not defined on ARM.
> 
> It looks ARM does not support the ACPI S4 state, then how do we know 
> if an ARM host supports hibernation or not?

Maybe we should start from understanding why you need to know whether
Hibernate is possible to answer your question ?

On ARM64 platforms system states are entered through PSCI firmware
interface that works for ACPI and device tree alike.

Lorenzo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] ACPI: PM: Export the function acpi_sleep_state_supported()
  2019-06-17 16:14     ` Lorenzo Pieralisi
@ 2019-06-19 19:54       ` Dexuan Cui
  2019-06-20 11:30         ` Pavel Machek
  0 siblings, 1 reply; 11+ messages in thread
From: Dexuan Cui @ 2019-06-19 19:54 UTC (permalink / raw)
  To: Lorenzo Pieralisi, linux-acpi, rjw, lenb, robert.moore,
	erik.schmauss, Russell King, Russ Dill, Sebastian Capella,
	Pavel Machek
  Cc: Michael Kelley, linux-hyperv, linux-kernel, KY Srinivasan,
	Stephen Hemminger, Haiyang Zhang, Sasha Levin, olaf, apw,
	jasowang, vkuznets, marcelo.cerri

> From: linux-hyperv-owner@vger.kernel.org
> <linux-hyperv-owner@vger.kernel.org> On Behalf Of Lorenzo Pieralisi
> Sent: Monday, June 17, 2019 9:15 AM
> > ...
> > + some ARM experts who worked on arch/arm/kernel/hibernate.c.
> >
> > drivers/acpi/sleep.c is only built if ACPI_SYSTEM_POWER_STATES_SUPPORT
> > is defined, but it looks this option is not defined on ARM.
> >
> > It looks ARM does not support the ACPI S4 state, then how do we know
> > if an ARM host supports hibernation or not?
> 
> Maybe we should start from understanding why you need to know whether
> Hibernate is possible to answer your question ?
> 
> On ARM64 platforms system states are entered through PSCI firmware
> interface that works for ACPI and device tree alike.
> 
> Lorenzo

Hi Lorenzo,
It looks I may have confused you as I didn't realize the word "ARM" only means
32-bit ARM. It looks the "ARM" arch and the "ARM64" arch are very different.

As far as I know, Hyper-V only supports x86 and "ARM64", and it's unlikely to
support 32-bit ARM in the future, so actually I don't really need to know if and
how a 32-bit ARM machine supports hibernation.

When a Linux guest runs on Hyper-V (x86_32, x86_64, or ARM64) , we have a
front-end balloon driver in the guest, which balloons up/down and
hot adds/removes the guest's memory when the host requests that. The problem
is: the back-end driver on the host can not really save and restore the states
related to the front-end balloon driver on guest hibernation, so we made the
decision that balloon up/down and hot-add/remove are not supported when
we enable hibernation for a guest; BTW, we still want to load the front-end
driver in the guest, because the dirver has a functionality of reporting the
guest's memory pressure to the host, which we think is useful.

On x86_32 and x86_64, we enable hibernation for a guest by enabling
the virtual ACPI S4 state for the guest; on ARM64, so far we don't have the
host side changes required to support guest hibernation, so the details are
still unclear.

After I discussed with Michael Kelley, it looks we don't really need to
export drivers/acpi/sleep.c: acpi_sleep_state_supported(), but I think we do
need to make it non-static.

Now I propose the below changes. I plan to submit a patch first for the
changes made to drivers/acpi/sleep.c and include/acpi/acpi_bus.h in a few
days, if there is no objection.

Please let me know how you think of this. Thanks!

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index a34deccd7317..18d2e5922448 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -79,7 +79,7 @@ static int acpi_sleep_prepare(u32 acpi_state)
 	return 0;
 }
 
-static bool acpi_sleep_state_supported(u8 sleep_state)
+bool acpi_sleep_state_supported(u8 sleep_state)
 {
 	acpi_status status;
 	u8 type_a, type_b;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 31b6c87d6240..3e6563e1a2c0 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -651,6 +651,12 @@ static inline int acpi_pm_set_bridge_wakeup(struct device *dev, bool enable)
 }
 #endif
 
+#ifdef CONFIG_ACPI_SYSTEM_POWER_STATES_SUPPORT
+bool acpi_sleep_state_supported(u8 sleep_state);
+#else
+bool acpi_sleep_state_supported(u8 sleep_state) { return false; }
+#endif
+
 #ifdef CONFIG_ACPI_SLEEP
 u32 acpi_target_system_state(void);
 #else
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 79f626ab8306..197e8fa32871 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -17,6 +17,7 @@
  *
  */
 
+#include <linux/acpi.h>
 #include <linux/efi.h>
 #include <linux/types.h>
 #include <asm/apic.h>
@@ -420,3 +421,9 @@ bool hv_is_hyperv_initialized(void)
 	return hypercall_msr.enable;
 }
 EXPORT_SYMBOL_GPL(hv_is_hyperv_initialized);
+
+bool hv_is_hibernation_supported(void)
+{
+	return acpi_sleep_state_supported(ACPI_STATE_S4);
+}
+EXPORT_SYMBOL_GPL(hv_is_hibernation_supported);
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index 0becb7d9704d..1cb40016ee53 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -166,9 +166,11 @@ static inline int cpumask_to_vpset(struct hv_vpset *vpset,
 void hyperv_report_panic(struct pt_regs *regs, long err);
 void hyperv_report_panic_msg(phys_addr_t pa, size_t size);
 bool hv_is_hyperv_initialized(void);
+bool hv_is_hibernation_supported(void);
 void hyperv_cleanup(void);
 #else /* CONFIG_HYPERV */
 static inline bool hv_is_hyperv_initialized(void) { return false; }
+static inline bool hv_is_hibernation_supported(void) { return false; }
 static inline void hyperv_cleanup(void) {}
 #endif /* CONFIG_HYPERV */
 
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 111ea3599659..39fd4e4a8fd7 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -34,6 +34,8 @@
 
 #include <linux/hyperv.h>
 
+#include <asm/mshyperv.h>
+
 #define CREATE_TRACE_POINTS
 #include "hv_trace_balloon.h"
 
@@ -1682,6 +1684,9 @@ static int balloon_probe(struct hv_device *dev,
 {
 	int ret;
 
+	if (hv_is_hibernation_supported())
+		hot_add = false;
+
 #ifdef CONFIG_MEMORY_HOTPLUG
 	do_hot_add = hot_add;
 #else


Thanks,
-- Dexuan

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] ACPI: PM: Export the function acpi_sleep_state_supported()
  2019-06-19 19:54       ` Dexuan Cui
@ 2019-06-20 11:30         ` Pavel Machek
  2019-06-21  7:15           ` Dexuan Cui
  0 siblings, 1 reply; 11+ messages in thread
From: Pavel Machek @ 2019-06-20 11:30 UTC (permalink / raw)
  To: Dexuan Cui
  Cc: Lorenzo Pieralisi, linux-acpi, rjw, lenb, robert.moore,
	erik.schmauss, Russell King, Russ Dill, Sebastian Capella,
	Michael Kelley, linux-hyperv, linux-kernel, KY Srinivasan,
	Stephen Hemminger, Haiyang Zhang, Sasha Levin, olaf, apw,
	jasowang, vkuznets, marcelo.cerri

> > From: linux-hyperv-owner@vger.kernel.org
> > <linux-hyperv-owner@vger.kernel.org> On Behalf Of Lorenzo Pieralisi
> > Sent: Monday, June 17, 2019 9:15 AM
> > > ...
> > > + some ARM experts who worked on arch/arm/kernel/hibernate.c.
> > >
> > > drivers/acpi/sleep.c is only built if ACPI_SYSTEM_POWER_STATES_SUPPORT
> > > is defined, but it looks this option is not defined on ARM.
> > >
> > > It looks ARM does not support the ACPI S4 state, then how do we know
> > > if an ARM host supports hibernation or not?
> > 
> > Maybe we should start from understanding why you need to know whether
> > Hibernate is possible to answer your question ?
> > 
> > On ARM64 platforms system states are entered through PSCI firmware
> > interface that works for ACPI and device tree alike.
> > 
> > Lorenzo
> 
> Hi Lorenzo,
> It looks I may have confused you as I didn't realize the word "ARM" only means
> 32-bit ARM. It looks the "ARM" arch and the "ARM64" arch are very different.
> 
> As far as I know, Hyper-V only supports x86 and "ARM64", and it's unlikely to
> support 32-bit ARM in the future, so actually I don't really need to know if and
> how a 32-bit ARM machine supports hibernation.
> 
> When a Linux guest runs on Hyper-V (x86_32, x86_64, or ARM64) , we have a
> front-end balloon driver in the guest, which balloons up/down and
> hot adds/removes the guest's memory when the host requests that. The problem
> is: the back-end driver on the host can not really save and restore the states
> related to the front-end balloon driver on guest hibernation, so we made the
> decision that balloon up/down and hot-add/remove are not supported when
> we enable hibernation for a guest; BTW, we still want to load the front-end
> driver in the guest, because the dirver has a functionality of reporting the
> guest's memory pressure to the host, which we think is useful.
> 
> On x86_32 and x86_64, we enable hibernation for a guest by enabling
> the virtual ACPI S4 state for the guest; on ARM64, so far we don't have the
> host side changes required to support guest hibernation, so the details are
> still unclear.
> 
> After I discussed with Michael Kelley, it looks we don't really need to
> export drivers/acpi/sleep.c: acpi_sleep_state_supported(), but I think we do
> need to make it non-static.
> 
> Now I propose the below changes. I plan to submit a patch first for the
> changes made to drivers/acpi/sleep.c and include/acpi/acpi_bus.h in a few
> days, if there is no objection.
> 
> Please let me know how you think of this. Thanks!

No.

Hibernation should be always supported, no matter what firmware. If it
can powerdown, it can hibernate.

That is for x86-32/64, too.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] ACPI: PM: Export the function acpi_sleep_state_supported()
  2019-06-20 11:30         ` Pavel Machek
@ 2019-06-21  7:15           ` Dexuan Cui
  0 siblings, 0 replies; 11+ messages in thread
From: Dexuan Cui @ 2019-06-21  7:15 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Lorenzo Pieralisi, linux-acpi, rjw, lenb, robert.moore,
	erik.schmauss, Russell King, Russ Dill, Sebastian Capella,
	Michael Kelley, linux-hyperv, linux-kernel, KY Srinivasan,
	Stephen Hemminger, Haiyang Zhang, Sasha Levin, olaf, apw,
	jasowang, vkuznets, marcelo.cerri

> From: linux-hyperv-owner@vger.kernel.org
> > ...
> > When a Linux guest runs on Hyper-V (x86_32, x86_64, or ARM64) , we have a
> > front-end balloon driver in the guest, which balloons up/down and
> > hot adds/removes the guest's memory when the host requests that. The
> > problem
> > is: the back-end driver on the host can not really save and restore the states
> > related to the front-end balloon driver on guest hibernation, so we made the
> > decision that balloon up/down and hot-add/remove are not supported when
> > we enable hibernation for a guest; BTW, we still want to load the front-end
> > driver in the guest, because the driver has a functionality of reporting the
> > guest's memory pressure to the host, which we think is useful.
> >
> > On x86_32 and x86_64, we enable hibernation for a guest by enabling
> > the virtual ACPI S4 state for the guest; on ARM64, so far we don't have the
> > host side changes required to support guest hibernation, so the details are
> > still unclear.
> >
> > After I discussed with Michael Kelley, it looks we don't really need to
> > export drivers/acpi/sleep.c: acpi_sleep_state_supported(), but I think we do
> > need to make it non-static.
> >
> > Now I propose the below changes. I plan to submit a patch first for the
> > changes made to drivers/acpi/sleep.c and include/acpi/acpi_bus.h in a few
> > days, if there is no objection.
> >
> > Please let me know how you think of this. Thanks!
> 
> No.
> 
> Hibernation should be always supported, no matter what firmware. If it
> can powerdown, it can hibernate.
> 
> That is for x86-32/64, too.
> 						Pavel

Hi Pavel,
Yes, I totally agree hibernation should be always supported, as long as the
system can power down. This is also true for Linux guest running on
Hyper-V, when the pava-virtualized drivers are not loaded in the guest

However, unluckily the situation is a little different when the pava-virtualized
drivers are loaded in the guest:

1) Old Hyper-V hosts are unable to support guest hibernation and these hosts
don't support the virtual ACPI S4 state.

2) The recent Hyper-V host is able to support guest hibernation, but when
guest hibernation is used, the guest needs to disable some features in the
guest balloon driver, as I explained in the previous mail; on the other hand,
we also want to keep the ability to enable the features, so the Hyper-V guys 
decided to use the absence of the virtual ACPI S4 state to signify the ability
of enabling the features, and there is a host command tool which can
enable or disable the virtual ACPI S4 state for a guest.

In summary:
1) On old Hyper-V hosts or a recent host, the virtual ACPI S4 state is
unsupported or disabled, respectively, and we don't allow Linux guest
hibernation so we can use the full features of the guest balloon driver.

2) On a recent host, after we use the host command tool to enable the
virtual ACPI S4 state, we allow Linux guest hibernation and we only use a
subset of the full features of the guest balloon driver. 

This is why I need to know if the virtual ACPI S4 state is supported-and-enabled.

Exporting acpi_sleep_state_supported() may be overkill, and now making it
non-static seems the only way to help me out. IMO the function is unlikely
to change in the future, and it should be stable enough to become non-static.

Looking forward to your insights!

Thanks,
-- Dexuan

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-06-21  7:16 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-14 18:19 [PATCH] ACPI: PM: Export the function acpi_sleep_state_supported() Dexuan Cui
2019-06-14 20:48 ` Michael Kelley
2019-06-14 22:19   ` Dexuan Cui
2019-06-14 22:33     ` Russell King - ARM Linux admin
2019-06-14 23:34       ` Dexuan Cui
2019-06-17 13:08     ` Pavel Machek
2019-06-17 13:09     ` Pavel Machek
2019-06-17 16:14     ` Lorenzo Pieralisi
2019-06-19 19:54       ` Dexuan Cui
2019-06-20 11:30         ` Pavel Machek
2019-06-21  7:15           ` Dexuan Cui

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).