Differential D54505 Diff 169079 sys/x86/cpufreq/hwpstate_amd.c

Changeset View

Standalone View

View Options

sys/x86/cpufreq/hwpstate_amd.c

Show First 20 Lines • Show All 192 Lines • ▼ Show 20 Lines static device_method_t hwpstate_methods[] = {

DEVMETHOD(cpufreq_drv_settings, hwpstate_settings), DEVMETHOD(cpufreq_drv_settings, hwpstate_settings),

DEVMETHOD(cpufreq_drv_type, hwpstate_type), DEVMETHOD(cpufreq_drv_type, hwpstate_type),

/* ACPI interface */ /* ACPI interface */

DEVMETHOD(acpi_get_features, hwpstate_features), DEVMETHOD(acpi_get_features, hwpstate_features),

{0, 0} {0, 0}

}; };

struct amdhwp_dump_sysctl_handler_request {

uint64_t enable;

uint64_t caps;

uint64_t req;

int res;

};

static void

amdhwp_dump_sysctl_handler_cb(void *args)

{

struct amdhwp_dump_sysctl_handler_request *req =

(struct amdhwp_dump_sysctl_handler_request *)args;

req->res = rdmsr_safe(MSR_AMD_CPPC_ENABLE, &req->enable);

if (req->res)

req->res = rdmsr_safe(MSR_AMD_CPPC_CAPS_1, &req->caps);

if (req->res)

req->res = rdmsr_safe(MSR_AMD_CPPC_REQUEST, &req->req);

olceUnsubmitted

Not Done

req->res = rdmsr_safe(MSR_AMD_CPPC_ENABLE, &req->enable);

- if (req->res)

+ if (req->res == 0)

req->res = rdmsr_safe(MSR_AMD_CPPC_CAPS_1, &req->caps);

- if (req->res)

+ if (req->res == 0)

req->res = rdmsr_safe(MSR_AMD_CPPC_REQUEST, &req->req);

return;

Inverted tests here.

olce: Inverted tests here.

return;

markjUnsubmitted

Not Done

The return statement can be removed too.

markj: The return statement can be removed too.

}

static int static int

amdhwp_dump_sysctl_handler(SYSCTL_HANDLER_ARGS) amdhwp_dump_sysctl_handler(SYSCTL_HANDLER_ARGS)

{ {

device_t dev; device_t dev;

struct pcpu *pc; struct pcpu *pc;

markjUnsubmitted

Not Done

Simpler:

req->res = rdmsr_safe(MSR_AMD_CPPC_ENABLE, &req->enable);
if (req->res == 0)
    req->res = rdmsr_safe(MSR_AMD_CPPC_CAPS_1, &req->caps);
if (req->res == 0)
    req->res = rdmsr_safe(MSR_AMD_CPPC_REQUEST, &req->req);

Just a suggestion, feel free to ignore.

markj: Simpler: ``` req->res = rdmsr_safe(MSR_AMD_CPPC_ENABLE, &req->enable); if (req->res == 0)…

struct sbuf *sb; struct sbuf *sb;

struct hwpstate_softc *sc; struct hwpstate_softc *sc;

struct amdhwp_dump_sysctl_handler_request request;

markjUnsubmitted

Not Done

Call it req for consistency with other places?

markj: Call it `req` for consistency with other places?

aokblastAuthorUnsubmitted

Done

SYSCTL_HANDLER_ARGS have already used req as variable name.

aokblast: SYSCTL_HANDLER_ARGS have already used req as variable name.

uint64_t data; uint64_t data;

int ret; int ret;

sc = (struct hwpstate_softc *)arg1; sc = (struct hwpstate_softc *)arg1;

dev = sc->dev; dev = sc->dev;

pc = cpu_get_pcpu(dev); pc = cpu_get_pcpu(dev);

if (pc == NULL) if (pc == NULL)

return (ENXIO); return (ENXIO);

sb = sbuf_new(NULL, NULL, 1024, SBUF_FIXEDLEN | SBUF_INCLUDENUL); sb = sbuf_new(NULL, NULL, 1024, SBUF_FIXEDLEN | SBUF_INCLUDENUL);

sbuf_putc(sb, '\n'); sbuf_putc(sb, '\n');

thread_lock(curthread); smp_rendezvous_cpu(pc->pc_cpuid, smp_no_rendezvous_barrier,

sched_bind(curthread, pc->pc_cpuid); amdhwp_dump_sysctl_handler_cb, smp_no_rendezvous_barrier, &request);

thread_unlock(curthread); if (request.res) {

ret = request.res;

goto out;

}

markjUnsubmitted

Not Done

Just a thought: it might be nice to have an smp_rendezvous_cpu() variant which takes a CPU ID instead of a cpuset.

markj: Just a thought: it might be nice to have an smp_rendezvous_cpu() variant which takes a CPU ID…

aokblastAuthorUnsubmitted

Done

I am also thinking about making some inline function like rdmsr_on_cpu and wrmsr_on_cpu which prevent us from writing callback each time when we just want to read and write a single MSR value at a time.

aokblast: I am also thinking about making some inline function like rdmsr_on_cpu and wrmsr_on_cpu which…

markjUnsubmitted

Not Done

I just remembered that it already exists: x86_msr_op().

markj: I just remembered that it already exists: x86_msr_op().

olceUnsubmitted

Not Done

amdhwp_dump_sysctl_handler_cb, smp_no_rendezvous_barrier, &request);

- if (request.res) {

- ret = request.res;

+ ret = request.res;

+ if (ret)

goto out;

- }

data = request.enable;

Need to initialize ret even in case of success (see also suggested change after out label).

olce: Need to initialize `ret` even in case of success (see also suggested change after `out` label).

rdmsr_safe(MSR_AMD_CPPC_ENABLE, &data); data = request.enable;

sbuf_printf(sb, "CPU%d: HWP %sabled\n", pc->pc_cpuid, sbuf_printf(sb, "CPU%d: HWP %sabled\n", pc->pc_cpuid,

((data & 1) ? "En" : "Dis")); ((data & 1) ? "En" : "Dis"));

if (data == 0) { if (data == 0) {

ret = 0; ret = 0;

goto out; goto out;

} }

olceUnsubmitted

Not Done

((data & 1) ? "En" : "Dis"));

- if (data == 0) {

- ret = 0;

+ if (data == 0)

goto out;

- }

data = request.caps;

Simplification after early init of ret above.

olce: Simplification after early init of `ret` above.

rdmsr_safe(MSR_AMD_CPPC_CAPS_1, &data); data = request.caps;

sbuf_printf(sb, "\tHighest Performance: %03ju\n", sbuf_printf(sb, "\tHighest Performance: %03ju\n",

BITS_VALUE(AMD_CPPC_CAPS_1_HIGH_PERF_BITS, data)); BITS_VALUE(AMD_CPPC_CAPS_1_HIGH_PERF_BITS, data));

sbuf_printf(sb, "\tGuaranteed Performance: %03ju\n", sbuf_printf(sb, "\tGuaranteed Performance: %03ju\n",

BITS_VALUE(AMD_CPPC_CAPS_1_NOMINAL_PERF_BITS, data)); BITS_VALUE(AMD_CPPC_CAPS_1_NOMINAL_PERF_BITS, data));

sbuf_printf(sb, "\tEfficient Performance: %03ju\n", sbuf_printf(sb, "\tEfficient Performance: %03ju\n",

BITS_VALUE(AMD_CPPC_CAPS_1_LOW_NONLIN_PERF_BITS, data)); BITS_VALUE(AMD_CPPC_CAPS_1_LOW_NONLIN_PERF_BITS, data));

sbuf_printf(sb, "\tLowest Performance: %03ju\n", sbuf_printf(sb, "\tLowest Performance: %03ju\n",

BITS_VALUE(AMD_CPPC_CAPS_1_LOW_PERF_BITS, data)); BITS_VALUE(AMD_CPPC_CAPS_1_LOW_PERF_BITS, data));

sbuf_putc(sb, '\n'); sbuf_putc(sb, '\n');

rdmsr_safe(MSR_AMD_CPPC_REQUEST, &data); data = request.req;

#define pkg_print(name, offset) \ #define pkg_print(name, offset) \

do { \ do { \

sbuf_printf(sb, "\t%s: %03u\n", name, \ sbuf_printf(sb, "\t%s: %03u\n", name, \

(unsigned)(data >> offset) & 0xff); \ (unsigned)(data >> offset) & 0xff); \

} while (0) } while (0)

pkg_print("Requested Efficiency Performance Preference", 24); pkg_print("Requested Efficiency Performance Preference", 24);

pkg_print("Requested Desired Performance", 16); pkg_print("Requested Desired Performance", 16);

pkg_print("Requested Maximum Performance", 8); pkg_print("Requested Maximum Performance", 8);

pkg_print("Requested Minimum Performance", 0); pkg_print("Requested Minimum Performance", 0);

#undef pkg_print #undef pkg_print

sbuf_putc(sb, '\n'); sbuf_putc(sb, '\n');

out: out:

thread_lock(curthread);

sched_unbind(curthread);

thread_unlock(curthread);

ret = sbuf_finish(sb); ret = sbuf_finish(sb);

olceUnsubmitted

Not Done

out:

- ret = sbuf_finish(sb);

+ if (ret == 0)

+ ret = sbuf_finish(sb);

if (ret == 0)

Bug: ret set before jumps to out is crushed.

olce: Bug: `ret` set before jumps to `out` is crushed.

if (ret == 0) if (ret == 0)

ret = SYSCTL_OUT(req, sbuf_data(sb), sbuf_len(sb)); ret = SYSCTL_OUT(req, sbuf_data(sb), sbuf_len(sb));

sbuf_delete(sb); sbuf_delete(sb);

return (ret); return (ret);

} }

static bool static void

sysctl_epp_select_per_core(const device_t hwp_device, uint32_t val) sysctl_epp_select_per_core(device_t hwp_device, uint32_t val)

{ {

struct hwpstate_softc *sc; struct hwpstate_softc *sc;

bool success = true;

int ret, cpuid;

cpuid = cpu_get_pcpu(hwp_device)->pc_cpuid;

thread_lock(curthread);

sched_bind(curthread, cpuid);

thread_unlock(curthread);

sc = device_get_softc(hwp_device); sc = device_get_softc(hwp_device);

markjUnsubmitted

Not Done

This should be done in the caller, not in the callback.

Keep in mind that smp rendezvous callbacks are invoked with interrupts disabled, so they should do as little work as possible. Calling device_printf() is ok, but other operations should be avoided if possible.

markj: This should be done in the caller, not in the callback. Keep in mind that smp rendezvous…

if (BITS_VALUE(AMD_CPPC_REQUEST_ENERGY_PERF_BITS, sc->req) == val) if (BITS_VALUE(AMD_CPPC_REQUEST_ENERGY_PERF_BITS, sc->req) == val)

goto end; return;

SET_BITS_VALUE(sc->req, AMD_CPPC_REQUEST_ENERGY_PERF_BITS, val); SET_BITS_VALUE(sc->req, AMD_CPPC_REQUEST_ENERGY_PERF_BITS, val);

ret = wrmsr_safe(MSR_AMD_CPPC_REQUEST, sc->req); x86_msr_op(MSR_AMD_CPPC_REQUEST,

if (ret != 0) { MSR_OP_RENDEZVOUS_ONE | MSR_OP_WRITE |

success = false; MSR_OP_CPUID(cpu_get_pcpu(hwp_device)->pc_cpuid),

device_printf(hwp_device, "Failed to set EPP to %u", val); sc->req, NULL);

goto end;

} }

end:

thread_lock(curthread);

sched_unbind(curthread);

thread_unlock(curthread);

return (success);

}

static int static int

sysctl_epp_select(SYSCTL_HANDLER_ARGS) sysctl_epp_select(SYSCTL_HANDLER_ARGS)

{ {

device_t dev, hwp_dev; device_t dev, hwp_dev;

devclass_t dc;

struct hwpstate_softc *sc; struct hwpstate_softc *sc;

const uint32_t max_energy_perf = const uint32_t max_energy_perf =

BITS_VALUE(AMD_CPPC_REQUEST_ENERGY_PERF_BITS, (uint64_t)-1); BITS_VALUE(AMD_CPPC_REQUEST_ENERGY_PERF_BITS, (uint64_t)-1);

devclass_t dc;

uint32_t val; uint32_t val;

int ret = 0; int ret = 0;

int cpu; int cpu;

dev = oidp->oid_arg1; dev = oidp->oid_arg1;

sc = device_get_softc(dev); sc = device_get_softc(dev);

if (!(sc->flags & PSTATE_CPPC)) if (!(sc->flags & PSTATE_CPPC))

▲ Show 20 Lines • Show All 253 Lines • ▼ Show 20 Lines hwpstate_identify(driver_t *driver, device_t parent)

if (resource_disabled(HWP_AMD_CLASSNAME, 0)) if (resource_disabled(HWP_AMD_CLASSNAME, 0))

return; return;

if (BUS_ADD_CHILD(parent, 10, HWP_AMD_CLASSNAME, if (BUS_ADD_CHILD(parent, 10, HWP_AMD_CLASSNAME,

device_get_unit(parent)) == NULL) device_get_unit(parent)) == NULL)

device_printf(parent, "hwpstate: add child failed\n"); device_printf(parent, "hwpstate: add child failed\n");

} }

static int struct amd_set_autonomous_hwp_request {

amd_set_autonomous_hwp(struct hwpstate_softc *sc) device_t dev;

int res;

};

static void

amd_set_autonomous_hwp_cb(void *args)

{ {

struct pcpu *pc; struct hwpstate_softc *sc;

struct amd_set_autonomous_hwp_request *req =

(struct amd_set_autonomous_hwp_request *)args;

device_t dev; device_t dev;

uint64_t caps; uint64_t caps;

int ret; int ret;

dev = sc->dev; dev = req->dev;

pc = cpu_get_pcpu(dev); sc = device_get_softc(dev);

if (pc == NULL)

return (ENXIO);

thread_lock(curthread);

sched_bind(curthread, pc->pc_cpuid);

thread_unlock(curthread);

ret = wrmsr_safe(MSR_AMD_CPPC_ENABLE, 1); ret = wrmsr_safe(MSR_AMD_CPPC_ENABLE, 1);

if (ret != 0) { if (ret != 0) {

device_printf(dev, "Failed to enable cppc for cpu%d (%d)\n", device_printf(dev, "Failed to enable cppc for cpu%d (%d)\n",

pc->pc_cpuid, ret); curcpu, ret);

goto out; req->res = ret;

} }

ret = rdmsr_safe(MSR_AMD_CPPC_REQUEST, &sc->req); ret = rdmsr_safe(MSR_AMD_CPPC_REQUEST, &sc->req);

if (ret != 0) { if (ret != 0) {

device_printf(dev, device_printf(dev,

"Failed to read CPPC request MSR for cpu%d (%d)\n", "Failed to read CPPC request MSR for cpu%d (%d)\n", curcpu,

pc->pc_cpuid, ret); ret);

goto out; req->res = ret;

} }

ret = rdmsr_safe(MSR_AMD_CPPC_CAPS_1, &caps); ret = rdmsr_safe(MSR_AMD_CPPC_CAPS_1, &caps);

if (ret != 0) { if (ret != 0) {

device_printf(dev, device_printf(dev,

"Failed to read HWP capabilities MSR for cpu%d (%d)\n", "Failed to read HWP capabilities MSR for cpu%d (%d)\n",

pc->pc_cpuid, ret); curcpu, ret);

goto out; req->res = ret;

return;

} }

/* /*

* In Intel's reference manual, the default value of EPP is 0x80u which * In Intel's reference manual, the default value of EPP is 0x80u which

* is the balanced mode. For consistency, we set the same value in AMD's * is the balanced mode. For consistency, we set the same value in AMD's

* CPPC driver. * CPPC driver.

*/ */

SET_BITS_VALUE(sc->req, AMD_CPPC_REQUEST_ENERGY_PERF_BITS, 0x80); SET_BITS_VALUE(sc->req, AMD_CPPC_REQUEST_ENERGY_PERF_BITS, 0x80);

SET_BITS_VALUE(sc->req, AMD_CPPC_REQUEST_MIN_PERF_BITS, SET_BITS_VALUE(sc->req, AMD_CPPC_REQUEST_MIN_PERF_BITS,

BITS_VALUE(AMD_CPPC_CAPS_1_LOW_PERF_BITS, caps)); BITS_VALUE(AMD_CPPC_CAPS_1_LOW_PERF_BITS, caps));

SET_BITS_VALUE(sc->req, AMD_CPPC_REQUEST_MAX_PERF_BITS, SET_BITS_VALUE(sc->req, AMD_CPPC_REQUEST_MAX_PERF_BITS,

BITS_VALUE(AMD_CPPC_CAPS_1_HIGH_PERF_BITS, caps)); BITS_VALUE(AMD_CPPC_CAPS_1_HIGH_PERF_BITS, caps));

/* enable autonomous mode by setting desired performance to 0 */ /* enable autonomous mode by setting desired performance to 0 */

SET_BITS_VALUE(sc->req, AMD_CPPC_REQUEST_DES_PERF_BITS, 0); SET_BITS_VALUE(sc->req, AMD_CPPC_REQUEST_DES_PERF_BITS, 0);

ret = wrmsr_safe(MSR_AMD_CPPC_REQUEST, sc->req); ret = wrmsr_safe(MSR_AMD_CPPC_REQUEST, sc->req);

if (ret) { if (ret) {

device_printf(dev, device_printf(dev, "Failed to setup autonomous HWP for cpu%d\n",

"Failed to setup autonomous HWP for cpu%d\n", curcpu);

pc->pc_cpuid); req->res = ret;

goto out; return;

} }

out: req->res = 0;

thread_lock(curthread); }

sched_unbind(curthread);

thread_unlock(curthread);

return (ret); static int

amd_set_autonomous_hwp(struct hwpstate_softc *sc)

{

struct amd_set_autonomous_hwp_request req;

device_t dev;

dev = sc->dev;

req.dev = dev;

smp_rendezvous_cpu(cpu_get_pcpu(dev)->pc_cpuid,

smp_no_rendezvous_barrier, amd_set_autonomous_hwp_cb,

smp_no_rendezvous_barrier, &req);

return (req.res);

} }

static int static int

hwpstate_probe(device_t dev) hwpstate_probe(device_t dev)

{ {

struct hwpstate_softc *sc; struct hwpstate_softc *sc;

device_t perf_dev; device_t perf_dev;

uint64_t msr; uint64_t msr;

▲ Show 20 Lines • Show All 265 Lines • Show Last 20 Lines