[AMD Official Use Only]
> -----Original Message-----
> From: Hans de Goede <hdegoede@xxxxxxxxxx>
> Sent: Friday, February 25, 2022 15:20
> To: Limonciello, Mario <Mario.Limonciello@xxxxxxx>; Damien Le Moal
> <damien.lemoal@xxxxxxxxxxxxxxxxxx>
> Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers) <linux-
> ide@xxxxxxxxxxxxxxx>; open list <linux-kernel@xxxxxxxxxxxxxxx>
> Subject: Re: [RFC 2/2] ata: ahci: Protect users from setting policies their
> drives don't support
>
> Hi,
>
> On 2/25/22 19:10, Mario Limonciello wrote:
> > As the default low power policy applies to more chipsets and drives, it's
> > important to make sure that drives actually support the policy that a user
> > selected in their kernel configuration.
> >
> > If the drive doesn't support slumber, don't let the default policy for the
> > ATA port be `min_power` or `min_power_with_partial`.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> > ---
> > drivers/ata/ahci.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> > index 17d757ad7111..af8999453084 100644
> > --- a/drivers/ata/ahci.c
> > +++ b/drivers/ata/ahci.c
> > @@ -1584,8 +1584,16 @@ static int ahci_init_msi(struct pci_dev *pdev,
> unsigned int n_ports,
> > static void ahci_update_initial_lpm_policy(struct ata_port *ap,
> > struct ahci_host_priv *hpriv)
> > {
> > + struct pci_dev *pdev = to_pci_dev(ap->host->dev);
> > int policy = CONFIG_SATA_LPM_POLICY;
> >
> > + if (policy >= ATA_LPM_MIN_POWER_WITH_PARTIAL &&
> > + !(hpriv->cap & HOST_CAP_SSC)) {
> > + dev_warn(&pdev->dev,
> > + "This drive doesn't support slumber; ignoring default
> SATA policy\n");
> > + return;
> > + }
> > +
>
> Don't the capabilties get checked later when the policy gets applied ?
>
> At least I think they do get checked later, but I have not looked
> at this code for years ... ?
There's a bunch of layers of indirection so I might have missed something,
but I didn't see anything in sata_link_scr_lpm or anywhere else for that
matter that actually checked HOST_CAP_SSC.
>
> Regards,
>
> Hans
>
>
> > /* user modified policy via module param */
> > if (mobile_lpm_policy != -1) {
> > policy = mobile_lpm_policy;
