[Public]
> On Thu, Sep 28, 2023 at 05:36:30PM -0500, Bjorn Helgaas wrote:
> > On Mon, Sep 18, 2023 at 02:48:01PM +0200, Lukas Wunner wrote:
> > > struct pci_dev contains two flags which govern whether the device may
> > > suspend to D3cold:
> > >
> > > * no_d3cold provides an opt-out for drivers (e.g. if a device is known
> > > to not wake from D3cold)
> > >
> > > * d3cold_allowed provides an opt-out for user space (default is true,
> > > user space may set to false)
> > >
> > > Since commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during
> suspend"),
> > > the user space setting overwrites the driver setting. Essentially user
> > > space is trusted to know better than the driver whether D3cold is
> > > working.
> > >
> > > That feels unsafe and wrong. Assume that the change was introduced
> > > inadvertently and do not overwrite no_d3cold when d3cold_allowed is
> > > modified. Instead, consider d3cold_allowed in addition to no_d3cold
> > > when choosing a suspend state for the device.
> > >
> > > That way, user space may opt out of D3cold if the driver hasn't, but it
> > > may no longer force an opt in if the driver has opted out.
> > >
> > > Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
> > > Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
> > > Cc: stable@xxxxxxxxxxxxxxx # v4.8+
> > > Cc: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> >
> > Mika and Mario, you both commented on this, but I *think* you were
> > both OK with it as-is for now? If so, can you give a Reviewed-by?
> > I don't want to go ahead if you have any concerns.
>
> No concerns from me,
>
> Reviewed-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
It is a step in the right direction. I do think a later enhancement to
fail the write from sysfs (to be ABI compatible but "deprecate it") and
internal optimization as a result is a good idea though.
Reviewed-by: Mario Limonciello <mario.limonciello@xxxxxxx>
