Saturday, May 28, 2022

Re: C states lost on amd64

On May 27 19:39:18, guenther@gmail.com wrote:
> 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.

This is a PC workstation that is always 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.

Thanks. The current snapshots seem to work fine (like they have been),
this was the only snapshot that didn't show the C-states.

Is anyone please aware of a change that could have
temporarily coused that around May 25th?

Jan


>
>
> 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