Re: [RFC PATCH v3 6/7] virtio_rtc: Add Arm Generic Timer cross-timestamping

Peter Hilber
2024-06-20T12:07:28Z

Changing virtio-dev address to the new one.

On 15.06.24 09:50, David Woodhouse wrote:
> On Mon, 2023-12-18 at 08:38 +0100, Peter Hilber wrote:
>>
>> +int viortc_hw_xtstamp_params(u16 *hw_counter, enum clocksource_ids *cs_id)
>> +{
>> + *hw_counter = VIRTIO_RTC_COUNTER_ARM_VIRT;
>
> Hm, but what if it isn't? I think you need to put this in
> drivers/clocksource/arm_arch_timer.c where it can do something like
> kvm_arch_ptp_get_crosststamp() does to decide:
>
> if (arch_timer_uses_ppi == ARCH_TIMER_VIRT_PPI)
> ptp_counter = KVM_PTP_VIRT_COUNTER;
> else
> ptp_counter = KVM_PTP_PHYS_COUNTER;
>
>
>> + *cs_id = CSID_ARM_ARCH_COUNTER;
>> +
>> + return 0;
>> +}
>

I had such a check in v1, but Marc Zyngier didn't like the distinction [1,
2] - maybe also exposing the timer through a generic helper was not
desired. Quoting from [2]:

>> This was the rationale to come up with the physical/virtual counter
>> distinction for the Virtio RTC device. Looking at extensions such as
>> FEAT_ECV, where the CNTPCT_EL0 value can depend on the EL, or FEAT_NV*,
>> it might be a bit simplistic.
>
> Not just simplistic. It doesn't make sense. For this to work, you'd
> need to know the global offset that KVM applies to the global counter,
> plus the *virtualised* CNTPOFF/CNTVOFF values that the guest can
> change at any time on a *per-CPU* basis. None of that is available
> outside of KVM, nor would it make any sense anyway.
>
>> Does this physical/virtual counter distinction sound like a good idea?
>> Otherwise I would drop the arch_timer_counter_get_type() in the next
>> iteration.
>
> My take on this is that only the global counter value makes any sense.
> That value is already available from the host as the virtual counter,
> because we guarantee that CNTVOFF is 0 when outside of the guest
> (well, technically, outside of the vcpu_load/vcpu_put section).

So I put the assumption that the virtual counter will always be the right
choice for current Linux kernels, from the Virtio device POV.

Thanks for the comment,

Peter

[1] https://lore.kernel.org/lkml/20230630171052.985577-1-peter.hilber@opensynergy.com/T/#ma8d596de1cbc8f4a78a18b2aa995db18423494a7
[2] https://lore.kernel.org/lkml/20230630171052.985577-1-peter.hilber@opensynergy.com/T/#m65fa1d715933360498c4e33d7225e4220215a9d6