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.

Call it req for consistency with other places?

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

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.

SYSCTL_HANDLER_ARGS have already used req as variable name.

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.

Indentation looks wrong here.

This should really be added in a separate patch.

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

This has been in a seperate commit. I will make another differnetial tomorrow.

Inverted tests here.

Extra newline

Extra newline

The return statement can be removed too.

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

Simplification after early init of ret above.

Bug: ret set before jumps to out is crushed.