Linux wireless drivers development
 help / color / mirror / Atom feed
From: Alex Williamson <alex@shazbot.org>
To: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
Cc: bhelgaas@google.com, jjohnson@kernel.org, mani@kernel.org,
	linux-pci@vger.kernel.org, linux-wireless@vger.kernel.org,
	ath11k@lists.infradead.org, ath12k@lists.infradead.org,
	mhi@lists.linux.dev, linux-kernel@vger.kernel.org,
	alex@shazbot.org
Subject: Re: [PATCH v8 2/3] PCI: Add device-specific reset for Qualcomm devices
Date: Tue, 9 Jun 2026 11:53:52 -0600	[thread overview]
Message-ID: <20260609115352.32acb6fe@shazbot.org> (raw)
In-Reply-To: <20260609163649.319755-3-jtornosm@redhat.com>

On Tue,  9 Jun 2026 18:36:48 +0200
Jose Ignacio Tornos Martinez <jtornosm@redhat.com> wrote:

> Some Qualcomm PCIe devices (WCN6855/WCN7850 WiFi cards, SDX62/SDX65 modems)
> lack working reset methods for VFIO passthrough scenarios. These devices
> have no FLR capability, advertise NoSoftRst+ (blocking PM reset), and have
> broken bus reset.
> 
> The problem manifests in VFIO passthrough scenarios:
> 
> - WCN6855 WiFi card (17cb:1103): Normal VM operation works fine, including
>   clean shutdown/reboot. However, when the VM terminates uncleanly
>   (crash, force-off), VFIO attempts to reset the device before it can
>   be assigned to another VM. Without a working reset method, the device
>   remains in an undefined state, preventing reuse.
> 
> - WCN7850 WiFi card (17cb:1107): Same behavior as WCN6855.
> 
> - SDX62/SDX65 5G modems (17cb:0308): Never successfully initialize even
>   on first VM assignment without proper reset capability.
> 
> Add device-specific reset entries for these Qualcomm devices using D3cold
> power cycling with automatic D3hot fallback. The implementation uses
> pci_set_power_state(D3cold) which automatically falls back to D3hot on
> platforms without ACPI _PR3 power resources. While not a complete reset
> (BARs preserved), testing shows D3hot transition provides sufficient reset
> for VFIO reuse.
> 
> Extract a shared pci_dev_d3cold_d0_cycle() helper function to avoid code
> duplication between pci_d3cold_reset() (strict _PR3 requirement) and the
> new reset_d3cold_d3hot() device-specific reset (automatic fallback).
> The helper performs only the power cycle; IOMMU handling is done by the
> caller (pci_d3cold_reset() for general method, __pci_dev_specific_reset()
> wrapper for device-specific methods).
> 
> Device-specific reset is position #1 in the reset hierarchy, so these
> Qualcomm devices will use power cycling as their primary reset method,
> with the general d3cold method (position #8) available as a fallback on
> _PR3-capable platforms if users override via sysfs.
> 
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
> ---
> v8: Fix commit message: correct IOMMU handling description. The helper
>     performs only the power cycle; IOMMU is handled by callers (this was
>     v6 fix)
> v7: https://lore.kernel.org/all/20260603105853.326290-3-jtornosm@redhat.com/
> 
>  drivers/pci/pci.c    | 37 +++++++++++++++++++++++++++----------
>  drivers/pci/pci.h    |  1 +
>  drivers/pci/quirks.c | 19 +++++++++++++++++++
>  3 files changed, 47 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 096868f80cd4..f7a7443287fd 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4491,6 +4491,32 @@ static int pci_pm_reset(struct pci_dev *dev, bool probe)
>  	return ret;
>  }
>  
> +/**
> + * pci_dev_d3cold_d0_cycle - Perform D3cold->D0 power cycle
> + * @dev: Device to power cycle
> + *
> + * Common helper to perform D3cold->D0 power cycle for reset methods.
> + * Attempts D3cold transition with automatic fallback to D3hot on platforms
> + * without ACPI _PR3 power resources.
> + *
> + * Caller must handle IOMMU preparation/cleanup if needed.
> + *
> + * Returns 0 on success, negative error code on failure.
> + */
> +int pci_dev_d3cold_d0_cycle(struct pci_dev *dev)
> +{
> +	int ret;
> +
> +	if (dev->current_state != PCI_D0)
> +		return -EINVAL;
> +
> +	ret = pci_set_power_state(dev, PCI_D3cold);
> +	if (ret)
> +		return ret;
> +
> +	return pci_set_power_state(dev, PCI_D0);
> +}
> +
>  /**
>   * pci_d3cold_reset - Put device into D3cold and back to D0 for reset
>   * @dev: PCI device to reset
> @@ -4520,22 +4546,13 @@ static int pci_d3cold_reset(struct pci_dev *dev, bool probe)
>  	if (probe)
>  		return 0;
>  
> -	if (dev->current_state != PCI_D0)
> -		return -EINVAL;
> -
>  	ret = pci_dev_reset_iommu_prepare(dev);
>  	if (ret) {
>  		pci_err(dev, "failed to stop IOMMU for a PCI reset: %d\n", ret);
>  		return ret;
>  	}
>  
> -	ret = pci_set_power_state(dev, PCI_D3cold);
> -	if (ret)
> -		goto done;
> -
> -	ret = pci_set_power_state(dev, PCI_D0);
> -
> -done:
> +	ret = pci_dev_d3cold_d0_cycle(dev);
>  	pci_dev_reset_iommu_done(dev);
>  	return ret;
>  }
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 4a14f88e543a..a9942787de9e 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -234,6 +234,7 @@ void pci_init_reset_methods(struct pci_dev *dev);
>  int pci_bridge_secondary_bus_reset(struct pci_dev *dev);
>  int pci_bus_error_reset(struct pci_dev *dev);
>  int pci_try_reset_bridge(struct pci_dev *bridge);
> +int pci_dev_d3cold_d0_cycle(struct pci_dev *dev);
>  
>  struct pci_cap_saved_data {
>  	u16		cap_nr;
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index e49136ac5dbf..70f3b0f26799 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -4237,6 +4237,22 @@ static int reset_hinic_vf_dev(struct pci_dev *pdev, bool probe)
>  	return 0;
>  }
>  
> +/*
> + * Device-specific reset method via D3cold/D3hot power cycle.
> + *
> + * Some devices lack working FLR, advertise NoSoftRst+ (blocking PM reset),
> + * and have broken bus reset. This function provides device-specific reset via
> + * power cycling, attempting D3cold with automatic fallback to D3hot on platforms
> + * without ACPI _PR3 power resources.
> + */

My only complaint is that this is positioned a bit generically.  If the
reset only achieves D3hot on a device that reports NoSoftRst+, then
this appears as a no-op reset method.  The claim here is instead that
despite reporting NoSoftRst+, the devices using this reset have notably
improved behavior, especially across unexpected resets in device
assignment scenarios.  This leads us to believe that some degree of
reset is achieved, for devices that otherwise have no viable
alternative reset method.

Also, I know from previous discussions that much of the testing was
done with M.2 adapters in slots without _PR3 support, which exercises
the D3hot fallback rather than an actual D3cold power cycle.  Has each
of these devices been tested on a platform where D3cold is actually
achieved through this method?  Thanks,

Alex

> +static int reset_d3cold_d3hot(struct pci_dev *dev, bool probe)
> +{
> +	if (probe)
> +		return 0;
> +
> +	return pci_dev_d3cold_d0_cycle(dev);
> +}
> +
>  static const struct pci_dev_reset_methods pci_dev_reset_methods[] = {
>  	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82599_SFP_VF,
>  		 reset_intel_82599_sfp_virtfn },
> @@ -4252,6 +4268,9 @@ static const struct pci_dev_reset_methods pci_dev_reset_methods[] = {
>  		reset_chelsio_generic_dev },
>  	{ PCI_VENDOR_ID_HUAWEI, PCI_DEVICE_ID_HINIC_VF,
>  		reset_hinic_vf_dev },
> +	{ PCI_VENDOR_ID_QCOM, 0x1103, reset_d3cold_d3hot },  /* WCN6855 */
> +	{ PCI_VENDOR_ID_QCOM, 0x1107, reset_d3cold_d3hot },  /* WCN7850 */
> +	{ PCI_VENDOR_ID_QCOM, 0x0308, reset_d3cold_d3hot },  /* SDX62/SDX65 */
>  	{ 0 }
>  };
>  


  reply	other threads:[~2026-06-09 17:53 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-09 16:36 [PATCH v8 0/3] PCI: Add d3cold and device-specific reset for Qualcomm devices Jose Ignacio Tornos Martinez
2026-06-09 16:36 ` [PATCH v8 1/3] PCI: Add d3cold as general reset method Jose Ignacio Tornos Martinez
2026-06-09 16:36 ` [PATCH v8 2/3] PCI: Add device-specific reset for Qualcomm devices Jose Ignacio Tornos Martinez
2026-06-09 17:53   ` Alex Williamson [this message]
2026-06-09 16:36 ` [PATCH v8 3/3] PCI: Disable broken bus reset on " Jose Ignacio Tornos Martinez

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=20260609115352.32acb6fe@shazbot.org \
    --to=alex@shazbot.org \
    --cc=ath11k@lists.infradead.org \
    --cc=ath12k@lists.infradead.org \
    --cc=bhelgaas@google.com \
    --cc=jjohnson@kernel.org \
    --cc=jtornosm@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=mani@kernel.org \
    --cc=mhi@lists.linux.dev \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox