[PATCH] drm/i915: Setting/clearing the memory access bit when enabling/disabling i915

[PATCH] drm/i915: Setting/clearing the memory access bit when enabling/disabling i915

[Jia Yao]

Make i915's PCI device management more robust by always setting/clearing the
memory access bit when enabling/disabling the device, and by consolidating this
 logic into helper functions.

It fixes kexec reboot issue by disabling memory access before shutting down
the device, which can block unsafe and unwanted access from DMA.

Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
Cc: Alex Zuo <alex.zuo@intel.com>
Cc: Shuicheng Lin <shuicheng.lin@intel.com>
Cc: Askar Safin <safinaskar@gmail.com>
Cc: Pingfan Liu <piliu@redhat.com>
Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
Signed-off-by: Jia Yao <jia.yao@intel.com>
---
 drivers/gpu/drm/i915/i915_driver.c | 33 +++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index b46cb54ef5dc..0120742e68d7 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -118,6 +118,31 @@
 
 static const struct drm_driver i915_drm_driver;
 
+static int i915_enable_device(struct pci_dev *pdev) {
+	u32 cmd;
+	int ret;
+
+	ret = pci_enable_device(pdev);
+	if (ret)
+		return ret;
+
+	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
+	if (!(cmd & PCI_COMMAND_MEMORY))
+		pci_write_config_dword(pdev, PCI_COMMAND, cmd | PCI_COMMAND_MEMORY);
+
+	return 0;
+}
+
+static void i915_disable_device(struct pci_dev *pdev) {
+	u32 cmd;
+
+	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
+	if (cmd & PCI_COMMAND_MEMORY)
+		pci_write_config_dword(pdev, PCI_COMMAND, cmd & ~PCI_COMMAND_MEMORY);
+
+	pci_disable_device(pdev);
+}
+
 static int i915_workqueues_init(struct drm_i915_private *dev_priv)
 {
 	/*
@@ -788,7 +813,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	struct intel_display *display;
 	int ret;
 
-	ret = pci_enable_device(pdev);
+	ret = i915_enable_device(pdev);
 	if (ret) {
 		pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
 		return ret;
@@ -796,7 +821,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	i915 = i915_driver_create(pdev, ent);
 	if (IS_ERR(i915)) {
-		pci_disable_device(pdev);
+		i915_disable_device(pdev);
 		return PTR_ERR(i915);
 	}
 
@@ -885,7 +910,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	enable_rpm_wakeref_asserts(&i915->runtime_pm);
 	i915_driver_late_release(i915);
 out_pci_disable:
-	pci_disable_device(pdev);
+	i915_disable_device(pdev);
 	i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
 	return ret;
 }
@@ -1003,6 +1028,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
 
 	intel_dmc_suspend(display);
 
+	intel_pxp_fini(i915);
 	i915_gem_suspend(i915);
 
 	/*
@@ -1020,6 +1046,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
 	enable_rpm_wakeref_asserts(&i915->runtime_pm);
 
 	intel_runtime_pm_driver_last_release(&i915->runtime_pm);
+	i915_disable_device(to_pci_dev(i915->drm.dev));
 }
 
 static bool suspend_to_idle(struct drm_i915_private *dev_priv)
-- 
2.34.1

[PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Jia Yao]

Make i915's PCI device management more robust by always setting/clearing
the memory access bit when enabling/disabling the device, and by
consolidating this logic into helper functions.

It fixes kexec reboot issue by disabling memory access before shutting
down the device, which can block unsafe and unwanted access from DMA.

v2:
  - follow brace style

Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
Cc: Alex Zuo <alex.zuo@intel.com>
Cc: Shuicheng Lin <shuicheng.lin@intel.com>
Cc: Askar Safin <safinaskar@gmail.com>
Cc: Pingfan Liu <piliu@redhat.com>
Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
Signed-off-by: Jia Yao <jia.yao@intel.com>
---
 drivers/gpu/drm/i915/i915_driver.c | 35 +++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index b46cb54ef5dc..766f85726b67 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -118,6 +118,33 @@
 
 static const struct drm_driver i915_drm_driver;
 
+static int i915_enable_device(struct pci_dev *pdev)
+{
+	u32 cmd;
+	int ret;
+
+	ret = pci_enable_device(pdev);
+	if (ret)
+		return ret;
+
+	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
+	if (!(cmd & PCI_COMMAND_MEMORY))
+		pci_write_config_dword(pdev, PCI_COMMAND, cmd | PCI_COMMAND_MEMORY);
+
+	return 0;
+}
+
+static void i915_disable_device(struct pci_dev *pdev)
+{
+	u32 cmd;
+
+	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
+	if (cmd & PCI_COMMAND_MEMORY)
+		pci_write_config_dword(pdev, PCI_COMMAND, cmd & ~PCI_COMMAND_MEMORY);
+
+	pci_disable_device(pdev);
+}
+
 static int i915_workqueues_init(struct drm_i915_private *dev_priv)
 {
 	/*
@@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	struct intel_display *display;
 	int ret;
 
-	ret = pci_enable_device(pdev);
+	ret = i915_enable_device(pdev);
 	if (ret) {
 		pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
 		return ret;
@@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	i915 = i915_driver_create(pdev, ent);
 	if (IS_ERR(i915)) {
-		pci_disable_device(pdev);
+		i915_disable_device(pdev);
 		return PTR_ERR(i915);
 	}
 
@@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	enable_rpm_wakeref_asserts(&i915->runtime_pm);
 	i915_driver_late_release(i915);
 out_pci_disable:
-	pci_disable_device(pdev);
+	i915_disable_device(pdev);
 	i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
 	return ret;
 }
@@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
 
 	intel_dmc_suspend(display);
 
+	intel_pxp_fini(i915);
 	i915_gem_suspend(i915);
 
 	/*
@@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
 	enable_rpm_wakeref_asserts(&i915->runtime_pm);
 
 	intel_runtime_pm_driver_last_release(&i915->runtime_pm);
+	i915_disable_device(to_pci_dev(i915->drm.dev));
 }
 
 static bool suspend_to_idle(struct drm_i915_private *dev_priv)
-- 
2.34.1

Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Ville Syrjälä]

On Tue, Oct 07, 2025 at 08:25:14PM +0000, Jia Yao wrote:
> Make i915's PCI device management more robust by always setting/clearing
> the memory access bit when enabling/disabling the device, and by
> consolidating this logic into helper functions.
> 
> It fixes kexec reboot issue by disabling memory access before shutting
> down the device, which can block unsafe and unwanted access from DMA.
> 
> v2:
>   - follow brace style
> 
> Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> Cc: Alex Zuo <alex.zuo@intel.com>
> Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> Cc: Askar Safin <safinaskar@gmail.com>
> Cc: Pingfan Liu <piliu@redhat.com>
> Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> Signed-off-by: Jia Yao <jia.yao@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_driver.c | 35 +++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index b46cb54ef5dc..766f85726b67 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -118,6 +118,33 @@
>  
>  static const struct drm_driver i915_drm_driver;
>  
> +static int i915_enable_device(struct pci_dev *pdev)
> +{
> +	u32 cmd;
> +	int ret;
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret)
> +		return ret;
> +
> +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +	if (!(cmd & PCI_COMMAND_MEMORY))
> +		pci_write_config_dword(pdev, PCI_COMMAND, cmd | PCI_COMMAND_MEMORY);
> +
> +	return 0;
> +}

NAK. If the pci code is broken then fix the problem there.
Do not add ugly hacks into random drivers.

> +
> +static void i915_disable_device(struct pci_dev *pdev)
> +{
> +	u32 cmd;
> +
> +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +	if (cmd & PCI_COMMAND_MEMORY)
> +		pci_write_config_dword(pdev, PCI_COMMAND, cmd & ~PCI_COMMAND_MEMORY);
> +
> +	pci_disable_device(pdev);
> +}
> +
>  static int i915_workqueues_init(struct drm_i915_private *dev_priv)
>  {
>  	/*
> @@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	struct intel_display *display;
>  	int ret;
>  
> -	ret = pci_enable_device(pdev);
> +	ret = i915_enable_device(pdev);
>  	if (ret) {
>  		pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
>  		return ret;
> @@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	i915 = i915_driver_create(pdev, ent);
>  	if (IS_ERR(i915)) {
> -		pci_disable_device(pdev);
> +		i915_disable_device(pdev);
>  		return PTR_ERR(i915);
>  	}
>  
> @@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
>  	i915_driver_late_release(i915);
>  out_pci_disable:
> -	pci_disable_device(pdev);
> +	i915_disable_device(pdev);
>  	i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
>  	return ret;
>  }
> @@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>  
>  	intel_dmc_suspend(display);
>  
> +	intel_pxp_fini(i915);

What is that doing in this patch?

>  	i915_gem_suspend(i915);
>  
>  	/*
> @@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
>  
>  	intel_runtime_pm_driver_last_release(&i915->runtime_pm);
> +	i915_disable_device(to_pci_dev(i915->drm.dev));
>  }
>  
>  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel

RE: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Yao, Jia]

You mean  intel_pxp_fini(i915)  ?
This is because mei_me_shutdown  is called after i915_driver_shutdown in pci_device_shutdown sequence.  If we don't close pxp in advance, it will cause 

[  295.584775] i915 0000:00:02.0: [drm] *ERROR* gt: MMIO unreliable (forcewake register returns 0xFFFFFFFF)!

Since we disabled PCI_COMMAND_MEMORY in  i915_driver_shutdown

Thanks,
Jia

-----Original Message-----
From: Ville Syrjälä <ville.syrjala@linux.intel.com> 
Sent: Tuesday, October 7, 2025 2:25 PM
To: Yao, Jia <jia.yao@intel.com>
Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson <chris.p.wilson@linux.intel.com>
Subject: Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

On Tue, Oct 07, 2025 at 08:25:14PM +0000, Jia Yao wrote:
> Make i915's PCI device management more robust by always 
> setting/clearing the memory access bit when enabling/disabling the 
> device, and by consolidating this logic into helper functions.
> 
> It fixes kexec reboot issue by disabling memory access before shutting 
> down the device, which can block unsafe and unwanted access from DMA.
> 
> v2:
>   - follow brace style
> 
> Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> Cc: Alex Zuo <alex.zuo@intel.com>
> Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> Cc: Askar Safin <safinaskar@gmail.com>
> Cc: Pingfan Liu <piliu@redhat.com>
> Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> Signed-off-by: Jia Yao <jia.yao@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_driver.c | 35 
> +++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_driver.c 
> b/drivers/gpu/drm/i915/i915_driver.c
> index b46cb54ef5dc..766f85726b67 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -118,6 +118,33 @@
>  
>  static const struct drm_driver i915_drm_driver;
>  
> +static int i915_enable_device(struct pci_dev *pdev) {
> +	u32 cmd;
> +	int ret;
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret)
> +		return ret;
> +
> +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +	if (!(cmd & PCI_COMMAND_MEMORY))
> +		pci_write_config_dword(pdev, PCI_COMMAND, cmd | 
> +PCI_COMMAND_MEMORY);
> +
> +	return 0;
> +}

NAK. If the pci code is broken then fix the problem there.
Do not add ugly hacks into random drivers.

> +
> +static void i915_disable_device(struct pci_dev *pdev) {
> +	u32 cmd;
> +
> +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +	if (cmd & PCI_COMMAND_MEMORY)
> +		pci_write_config_dword(pdev, PCI_COMMAND, cmd & 
> +~PCI_COMMAND_MEMORY);
> +
> +	pci_disable_device(pdev);
> +}
> +
>  static int i915_workqueues_init(struct drm_i915_private *dev_priv)  {
>  	/*
> @@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	struct intel_display *display;
>  	int ret;
>  
> -	ret = pci_enable_device(pdev);
> +	ret = i915_enable_device(pdev);
>  	if (ret) {
>  		pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
>  		return ret;
> @@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, const 
> struct pci_device_id *ent)
>  
>  	i915 = i915_driver_create(pdev, ent);
>  	if (IS_ERR(i915)) {
> -		pci_disable_device(pdev);
> +		i915_disable_device(pdev);
>  		return PTR_ERR(i915);
>  	}
>  
> @@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
>  	i915_driver_late_release(i915);
>  out_pci_disable:
> -	pci_disable_device(pdev);
> +	i915_disable_device(pdev);
>  	i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
>  	return ret;
>  }
> @@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct 
> drm_i915_private *i915)
>  
>  	intel_dmc_suspend(display);
>  
> +	intel_pxp_fini(i915);

What is that doing in this patch?

>  	i915_gem_suspend(i915);
>  
>  	/*
> @@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
>  
>  	intel_runtime_pm_driver_last_release(&i915->runtime_pm);
> +	i915_disable_device(to_pci_dev(i915->drm.dev));
>  }
>  
>  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
> --
> 2.34.1

--
Ville Syrjälä
Intel

Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Ville Syrjälä]

On Tue, Oct 07, 2025 at 09:40:45PM +0000, Yao, Jia wrote:
> You mean  intel_pxp_fini(i915)  ?
> This is because mei_me_shutdown  is called after i915_driver_shutdown in pci_device_shutdown sequence.  If we don't close pxp in advance, it will cause 
> 
> [  295.584775] i915 0000:00:02.0: [drm] *ERROR* gt: MMIO unreliable (forcewake register returns 0xFFFFFFFF)!

So that is the actual bug you're trying to fix? Please just submit
the pxp fix on its own.

> 
> Since we disabled PCI_COMMAND_MEMORY in  i915_driver_shutdown
> 
> Thanks,
> Jia
> 
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com> 
> Sent: Tuesday, October 7, 2025 2:25 PM
> To: Yao, Jia <jia.yao@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson <chris.p.wilson@linux.intel.com>
> Subject: Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915
> 
> On Tue, Oct 07, 2025 at 08:25:14PM +0000, Jia Yao wrote:
> > Make i915's PCI device management more robust by always 
> > setting/clearing the memory access bit when enabling/disabling the 
> > device, and by consolidating this logic into helper functions.
> > 
> > It fixes kexec reboot issue by disabling memory access before shutting 
> > down the device, which can block unsafe and unwanted access from DMA.
> > 
> > v2:
> >   - follow brace style
> > 
> > Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> > Cc: Alex Zuo <alex.zuo@intel.com>
> > Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> > Cc: Askar Safin <safinaskar@gmail.com>
> > Cc: Pingfan Liu <piliu@redhat.com>
> > Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> > Signed-off-by: Jia Yao <jia.yao@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_driver.c | 35 
> > +++++++++++++++++++++++++++---
> >  1 file changed, 32 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_driver.c 
> > b/drivers/gpu/drm/i915/i915_driver.c
> > index b46cb54ef5dc..766f85726b67 100644
> > --- a/drivers/gpu/drm/i915/i915_driver.c
> > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > @@ -118,6 +118,33 @@
> >  
> >  static const struct drm_driver i915_drm_driver;
> >  
> > +static int i915_enable_device(struct pci_dev *pdev) {
> > +	u32 cmd;
> > +	int ret;
> > +
> > +	ret = pci_enable_device(pdev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> > +	if (!(cmd & PCI_COMMAND_MEMORY))
> > +		pci_write_config_dword(pdev, PCI_COMMAND, cmd | 
> > +PCI_COMMAND_MEMORY);
> > +
> > +	return 0;
> > +}
> 
> NAK. If the pci code is broken then fix the problem there.
> Do not add ugly hacks into random drivers.
> 
> > +
> > +static void i915_disable_device(struct pci_dev *pdev) {
> > +	u32 cmd;
> > +
> > +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> > +	if (cmd & PCI_COMMAND_MEMORY)
> > +		pci_write_config_dword(pdev, PCI_COMMAND, cmd & 
> > +~PCI_COMMAND_MEMORY);
> > +
> > +	pci_disable_device(pdev);
> > +}
> > +
> >  static int i915_workqueues_init(struct drm_i915_private *dev_priv)  {
> >  	/*
> > @@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  	struct intel_display *display;
> >  	int ret;
> >  
> > -	ret = pci_enable_device(pdev);
> > +	ret = i915_enable_device(pdev);
> >  	if (ret) {
> >  		pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
> >  		return ret;
> > @@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, const 
> > struct pci_device_id *ent)
> >  
> >  	i915 = i915_driver_create(pdev, ent);
> >  	if (IS_ERR(i915)) {
> > -		pci_disable_device(pdev);
> > +		i915_disable_device(pdev);
> >  		return PTR_ERR(i915);
> >  	}
> >  
> > @@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
> >  	i915_driver_late_release(i915);
> >  out_pci_disable:
> > -	pci_disable_device(pdev);
> > +	i915_disable_device(pdev);
> >  	i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
> >  	return ret;
> >  }
> > @@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct 
> > drm_i915_private *i915)
> >  
> >  	intel_dmc_suspend(display);
> >  
> > +	intel_pxp_fini(i915);
> 
> What is that doing in this patch?
> 
> >  	i915_gem_suspend(i915);
> >  
> >  	/*
> > @@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
> >  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
> >  
> >  	intel_runtime_pm_driver_last_release(&i915->runtime_pm);
> > +	i915_disable_device(to_pci_dev(i915->drm.dev));
> >  }
> >  
> >  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
> > --
> > 2.34.1
> 
> --
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel

RE: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Yao, Jia]

The actual bug is showing in https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
if CONFIG_INTEL_IOMMU_DEFAULT_ON=y  ,  that IOMMU prevent the invalid access,  but if  CONFIG_INTEL_IOMMU_DEFAULT_ON=n,   the invalid access will directly cause system crash after kexec reboot.

-----Original Message-----
From: Ville Syrjälä <ville.syrjala@linux.intel.com> 
Sent: Wednesday, October 8, 2025 5:22 AM
To: Yao, Jia <jia.yao@intel.com>
Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson <chris.p.wilson@linux.intel.com>
Subject: Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

On Tue, Oct 07, 2025 at 09:40:45PM +0000, Yao, Jia wrote:
> You mean  intel_pxp_fini(i915)  ?
> This is because mei_me_shutdown  is called after i915_driver_shutdown 
> in pci_device_shutdown sequence.  If we don't close pxp in advance, it 
> will cause
> 
> [  295.584775] i915 0000:00:02.0: [drm] *ERROR* gt: MMIO unreliable (forcewake register returns 0xFFFFFFFF)!

So that is the actual bug you're trying to fix? Please just submit the pxp fix on its own.

> 
> Since we disabled PCI_COMMAND_MEMORY in  i915_driver_shutdown
> 
> Thanks,
> Jia
> 
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Tuesday, October 7, 2025 2:25 PM
> To: Yao, Jia <jia.yao@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; 
> Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin 
> <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson 
> <chris.p.wilson@linux.intel.com>
> Subject: Re: [PATCH v2] drm/i915: Setting/clearing the memory access 
> bit when en/disabling i915
> 
> On Tue, Oct 07, 2025 at 08:25:14PM +0000, Jia Yao wrote:
> > Make i915's PCI device management more robust by always 
> > setting/clearing the memory access bit when enabling/disabling the 
> > device, and by consolidating this logic into helper functions.
> > 
> > It fixes kexec reboot issue by disabling memory access before 
> > shutting down the device, which can block unsafe and unwanted access from DMA.
> > 
> > v2:
> >   - follow brace style
> > 
> > Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> > Cc: Alex Zuo <alex.zuo@intel.com>
> > Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> > Cc: Askar Safin <safinaskar@gmail.com>
> > Cc: Pingfan Liu <piliu@redhat.com>
> > Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> > Signed-off-by: Jia Yao <jia.yao@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_driver.c | 35
> > +++++++++++++++++++++++++++---
> >  1 file changed, 32 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_driver.c
> > b/drivers/gpu/drm/i915/i915_driver.c
> > index b46cb54ef5dc..766f85726b67 100644
> > --- a/drivers/gpu/drm/i915/i915_driver.c
> > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > @@ -118,6 +118,33 @@
> >  
> >  static const struct drm_driver i915_drm_driver;
> >  
> > +static int i915_enable_device(struct pci_dev *pdev) {
> > +	u32 cmd;
> > +	int ret;
> > +
> > +	ret = pci_enable_device(pdev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> > +	if (!(cmd & PCI_COMMAND_MEMORY))
> > +		pci_write_config_dword(pdev, PCI_COMMAND, cmd | 
> > +PCI_COMMAND_MEMORY);
> > +
> > +	return 0;
> > +}
> 
> NAK. If the pci code is broken then fix the problem there.
> Do not add ugly hacks into random drivers.
> 
> > +
> > +static void i915_disable_device(struct pci_dev *pdev) {
> > +	u32 cmd;
> > +
> > +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> > +	if (cmd & PCI_COMMAND_MEMORY)
> > +		pci_write_config_dword(pdev, PCI_COMMAND, cmd & 
> > +~PCI_COMMAND_MEMORY);
> > +
> > +	pci_disable_device(pdev);
> > +}
> > +
> >  static int i915_workqueues_init(struct drm_i915_private *dev_priv)  {
> >  	/*
> > @@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  	struct intel_display *display;
> >  	int ret;
> >  
> > -	ret = pci_enable_device(pdev);
> > +	ret = i915_enable_device(pdev);
> >  	if (ret) {
> >  		pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
> >  		return ret;
> > @@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, 
> > const struct pci_device_id *ent)
> >  
> >  	i915 = i915_driver_create(pdev, ent);
> >  	if (IS_ERR(i915)) {
> > -		pci_disable_device(pdev);
> > +		i915_disable_device(pdev);
> >  		return PTR_ERR(i915);
> >  	}
> >  
> > @@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
> >  	i915_driver_late_release(i915);
> >  out_pci_disable:
> > -	pci_disable_device(pdev);
> > +	i915_disable_device(pdev);
> >  	i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
> >  	return ret;
> >  }
> > @@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct 
> > drm_i915_private *i915)
> >  
> >  	intel_dmc_suspend(display);
> >  
> > +	intel_pxp_fini(i915);
> 
> What is that doing in this patch?
> 
> >  	i915_gem_suspend(i915);
> >  
> >  	/*
> > @@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
> >  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
> >  
> >  	intel_runtime_pm_driver_last_release(&i915->runtime_pm);
> > +	i915_disable_device(to_pci_dev(i915->drm.dev));
> >  }
> >  
> >  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
> > --
> > 2.34.1
> 
> --
> Ville Syrjälä
> Intel

--
Ville Syrjälä
Intel

Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Ville Syrjälä]

On Wed, Oct 08, 2025 at 04:06:39PM +0000, Yao, Jia wrote:
> The actual bug is showing in https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> if CONFIG_INTEL_IOMMU_DEFAULT_ON=y  ,  that IOMMU prevent the invalid access,  but if  CONFIG_INTEL_IOMMU_DEFAULT_ON=n,   the invalid access will directly cause system crash after kexec reboot.

I was asking you whether that invalid access was caused by that
pxp stuff or not?

If yes, then just fix it.

If not, then I guess someone needs to keep on debugging.

> 
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com> 
> Sent: Wednesday, October 8, 2025 5:22 AM
> To: Yao, Jia <jia.yao@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson <chris.p.wilson@linux.intel.com>
> Subject: Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915
> 
> On Tue, Oct 07, 2025 at 09:40:45PM +0000, Yao, Jia wrote:
> > You mean  intel_pxp_fini(i915)  ?
> > This is because mei_me_shutdown  is called after i915_driver_shutdown 
> > in pci_device_shutdown sequence.  If we don't close pxp in advance, it 
> > will cause
> > 
> > [  295.584775] i915 0000:00:02.0: [drm] *ERROR* gt: MMIO unreliable (forcewake register returns 0xFFFFFFFF)!
> 
> So that is the actual bug you're trying to fix? Please just submit the pxp fix on its own.
> 
> > 
> > Since we disabled PCI_COMMAND_MEMORY in  i915_driver_shutdown
> > 
> > Thanks,
> > Jia
> > 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: Tuesday, October 7, 2025 2:25 PM
> > To: Yao, Jia <jia.yao@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; 
> > Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin 
> > <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson 
> > <chris.p.wilson@linux.intel.com>
> > Subject: Re: [PATCH v2] drm/i915: Setting/clearing the memory access 
> > bit when en/disabling i915
> > 
> > On Tue, Oct 07, 2025 at 08:25:14PM +0000, Jia Yao wrote:
> > > Make i915's PCI device management more robust by always 
> > > setting/clearing the memory access bit when enabling/disabling the 
> > > device, and by consolidating this logic into helper functions.
> > > 
> > > It fixes kexec reboot issue by disabling memory access before 
> > > shutting down the device, which can block unsafe and unwanted access from DMA.
> > > 
> > > v2:
> > >   - follow brace style
> > > 
> > > Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> > > Cc: Alex Zuo <alex.zuo@intel.com>
> > > Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> > > Cc: Askar Safin <safinaskar@gmail.com>
> > > Cc: Pingfan Liu <piliu@redhat.com>
> > > Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> > > Signed-off-by: Jia Yao <jia.yao@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_driver.c | 35
> > > +++++++++++++++++++++++++++---
> > >  1 file changed, 32 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_driver.c
> > > b/drivers/gpu/drm/i915/i915_driver.c
> > > index b46cb54ef5dc..766f85726b67 100644
> > > --- a/drivers/gpu/drm/i915/i915_driver.c
> > > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > > @@ -118,6 +118,33 @@
> > >  
> > >  static const struct drm_driver i915_drm_driver;
> > >  
> > > +static int i915_enable_device(struct pci_dev *pdev) {
> > > +	u32 cmd;
> > > +	int ret;
> > > +
> > > +	ret = pci_enable_device(pdev);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> > > +	if (!(cmd & PCI_COMMAND_MEMORY))
> > > +		pci_write_config_dword(pdev, PCI_COMMAND, cmd | 
> > > +PCI_COMMAND_MEMORY);
> > > +
> > > +	return 0;
> > > +}
> > 
> > NAK. If the pci code is broken then fix the problem there.
> > Do not add ugly hacks into random drivers.
> > 
> > > +
> > > +static void i915_disable_device(struct pci_dev *pdev) {
> > > +	u32 cmd;
> > > +
> > > +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> > > +	if (cmd & PCI_COMMAND_MEMORY)
> > > +		pci_write_config_dword(pdev, PCI_COMMAND, cmd & 
> > > +~PCI_COMMAND_MEMORY);
> > > +
> > > +	pci_disable_device(pdev);
> > > +}
> > > +
> > >  static int i915_workqueues_init(struct drm_i915_private *dev_priv)  {
> > >  	/*
> > > @@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > >  	struct intel_display *display;
> > >  	int ret;
> > >  
> > > -	ret = pci_enable_device(pdev);
> > > +	ret = i915_enable_device(pdev);
> > >  	if (ret) {
> > >  		pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
> > >  		return ret;
> > > @@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, 
> > > const struct pci_device_id *ent)
> > >  
> > >  	i915 = i915_driver_create(pdev, ent);
> > >  	if (IS_ERR(i915)) {
> > > -		pci_disable_device(pdev);
> > > +		i915_disable_device(pdev);
> > >  		return PTR_ERR(i915);
> > >  	}
> > >  
> > > @@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > >  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
> > >  	i915_driver_late_release(i915);
> > >  out_pci_disable:
> > > -	pci_disable_device(pdev);
> > > +	i915_disable_device(pdev);
> > >  	i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
> > >  	return ret;
> > >  }
> > > @@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct 
> > > drm_i915_private *i915)
> > >  
> > >  	intel_dmc_suspend(display);
> > >  
> > > +	intel_pxp_fini(i915);
> > 
> > What is that doing in this patch?
> > 
> > >  	i915_gem_suspend(i915);
> > >  
> > >  	/*
> > > @@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
> > >  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
> > >  
> > >  	intel_runtime_pm_driver_last_release(&i915->runtime_pm);
> > > +	i915_disable_device(to_pci_dev(i915->drm.dev));
> > >  }
> > >  
> > >  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
> > > --
> > > 2.34.1
> > 
> > --
> > Ville Syrjälä
> > Intel
> 
> --
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel

RE: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Yao, Jia]

Good questions.   @Pingfan Liu  @Ville Syrjälä

Driver-wise,  no other access except for pxp stuff,  for that after disabling PCI_COMMAND_MEMORY,  any access will cause MMIO failure,  they will not be able to be hidden.
And the invalid access is not from pxp,  otherwise just doing   intel_pxp_fini   in i915_driver_shutdown will fix the issue, it might come from firmware or other that i915 driver can't see.  

Current solution is defensive,  not harmful just like turning on write protection on a floppy disk when not using it.

Thanks,
Jia

-----Original Message-----
From: Ville Syrjälä <ville.syrjala@linux.intel.com> 
Sent: Wednesday, October 8, 2025 9:16 AM
To: Yao, Jia <jia.yao@intel.com>
Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson <chris.p.wilson@linux.intel.com>
Subject: Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

On Wed, Oct 08, 2025 at 04:06:39PM +0000, Yao, Jia wrote:
> The actual bug is showing in https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> if CONFIG_INTEL_IOMMU_DEFAULT_ON=y  ,  that IOMMU prevent the invalid access,  but if  CONFIG_INTEL_IOMMU_DEFAULT_ON=n,   the invalid access will directly cause system crash after kexec reboot.

I was asking you whether that invalid access was caused by that pxp stuff or not?

If yes, then just fix it.

If not, then I guess someone needs to keep on debugging.

> 
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Wednesday, October 8, 2025 5:22 AM
> To: Yao, Jia <jia.yao@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; 
> Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin 
> <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson 
> <chris.p.wilson@linux.intel.com>
> Subject: Re: [PATCH v2] drm/i915: Setting/clearing the memory access 
> bit when en/disabling i915
> 
> On Tue, Oct 07, 2025 at 09:40:45PM +0000, Yao, Jia wrote:
> > You mean  intel_pxp_fini(i915)  ?
> > This is because mei_me_shutdown  is called after 
> > i915_driver_shutdown in pci_device_shutdown sequence.  If we don't 
> > close pxp in advance, it will cause
> > 
> > [  295.584775] i915 0000:00:02.0: [drm] *ERROR* gt: MMIO unreliable (forcewake register returns 0xFFFFFFFF)!
> 
> So that is the actual bug you're trying to fix? Please just submit the pxp fix on its own.
> 
> > 
> > Since we disabled PCI_COMMAND_MEMORY in  i915_driver_shutdown
> > 
> > Thanks,
> > Jia
> > 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: Tuesday, October 7, 2025 2:25 PM
> > To: Yao, Jia <jia.yao@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; 
> > Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin 
> > <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson 
> > <chris.p.wilson@linux.intel.com>
> > Subject: Re: [PATCH v2] drm/i915: Setting/clearing the memory access 
> > bit when en/disabling i915
> > 
> > On Tue, Oct 07, 2025 at 08:25:14PM +0000, Jia Yao wrote:
> > > Make i915's PCI device management more robust by always 
> > > setting/clearing the memory access bit when enabling/disabling the 
> > > device, and by consolidating this logic into helper functions.
> > > 
> > > It fixes kexec reboot issue by disabling memory access before 
> > > shutting down the device, which can block unsafe and unwanted access from DMA.
> > > 
> > > v2:
> > >   - follow brace style
> > > 
> > > Link: 
> > > https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> > > Cc: Alex Zuo <alex.zuo@intel.com>
> > > Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> > > Cc: Askar Safin <safinaskar@gmail.com>
> > > Cc: Pingfan Liu <piliu@redhat.com>
> > > Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> > > Signed-off-by: Jia Yao <jia.yao@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_driver.c | 35
> > > +++++++++++++++++++++++++++---
> > >  1 file changed, 32 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_driver.c
> > > b/drivers/gpu/drm/i915/i915_driver.c
> > > index b46cb54ef5dc..766f85726b67 100644
> > > --- a/drivers/gpu/drm/i915/i915_driver.c
> > > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > > @@ -118,6 +118,33 @@
> > >  
> > >  static const struct drm_driver i915_drm_driver;
> > >  
> > > +static int i915_enable_device(struct pci_dev *pdev) {
> > > +	u32 cmd;
> > > +	int ret;
> > > +
> > > +	ret = pci_enable_device(pdev);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> > > +	if (!(cmd & PCI_COMMAND_MEMORY))
> > > +		pci_write_config_dword(pdev, PCI_COMMAND, cmd | 
> > > +PCI_COMMAND_MEMORY);
> > > +
> > > +	return 0;
> > > +}
> > 
> > NAK. If the pci code is broken then fix the problem there.
> > Do not add ugly hacks into random drivers.
> > 
> > > +
> > > +static void i915_disable_device(struct pci_dev *pdev) {
> > > +	u32 cmd;
> > > +
> > > +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> > > +	if (cmd & PCI_COMMAND_MEMORY)
> > > +		pci_write_config_dword(pdev, PCI_COMMAND, cmd & 
> > > +~PCI_COMMAND_MEMORY);
> > > +
> > > +	pci_disable_device(pdev);
> > > +}
> > > +
> > >  static int i915_workqueues_init(struct drm_i915_private *dev_priv)  {
> > >  	/*
> > > @@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > >  	struct intel_display *display;
> > >  	int ret;
> > >  
> > > -	ret = pci_enable_device(pdev);
> > > +	ret = i915_enable_device(pdev);
> > >  	if (ret) {
> > >  		pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
> > >  		return ret;
> > > @@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, 
> > > const struct pci_device_id *ent)
> > >  
> > >  	i915 = i915_driver_create(pdev, ent);
> > >  	if (IS_ERR(i915)) {
> > > -		pci_disable_device(pdev);
> > > +		i915_disable_device(pdev);
> > >  		return PTR_ERR(i915);
> > >  	}
> > >  
> > > @@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > >  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
> > >  	i915_driver_late_release(i915);
> > >  out_pci_disable:
> > > -	pci_disable_device(pdev);
> > > +	i915_disable_device(pdev);
> > >  	i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
> > >  	return ret;
> > >  }
> > > @@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct 
> > > drm_i915_private *i915)
> > >  
> > >  	intel_dmc_suspend(display);
> > >  
> > > +	intel_pxp_fini(i915);
> > 
> > What is that doing in this patch?
> > 
> > >  	i915_gem_suspend(i915);
> > >  
> > >  	/*
> > > @@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
> > >  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
> > >  
> > >  	intel_runtime_pm_driver_last_release(&i915->runtime_pm);
> > > +	i915_disable_device(to_pci_dev(i915->drm.dev));
> > >  }
> > >  
> > >  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
> > > --
> > > 2.34.1
> > 
> > --
> > Ville Syrjälä
> > Intel
> 
> --
> Ville Syrjälä
> Intel

--
Ville Syrjälä
Intel

Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Pingfan Liu]

On Thu, Oct 9, 2025 at 1:14 AM Yao, Jia <jia.yao@intel.com> wrote:
>
> Good questions.   @Pingfan Liu  @Ville Syrjälä
>
> Driver-wise,  no other access except for pxp stuff,  for that after disabling PCI_COMMAND_MEMORY,  any access will cause MMIO failure,  they will not be able to be hidden.
> And the invalid access is not from pxp,  otherwise just doing   intel_pxp_fini   in i915_driver_shutdown will fix the issue, it might come from firmware or other that i915 driver can't see.
>

Thank you for the clear explanation.

> Current solution is defensive,  not harmful just like turning on write protection on a floppy disk when not using it.
>

I have a couple of questions about this patch:
First, at what point (in the shutdown sequence) should the
PCI_COMMAND_MEMORY bit be cleared in i915_driver_shutdown()? Does your
patch clear it too late?
Second, in the second kernel (after kexec), is there a proactive way
to force the i915 driver into a clean state so it can recover from
invalid memory accesses that occurred in the first kernel? I think
this will be a better choice.

I realize I may be nitpicking a bit, since these details might not be
publicly documented. So I think your patch is reasonable.


Thanks,

Pingfan


> Thanks,
> Jia
>
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Wednesday, October 8, 2025 9:16 AM
> To: Yao, Jia <jia.yao@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson <chris.p.wilson@linux.intel.com>
> Subject: Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915
>
> On Wed, Oct 08, 2025 at 04:06:39PM +0000, Yao, Jia wrote:
> > The actual bug is showing in https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> > if CONFIG_INTEL_IOMMU_DEFAULT_ON=y  ,  that IOMMU prevent the invalid access,  but if  CONFIG_INTEL_IOMMU_DEFAULT_ON=n,   the invalid access will directly cause system crash after kexec reboot.
>
> I was asking you whether that invalid access was caused by that pxp stuff or not?
>
> If yes, then just fix it.
>
> If not, then I guess someone needs to keep on debugging.
>
> >
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: Wednesday, October 8, 2025 5:22 AM
> > To: Yao, Jia <jia.yao@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>;
> > Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin
> > <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson
> > <chris.p.wilson@linux.intel.com>
> > Subject: Re: [PATCH v2] drm/i915: Setting/clearing the memory access
> > bit when en/disabling i915
> >
> > On Tue, Oct 07, 2025 at 09:40:45PM +0000, Yao, Jia wrote:
> > > You mean  intel_pxp_fini(i915)  ?
> > > This is because mei_me_shutdown  is called after
> > > i915_driver_shutdown in pci_device_shutdown sequence.  If we don't
> > > close pxp in advance, it will cause
> > >
> > > [  295.584775] i915 0000:00:02.0: [drm] *ERROR* gt: MMIO unreliable (forcewake register returns 0xFFFFFFFF)!
> >
> > So that is the actual bug you're trying to fix? Please just submit the pxp fix on its own.
> >
> > >
> > > Since we disabled PCI_COMMAND_MEMORY in  i915_driver_shutdown
> > >
> > > Thanks,
> > > Jia
> > >
> > > -----Original Message-----
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Sent: Tuesday, October 7, 2025 2:25 PM
> > > To: Yao, Jia <jia.yao@intel.com>
> > > Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>;
> > > Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin
> > > <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson
> > > <chris.p.wilson@linux.intel.com>
> > > Subject: Re: [PATCH v2] drm/i915: Setting/clearing the memory access
> > > bit when en/disabling i915
> > >
> > > On Tue, Oct 07, 2025 at 08:25:14PM +0000, Jia Yao wrote:
> > > > Make i915's PCI device management more robust by always
> > > > setting/clearing the memory access bit when enabling/disabling the
> > > > device, and by consolidating this logic into helper functions.
> > > >
> > > > It fixes kexec reboot issue by disabling memory access before
> > > > shutting down the device, which can block unsafe and unwanted access from DMA.
> > > >
> > > > v2:
> > > >   - follow brace style
> > > >
> > > > Link:
> > > > https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> > > > Cc: Alex Zuo <alex.zuo@intel.com>
> > > > Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> > > > Cc: Askar Safin <safinaskar@gmail.com>
> > > > Cc: Pingfan Liu <piliu@redhat.com>
> > > > Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> > > > Signed-off-by: Jia Yao <jia.yao@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_driver.c | 35
> > > > +++++++++++++++++++++++++++---
> > > >  1 file changed, 32 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_driver.c
> > > > b/drivers/gpu/drm/i915/i915_driver.c
> > > > index b46cb54ef5dc..766f85726b67 100644
> > > > --- a/drivers/gpu/drm/i915/i915_driver.c
> > > > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > > > @@ -118,6 +118,33 @@
> > > >
> > > >  static const struct drm_driver i915_drm_driver;
> > > >
> > > > +static int i915_enable_device(struct pci_dev *pdev) {
> > > > + u32 cmd;
> > > > + int ret;
> > > > +
> > > > + ret = pci_enable_device(pdev);
> > > > + if (ret)
> > > > +         return ret;
> > > > +
> > > > + pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> > > > + if (!(cmd & PCI_COMMAND_MEMORY))
> > > > +         pci_write_config_dword(pdev, PCI_COMMAND, cmd |
> > > > +PCI_COMMAND_MEMORY);
> > > > +
> > > > + return 0;
> > > > +}
> > >
> > > NAK. If the pci code is broken then fix the problem there.
> > > Do not add ugly hacks into random drivers.
> > >
> > > > +
> > > > +static void i915_disable_device(struct pci_dev *pdev) {
> > > > + u32 cmd;
> > > > +
> > > > + pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> > > > + if (cmd & PCI_COMMAND_MEMORY)
> > > > +         pci_write_config_dword(pdev, PCI_COMMAND, cmd &
> > > > +~PCI_COMMAND_MEMORY);
> > > > +
> > > > + pci_disable_device(pdev);
> > > > +}
> > > > +
> > > >  static int i915_workqueues_init(struct drm_i915_private *dev_priv)  {
> > > >   /*
> > > > @@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > > >   struct intel_display *display;
> > > >   int ret;
> > > >
> > > > - ret = pci_enable_device(pdev);
> > > > + ret = i915_enable_device(pdev);
> > > >   if (ret) {
> > > >           pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
> > > >           return ret;
> > > > @@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev,
> > > > const struct pci_device_id *ent)
> > > >
> > > >   i915 = i915_driver_create(pdev, ent);
> > > >   if (IS_ERR(i915)) {
> > > > -         pci_disable_device(pdev);
> > > > +         i915_disable_device(pdev);
> > > >           return PTR_ERR(i915);
> > > >   }
> > > >
> > > > @@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > > >   enable_rpm_wakeref_asserts(&i915->runtime_pm);
> > > >   i915_driver_late_release(i915);
> > > >  out_pci_disable:
> > > > - pci_disable_device(pdev);
> > > > + i915_disable_device(pdev);
> > > >   i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
> > > >   return ret;
> > > >  }
> > > > @@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct
> > > > drm_i915_private *i915)
> > > >
> > > >   intel_dmc_suspend(display);
> > > >
> > > > + intel_pxp_fini(i915);
> > >
> > > What is that doing in this patch?
> > >
> > > >   i915_gem_suspend(i915);
> > > >
> > > >   /*
> > > > @@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
> > > >   enable_rpm_wakeref_asserts(&i915->runtime_pm);
> > > >
> > > >   intel_runtime_pm_driver_last_release(&i915->runtime_pm);
> > > > + i915_disable_device(to_pci_dev(i915->drm.dev));
> > > >  }
> > > >
> > > >  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
> > > > --
> > > > 2.34.1
> > >
> > > --
> > > Ville Syrjälä
> > > Intel
> >
> > --
> > Ville Syrjälä
> > Intel
>
> --
> Ville Syrjälä
> Intel
>

Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Askar Safin]

"Ville Syrjälä" <ville.syrjala@linux.intel.com>:
> I was asking you whether that invalid access was caused by that
> pxp stuff or not?

I just tested patch, which adds intel_pxp_fini(i915) to
i915_driver_shutdown and does nothing else.

The bug was not fixed.

Also, do you have machines for testing, except for my laptop? :)

Anyway, if you want me to test something, just ask.

-- 
Askar Safin

Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Askar Safin]

Also, I just tested opposite thing. I. e. I tested v2 patch, but without
intel_pxp_fini(i915) line. And the patch worked.
I. e. intel_pxp_fini(i915) line is not needed for fix to work.

-- 
Askar Safin

RE: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Yao, Jia]

Thanks, Askar Safin,  

intel_pxp_fini(i915) line is needed.  The error message can only be seen in serial log,  dmesg can't show it.

-----Original Message-----
From: Askar Safin <safinaskar@gmail.com> 
Sent: Saturday, October 11, 2025 5:49 AM
To: ville.syrjala@linux.intel.com
Cc: Zuo, Alex <alex.zuo@intel.com>; chris.p.wilson@linux.intel.com; intel-gfx@lists.freedesktop.org; Yao, Jia <jia.yao@intel.com>; piliu@redhat.com; Lin, Shuicheng <shuicheng.lin@intel.com>
Subject: Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

Also, I just tested opposite thing. I. e. I tested v2 patch, but without
intel_pxp_fini(i915) line. And the patch worked.
I. e. intel_pxp_fini(i915) line is not needed for fix to work.

-- 
Askar Safin

Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Pingfan Liu]

On Sat, Oct 11, 2025 at 8:49 PM Askar Safin <safinaskar@gmail.com> wrote:
>
> Also, I just tested opposite thing. I. e. I tested v2 patch, but without
> intel_pxp_fini(i915) line. And the patch worked.
> I. e. intel_pxp_fini(i915) line is not needed for fix to work.
>

Since the best solution is not available, can this patch be considered
as an acceptable remedy for the kexec case?

Thanks,

Pingfan

Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Askar Safin]

Pingfan Liu <piliu@redhat.com>:
> Since the best solution is not available, can this patch be considered
> as an acceptable remedy for the kexec case?

This is question to me? I'm okay with any solution, which works. :)

As I said, v2 patch works for me. Both with and without "intel_pxp_fini(i915)"

-- 
Askar Safin

Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Pingfan Liu]

Hi Jia,

Thanks for the patch, please see the comments below.

On Wed, Oct 8, 2025 at 4:25 AM Jia Yao <jia.yao@intel.com> wrote:
>
> Make i915's PCI device management more robust by always setting/clearing
> the memory access bit when enabling/disabling the device, and by
> consolidating this logic into helper functions.
>
> It fixes kexec reboot issue by disabling memory access before shutting
> down the device, which can block unsafe and unwanted access from DMA.
>

PCI_COMMAND_MEMORY blocks the access to i915
PCI_COMMAND_MASTER blocks i915 from accessing the system memory.

In the case of kexec-reboot, I think PCI_COMMAND_MASTER has been set
on all pci devices. So I can not figure out how clearing
PCI_COMMAND_MEMORY can help in this case.

Can you explain a little bit?

Thanks,

Pingfan

> v2:
>   - follow brace style
>
> Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> Cc: Alex Zuo <alex.zuo@intel.com>
> Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> Cc: Askar Safin <safinaskar@gmail.com>
> Cc: Pingfan Liu <piliu@redhat.com>
> Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> Signed-off-by: Jia Yao <jia.yao@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_driver.c | 35 +++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index b46cb54ef5dc..766f85726b67 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -118,6 +118,33 @@
>
>  static const struct drm_driver i915_drm_driver;
>
> +static int i915_enable_device(struct pci_dev *pdev)
> +{
> +       u32 cmd;
> +       int ret;
> +
> +       ret = pci_enable_device(pdev);
> +       if (ret)
> +               return ret;
> +
> +       pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +       if (!(cmd & PCI_COMMAND_MEMORY))
> +               pci_write_config_dword(pdev, PCI_COMMAND, cmd | PCI_COMMAND_MEMORY);
> +
> +       return 0;
> +}
> +
> +static void i915_disable_device(struct pci_dev *pdev)
> +{
> +       u32 cmd;
> +
> +       pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +       if (cmd & PCI_COMMAND_MEMORY)
> +               pci_write_config_dword(pdev, PCI_COMMAND, cmd & ~PCI_COMMAND_MEMORY);
> +
> +       pci_disable_device(pdev);
> +}
> +
>  static int i915_workqueues_init(struct drm_i915_private *dev_priv)
>  {
>         /*
> @@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>         struct intel_display *display;
>         int ret;
>
> -       ret = pci_enable_device(pdev);
> +       ret = i915_enable_device(pdev);
>         if (ret) {
>                 pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
>                 return ret;
> @@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
>         i915 = i915_driver_create(pdev, ent);
>         if (IS_ERR(i915)) {
> -               pci_disable_device(pdev);
> +               i915_disable_device(pdev);
>                 return PTR_ERR(i915);
>         }
>
> @@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>         enable_rpm_wakeref_asserts(&i915->runtime_pm);
>         i915_driver_late_release(i915);
>  out_pci_disable:
> -       pci_disable_device(pdev);
> +       i915_disable_device(pdev);
>         i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
>         return ret;
>  }
> @@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>
>         intel_dmc_suspend(display);
>
> +       intel_pxp_fini(i915);
>         i915_gem_suspend(i915);
>
>         /*
> @@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>         enable_rpm_wakeref_asserts(&i915->runtime_pm);
>
>         intel_runtime_pm_driver_last_release(&i915->runtime_pm);
> +       i915_disable_device(to_pci_dev(i915->drm.dev));
>  }
>
>  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
> --
> 2.34.1
>

RE: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Yao, Jia]

Hi Pingfan,

You're welcome.  I guess you are referring to the following code, 

static void pci_device_shutdown(struct device *dev)
{
    struct pci_dev *pci_dev = to_pci_dev(dev);
    struct pci_driver *drv = pci_dev->driver;

    pm_runtime_resume(dev);

    if (drv && drv->shutdown)
        drv->shutdown(pci_dev);

    /*
     * If this is a kexec reboot, turn off Bus Master bit on the
     * device to tell it to not continue to do DMA. Don't touch
     * devices in D3cold or unknown states.
     * If it is not a kexec reboot, firmware will hit the PCI
     * devices with big hammer and stop their DMA any way.
     */
    if (kexec_in_progress && (pci_dev->current_state <= PCI_D3hot))
        pci_clear_master(pci_dev);
}

but it just disables the i915 device as a DMA master,   not prevent DMA access to i915 device. 

Thanks,
Jia

-----Original Message-----
From: Pingfan Liu <piliu@redhat.com> 
Sent: Tuesday, October 7, 2025 10:18 PM
To: Yao, Jia <jia.yao@intel.com>
Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin <safinaskar@gmail.com>; Chris Wilson <chris.p.wilson@linux.intel.com>
Subject: Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

Hi Jia,

Thanks for the patch, please see the comments below.

On Wed, Oct 8, 2025 at 4:25 AM Jia Yao <jia.yao@intel.com> wrote:
>
> Make i915's PCI device management more robust by always 
> setting/clearing the memory access bit when enabling/disabling the 
> device, and by consolidating this logic into helper functions.
>
> It fixes kexec reboot issue by disabling memory access before shutting 
> down the device, which can block unsafe and unwanted access from DMA.
>

PCI_COMMAND_MEMORY blocks the access to i915 PCI_COMMAND_MASTER blocks i915 from accessing the system memory.

In the case of kexec-reboot, I think PCI_COMMAND_MASTER has been set on all pci devices. So I can not figure out how clearing PCI_COMMAND_MEMORY can help in this case.

Can you explain a little bit?

Thanks,

Pingfan

> v2:
>   - follow brace style
>
> Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> Cc: Alex Zuo <alex.zuo@intel.com>
> Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> Cc: Askar Safin <safinaskar@gmail.com>
> Cc: Pingfan Liu <piliu@redhat.com>
> Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> Signed-off-by: Jia Yao <jia.yao@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_driver.c | 35 
> +++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_driver.c 
> b/drivers/gpu/drm/i915/i915_driver.c
> index b46cb54ef5dc..766f85726b67 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -118,6 +118,33 @@
>
>  static const struct drm_driver i915_drm_driver;
>
> +static int i915_enable_device(struct pci_dev *pdev) {
> +       u32 cmd;
> +       int ret;
> +
> +       ret = pci_enable_device(pdev);
> +       if (ret)
> +               return ret;
> +
> +       pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +       if (!(cmd & PCI_COMMAND_MEMORY))
> +               pci_write_config_dword(pdev, PCI_COMMAND, cmd | 
> + PCI_COMMAND_MEMORY);
> +
> +       return 0;
> +}
> +
> +static void i915_disable_device(struct pci_dev *pdev) {
> +       u32 cmd;
> +
> +       pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +       if (cmd & PCI_COMMAND_MEMORY)
> +               pci_write_config_dword(pdev, PCI_COMMAND, cmd & 
> + ~PCI_COMMAND_MEMORY);
> +
> +       pci_disable_device(pdev);
> +}
> +
>  static int i915_workqueues_init(struct drm_i915_private *dev_priv)  {
>         /*
> @@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>         struct intel_display *display;
>         int ret;
>
> -       ret = pci_enable_device(pdev);
> +       ret = i915_enable_device(pdev);
>         if (ret) {
>                 pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
>                 return ret;
> @@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, const 
> struct pci_device_id *ent)
>
>         i915 = i915_driver_create(pdev, ent);
>         if (IS_ERR(i915)) {
> -               pci_disable_device(pdev);
> +               i915_disable_device(pdev);
>                 return PTR_ERR(i915);
>         }
>
> @@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>         enable_rpm_wakeref_asserts(&i915->runtime_pm);
>         i915_driver_late_release(i915);
>  out_pci_disable:
> -       pci_disable_device(pdev);
> +       i915_disable_device(pdev);
>         i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
>         return ret;
>  }
> @@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct 
> drm_i915_private *i915)
>
>         intel_dmc_suspend(display);
>
> +       intel_pxp_fini(i915);
>         i915_gem_suspend(i915);
>
>         /*
> @@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>         enable_rpm_wakeref_asserts(&i915->runtime_pm);
>
>         intel_runtime_pm_driver_last_release(&i915->runtime_pm);
> +       i915_disable_device(to_pci_dev(i915->drm.dev));
>  }
>
>  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
> --
> 2.34.1
>

Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Pingfan Liu]

On Wed, Oct 8, 2025 at 3:06 PM Yao, Jia <jia.yao@intel.com> wrote:
>
> Hi Pingfan,
>
> You're welcome.  I guess you are referring to the following code,
>
> static void pci_device_shutdown(struct device *dev)
> {
>     struct pci_dev *pci_dev = to_pci_dev(dev);
>     struct pci_driver *drv = pci_dev->driver;
>
>     pm_runtime_resume(dev);
>
>     if (drv && drv->shutdown)
>         drv->shutdown(pci_dev);
>
>     /*
>      * If this is a kexec reboot, turn off Bus Master bit on the
>      * device to tell it to not continue to do DMA. Don't touch
>      * devices in D3cold or unknown states.
>      * If it is not a kexec reboot, firmware will hit the PCI
>      * devices with big hammer and stop their DMA any way.
>      */
>     if (kexec_in_progress && (pci_dev->current_state <= PCI_D3hot))
>         pci_clear_master(pci_dev);
> }
>
> but it just disables the i915 device as a DMA master,   not prevent DMA access to i915 device.
>

That is not all of my questions. Let me rephrase my question in a
different way: which device accesses i915's memory? And does it send
i915 to an insane state, which can not be recovered in the second
kernel?


Thanks,

Pingfan

> Thanks,
> Jia
>
> -----Original Message-----
> From: Pingfan Liu <piliu@redhat.com>
> Sent: Tuesday, October 7, 2025 10:18 PM
> To: Yao, Jia <jia.yao@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin <safinaskar@gmail.com>; Chris Wilson <chris.p.wilson@linux.intel.com>
> Subject: Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915
>
> Hi Jia,
>
> Thanks for the patch, please see the comments below.
>
> On Wed, Oct 8, 2025 at 4:25 AM Jia Yao <jia.yao@intel.com> wrote:
> >
> > Make i915's PCI device management more robust by always
> > setting/clearing the memory access bit when enabling/disabling the
> > device, and by consolidating this logic into helper functions.
> >
> > It fixes kexec reboot issue by disabling memory access before shutting
> > down the device, which can block unsafe and unwanted access from DMA.
> >
>
> PCI_COMMAND_MEMORY blocks the access to i915 PCI_COMMAND_MASTER blocks i915 from accessing the system memory.
>
> In the case of kexec-reboot, I think PCI_COMMAND_MASTER has been set on all pci devices. So I can not figure out how clearing PCI_COMMAND_MEMORY can help in this case.
>
> Can you explain a little bit?
>
> Thanks,
>
> Pingfan
>
> > v2:
> >   - follow brace style
> >
> > Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> > Cc: Alex Zuo <alex.zuo@intel.com>
> > Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> > Cc: Askar Safin <safinaskar@gmail.com>
> > Cc: Pingfan Liu <piliu@redhat.com>
> > Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> > Signed-off-by: Jia Yao <jia.yao@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_driver.c | 35
> > +++++++++++++++++++++++++++---
> >  1 file changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_driver.c
> > b/drivers/gpu/drm/i915/i915_driver.c
> > index b46cb54ef5dc..766f85726b67 100644
> > --- a/drivers/gpu/drm/i915/i915_driver.c
> > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > @@ -118,6 +118,33 @@
> >
> >  static const struct drm_driver i915_drm_driver;
> >
> > +static int i915_enable_device(struct pci_dev *pdev) {
> > +       u32 cmd;
> > +       int ret;
> > +
> > +       ret = pci_enable_device(pdev);
> > +       if (ret)
> > +               return ret;
> > +
> > +       pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> > +       if (!(cmd & PCI_COMMAND_MEMORY))
> > +               pci_write_config_dword(pdev, PCI_COMMAND, cmd |
> > + PCI_COMMAND_MEMORY);
> > +
> > +       return 0;
> > +}
> > +
> > +static void i915_disable_device(struct pci_dev *pdev) {
> > +       u32 cmd;
> > +
> > +       pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> > +       if (cmd & PCI_COMMAND_MEMORY)
> > +               pci_write_config_dword(pdev, PCI_COMMAND, cmd &
> > + ~PCI_COMMAND_MEMORY);
> > +
> > +       pci_disable_device(pdev);
> > +}
> > +
> >  static int i915_workqueues_init(struct drm_i915_private *dev_priv)  {
> >         /*
> > @@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >         struct intel_display *display;
> >         int ret;
> >
> > -       ret = pci_enable_device(pdev);
> > +       ret = i915_enable_device(pdev);
> >         if (ret) {
> >                 pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
> >                 return ret;
> > @@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> >
> >         i915 = i915_driver_create(pdev, ent);
> >         if (IS_ERR(i915)) {
> > -               pci_disable_device(pdev);
> > +               i915_disable_device(pdev);
> >                 return PTR_ERR(i915);
> >         }
> >
> > @@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >         enable_rpm_wakeref_asserts(&i915->runtime_pm);
> >         i915_driver_late_release(i915);
> >  out_pci_disable:
> > -       pci_disable_device(pdev);
> > +       i915_disable_device(pdev);
> >         i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
> >         return ret;
> >  }
> > @@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct
> > drm_i915_private *i915)
> >
> >         intel_dmc_suspend(display);
> >
> > +       intel_pxp_fini(i915);
> >         i915_gem_suspend(i915);
> >
> >         /*
> > @@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
> >         enable_rpm_wakeref_asserts(&i915->runtime_pm);
> >
> >         intel_runtime_pm_driver_last_release(&i915->runtime_pm);
> > +       i915_disable_device(to_pci_dev(i915->drm.dev));
> >  }
> >
> >  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
> > --
> > 2.34.1
> >
>

Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Askar Safin]

On Tue, Oct 7, 2025 at 11:25 PM Jia Yao <jia.yao@intel.com> wrote:
> Make i915's PCI device management more robust by always setting/clearing

Thank you! I tested this patch, and it indeed fixes the problem. You may add:

Tested-By: Askar Safin <safinaskar@gmail.com>

Please, CC me with further patches, I will test them.

Also, I will repeat that the bug doesn't happen if we have
CONFIG_INTEL_IOMMU_DEFAULT_ON=y.
So, probably we just need to patch all distro's configs to include
CONFIG_INTEL_IOMMU_DEFAULT_ON=y
(Ubuntu already does that.)
On the other hand, if there is some deeper issue to fix, then, of
course, I'm glad it is being fixed!

Also, please, add some "Fixes" tag and CC stable to make sure this fix
will be backported to stable kernels.

For example:

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: <stable@vger.kernel.org>

Here I chose the initial git commit, because i915 always was in git history.

-- 
Askar Safin

Re: [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915

[Askar Safin]

[It seems this letter got lost, so I'm sending it again, this time using git-send-email.
Please, CC lkml additionally next time, hopefully it will work better than intel-gfx@freedesktop ]

On Tue, Oct 7, 2025 at 11:25 PM Jia Yao <jia.yao@intel.com> wrote:
> Make i915's PCI device management more robust by always setting/clearing

Thank you! I tested this patch, and it indeed fixes the problem. You may add:

Tested-By: Askar Safin <safinaskar@gmail.com>

Please, CC me with further patches, I will test them.

Also, I will repeat that the bug doesn't happen if we have
CONFIG_INTEL_IOMMU_DEFAULT_ON=y.
So, probably we just need to patch all distro's configs to include
CONFIG_INTEL_IOMMU_DEFAULT_ON=y
(Ubuntu already does that.)
On the other hand, if there is some deeper issue to fix, then, of
course, I'm glad it is being fixed!

Also, please, add some "Fixes" tag and CC stable to make sure this fix
will be backported to stable kernels.

For example:

  Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
  Cc: <stable@vger.kernel.org>

Here I chose the initial git commit, because i915 always was in git history.

-- 
Askar Safin

✓ i915.CI.BAT: success for drm/i915: Setting/clearing the memory access bit when enabling/disabling i915

[Patchwork]

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

== Series Details ==

Series: drm/i915: Setting/clearing the memory access bit when enabling/disabling i915
URL   : https://patchwork.freedesktop.org/series/155554/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_17323 -> Patchwork_155554v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v1/index.html

Participating hosts (44 -> 42)
------------------------------

  Missing    (2): fi-glk-j4005 fi-snb-2520m 

New tests
---------

  New tests have been introduced between CI_DRM_17323 and Patchwork_155554v1:

### New IGT tests (2) ###

  * igt@gem_exec_parallel@engines@userptr:
    - Statuses : 41 pass(s)
    - Exec time: [1.29, 6.48] s

  * igt@i915_selftest@live@guc:
    - Statuses : 41 pass(s)
    - Exec time: [1.06, 7.90] s

  

Known issues
------------

  Here are the changes found in Patchwork_155554v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@dmabuf@all-tests:
    - bat-apl-1:          [PASS][1] -> [ABORT][2] ([i915#12904]) +1 other test abort
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17323/bat-apl-1/igt@dmabuf@all-tests.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v1/bat-apl-1/igt@dmabuf@all-tests.html

  * igt@i915_selftest@live@workarounds:
    - bat-arls-5:         [PASS][3] -> [DMESG-FAIL][4] ([i915#12061]) +1 other test dmesg-fail
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17323/bat-arls-5/igt@i915_selftest@live@workarounds.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v1/bat-arls-5/igt@i915_selftest@live@workarounds.html

  
#### Possible fixes ####

  * igt@i915_selftest@live:
    - fi-bsw-n3050:       [DMESG-FAIL][5] ([i915#14808]) -> [PASS][6] +1 other test pass
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17323/fi-bsw-n3050/igt@i915_selftest@live.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v1/fi-bsw-n3050/igt@i915_selftest@live.html

  * igt@i915_selftest@live@workarounds:
    - bat-arlh-3:         [DMESG-FAIL][7] ([i915#12061]) -> [PASS][8] +1 other test pass
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17323/bat-arlh-3/igt@i915_selftest@live@workarounds.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v1/bat-arlh-3/igt@i915_selftest@live@workarounds.html

  
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#12904]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12904
  [i915#14808]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14808


Build changes
-------------

  * Linux: CI_DRM_17323 -> Patchwork_155554v1

  CI-20190529: 20190529
  CI_DRM_17323: 90c02675d5fedea7f1d28c4e5c45f958de9bfee4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8575: 7efc313da1339df43a1b11bba57fef6c1257e65f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_155554v1: 90c02675d5fedea7f1d28c4e5c45f958de9bfee4 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v1/index.html

[-- Attachment #2: Type: text/html, Size: 4042 bytes --]

✓ i915.CI.BAT: success for drm/i915: Setting/clearing the memory access bit when enabling/disabling i915 (rev2)

[Patchwork]

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

== Series Details ==

Series: drm/i915: Setting/clearing the memory access bit when enabling/disabling i915 (rev2)
URL   : https://patchwork.freedesktop.org/series/155554/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_17324 -> Patchwork_155554v2
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v2/index.html

Participating hosts (44 -> 43)
------------------------------

  Missing    (1): fi-snb-2520m 

Known issues
------------

  Here are the changes found in Patchwork_155554v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@dmabuf@all-tests:
    - bat-apl-1:          [PASS][1] -> [ABORT][2] ([i915#12904]) +1 other test abort
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17324/bat-apl-1/igt@dmabuf@all-tests.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v2/bat-apl-1/igt@dmabuf@all-tests.html

  * igt@i915_selftest@live@workarounds:
    - bat-arls-5:         [PASS][3] -> [DMESG-FAIL][4] ([i915#12061]) +1 other test dmesg-fail
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17324/bat-arls-5/igt@i915_selftest@live@workarounds.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v2/bat-arls-5/igt@i915_selftest@live@workarounds.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@workarounds:
    - bat-arls-6:         [DMESG-FAIL][5] ([i915#12061]) -> [PASS][6] +1 other test pass
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17324/bat-arls-6/igt@i915_selftest@live@workarounds.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v2/bat-arls-6/igt@i915_selftest@live@workarounds.html

  
#### Warnings ####

  * igt@i915_selftest@live:
    - bat-atsm-1:         [DMESG-FAIL][7] ([i915#12061] / [i915#14204]) -> [DMESG-FAIL][8] ([i915#12061] / [i915#13929])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17324/bat-atsm-1/igt@i915_selftest@live.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v2/bat-atsm-1/igt@i915_selftest@live.html

  * igt@i915_selftest@live@mman:
    - bat-atsm-1:         [DMESG-FAIL][9] ([i915#14204]) -> [DMESG-FAIL][10] ([i915#13929])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17324/bat-atsm-1/igt@i915_selftest@live@mman.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v2/bat-atsm-1/igt@i915_selftest@live@mman.html

  
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#12904]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12904
  [i915#13929]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13929
  [i915#14204]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14204


Build changes
-------------

  * Linux: CI_DRM_17324 -> Patchwork_155554v2

  CI-20190529: 20190529
  CI_DRM_17324: 679a7939422e7585cce7bf5d3350fc522e765343 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8575: 7efc313da1339df43a1b11bba57fef6c1257e65f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_155554v2: 679a7939422e7585cce7bf5d3350fc522e765343 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v2/index.html

[-- Attachment #2: Type: text/html, Size: 4432 bytes --]

[PATCH v3] drm/i915: Clearing the Memory Space Enable bit when disabling i915

[Jia Yao]

In a kexec reboot scenario, the GPU's Global Graphics Translation Table
(GGTT) retains its previous state after the kernel is reloaded, until i915
reinitializes the GGTT.

The simple-framebuffer driver is initialized before i915 and accesses the
PCIe memory space (GPU aperture) through outdated GGTT entries. This leads
to invalid physical memory accesses, causing GPF or data corruption.

To prevent such issues, the Memory Space Enable (MSE) bit in the PCI Command
Register is cleared during i915_driver_shutdown. This disables all PCIe
memory space access (including MMIO and aperture) at the hardware level.
After the kernel is reloaded, access to the PCIe memory space will be
forbidden until i915 is re-initialized.

Since disabling PCIe memory space affects all MMIO operations, PXP shutdown
needs to be completed before this point. Calls intel_pxp_fini() before
disabling memory space to ensure PXP cleanup can still access MMIO registers.

v2:
  - follow brace style

v3:
  - revise description

Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
Cc: Alex Zuo <alex.zuo@intel.com>
Cc: Shuicheng Lin <shuicheng.lin@intel.com>
Cc: Askar Safin <safinaskar@gmail.com>
Cc: Pingfan Liu <piliu@redhat.com>
Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
Signed-off-by: Jia Yao <jia.yao@intel.com>
---
 drivers/gpu/drm/i915/i915_driver.c | 35 +++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index b46cb54ef5dc..766f85726b67 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -118,6 +118,33 @@
 
 static const struct drm_driver i915_drm_driver;
 
+static int i915_enable_device(struct pci_dev *pdev)
+{
+	u32 cmd;
+	int ret;
+
+	ret = pci_enable_device(pdev);
+	if (ret)
+		return ret;
+
+	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
+	if (!(cmd & PCI_COMMAND_MEMORY))
+		pci_write_config_dword(pdev, PCI_COMMAND, cmd | PCI_COMMAND_MEMORY);
+
+	return 0;
+}
+
+static void i915_disable_device(struct pci_dev *pdev)
+{
+	u32 cmd;
+
+	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
+	if (cmd & PCI_COMMAND_MEMORY)
+		pci_write_config_dword(pdev, PCI_COMMAND, cmd & ~PCI_COMMAND_MEMORY);
+
+	pci_disable_device(pdev);
+}
+
 static int i915_workqueues_init(struct drm_i915_private *dev_priv)
 {
 	/*
@@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	struct intel_display *display;
 	int ret;
 
-	ret = pci_enable_device(pdev);
+	ret = i915_enable_device(pdev);
 	if (ret) {
 		pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
 		return ret;
@@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	i915 = i915_driver_create(pdev, ent);
 	if (IS_ERR(i915)) {
-		pci_disable_device(pdev);
+		i915_disable_device(pdev);
 		return PTR_ERR(i915);
 	}
 
@@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	enable_rpm_wakeref_asserts(&i915->runtime_pm);
 	i915_driver_late_release(i915);
 out_pci_disable:
-	pci_disable_device(pdev);
+	i915_disable_device(pdev);
 	i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
 	return ret;
 }
@@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
 
 	intel_dmc_suspend(display);
 
+	intel_pxp_fini(i915);
 	i915_gem_suspend(i915);
 
 	/*
@@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
 	enable_rpm_wakeref_asserts(&i915->runtime_pm);
 
 	intel_runtime_pm_driver_last_release(&i915->runtime_pm);
+	i915_disable_device(to_pci_dev(i915->drm.dev));
 }
 
 static bool suspend_to_idle(struct drm_i915_private *dev_priv)
-- 
2.34.1

Re: [PATCH v3] drm/i915: Clearing the Memory Space Enable bit when disabling i915

[Jani Nikula]

On Tue, 20 Jan 2026, Jia Yao <jia.yao@intel.com> wrote:
> In a kexec reboot scenario, the GPU's Global Graphics Translation Table
> (GGTT) retains its previous state after the kernel is reloaded, until i915
> reinitializes the GGTT.
>
> The simple-framebuffer driver is initialized before i915 and accesses the
> PCIe memory space (GPU aperture) through outdated GGTT entries. This leads
> to invalid physical memory accesses, causing GPF or data corruption.
>
> To prevent such issues, the Memory Space Enable (MSE) bit in the PCI Command
> Register is cleared during i915_driver_shutdown. This disables all PCIe
> memory space access (including MMIO and aperture) at the hardware level.
> After the kernel is reloaded, access to the PCIe memory space will be
> forbidden until i915 is re-initialized.
>
> Since disabling PCIe memory space affects all MMIO operations, PXP shutdown
> needs to be completed before this point. Calls intel_pxp_fini() before
> disabling memory space to ensure PXP cleanup can still access MMIO registers.
>
> v2:
>   - follow brace style
>
> v3:
>   - revise description
>
> Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> Cc: Alex Zuo <alex.zuo@intel.com>
> Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> Cc: Askar Safin <safinaskar@gmail.com>
> Cc: Pingfan Liu <piliu@redhat.com>
> Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> Signed-off-by: Jia Yao <jia.yao@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_driver.c | 35 +++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index b46cb54ef5dc..766f85726b67 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -118,6 +118,33 @@
>  
>  static const struct drm_driver i915_drm_driver;
>  
> +static int i915_enable_device(struct pci_dev *pdev)
> +{
> +	u32 cmd;
> +	int ret;
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret)
> +		return ret;
> +
> +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +	if (!(cmd & PCI_COMMAND_MEMORY))
> +		pci_write_config_dword(pdev, PCI_COMMAND, cmd | PCI_COMMAND_MEMORY);
> +
> +	return 0;
> +}
> +
> +static void i915_disable_device(struct pci_dev *pdev)
> +{
> +	u32 cmd;
> +
> +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +	if (cmd & PCI_COMMAND_MEMORY)
> +		pci_write_config_dword(pdev, PCI_COMMAND, cmd & ~PCI_COMMAND_MEMORY);
> +
> +	pci_disable_device(pdev);
> +}
> +
>  static int i915_workqueues_init(struct drm_i915_private *dev_priv)
>  {
>  	/*
> @@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	struct intel_display *display;
>  	int ret;
>  
> -	ret = pci_enable_device(pdev);
> +	ret = i915_enable_device(pdev);
>  	if (ret) {
>  		pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
>  		return ret;
> @@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	i915 = i915_driver_create(pdev, ent);
>  	if (IS_ERR(i915)) {
> -		pci_disable_device(pdev);
> +		i915_disable_device(pdev);
>  		return PTR_ERR(i915);
>  	}
>  
> @@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
>  	i915_driver_late_release(i915);
>  out_pci_disable:
> -	pci_disable_device(pdev);
> +	i915_disable_device(pdev);
>  	i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
>  	return ret;
>  }
> @@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>  
>  	intel_dmc_suspend(display);
>  
> +	intel_pxp_fini(i915);

This is now called twice. Please fix it.

>  	i915_gem_suspend(i915);
>  
>  	/*
> @@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
>  
>  	intel_runtime_pm_driver_last_release(&i915->runtime_pm);
> +	i915_disable_device(to_pci_dev(i915->drm.dev));
>  }
>  
>  static bool suspend_to_idle(struct drm_i915_private *dev_priv)

-- 
Jani Nikula, Intel

RE: [PATCH v3] drm/i915: Clearing the Memory Space Enable bit when disabling i915

[Yao, Jia]

Within the i915 driver, intel_pxp_fini is called through different paths.

i915_pci_shutdown → i915_driver_shutdown → intel_pxp_fini  (new added)
i915_pci_remove → i915_driver_remove → i915_driver_unregister → intel_pxp_fini

For MEI-based PXP,  which is mentioned in the commit message.
> Since disabling PCIe memory space affects all MMIO operations, PXP 
> shutdown needs to be completed before this point. Calls 
> intel_pxp_fini() before disabling memory space to ensure PXP cleanup can still access MMIO registers.

From i915

Kernel_kexec
	-> kernel_restart_prepare
		-> pci_device_shutdown
			-> i915_driver_shutdown
				->  intel_pxp_fini
					-> intel_pxp_tee_component_fini                // will delete component

From mei

Kernel_kexec
	-> kernel_restart_prepare
		-> pci_device_shutdown
			-> mei_me_shutdown
				-> mei_stop
					-> mei_cl_bus_remove_device
						-> device_release_driver
							-> device_release_driver_internal
								-> mei_cl_device_remove
									-> mei_pxp_remove
										-> component_master_del
											->take_down_aggregate_device
												-> mei_component_master_unbind
													->component_unbind_all
														->component_unbind                    // will not be called
															->i915_pxp_tee_component_unbind
																-> intel_pxp_fini_hw


After intel_pxp_tee_component_fini is called,  for mei_me_shutdown stack,  component_unbind will not be called for component has been deleted.

-----Original Message-----
From: Jani Nikula <jani.nikula@linux.intel.com> 
Sent: Tuesday, January 20, 2026 1:51 AM
To: Yao, Jia <jia.yao@intel.com>; intel-gfx@lists.freedesktop.org
Cc: Yao, Jia <jia.yao@intel.com>; Zuo, Alex <alex.zuo@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson <chris.p.wilson@linux.intel.com>
Subject: Re: [PATCH v3] drm/i915: Clearing the Memory Space Enable bit when disabling i915

On Tue, 20 Jan 2026, Jia Yao <jia.yao@intel.com> wrote:
> In a kexec reboot scenario, the GPU's Global Graphics Translation 
> Table
> (GGTT) retains its previous state after the kernel is reloaded, until 
> i915 reinitializes the GGTT.
>
> The simple-framebuffer driver is initialized before i915 and accesses 
> the PCIe memory space (GPU aperture) through outdated GGTT entries. 
> This leads to invalid physical memory accesses, causing GPF or data corruption.
>
> To prevent such issues, the Memory Space Enable (MSE) bit in the PCI 
> Command Register is cleared during i915_driver_shutdown. This disables 
> all PCIe memory space access (including MMIO and aperture) at the hardware level.
> After the kernel is reloaded, access to the PCIe memory space will be 
> forbidden until i915 is re-initialized.
>
> Since disabling PCIe memory space affects all MMIO operations, PXP 
> shutdown needs to be completed before this point. Calls 
> intel_pxp_fini() before disabling memory space to ensure PXP cleanup can still access MMIO registers.
>
> v2:
>   - follow brace style
>
> v3:
>   - revise description
>
> Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> Cc: Alex Zuo <alex.zuo@intel.com>
> Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> Cc: Askar Safin <safinaskar@gmail.com>
> Cc: Pingfan Liu <piliu@redhat.com>
> Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> Signed-off-by: Jia Yao <jia.yao@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_driver.c | 35 
> +++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_driver.c 
> b/drivers/gpu/drm/i915/i915_driver.c
> index b46cb54ef5dc..766f85726b67 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -118,6 +118,33 @@
>  
>  static const struct drm_driver i915_drm_driver;
>  
> +static int i915_enable_device(struct pci_dev *pdev) {
> +	u32 cmd;
> +	int ret;
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret)
> +		return ret;
> +
> +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +	if (!(cmd & PCI_COMMAND_MEMORY))
> +		pci_write_config_dword(pdev, PCI_COMMAND, cmd | 
> +PCI_COMMAND_MEMORY);
> +
> +	return 0;
> +}
> +
> +static void i915_disable_device(struct pci_dev *pdev) {
> +	u32 cmd;
> +
> +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +	if (cmd & PCI_COMMAND_MEMORY)
> +		pci_write_config_dword(pdev, PCI_COMMAND, cmd & 
> +~PCI_COMMAND_MEMORY);
> +
> +	pci_disable_device(pdev);
> +}
> +
>  static int i915_workqueues_init(struct drm_i915_private *dev_priv)  {
>  	/*
> @@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	struct intel_display *display;
>  	int ret;
>  
> -	ret = pci_enable_device(pdev);
> +	ret = i915_enable_device(pdev);
>  	if (ret) {
>  		pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
>  		return ret;
> @@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, const 
> struct pci_device_id *ent)
>  
>  	i915 = i915_driver_create(pdev, ent);
>  	if (IS_ERR(i915)) {
> -		pci_disable_device(pdev);
> +		i915_disable_device(pdev);
>  		return PTR_ERR(i915);
>  	}
>  
> @@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
>  	i915_driver_late_release(i915);
>  out_pci_disable:
> -	pci_disable_device(pdev);
> +	i915_disable_device(pdev);
>  	i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
>  	return ret;
>  }
> @@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct 
> drm_i915_private *i915)
>  
>  	intel_dmc_suspend(display);
>  
> +	intel_pxp_fini(i915);

This is now called twice. Please fix it.

>  	i915_gem_suspend(i915);
>  
>  	/*
> @@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
>  
>  	intel_runtime_pm_driver_last_release(&i915->runtime_pm);
> +	i915_disable_device(to_pci_dev(i915->drm.dev));
>  }
>  
>  static bool suspend_to_idle(struct drm_i915_private *dev_priv)

--
Jani Nikula, Intel

Re: [PATCH v3] drm/i915: Clearing the Memory Space Enable bit when disabling i915

[Ville Syrjälä]

On Tue, Jan 20, 2026 at 04:42:03AM +0000, Jia Yao wrote:
> In a kexec reboot scenario, the GPU's Global Graphics Translation Table
> (GGTT) retains its previous state after the kernel is reloaded, until i915
> reinitializes the GGTT.
> 
> The simple-framebuffer driver is initialized before i915 and accesses the
> PCIe memory space (GPU aperture) through outdated GGTT entries. This leads
> to invalid physical memory accesses, causing GPF or data corruption.
> 
> To prevent such issues, the Memory Space Enable (MSE) bit in the PCI Command
> Register is cleared during i915_driver_shutdown. This disables all PCIe
> memory space access (including MMIO and aperture) at the hardware level.
> After the kernel is reloaded, access to the PCIe memory space will be
> forbidden until i915 is re-initialized.

Still looks like a hack. I think the correct fix would involve
preventing the kexec'd kernel from initializing the fb driver that
is doing the invalid memory accesses.

> 
> Since disabling PCIe memory space affects all MMIO operations, PXP shutdown
> needs to be completed before this point. Calls intel_pxp_fini() before
> disabling memory space to ensure PXP cleanup can still access MMIO registers.
> 
> v2:
>   - follow brace style
> 
> v3:
>   - revise description
> 
> Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> Cc: Alex Zuo <alex.zuo@intel.com>
> Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> Cc: Askar Safin <safinaskar@gmail.com>
> Cc: Pingfan Liu <piliu@redhat.com>
> Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> Signed-off-by: Jia Yao <jia.yao@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_driver.c | 35 +++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index b46cb54ef5dc..766f85726b67 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -118,6 +118,33 @@
>  
>  static const struct drm_driver i915_drm_driver;
>  
> +static int i915_enable_device(struct pci_dev *pdev)
> +{
> +	u32 cmd;
> +	int ret;
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret)
> +		return ret;
> +
> +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +	if (!(cmd & PCI_COMMAND_MEMORY))
> +		pci_write_config_dword(pdev, PCI_COMMAND, cmd | PCI_COMMAND_MEMORY);
> +
> +	return 0;
> +}
> +
> +static void i915_disable_device(struct pci_dev *pdev)
> +{
> +	u32 cmd;
> +
> +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +	if (cmd & PCI_COMMAND_MEMORY)
> +		pci_write_config_dword(pdev, PCI_COMMAND, cmd & ~PCI_COMMAND_MEMORY);
> +
> +	pci_disable_device(pdev);
> +}
> +
>  static int i915_workqueues_init(struct drm_i915_private *dev_priv)
>  {
>  	/*
> @@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	struct intel_display *display;
>  	int ret;
>  
> -	ret = pci_enable_device(pdev);
> +	ret = i915_enable_device(pdev);
>  	if (ret) {
>  		pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
>  		return ret;
> @@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	i915 = i915_driver_create(pdev, ent);
>  	if (IS_ERR(i915)) {
> -		pci_disable_device(pdev);
> +		i915_disable_device(pdev);
>  		return PTR_ERR(i915);
>  	}
>  
> @@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
>  	i915_driver_late_release(i915);
>  out_pci_disable:
> -	pci_disable_device(pdev);
> +	i915_disable_device(pdev);
>  	i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
>  	return ret;
>  }
> @@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>  
>  	intel_dmc_suspend(display);
>  
> +	intel_pxp_fini(i915);
>  	i915_gem_suspend(i915);
>  
>  	/*
> @@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
>  
>  	intel_runtime_pm_driver_last_release(&i915->runtime_pm);
> +	i915_disable_device(to_pci_dev(i915->drm.dev));
>  }
>  
>  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel

RE: [PATCH v3] drm/i915: Clearing the Memory Space Enable bit when disabling i915

[Yao, Jia]

The problem is that the fb driver issues is doing valid access, but because of an incorrect GGTT translation  (Once GGTT has been initialized by i915,  it can't be restored to hardware initial status without reboot), these accesses are mapped to incorrect addresses.
As for blocking fb driver,  Some Linux distributions compile CONFIG_DRM_SIMPLEDRM=y into the kernel as a built‑in option rather than a loadable module.
Considering these factors, this remains the only feasible approach.

-----Original Message-----
From: Ville Syrjälä <ville.syrjala@linux.intel.com> 
Sent: Tuesday, January 20, 2026 8:11 AM
To: Yao, Jia <jia.yao@intel.com>
Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson <chris.p.wilson@linux.intel.com>
Subject: Re: [PATCH v3] drm/i915: Clearing the Memory Space Enable bit when disabling i915

On Tue, Jan 20, 2026 at 04:42:03AM +0000, Jia Yao wrote:
> In a kexec reboot scenario, the GPU's Global Graphics Translation 
> Table
> (GGTT) retains its previous state after the kernel is reloaded, until 
> i915 reinitializes the GGTT.
> 
> The simple-framebuffer driver is initialized before i915 and accesses 
> the PCIe memory space (GPU aperture) through outdated GGTT entries. 
> This leads to invalid physical memory accesses, causing GPF or data corruption.
> 
> To prevent such issues, the Memory Space Enable (MSE) bit in the PCI 
> Command Register is cleared during i915_driver_shutdown. This disables 
> all PCIe memory space access (including MMIO and aperture) at the hardware level.
> After the kernel is reloaded, access to the PCIe memory space will be 
> forbidden until i915 is re-initialized.

Still looks like a hack. I think the correct fix would involve preventing the kexec'd kernel from initializing the fb driver that is doing the invalid memory accesses.

> 
> Since disabling PCIe memory space affects all MMIO operations, PXP 
> shutdown needs to be completed before this point. Calls 
> intel_pxp_fini() before disabling memory space to ensure PXP cleanup can still access MMIO registers.
> 
> v2:
>   - follow brace style
> 
> v3:
>   - revise description
> 
> Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> Cc: Alex Zuo <alex.zuo@intel.com>
> Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> Cc: Askar Safin <safinaskar@gmail.com>
> Cc: Pingfan Liu <piliu@redhat.com>
> Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> Signed-off-by: Jia Yao <jia.yao@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_driver.c | 35 
> +++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_driver.c 
> b/drivers/gpu/drm/i915/i915_driver.c
> index b46cb54ef5dc..766f85726b67 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -118,6 +118,33 @@
>  
>  static const struct drm_driver i915_drm_driver;
>  
> +static int i915_enable_device(struct pci_dev *pdev) {
> +	u32 cmd;
> +	int ret;
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret)
> +		return ret;
> +
> +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +	if (!(cmd & PCI_COMMAND_MEMORY))
> +		pci_write_config_dword(pdev, PCI_COMMAND, cmd | 
> +PCI_COMMAND_MEMORY);
> +
> +	return 0;
> +}
> +
> +static void i915_disable_device(struct pci_dev *pdev) {
> +	u32 cmd;
> +
> +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +	if (cmd & PCI_COMMAND_MEMORY)
> +		pci_write_config_dword(pdev, PCI_COMMAND, cmd & 
> +~PCI_COMMAND_MEMORY);
> +
> +	pci_disable_device(pdev);
> +}
> +
>  static int i915_workqueues_init(struct drm_i915_private *dev_priv)  {
>  	/*
> @@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	struct intel_display *display;
>  	int ret;
>  
> -	ret = pci_enable_device(pdev);
> +	ret = i915_enable_device(pdev);
>  	if (ret) {
>  		pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
>  		return ret;
> @@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, const 
> struct pci_device_id *ent)
>  
>  	i915 = i915_driver_create(pdev, ent);
>  	if (IS_ERR(i915)) {
> -		pci_disable_device(pdev);
> +		i915_disable_device(pdev);
>  		return PTR_ERR(i915);
>  	}
>  
> @@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
>  	i915_driver_late_release(i915);
>  out_pci_disable:
> -	pci_disable_device(pdev);
> +	i915_disable_device(pdev);
>  	i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
>  	return ret;
>  }
> @@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct 
> drm_i915_private *i915)
>  
>  	intel_dmc_suspend(display);
>  
> +	intel_pxp_fini(i915);
>  	i915_gem_suspend(i915);
>  
>  	/*
> @@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
>  
>  	intel_runtime_pm_driver_last_release(&i915->runtime_pm);
> +	i915_disable_device(to_pci_dev(i915->drm.dev));
>  }
>  
>  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
> --
> 2.34.1

--
Ville Syrjälä
Intel

Re: [PATCH v3] drm/i915: Clearing the Memory Space Enable bit when disabling i915

[Ville Syrjälä]

On Wed, Jan 21, 2026 at 07:19:03AM +0000, Yao, Jia wrote:
> The problem is that the fb driver issues is doing valid access, but because of an incorrect GGTT translation  (Once GGTT has been initialized by i915,  it can't be restored to hardware initial status without reboot),

It will never be restored to that state.

> these accesses are mapped to incorrect addresses.
> As for blocking fb driver,  Some Linux distributions compile CONFIG_DRM_SIMPLEDRM=y into the kernel as a built‑in option rather than a loadable module.
> Considering these factors, this remains the only feasible approach.

Whatever information is passed to the kexec'd kernel to inform it
about the firmware framebuffer should be zapped so that it knows
not to do anything anything with it.

> 
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com> 
> Sent: Tuesday, January 20, 2026 8:11 AM
> To: Yao, Jia <jia.yao@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson <chris.p.wilson@linux.intel.com>
> Subject: Re: [PATCH v3] drm/i915: Clearing the Memory Space Enable bit when disabling i915
> 
> On Tue, Jan 20, 2026 at 04:42:03AM +0000, Jia Yao wrote:
> > In a kexec reboot scenario, the GPU's Global Graphics Translation 
> > Table
> > (GGTT) retains its previous state after the kernel is reloaded, until 
> > i915 reinitializes the GGTT.
> > 
> > The simple-framebuffer driver is initialized before i915 and accesses 
> > the PCIe memory space (GPU aperture) through outdated GGTT entries. 
> > This leads to invalid physical memory accesses, causing GPF or data corruption.
> > 
> > To prevent such issues, the Memory Space Enable (MSE) bit in the PCI 
> > Command Register is cleared during i915_driver_shutdown. This disables 
> > all PCIe memory space access (including MMIO and aperture) at the hardware level.
> > After the kernel is reloaded, access to the PCIe memory space will be 
> > forbidden until i915 is re-initialized.
> 
> Still looks like a hack. I think the correct fix would involve preventing the kexec'd kernel from initializing the fb driver that is doing the invalid memory accesses.
> 
> > 
> > Since disabling PCIe memory space affects all MMIO operations, PXP 
> > shutdown needs to be completed before this point. Calls 
> > intel_pxp_fini() before disabling memory space to ensure PXP cleanup can still access MMIO registers.
> > 
> > v2:
> >   - follow brace style
> > 
> > v3:
> >   - revise description
> > 
> > Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> > Cc: Alex Zuo <alex.zuo@intel.com>
> > Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> > Cc: Askar Safin <safinaskar@gmail.com>
> > Cc: Pingfan Liu <piliu@redhat.com>
> > Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> > Signed-off-by: Jia Yao <jia.yao@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_driver.c | 35 
> > +++++++++++++++++++++++++++---
> >  1 file changed, 32 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_driver.c 
> > b/drivers/gpu/drm/i915/i915_driver.c
> > index b46cb54ef5dc..766f85726b67 100644
> > --- a/drivers/gpu/drm/i915/i915_driver.c
> > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > @@ -118,6 +118,33 @@
> >  
> >  static const struct drm_driver i915_drm_driver;
> >  
> > +static int i915_enable_device(struct pci_dev *pdev) {
> > +	u32 cmd;
> > +	int ret;
> > +
> > +	ret = pci_enable_device(pdev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> > +	if (!(cmd & PCI_COMMAND_MEMORY))
> > +		pci_write_config_dword(pdev, PCI_COMMAND, cmd | 
> > +PCI_COMMAND_MEMORY);
> > +
> > +	return 0;
> > +}
> > +
> > +static void i915_disable_device(struct pci_dev *pdev) {
> > +	u32 cmd;
> > +
> > +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> > +	if (cmd & PCI_COMMAND_MEMORY)
> > +		pci_write_config_dword(pdev, PCI_COMMAND, cmd & 
> > +~PCI_COMMAND_MEMORY);
> > +
> > +	pci_disable_device(pdev);
> > +}
> > +
> >  static int i915_workqueues_init(struct drm_i915_private *dev_priv)  {
> >  	/*
> > @@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  	struct intel_display *display;
> >  	int ret;
> >  
> > -	ret = pci_enable_device(pdev);
> > +	ret = i915_enable_device(pdev);
> >  	if (ret) {
> >  		pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
> >  		return ret;
> > @@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, const 
> > struct pci_device_id *ent)
> >  
> >  	i915 = i915_driver_create(pdev, ent);
> >  	if (IS_ERR(i915)) {
> > -		pci_disable_device(pdev);
> > +		i915_disable_device(pdev);
> >  		return PTR_ERR(i915);
> >  	}
> >  
> > @@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
> >  	i915_driver_late_release(i915);
> >  out_pci_disable:
> > -	pci_disable_device(pdev);
> > +	i915_disable_device(pdev);
> >  	i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
> >  	return ret;
> >  }
> > @@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct 
> > drm_i915_private *i915)
> >  
> >  	intel_dmc_suspend(display);
> >  
> > +	intel_pxp_fini(i915);
> >  	i915_gem_suspend(i915);
> >  
> >  	/*
> > @@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
> >  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
> >  
> >  	intel_runtime_pm_driver_last_release(&i915->runtime_pm);
> > +	i915_disable_device(to_pci_dev(i915->drm.dev));
> >  }
> >  
> >  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
> > --
> > 2.34.1
> 
> --
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel

RE: [PATCH v3] drm/i915: Clearing the Memory Space Enable bit when disabling i915

[Yao, Jia]

> Whatever information is passed to the kexec'd kernel to inform it about the firmware framebuffer should be zapped so that it knows not to do anything anything with it.

Thanks Ville Syrjälä,
Your idea enlightened me;  I do find a more graceful way to fix this issue.  Do not write EFI framebuffer info into screen_info 
Initial testing indicates that it works. I'll clean it up and send a new patch out.

-----Original Message-----
From: Ville Syrjälä <ville.syrjala@linux.intel.com> 
Sent: Wednesday, January 21, 2026 7:02 AM
To: Yao, Jia <jia.yao@intel.com>
Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson <chris.p.wilson@linux.intel.com>
Subject: Re: [PATCH v3] drm/i915: Clearing the Memory Space Enable bit when disabling i915

On Wed, Jan 21, 2026 at 07:19:03AM +0000, Yao, Jia wrote:
> The problem is that the fb driver issues is doing valid access, but 
> because of an incorrect GGTT translation  (Once GGTT has been 
> initialized by i915,  it can't be restored to hardware initial status 
> without reboot),

It will never be restored to that state.

> these accesses are mapped to incorrect addresses.
> As for blocking fb driver,  Some Linux distributions compile CONFIG_DRM_SIMPLEDRM=y into the kernel as a built‑in option rather than a loadable module.
> Considering these factors, this remains the only feasible approach.

Whatever information is passed to the kexec'd kernel to inform it about the firmware framebuffer should be zapped so that it knows not to do anything anything with it.

> 
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Tuesday, January 20, 2026 8:11 AM
> To: Yao, Jia <jia.yao@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Zuo, Alex <alex.zuo@intel.com>; 
> Lin, Shuicheng <shuicheng.lin@intel.com>; Askar Safin 
> <safinaskar@gmail.com>; Pingfan Liu <piliu@redhat.com>; Chris Wilson 
> <chris.p.wilson@linux.intel.com>
> Subject: Re: [PATCH v3] drm/i915: Clearing the Memory Space Enable bit 
> when disabling i915
> 
> On Tue, Jan 20, 2026 at 04:42:03AM +0000, Jia Yao wrote:
> > In a kexec reboot scenario, the GPU's Global Graphics Translation 
> > Table
> > (GGTT) retains its previous state after the kernel is reloaded, 
> > until
> > i915 reinitializes the GGTT.
> > 
> > The simple-framebuffer driver is initialized before i915 and 
> > accesses the PCIe memory space (GPU aperture) through outdated GGTT entries.
> > This leads to invalid physical memory accesses, causing GPF or data corruption.
> > 
> > To prevent such issues, the Memory Space Enable (MSE) bit in the PCI 
> > Command Register is cleared during i915_driver_shutdown. This 
> > disables all PCIe memory space access (including MMIO and aperture) at the hardware level.
> > After the kernel is reloaded, access to the PCIe memory space will 
> > be forbidden until i915 is re-initialized.
> 
> Still looks like a hack. I think the correct fix would involve preventing the kexec'd kernel from initializing the fb driver that is doing the invalid memory accesses.
> 
> > 
> > Since disabling PCIe memory space affects all MMIO operations, PXP 
> > shutdown needs to be completed before this point. Calls
> > intel_pxp_fini() before disabling memory space to ensure PXP cleanup can still access MMIO registers.
> > 
> > v2:
> >   - follow brace style
> > 
> > v3:
> >   - revise description
> > 
> > Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14598
> > Cc: Alex Zuo <alex.zuo@intel.com>
> > Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> > Cc: Askar Safin <safinaskar@gmail.com>
> > Cc: Pingfan Liu <piliu@redhat.com>
> > Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> > Signed-off-by: Jia Yao <jia.yao@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_driver.c | 35
> > +++++++++++++++++++++++++++---
> >  1 file changed, 32 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_driver.c
> > b/drivers/gpu/drm/i915/i915_driver.c
> > index b46cb54ef5dc..766f85726b67 100644
> > --- a/drivers/gpu/drm/i915/i915_driver.c
> > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > @@ -118,6 +118,33 @@
> >  
> >  static const struct drm_driver i915_drm_driver;
> >  
> > +static int i915_enable_device(struct pci_dev *pdev) {
> > +	u32 cmd;
> > +	int ret;
> > +
> > +	ret = pci_enable_device(pdev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> > +	if (!(cmd & PCI_COMMAND_MEMORY))
> > +		pci_write_config_dword(pdev, PCI_COMMAND, cmd | 
> > +PCI_COMMAND_MEMORY);
> > +
> > +	return 0;
> > +}
> > +
> > +static void i915_disable_device(struct pci_dev *pdev) {
> > +	u32 cmd;
> > +
> > +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> > +	if (cmd & PCI_COMMAND_MEMORY)
> > +		pci_write_config_dword(pdev, PCI_COMMAND, cmd & 
> > +~PCI_COMMAND_MEMORY);
> > +
> > +	pci_disable_device(pdev);
> > +}
> > +
> >  static int i915_workqueues_init(struct drm_i915_private *dev_priv)  {
> >  	/*
> > @@ -788,7 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  	struct intel_display *display;
> >  	int ret;
> >  
> > -	ret = pci_enable_device(pdev);
> > +	ret = i915_enable_device(pdev);
> >  	if (ret) {
> >  		pr_err("Failed to enable graphics device: %pe\n", ERR_PTR(ret));
> >  		return ret;
> > @@ -796,7 +823,7 @@ int i915_driver_probe(struct pci_dev *pdev, 
> > const struct pci_device_id *ent)
> >  
> >  	i915 = i915_driver_create(pdev, ent);
> >  	if (IS_ERR(i915)) {
> > -		pci_disable_device(pdev);
> > +		i915_disable_device(pdev);
> >  		return PTR_ERR(i915);
> >  	}
> >  
> > @@ -885,7 +912,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
> >  	i915_driver_late_release(i915);
> >  out_pci_disable:
> > -	pci_disable_device(pdev);
> > +	i915_disable_device(pdev);
> >  	i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
> >  	return ret;
> >  }
> > @@ -1003,6 +1030,7 @@ void i915_driver_shutdown(struct 
> > drm_i915_private *i915)
> >  
> >  	intel_dmc_suspend(display);
> >  
> > +	intel_pxp_fini(i915);
> >  	i915_gem_suspend(i915);
> >  
> >  	/*
> > @@ -1020,6 +1048,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
> >  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
> >  
> >  	intel_runtime_pm_driver_last_release(&i915->runtime_pm);
> > +	i915_disable_device(to_pci_dev(i915->drm.dev));
> >  }
> >  
> >  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
> > --
> > 2.34.1
> 
> --
> Ville Syrjälä
> Intel

--
Ville Syrjälä
Intel

✗ i915.CI.BAT: failure for drm/i915: Setting/clearing the memory access bit when enabling/disabling i915 (rev3)

[Patchwork]

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

== Series Details ==

Series: drm/i915: Setting/clearing the memory access bit when enabling/disabling i915 (rev3)
URL   : https://patchwork.freedesktop.org/series/155554/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_17845 -> Patchwork_155554v3
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_155554v3 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_155554v3, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v3/index.html

Participating hosts (42 -> 40)
------------------------------

  Missing    (2): bat-dg2-13 fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_155554v3:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@reload:
    - bat-twl-1:          [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17845/bat-twl-1/igt@i915_module_load@reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v3/bat-twl-1/igt@i915_module_load@reload.html

  
Known issues
------------

  Here are the changes found in Patchwork_155554v3 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live:
    - bat-mtlp-8:         [PASS][3] -> [DMESG-FAIL][4] ([i915#12061]) +1 other test dmesg-fail
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17845/bat-mtlp-8/igt@i915_selftest@live.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v3/bat-mtlp-8/igt@i915_selftest@live.html
    - bat-jsl-1:          [PASS][5] -> [DMESG-FAIL][6] ([i915#15394]) +1 other test dmesg-fail
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17845/bat-jsl-1/igt@i915_selftest@live.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v3/bat-jsl-1/igt@i915_selftest@live.html

  * igt@i915_selftest@live@workarounds:
    - bat-dg2-9:          [PASS][7] -> [DMESG-FAIL][8] ([i915#12061]) +1 other test dmesg-fail
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17845/bat-dg2-9/igt@i915_selftest@live@workarounds.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v3/bat-dg2-9/igt@i915_selftest@live@workarounds.html
    - bat-dg2-14:         [PASS][9] -> [DMESG-FAIL][10] ([i915#12061]) +1 other test dmesg-fail
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17845/bat-dg2-14/igt@i915_selftest@live@workarounds.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v3/bat-dg2-14/igt@i915_selftest@live@workarounds.html
    - bat-arls-6:         [PASS][11] -> [DMESG-FAIL][12] ([i915#12061]) +1 other test dmesg-fail
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17845/bat-arls-6/igt@i915_selftest@live@workarounds.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v3/bat-arls-6/igt@i915_selftest@live@workarounds.html

  
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#15394]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15394


Build changes
-------------

  * Linux: CI_DRM_17845 -> Patchwork_155554v3

  CI-20190529: 20190529
  CI_DRM_17845: d5c98420766570dc019dc42dfbbe5a1061c03085 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8706: 8706
  Patchwork_155554v3: d5c98420766570dc019dc42dfbbe5a1061c03085 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155554v3/index.html

[-- Attachment #2: Type: text/html, Size: 4710 bytes --]