On Tue, 10 Aug 2021 10:44:01 +0100,
Oliver Upton [off-list ref] wrote:

On Tue, Aug 10, 2021 at 2:35 AM Marc Zyngier [off-list ref] wrote:

quoted

On Wed, 04 Aug 2021 09:58:11 +0100,
Oliver Upton [off-list ref] wrote:

quoted

Allow userspace to access the guest's virtual counter-timer offset
through the ONE_REG interface. The value read or written is defined to
be an offset from the guest's physical counter-timer. Add some
documentation to clarify how a VMM should use this and the existing
CNTVCT_EL0.

Signed-off-by: Oliver Upton <redacted>
---
 Documentation/virt/kvm/api.rst    | 10 ++++++++++
 arch/arm64/include/uapi/asm/kvm.h |  1 +
 arch/arm64/kvm/arch_timer.c       | 11 +++++++++++
 arch/arm64/kvm/guest.c            |  6 +++++-
 include/kvm/arm_arch_timer.h      |  1 +
 5 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 8d4a3471ad9e..28a65dc89985 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst

@@ -2487,6 +2487,16 @@ arm64 system registers have the following id bit patterns::
      derived from the register encoding for CNTV_CVAL_EL0.  As this is
      API, it must remain this way.

+.. warning::
+
+     The value of KVM_REG_ARM_TIMER_OFFSET is defined as an offset from
+     the guest's view of the physical counter-timer.
+
+     Userspace should use either KVM_REG_ARM_TIMER_OFFSET or
+     KVM_REG_ARM_TIMER_CVAL to pause and resume a guest's virtual

You probably mean KVM_REG_ARM_TIMER_CNT here, despite the broken
encoding.

Indeed I do!

quoted

quoted

+     counter-timer. Mixed use of these registers could result in an
+     unpredictable guest counter value.
+
 arm64 firmware pseudo-registers have the following bit pattern::

   0x6030 0000 0014 <regno:16>

diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index b3edde68bc3e..949a31bc10f0 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h

@@ -255,6 +255,7 @@ struct kvm_arm_copy_mte_tags {
 #define KVM_REG_ARM_TIMER_CTL                ARM64_SYS_REG(3, 3, 14, 3, 1)
 #define KVM_REG_ARM_TIMER_CVAL               ARM64_SYS_REG(3, 3, 14, 0, 2)
 #define KVM_REG_ARM_TIMER_CNT                ARM64_SYS_REG(3, 3, 14, 3, 2)
+#define KVM_REG_ARM_TIMER_OFFSET     ARM64_SYS_REG(3, 4, 14, 0, 3)

I don't think we can use the encoding for CNTPOFF_EL2 here, as it will
eventually clash with a NV guest using the same feature for its own
purpose. We don't want this offset to overlap with any of the existing
features.

I actually liked your previous proposal of controlling the physical
offset via a device property, as it clearly indicated that you were
dealing with non-architectural state.

That's actually exactly what I did here :) That said, the macro name
is horribly obfuscated from CNTVOFF_EL2. I did this for the sake of
symmetry with other virtual counter-timer registers above, though this
may warrant special casing given the fact that we have a similarly
named device attribute to handle the physical offset.

Gah, you are of course right. Ignore my rambling. The name is fine (or
at least in keeping with existing quality level of the making).

For the physical offset, something along the lines of
KVM_ARM_VCPU_TIMER_PHYS_OFFSET is probably right (but feel free to be
creative, I'm terrible at this stuff [1]).

Thanks,

	M.

[1] https://twitter.com/codinghorror/status/506010907021828096?lang=en

-- 
Without deviation from the norm, progress is not possible.