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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 20 Jun 2024 14:06:32 +0200
From: Peter Hilber <peter.hilber@...nsynergy.com>
To: David Woodhouse <dwmw2@...radead.org>, linux-kernel@...r.kernel.org,
 virtualization@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
 virtio-dev@...ts.linux.dev
Cc: "Michael S. Tsirkin" <mst@...hat.com>, Jason Wang <jasowang@...hat.com>,
 Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Marc Zyngier <maz@...nel.org>,
 Mark Rutland <mark.rutland@....com>,
 Daniel Lezcano <daniel.lezcano@...aro.org>,
 Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [RFC PATCH v3 6/7] virtio_rtc: Add Arm Generic Timer
 cross-timestamping

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