On 26.03.2025 20:49, Oleksii Kurochko wrote:
> On 3/26/25 4:13 PM, Jan Beulich wrote:
>> On 25.03.2025 18:36, Oleksii Kurochko wrote:
>>> +/* Set up the timer on the boot CPU (early init function) */
>>> +static void __init preinit_dt_xen_time(void)
>>> +{
>>> + static const struct dt_device_match __initconstrel timer_ids[] =
>>> + {
>>> + DT_MATCH_PATH("/cpus"),
>>> + { /* sentinel */ },
>>> + };
>>> + uint32_t rate;
>>> +
>>> + timer = dt_find_matching_node(NULL, timer_ids);
>>> + if ( !timer )
>>> + panic("Unable to find a compatible timer in the device tree\n");
>>> +
>>> + dt_device_set_used_by(timer, DOMID_XEN);
>>> +
>>> + if ( !dt_property_read_u32(timer, "timebase-frequency", &rate) )
>>> + panic("Unable to find clock frequency\n");
>>> +
>>> + cpu_khz = rate / 1000;
>> "rate" being only 32 bits wide, what about clock rates above 4GHz? Or is
>> this some external clock running at a much lower rate than the CPU would?
>
> It is the frequency of the hardware timer that drives the
> |mtime|register, it defines the frequency (in Hz) at which the timer
> increments.
And that timer can't plausibly run at more than 4 GHz?
>>> +void __init preinit_xen_time(void)
>>> +{
>>> + preinit_dt_xen_time();
>>> +
>>> + xen_start_clock_cycles = get_cycles();
>>> +}
>> I take it that more stuff is going to be added to this function? Else I
>> wouldn't see why preinit_dt_xen_time() needed to be a separate function.
>
> Actually, I checked my latest downstream branch and I haven't added nothing
> extra to this function.
> Only one thing that I want to add is ACPI case:
> if ( acpi_disabled )
> preinit_dt_xen_time();
> else
> panic("TBD\n"); /* preinit_acpi_xen_time(); */
That's probably a good enough reason then to keep the function separate.
Jan
