git: 69a303ace76f - main - acpi_spmc(4): Auto-detect DSM revisions by default
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 13 May 2026 12:40:06 UTC
The branch main has been updated by olce:
URL: https://cgit.FreeBSD.org/src/commit/?id=69a303ace76f42630ca07bcc5d2d090774a0364d
commit 69a303ace76f42630ca07bcc5d2d090774a0364d
Author: Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2026-05-07 08:22:13 +0000
Commit: Olivier Certner <olce@FreeBSD.org>
CommitDate: 2026-05-13 12:38:26 +0000
acpi_spmc(4): Auto-detect DSM revisions by default
Which revisions to use for the Intel and AMD DSMs is unclear. For the
Intel one, the written specification indicates only 0, but Linux uses
1 (possibly an oversight). For the AMD one, for which there is no
specification, Linux uses 0, but at least on the Framework 13 AMD 7040
series, the "enumerate functions" function only returns a mask that
covers all the functions we expect when called with revision 2.
Introduce an auto-detection strategy where each revision starting from
0 is tried in turn up to some limit (included; default: 15). As soon as
a revision implements all expected functions, we stop the loop and use
that one, in effect selecting the minimum revision that implements all
we need, which should avoid potential backwards-compatibility problems.
If no revision implements all expected functions, the highest available
revision in the checked range is selected, but higher revisions that do
not bring new functions are discarded (see the explanatory comment in
acpi_spmc_probe_dsm()).
The revision policy is still tunable using the same existing sysctl(8)
knobs 'debug.acpi.spmc.intel_dsm_revision' and
'debug.acpi.spmc.amd_dsm_revision'. They have been extended so that
a negative value indicates to use the auto-detection mechanism up to
a revision of minus the value. As before, a 0 or positive value
requests a specific revision. A new knob is introduced for the
Microsoft DSM just in case ('debug.acpi.spmc.ms_dsm_revision').
Since now the revision can be auto-detected, and thus depends on
a particular device instance, move it into 'struct dsm_info' on the
softc. This also enables finishing the split between static and
dynamic/tunable information, allowing to constify all the DSM
descriptors.
Print the revision eventually used along with the supported functions.
Tested on an Intel Framework laptop.
Reviewed by: obiwac
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D56882
---
sys/dev/acpica/acpi_spmc.c | 249 +++++++++++++++++++++++++++++++--------------
1 file changed, 175 insertions(+), 74 deletions(-)
diff --git a/sys/dev/acpica/acpi_spmc.c b/sys/dev/acpica/acpi_spmc.c
index b1f97685e50c..fcd4105b2a76 100644
--- a/sys/dev/acpica/acpi_spmc.c
+++ b/sys/dev/acpica/acpi_spmc.c
@@ -32,6 +32,39 @@ static char *spmc_ids[] = {
NULL
};
+/* sysctl(8) knobs */
+static SYSCTL_NODE(_debug_acpi, OID_AUTO, spmc, CTLFLAG_RD | CTLFLAG_MPSAFE,
+ NULL, "SPMC debugging");
+
+int8_t dsm_intel_revision = -15;
+SYSCTL_S8(_debug_acpi_spmc, OID_AUTO, intel_dsm_revision, CTLFLAG_RW,
+ &dsm_intel_revision, 0,
+ "Revision to use with the Intel DSM "
+ "(negative: auto, try from 0 to minus the value)");
+
+int8_t dsm_amd_revision = -15;
+SYSCTL_S8(_debug_acpi_spmc, OID_AUTO, amd_dsm_revision, CTLFLAG_RW,
+ &dsm_amd_revision, 0,
+ "Revision to use with the AMD DSM "
+ "(negative: auto, try from 0 to minus the value)");
+
+int8_t dsm_ms_revision = -15;
+SYSCTL_S8(_debug_acpi_spmc, OID_AUTO, ms_dsm_revision, CTLFLAG_RW,
+ &dsm_ms_revision, 0,
+ "Revision to use with the Microsoft DSM "
+ "(negative: auto, try from 0 to minus the value)");
+
+static int verbose;
+SYSCTL_INT(_debug_acpi_spmc, OID_AUTO, verbose, CTLFLAG_RW,
+ &verbose, 0, "acpi_spmc(4) verbosity");
+
+#define VERBOSE() (verbose || bootverbose)
+
+static bool force_call_expected_functions;
+SYSCTL_BOOL(_debug_acpi_spmc, OID_AUTO, always_call_expected_functions,
+ CTLFLAG_RW, &force_call_expected_functions, 0,
+ "Call all expected functions on a present DSM, even those not enumerated.");
+
/* Conversion of an index to a mask. */
#define IDX_TO_BIT(idx) (1ull << (idx))
@@ -64,21 +97,20 @@ static char *spmc_ids[] = {
struct dsm_desc {
const char *name;
- /* Index in the dsms[] array below. */
- int index;
+ struct uuid uuid;
/*
- * Revisions are zero or a positive number. Strictly speaking, next
- * field should be a 'uint64_t' as per the ACPI spec, but our ACPI DSM
- * interface takes an 'int' and anyway actual revision numbers never
- * even exceed the limits of a 'uint8_t'.
+ * Points to an integer which, if negative, indicates to auto-detect the
+ * revision by trying all revisions between 0 and minus the value, else
+ * is the sole revision to try.
*/
- int revision;
- struct uuid uuid;
+ const int8_t *revision_spec;
uint64_t expected_functions;
uint64_t extra_functions;
/* Human-friendly names of known functions. */
const char *const *function_names;
int function_names_nb;
+ /* Index in the dsms[] array below. */
+ int index;
};
static const char *const dsm_intel_function_names[] = {
@@ -90,22 +122,14 @@ static const char *const dsm_intel_function_names[] = {
[DSM_INTEL_MS_LPI_EXIT_NOTIF] = "LPI_EXIT",
};
-static struct dsm_desc dsm_intel = {
+static const struct dsm_desc dsm_intel = {
.index = DSM_INTEL,
.name = "Intel",
.uuid = { /* c4eb40a0-6cd2-11e2-bcfd-0800200c9a66 */
0xc4eb40a0, 0x6cd2, 0x11e2, 0xbc, 0xfd,
{0x08, 0x00, 0x20, 0x0c, 0x9a, 0x66}
},
- /*
- * XXX Linux uses 1 for the revision on Intel DSMs, but doesn't explain
- * why. The commit that introduces this links to a document mentioning
- * revision 0, so default this to 0.
- *
- * The debug.acpi.spmc.intel_dsm_revision sysctl may be used to configure
- * this just in case.
- */
- .revision = 0,
+ .revision_spec = &dsm_intel_revision,
.expected_functions =
IDX_TO_BIT(DSM_GET_DEVICE_CONSTRAINTS) |
IDX_TO_BIT(DSM_INTEL_MS_DISPLAY_OFF_NOTIF) |
@@ -135,7 +159,7 @@ static const struct dsm_desc dsm_ms = {
0x11e00d56, 0xce64, 0x47ce, 0x83, 0x7b,
{0x1f, 0x89, 0x8f, 0x9a, 0xa4, 0x61}
},
- .revision = 0,
+ .revision_spec = &dsm_ms_revision,
.expected_functions =
IDX_TO_BIT(DSM_INTEL_MS_DISPLAY_OFF_NOTIF) |
IDX_TO_BIT(DSM_INTEL_MS_DISPLAY_ON_NOTIF) |
@@ -157,23 +181,14 @@ static const char *const dsm_amd_function_names[] = {
[DSM_AMD_LPI_EXIT_NOTIF] = "LPI_EXIT",
};
-static struct dsm_desc dsm_amd = {
+static const struct dsm_desc dsm_amd = {
.index = DSM_AMD,
.name = "AMD",
.uuid = { /* e3f32452-febc-43ce-9039-932122d37721 */
0xe3f32452, 0xfebc, 0x43ce, 0x90, 0x39,
{0x93, 0x21, 0x22, 0xd3, 0x77, 0x21}
},
- /*
- * XXX Linux uses 0 for the revision on AMD DSMs, but at least on the
- * Framework 13 AMD 7040 series, the "enumerate functions" DSM function
- * needs to be called with revision 2 to return a mask that covers all
- * the functions we would like to call.
- *
- * The debug.acpi.spmc.amd_dsm_revision sysctl may be used to configure
- * this just in case.
- */
- .revision = 2,
+ .revision_spec = &dsm_amd_revision,
.expected_functions =
IDX_TO_BIT(DSM_GET_DEVICE_CONSTRAINTS) |
IDX_TO_BIT(DSM_AMD_DISPLAY_OFF_NOTIF) |
@@ -190,30 +205,16 @@ static const struct dsm_desc *const dsms[] = {
[DSM_AMD] = &dsm_amd,
};
-static SYSCTL_NODE(_debug_acpi, OID_AUTO, spmc, CTLFLAG_RD | CTLFLAG_MPSAFE,
- NULL, "SPMC debugging");
-
-SYSCTL_INT(_debug_acpi_spmc, OID_AUTO, intel_dsm_revision, CTLFLAG_RW,
- &dsm_intel.revision, 0,
- "Revision to use when evaluating Intel SPMC DSMs");
-
-SYSCTL_INT(_debug_acpi_spmc, OID_AUTO, amd_dsm_revision, CTLFLAG_RW,
- &dsm_amd.revision, 0, "Revision to use when evaluating AMD SPMC DSMs");
-
-static int verbose;
-SYSCTL_INT(_debug_acpi_spmc, OID_AUTO, verbose, CTLFLAG_RW,
- &verbose, 0, "acpi_spmc(4) verbosity");
-
-#define VERBOSE() (verbose || bootverbose)
-
-static bool force_call_expected_functions;
-SYSCTL_BOOL(_debug_acpi_spmc, OID_AUTO, always_call_expected_functions,
- CTLFLAG_RW, &force_call_expected_functions, 0,
- "Call all expected functions on a present DSM, even those not enumerated.");
-
/* Per DSM probed information. */
struct dsm_info {
uint64_t supported_functions;
+ /*
+ * Revisions are zero or a positive number. Strictly speaking, next
+ * field should be a 'uint64_t' as per the ACPI spec, but our ACPI DSM
+ * interface takes an 'int' and anyway actual revision numbers never
+ * even exceed the limits of a 'uint8_t'.
+ */
+ uint8_t revision;
};
struct acpi_spmc_constraint {
@@ -253,20 +254,60 @@ resolve_dsm(int dsm_index)
return (dsms[dsm_index]);
}
+static struct dsm_info *
+get_dsm_info(struct acpi_spmc_softc *const sc, const int dsm_index)
+{
+ MPASS(0 <= dsm_index && dsm_index < nitems(dsms));
+ return (&sc->dsms_info[dsm_index]);
+}
+
+static const struct dsm_info *
+get_const_dsm_info(const struct acpi_spmc_softc *const sc, const int dsm_index)
+{
+ MPASS(0 <= dsm_index && dsm_index < nitems(dsms));
+ return (&sc->dsms_info[dsm_index]);
+}
+
+static const uint64_t
+get_supported_functions(const struct acpi_spmc_softc *const sc,
+ const int dsm_index)
+{
+ return (get_const_dsm_info(sc, dsm_index)->supported_functions);
+}
+
+static const uint8_t
+get_revision(const struct acpi_spmc_softc *const sc, const int dsm_index)
+{
+ return (get_const_dsm_info(sc, dsm_index)->revision);
+}
+
+static bool
+supports_function_bitset(const uint64_t supported_functions,
+ const int function_index)
+{
+ return ((supported_functions & IDX_TO_BIT(function_index)) != 0);
+}
+
static bool
supports_function(const struct acpi_spmc_softc *const sc, const int dsm_index,
const int function_index)
{
- MPASS(0 <= dsm_index && dsm_index < nitems(dsms));
+ return (supports_function_bitset(get_supported_functions(sc, dsm_index),
+ function_index));
+}
- return ((sc->dsms_info[dsm_index].supported_functions &
- IDX_TO_BIT(function_index)) != 0);
+static bool
+has_dsm_bitset(const uint64_t supported_functions)
+{
+ /* DSM is supported if bit DSM_ENUM_FUNCTIONS (0) is set. */
+ return (supports_function_bitset(supported_functions,
+ DSM_ENUM_FUNCTIONS));
}
static bool
has_dsm(const struct acpi_spmc_softc *const sc, const int dsm_index)
{
- return (supports_function(sc, dsm_index, DSM_ENUM_FUNCTIONS));
+ return (has_dsm_bitset(get_supported_functions(sc, dsm_index)));
}
typedef const char *pbf_get_name_t(const int, const void *const);
@@ -344,12 +385,13 @@ failed_to_call_dsm(const struct acpi_spmc_softc *const sc,
{
(void)device_printf(sc->dev,
"Failed to call DSM %s (rev %u) function %s\n",
- dsm->name, dsm->revision, dsm_function_name(dsm, function_index));
+ dsm->name, get_revision(sc, dsm->index),
+ dsm_function_name(dsm, function_index));
}
static void acpi_spmc_probe_dsm(struct acpi_spmc_softc *const sc,
const struct dsm_desc *const dsm);
-static void acpi_spmc_dsm_print_functions(
+static void acpi_spmc_dsm_print(
const struct acpi_spmc_softc *const sc,
const struct dsm_desc *const dsm);
static int acpi_spmc_get_constraints(struct acpi_spmc_softc *const sc);
@@ -416,7 +458,7 @@ acpi_spmc_attach(device_t dev)
/* Print supported functions of usable DSMs. */
for (int i = 0; i < nitems(dsms); ++i)
if (has_dsm(sc, i))
- acpi_spmc_dsm_print_functions(sc, dsms[i]);
+ acpi_spmc_dsm_print(sc, dsms[i]);
/* Get device constraints. We can only call this once so do this now. */
error = acpi_spmc_get_constraints(sc);
@@ -445,8 +487,15 @@ acpi_spmc_detach(device_t dev)
return (0);
}
+static uint64_t
+dsm_missing_functions(const struct dsm_desc *const dsm,
+ uint64_t supported_functions)
+{
+ return (dsm->expected_functions & ~supported_functions);
+}
+
static void
-acpi_spmc_dsm_print_functions(const struct acpi_spmc_softc *const sc,
+acpi_spmc_dsm_print(const struct acpi_spmc_softc *const sc,
const struct dsm_desc *const dsm)
{
/*
@@ -455,16 +504,17 @@ acpi_spmc_dsm_print_functions(const struct acpi_spmc_softc *const sc,
* report as unknown.
*/
const uint64_t supported_functions = ~IDX_TO_BIT(DSM_ENUM_FUNCTIONS) &
- sc->dsms_info[dsm->index].supported_functions;
- const uint64_t missing = dsm->expected_functions & ~supported_functions;
+ get_supported_functions(sc, dsm->index);
+ const uint64_t missing = dsm_missing_functions(dsm, supported_functions);
const uint64_t unknown = supported_functions &
~(dsm->expected_functions | dsm->extra_functions);
char buf[128];
print_bit_field(buf, sizeof(buf), supported_functions,
"FUNC", pbf_function_name, dsm);
- device_printf(sc->dev, "DSM %s: Supported functions: %#" PRIx64 "%s\n",
- dsm->name, supported_functions, buf);
+ device_printf(sc->dev,
+ "DSM %s, revision %d: Supported functions: %#" PRIx64 "%s\n",
+ dsm->name, get_revision(sc, dsm->index), supported_functions, buf);
if (VERBOSE() && missing != 0) {
print_bit_field(buf, sizeof(buf), missing, "FUNC",
@@ -483,19 +533,69 @@ acpi_spmc_dsm_print_functions(const struct acpi_spmc_softc *const sc,
}
}
+/* Returns whether the DSM is supported (enumeration succeeds). */
+static bool
+probe_dsm_revision(const struct acpi_spmc_softc *const sc,
+ const struct dsm_desc *const dsm, const uint8_t revision,
+ uint64_t *const supported_functions)
+{
+ *supported_functions = acpi_DSMQuery(sc->handle,
+ (const uint8_t *)&dsm->uuid, revision);
+ return (has_dsm_bitset(*supported_functions));
+}
+
+static void
+set_dsm_revision(struct acpi_spmc_softc *const sc,
+ const struct dsm_desc *const dsm, const uint8_t revision,
+ uint64_t supported_functions)
+{
+ struct dsm_info *const dsm_info = get_dsm_info(sc, dsm->index);
+
+ MPASS(has_dsm_bitset(supported_functions));
+ dsm_info->supported_functions = supported_functions;
+ dsm_info->revision = revision;
+}
+
static void
acpi_spmc_probe_dsm(struct acpi_spmc_softc *const sc,
const struct dsm_desc *const dsm)
{
- const uint64_t supported_functions = acpi_DSMQuery(sc->handle,
- (const uint8_t *)&dsm->uuid, dsm->revision);
+ const int8_t revision_spec = *dsm->revision_spec;
+ uint64_t supported_functions;
+
+ if (revision_spec >= 0) {
+ /* Specific revision specified. */
+ if (probe_dsm_revision(sc, dsm, revision_spec,
+ &supported_functions))
+ set_dsm_revision(sc, dsm, revision_spec,
+ supported_functions);
+ return;
+ }
/*
- * DSM is supported if bit 0 is set.
+ * Auto-detect. We try revisions in ascending order, selecting the
+ * first that has all the functions we expect in the hope to avoid potential
+ * backwards-compatibility problems, else continuing with higher ones
+ * but adopting them only if they actually add new functions (it seems
+ * common that firmwares do not care about the revision, or will return
+ * the same supported functions after a revision limit).
*/
- if ((supported_functions & IDX_TO_BIT(DSM_ENUM_FUNCTIONS)) == 0)
- return;
- sc->dsms_info[dsm->index].supported_functions = supported_functions;
+ for (uint8_t revision = 0; revision <= -revision_spec; ++revision) {
+ if (!probe_dsm_revision(sc, dsm, revision,
+ &supported_functions))
+ continue;
+ if ((~get_supported_functions(sc, dsm->index) &
+ supported_functions) == 0)
+ /* This revision adds no new function, skip it. */
+ continue;
+
+ set_dsm_revision(sc, dsm, revision, supported_functions);
+
+ if (dsm_missing_functions(dsm, ~IDX_TO_BIT(DSM_ENUM_FUNCTIONS) &
+ supported_functions) == 0)
+ /* We have all expected functions, bail out. */
+ break;
+ }
}
static void
@@ -627,7 +727,7 @@ acpi_spmc_parse_constraints_amd(struct acpi_spmc_softc *sc, ACPI_OBJECT *object)
static int
acpi_spmc_get_constraints(struct acpi_spmc_softc *const sc)
{
- struct dsm_desc *dsm;
+ const struct dsm_desc *dsm;
ACPI_STATUS status;
ACPI_BUFFER result;
ACPI_OBJECT *object;
@@ -657,9 +757,9 @@ acpi_spmc_get_constraints(struct acpi_spmc_softc *const sc)
return (0);
/* It seems like this DSM can fail if called more than once. */
- status = acpi_EvaluateDSMTyped(sc->handle, (uint8_t *)&dsm->uuid,
- dsm->revision, DSM_GET_DEVICE_CONSTRAINTS, NULL, &result,
- ACPI_TYPE_PACKAGE);
+ status = acpi_EvaluateDSMTyped(sc->handle, (const uint8_t *)&dsm->uuid,
+ get_revision(sc, dsm->index), DSM_GET_DEVICE_CONSTRAINTS, NULL,
+ &result, ACPI_TYPE_PACKAGE);
if (ACPI_FAILURE(status)) {
failed_to_call_dsm(sc, dsm, DSM_GET_DEVICE_CONSTRAINTS);
return (ENXIO);
@@ -765,7 +865,8 @@ acpi_spmc_run(device_t dev, const struct dsm_desc *const dsm,
return;
status = acpi_EvaluateDSMTyped(sc->handle, (const uint8_t *)&dsm->uuid,
- dsm->revision, function_index, NULL, &result, ACPI_TYPE_ANY);
+ get_revision(sc, dsm->index), function_index, NULL,
+ &result, ACPI_TYPE_ANY);
if (ACPI_FAILURE(status))
failed_to_call_dsm(sc, dsm, function_index);