hwpstate_intel: Use ipi instead of thread_lock + sched_bind

[olce]

Some minor stuff to change.

I also added some notes about pre-existing problems as reminders (not necessarily to be fixed in this change).

sys/x86/cpufreq/hwpstate_intel.c

5

Probably missing an additional line like:

Portions of this software were developed by ShengYi Hung <[e-mail]> under sponsorship from the FreeBSD Foundation.

?

140–142

Ideally, I'd like some fine-grained error reporting like I did for hwpstate_amd(4), especially as this is a debug knob. At least a single return value would already make a big difference.

150–153

Same as in D55604: Let's keep this debug dumper free from all cached value, and always re-read it from the MSR.

(The cached value is accessible from the sysctl knob.)

359–363

Side note: Pre-existing problem: There's nothing preventing a concurrent update of sc->req, and this operation is not atomic (note that, in hwpstate_amd(4), I've taken care of such situations by having read-updates execute on the target CPU in a critical section (actually, with interrupts disabled)).

430–431

Error is signed, so I'd rather have it in a separate field.

But I think it's even better to remove these macros completely. There is not need to pass an error, as *msr_safe() can only return 0 or EFAULT.

454

Pre-existing problem needing the same change as in hwpstate_amd(4): Once enabled, CPPC cannot be disabled, so we must not bail out on any error after the write to MSR_IA32_PM_ENABLE succeeded.

Mentioning this because, in this change, HWP_ERROR_CPPC_* are not set in an independent manner (i.e., when bailing out, bits for all steps not performed should also be set), which is fine for the time being, but will need to be changed when fixing the above-mentioned problem.

520–558

Some pre-existing problems: As a return value, we shouldn't return EFAULT but rather ENXIO on HWP_ERROR_CPPC_ENABLE, and 0 in *all* other cases (and the driver must cope with specific MSR errors once CPPC is enabled). For the second part, please see enable_cppc() in hwpstate_amd(4), in particular, commits bd58239d3653d and 7f36d7a9505a.

[aokblast]

Fixes

[aokblast]

sys/x86/cpufreq/hwpstate_intel.c

150–153

The values are not from the cached value already.

[olce]

sys/x86/cpufreq/hwpstate_intel.c

326

I'd read sc->req into req_cached under the mutex, and then perform the other operations outside of it.

326

Introducing a mutex seems a bit heavy-handed, simply using atomic ops would work, but that's OK.

This should be done in a separate commit.

360–364

Separate commit.

559

As said, we must return 0 here (as soon as CPPC has been activated, regardless of the errors that might happen after).

753–755

I'm fine with returning EFAULT in all cases (returning ENXIO in the case CPPC cannot be re-enabled might be preferable).

Anyway, callers of resume methods in general do not care about the returned value, except for very limited diagnostic printing.

[olce]

sys/x86/cpufreq/hwpstate_intel.c

150–153

Right, I must have been confused.

[aokblast]

Minor fixes