[Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

[ankitprasad . r . sharma]

From: Ankitprasad Sharma 

The BIOS RapidStartTechnology may corrupt the stolen memory across S3
suspend due to unalarmed hibernation, in which case we will not be able
to preserve the User data stored in the stolen region. Hence this patch
tries to identify presence of the RST device on the ACPI bus, and
disables use of stolen memory (for persistent data) if found.

v2: Updated comment, updated/corrected new functions private to driver
(Chris/Tvrtko)

v3: Disabling stolen by default, wait till required acpi changes to
detect device presence are pulled in (Ankit)

v4: Enabled stolen by default as required acpi changes are merged
(Ankit)

v5: renamed variable, is IS_ENABLED() in place of #ifdef, use char*
instead of structures (Lukas)

v6: Return CREATE_VERSION as 1 if stolen memory is volatile due
to presence of Intel RST (CI testing)

Signed-off-by: Ankitprasad Sharma 
Cc: Lukas Wunner 
---
 drivers/gpu/drm/i915/i915_dma.c|  5 -
 drivers/gpu/drm/i915/i915_drv.h| 11 +++
 drivers/gpu/drm/i915/i915_gem.c|  8 
 drivers/gpu/drm/i915/i915_gem_stolen.c | 12 
 drivers/gpu/drm/i915/intel_acpi.c  |  7 +++
 5 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2ae460e..6f8d971 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -173,7 +173,10 @@ static int i915_getparam(struct drm_device *dev, void 
*data,
value = 1;
break;
case I915_PARAM_CREATE_VERSION:
-   value = 2;
+   if (dev_priv->mm.volatile_stolen)
+   value = 1;
+   else
+   value = 2;
break;
default:
DRM_DEBUG("Unknown parameter %d\n", param->param);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f9c580e..50ed31e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1301,6 +1301,16 @@ struct i915_gem_mm {
 */
bool busy;
 
+   /**
+* Stolen will be lost upon hibernate (as the memory is unpowered).
+* Across resume, we expect stolen to be intact - however, it may
+* also be utililised by third parties (e.g. Intel RapidStart
+* Technology) and if so we have to assume that any data stored in
+* stolen across resume is lost and we set this flag to indicate that
+* the stolen memory is volatile.
+*/
+   bool volatile_stolen;
+
/* the indicator for dispatch video commands on two BSD rings */
unsigned int bsd_ring_dispatch_index;
 
@@ -3427,6 +3437,7 @@ intel_opregion_notify_adapter(struct drm_device *dev, 
pci_power_t state)
 #endif
 
 /* intel_acpi.c */
+bool intel_detect_acpi_rst(void);
 #ifdef CONFIG_ACPI
 extern void intel_register_dsm_handler(void);
 extern void intel_unregister_dsm_handler(void);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e96975a..ddc0d5f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -396,8 +396,16 @@ static struct drm_i915_gem_object *
 i915_gem_alloc_object_stolen(struct drm_device *dev, size_t size)
 {
struct drm_i915_gem_object *obj;
+   struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
 
+   if (dev_priv->mm.volatile_stolen) {
+   /* Stolen may be overwritten by external parties
+* so unsuitable for persistent user data.
+*/
+   return ERR_PTR(-ENODEV);
+   }
+
mutex_lock(>struct_mutex);
obj = i915_gem_object_create_stolen(dev, size);
if (IS_ERR(obj))
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 6244dae..9238a79 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -485,6 +485,18 @@ int i915_gem_init_stolen(struct drm_device *dev)
 */
drm_mm_init(_priv->mm.stolen, 0, dev_priv->gtt.stolen_usable_size);
 
+   /* If the stolen region can be modified behind our backs upon suspend,
+* then we cannot use it to store nonvolatile contents (i.e user data)
+* as it will be corrupted upon resume.
+*/
+   dev_priv->mm.volatile_stolen = false;
+   if (IS_ENABLED(CONFIG_SUSPEND)) {
+   /* BIOSes using RapidStart Technology have been reported
+* to overwrite stolen across S3, not just S4.
+*/
+   dev_priv->mm.volatile_stolen = intel_detect_acpi_rst();
+   }
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_acpi.c 
b/drivers/gpu/drm/i915/intel_acpi.c
index eb638a1..05fd67f 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ 

[Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

[ankitprasad . r . sharma]

From: Ankitprasad Sharma 

The BIOS RapidStartTechnology may corrupt the stolen memory across S3
suspend due to unalarmed hibernation, in which case we will not be able
to preserve the User data stored in the stolen region. Hence this patch
tries to identify presence of the RST device on the ACPI bus, and
disables use of stolen memory (for persistent data) if found.

v2: Updated comment, updated/corrected new functions private to driver
(Chris/Tvrtko)

v3: Disabling stolen by default, wait till required acpi changes to
detect device presence are pulled in (Ankit)

v4: Enabled stolen by default as required acpi changes are merged
(Ankit)

v5: renamed variable, is IS_ENABLED() in place of #ifdef, use char*
instead of structures (Lukas)

Signed-off-by: Ankitprasad Sharma 
Cc: Lukas Wunner 
Reviewed-by: Tvrtko Ursulin 
---
 drivers/gpu/drm/i915/i915_drv.h| 11 +++
 drivers/gpu/drm/i915/i915_gem.c|  8 
 drivers/gpu/drm/i915/i915_gem_stolen.c | 12 
 drivers/gpu/drm/i915/intel_acpi.c  |  7 +++
 4 files changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2772517..74126ce 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1349,6 +1349,16 @@ struct i915_gem_mm {
 */
bool busy;
 
+   /**
+* Stolen will be lost upon hibernate (as the memory is unpowered).
+* Across resume, we expect stolen to be intact - however, it may
+* also be utililised by third parties (e.g. Intel RapidStart
+* Technology) and if so we have to assume that any data stored in
+* stolen across resume is lost and we set this flag to indicate that
+* the stolen memory is volatile.
+*/
+   bool volatile_stolen;
+
/* the indicator for dispatch video commands on two BSD rings */
unsigned int bsd_ring_dispatch_index;
 
@@ -3463,6 +3473,7 @@ intel_opregion_notify_adapter(struct drm_device *dev, 
pci_power_t state)
 #endif
 
 /* intel_acpi.c */
+bool intel_detect_acpi_rst(void);
 #ifdef CONFIG_ACPI
 extern void intel_register_dsm_handler(void);
 extern void intel_unregister_dsm_handler(void);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9aab0a0..7ec8da6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -396,8 +396,16 @@ static struct drm_i915_gem_object *
 i915_gem_alloc_object_stolen(struct drm_device *dev, size_t size)
 {
struct drm_i915_gem_object *obj;
+   struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
 
+   if (dev_priv->mm.volatile_stolen) {
+   /* Stolen may be overwritten by external parties
+* so unsuitable for persistent user data.
+*/
+   return ERR_PTR(-ENODEV);
+   }
+
mutex_lock(>struct_mutex);
obj = i915_gem_object_create_stolen(dev, size);
if (IS_ERR(obj))
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 6244dae..9238a79 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -485,6 +485,18 @@ int i915_gem_init_stolen(struct drm_device *dev)
 */
drm_mm_init(_priv->mm.stolen, 0, dev_priv->gtt.stolen_usable_size);
 
+   /* If the stolen region can be modified behind our backs upon suspend,
+* then we cannot use it to store nonvolatile contents (i.e user data)
+* as it will be corrupted upon resume.
+*/
+   dev_priv->mm.volatile_stolen = false;
+   if (IS_ENABLED(CONFIG_SUSPEND)) {
+   /* BIOSes using RapidStart Technology have been reported
+* to overwrite stolen across S3, not just S4.
+*/
+   dev_priv->mm.volatile_stolen = intel_detect_acpi_rst();
+   }
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_acpi.c 
b/drivers/gpu/drm/i915/intel_acpi.c
index eb638a1..05fd67f 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -23,6 +23,8 @@ static const u8 intel_dsm_guid[] = {
0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
 };
 
+static const char *irst_id = "INT3392";
+
 static char *intel_dsm_port_name(u8 id)
 {
switch (id) {
@@ -162,3 +164,8 @@ void intel_register_dsm_handler(void)
 void intel_unregister_dsm_handler(void)
 {
 }
+
+bool intel_detect_acpi_rst(void)
+{
+   return acpi_dev_present(irst_id);
+}
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

[Tvrtko Ursulin]

On 19/02/16 06:51, ankitprasad.r.sha...@intel.com wrote:

From: Ankitprasad Sharma 

The BIOS RapidStartTechnology may corrupt the stolen memory across S3
suspend due to unalarmed hibernation, in which case we will not be able
to preserve the User data stored in the stolen region. Hence this patch
tries to identify presence of the RST device on the ACPI bus, and
disables use of stolen memory (for persistent data) if found.

v2: Updated comment, updated/corrected new functions private to driver
(Chris/Tvrtko)

v3: Disabling stolen by default, wait till required acpi changes to
detect device presence are pulled in (Ankit)

v4: Enabled stolen by default as required acpi changes are merged
(Ankit)

v5: renamed variable, is IS_ENABLED() in place of #ifdef, use char*
instead of structures (Lukas)



Reviewed-by: Tvrtko Ursulin 

Regards,

Tvrtko


Signed-off-by: Ankitprasad Sharma 
Cc: Lukas Wunner 
---
  drivers/gpu/drm/i915/i915_drv.h| 11 +++
  drivers/gpu/drm/i915/i915_gem.c|  8 
  drivers/gpu/drm/i915/i915_gem_stolen.c | 12 
  drivers/gpu/drm/i915/intel_acpi.c  |  7 +++
  4 files changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 16f2f94..75e6935 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1349,6 +1349,16 @@ struct i915_gem_mm {
 */
bool busy;

+   /**
+* Stolen will be lost upon hibernate (as the memory is unpowered).
+* Across resume, we expect stolen to be intact - however, it may
+* also be utililised by third parties (e.g. Intel RapidStart
+* Technology) and if so we have to assume that any data stored in
+* stolen across resume is lost and we set this flag to indicate that
+* the stolen memory is volatile.
+*/
+   bool volatile_stolen;
+
/* the indicator for dispatch video commands on two BSD rings */
unsigned int bsd_ring_dispatch_index;

@@ -3465,6 +3475,7 @@ intel_opregion_notify_adapter(struct drm_device *dev, 
pci_power_t state)
  #endif

  /* intel_acpi.c */
+bool intel_detect_acpi_rst(void);
  #ifdef CONFIG_ACPI
  extern void intel_register_dsm_handler(void);
  extern void intel_unregister_dsm_handler(void);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 587beea..8e5fce4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -396,8 +396,16 @@ static struct drm_i915_gem_object *
  i915_gem_alloc_object_stolen(struct drm_device *dev, size_t size)
  {
struct drm_i915_gem_object *obj;
+   struct drm_i915_private *dev_priv = dev->dev_private;
int ret;

+   if (dev_priv->mm.volatile_stolen) {
+   /* Stolen may be overwritten by external parties
+* so unsuitable for persistent user data.
+*/
+   return ERR_PTR(-ENODEV);
+   }
+
mutex_lock(>struct_mutex);
obj = i915_gem_object_create_stolen(dev, size);
if (IS_ERR(obj))
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 335a1ef..88ee036 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -482,6 +482,18 @@ int i915_gem_init_stolen(struct drm_device *dev)
 */
drm_mm_init(_priv->mm.stolen, 0, dev_priv->gtt.stolen_usable_size);

+   /* If the stolen region can be modified behind our backs upon suspend,
+* then we cannot use it to store nonvolatile contents (i.e user data)
+* as it will be corrupted upon resume.
+*/
+   dev_priv->mm.volatile_stolen = false;
+   if (IS_ENABLED(CONFIG_SUSPEND)) {
+   /* BIOSes using RapidStart Technology have been reported
+* to overwrite stolen across S3, not just S4.
+*/
+   dev_priv->mm.volatile_stolen = intel_detect_acpi_rst();
+   }
+
return 0;
  }

diff --git a/drivers/gpu/drm/i915/intel_acpi.c 
b/drivers/gpu/drm/i915/intel_acpi.c
index eb638a1..05fd67f 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -23,6 +23,8 @@ static const u8 intel_dsm_guid[] = {
0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
  };

+static const char *irst_id = "INT3392";
+
  static char *intel_dsm_port_name(u8 id)
  {
switch (id) {
@@ -162,3 +164,8 @@ void intel_register_dsm_handler(void)
  void intel_unregister_dsm_handler(void)
  {
  }
+
+bool intel_detect_acpi_rst(void)
+{
+   return acpi_dev_present(irst_id);
+}


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

[ankitprasad . r . sharma]

From: Ankitprasad Sharma 

The BIOS RapidStartTechnology may corrupt the stolen memory across S3
suspend due to unalarmed hibernation, in which case we will not be able
to preserve the User data stored in the stolen region. Hence this patch
tries to identify presence of the RST device on the ACPI bus, and
disables use of stolen memory (for persistent data) if found.

v2: Updated comment, updated/corrected new functions private to driver
(Chris/Tvrtko)

v3: Disabling stolen by default, wait till required acpi changes to
detect device presence are pulled in (Ankit)

v4: Enabled stolen by default as required acpi changes are merged
(Ankit)

v5: renamed variable, is IS_ENABLED() in place of #ifdef, use char*
instead of structures (Lukas)

Signed-off-by: Ankitprasad Sharma 
Cc: Lukas Wunner 
---
 drivers/gpu/drm/i915/i915_drv.h| 11 +++
 drivers/gpu/drm/i915/i915_gem.c|  8 
 drivers/gpu/drm/i915/i915_gem_stolen.c | 12 
 drivers/gpu/drm/i915/intel_acpi.c  |  7 +++
 4 files changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 16f2f94..75e6935 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1349,6 +1349,16 @@ struct i915_gem_mm {
 */
bool busy;
 
+   /**
+* Stolen will be lost upon hibernate (as the memory is unpowered).
+* Across resume, we expect stolen to be intact - however, it may
+* also be utililised by third parties (e.g. Intel RapidStart
+* Technology) and if so we have to assume that any data stored in
+* stolen across resume is lost and we set this flag to indicate that
+* the stolen memory is volatile.
+*/
+   bool volatile_stolen;
+
/* the indicator for dispatch video commands on two BSD rings */
unsigned int bsd_ring_dispatch_index;
 
@@ -3465,6 +3475,7 @@ intel_opregion_notify_adapter(struct drm_device *dev, 
pci_power_t state)
 #endif
 
 /* intel_acpi.c */
+bool intel_detect_acpi_rst(void);
 #ifdef CONFIG_ACPI
 extern void intel_register_dsm_handler(void);
 extern void intel_unregister_dsm_handler(void);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 587beea..8e5fce4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -396,8 +396,16 @@ static struct drm_i915_gem_object *
 i915_gem_alloc_object_stolen(struct drm_device *dev, size_t size)
 {
struct drm_i915_gem_object *obj;
+   struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
 
+   if (dev_priv->mm.volatile_stolen) {
+   /* Stolen may be overwritten by external parties
+* so unsuitable for persistent user data.
+*/
+   return ERR_PTR(-ENODEV);
+   }
+
mutex_lock(>struct_mutex);
obj = i915_gem_object_create_stolen(dev, size);
if (IS_ERR(obj))
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 335a1ef..88ee036 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -482,6 +482,18 @@ int i915_gem_init_stolen(struct drm_device *dev)
 */
drm_mm_init(_priv->mm.stolen, 0, dev_priv->gtt.stolen_usable_size);
 
+   /* If the stolen region can be modified behind our backs upon suspend,
+* then we cannot use it to store nonvolatile contents (i.e user data)
+* as it will be corrupted upon resume.
+*/
+   dev_priv->mm.volatile_stolen = false;
+   if (IS_ENABLED(CONFIG_SUSPEND)) {
+   /* BIOSes using RapidStart Technology have been reported
+* to overwrite stolen across S3, not just S4.
+*/
+   dev_priv->mm.volatile_stolen = intel_detect_acpi_rst();
+   }
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_acpi.c 
b/drivers/gpu/drm/i915/intel_acpi.c
index eb638a1..05fd67f 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -23,6 +23,8 @@ static const u8 intel_dsm_guid[] = {
0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
 };
 
+static const char *irst_id = "INT3392";
+
 static char *intel_dsm_port_name(u8 id)
 {
switch (id) {
@@ -162,3 +164,8 @@ void intel_register_dsm_handler(void)
 void intel_unregister_dsm_handler(void)
 {
 }
+
+bool intel_detect_acpi_rst(void)
+{
+   return acpi_dev_present(irst_id);
+}
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

[Tvrtko Ursulin]

On 04/02/16 09:30, ankitprasad.r.sha...@intel.com wrote:

From: Ankitprasad Sharma 

The BIOS RapidStartTechnology may corrupt the stolen memory across S3
suspend due to unalarmed hibernation, in which case we will not be able
to preserve the User data stored in the stolen region. Hence this patch
tries to identify presence of the RST device on the ACPI bus, and
disables use of stolen memory (for persistent data) if found.

v2: Updated comment, updated/corrected new functions private to driver
(Chris/Tvrtko)

v3: Disabling stolen by default, wait till required acpi changes to
detect device presence are pulled in (Ankit)

v4: Enabled stolen by default as required acpi changes are merged
(Ankit)

Signed-off-by: Ankitprasad Sharma 
---
  drivers/gpu/drm/i915/i915_drv.h| 11 +++
  drivers/gpu/drm/i915/i915_gem.c|  8 
  drivers/gpu/drm/i915/i915_gem_stolen.c | 14 ++
  drivers/gpu/drm/i915/intel_acpi.c  | 10 ++
  4 files changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 16f2f94..9d67097 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1349,6 +1349,16 @@ struct i915_gem_mm {
 */
bool busy;

+   /**
+* Stolen will be lost upon hibernate (as the memory is unpowered).
+* Across resume, we expect stolen to be intact - however, it may
+* also be utililised by third parties (e.g. Intel RapidStart
+* Technology) and if so we have to assume that any data stored in
+* stolen across resume is lost and we set this flag to indicate that
+* the stolen memory is volatile.
+*/
+   bool nonvolatile_stolen;


I agree with a point made by Lukas that volatile_stolen would be better. 
Even the comment above suggests so. :)



+
/* the indicator for dispatch video commands on two BSD rings */
unsigned int bsd_ring_dispatch_index;

@@ -3465,6 +3475,7 @@ intel_opregion_notify_adapter(struct drm_device *dev, 
pci_power_t state)
  #endif

  /* intel_acpi.c */
+bool intel_detect_acpi_rst(void);
  #ifdef CONFIG_ACPI
  extern void intel_register_dsm_handler(void);
  extern void intel_unregister_dsm_handler(void);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0cd57d4..63dab63 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -396,8 +396,16 @@ static struct drm_i915_gem_object *
  i915_gem_alloc_object_stolen(struct drm_device *dev, size_t size)
  {
struct drm_i915_gem_object *obj;
+   struct drm_i915_private *dev_priv = dev->dev_private;
int ret;

+   if (!dev_priv->mm.nonvolatile_stolen) {
+   /* Stolen may be overwritten by external parties
+* so unsuitable for persistent user data.
+*/
+   return ERR_PTR(-ENODEV);
+   }
+
mutex_lock(>struct_mutex);
obj = i915_gem_object_create_stolen(dev, size);
if (IS_ERR(obj))
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 335a1ef..4f44531 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -482,6 +482,20 @@ int i915_gem_init_stolen(struct drm_device *dev)
 */
drm_mm_init(_priv->mm.stolen, 0, dev_priv->gtt.stolen_usable_size);

+   /* If the stolen region can be modified behind our backs upon suspend,
+* then we cannot use it to store nonvolatile contents (i.e user data)
+* as it will be corrupted upon resume.
+*/
+   dev_priv->mm.nonvolatile_stolen = true;
+#ifdef CONFIG_SUSPEND
+   if (intel_detect_acpi_rst()) {
+   /* BIOSes using RapidStart Technology have been reported
+* to overwrite stolen across S3, not just S4.
+*/
+   dev_priv->mm.nonvolatile_stolen = false;
+   }
+#endif


I also agree with the IS_ENABLED(CONFIG_SUSPEND) suggestion.


+
return 0;
  }

diff --git a/drivers/gpu/drm/i915/intel_acpi.c 
b/drivers/gpu/drm/i915/intel_acpi.c
index eb638a1..67dc9b2 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -23,6 +23,11 @@ static const u8 intel_dsm_guid[] = {
0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
  };

+static const struct acpi_device_id irst_ids[] = {
+   {"INT3392", 0},
+   {"", 0}
+};
+
  static char *intel_dsm_port_name(u8 id)
  {
switch (id) {
@@ -162,3 +167,8 @@ void intel_register_dsm_handler(void)
  void intel_unregister_dsm_handler(void)
  {
  }
+
+bool intel_detect_acpi_rst(void)
+{
+   return acpi_dev_present(irst_ids[0].id);
+}


I also agree with Lukas'es suggestion to get rid of this and just do:

dev_priv->mm.volatile_stolen = IS_ENABLED(CONFIG_SUSPEND) &&
   

Re: [Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

[Lukas Wunner]

Hi Ankitprasad,

On Thu, Feb 04, 2016 at 05:43:17PM +0100, Lukas Wunner wrote:
> On Thu, Feb 04, 2016 at 04:05:04PM +, Chris Wilson wrote:
> > We could #define INTEL_RAPID_START "INT3392" for
> > if (IS_ENABLED(CONFIG_SUSPEND) && acpi_dev_present(INTEL_RAPID_START))
> > but Anki wanted to keep the acpi details themselves out of stolen (hence
> > the current split).
> 
> Less code is almost always better, it's more work for a reader to
> follow the logic if things are split across multiple files.
> 
> If you absolutely positively want to keep the current split,
> the "static const struct acpi_device_id irst_ids[]" data structure
> should be replaced by a "static const char*" in order to not waste
> memory.

As I've learned the hard way yesterday, acpi_dev_present() is undefined
if the kernel is compiled without CONFIG_ACPI, so you made the right
call splitting that out into intel_acpi.c and my suggested simplification
was wrong. Sorry for the noise. :-/

I'd still suggest to invoke acpi_dev_present() with the string literal
"INT3392" in intel_acpi.c:intel_detect_acpi_rst() though, or at least
replace "static const struct acpi_device_id irst_ids[]" with
"static const char*".

Best regards,

Lukas
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

[ankitprasad . r . sharma]

From: Ankitprasad Sharma 

The BIOS RapidStartTechnology may corrupt the stolen memory across S3
suspend due to unalarmed hibernation, in which case we will not be able
to preserve the User data stored in the stolen region. Hence this patch
tries to identify presence of the RST device on the ACPI bus, and
disables use of stolen memory (for persistent data) if found.

v2: Updated comment, updated/corrected new functions private to driver
(Chris/Tvrtko)

v3: Disabling stolen by default, wait till required acpi changes to
detect device presence are pulled in (Ankit)

v4: Enabled stolen by default as required acpi changes are merged
(Ankit)

Signed-off-by: Ankitprasad Sharma 
---
 drivers/gpu/drm/i915/i915_drv.h| 11 +++
 drivers/gpu/drm/i915/i915_gem.c|  8 
 drivers/gpu/drm/i915/i915_gem_stolen.c | 14 ++
 drivers/gpu/drm/i915/intel_acpi.c  | 10 ++
 4 files changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 16f2f94..9d67097 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1349,6 +1349,16 @@ struct i915_gem_mm {
 */
bool busy;
 
+   /**
+* Stolen will be lost upon hibernate (as the memory is unpowered).
+* Across resume, we expect stolen to be intact - however, it may
+* also be utililised by third parties (e.g. Intel RapidStart
+* Technology) and if so we have to assume that any data stored in
+* stolen across resume is lost and we set this flag to indicate that
+* the stolen memory is volatile.
+*/
+   bool nonvolatile_stolen;
+
/* the indicator for dispatch video commands on two BSD rings */
unsigned int bsd_ring_dispatch_index;
 
@@ -3465,6 +3475,7 @@ intel_opregion_notify_adapter(struct drm_device *dev, 
pci_power_t state)
 #endif
 
 /* intel_acpi.c */
+bool intel_detect_acpi_rst(void);
 #ifdef CONFIG_ACPI
 extern void intel_register_dsm_handler(void);
 extern void intel_unregister_dsm_handler(void);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0cd57d4..63dab63 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -396,8 +396,16 @@ static struct drm_i915_gem_object *
 i915_gem_alloc_object_stolen(struct drm_device *dev, size_t size)
 {
struct drm_i915_gem_object *obj;
+   struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
 
+   if (!dev_priv->mm.nonvolatile_stolen) {
+   /* Stolen may be overwritten by external parties
+* so unsuitable for persistent user data.
+*/
+   return ERR_PTR(-ENODEV);
+   }
+
mutex_lock(>struct_mutex);
obj = i915_gem_object_create_stolen(dev, size);
if (IS_ERR(obj))
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 335a1ef..4f44531 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -482,6 +482,20 @@ int i915_gem_init_stolen(struct drm_device *dev)
 */
drm_mm_init(_priv->mm.stolen, 0, dev_priv->gtt.stolen_usable_size);
 
+   /* If the stolen region can be modified behind our backs upon suspend,
+* then we cannot use it to store nonvolatile contents (i.e user data)
+* as it will be corrupted upon resume.
+*/
+   dev_priv->mm.nonvolatile_stolen = true;
+#ifdef CONFIG_SUSPEND
+   if (intel_detect_acpi_rst()) {
+   /* BIOSes using RapidStart Technology have been reported
+* to overwrite stolen across S3, not just S4.
+*/
+   dev_priv->mm.nonvolatile_stolen = false;
+   }
+#endif
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_acpi.c 
b/drivers/gpu/drm/i915/intel_acpi.c
index eb638a1..67dc9b2 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -23,6 +23,11 @@ static const u8 intel_dsm_guid[] = {
0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
 };
 
+static const struct acpi_device_id irst_ids[] = {
+   {"INT3392", 0},
+   {"", 0}
+};
+
 static char *intel_dsm_port_name(u8 id)
 {
switch (id) {
@@ -162,3 +167,8 @@ void intel_register_dsm_handler(void)
 void intel_unregister_dsm_handler(void)
 {
 }
+
+bool intel_detect_acpi_rst(void)
+{
+   return acpi_dev_present(irst_ids[0].id);
+}
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

[Chris Wilson]

On Thu, Feb 04, 2016 at 04:46:55PM +0100, Lukas Wunner wrote:
> > +   /* If the stolen region can be modified behind our backs upon suspend,
> > +* then we cannot use it to store nonvolatile contents (i.e user data)
> > +* as it will be corrupted upon resume.
> > +*/
> > +   dev_priv->mm.nonvolatile_stolen = true;
> > +#ifdef CONFIG_SUSPEND
> > +   if (intel_detect_acpi_rst()) {
> > +   /* BIOSes using RapidStart Technology have been reported
> > +* to overwrite stolen across S3, not just S4.
> > +*/
> > +   dev_priv->mm.nonvolatile_stolen = false;
> > +   }
> > +#endif
> > +
> 
> I'd suggest simplifying it like this:
> 
>   dev_priv->mm.nonvolatile_stolen = !(IS_ENABLED(CONFIG_SUSPEND) &&
>   acpi_dev_present("INT3392"));

Using if (IS_ENABLED(CONFIG_SUSPEND) && intel_detect_acpi_rst()) would
be better indeed. My main concern here is that we document carefully why we had
to disable this, and to leave room for future caveats. Hence my
preference for the verbose layout.

We could #define INTEL_RAPID_START "INT339" for
if (IS_ENABLED(CONFIG_SUSPEND) && acpi_dev_present(INTEL_RAPID_START))
but Anki wanted to keep the acpi details themselves out of stolen (hence
the current split).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

[Lukas Wunner]

Hi,

On Thu, Feb 04, 2016 at 03:00:11PM +0530, ankitprasad.r.sha...@intel.com wrote:
> From: Ankitprasad Sharma 
> 
> The BIOS RapidStartTechnology may corrupt the stolen memory across S3
> suspend due to unalarmed hibernation, in which case we will not be able
> to preserve the User data stored in the stolen region. Hence this patch
> tries to identify presence of the RST device on the ACPI bus, and
> disables use of stolen memory (for persistent data) if found.
> 
> v2: Updated comment, updated/corrected new functions private to driver
> (Chris/Tvrtko)
> 
> v3: Disabling stolen by default, wait till required acpi changes to
> detect device presence are pulled in (Ankit)
> 
> v4: Enabled stolen by default as required acpi changes are merged
> (Ankit)
> 
> Signed-off-by: Ankitprasad Sharma 
> ---
>  drivers/gpu/drm/i915/i915_drv.h| 11 +++
>  drivers/gpu/drm/i915/i915_gem.c|  8 
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 14 ++
>  drivers/gpu/drm/i915/intel_acpi.c  | 10 ++
>  4 files changed, 43 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 16f2f94..9d67097 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1349,6 +1349,16 @@ struct i915_gem_mm {
>*/
>   bool busy;
>  
> + /**
> +  * Stolen will be lost upon hibernate (as the memory is unpowered).
> +  * Across resume, we expect stolen to be intact - however, it may
> +  * also be utililised by third parties (e.g. Intel RapidStart
> +  * Technology) and if so we have to assume that any data stored in
> +  * stolen across resume is lost and we set this flag to indicate that
> +  * the stolen memory is volatile.
> +  */
> + bool nonvolatile_stolen;
> +
>   /* the indicator for dispatch video commands on two BSD rings */
>   unsigned int bsd_ring_dispatch_index;
>  
> @@ -3465,6 +3475,7 @@ intel_opregion_notify_adapter(struct drm_device *dev, 
> pci_power_t state)
>  #endif
>  
>  /* intel_acpi.c */
> +bool intel_detect_acpi_rst(void);
>  #ifdef CONFIG_ACPI
>  extern void intel_register_dsm_handler(void);
>  extern void intel_unregister_dsm_handler(void);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 0cd57d4..63dab63 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -396,8 +396,16 @@ static struct drm_i915_gem_object *
>  i915_gem_alloc_object_stolen(struct drm_device *dev, size_t size)
>  {
>   struct drm_i915_gem_object *obj;
> + struct drm_i915_private *dev_priv = dev->dev_private;
>   int ret;
>  
> + if (!dev_priv->mm.nonvolatile_stolen) {
> + /* Stolen may be overwritten by external parties
> +  * so unsuitable for persistent user data.
> +  */
> + return ERR_PTR(-ENODEV);
> + }
> +
>   mutex_lock(>struct_mutex);
>   obj = i915_gem_object_create_stolen(dev, size);
>   if (IS_ERR(obj))
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 335a1ef..4f44531 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -482,6 +482,20 @@ int i915_gem_init_stolen(struct drm_device *dev)
>*/
>   drm_mm_init(_priv->mm.stolen, 0, dev_priv->gtt.stolen_usable_size);
>  
> + /* If the stolen region can be modified behind our backs upon suspend,
> +  * then we cannot use it to store nonvolatile contents (i.e user data)
> +  * as it will be corrupted upon resume.
> +  */
> + dev_priv->mm.nonvolatile_stolen = true;
> +#ifdef CONFIG_SUSPEND
> + if (intel_detect_acpi_rst()) {
> + /* BIOSes using RapidStart Technology have been reported
> +  * to overwrite stolen across S3, not just S4.
> +  */
> + dev_priv->mm.nonvolatile_stolen = false;
> + }
> +#endif
> +

I'd suggest simplifying it like this:

dev_priv->mm.nonvolatile_stolen = !(IS_ENABLED(CONFIG_SUSPEND) &&
acpi_dev_present("INT3392"));

And add to i915_gem_stolen.c:

#include 

Best regards,

Lukas

>   return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_acpi.c 
> b/drivers/gpu/drm/i915/intel_acpi.c
> index eb638a1..67dc9b2 100644
> --- a/drivers/gpu/drm/i915/intel_acpi.c
> +++ b/drivers/gpu/drm/i915/intel_acpi.c
> @@ -23,6 +23,11 @@ static const u8 intel_dsm_guid[] = {
>   0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
>  };
>  
> +static const struct acpi_device_id irst_ids[] = {
> + {"INT3392", 0},
> + {"", 0}
> +};
> +
>  static char *intel_dsm_port_name(u8 id)
>  {
>   switch (id) {
> @@ -162,3 +167,8 @@ void intel_register_dsm_handler(void)
>  void intel_unregister_dsm_handler(void)
>  {
>  }
> +
> +bool 

Re: [Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

[Lukas Wunner]

Hi,

On Thu, Feb 04, 2016 at 04:05:04PM +, Chris Wilson wrote:
> On Thu, Feb 04, 2016 at 04:46:55PM +0100, Lukas Wunner wrote:
> > > + /* If the stolen region can be modified behind our backs upon suspend,
> > > +  * then we cannot use it to store nonvolatile contents (i.e user data)
> > > +  * as it will be corrupted upon resume.
> > > +  */
> > > + dev_priv->mm.nonvolatile_stolen = true;
> > > +#ifdef CONFIG_SUSPEND
> > > + if (intel_detect_acpi_rst()) {
> > > + /* BIOSes using RapidStart Technology have been reported
> > > +  * to overwrite stolen across S3, not just S4.
> > > +  */
> > > + dev_priv->mm.nonvolatile_stolen = false;
> > > + }
> > > +#endif
> > > +
> > 
> > I'd suggest simplifying it like this:
> > 
> > dev_priv->mm.nonvolatile_stolen = !(IS_ENABLED(CONFIG_SUSPEND) &&
> > acpi_dev_present("INT3392"));
> 
> Using if (IS_ENABLED(CONFIG_SUSPEND) && intel_detect_acpi_rst()) would
> be better indeed. My main concern here is that we document carefully why
> we had to disable this, and to leave room for future caveats.

Yes absolutely, keep the comments. (Or meld them into one if simplifying
the code as suggested.)

> We could #define INTEL_RAPID_START "INT3392" for
> if (IS_ENABLED(CONFIG_SUSPEND) && acpi_dev_present(INTEL_RAPID_START))
> but Anki wanted to keep the acpi details themselves out of stolen (hence
> the current split).

At the very least the #ifdef needs to be replaced by IS_ENABLED,
Documentation/CodingStyle chapter 20 very clearly states this is
to be preferred.

Less code is almost always better, it's more work for a reader to
follow the logic if things are split across multiple files.

If you absolutely positively want to keep the current split,
the "static const struct acpi_device_id irst_ids[]" data structure
should be replaced by a "static const char*" in order to not waste
memory.

I'd also suggest renaming "dev_priv->mm.nonvolatile_stolen" to
"volatile_stolen" since the only user of the flag uses its negation
and its calculation requires negation as well.

Best regards,

Lukas
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

[Chris Wilson]

On Tue, Jan 26, 2016 at 01:13:36AM +0530, ankitprasad.r.sha...@intel.com wrote:
> From: Ankitprasad Sharma 
> 
> The BIOS RapidStartTechnology may corrupt the stolen memory across S3
> suspend due to unalarmed hibernation, in which case we will not be able
> to preserve the User data stored in the stolen region. Hence this patch
> tries to identify presence of the RST device on the ACPI bus, and
> disables use of stolen memory (for persistent data) if found.
> 
> v2: Updated comment, updated/corrected new functions private to driver
> (Chris/Tvrtko)
> 
> v3: Disabling stolen by default, wait till required acpi changes to
> detect device presence are pulled in (Ankit)
> 
> Signed-off-by: Ankitprasad Sharma 

Ideally this would be earlier in the sequence so that we don't introduce
the new API will a known problem.


> ---
>  drivers/gpu/drm/i915/i915_drv.h| 11 +++
>  drivers/gpu/drm/i915/i915_gem.c|  8 
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 14 ++
>  drivers/gpu/drm/i915/intel_acpi.c  | 10 ++
>  4 files changed, 43 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d77d2ed..8037609 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1311,6 +1311,16 @@ struct i915_gem_mm {
>*/
>   bool busy;
>  
> + /**
> +  * Stolen will be lost upon hibernate (as the memory is unpowered).
> +  * Across resume, we expect stolen to be intact - however, it may
> +  * also be utililised by third parties (e.g. Intel RapidStart
> +  * Technology) and if so we have to assume that any data stored in
> +  * stolen across resume is lost and we set this flag to indicate that
> +  * the stolen memory is volatile.
> +  */
> + bool nonvolatile_stolen;
> +
>   /* the indicator for dispatch video commands on two BSD rings */
>   unsigned int bsd_ring_dispatch_index;
>  
> @@ -3418,6 +3428,7 @@ intel_opregion_notify_adapter(struct drm_device *dev, 
> pci_power_t state)
>  #endif
>  
>  /* intel_acpi.c */
> +bool intel_detect_acpi_rst(void);
>  #ifdef CONFIG_ACPI
>  extern void intel_register_dsm_handler(void);
>  extern void intel_unregister_dsm_handler(void);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9dcefb1..f7c9420 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -396,8 +396,16 @@ static struct drm_i915_gem_object *
>  i915_gem_alloc_object_stolen(struct drm_device *dev, size_t size)
>  {
>   struct drm_i915_gem_object *obj;
> + struct drm_i915_private *dev_priv = dev->dev_private;
>   int ret;
>  
> + if (!dev_priv->mm.nonvolatile_stolen) {
> + /* Stolen may be overwritten by external parties
> +  * so unsuitable for persistent user data.
> +  */
> + return ERR_PTR(-ENODEV);
> + }
> +
>   mutex_lock(>struct_mutex);
>   obj = i915_gem_object_create_stolen(dev, size);
>   if (IS_ERR(obj))
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 335a1ef..4f44531 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -482,6 +482,20 @@ int i915_gem_init_stolen(struct drm_device *dev)
>*/
>   drm_mm_init(_priv->mm.stolen, 0, dev_priv->gtt.stolen_usable_size);
>  
> + /* If the stolen region can be modified behind our backs upon suspend,
> +  * then we cannot use it to store nonvolatile contents (i.e user data)
> +  * as it will be corrupted upon resume.
> +  */
> + dev_priv->mm.nonvolatile_stolen = true;
> +#ifdef CONFIG_SUSPEND
> + if (intel_detect_acpi_rst()) {
> + /* BIOSes using RapidStart Technology have been reported
> +  * to overwrite stolen across S3, not just S4.
> +  */
> + dev_priv->mm.nonvolatile_stolen = false;
> + }
> +#endif
> +
>   return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_acpi.c 
> b/drivers/gpu/drm/i915/intel_acpi.c
> index eb638a1..8add47d 100644
> --- a/drivers/gpu/drm/i915/intel_acpi.c
> +++ b/drivers/gpu/drm/i915/intel_acpi.c
> @@ -23,6 +23,11 @@ static const u8 intel_dsm_guid[] = {
>   0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
>  };
>  
> +static const struct acpi_device_id irst_ids[] = {
> + {"INT3392", 0},
> + {"", 0}
> +};

Dead code (atm).

> +
>  static char *intel_dsm_port_name(u8 id)
>  {
>   switch (id) {
> @@ -162,3 +167,8 @@ void intel_register_dsm_handler(void)
>  void intel_unregister_dsm_handler(void)
>  {
>  }
> +
> +bool intel_detect_acpi_rst(void)
> +{

/* Explain why this code is dead or else we will remove it! */
> + return true;
> +}

-- 
Chris Wilson, Intel Open Source Technology Centre
___

[Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

[ankitprasad . r . sharma]

From: Ankitprasad Sharma 

The BIOS RapidStartTechnology may corrupt the stolen memory across S3
suspend due to unalarmed hibernation, in which case we will not be able
to preserve the User data stored in the stolen region. Hence this patch
tries to identify presence of the RST device on the ACPI bus, and
disables use of stolen memory (for persistent data) if found.

v2: Updated comment, updated/corrected new functions private to driver
(Chris/Tvrtko)

v3: Disabling stolen by default, wait till required acpi changes to
detect device presence are pulled in (Ankit)

Signed-off-by: Ankitprasad Sharma 
---
 drivers/gpu/drm/i915/i915_drv.h| 11 +++
 drivers/gpu/drm/i915/i915_gem.c|  8 
 drivers/gpu/drm/i915/i915_gem_stolen.c | 14 ++
 drivers/gpu/drm/i915/intel_acpi.c  | 10 ++
 4 files changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d77d2ed..8037609 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1311,6 +1311,16 @@ struct i915_gem_mm {
 */
bool busy;
 
+   /**
+* Stolen will be lost upon hibernate (as the memory is unpowered).
+* Across resume, we expect stolen to be intact - however, it may
+* also be utililised by third parties (e.g. Intel RapidStart
+* Technology) and if so we have to assume that any data stored in
+* stolen across resume is lost and we set this flag to indicate that
+* the stolen memory is volatile.
+*/
+   bool nonvolatile_stolen;
+
/* the indicator for dispatch video commands on two BSD rings */
unsigned int bsd_ring_dispatch_index;
 
@@ -3418,6 +3428,7 @@ intel_opregion_notify_adapter(struct drm_device *dev, 
pci_power_t state)
 #endif
 
 /* intel_acpi.c */
+bool intel_detect_acpi_rst(void);
 #ifdef CONFIG_ACPI
 extern void intel_register_dsm_handler(void);
 extern void intel_unregister_dsm_handler(void);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9dcefb1..f7c9420 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -396,8 +396,16 @@ static struct drm_i915_gem_object *
 i915_gem_alloc_object_stolen(struct drm_device *dev, size_t size)
 {
struct drm_i915_gem_object *obj;
+   struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
 
+   if (!dev_priv->mm.nonvolatile_stolen) {
+   /* Stolen may be overwritten by external parties
+* so unsuitable for persistent user data.
+*/
+   return ERR_PTR(-ENODEV);
+   }
+
mutex_lock(>struct_mutex);
obj = i915_gem_object_create_stolen(dev, size);
if (IS_ERR(obj))
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 335a1ef..4f44531 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -482,6 +482,20 @@ int i915_gem_init_stolen(struct drm_device *dev)
 */
drm_mm_init(_priv->mm.stolen, 0, dev_priv->gtt.stolen_usable_size);
 
+   /* If the stolen region can be modified behind our backs upon suspend,
+* then we cannot use it to store nonvolatile contents (i.e user data)
+* as it will be corrupted upon resume.
+*/
+   dev_priv->mm.nonvolatile_stolen = true;
+#ifdef CONFIG_SUSPEND
+   if (intel_detect_acpi_rst()) {
+   /* BIOSes using RapidStart Technology have been reported
+* to overwrite stolen across S3, not just S4.
+*/
+   dev_priv->mm.nonvolatile_stolen = false;
+   }
+#endif
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_acpi.c 
b/drivers/gpu/drm/i915/intel_acpi.c
index eb638a1..8add47d 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -23,6 +23,11 @@ static const u8 intel_dsm_guid[] = {
0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
 };
 
+static const struct acpi_device_id irst_ids[] = {
+   {"INT3392", 0},
+   {"", 0}
+};
+
 static char *intel_dsm_port_name(u8 id)
 {
switch (id) {
@@ -162,3 +167,8 @@ void intel_register_dsm_handler(void)
 void intel_unregister_dsm_handler(void)
 {
 }
+
+bool intel_detect_acpi_rst(void)
+{
+   return true;
+}
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

[Daniel Vetter]

On Tue, Dec 22, 2015 at 11:50:53AM +0530, ankitprasad.r.sha...@intel.com wrote:
> From: Ankitprasad Sharma 
> 
> The BIOS RapidStartTechnology may corrupt the stolen memory across S3
> suspend due to unalarmed hibernation, in which case we will not be able
> to preserve the User data stored in the stolen region. Hence this patch
> tries to identify presence of the RST device on the ACPI bus, and
> disables use of stolen memory (for persistent data) if found.
> 
> Signed-off-by: Ankitprasad Sharma 

Hm, could be that acpi mainatainers want to encapsulate this logic within
the acpi subsystem, and only expose a simple helper to drivers like i915.
Please add the same Cc: list to this patch as to the acpi one when
resending, so that acpi maintainers have the full context.

Thanks, Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h|  7 +++
>  drivers/gpu/drm/i915/i915_gem.c|  8 
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 14 ++
>  drivers/gpu/drm/i915/intel_acpi.c  | 20 
>  4 files changed, 49 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 492878a..d26a8f1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1309,6 +1309,9 @@ struct i915_gem_mm {
>*/
>   bool busy;
>  
> + /* Intel RapidStart Technology info */
> + bool nonvolatile_stolen;
> +
>   /* the indicator for dispatch video commands on two BSD rings */
>   int bsd_ring_dispatch_index;
>  
> @@ -3417,9 +3420,13 @@ intel_opregion_notify_adapter(struct drm_device *dev, 
> pci_power_t state)
>  #ifdef CONFIG_ACPI
>  extern void intel_register_dsm_handler(void);
>  extern void intel_unregister_dsm_handler(void);
> +extern bool intel_detect_acpi_rst(void);
> +extern int match_device(struct device *dev, void* ids);
>  #else
>  static inline void intel_register_dsm_handler(void) { return; }
>  static inline void intel_unregister_dsm_handler(void) { return; }
> +static inline bool intel_detect_acpi_rst(void) { return false; }
> +static int match_device(struct device *dev, void* ids) { return 0; }
>  #endif /* CONFIG_ACPI */
>  
>  /* modesetting */
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index d27a41e..daca05f 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -399,6 +399,7 @@ i915_gem_create(struct drm_file *file,
>   uint32_t *handle_p)
>  {
>   struct drm_i915_gem_object *obj;
> + struct drm_i915_private *dev_priv = dev->dev_private;
>   int ret;
>   u32 handle;
>  
> @@ -411,6 +412,13 @@ i915_gem_create(struct drm_file *file,
>  
>   /* Allocate the new object */
>   if (flags & I915_CREATE_PLACEMENT_STOLEN) {
> + if (!dev_priv->mm.nonvolatile_stolen) {
> + /* Stolen may be overwritten by external parties
> +  * so unsuitable for persistent user data.
> +  */
> + return -ENODEV;
> + }
> +
>   mutex_lock(>struct_mutex);
>   obj = i915_gem_object_create_stolen(dev, size);
>   if (IS_ERR(obj)) {
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 6d1af9d..98b4998 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -482,6 +482,20 @@ int i915_gem_init_stolen(struct drm_device *dev)
>*/
>   drm_mm_init(_priv->mm.stolen, 0, dev_priv->gtt.stolen_usable_size);
>  
> + /* If the stolen region can be modified behind our backs upon suspend,
> +  * then we cannot use it to store nonvolatile contents (i.e user data)
> +  * as it will be corrupted upon resume.
> +  */
> + dev_priv->mm.nonvolatile_stolen = true;
> +#ifdef CONFIG_SUSPEND
> + if (intel_detect_acpi_rst()) {
> + /* BIOSes using RapidStart Technology have been reported
> +  * to overwrite stolen across S3, not just S4.
> +  */
> + dev_priv->mm.nonvolatile_stolen = false;
> + }
> +#endif
> +
>   return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_acpi.c 
> b/drivers/gpu/drm/i915/intel_acpi.c
> index eb638a1..5f7713d 100644
> --- a/drivers/gpu/drm/i915/intel_acpi.c
> +++ b/drivers/gpu/drm/i915/intel_acpi.c
> @@ -23,6 +23,11 @@ static const u8 intel_dsm_guid[] = {
>   0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
>  };
>  
> +static const struct acpi_device_id irst_ids[] = {
> + {"INT3392", 0},
> + {"", 0}
> +};
> +
>  static char *intel_dsm_port_name(u8 id)
>  {
>   switch (id) {
> @@ -162,3 +167,18 @@ void intel_register_dsm_handler(void)
>  void intel_unregister_dsm_handler(void)
>  {
>  }
> +
> +int match_device(struct device *dev, void* ids)
> +{
> + if (acpi_match_device(irst_ids, dev))
> +  

Re: [Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

[Tvrtko Ursulin]

Hi,

On 22/12/15 06:20, ankitprasad.r.sha...@intel.com wrote:

From: Ankitprasad Sharma 

The BIOS RapidStartTechnology may corrupt the stolen memory across S3
suspend due to unalarmed hibernation, in which case we will not be able
to preserve the User data stored in the stolen region. Hence this patch
tries to identify presence of the RST device on the ACPI bus, and
disables use of stolen memory (for persistent data) if found.

Signed-off-by: Ankitprasad Sharma 
---
  drivers/gpu/drm/i915/i915_drv.h|  7 +++
  drivers/gpu/drm/i915/i915_gem.c|  8 
  drivers/gpu/drm/i915/i915_gem_stolen.c | 14 ++
  drivers/gpu/drm/i915/intel_acpi.c  | 20 
  4 files changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 492878a..d26a8f1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1309,6 +1309,9 @@ struct i915_gem_mm {
 */
bool busy;

+   /* Intel RapidStart Technology info */
+   bool nonvolatile_stolen;
+
/* the indicator for dispatch video commands on two BSD rings */
int bsd_ring_dispatch_index;

@@ -3417,9 +3420,13 @@ intel_opregion_notify_adapter(struct drm_device *dev, 
pci_power_t state)
  #ifdef CONFIG_ACPI
  extern void intel_register_dsm_handler(void);
  extern void intel_unregister_dsm_handler(void);
+extern bool intel_detect_acpi_rst(void);
+extern int match_device(struct device *dev, void* ids);


intel_ prefix?

It also looks acpi_match_device already handles !CONFIG_ACPI in 
include/linux/acpi.h so maybe you don't need this declarations at all.


Just define it locally and unconditionally where intel_detect_acpi_rst is.

Thing I am not sure about is can the ACPI be compiled in but disabled. 
acpi=off on the kernel command line suggests it can.


What do we want to do in that case?


  #else
  static inline void intel_register_dsm_handler(void) { return; }
  static inline void intel_unregister_dsm_handler(void) { return; }
+static inline bool intel_detect_acpi_rst(void) { return false; }
+static int match_device(struct device *dev, void* ids) { return 0; }
  #endif /* CONFIG_ACPI */

  /* modesetting */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d27a41e..daca05f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -399,6 +399,7 @@ i915_gem_create(struct drm_file *file,
uint32_t *handle_p)
  {
struct drm_i915_gem_object *obj;
+   struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
u32 handle;

@@ -411,6 +412,13 @@ i915_gem_create(struct drm_file *file,

/* Allocate the new object */
if (flags & I915_CREATE_PLACEMENT_STOLEN) {
+   if (!dev_priv->mm.nonvolatile_stolen) {
+   /* Stolen may be overwritten by external parties
+* so unsuitable for persistent user data.
+*/
+   return -ENODEV;
+   }
+
mutex_lock(>struct_mutex);
obj = i915_gem_object_create_stolen(dev, size);
if (IS_ERR(obj)) {
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 6d1af9d..98b4998 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -482,6 +482,20 @@ int i915_gem_init_stolen(struct drm_device *dev)
 */
drm_mm_init(_priv->mm.stolen, 0, dev_priv->gtt.stolen_usable_size);

+   /* If the stolen region can be modified behind our backs upon suspend,
+* then we cannot use it to store nonvolatile contents (i.e user data)
+* as it will be corrupted upon resume.
+*/
+   dev_priv->mm.nonvolatile_stolen = true;
+#ifdef CONFIG_SUSPEND
+   if (intel_detect_acpi_rst()) {
+   /* BIOSes using RapidStart Technology have been reported
+* to overwrite stolen across S3, not just S4.
+*/
+   dev_priv->mm.nonvolatile_stolen = false;
+   }
+#endif
+
return 0;
  }

diff --git a/drivers/gpu/drm/i915/intel_acpi.c 
b/drivers/gpu/drm/i915/intel_acpi.c
index eb638a1..5f7713d 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -23,6 +23,11 @@ static const u8 intel_dsm_guid[] = {
0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
  };

+static const struct acpi_device_id irst_ids[] = {
+   {"INT3392", 0},
+   {"", 0}
+};
+
  static char *intel_dsm_port_name(u8 id)
  {
switch (id) {
@@ -162,3 +167,18 @@ void intel_register_dsm_handler(void)
  void intel_unregister_dsm_handler(void)
  {
  }
+
+int match_device(struct device *dev, void* ids)
+{
+   if (acpi_match_device(irst_ids, dev))
+   return 1;
+
+   return 0;
+}
+bool 

Re: [Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

[Chris Wilson]

On Tue, Dec 22, 2015 at 11:50:53AM +0530, ankitprasad.r.sha...@intel.com wrote:
> From: Ankitprasad Sharma 
> 
> The BIOS RapidStartTechnology may corrupt the stolen memory across S3
> suspend due to unalarmed hibernation, in which case we will not be able
> to preserve the User data stored in the stolen region. Hence this patch
> tries to identify presence of the RST device on the ACPI bus, and
> disables use of stolen memory (for persistent data) if found.
> 
> Signed-off-by: Ankitprasad Sharma 
> ---
>  drivers/gpu/drm/i915/i915_drv.h|  7 +++
>  drivers/gpu/drm/i915/i915_gem.c|  8 
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 14 ++
>  drivers/gpu/drm/i915/intel_acpi.c  | 20 
>  4 files changed, 49 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 492878a..d26a8f1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1309,6 +1309,9 @@ struct i915_gem_mm {
>*/
>   bool busy;
>  
> + /* Intel RapidStart Technology info */

This defeats the purpose of using a generic term.

/** Stolen will be lost upon hibernate (as the memory is unpowered).
 * Across resume, we expect stolen to be intact - however, it may
 * also be utililised by third parties (e.g. Intel RapidStart
 * Technology) and if so we hae to assume that any data stored in
 * stolen across resume is lost and we set this flag to indicate that
 * the stolen memory is volatile.
 */
> + bool nonvolatile_stolen;
> +
>   /* the indicator for dispatch video commands on two BSD rings */
>   int bsd_ring_dispatch_index;
>  
> @@ -3417,9 +3420,13 @@ intel_opregion_notify_adapter(struct drm_device *dev, 
> pci_power_t state)
>  #ifdef CONFIG_ACPI
>  extern void intel_register_dsm_handler(void);
>  extern void intel_unregister_dsm_handler(void);
> +extern bool intel_detect_acpi_rst(void);
> +extern int match_device(struct device *dev, void* ids);

As Tvrtko mentioned this is private to the driver.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

[ankitprasad . r . sharma]

From: Ankitprasad Sharma 

The BIOS RapidStartTechnology may corrupt the stolen memory across S3
suspend due to unalarmed hibernation, in which case we will not be able
to preserve the User data stored in the stolen region. Hence this patch
tries to identify presence of the RST device on the ACPI bus, and
disables use of stolen memory (for persistent data) if found.

Signed-off-by: Ankitprasad Sharma 
---
 drivers/gpu/drm/i915/i915_drv.h|  7 +++
 drivers/gpu/drm/i915/i915_gem.c|  8 
 drivers/gpu/drm/i915/i915_gem_stolen.c | 14 ++
 drivers/gpu/drm/i915/intel_acpi.c  | 20 
 4 files changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 492878a..d26a8f1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1309,6 +1309,9 @@ struct i915_gem_mm {
 */
bool busy;
 
+   /* Intel RapidStart Technology info */
+   bool nonvolatile_stolen;
+
/* the indicator for dispatch video commands on two BSD rings */
int bsd_ring_dispatch_index;
 
@@ -3417,9 +3420,13 @@ intel_opregion_notify_adapter(struct drm_device *dev, 
pci_power_t state)
 #ifdef CONFIG_ACPI
 extern void intel_register_dsm_handler(void);
 extern void intel_unregister_dsm_handler(void);
+extern bool intel_detect_acpi_rst(void);
+extern int match_device(struct device *dev, void* ids);
 #else
 static inline void intel_register_dsm_handler(void) { return; }
 static inline void intel_unregister_dsm_handler(void) { return; }
+static inline bool intel_detect_acpi_rst(void) { return false; }
+static int match_device(struct device *dev, void* ids) { return 0; }
 #endif /* CONFIG_ACPI */
 
 /* modesetting */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d27a41e..daca05f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -399,6 +399,7 @@ i915_gem_create(struct drm_file *file,
uint32_t *handle_p)
 {
struct drm_i915_gem_object *obj;
+   struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
u32 handle;
 
@@ -411,6 +412,13 @@ i915_gem_create(struct drm_file *file,
 
/* Allocate the new object */
if (flags & I915_CREATE_PLACEMENT_STOLEN) {
+   if (!dev_priv->mm.nonvolatile_stolen) {
+   /* Stolen may be overwritten by external parties
+* so unsuitable for persistent user data.
+*/
+   return -ENODEV;
+   }
+
mutex_lock(>struct_mutex);
obj = i915_gem_object_create_stolen(dev, size);
if (IS_ERR(obj)) {
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 6d1af9d..98b4998 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -482,6 +482,20 @@ int i915_gem_init_stolen(struct drm_device *dev)
 */
drm_mm_init(_priv->mm.stolen, 0, dev_priv->gtt.stolen_usable_size);
 
+   /* If the stolen region can be modified behind our backs upon suspend,
+* then we cannot use it to store nonvolatile contents (i.e user data)
+* as it will be corrupted upon resume.
+*/
+   dev_priv->mm.nonvolatile_stolen = true;
+#ifdef CONFIG_SUSPEND
+   if (intel_detect_acpi_rst()) {
+   /* BIOSes using RapidStart Technology have been reported
+* to overwrite stolen across S3, not just S4.
+*/
+   dev_priv->mm.nonvolatile_stolen = false;
+   }
+#endif
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_acpi.c 
b/drivers/gpu/drm/i915/intel_acpi.c
index eb638a1..5f7713d 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -23,6 +23,11 @@ static const u8 intel_dsm_guid[] = {
0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
 };
 
+static const struct acpi_device_id irst_ids[] = {
+   {"INT3392", 0},
+   {"", 0}
+};
+
 static char *intel_dsm_port_name(u8 id)
 {
switch (id) {
@@ -162,3 +167,18 @@ void intel_register_dsm_handler(void)
 void intel_unregister_dsm_handler(void)
 {
 }
+
+int match_device(struct device *dev, void* ids)
+{
+   if (acpi_match_device(irst_ids, dev))
+   return 1;
+
+   return 0;
+}
+bool intel_detect_acpi_rst(void)
+{
+   if (bus_for_each_dev(_bus_type, NULL, NULL, match_device))
+   return true;;
+
+   return false;
+}
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx