linux-kernel - Re: [PATCH] ahci: disable DEVSLP for Intel Valleyview

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 11 Apr 2014 12:14:14 +0200
From:	Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
To:	Jacob Pan <jacob.jun.pan@...ux.intel.com>
Cc:	linux-ide@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
	Tejun Heo <tj@...nel.org>, Alan Cox <alan@...ux.intel.com>,
	Dan Williams <dan.j.williams@...el.com>,
	Kristen Carlson Accardi <kristen.c.accardi@...el.com>,
	Kefeng Wang <kefeng.wang@...aro.org>
Subject: Re: [PATCH] ahci: disable DEVSLP for Intel Valleyview


Hi,

On Thursday, April 10, 2014 02:58:54 AM Jacob Pan wrote:
> On Intel Valleyview SoC, SATA device sleep is not reliable. When
> DEVSLP is attempted on certain SSDs, port_devslp write would fail
> and result in malfunction of AHCI controller. AHCI controller may
> be not shown in PCI enumeration after reset. Complete power source
> removal may be required to recver from this failure. So we blacklist

s/recver/recover/

> this device and override host device reported capabilities such that
> device LPM will only attempt slumber but not devslp.

s/devslp/DEVSLP/

> Signed-off-by: Jacob Pan <jacob.jun.pan@...ux.intel.com>
> Acked-by: Dan Wiilliams <dan.j.williams@...el.com>

s/Wiilliams/Williams/

> ---
>  drivers/ata/ahci.c    | 17 +++++++++++++++++
>  drivers/ata/ahci.h    |  1 +
>  drivers/ata/libahci.c |  7 +++++++
>  3 files changed, 25 insertions(+)
> 
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 5a0bf8e..24684a1 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -1115,6 +1115,19 @@ static bool ahci_broken_online(struct pci_dev *pdev)
>  	return pdev->bus->number == (val >> 8) && pdev->devfn == (val & 0xff);
>  }
>  
> +#define PCI_DEVICE_ID_INTEL_VLV_SATA   0x0f23
> +static bool ahci_broken_devslp(struct pci_dev *pdev)
> +{
> +	/* device with broken DEVSLP but still showing SDS capability */
> +	static const struct pci_device_id ids[] = {
> +		{ PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> +				PCI_DEVICE_ID_INTEL_VLV_SATA) },

I think that you can use PCI_VDEVICE() macro instead.

> +		{}
> +	};
> +
> +	return pci_match_id(ids, pdev);
> +}
> +
>  #ifdef CONFIG_ATA_ACPI
>  static void ahci_gtf_filter_workaround(struct ata_host *host)
>  {
> @@ -1357,6 +1370,10 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	hpriv->mmio = pcim_iomap_table(pdev)[ahci_pci_bar];
>  
> +	/* must set flag prior to save config in order to take effect */
> +	if (ahci_broken_devslp(pdev))
> +		hpriv->flags |= AHCI_HFLAG_NO_DEVSLP;
> +
>  	/* save initial config */
>  	ahci_pci_save_initial_config(pdev, hpriv);
>  
> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> index 0b670c0..630089a 100644
> --- a/drivers/ata/ahci.h
> +++ b/drivers/ata/ahci.h
> @@ -235,6 +235,7 @@ enum {
>  						        port start (wait until
>  						        error-handling stage) */
>  	AHCI_HFLAG_MULTI_MSI		= (1 << 16), /* multiple PCI MSIs */
> +	AHCI_HFLAG_NO_DEVSLP		= (1 << 17), /* no device sleep */

This change conflicts with "[PATCH V2 1/2] ata: ahci: append new hflag
AHCI_HFLAG_NO_FBS" patch (https://lkml.org/lkml/2014/4/10/36).

>  	/* ap->flags bits */
>  
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index 8cd0184..32fd860 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -653,6 +653,13 @@ void ahci_save_initial_config(struct device *dev,
>  		cap &= ~HOST_CAP_SNTF;
>  	}
>  
> +	if ((cap2 & HOST_CAP2_SDS) && (hpriv->flags & AHCI_HFLAG_NO_DEVSLP)) {
> +		dev_info(dev,
> +			 "controller can't do DEVSLP, turning off\n");
> +		cap2 &= ~HOST_CAP2_SDS;
> +		cap2 &= ~HOST_CAP2_SADM;
> +	}
> +
>  	if (!(cap & HOST_CAP_FBS) && (hpriv->flags & AHCI_HFLAG_YES_FBS)) {
>  		dev_info(dev, "controller can do FBS, turning on CAP_FBS\n");
>  		cap |= HOST_CAP_FBS;

ditto

You most likely need to rebase your changes on top of the aforementioned
patch (Tejun?).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/