linux-kernel - Re: [PATCH v6] ACPI: device_sysfs: Add sysfs support for

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]
Message-ID: <CAJZ5v0gwMSWXMA4XmEioUr2ML3y_NPe9WYUCj2XUj++9ivSLTA@mail.gmail.com>
Date:   Tue, 15 Feb 2022 15:04:54 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Won Chung <wonchung@...gle.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Len Brown <lenb@...nel.org>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Benson Leung <bleung@...omium.org>,
        Prashant Malani <pmalani@...omium.org>,
        ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6] ACPI: device_sysfs: Add sysfs support for _PLD

Adding Greg, who should be involved in this discussion IMO.

On Mon, Feb 14, 2022 at 11:59 PM Won Chung <wonchung@...gle.com> wrote:
>
> On Mon, Feb 14, 2022 at 12:30 PM Won Chung <wonchung@...gle.com> wrote:
> >
> > On Mon, Feb 14, 2022 at 11:12 AM Rafael J. Wysocki <rafael@...nel.org> wrote:
> > >
> > > On Fri, Feb 11, 2022 at 3:30 AM Won Chung <wonchung@...gle.com> wrote:
> > > >
> > > > When ACPI table includes _PLD fields for a device, create a new
> > > > directory (pld) in sysfs to share _PLD fields.
> > >
> > > This version of the patch loos better to me, but I'm not sure if it
> > > goes into the right direction overall.
> > >
> > > > Currently without PLD information, when there are multiple of same
> > > > devices, it is hard to distinguish which device corresponds to which
> > > > physical device in which location. For example, when there are two Type
> > > > C connectors, it is hard to find out which connector corresponds to the
> > > > Type C port on the left panel versus the Type C port on the right panel.
> > >
> > > So I think that this is your primary use case and I'm wondering if
> > > this is the best way to address it.
> > >
> > > Namely, by exposing _PLD information under the ACPI device object,
> > > you'll make user space wanting to use that information depend on this
> > > interface, but the problem is not ACPI-specific (inevitably, it will
> > > appear on systems using DT, sooner or later) and making the user space
> > > interface related to it depend on ACPI doesn't look like a perfect
> > > choice.
> > >
> > > IOW, why don't you create a proper ABI for this in the Type C
> > > subsystem and expose the information needed by user space in a generic
> > > way that can be based on the _PLD information on systems with ACPI?
> >
> > Hi Rafael,
> >
> > Thank you for the review.
> >
> > I was thinking that _PLD info is specific to ACPI since it is part of
> > the ACPI table. Could you explain a little bit more on why you think
> > exposing _PLD fields is not an ACPI-specific problem?

_PLD is an interface defined by ACPI, but its purpose is not ACPI-specific.

> Hi Rafael again,
>
> Sorry for the silly question here. I misunderstood your comment a bit,
> but I talked to Benson and Prashant for clarification. I understand
> now what you mean by it is not an ACPI-specific problem and exposing
> PLD would depend on ACPI.

Right.

> >
> > I gave an example of how _PLD fields can be used for specifying Type C
> > connectors, but it is not Type C specific. For Chrome OS, we plan to
> > initially add PLD to not only Type C connectors but also USB port
> > devices (including Type C and Type A). Also, PLD can be used in the
> > future for describing other types of ports too like HDMI. (Benson and
> > Prashant, please correct or add if I am wrong or missing some
> > information) Maybe my commit message was not detailed enough..
> >
> > I am also curious what Heikki thinks about this. Heikki, can you take
> > a look and share your thoughts?
>
> I am still curious what you and Heikki think about this since it may
> not be a Type C specific issue. We can start from adding generic
> location info to Type C subsystem first, as you suggested, then
> consider how to do the same for USB devices and Type A ports
> afterwards. I would appreciate sharing any thoughts or feedback. Thank
> you very much!

I don't really think that this is a Type C problem either.

It has existed for a long time in the USB world, for example, or
wherever there are user-accessible ports, but it looks like in the
Type C case it has become vitally important.

My point is that writing user space depending on accessing _PLD
information exposed under an ACPI device interface that only
corresponds to the device in question and in the ACPI-specific format
would be a mistake (Greg, please let me know if you disagree).  That's
because (a) it would depend on ACPI tables being present (so it
wouldn't work on systems without them) and (b) it would depend on the
format of data which covers information that isn't likely to be
relevant.

If this information is exposed by the kernel verbatim and user space
depending on this information is created, it will not be possible to
unexpose it even if it turns out that exposing it has been a mistake.

OTOH, if only the relevant pieces of information are exposed in a
generic way, it is always possible to expose more pieces of it in the
future as needed.