Friday, May 27, 2022

Re: C states lost on amd64

On Fri, 27 May 2022, Jan Stary wrote:
> ... and with the latest snapshot, they are back.
...
> acpicpu0 at acpi0: C3(350@96 mwait.1@0x20), C2(500@64 mwait.1@0x10), C1(1000@1 mwait.1), PSS
> acpicpu1 at acpi0: C3(350@96 mwait.1@0x20), C2(500@64 mwait.1@0x10), C1(1000@1 mwait.1), PSS
> acpicpu2 at acpi0: C3(350@96 mwait.1@0x20), C2(500@64 mwait.1@0x10), C1(1000@1 mwait.1), PSS
> acpicpu3 at acpi0: C3(350@96 mwait.1@0x20), C2(500@64 mwait.1@0x10), C1(1000@1 mwait.1), PSS
>
> On May 26 14:34:43, hans@stare.cz wrote:
> > This is current/adm64, dmesgs below.
> > With the current snapshot, the C states are gone:
> >
> > -acpicpu0 at acpi0: C3(350@96 mwait.1@0x20), C2(500@64 mwait.1@0x10), C1(1000@1 mwait.1), PSS
> > -acpicpu1 at acpi0: C3(350@96 mwait.1@0x20), C2(500@64 mwait.1@0x10), C1(1000@1 mwait.1), PSS
> > -acpicpu2 at acpi0: C3(350@96 mwait.1@0x20), C2(500@64 mwait.1@0x10), C1(1000@1 mwait.1), PSS
> > -acpicpu3 at acpi0: C3(350@96 mwait.1@0x20), C2(500@64 mwait.1@0x10), C1(1000@1 mwait.1), PSS
> > +acpicpu0 at acpi0: C1(@1 halt!), PSS
> > +acpicpu1 at acpi0: C1(@1 halt!), PSS
> > +acpicpu2 at acpi0: C1(@1 halt!), PSS
> > +acpicpu3 at acpi0: C1(@1 halt!), PSS
> >
> > Is this expected?
> > Is it related to the recent apmd -A change?

Not really. Well, unless your box is one where the states change
depending on, say, whether the box is plugged in.

You could give this diff a shot. It enables processing of CST change
notifications. No committers have a (working) box that does that, so I
couldn't get any interest and I have no idea when--or even if--it might go
in.

Philip Guenther


Index: sys/dev/acpi/acpicpu.c
===================================================================
RCS file: /data/src/openbsd/src/sys/dev/acpi/acpicpu.c,v
retrieving revision 1.92
diff -u -p -r1.92 acpicpu.c
--- sys/dev/acpi/acpicpu.c 6 Apr 2022 18:59:27 -0000 1.92
+++ sys/dev/acpi/acpicpu.c 12 Apr 2022 06:13:55 -0000
@@ -25,6 +25,7 @@
#include <sys/malloc.h>
#include <sys/queue.h>
#include <sys/atomic.h>
+#include <sys/mutex.h>

#include <machine/bus.h>
#include <machine/cpu.h>
@@ -80,6 +81,7 @@ void acpicpu_setperf_ppc_change(struct a
#define CST_FLAG_FALLBACK 0x4000 /* fallback for broken _CST */
#define CST_FLAG_SKIP 0x8000 /* state is worse choice */

+#define FLAGS_NOCST 0x01
#define FLAGS_MWAIT_ONLY 0x02
#define FLAGS_BMCHECK 0x04
#define FLAGS_NOTHROTTLE 0x08
@@ -113,8 +115,10 @@ struct acpi_cstate
uint64_t address; /* or mwait hint */
};

-unsigned long cst_stats[4] = { 0 };
-
+/*
+ * Locking:
+ * m sc_mtx
+ */
struct acpicpu_softc {
struct device sc_dev;
int sc_cpu;
@@ -130,6 +134,10 @@ struct acpicpu_softc {
struct cpu_info *sc_ci;
SLIST_HEAD(,acpi_cstate) sc_cstates;

+ struct mutex sc_mtx;
+ struct acpi_cstate *sc_cstates_active; /* [m] */
+ int sc_mwait_only; /* [m] */
+
bus_space_tag_t sc_iot;
bus_space_handle_t sc_ioh;

@@ -161,10 +169,13 @@ struct acpicpu_softc {

void acpicpu_add_cstatepkg(struct aml_value *, void *);
void acpicpu_add_cdeppkg(struct aml_value *, void *);
+void acpicpu_cst_activate(struct acpicpu_softc *);
int acpicpu_getppc(struct acpicpu_softc *);
int acpicpu_getpct(struct acpicpu_softc *);
int acpicpu_getpss(struct acpicpu_softc *);
int acpicpu_getcst(struct acpicpu_softc *);
+int acpicpu_cst_changed(struct acpicpu_softc *);
+void acpicpu_free_states(struct acpi_cstate *);
void acpicpu_getcst_from_fadt(struct acpicpu_softc *);
void acpicpu_print_one_cst(struct acpi_cstate *_cx);
void acpicpu_print_cst(struct acpicpu_softc *_sc);
@@ -510,11 +521,11 @@ acpicpu_getcst(struct acpicpu_softc *sc)
struct acpi_cstate *cx, *next_cx;
int use_nonmwait;

- /* delete the existing list */
- while ((cx = SLIST_FIRST(&sc->sc_cstates)) != NULL) {
- SLIST_REMOVE_HEAD(&sc->sc_cstates, link);
- free(cx, M_DEVBUF, sizeof(*cx));
- }
+ /* set aside the existing list and free it if not active */
+ cx = SLIST_FIRST(&sc->sc_cstates);
+ SLIST_INIT(&sc->sc_cstates);
+ if (cx != sc->sc_cstates_active)
+ acpicpu_free_states(cx);

/* provide a fallback C1-via-halt in case _CST's C1 is bogus */
acpicpu_add_cstate(sc, ACPI_STATE_C1, CST_METH_HALT,
@@ -528,8 +539,10 @@ acpicpu_getcst(struct acpicpu_softc *sc)

/* only have fallback state? then no _CST objects were understood */
cx = SLIST_FIRST(&sc->sc_cstates);
- if (cx->flags & CST_FLAG_FALLBACK)
+ if (cx->flags & CST_FLAG_FALLBACK) {
+ sc->sc_flags &= ~FLAGS_MWAIT_ONLY;
return (1);
+ }

/*
* Skip states >= C2 if the CPU's LAPIC timer stops in deep
@@ -558,6 +571,38 @@ acpicpu_getcst(struct acpicpu_softc *sc)
return (0);
}

+int
+acpicpu_cst_changed(struct acpicpu_softc *sc)
+{
+ struct acpi_cstate *active, *new;
+
+ active = sc->sc_cstates_active;
+ new = SLIST_FIRST(&sc->sc_cstates);
+
+ while (active != NULL && new != NULL) {
+ if (active->state != new->state ||
+ active->method != new->method ||
+ active->flags != new->flags ||
+ active->latency != new->latency ||
+ active->address != new->address)
+ return 1;
+ active = SLIST_NEXT(active, link);
+ new = SLIST_NEXT(new, link);
+ }
+
+ return active != NULL || new != NULL;
+}
+
+void
+acpicpu_free_states(struct acpi_cstate *cx)
+{
+ while (cx != NULL) {
+ struct acpi_cstate *next_cx = SLIST_NEXT(cx, link);
+ free(cx, M_DEVBUF, sizeof(*cx));
+ cx = next_cx;
+ }
+}
+
/*
* old-style fixed C-state info in the FADT.
* Note that this has extra restrictions on values and flags.
@@ -689,7 +734,9 @@ acpicpu_attach(struct device *parent, st
sc->sc_acpi = (struct acpi_softc *)parent;
sc->sc_devnode = aa->aaa_node;

+ mtx_init(&sc->sc_mtx, IPL_NONE);
SLIST_INIT(&sc->sc_cstates);
+ sc->sc_cstates_active = NULL;

if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode,
"_UID", 0, NULL, &uid) == 0)
@@ -734,9 +781,10 @@ acpicpu_attach(struct device *parent, st

No comments:

Post a Comment