Re: [RFC PATCH v6 4/7] time: Add mechanism to recognize clocksource in time_get_snapshot

2019-11-06 Thread Thomas Gleixner
On Thu, 24 Oct 2019, Jianyong Wu wrote:
> From: Thomas Gleixner 
>
> In some scenario like return device time to ptp_kvm guest,
> we need identify the current clocksource outside core time code.
>
> This patch add a mechanism to recognize the current clocksource
> by export clocksource id in time_get_snapshot.

Please check Documentation/process/submitting-patches.rst and search for
'This patch'.

> diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
> index b21db536fd52..ac8016b22734 100644
> --- a/include/linux/clocksource.h
> +++ b/include/linux/clocksource.h
> @@ -19,6 +19,7 @@
>  #include 
>  #include 
>  #include 
> +#include 

Please place that include to the other linux includes. You might notice
that there is ordering here.

But where is that include? It's not part of that series, so how is this
supposed to compile?

Thanks,

tglx
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


RE: [RFC PATCH v6 0/7] Enable ptp_kvm for arm64

2019-11-06 Thread Jianyong Wu (Arm Technology China)
Hi all,

Any comments?

Thanks
Jianyong

> -Original Message-
> From: Jianyong Wu 
> Sent: Thursday, October 24, 2019 7:02 PM
> To: net...@vger.kernel.org; yangbo...@nxp.com; john.stu...@linaro.org;
> t...@linutronix.de; pbonz...@redhat.com; sean.j.christopher...@intel.com;
> m...@kernel.org; richardcoch...@gmail.com; Mark Rutland
> ; w...@kernel.org; Suzuki Poulose
> 
> Cc: linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> kvmarm@lists.cs.columbia.edu; k...@vger.kernel.org; Steve Capper
> ; Kaly Xin (Arm Technology China)
> ; Justin He (Arm Technology China)
> ; Jianyong Wu (Arm Technology China)
> ; nd 
> Subject: [RFC PATCH v6 0/7] Enable ptp_kvm for arm64
> 
> kvm ptp targets to provide high precision time sync between guest and host
> in virtualization environment. This patch enable kvm ptp for arm64.
> This patch set base on [1][2][3]
> 
> change log:
> from v5 to v6:
> (1) apply Mark's patch[4] to get SMCCC conduit.
> (2) add mechanism to recognize current clocksource by add
> clocksouce_id value into struct clocksource instead of method in patch-v5.
> (3) rename kvm_arch_ptp_get_clock_fn into
> kvm_arch_ptp_get_crosststamp.
> 
> from v4 to v5:
> (1) remove hvc delay compensasion as it should leave to userspace.
> (2) check current clocksource in hvc call service.
> (3) expose current clocksource by adding it to system_time_snapshot.
> (4) add helper to check if clocksource is arm_arch_counter.
> (5) rename kvm_ptp.c to ptp_kvm_common.c
> 
> from v3 to v4:
> (1) fix clocksource of ptp_kvm to arch_sys_counter.
> (2) move kvm_arch_ptp_get_clock_fn into arm_arch_timer.c
> (3) subtract cntvoff before return cycles from host.
> (4) use ktime_get_snapshot instead of getnstimeofday and
> get_current_counterval to return time and counter value.
> (5) split ktime and counter into two 32-bit block respectively to 
> avoid
> Y2038-safe issue.
> (6) set time compensation to device time as half of the delay of hvc 
> call.
> (7) add ARM_ARCH_TIMER as dependency of ptp_kvm for arm64.
> 
> from v2 to v3:
> (1) fix some issues in commit log.
> (2) add some receivers in send list.
> 
> from v1 to v2:
> (1) move arch-specific code from arch/ to driver/ptp/
> (2) offer mechanism to inform userspace if ptp_kvm service is 
> available.
> (3) separate ptp_kvm code for arm64 into hypervisor part and guest 
> part.
> (4) add API to expose monotonic clock and counter value.
> (5) refine code: remove no necessary part and reconsitution.
> 
> [1]https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/
> commit/?h=kvm/hvc&id=125ea89e4a21e2fc5235410f966a996a1a7148bf
> [2]https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/
> commit/?h=kvm/hvc&id=464f5a1741e5959c3e4d2be1966ae0093b4dce06
> [3]https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/
> commit/?h=kvm/hvc&id=6597490e005d0eeca8ed8c1c1d7b4318ee014681
> [4]https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/
> commit/?h=for-next/smccc-conduit-
> cleanup&id=6b7fe77c334ae59fed9500140e08f4f896b36871
> 
> Jianyong Wu (6):
>   psci: let arm_smccc_1_1_invoke available by modules
>   ptp: Reorganize ptp_kvm modules to make it arch-independent.
>   time: Add mechanism to recognize clocksource in time_get_snapshot
>   psci: Add hvc call service for ptp_kvm.
>   ptp: arm64: Enable ptp_kvm for arm64
>   kvm: arm64: Add capability check extension for ptp_kvm
> 
> Mark Rutland (1):
>   arm/arm64: smccc/psci: add arm_smccc_1_1_get_conduit()
> 
>  drivers/clocksource/arm_arch_timer.c| 24 ++
>  drivers/firmware/psci/psci.c| 16 
>  drivers/ptp/Kconfig |  2 +-
>  drivers/ptp/Makefile|  1 +
>  drivers/ptp/ptp_kvm_arm64.c | 53 +
>  drivers/ptp/{ptp_kvm.c => ptp_kvm_common.c} | 77 +-
>  drivers/ptp/ptp_kvm_x86.c   | 87 +
>  include/asm-generic/ptp_kvm.h   | 12 +++
>  include/clocksource/arm_arch_timer.h|  4 +
>  include/linux/arm-smccc.h   | 30 ++-
>  include/linux/clocksource.h |  6 ++
>  include/linux/timekeeping.h | 12 +--
>  include/uapi/linux/kvm.h|  1 +
>  kernel/time/clocksource.c   |  3 +
>  kernel/time/timekeeping.c   |  1 +
>  virt/kvm/arm/arm.c  |  1 +
>  virt/kvm/arm/psci.c | 22 ++
>  17 files changed, 286 insertions(+), 66 deletions(-)  create mode 100644
> drivers/ptp/ptp_kvm_arm64.c  rename drivers/ptp/{ptp_kvm.c =>
> ptp_kvm_common.c} (63%)  create mode 100644
> drivers/ptp/ptp_kvm_x86.c  create mode 100644 include/asm-
> generic/ptp_kvm.h
> 
> --
> 2.17.1

___
kvma

Re: [PATCH v2] KVM: arm64: Don't set HCR_EL2.TVM when S2FWB is supported

2019-11-06 Thread Christoffer Dall
Hi Alexandru,

On Wed, Nov 06, 2019 at 01:02:21PM +, Alexandru Elisei wrote:
> 
> On 10/28/19 1:05 PM, Christoffer Dall wrote:
> > On CPUs that support S2FWB (Armv8.4+), KVM configures the stage 2 page
> > tables to override the memory attributes of memory accesses, regardless
> > of the stage 1 page table configurations, and also when the stage 1 MMU
> > is turned off.  This results in all memory accesses to RAM being
> > cacheable, including during early boot of the guest.
> >
> > On CPUs without this feature, memory accesses were non-cacheable during
> > boot until the guest turned on the stage 1 MMU, and we had to detect
> > when the guest turned on the MMU, such that we could invalidate all cache
> > entries and ensure a consistent view of memory with the MMU turned on.
> > When the guest turned on the caches, we would call stage2_flush_vm()
> > from kvm_toggle_cache().
> >
> > However, stage2_flush_vm() walks all the stage 2 tables, and calls
> > __kvm_flush-dcache_pte, which on a system with S2FWD does ... absolutely
> > nothing.
> >
> > We can avoid that whole song and dance, and simply not set TVM when
> > creating a VM on a system that has S2FWB.
> >
> > Signed-off-by: Christoffer Dall 
> > Reviewed-by: Mark Rutland 
> > ---
> > I was only able to test this on the model with cache modeling enabled,
> > but even removing TVM from HCR_EL2 without having FWB also worked with
> > that setup, so the testing of this has been light.  It seems like it
> > should obviously work, but it would be good if someone with access to
> > appropriate hardware could give this a spin.
> >
> >  arch/arm64/include/asm/kvm_arm.h |  3 +--
> >  arch/arm64/include/asm/kvm_emulate.h | 12 +++-
> >  2 files changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_arm.h 
> > b/arch/arm64/include/asm/kvm_arm.h
> > index ddf9d762ac62..6e5d839f42b5 100644
> > --- a/arch/arm64/include/asm/kvm_arm.h
> > +++ b/arch/arm64/include/asm/kvm_arm.h
> > @@ -61,7 +61,6 @@
> >   * RW: 64bit by default, can be overridden for 32bit VMs
> >   * TAC:Trap ACTLR
> >   * TSC:Trap SMC
> > - * TVM:Trap VM ops (until M+C set in SCTLR_EL1)
> >   * TSW:Trap cache operations by set/way
> >   * TWE:Trap WFE
> >   * TWI:Trap WFI
> > @@ -74,7 +73,7 @@
> >   * SWIO:   Turn set/way invalidates into set/way clean+invalidate
> >   */
> >  #define HCR_GUEST_FLAGS (HCR_TSC | HCR_TSW | HCR_TWE | HCR_TWI | HCR_VM | \
> > -HCR_TVM | HCR_BSU_IS | HCR_FB | HCR_TAC | \
> > +HCR_BSU_IS | HCR_FB | HCR_TAC | \
> >  HCR_AMO | HCR_SWIO | HCR_TIDCP | HCR_RW | HCR_TLOR | \
> >  HCR_FMO | HCR_IMO)
> >  #define HCR_VIRT_EXCP_MASK (HCR_VSE | HCR_VI | HCR_VF)
> > diff --git a/arch/arm64/include/asm/kvm_emulate.h 
> > b/arch/arm64/include/asm/kvm_emulate.h
> > index d69c1efc63e7..70509799a2a9 100644
> > --- a/arch/arm64/include/asm/kvm_emulate.h
> > +++ b/arch/arm64/include/asm/kvm_emulate.h
> > @@ -53,8 +53,18 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
> > /* trap error record accesses */
> > vcpu->arch.hcr_el2 |= HCR_TERR;
> > }
> > -   if (cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))
> > +
> > +   if (cpus_have_const_cap(ARM64_HAS_STAGE2_FWB)) {
> > vcpu->arch.hcr_el2 |= HCR_FWB;
> > +   } else {
> > +   /*
> > +* For non-FWB CPUs, we trap VM ops (HCR_EL2.TVM) until M+C
> > +* get set in SCTLR_EL1 such that we can detect when the guest
> > +* MMU gets turned off and do the necessary cache maintenance
> > +* then.
> > +*/
> > +   vcpu->arch.hcr_el2 &= ~HCR_TVM;
> > +   }
> >  
> > if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features))
> > vcpu->arch.hcr_el2 &= ~HCR_RW;
> 
> This patch makes sense to me: when FWB is available, the guest memory is 
> cacheable
> even when the stage 1 MMU is disabled, which means it's now impossible to 
> have a
> situation where the data in memory is newer than the data in the cache.
> 
> I tested the patch with the fix suggested by Marc by doing a linux boot and 
> then a
> 'ls -R /', and by running kvm-unit-tests in a loop a couple dozen times. For 
> what
> it's worth:
> 
> Tested-by: Alexandru Elisei 
> 
> I do need to point out that I haven't been able to make a guest misbehave 
> when FWB
> is not enabled *and* KVM doesn't do a stage2_flush_vm when the stage 1 MMU is
> enabled. I tried to write two different tests in kvm-unit-tests:
> 
> 1. With the MMU never enabled, the test tells the host to read a value from 
> memory
> (so a cache line is allocated), writes another value to the same memory 
> location,
> and then enables the MMU and reads the memory back. I always got the latest 
> value
> that was written while the MMU was off.
> 
> 2. One thread tells the host to read the memor

Re: [PATCH v2] KVM: arm64: Don't set HCR_EL2.TVM when S2FWB is supported

2019-11-06 Thread Alexandru Elisei
Hi,

On 10/28/19 1:05 PM, Christoffer Dall wrote:
> On CPUs that support S2FWB (Armv8.4+), KVM configures the stage 2 page
> tables to override the memory attributes of memory accesses, regardless
> of the stage 1 page table configurations, and also when the stage 1 MMU
> is turned off.  This results in all memory accesses to RAM being
> cacheable, including during early boot of the guest.
>
> On CPUs without this feature, memory accesses were non-cacheable during
> boot until the guest turned on the stage 1 MMU, and we had to detect
> when the guest turned on the MMU, such that we could invalidate all cache
> entries and ensure a consistent view of memory with the MMU turned on.
> When the guest turned on the caches, we would call stage2_flush_vm()
> from kvm_toggle_cache().
>
> However, stage2_flush_vm() walks all the stage 2 tables, and calls
> __kvm_flush-dcache_pte, which on a system with S2FWD does ... absolutely
> nothing.
>
> We can avoid that whole song and dance, and simply not set TVM when
> creating a VM on a system that has S2FWB.
>
> Signed-off-by: Christoffer Dall 
> Reviewed-by: Mark Rutland 
> ---
> I was only able to test this on the model with cache modeling enabled,
> but even removing TVM from HCR_EL2 without having FWB also worked with
> that setup, so the testing of this has been light.  It seems like it
> should obviously work, but it would be good if someone with access to
> appropriate hardware could give this a spin.
>
>  arch/arm64/include/asm/kvm_arm.h |  3 +--
>  arch/arm64/include/asm/kvm_emulate.h | 12 +++-
>  2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_arm.h 
> b/arch/arm64/include/asm/kvm_arm.h
> index ddf9d762ac62..6e5d839f42b5 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -61,7 +61,6 @@
>   * RW:   64bit by default, can be overridden for 32bit VMs
>   * TAC:  Trap ACTLR
>   * TSC:  Trap SMC
> - * TVM:  Trap VM ops (until M+C set in SCTLR_EL1)
>   * TSW:  Trap cache operations by set/way
>   * TWE:  Trap WFE
>   * TWI:  Trap WFI
> @@ -74,7 +73,7 @@
>   * SWIO: Turn set/way invalidates into set/way clean+invalidate
>   */
>  #define HCR_GUEST_FLAGS (HCR_TSC | HCR_TSW | HCR_TWE | HCR_TWI | HCR_VM | \
> -  HCR_TVM | HCR_BSU_IS | HCR_FB | HCR_TAC | \
> +  HCR_BSU_IS | HCR_FB | HCR_TAC | \
>HCR_AMO | HCR_SWIO | HCR_TIDCP | HCR_RW | HCR_TLOR | \
>HCR_FMO | HCR_IMO)
>  #define HCR_VIRT_EXCP_MASK (HCR_VSE | HCR_VI | HCR_VF)
> diff --git a/arch/arm64/include/asm/kvm_emulate.h 
> b/arch/arm64/include/asm/kvm_emulate.h
> index d69c1efc63e7..70509799a2a9 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -53,8 +53,18 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
>   /* trap error record accesses */
>   vcpu->arch.hcr_el2 |= HCR_TERR;
>   }
> - if (cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))
> +
> + if (cpus_have_const_cap(ARM64_HAS_STAGE2_FWB)) {
>   vcpu->arch.hcr_el2 |= HCR_FWB;
> + } else {
> + /*
> +  * For non-FWB CPUs, we trap VM ops (HCR_EL2.TVM) until M+C
> +  * get set in SCTLR_EL1 such that we can detect when the guest
> +  * MMU gets turned off and do the necessary cache maintenance
> +  * then.
> +  */
> + vcpu->arch.hcr_el2 &= ~HCR_TVM;
> + }
>  
>   if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features))
>   vcpu->arch.hcr_el2 &= ~HCR_RW;

This patch makes sense to me: when FWB is available, the guest memory is 
cacheable
even when the stage 1 MMU is disabled, which means it's now impossible to have a
situation where the data in memory is newer than the data in the cache.

I tested the patch with the fix suggested by Marc by doing a linux boot and 
then a
'ls -R /', and by running kvm-unit-tests in a loop a couple dozen times. For 
what
it's worth:

Tested-by: Alexandru Elisei 

I do need to point out that I haven't been able to make a guest misbehave when 
FWB
is not enabled *and* KVM doesn't do a stage2_flush_vm when the stage 1 MMU is
enabled. I tried to write two different tests in kvm-unit-tests:

1. With the MMU never enabled, the test tells the host to read a value from 
memory
(so a cache line is allocated), writes another value to the same memory 
location,
and then enables the MMU and reads the memory back. I always got the latest 
value
that was written while the MMU was off.

2. One thread tells the host to read the memory location in a loop (to make sure
that the cache line doesn't get evicted), while the other thread writes a value
with the MMU off, enables the MMU and reads the memory back. I still got the
latest value written with the MMU off.

I can shar