[PATCH] arm64: Restore trapless ptimer access

[PATCH] arm64: Restore trapless ptimer access

[Colton Lewis]

Due to recent KVM changes, QEMU is setting a ptimer offset resulting
in unintended trap and emulate access and a consequent performance
hit. Filter out the PTIMER_CNT register to restore trapless ptimer
access.

Quoting Andrew Jones:

Simply reading the CNT register and writing back the same value is
enough to set an offset, since the timer will have certainly moved
past whatever value was read by the time it's written.  QEMU
frequently saves and restores all registers in the get-reg-list array,
unless they've been explicitly filtered out (with Linux commit
680232a94c12, KVM_REG_ARM_PTIMER_CNT is now in the array). So, to
restore trapless ptimer accesses, we need a QEMU patch to filter out
the register.

See
https://lore.kernel.org/kvmarm/gsntttsonus5.fsf@coltonlewis-kvm.c.googlers.com/T/#m0770023762a821db2a3f0dd0a7dc6aa54e0d0da9
for additional context.

Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
 target/arm/kvm64.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 4d904a1d11..2dd46e0a99 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -672,6 +672,7 @@ typedef struct CPRegStateLevel {
  */
 static const CPRegStateLevel non_runtime_cpregs[] = {
     { KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE },
+    { KVM_REG_ARM_PTIMER_CNT, KVM_PUT_FULL_STATE },
 };
 
 int kvm_arm_cpreg_level(uint64_t regidx)
-- 
2.42.0.283.g2d96d420d3-goog

Re: [PATCH] arm64: Restore trapless ptimer access

[Richard Henderson]

On 8/31/23 12:00, Colton Lewis wrote:
> Due to recent KVM changes, QEMU is setting a ptimer offset resulting
> in unintended trap and emulate access and a consequent performance
> hit. Filter out the PTIMER_CNT register to restore trapless ptimer
> access.
> 
> Quoting Andrew Jones:
> 
> Simply reading the CNT register and writing back the same value is
> enough to set an offset, since the timer will have certainly moved
> past whatever value was read by the time it's written.  QEMU
> frequently saves and restores all registers in the get-reg-list array,
> unless they've been explicitly filtered out (with Linux commit
> 680232a94c12, KVM_REG_ARM_PTIMER_CNT is now in the array). So, to
> restore trapless ptimer accesses, we need a QEMU patch to filter out
> the register.
> 
> See
> https://lore.kernel.org/kvmarm/gsntttsonus5.fsf@coltonlewis-kvm.c.googlers.com/T/#m0770023762a821db2a3f0dd0a7dc6aa54e0d0da9
> for additional context.
> 
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>

Cc: qemu-stable@nongnu.org
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>



r~

> ---
>   target/arm/kvm64.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 4d904a1d11..2dd46e0a99 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -672,6 +672,7 @@ typedef struct CPRegStateLevel {
>    */
>   static const CPRegStateLevel non_runtime_cpregs[] = {
>       { KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE },
> +    { KVM_REG_ARM_PTIMER_CNT, KVM_PUT_FULL_STATE },
>   };
>   
>   int kvm_arm_cpreg_level(uint64_t regidx)

Re: [PATCH] arm64: Restore trapless ptimer access

[Andrew Jones]

On Thu, Aug 31, 2023 at 07:00:52PM +0000, Colton Lewis wrote:
> Due to recent KVM changes, QEMU is setting a ptimer offset resulting
> in unintended trap and emulate access and a consequent performance
> hit. Filter out the PTIMER_CNT register to restore trapless ptimer
> access.
> 
> Quoting Andrew Jones:
> 
> Simply reading the CNT register and writing back the same value is
> enough to set an offset, since the timer will have certainly moved
> past whatever value was read by the time it's written.  QEMU
> frequently saves and restores all registers in the get-reg-list array,
> unless they've been explicitly filtered out (with Linux commit
> 680232a94c12, KVM_REG_ARM_PTIMER_CNT is now in the array). So, to
> restore trapless ptimer accesses, we need a QEMU patch to filter out
> the register.
> 
> See
> https://lore.kernel.org/kvmarm/gsntttsonus5.fsf@coltonlewis-kvm.c.googlers.com/T/#m0770023762a821db2a3f0dd0a7dc6aa54e0d0da9

The link can be shorter with

https://lore.kernel.org/all/20230823200408.1214332-1-coltonlewis@google.com/

> for additional context.
> 
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>

Thanks for the testing and posting, Colton. Please add your s-o-b and a
Tested-by tag as well.

Thanks,
drew

> ---
>  target/arm/kvm64.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 4d904a1d11..2dd46e0a99 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -672,6 +672,7 @@ typedef struct CPRegStateLevel {
>   */
>  static const CPRegStateLevel non_runtime_cpregs[] = {
>      { KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE },
> +    { KVM_REG_ARM_PTIMER_CNT, KVM_PUT_FULL_STATE },
>  };
>  
>  int kvm_arm_cpreg_level(uint64_t regidx)
> -- 
> 2.42.0.283.g2d96d420d3-goog
> 

Re: [PATCH] arm64: Restore trapless ptimer access

[Colton Lewis]

On Fri, Sep 01, 2023 at 09:35:47AM +0200, Andrew Jones wrote:
> On Thu, Aug 31, 2023 at 07:00:52PM +0000, Colton Lewis wrote:
> > Due to recent KVM changes, QEMU is setting a ptimer offset resulting
> > in unintended trap and emulate access and a consequent performance
> > hit. Filter out the PTIMER_CNT register to restore trapless ptimer
> > access.
> >
> > Quoting Andrew Jones:
> >
> > Simply reading the CNT register and writing back the same value is
> > enough to set an offset, since the timer will have certainly moved
> > past whatever value was read by the time it's written.  QEMU
> > frequently saves and restores all registers in the get-reg-list array,
> > unless they've been explicitly filtered out (with Linux commit
> > 680232a94c12, KVM_REG_ARM_PTIMER_CNT is now in the array). So, to
> > restore trapless ptimer accesses, we need a QEMU patch to filter out
> > the register.
> >
> > See
> > https://lore.kernel.org/kvmarm/gsntttsonus5.fsf@coltonlewis-kvm.c.googlers.com/T/#m0770023762a821db2a3f0dd0a7dc6aa54e0d0da9
>
> The link can be shorter with
>
> https://lore.kernel.org/all/20230823200408.1214332-1-coltonlewis@google.com/

I will keep that in mind next time.

> > for additional context.
> >
> > Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
>
> Thanks for the testing and posting, Colton. Please add your s-o-b and a
> Tested-by tag as well.

Assuming it is sufficient to add here instead of reposting the whole patch:

Signed-off-by: Colton Lewis <coltonlewis@google.com>
Tested-by: Colton Lewis <coltonlewis@google.com>

> > ---
> >  target/arm/kvm64.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> > index 4d904a1d11..2dd46e0a99 100644
> > --- a/target/arm/kvm64.c
> > +++ b/target/arm/kvm64.c
> > @@ -672,6 +672,7 @@ typedef struct CPRegStateLevel {
> >   */
> >  static const CPRegStateLevel non_runtime_cpregs[] = {
> >      { KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE },
> > +    { KVM_REG_ARM_PTIMER_CNT, KVM_PUT_FULL_STATE },
> >  };
> >
> >  int kvm_arm_cpreg_level(uint64_t regidx)
> > --
> > 2.42.0.283.g2d96d420d3-goog
> >

Re: [PATCH] arm64: Restore trapless ptimer access

[Claudio Fontana]

Hi,

I think this discussion from ~2015 could potentially be be historically relevant for context,
at the time we had the problem with CNTVOFF IIRC so KVM_REG_ARM_TIMER_CNT being read and rewritten causing time warps in the guest:

https://patchwork.kernel.org/project/linux-arm-kernel/patch/1435157697-28579-1-git-send-email-marc.zyngier@arm.com/

I could not remember or find if/where the problem was fixed in the end in QEMU,

Ciao,

Claudio

On 9/1/23 21:23, Colton Lewis wrote:
> On Fri, Sep 01, 2023 at 09:35:47AM +0200, Andrew Jones wrote:
>> On Thu, Aug 31, 2023 at 07:00:52PM +0000, Colton Lewis wrote:
>>> Due to recent KVM changes, QEMU is setting a ptimer offset resulting
>>> in unintended trap and emulate access and a consequent performance
>>> hit. Filter out the PTIMER_CNT register to restore trapless ptimer
>>> access.
>>>
>>> Quoting Andrew Jones:
>>>
>>> Simply reading the CNT register and writing back the same value is
>>> enough to set an offset, since the timer will have certainly moved
>>> past whatever value was read by the time it's written.  QEMU
>>> frequently saves and restores all registers in the get-reg-list array,
>>> unless they've been explicitly filtered out (with Linux commit
>>> 680232a94c12, KVM_REG_ARM_PTIMER_CNT is now in the array). So, to
>>> restore trapless ptimer accesses, we need a QEMU patch to filter out
>>> the register.
>>>
>>> See
>>> https://lore.kernel.org/kvmarm/gsntttsonus5.fsf@coltonlewis-kvm.c.googlers.com/T/#m0770023762a821db2a3f0dd0a7dc6aa54e0d0da9
>>
>> The link can be shorter with
>>
>> https://lore.kernel.org/all/20230823200408.1214332-1-coltonlewis@google.com/
> 
> I will keep that in mind next time.
> 
>>> for additional context.
>>>
>>> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
>>
>> Thanks for the testing and posting, Colton. Please add your s-o-b and a
>> Tested-by tag as well.
> 
> Assuming it is sufficient to add here instead of reposting the whole patch:
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> Tested-by: Colton Lewis <coltonlewis@google.com>
> 
>>> ---
>>>  target/arm/kvm64.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
>>> index 4d904a1d11..2dd46e0a99 100644
>>> --- a/target/arm/kvm64.c
>>> +++ b/target/arm/kvm64.c
>>> @@ -672,6 +672,7 @@ typedef struct CPRegStateLevel {
>>>   */
>>>  static const CPRegStateLevel non_runtime_cpregs[] = {
>>>      { KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE },
>>> +    { KVM_REG_ARM_PTIMER_CNT, KVM_PUT_FULL_STATE },
>>>  };
>>>
>>>  int kvm_arm_cpreg_level(uint64_t regidx)
>>> --
>>> 2.42.0.283.g2d96d420d3-goog
>>>
> 

Re: [PATCH] arm64: Restore trapless ptimer access

[Andrew Jones]

On Mon, Sep 04, 2023 at 10:18:05AM +0200, Claudio Fontana wrote:
> Hi,
> 
> I think this discussion from ~2015 could potentially be be historically relevant for context,
> at the time we had the problem with CNTVOFF IIRC so KVM_REG_ARM_TIMER_CNT being read and rewritten causing time warps in the guest:
> 
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/1435157697-28579-1-git-send-email-marc.zyngier@arm.com/
> 
> I could not remember or find if/where the problem was fixed in the end in QEMU,

It's most likely commit 4b7a6bf402bd ("target-arm: kvm: Differentiate
registers based on write-back levels")

Thanks,
drew

> 
> Ciao,
> 
> Claudio
> 
> On 9/1/23 21:23, Colton Lewis wrote:
> > On Fri, Sep 01, 2023 at 09:35:47AM +0200, Andrew Jones wrote:
> >> On Thu, Aug 31, 2023 at 07:00:52PM +0000, Colton Lewis wrote:
> >>> Due to recent KVM changes, QEMU is setting a ptimer offset resulting
> >>> in unintended trap and emulate access and a consequent performance
> >>> hit. Filter out the PTIMER_CNT register to restore trapless ptimer
> >>> access.
> >>>
> >>> Quoting Andrew Jones:
> >>>
> >>> Simply reading the CNT register and writing back the same value is
> >>> enough to set an offset, since the timer will have certainly moved
> >>> past whatever value was read by the time it's written.  QEMU
> >>> frequently saves and restores all registers in the get-reg-list array,
> >>> unless they've been explicitly filtered out (with Linux commit
> >>> 680232a94c12, KVM_REG_ARM_PTIMER_CNT is now in the array). So, to
> >>> restore trapless ptimer accesses, we need a QEMU patch to filter out
> >>> the register.
> >>>
> >>> See
> >>> https://lore.kernel.org/kvmarm/gsntttsonus5.fsf@coltonlewis-kvm.c.googlers.com/T/#m0770023762a821db2a3f0dd0a7dc6aa54e0d0da9
> >>
> >> The link can be shorter with
> >>
> >> https://lore.kernel.org/all/20230823200408.1214332-1-coltonlewis@google.com/
> > 
> > I will keep that in mind next time.
> > 
> >>> for additional context.
> >>>
> >>> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> >>
> >> Thanks for the testing and posting, Colton. Please add your s-o-b and a
> >> Tested-by tag as well.
> > 
> > Assuming it is sufficient to add here instead of reposting the whole patch:
> > 
> > Signed-off-by: Colton Lewis <coltonlewis@google.com>
> > Tested-by: Colton Lewis <coltonlewis@google.com>
> > 
> >>> ---
> >>>  target/arm/kvm64.c | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> >>> index 4d904a1d11..2dd46e0a99 100644
> >>> --- a/target/arm/kvm64.c
> >>> +++ b/target/arm/kvm64.c
> >>> @@ -672,6 +672,7 @@ typedef struct CPRegStateLevel {
> >>>   */
> >>>  static const CPRegStateLevel non_runtime_cpregs[] = {
> >>>      { KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE },
> >>> +    { KVM_REG_ARM_PTIMER_CNT, KVM_PUT_FULL_STATE },
> >>>  };
> >>>
> >>>  int kvm_arm_cpreg_level(uint64_t regidx)
> >>> --
> >>> 2.42.0.283.g2d96d420d3-goog
> >>>
> > 
> 

Re: [PATCH] arm64: Restore trapless ptimer access

[Claudio Fontana]

On 9/4/23 13:07, Andrew Jones wrote:
> On Mon, Sep 04, 2023 at 10:18:05AM +0200, Claudio Fontana wrote:
>> Hi,
>>
>> I think this discussion from ~2015 could potentially be be historically relevant for context,
>> at the time we had the problem with CNTVOFF IIRC so KVM_REG_ARM_TIMER_CNT being read and rewritten causing time warps in the guest:
>>
>> https://patchwork.kernel.org/project/linux-arm-kernel/patch/1435157697-28579-1-git-send-email-marc.zyngier@arm.com/
>>
>> I could not remember or find if/where the problem was fixed in the end in QEMU,
> 
> It's most likely commit 4b7a6bf402bd ("target-arm: kvm: Differentiate
> registers based on write-back levels")

Indeed, thanks!

C

> Thanks,
> drew
> 
>>
>> Ciao,
>>
>> Claudio
>>
>> On 9/1/23 21:23, Colton Lewis wrote:
>>> On Fri, Sep 01, 2023 at 09:35:47AM +0200, Andrew Jones wrote:
>>>> On Thu, Aug 31, 2023 at 07:00:52PM +0000, Colton Lewis wrote:
>>>>> Due to recent KVM changes, QEMU is setting a ptimer offset resulting
>>>>> in unintended trap and emulate access and a consequent performance
>>>>> hit. Filter out the PTIMER_CNT register to restore trapless ptimer
>>>>> access.
>>>>>
>>>>> Quoting Andrew Jones:
>>>>>
>>>>> Simply reading the CNT register and writing back the same value is
>>>>> enough to set an offset, since the timer will have certainly moved
>>>>> past whatever value was read by the time it's written.  QEMU
>>>>> frequently saves and restores all registers in the get-reg-list array,
>>>>> unless they've been explicitly filtered out (with Linux commit
>>>>> 680232a94c12, KVM_REG_ARM_PTIMER_CNT is now in the array). So, to
>>>>> restore trapless ptimer accesses, we need a QEMU patch to filter out
>>>>> the register.
>>>>>
>>>>> See
>>>>> https://lore.kernel.org/kvmarm/gsntttsonus5.fsf@coltonlewis-kvm.c.googlers.com/T/#m0770023762a821db2a3f0dd0a7dc6aa54e0d0da9
>>>>
>>>> The link can be shorter with
>>>>
>>>> https://lore.kernel.org/all/20230823200408.1214332-1-coltonlewis@google.com/
>>>
>>> I will keep that in mind next time.
>>>
>>>>> for additional context.
>>>>>
>>>>> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
>>>>
>>>> Thanks for the testing and posting, Colton. Please add your s-o-b and a
>>>> Tested-by tag as well.
>>>
>>> Assuming it is sufficient to add here instead of reposting the whole patch:
>>>
>>> Signed-off-by: Colton Lewis <coltonlewis@google.com>
>>> Tested-by: Colton Lewis <coltonlewis@google.com>
>>>
>>>>> ---
>>>>>  target/arm/kvm64.c | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
>>>>> index 4d904a1d11..2dd46e0a99 100644
>>>>> --- a/target/arm/kvm64.c
>>>>> +++ b/target/arm/kvm64.c
>>>>> @@ -672,6 +672,7 @@ typedef struct CPRegStateLevel {
>>>>>   */
>>>>>  static const CPRegStateLevel non_runtime_cpregs[] = {
>>>>>      { KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE },
>>>>> +    { KVM_REG_ARM_PTIMER_CNT, KVM_PUT_FULL_STATE },
>>>>>  };
>>>>>
>>>>>  int kvm_arm_cpreg_level(uint64_t regidx)
>>>>> --
>>>>> 2.42.0.283.g2d96d420d3-goog
>>>>>
>>>
>>

Re: [PATCH] arm64: Restore trapless ptimer access

[Michael Tokarev]

31.08.2023 22:00, Colton Lewis wrote:
> Due to recent KVM changes, QEMU is setting a ptimer offset resulting
> in unintended trap and emulate access and a consequent performance
> hit. Filter out the PTIMER_CNT register to restore trapless ptimer
> access.
> 
> Quoting Andrew Jones:
> 
> Simply reading the CNT register and writing back the same value is
> enough to set an offset, since the timer will have certainly moved
> past whatever value was read by the time it's written.  QEMU
> frequently saves and restores all registers in the get-reg-list array,
> unless they've been explicitly filtered out (with Linux commit
> 680232a94c12, KVM_REG_ARM_PTIMER_CNT is now in the array). So, to
> restore trapless ptimer accesses, we need a QEMU patch to filter out
> the register.
> 
> See
> https://lore.kernel.org/kvmarm/gsntttsonus5.fsf@coltonlewis-kvm.c.googlers.com/T/#m0770023762a821db2a3f0dd0a7dc6aa54e0d0da9
> for additional context.
> 
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> ---
>   target/arm/kvm64.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 4d904a1d11..2dd46e0a99 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -672,6 +672,7 @@ typedef struct CPRegStateLevel {
>    */
>   static const CPRegStateLevel non_runtime_cpregs[] = {
>       { KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE },
> +    { KVM_REG_ARM_PTIMER_CNT, KVM_PUT_FULL_STATE },
>   };
>   
>   int kvm_arm_cpreg_level(uint64_t regidx)

While this patch itself is one-liner and trivial and all, I'd rather
not apply this to the trivial-patches tree, - it requires a little
bit more than trivial expertise in this area.

So basically, ping for qemu-arm@ ? :)

Thanks,

/mjt

Re: [PATCH] arm64: Restore trapless ptimer access

[Andrew Jones]

On Thu, Sep 07, 2023 at 10:31:20PM +0300, Michael Tokarev wrote:
> 31.08.2023 22:00, Colton Lewis wrote:
> > Due to recent KVM changes, QEMU is setting a ptimer offset resulting
> > in unintended trap and emulate access and a consequent performance
> > hit. Filter out the PTIMER_CNT register to restore trapless ptimer
> > access.
> > 
> > Quoting Andrew Jones:
> > 
> > Simply reading the CNT register and writing back the same value is
> > enough to set an offset, since the timer will have certainly moved
> > past whatever value was read by the time it's written.  QEMU
> > frequently saves and restores all registers in the get-reg-list array,
> > unless they've been explicitly filtered out (with Linux commit
> > 680232a94c12, KVM_REG_ARM_PTIMER_CNT is now in the array). So, to
> > restore trapless ptimer accesses, we need a QEMU patch to filter out
> > the register.
> > 
> > See
> > https://lore.kernel.org/kvmarm/gsntttsonus5.fsf@coltonlewis-kvm.c.googlers.com/T/#m0770023762a821db2a3f0dd0a7dc6aa54e0d0da9
> > for additional context.
> > 
> > Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> > ---
> >   target/arm/kvm64.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> > index 4d904a1d11..2dd46e0a99 100644
> > --- a/target/arm/kvm64.c
> > +++ b/target/arm/kvm64.c
> > @@ -672,6 +672,7 @@ typedef struct CPRegStateLevel {
> >    */
> >   static const CPRegStateLevel non_runtime_cpregs[] = {
> >       { KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE },
> > +    { KVM_REG_ARM_PTIMER_CNT, KVM_PUT_FULL_STATE },
> >   };
> >   int kvm_arm_cpreg_level(uint64_t regidx)
> 
> While this patch itself is one-liner and trivial and all, I'd rather
> not apply this to the trivial-patches tree, - it requires a little
> bit more than trivial expertise in this area.
> 
> So basically, ping for qemu-arm@ ? :)
>

I agree that qemu-trivial should not have been CC'ed for this patch.

Thanks,
drew

Re: [PATCH] arm64: Restore trapless ptimer access

[Peter Maydell]

On Thu, 7 Sept 2023 at 20:31, Michael Tokarev <mjt@tls.msk.ru> wrote:
>
> 31.08.2023 22:00, Colton Lewis wrote:
> > Due to recent KVM changes, QEMU is setting a ptimer offset resulting
> > in unintended trap and emulate access and a consequent performance
> > hit. Filter out the PTIMER_CNT register to restore trapless ptimer
> > access.
> >
> > Quoting Andrew Jones:
> >
> > Simply reading the CNT register and writing back the same value is
> > enough to set an offset, since the timer will have certainly moved
> > past whatever value was read by the time it's written.  QEMU
> > frequently saves and restores all registers in the get-reg-list array,
> > unless they've been explicitly filtered out (with Linux commit
> > 680232a94c12, KVM_REG_ARM_PTIMER_CNT is now in the array). So, to
> > restore trapless ptimer accesses, we need a QEMU patch to filter out
> > the register.
> >
> > See
> > https://lore.kernel.org/kvmarm/gsntttsonus5.fsf@coltonlewis-kvm.c.googlers.com/T/#m0770023762a821db2a3f0dd0a7dc6aa54e0d0da9
> > for additional context.
> >
> > Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> > ---
> >   target/arm/kvm64.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> > index 4d904a1d11..2dd46e0a99 100644
> > --- a/target/arm/kvm64.c
> > +++ b/target/arm/kvm64.c
> > @@ -672,6 +672,7 @@ typedef struct CPRegStateLevel {
> >    */
> >   static const CPRegStateLevel non_runtime_cpregs[] = {
> >       { KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE },
> > +    { KVM_REG_ARM_PTIMER_CNT, KVM_PUT_FULL_STATE },
> >   };
> >
> >   int kvm_arm_cpreg_level(uint64_t regidx)
>
> While this patch itself is one-liner and trivial and all, I'd rather
> not apply this to the trivial-patches tree, - it requires a little
> bit more than trivial expertise in this area.
>
> So basically, ping for qemu-arm@ ? :)

It is on my to-review/apply queue, yes.

thanks
-- PMM

Re: [PATCH] arm64: Restore trapless ptimer access

[Peter Maydell]

On Thu, 31 Aug 2023 at 20:01, Colton Lewis <coltonlewis@google.com> wrote:
>
> Due to recent KVM changes, QEMU is setting a ptimer offset resulting
> in unintended trap and emulate access and a consequent performance
> hit. Filter out the PTIMER_CNT register to restore trapless ptimer
> access.
>
> Quoting Andrew Jones:
>
> Simply reading the CNT register and writing back the same value is
> enough to set an offset, since the timer will have certainly moved
> past whatever value was read by the time it's written.  QEMU
> frequently saves and restores all registers in the get-reg-list array,
> unless they've been explicitly filtered out (with Linux commit
> 680232a94c12, KVM_REG_ARM_PTIMER_CNT is now in the array). So, to
> restore trapless ptimer accesses, we need a QEMU patch to filter out
> the register.
>
> See
> https://lore.kernel.org/kvmarm/gsntttsonus5.fsf@coltonlewis-kvm.c.googlers.com/T/#m0770023762a821db2a3f0dd0a7dc6aa54e0d0da9
> for additional context.
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> ---



Applied to target-arm.next, thanks.

-- PMM