diff mbox series

[v11,3/8] arm64: KVM: add accessors to track guest/host only counters

Message ID 20190308120746.56897-4-andrew.murray@arm.com (mailing list archive)
State New, archived
Headers show
Series arm64: Support perf event modifiers :G and :H | expand

Commit Message

Andrew Murray March 8, 2019, 12:07 p.m. UTC
In order to effeciently switch events_{guest,host} perf counters at
guest entry/exit we add bitfields to kvm_cpu_context for guest and host
events as well as accessors for updating them.

A function is also provided which allows the PMU driver to determine
if a counter should start counting when it is enabled. With exclude_host,
events on !VHE we may only start counting when entering the guest.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 arch/arm64/include/asm/kvm_host.h | 17 +++++++++++
 arch/arm64/kvm/Makefile           |  2 +-
 arch/arm64/kvm/pmu.c              | 49 +++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/kvm/pmu.c

Comments

Julien Thierry March 8, 2019, 4:40 p.m. UTC | #1
Hi Andrew,

On 08/03/2019 12:07, Andrew Murray wrote:
> In order to effeciently switch events_{guest,host} perf counters at
> guest entry/exit we add bitfields to kvm_cpu_context for guest and host
> events as well as accessors for updating them.
> 
> A function is also provided which allows the PMU driver to determine
> if a counter should start counting when it is enabled. With exclude_host,
> events on !VHE we may only start counting when entering the guest.
> 

I might have missed something here. Why is that true only for !VHE? Is
it because with VHE we can just exclude EL1?
(It's also a bit confusing since the patch does not seem to contain any
VHE/nVHE distinction)

> Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> ---
>  arch/arm64/include/asm/kvm_host.h | 17 +++++++++++
>  arch/arm64/kvm/Makefile           |  2 +-
>  arch/arm64/kvm/pmu.c              | 49 +++++++++++++++++++++++++++++++
>  3 files changed, 67 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm64/kvm/pmu.c
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 1d36619d6650..4b7219128f2d 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -207,8 +207,14 @@ struct kvm_cpu_context {
>  	struct kvm_vcpu *__hyp_running_vcpu;
>  };
>  
> +struct kvm_pmu_events {
> +	u32 events_host;
> +	u32 events_guest;
> +};
> +
>  struct kvm_host_data {
>  	struct kvm_cpu_context host_ctxt;
> +	struct kvm_pmu_events pmu_events;
>  };
>  
>  typedef struct kvm_host_data kvm_host_data_t;
> @@ -479,11 +485,22 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu);
>  void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu);
>  void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
>  
> +static inline bool kvm_pmu_counter_defered(struct perf_event_attr *attr)
> +{
> +	return attr->exclude_host;
> +}
> +
>  #ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
>  static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
>  {
>  	return kvm_arch_vcpu_run_map_fp(vcpu);
>  }
> +
> +void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr);
> +void kvm_clr_pmu_events(u32 clr);
> +#else
> +static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {}
> +static inline void kvm_clr_pmu_events(u32 clr) {}
>  #endif
>  
>  static inline void kvm_arm_vhe_guest_enter(void)
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index 0f2a135ba15b..f34cb49b66ae 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -19,7 +19,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
>  kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o va_layout.o
>  kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
>  kvm-$(CONFIG_KVM_ARM_HOST) += guest.o debug.o reset.o sys_regs.o sys_regs_generic_v8.o
> -kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o
> +kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o pmu.o
>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/aarch32.o
>  
>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic.o
> diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
> new file mode 100644
> index 000000000000..43965a3cc0f4
> --- /dev/null
> +++ b/arch/arm64/kvm/pmu.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * arch/arm64/kvm/pmu.c: Switching between guest and host counters
> + *
> + * Copyright 2019 Arm Limited
> + * Author: Andrew Murray <Andrew.Murray@arm.com>
> + */
> +#include <linux/kvm_host.h>
> +#include <linux/perf_event.h>
> +
> +DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
> +
> +/*
> + * Given the exclude_{host,guest} attributes, determine if we are going
> + * to need to switch counters at guest entry/exit.
> + */
> +static bool kvm_pmu_switch_needed(struct perf_event_attr *attr)
> +{
> +	/* Only switch if attributes are different */
> +	return (attr->exclude_host ^ attr->exclude_guest);

Nit:

Is there any benefit to this rather than doing "attr->exclude_host !=
attr->exclude_guest" ? The code generated is most likely the same, I
just find the latter slightly more straightforward.

Cheers,
Suzuki K Poulose March 11, 2019, 11 a.m. UTC | #2
Hi Andrew,


On 08/03/2019 12:07, Andrew Murray wrote:
> In order to effeciently switch events_{guest,host} perf counters at
> guest entry/exit we add bitfields to kvm_cpu_context for guest and host
> events as well as accessors for updating them.
> 
> A function is also provided which allows the PMU driver to determine
> if a counter should start counting when it is enabled. With exclude_host,
> events on !VHE we may only start counting when entering the guest.

Some minor comments below.

> 
> Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> ---
>   arch/arm64/include/asm/kvm_host.h | 17 +++++++++++
>   arch/arm64/kvm/Makefile           |  2 +-
>   arch/arm64/kvm/pmu.c              | 49 +++++++++++++++++++++++++++++++
>   3 files changed, 67 insertions(+), 1 deletion(-)
>   create mode 100644 arch/arm64/kvm/pmu.c
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 1d36619d6650..4b7219128f2d 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -207,8 +207,14 @@ struct kvm_cpu_context {
>   	struct kvm_vcpu *__hyp_running_vcpu;
>   };
>   
> +struct kvm_pmu_events {
> +	u32 events_host;
> +	u32 events_guest;
> +};
> +
>   struct kvm_host_data {
>   	struct kvm_cpu_context host_ctxt;
> +	struct kvm_pmu_events pmu_events;
>   };
>   
>   typedef struct kvm_host_data kvm_host_data_t;
> @@ -479,11 +485,22 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu);
>   void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu);
>   void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
>   
> +static inline bool kvm_pmu_counter_defered(struct perf_event_attr *attr)

nit: s/defered/deferred/

> +{
> +	return attr->exclude_host;
> +}
> +

Does it need a definition for !CONFIG_KVM case, to make sure that
the events are always enabled for that case ? i.e, does this introduce
a change in behavior for !CONFIG_KVM case ?

>   #ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
>   static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
>   {
>   	return kvm_arch_vcpu_run_map_fp(vcpu);
>   }
> +
> +void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr);
> +void kvm_clr_pmu_events(u32 clr);
> +#else
> +static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {}
> +static inline void kvm_clr_pmu_events(u32 clr) {}
>   #endif
>   
>   static inline void kvm_arm_vhe_guest_enter(void)
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index 0f2a135ba15b..f34cb49b66ae 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -19,7 +19,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
>   kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o va_layout.o
>   kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
>   kvm-$(CONFIG_KVM_ARM_HOST) += guest.o debug.o reset.o sys_regs.o sys_regs_generic_v8.o
> -kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o
> +kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o pmu.o
>   kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/aarch32.o
>   
>   kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic.o
> diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
> new file mode 100644
> index 000000000000..43965a3cc0f4
> --- /dev/null
> +++ b/arch/arm64/kvm/pmu.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * arch/arm64/kvm/pmu.c: Switching between guest and host counters

minor nit: You don't need the file name, it is superfluous.

> + *
> + * Copyright 2019 Arm Limited
> + * Author: Andrew Murray <Andrew.Murray@arm.com>
> + */
> +#include <linux/kvm_host.h>
> +#include <linux/perf_event.h>
> +

> +DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);

nit: Do we really need this ? This is already in asm/kvm_host.h.

> +
> +/*
> + * Given the exclude_{host,guest} attributes, determine if we are going
> + * to need to switch counters at guest entry/exit.
> + */
> +static bool kvm_pmu_switch_needed(struct perf_event_attr *attr)
> +{
> +	/* Only switch if attributes are different */
> +	return (attr->exclude_host ^ attr->exclude_guest);

I back Julien's suggestion to keep this simple.

> +}
> +
> +/*
> + * Add events to track that we may want to switch at guest entry/exit
> + * time.
> + */
> +void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr)
> +{
> +	struct kvm_host_data *ctx = this_cpu_ptr(&kvm_host_data);
> +
> +	if (!kvm_pmu_switch_needed(attr))
> +		return;
> +
> +	if (!attr->exclude_host)
> +		ctx->pmu_events.events_host |= set;
> +	if (!attr->exclude_guest)
> +		ctx->pmu_events.events_guest |= set;
> +}
> +
> +/*
> + * Stop tracking events
> + */
> +void kvm_clr_pmu_events(u32 clr)
> +{
> +	struct kvm_host_data *ctx = this_cpu_ptr(&kvm_host_data);
> +
> +	ctx->pmu_events.events_host &= ~clr;
> +	ctx->pmu_events.events_guest &= ~clr;
> +}
> 

Rest looks fine.

Suzuki
Andrew Murray March 11, 2019, 11:45 a.m. UTC | #3
On Fri, Mar 08, 2019 at 04:40:51PM +0000, Julien Thierry wrote:
> Hi Andrew,
> 
> On 08/03/2019 12:07, Andrew Murray wrote:
> > In order to effeciently switch events_{guest,host} perf counters at
> > guest entry/exit we add bitfields to kvm_cpu_context for guest and host
> > events as well as accessors for updating them.
> > 
> > A function is also provided which allows the PMU driver to determine
> > if a counter should start counting when it is enabled. With exclude_host,
> > events on !VHE we may only start counting when entering the guest.
> > 
> 
> I might have missed something here. Why is that true only for !VHE? Is
> it because with VHE we can just exclude EL1?

That's correct. For VHE we never defer counting and can rely on the PMU
filtering capabilities (even though for EL0 we have to reconfigure the
filtering upon entering/exiting the guest).

> (It's also a bit confusing since the patch does not seem to contain any
> VHE/nVHE distinction)

This is updated in the later patches of this series. I felt the series
would be easier to understand if I add the special VHE case last.

I will update the commit message such that it reads "With exclude_host, we
may only start counting when entering the guest.". I.e. the changes here
are valid for both VHE and !VHE until the later patches change it for VHE.

> 
> > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > ---
> >  arch/arm64/include/asm/kvm_host.h | 17 +++++++++++
> >  arch/arm64/kvm/Makefile           |  2 +-
> >  arch/arm64/kvm/pmu.c              | 49 +++++++++++++++++++++++++++++++
> >  3 files changed, 67 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/arm64/kvm/pmu.c
> > 
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index 1d36619d6650..4b7219128f2d 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -207,8 +207,14 @@ struct kvm_cpu_context {
> >  	struct kvm_vcpu *__hyp_running_vcpu;
> >  };
> >  
> > +struct kvm_pmu_events {
> > +	u32 events_host;
> > +	u32 events_guest;
> > +};
> > +
> >  struct kvm_host_data {
> >  	struct kvm_cpu_context host_ctxt;
> > +	struct kvm_pmu_events pmu_events;
> >  };
> >  
> >  typedef struct kvm_host_data kvm_host_data_t;
> > @@ -479,11 +485,22 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu);
> >  void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu);
> >  void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
> >  
> > +static inline bool kvm_pmu_counter_defered(struct perf_event_attr *attr)
> > +{
> > +	return attr->exclude_host;
> > +}
> > +
> >  #ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
> >  static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
> >  {
> >  	return kvm_arch_vcpu_run_map_fp(vcpu);
> >  }
> > +
> > +void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr);
> > +void kvm_clr_pmu_events(u32 clr);
> > +#else
> > +static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {}
> > +static inline void kvm_clr_pmu_events(u32 clr) {}
> >  #endif
> >  
> >  static inline void kvm_arm_vhe_guest_enter(void)
> > diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> > index 0f2a135ba15b..f34cb49b66ae 100644
> > --- a/arch/arm64/kvm/Makefile
> > +++ b/arch/arm64/kvm/Makefile
> > @@ -19,7 +19,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
> >  kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o va_layout.o
> >  kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
> >  kvm-$(CONFIG_KVM_ARM_HOST) += guest.o debug.o reset.o sys_regs.o sys_regs_generic_v8.o
> > -kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o
> > +kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o pmu.o
> >  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/aarch32.o
> >  
> >  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic.o
> > diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
> > new file mode 100644
> > index 000000000000..43965a3cc0f4
> > --- /dev/null
> > +++ b/arch/arm64/kvm/pmu.c
> > @@ -0,0 +1,49 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * arch/arm64/kvm/pmu.c: Switching between guest and host counters
> > + *
> > + * Copyright 2019 Arm Limited
> > + * Author: Andrew Murray <Andrew.Murray@arm.com>
> > + */
> > +#include <linux/kvm_host.h>
> > +#include <linux/perf_event.h>
> > +
> > +DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
> > +
> > +/*
> > + * Given the exclude_{host,guest} attributes, determine if we are going
> > + * to need to switch counters at guest entry/exit.
> > + */
> > +static bool kvm_pmu_switch_needed(struct perf_event_attr *attr)
> > +{
> > +	/* Only switch if attributes are different */
> > +	return (attr->exclude_host ^ attr->exclude_guest);
> 
> Nit:
> 
> Is there any benefit to this rather than doing "attr->exclude_host !=
> attr->exclude_guest" ? The code generated is most likely the same, I
> just find the latter slightly more straightforward.

Nope, I'll change it. Not sure why I ended up with this.

Thanks,

Andrew Murray

> 
> Cheers,
> 
> -- 
> Julien Thierry
Andrew Murray March 11, 2019, 11:59 a.m. UTC | #4
On Mon, Mar 11, 2019 at 11:00:04AM +0000, Suzuki K Poulose wrote:
> Hi Andrew,
> 
> 
> On 08/03/2019 12:07, Andrew Murray wrote:
> > In order to effeciently switch events_{guest,host} perf counters at
> > guest entry/exit we add bitfields to kvm_cpu_context for guest and host
> > events as well as accessors for updating them.
> > 
> > A function is also provided which allows the PMU driver to determine
> > if a counter should start counting when it is enabled. With exclude_host,
> > events on !VHE we may only start counting when entering the guest.
> 
> Some minor comments below.
> 
> > 
> > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > ---
> >   arch/arm64/include/asm/kvm_host.h | 17 +++++++++++
> >   arch/arm64/kvm/Makefile           |  2 +-
> >   arch/arm64/kvm/pmu.c              | 49 +++++++++++++++++++++++++++++++
> >   3 files changed, 67 insertions(+), 1 deletion(-)
> >   create mode 100644 arch/arm64/kvm/pmu.c
> > 
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index 1d36619d6650..4b7219128f2d 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -207,8 +207,14 @@ struct kvm_cpu_context {
> >   	struct kvm_vcpu *__hyp_running_vcpu;
> >   };
> > +struct kvm_pmu_events {
> > +	u32 events_host;
> > +	u32 events_guest;
> > +};
> > +
> >   struct kvm_host_data {
> >   	struct kvm_cpu_context host_ctxt;
> > +	struct kvm_pmu_events pmu_events;
> >   };
> >   typedef struct kvm_host_data kvm_host_data_t;
> > @@ -479,11 +485,22 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu);
> >   void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu);
> >   void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
> > +static inline bool kvm_pmu_counter_defered(struct perf_event_attr *attr)
> 
> nit: s/defered/deferred/

Thanks.

> 
> > +{
> > +	return attr->exclude_host;
> > +}
> > +
> 
> Does it need a definition for !CONFIG_KVM case, to make sure that
> the events are always enabled for that case ? i.e, does this introduce
> a change in behavior for !CONFIG_KVM case ?

I think this hunk is correct. It makes sense to not count with exclude_host
regardless to if there are guests or not. (By the way v10 has this test,
we've just moved it to this file.)

Later in this series we update the hunk to condition it on !has_vhe(), this
is still OK - it means for VHE we always enable to counter (despite
exclude_host) but that's OK because on VHE with exclude_host we exclude EL2,
EL0 (armv8pmu_set_event_filter). This does mean that we're enabling a counter
that doesn't do anything, but then from the user perspective it is a bit
pointless to use exclude_host on a system without KVM or guests.

> 
> >   #ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
> >   static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
> >   {
> >   	return kvm_arch_vcpu_run_map_fp(vcpu);
> >   }
> > +
> > +void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr);
> > +void kvm_clr_pmu_events(u32 clr);
> > +#else
> > +static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {}
> > +static inline void kvm_clr_pmu_events(u32 clr) {}
> >   #endif
> >   static inline void kvm_arm_vhe_guest_enter(void)
> > diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> > index 0f2a135ba15b..f34cb49b66ae 100644
> > --- a/arch/arm64/kvm/Makefile
> > +++ b/arch/arm64/kvm/Makefile
> > @@ -19,7 +19,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
> >   kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o va_layout.o
> >   kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
> >   kvm-$(CONFIG_KVM_ARM_HOST) += guest.o debug.o reset.o sys_regs.o sys_regs_generic_v8.o
> > -kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o
> > +kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o pmu.o
> >   kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/aarch32.o
> >   kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic.o
> > diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
> > new file mode 100644
> > index 000000000000..43965a3cc0f4
> > --- /dev/null
> > +++ b/arch/arm64/kvm/pmu.c
> > @@ -0,0 +1,49 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * arch/arm64/kvm/pmu.c: Switching between guest and host counters
> 
> minor nit: You don't need the file name, it is superfluous.
> 
> > + *
> > + * Copyright 2019 Arm Limited
> > + * Author: Andrew Murray <Andrew.Murray@arm.com>
> > + */
> > +#include <linux/kvm_host.h>
> > +#include <linux/perf_event.h>
> > +
> 
> > +DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
> 
> nit: Do we really need this ? This is already in asm/kvm_host.h.

No we don't - I'll remove it.

> 
> > +
> > +/*
> > + * Given the exclude_{host,guest} attributes, determine if we are going
> > + * to need to switch counters at guest entry/exit.
> > + */
> > +static bool kvm_pmu_switch_needed(struct perf_event_attr *attr)
> > +{
> > +	/* Only switch if attributes are different */
> > +	return (attr->exclude_host ^ attr->exclude_guest);
> 
> I back Julien's suggestion to keep this simple.
> 
> > +}
> > +
> > +/*
> > + * Add events to track that we may want to switch at guest entry/exit
> > + * time.
> > + */
> > +void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr)
> > +{
> > +	struct kvm_host_data *ctx = this_cpu_ptr(&kvm_host_data);
> > +
> > +	if (!kvm_pmu_switch_needed(attr))
> > +		return;
> > +
> > +	if (!attr->exclude_host)
> > +		ctx->pmu_events.events_host |= set;
> > +	if (!attr->exclude_guest)
> > +		ctx->pmu_events.events_guest |= set;
> > +}
> > +
> > +/*
> > + * Stop tracking events
> > + */
> > +void kvm_clr_pmu_events(u32 clr)
> > +{
> > +	struct kvm_host_data *ctx = this_cpu_ptr(&kvm_host_data);
> > +
> > +	ctx->pmu_events.events_host &= ~clr;
> > +	ctx->pmu_events.events_guest &= ~clr;
> > +}
> > 
> 
> Rest looks fine.

Thanks for the review and nits.

Andrew Murray

> 
> Suzuki
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 1d36619d6650..4b7219128f2d 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -207,8 +207,14 @@  struct kvm_cpu_context {
 	struct kvm_vcpu *__hyp_running_vcpu;
 };
 
+struct kvm_pmu_events {
+	u32 events_host;
+	u32 events_guest;
+};
+
 struct kvm_host_data {
 	struct kvm_cpu_context host_ctxt;
+	struct kvm_pmu_events pmu_events;
 };
 
 typedef struct kvm_host_data kvm_host_data_t;
@@ -479,11 +485,22 @@  void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu);
 void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu);
 void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
 
+static inline bool kvm_pmu_counter_defered(struct perf_event_attr *attr)
+{
+	return attr->exclude_host;
+}
+
 #ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
 static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
 {
 	return kvm_arch_vcpu_run_map_fp(vcpu);
 }
+
+void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr);
+void kvm_clr_pmu_events(u32 clr);
+#else
+static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {}
+static inline void kvm_clr_pmu_events(u32 clr) {}
 #endif
 
 static inline void kvm_arm_vhe_guest_enter(void)
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 0f2a135ba15b..f34cb49b66ae 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -19,7 +19,7 @@  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
 kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o va_layout.o
 kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
 kvm-$(CONFIG_KVM_ARM_HOST) += guest.o debug.o reset.o sys_regs.o sys_regs_generic_v8.o
-kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o
+kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o pmu.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/aarch32.o
 
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic.o
diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
new file mode 100644
index 000000000000..43965a3cc0f4
--- /dev/null
+++ b/arch/arm64/kvm/pmu.c
@@ -0,0 +1,49 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * arch/arm64/kvm/pmu.c: Switching between guest and host counters
+ *
+ * Copyright 2019 Arm Limited
+ * Author: Andrew Murray <Andrew.Murray@arm.com>
+ */
+#include <linux/kvm_host.h>
+#include <linux/perf_event.h>
+
+DECLARE_PER_CPU(kvm_host_data_t, kvm_host_data);
+
+/*
+ * Given the exclude_{host,guest} attributes, determine if we are going
+ * to need to switch counters at guest entry/exit.
+ */
+static bool kvm_pmu_switch_needed(struct perf_event_attr *attr)
+{
+	/* Only switch if attributes are different */
+	return (attr->exclude_host ^ attr->exclude_guest);
+}
+
+/*
+ * Add events to track that we may want to switch at guest entry/exit
+ * time.
+ */
+void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr)
+{
+	struct kvm_host_data *ctx = this_cpu_ptr(&kvm_host_data);
+
+	if (!kvm_pmu_switch_needed(attr))
+		return;
+
+	if (!attr->exclude_host)
+		ctx->pmu_events.events_host |= set;
+	if (!attr->exclude_guest)
+		ctx->pmu_events.events_guest |= set;
+}
+
+/*
+ * Stop tracking events
+ */
+void kvm_clr_pmu_events(u32 clr)
+{
+	struct kvm_host_data *ctx = this_cpu_ptr(&kvm_host_data);
+
+	ctx->pmu_events.events_host &= ~clr;
+	ctx->pmu_events.events_guest &= ~clr;
+}