linux-kernel - Re: [PATCH v1 3/4] ACPI: scan: Rework Device Check and Bus Check notification handling

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] [day] [month] [year] [list]

Date: Mon, 26 Feb 2024 16:54:27 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Jonathan Cameron <jonathan.cameron@...wei.com>
Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>, Linux ACPI <linux-acpi@...r.kernel.org>, 
	LKML <linux-kernel@...r.kernel.org>, 
	Mika Westerberg <mika.westerberg@...ux.intel.com>, 
	"Russell King (Oracle)" <linux@...linux.org.uk>
Subject: Re: [PATCH v1 3/4] ACPI: scan: Rework Device Check and Bus Check
 notification handling

On Mon, Feb 26, 2024 at 4:37 PM Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> On Thu, Feb 22, 2024 at 7:28 PM Jonathan Cameron
> <Jonathan.Cameron@...wei.com> wrote:
> >
> > On Wed, 21 Feb 2024 21:02:33 +0100
> > "Rafael J. Wysocki" <rjw@...ysocki.net> wrote:

[...]

> > > +bool acpi_device_is_enabled(const struct acpi_device *adev)
> > > +{
> > > +     return acpi_device_is_present(adev) && adev->status.enabled;
> >
> > This resolves as (present or functional) && enabled.
> >
> > By my reading you are not allowed functional && enabled, but not present.
> > Line one of the description says.
> >
> > "If bit [0] is cleared, then bit 1 must also be cleared (in other words, a device that is not present cannot be enabled)."
> >
> > I don't much care about that though (I think we discussed this
> > in Russel's earlier series)
>
> Functional and enabled, but not present would go against the spec.  I
> guess the kernel could protect itself against this, but then whatever
> it chooses to do has not been defined anyway.
>
> The spec doesn't actually say what the OSPM is supposed to do when it
> sees that combination of bits.  I'm inclined to clarify it to say "if
> bit [0] is cleared, bit [1] has no defined meaning and it should be
> ignored by the OSPM".  To be consistent with this interpretation,
> acpi_device_is_enabled() should return "(present and enabled) or
> functional".
>
> I'll change it along these lines.

Actually, I don't think that the "functional" bit has any bearing on
this.  It only means that the OSPM should continue the enumeration
below the device regardless of the present bit value.

In the acpi_processor_add() case it is clearly irrelevant.

acpi_scan_bus_check() needs to walk the entire subtree below the
target device anyway.

As for acpi_scan_device_check(), IMO it's better to make it walk the
subtree below the device if it is present, but not enabled, for
backwards compatibility.