[PATCH] virhostcpu: fix getting CPU freq for Apple Silicon

[Andrea Bolognani]

On Mon, Feb 07, 2022 at 04:48:06PM +0800, Menci wrote:
> The current implementation of virHostCPUGetInfo for macOS (__APPLE__)
> reads "hw.cpufrequency" from sysctl, which is available only on x86_64
> architecture.
>
> On Apple Silicon ARM Macs, it's not available:
>
>   $ sysctl hw.cpufrequency              # No output
>   $ arch -x86_64 sysctl hw.cpufrequency # Run with Rosetta 2
>   hw.cpufrequency: 2400000000
>
> When running libvirtd on Apple Silicon, I got the error:
>
>   cannot obtain CPU freq: No such file or directory.
>
> To fix it, we can calculate it with "hw.tbfrequency" and
> "kern.clockrate" instead:
>
>   $ sysctl hw.tbfrequency
>   hw.tbfrequency: 24000000
>   $ sysctl kern.clockrate
>   kern.clockrate: { hz = 100, tick = 10000, tickadj = 0, profhz = 100, stathz = 100 }
>
> The result value would be hw.tbfrequency * kern.clockrate.hz.
>
> Signed-off-by: Menci <huanghaorui301 at gmail.com>

The tag should be in the form

  Signed-off-by: Firstname Lastname <email at domain.tld>

Can you please update it, and your git authorship information too?

> +    /* This works for Intel Macs */
>      if (sysctlbyname("hw.cpufrequency", &cpu_freq, &cpu_freq_len, NULL, 0) < 0) {
> -        virReportSystemError(errno, "%s", _("cannot obtain CPU freq"));
> -        return -1;
> +        /* On Apple Silicon fallback to hw.tbfrequency and kern.clockrate.hz */
> +        struct clockinfo clockrate;
> +        size_t clockrate_len = sizeof(clockrate);
> +        if (sysctlbyname("hw.tbfrequency", &cpu_freq, &cpu_freq_len, NULL, 0) < 0 ||
> +            sysctlbyname("kern.clockrate", &clockrate, &clockrate_len, NULL, 0) < 0) {
> +            virReportSystemError(errno, "%s", _("cannot obtain CPU freq"));
> +            return -1;
> +        }
> +
> +        cpu_freq *= clockrate.hz;

Do you have any reference explaining why these specific values,
combined in this specific way, are going to result in the CPU
freqency? In the examples you provided in the commit message the
values seem to match up, but I'd like to see some stronger evidence.

Additionally, is there any reason why we wouldn't just use the new
set of sysctls regardless of architecture? Are those not available on
Intel?

One last thing to keep in mind: even when running on Linux, we don't
report the CPU frequency for Arm CPUs. This is explicitly allowed:

  struct virNodeInfo {
      ...
      unsigned int mhz;     /* expected CPU frequency, 0 if not known or
                               on unusual architectures */
      ...

So if there is no way to accurately detect the CPU frequency on Apple
Silicon, it would be perfectly fine to do something like

  if (sysctlbyname("hw.cpufrequency", &cpu_freq, &cpu_freq_len, NULL, 0) < 0) {
      if (errno == ENOENT) {
          cpu_freq = 0;
      } else {
          virReportSystemError(errno, "%s", _("cannot obtain CPU freq"));
          return -1;
      }
      ...

and simply report the fact that we couldn't figure out the CPU
frequency to the user.

-- 
Andrea Bolognani / Red Hat / Virtualization