> Date: Tue, 25 Jan 2022 19:44:23 +0100 (CET)
> From: Mark Kettenis <[email protected]>
>
> > Date: Tue, 25 Jan 2022 07:01:41 +0100
> > From: Anton Lindqvist <[email protected]>
> >
> > On Mon, Jan 24, 2022 at 08:40:36PM +0100, Mark Kettenis wrote:
> > > > Date: Mon, 24 Jan 2022 20:19:46 +0100
> > > > From: Anton Lindqvist <[email protected]>
> > > >
> > > > On Mon, Jan 24, 2022 at 05:31:49PM +0100, Mark Kettenis wrote:
> > > > > Currently we attach ACPI devices that are present in a machine.
> > > > > However, in some cases ACPI devices can be present, but not enabled.
> > > > > Attaching a device driver to devices that are not enabled is not a
> > > > > good idea since reading and writing from/to its registers will fail
> > > > > and the driver will malfunction in interesting ways. Such as a com(4)
> > > > > serial port that is misdetected and hangs the kernel when it is
> > > > > actually opened.
> > > > >
> > > > > The diff below makes sure we only enable devices that are actually
> > > > > enabled. This may cause some devices to disappear in OpenBSD.
> > > > > However those devices should have been unusable anyway, so that isn't
> > > > > an issue.
> > > > >
> > > > > ok?
> > > >
> > > > According to the ACPI specification[1]:
> > > >
> > > > > A device can only decode its hardware resources if both bits 0 and 1
> > > > > are
> > > > > set. If the device is not present (bit [0] cleared) or not enabled
> > > > > (bit
> > > > > [1] cleared), then the device must not decode its resources.
> > >
> > > Just before that it 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).
> > >
> > > > Should we therefore check for presence of both STA_PRESENT and
> > > > STA_ENABLED?
> > >
> > > So according to the ACPI specification we don't need to do that.
> > > Should we do it just to be safe?
> >
> > I would still vote for it.
>
> Since several people pointed this out, here is a diff that checks both
> bits.
>
> ok?
No. As a fellow developer pointed out, this is wrong. Correct diff
below.
Index: dev/acpi/acpi.c
===================================================================
RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.405
diff -u -p -r1.405 acpi.c
--- dev/acpi/acpi.c 12 Jan 2022 11:18:30 -0000 1.405
+++ dev/acpi/acpi.c 25 Jan 2022 18:59:52 -0000
@@ -3321,7 +3321,7 @@ acpi_foundhid(struct aml_node *node, voi
return (0);
sta = acpi_getsta(sc, node->parent);
- if ((sta & STA_PRESENT) == 0)
+ if ((sta & (STA_PRESENT | STA_ENABLED)) != (STA_PRESENT | STA_ENABLED))
return (0);
if (aml_evalinteger(sc, node->parent, "_CCA", 0, NULL, &cca))
