All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jia Yao <jia.yao@intel.com>
Cc: intel-gfx@lists.freedesktop.org, Alex Zuo <alex.zuo@intel.com>,
	Shuicheng Lin <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
Date: Tue, 20 Jan 2026 18:11:28 +0200	[thread overview]
Message-ID: <aW-pMDuVDR_uIAQx@intel.com> (raw)
In-Reply-To: <20260120044203.2436044-1-jia.yao@intel.com>

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

  parent reply	other threads:[~2026-01-20 16:11 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-07 18:06 [PATCH] drm/i915: Setting/clearing the memory access bit when enabling/disabling i915 Jia Yao
2025-10-07 20:25 ` [PATCH v2] drm/i915: Setting/clearing the memory access bit when en/disabling i915 Jia Yao
2025-10-07 21:25   ` Ville Syrjälä
2025-10-07 21:40     ` Yao, Jia
2025-10-08 12:21       ` Ville Syrjälä
2025-10-08 16:06         ` Yao, Jia
2025-10-08 16:15           ` Ville Syrjälä
2025-10-08 17:14             ` Yao, Jia
2025-10-09  1:55               ` Pingfan Liu
2025-10-11 12:35             ` Askar Safin
2025-10-11 12:49             ` Askar Safin
2025-10-13 16:16               ` Yao, Jia
2025-10-14  6:29               ` Pingfan Liu
2025-11-01 16:02                 ` Askar Safin
2025-10-08  5:17   ` Pingfan Liu
2025-10-08  7:05     ` Yao, Jia
2025-10-08 10:58       ` Pingfan Liu
2025-10-08  8:50   ` Askar Safin
2025-10-09  1:10   ` Askar Safin
2025-10-08  4:06 ` ✓ i915.CI.BAT: success for drm/i915: Setting/clearing the memory access bit when enabling/disabling i915 Patchwork
2025-10-08  4:29 ` ✓ i915.CI.BAT: success for drm/i915: Setting/clearing the memory access bit when enabling/disabling i915 (rev2) Patchwork
2026-01-20  4:42 ` [PATCH v3] drm/i915: Clearing the Memory Space Enable bit when disabling i915 Jia Yao
2026-01-20  9:50   ` Jani Nikula
2026-01-21 21:51     ` Yao, Jia
2026-01-20 16:11   ` Ville Syrjälä [this message]
2026-01-21  7:19     ` Yao, Jia
2026-01-21 15:02       ` Ville Syrjälä
2026-01-22  6:43         ` Yao, Jia
2026-01-20  5:31 ` ✗ i915.CI.BAT: failure for drm/i915: Setting/clearing the memory access bit when enabling/disabling i915 (rev3) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aW-pMDuVDR_uIAQx@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=alex.zuo@intel.com \
    --cc=chris.p.wilson@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jia.yao@intel.com \
    --cc=piliu@redhat.com \
    --cc=safinaskar@gmail.com \
    --cc=shuicheng.lin@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.