Re: [PATCH 1/2] KVM: arm64: Move pmu hyp code under hyp's Makefile to avoid instrumentation
Hi James,
On 21/05/2019 18:25, James Morse wrote:
> KVM's pmu.c contains the __hyp_text needed to switch the pmu registers
> between host and guest. Because this isn't covered by the 'hyp' Makefile,
> it can be built with kasan and friends when these are enabled in Kconfig.
>
> When starting a guest, this results in:
> | Kernel panic - not syncing: HYP panic:
> | PS:a3c9 PC:8328ada0 ESR:8607
> | FAR:8328ada0 HPFAR:29df5300 PAR:
> | VCPU:4e10b7d6
> | CPU: 0 PID: 3088 Comm: qemu-system-aar Not tainted 5.2.0-rc1 #11026
> | Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno Development
> Plat
> | Call trace:
> | dump_backtrace+0x0/0x200
> | show_stack+0x20/0x30
> | dump_stack+0xec/0x158
> | panic+0x1ec/0x420
> | panic+0x0/0x420
> | SMP: stopping secondary CPUs
> | Kernel Offset: disabled
> | CPU features: 0x002,25006082
> | Memory Limit: none
> | ---[ end Kernel panic - not syncing: HYP panic:
>
> This is caused by functions in pmu.c calling the instrumented
> code, which isn't mapped to hyp. From objdump -r:
> | RELOCATION RECORDS FOR [.hyp.text]:
> | OFFSET TYPE VALUE
> | 0010 R_AARCH64_CALL26 __sanitizer_cov_trace_pc
> | 0018 R_AARCH64_CALL26 __asan_load4_noabort
> | 0024 R_AARCH64_CALL26 __asan_load4_noabort
>
> Move the affected code to a new file under 'hyp's Makefile.
>
> Fixes: 3d91befbb3a0 ("arm64: KVM: Enable !VHE support for :G/:H perf event
> modifiers")
> Cc: Andrew Murray
> Signed-off-by: James Morse
> ---
> arch/arm64/kvm/hyp/switch.c | 39 +
> arch/arm64/kvm/pmu.c| 38
> 2 files changed, 39 insertions(+), 38 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 22b4c335e0b2..030c3fa28e0e 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -16,6 +16,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -703,3 +704,41 @@ void __hyp_text __noreturn hyp_panic(struct
> kvm_cpu_context *host_ctxt)
>
> unreachable();
> }
> +
> +/**
> + * Disable host events, enable guest events
> + */
> +bool __hyp_text __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
I think this could now be made static, and the declarations removed from
asm/kvm_host.h.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 2/2] KVM: arm/arm64: Move cc/it checks under hyp's Makefile to avoid instrumentation
KVM has helpers to handle the condition codes of trapped aarch32
instructions. These are marked __hyp_text and used from HYP, but they
aren't built by the 'hyp' Makefile, which has all the runes to avoid ASAN
and KCOV instrumentation, used when debugging.
Move this code to a new hyp/aarch32.c to avoid a hyp-panic when starting
an aarch32 guest on a host built with the ASAN/KCOV debug options.
Fixes: 021234ef3752f ("KVM: arm64: Make kvm_condition_valid32() accessible from
EL2")
Fixes: 8cebe750c4d9a ("arm64: KVM: Make kvm_skip_instr32 available to HYP")
Signed-off-by: James Morse
---
arch/arm/kvm/hyp/Makefile | 1 +
arch/arm64/kvm/hyp/Makefile | 1 +
virt/kvm/arm/aarch32.c | 121
virt/kvm/arm/hyp/aarch32.c | 136
4 files changed, 138 insertions(+), 121 deletions(-)
create mode 100644 virt/kvm/arm/hyp/aarch32.c
diff --git a/arch/arm/kvm/hyp/Makefile b/arch/arm/kvm/hyp/Makefile
index d2b5ec9c4b92..ba88b1eca93c 100644
--- a/arch/arm/kvm/hyp/Makefile
+++ b/arch/arm/kvm/hyp/Makefile
@@ -11,6 +11,7 @@ CFLAGS_ARMV7VE :=$(call cc-option,
-march=armv7ve)
obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v3-sr.o
obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/timer-sr.o
+obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/aarch32.o
obj-$(CONFIG_KVM_ARM_HOST) += tlb.o
obj-$(CONFIG_KVM_ARM_HOST) += cp15-sr.o
diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
index 82d1904328ad..ea710f674cb6 100644
--- a/arch/arm64/kvm/hyp/Makefile
+++ b/arch/arm64/kvm/hyp/Makefile
@@ -10,6 +10,7 @@ KVM=../../../../virt/kvm
obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v3-sr.o
obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/timer-sr.o
+obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/aarch32.o
obj-$(CONFIG_KVM_ARM_HOST) += vgic-v2-cpuif-proxy.o
obj-$(CONFIG_KVM_ARM_HOST) += sysreg-sr.o
diff --git a/virt/kvm/arm/aarch32.c b/virt/kvm/arm/aarch32.c
index 5abbe9b3c652..6880236974b8 100644
--- a/virt/kvm/arm/aarch32.c
+++ b/virt/kvm/arm/aarch32.c
@@ -25,127 +25,6 @@
#include
#include
-/*
- * stolen from arch/arm/kernel/opcodes.c
- *
- * condition code lookup table
- * index into the table is test code: EQ, NE, ... LT, GT, AL, NV
- *
- * bit position in short is condition code: NZCV
- */
-static const unsigned short cc_map[16] = {
- 0xF0F0, /* EQ == Z set*/
- 0x0F0F, /* NE */
- 0x, /* CS == C set*/
- 0x, /* CC */
- 0xFF00, /* MI == N set*/
- 0x00FF, /* PL */
- 0x, /* VS == V set*/
- 0x, /* VC */
- 0x0C0C, /* HI == C set && Z clear */
- 0xF3F3, /* LS == C clear || Z set */
- 0xAA55, /* GE == (N==V) */
- 0x55AA, /* LT == (N!=V) */
- 0x0A05, /* GT == (!Z && (N==V)) */
- 0xF5FA, /* LE == (Z || (N!=V))*/
- 0x, /* AL always */
- 0 /* NV */
-};
-
-/*
- * Check if a trapped instruction should have been executed or not.
- */
-bool __hyp_text kvm_condition_valid32(const struct kvm_vcpu *vcpu)
-{
- unsigned long cpsr;
- u32 cpsr_cond;
- int cond;
-
- /* Top two bits non-zero? Unconditional. */
- if (kvm_vcpu_get_hsr(vcpu) >> 30)
- return true;
-
- /* Is condition field valid? */
- cond = kvm_vcpu_get_condition(vcpu);
- if (cond == 0xE)
- return true;
-
- cpsr = *vcpu_cpsr(vcpu);
-
- if (cond < 0) {
- /* This can happen in Thumb mode: examine IT state. */
- unsigned long it;
-
- it = ((cpsr >> 8) & 0xFC) | ((cpsr >> 25) & 0x3);
-
- /* it == 0 => unconditional. */
- if (it == 0)
- return true;
-
- /* The cond for this insn works out as the top 4 bits. */
- cond = (it >> 4);
- }
-
- cpsr_cond = cpsr >> 28;
-
- if (!((cc_map[cond] >> cpsr_cond) & 1))
- return false;
-
- return true;
-}
-
-/**
- * adjust_itstate - adjust ITSTATE when emulating instructions in IT-block
- * @vcpu: The VCPU pointer
- *
- * When exceptions occur while instructions are executed in Thumb IF-THEN
- * blocks, the ITSTATE field of the CPSR is not advanced (updated), so we have
- * to do this little bit of work manually. The fields map like this:
- *
- * IT[7:0] -> CPSR[26:25],CPSR[15:10]
- */
-static void __hyp_text kvm_adjust_itstate(struct kvm_vcpu *vcpu)
-{
- unsigned long itbits, cond;
- unsigned long cpsr = *vcpu_cpsr(vcpu);
-
[PATCH 1/2] KVM: arm64: Move pmu hyp code under hyp's Makefile to avoid instrumentation
KVM's pmu.c contains the __hyp_text needed to switch the pmu registers
between host and guest. Because this isn't covered by the 'hyp' Makefile,
it can be built with kasan and friends when these are enabled in Kconfig.
When starting a guest, this results in:
| Kernel panic - not syncing: HYP panic:
| PS:a3c9 PC:8328ada0 ESR:8607
| FAR:8328ada0 HPFAR:29df5300 PAR:
| VCPU:4e10b7d6
| CPU: 0 PID: 3088 Comm: qemu-system-aar Not tainted 5.2.0-rc1 #11026
| Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno Development Plat
| Call trace:
| dump_backtrace+0x0/0x200
| show_stack+0x20/0x30
| dump_stack+0xec/0x158
| panic+0x1ec/0x420
| panic+0x0/0x420
| SMP: stopping secondary CPUs
| Kernel Offset: disabled
| CPU features: 0x002,25006082
| Memory Limit: none
| ---[ end Kernel panic - not syncing: HYP panic:
This is caused by functions in pmu.c calling the instrumented
code, which isn't mapped to hyp. From objdump -r:
| RELOCATION RECORDS FOR [.hyp.text]:
| OFFSET TYPE VALUE
| 0010 R_AARCH64_CALL26 __sanitizer_cov_trace_pc
| 0018 R_AARCH64_CALL26 __asan_load4_noabort
| 0024 R_AARCH64_CALL26 __asan_load4_noabort
Move the affected code to a new file under 'hyp's Makefile.
Fixes: 3d91befbb3a0 ("arm64: KVM: Enable !VHE support for :G/:H perf event
modifiers")
Cc: Andrew Murray
Signed-off-by: James Morse
---
arch/arm64/kvm/hyp/switch.c | 39 +
arch/arm64/kvm/pmu.c| 38
2 files changed, 39 insertions(+), 38 deletions(-)
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 22b4c335e0b2..030c3fa28e0e 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -16,6 +16,7 @@
*/
#include
+#include
#include
#include
#include
@@ -703,3 +704,41 @@ void __hyp_text __noreturn hyp_panic(struct
kvm_cpu_context *host_ctxt)
unreachable();
}
+
+/**
+ * Disable host events, enable guest events
+ */
+bool __hyp_text __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
+{
+ struct kvm_host_data *host;
+ struct kvm_pmu_events *pmu;
+
+ host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
+ pmu = &host->pmu_events;
+
+ if (pmu->events_host)
+ write_sysreg(pmu->events_host, pmcntenclr_el0);
+
+ if (pmu->events_guest)
+ write_sysreg(pmu->events_guest, pmcntenset_el0);
+
+ return (pmu->events_host || pmu->events_guest);
+}
+
+/**
+ * Disable guest events, enable host events
+ */
+void __hyp_text __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
+{
+ struct kvm_host_data *host;
+ struct kvm_pmu_events *pmu;
+
+ host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
+ pmu = &host->pmu_events;
+
+ if (pmu->events_guest)
+ write_sysreg(pmu->events_guest, pmcntenclr_el0);
+
+ if (pmu->events_host)
+ write_sysreg(pmu->events_host, pmcntenset_el0);
+}
diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
index 3da94a5bb6b7..e71d00bb5271 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -53,44 +53,6 @@ void kvm_clr_pmu_events(u32 clr)
ctx->pmu_events.events_guest &= ~clr;
}
-/**
- * Disable host events, enable guest events
- */
-bool __hyp_text __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
-{
- struct kvm_host_data *host;
- struct kvm_pmu_events *pmu;
-
- host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
- pmu = &host->pmu_events;
-
- if (pmu->events_host)
- write_sysreg(pmu->events_host, pmcntenclr_el0);
-
- if (pmu->events_guest)
- write_sysreg(pmu->events_guest, pmcntenset_el0);
-
- return (pmu->events_host || pmu->events_guest);
-}
-
-/**
- * Disable guest events, enable host events
- */
-void __hyp_text __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
-{
- struct kvm_host_data *host;
- struct kvm_pmu_events *pmu;
-
- host = container_of(host_ctxt, struct kvm_host_data, host_ctxt);
- pmu = &host->pmu_events;
-
- if (pmu->events_guest)
- write_sysreg(pmu->events_guest, pmcntenclr_el0);
-
- if (pmu->events_host)
- write_sysreg(pmu->events_host, pmcntenset_el0);
-}
-
#define PMEVTYPER_READ_CASE(idx) \
case idx: \
return read_sysreg(pmevtyper##idx##_el0)
--
2.20.1
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 0/2] Move __hyp_text code under no-asan Makefiles
The fancy new pmu code added its __hyp_text code in part of the tree that doesn't get covered by the no-asan/no-kcov kconfig decorations. This shows up as a hyp-panic on v8.0 hardware when the host kernel is built with debug options like kasan. This same bug has been living happily in the aarch32 emulation code since v4.9. (commit 8cebe750c4d9a "arm64: KVM: Make kvm_skip_instr32 available to HYP"). Patch 2 has the two relevant fixes tag, but won't apply cleanly before v4.19 due to the churn. Fix them both by shuffling the code around. Thanks, James Morse (2): KVM: arm64: Move pmu hyp code under hyp's Makefile to avoid instrumentation KVM: arm/arm64: Move cc/it checks under hyp's Makefile to avoid instrumentation arch/arm/kvm/hyp/Makefile | 1 + arch/arm64/kvm/hyp/Makefile | 1 + arch/arm64/kvm/hyp/switch.c | 39 +++ arch/arm64/kvm/pmu.c| 38 -- virt/kvm/arm/aarch32.c | 121 virt/kvm/arm/hyp/aarch32.c | 136 6 files changed, 177 insertions(+), 159 deletions(-) create mode 100644 virt/kvm/arm/hyp/aarch32.c -- 2.20.1 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 3/3] arm64/fpsimd: Don't disable softirq when touching FPSIMD/SVE state
When the kernel is compiled with CONFIG_KERNEL_MODE_NEON, some part of
the kernel may be able to use FPSIMD/SVE. This is for instance the case
for crypto code.
Any use of FPSIMD/SVE in the kernel are clearly marked by using the
function kernel_neon_{begin, end}. Furthermore, this can only be used
when may_use_simd() returns true.
The current implementation of may_use_simd() allows softirq to use
FPSIMD/SVE unless it is currently in use (i.e kernel_neon_busy is true).
When in use, softirqs usually fall back to a software method.
At the moment, as a softirq may use FPSIMD/SVE, softirqs are disabled
when touching the FPSIMD/SVE context. This has the drawback to disable
all softirqs even if they are not using FPSIMD/SVE.
Since a softirq is supposed to check may_use_simd() anyway before
attempting to use FPSIMD/SVE, there is limited reason to keep softirq
disabled when touching the FPSIMD/SVE context. Instead, we can simply
disable preemption and mark the FPSIMD/SVE context as in use by setting
CPU's fpsimd_context_busy flag.
Two new helpers {get, put}_cpu_fpsimd_context are introduced to mark
the area using FPSIMD/SVE context and they are used to replace
local_bh_{disable, enable}. The functions kernel_neon_{begin, end} are
also re-implemented to use the new helpers.
Additionally, double-underscored versions of the helpers are provided to
called when preemption is already disabled. These are only relevant on
paths where irqs are disabled anyway, so they are not needed for
correctness in the current code. Let's use them anyway though: this
marks critical sections clearly and will help to avoid mistakes during
future maintenance.
The change has been benchmarked on Linux 5.1-rc4 with defconfig.
On Juno2:
* hackbench 100 process 1000 (10 times)
* .7% quicker
On ThunderX 2:
* hackbench 1000 process 1000 (20 times)
* 3.4% quicker
Signed-off-by: Julien Grall
Reviewed-by: Dave Martin
---
Changes in v5:
- Update commit message
- Add Dave's reviewed-by
Changes in v4:
- Clarify the comment on top of get_cpu_fpsimd_context()
- Use double-underscore version in fpsimd_save_and_flush_cpu_state()
Changes in v3:
- Fix typoes in the commit message
- Rework a bit the commit message
- Use imperative mood
- Rename kernel_neon_busy to fpsimd_context_busy
- Remove debug code
- Update comments
- Don't require preemption when calling
fpsimd_save_and_flush_cpu_state()
Changes in v2:
- Remove spurious call to kernel_neon_enable in kernel_neon_begin.
- Rename kernel_neon_{enable, disable} to {get, put}_cpu_fpsimd_context
- Introduce a double-underscore version of the helpers for case
where preemption is already disabled
- Introduce have_cpu_fpsimd_context() and use it in WARN_ON(...)
- Surround more places in the code with the new helpers
- Rework the comments
- Update the commit message with the benchmark result
---
arch/arm64/include/asm/simd.h | 10 ++--
arch/arm64/kernel/fpsimd.c| 124 --
2 files changed, 89 insertions(+), 45 deletions(-)
diff --git a/arch/arm64/include/asm/simd.h b/arch/arm64/include/asm/simd.h
index 6495cc51246f..a6307e43b8c2 100644
--- a/arch/arm64/include/asm/simd.h
+++ b/arch/arm64/include/asm/simd.h
@@ -15,9 +15,9 @@
#include
#include
-#ifdef CONFIG_KERNEL_MODE_NEON
+DECLARE_PER_CPU(bool, fpsimd_context_busy);
-DECLARE_PER_CPU(bool, kernel_neon_busy);
+#ifdef CONFIG_KERNEL_MODE_NEON
/*
* may_use_simd - whether it is allowable at this time to issue SIMD
@@ -29,15 +29,15 @@ DECLARE_PER_CPU(bool, kernel_neon_busy);
static __must_check inline bool may_use_simd(void)
{
/*
-* kernel_neon_busy is only set while preemption is disabled,
+* fpsimd_context_busy is only set while preemption is disabled,
* and is clear whenever preemption is enabled. Since
-* this_cpu_read() is atomic w.r.t. preemption, kernel_neon_busy
+* this_cpu_read() is atomic w.r.t. preemption, fpsimd_context_busy
* cannot change under our feet -- if it's set we cannot be
* migrated, and if it's clear we cannot be migrated to a CPU
* where it is set.
*/
return !in_irq() && !irqs_disabled() && !in_nmi() &&
- !this_cpu_read(kernel_neon_busy);
+ !this_cpu_read(fpsimd_context_busy);
}
#else /* ! CONFIG_KERNEL_MODE_NEON */
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 6448921a2f59..c7c454df2779 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -92,7 +92,8 @@
* To prevent this from racing with the manipulation of the task's FPSIMD state
* from task context and thereby corrupting the state, it is necessary to
* protect any manipulation of a task's fpsimd_state or TIF_FOREIGN_FPSTATE
- * flag with local_bh_disable() un
[PATCH v5 0/3] arm64/fpsimd: Don't disable softirq when touching FPSIMD/SVE state
Hi all, This patch series keeps softirqs enabled while touching FPSIMD/SVE state. For more details on the impact see patch #3. This patch series has been benchmarked on Linux 5.1-rc4 with defconfig. On Juno2: * hackbench 100 process 1000 (10 times) * .7% quicker On ThunderX 2: * hackbench 1000 process 1000 (20 times) * 3.4% quicker Note that while the benchmark has been done on 5.1-rc4, the patch series is based on 5.2-rc1. Cheers, Julien Grall (3): arm64/fpsimd: Remove the prototype for sve_flush_cpu_state() arch/arm64: fpsimd: Introduce fpsimd_save_and_flush_cpu_state() and use it arm64/fpsimd: Don't disable softirq when touching FPSIMD/SVE state arch/arm64/include/asm/fpsimd.h | 5 +- arch/arm64/include/asm/simd.h | 10 +-- arch/arm64/kernel/fpsimd.c | 139 +++- arch/arm64/kvm/fpsimd.c | 4 +- 4 files changed, 103 insertions(+), 55 deletions(-) -- 2.11.0 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 2/3] arch/arm64: fpsimd: Introduce fpsimd_save_and_flush_cpu_state() and use it
The only external user of fpsimd_save() and fpsimd_flush_cpu_state() is
the KVM FPSIMD code.
A following patch will introduce a mechanism to acquire owernship of the
FPSIMD/SVE context for performing context management operations. Rather
than having to export the new helpers to get/put the context, we can just
introduce a new function to combine fpsimd_save() and
fpsimd_flush_cpu_state().
This has also the advantage to remove any external call of fpsimd_save()
and fpsimd_flush_cpu_state(), so they can be turned static.
Lastly, the new function can also be used in the PM notifier.
Signed-off-by: Julien Grall
Reviewed-by: Dave Martin
---
kernel_neon_begin() does not use fpsimd_save_and_flush_cpu_state()
because the next patch will modify the function to also grab the
FPSIMD/SVE context.
Changes in v4:
- Remove newline before the new prototype
- Add Dave's reviewed-by
Changes in v3:
- Rework the commit message
- Move the prototype of fpsimd_save_and_flush_cpu_state()
further down in the header
- Remove comment in kvm_arch_vcpu_put_fp()
Changes in v2:
- Patch added
---
arch/arm64/include/asm/fpsimd.h | 4 +---
arch/arm64/kernel/fpsimd.c | 17 +
arch/arm64/kvm/fpsimd.c | 4 +---
3 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
index b73d12fcc7f9..4154851c21ab 100644
--- a/arch/arm64/include/asm/fpsimd.h
+++ b/arch/arm64/include/asm/fpsimd.h
@@ -48,8 +48,6 @@ struct task_struct;
extern void fpsimd_save_state(struct user_fpsimd_state *state);
extern void fpsimd_load_state(struct user_fpsimd_state *state);
-extern void fpsimd_save(void);
-
extern void fpsimd_thread_switch(struct task_struct *next);
extern void fpsimd_flush_thread(void);
@@ -63,7 +61,7 @@ extern void fpsimd_bind_state_to_cpu(struct user_fpsimd_state
*state,
void *sve_state, unsigned int sve_vl);
extern void fpsimd_flush_task_state(struct task_struct *target);
-extern void fpsimd_flush_cpu_state(void);
+extern void fpsimd_save_and_flush_cpu_state(void);
/* Maximum VL that SVE VL-agnostic software can transparently support */
#define SVE_VL_ARCH_MAX 0x100
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index a38bf74bcca8..6448921a2f59 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -246,7 +246,7 @@ static void task_fpsimd_load(void)
*
* Softirqs (and preemption) must be disabled.
*/
-void fpsimd_save(void)
+static void fpsimd_save(void)
{
struct fpsimd_last_state_struct const *last =
this_cpu_ptr(&fpsimd_last_state);
@@ -1122,12 +1122,22 @@ void fpsimd_flush_task_state(struct task_struct *t)
* Invalidate any task's FPSIMD state that is present on this cpu.
* This function must be called with softirqs disabled.
*/
-void fpsimd_flush_cpu_state(void)
+static void fpsimd_flush_cpu_state(void)
{
__this_cpu_write(fpsimd_last_state.st, NULL);
set_thread_flag(TIF_FOREIGN_FPSTATE);
}
+/*
+ * Save the FPSIMD state to memory and invalidate cpu view.
+ * This function must be called with softirqs (and preemption) disabled.
+ */
+void fpsimd_save_and_flush_cpu_state(void)
+{
+ fpsimd_save();
+ fpsimd_flush_cpu_state();
+}
+
#ifdef CONFIG_KERNEL_MODE_NEON
DEFINE_PER_CPU(bool, kernel_neon_busy);
@@ -1284,8 +1294,7 @@ static int fpsimd_cpu_pm_notifier(struct notifier_block
*self,
{
switch (cmd) {
case CPU_PM_ENTER:
- fpsimd_save();
- fpsimd_flush_cpu_state();
+ fpsimd_save_and_flush_cpu_state();
break;
case CPU_PM_EXIT:
break;
diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
index 6e3c9c8b2df9..525010504f9d 100644
--- a/arch/arm64/kvm/fpsimd.c
+++ b/arch/arm64/kvm/fpsimd.c
@@ -112,9 +112,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) {
u64 *guest_zcr = &vcpu->arch.ctxt.sys_regs[ZCR_EL1];
- /* Clean guest FP state to memory and invalidate cpu view */
- fpsimd_save();
- fpsimd_flush_cpu_state();
+ fpsimd_save_and_flush_cpu_state();
if (guest_has_sve)
*guest_zcr = read_sysreg_s(SYS_ZCR_EL12);
--
2.11.0
[PATCH v5 1/3] arm64/fpsimd: Remove the prototype for sve_flush_cpu_state()
The function sve_flush_cpu_state() has been removed in commit 21cdd7fd76e3
("KVM: arm64: Remove eager host SVE state saving").
So remove the associated prototype in asm/fpsimd.h.
Signed-off-by: Julien Grall
Reviewed-by: Dave Martin
---
Changes in v3:
- Add Dave's reviewed-by
- Fix checkpatch style error when mentioning a commit
Changes in v2:
- Patch added
---
arch/arm64/include/asm/fpsimd.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
index df62bbd33a9a..b73d12fcc7f9 100644
--- a/arch/arm64/include/asm/fpsimd.h
+++ b/arch/arm64/include/asm/fpsimd.h
@@ -64,7 +64,6 @@ extern void fpsimd_bind_state_to_cpu(struct user_fpsimd_state
*state,
extern void fpsimd_flush_task_state(struct task_struct *target);
extern void fpsimd_flush_cpu_state(void);
-extern void sve_flush_cpu_state(void);
/* Maximum VL that SVE VL-agnostic software can transparently support */
#define SVE_VL_ARCH_MAX 0x100
--
2.11.0
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v7 5/5] KVM: arm/arm64: support chained PMU counters
Hi Andrew,
On 05/21/2019 04:52 PM, Andrew Murray wrote:
> ARMv8 provides support for chained PMU counters, where an event type
> of 0x001E is set for odd-numbered counters, the event counter will
> increment by one for each overflow of the preceding even-numbered
> counter. Let's emulate this in KVM by creating a 64 bit perf counter
> when a user chains two emulated counters together.
>
> For chained events we only support generating an overflow interrupt
> on the high counter. We use the attributes of the low counter to
> determine the attributes of the perf event.
>
> Suggested-by: Marc Zyngier
> Signed-off-by: Andrew Murray
> ---
> include/kvm/arm_pmu.h | 2 +
> virt/kvm/arm/pmu.c| 246 --
> 2 files changed, 215 insertions(+), 33 deletions(-)
>
> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> index b73f31baca52..8b434745500a 100644
> --- a/include/kvm/arm_pmu.h
> +++ b/include/kvm/arm_pmu.h
> @@ -22,6 +22,7 @@
> #include
>
> #define ARMV8_PMU_CYCLE_IDX (ARMV8_PMU_MAX_COUNTERS - 1)
> +#define ARMV8_PMU_MAX_COUNTER_PAIRS ((ARMV8_PMU_MAX_COUNTERS + 1) >> 1)
>
> #ifdef CONFIG_KVM_ARM_PMU
>
> @@ -34,6 +35,7 @@ struct kvm_pmc {
> struct kvm_pmu {
> int irq_num;
> struct kvm_pmc pmc[ARMV8_PMU_MAX_COUNTERS];
> + DECLARE_BITMAP(chained, ARMV8_PMU_MAX_COUNTER_PAIRS);
> bool ready;
> bool created;
> bool irq_level;
> diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
> index ae1e886d4a1a..4b0981c402c6 100644
> --- a/virt/kvm/arm/pmu.c
> +++ b/virt/kvm/arm/pmu.c
> @@ -25,28 +25,128 @@
> #include
>
> static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx);
> +
> +#define PERF_ATTR_CFG1_KVM_PMU_CHAINED 0x1
> +
> +static struct kvm_vcpu *kvm_pmc_to_vcpu(struct kvm_pmc *pmc)
> +{
> + struct kvm_pmu *pmu;
> + struct kvm_vcpu_arch *vcpu_arch;
> +
> + pmc -= pmc->idx;
> + pmu = container_of(pmc, struct kvm_pmu, pmc[0]);
> + vcpu_arch = container_of(pmu, struct kvm_vcpu_arch, pmu);
> + return container_of(vcpu_arch, struct kvm_vcpu, arch);
> +}
> +
> /**
> - * kvm_pmu_get_counter_value - get PMU counter value
> + * kvm_pmu_pmc_is_chained - determine if the pmc is chained
> + * @pmc: The PMU counter pointer
> + */
> +static bool kvm_pmu_pmc_is_chained(struct kvm_pmc *pmc)
> +{
> + struct kvm_vcpu *vcpu = kvm_pmc_to_vcpu(pmc);
> +
> + return test_bit(pmc->idx >> 1, vcpu->arch.pmu.chained);
> +}
> +
> +/**
> + * kvm_pmu_pmc_is_high_counter - determine if select_idx is a high/low
> counter
> + * @select_idx: The counter index
> + */
> +static bool kvm_pmu_pmc_is_high_counter(u64 select_idx)
> +{
> + return select_idx & 0x1;
> +}
> +
> +/**
> + * kvm_pmu_get_canonical_pmc - obtain the canonical pmc
> + * @pmc: The PMU counter pointer
> + *
> + * When a pair of PMCs are chained together we use the low counter
> (canonical)
> + * to hold the underlying perf event.
> + */
> +static struct kvm_pmc *kvm_pmu_get_canonical_pmc(struct kvm_pmc *pmc)
> +{
> + if (kvm_pmu_pmc_is_chained(pmc) &&
> + kvm_pmu_pmc_is_high_counter(pmc->idx))
> + return pmc - 1;
> +
> + return pmc;
> +}
> +
> +/**
> + * kvm_pmu_idx_has_chain_evtype - determine if the event type is chain
> * @vcpu: The vcpu pointer
> * @select_idx: The counter index
> */
> -u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64 select_idx)
> +static bool kvm_pmu_idx_has_chain_evtype(struct kvm_vcpu *vcpu, u64
> select_idx)
> {
> - u64 counter, reg, enabled, running;
> - struct kvm_pmu *pmu = &vcpu->arch.pmu;
> - struct kvm_pmc *pmc = &pmu->pmc[select_idx];
> + u64 eventsel, reg;
>
> - reg = (select_idx == ARMV8_PMU_CYCLE_IDX)
> - ? PMCCNTR_EL0 : PMEVCNTR0_EL0 + select_idx;
> - counter = __vcpu_sys_reg(vcpu, reg);
> + select_idx |= 0x1;
> +
> + if (select_idx == ARMV8_PMU_CYCLE_IDX)
> + return false;
> +
> + reg = PMEVTYPER0_EL0 + select_idx;
> + eventsel = __vcpu_sys_reg(vcpu, reg) & ARMV8_PMU_EVTYPE_EVENT;
> +
> + return armv8pmu_evtype_is_chain(eventsel);
> +}
> +
> +/**
> + * kvm_pmu_get_pair_counter_value - get PMU counter value
> + * @vcpu: The vcpu pointer
> + * @pmc: The PMU counter pointer
> + */
> +static u64 kvm_pmu_get_pair_counter_value(struct kvm_vcpu *vcpu,
> + struct kvm_pmc *pmc)
> +{
> + u64 counter, counter_high, reg, enabled, running;
> +
> + if (kvm_pmu_pmc_is_chained(pmc)) {
> + pmc = kvm_pmu_get_canonical_pmc(pmc);
> + reg = PMEVCNTR0_EL0 + pmc->idx;
> +
> + counter = __vcpu_sys_reg(vcpu, reg);
> + counter_high = __vcpu_sys_reg(vcpu, reg + 1);
> +
> + counter = lower_32_bits(counter) | (counter_high << 32);
> + } else {
> + reg = (pmc->idx == ARMV8_PMU_CYCLE_IDX)
> + ? PMCCNTR_EL0 : PMEVCNTR0_EL0 + pmc->idx;
> + counte
Re: [PATCH] KVM: ARM64: Update perf event when setting PMU count value
On Sun, May 19, 2019 at 06:05:59PM +0800, Xiang Zheng wrote:
> Guest will adjust the sample period and set PMU counter value when
> it takes a long time to handle the PMU interrupts.
>
> However, we don't have a corresponding change on the virtual PMU
> which is emulated via a perf event. It could cause a large number
> of PMU interrupts injected to guest. Then guest will get hang for
> handling these interrupts.
Yes this is indeed an issue. I believe I've addressed this in my 'chained
pmu' series - the relevant patch is here...
https://lists.cs.columbia.edu/pipermail/kvmarm/2019-May/035933.html
Some other comments below.
>
> So update the sample_period of perf event if the counter value is
> changed to avoid this case.
>
> Signed-off-by: Xiang Zheng
> ---
> virt/kvm/arm/pmu.c | 54
> +-
> 1 file changed, 45 insertions(+), 9 deletions(-)
>
> diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
> index 1c5b76c..cbad3ec 100644
> --- a/virt/kvm/arm/pmu.c
> +++ b/virt/kvm/arm/pmu.c
> @@ -24,6 +24,11 @@
> #include
> #include
>
> +static void kvm_pmu_stop_counter(struct kvm_vcpu *vcpu, struct kvm_pmc *pmc);
> +static struct perf_event *kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu,
> + struct kvm_pmc *pmc,
> + struct perf_event_attr
> *attr);
> +
> /**
> * kvm_pmu_get_counter_value - get PMU counter value
> * @vcpu: The vcpu pointer
> @@ -57,11 +62,29 @@ u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64
> select_idx)
> */
> void kvm_pmu_set_counter_value(struct kvm_vcpu *vcpu, u64 select_idx, u64
> val)
> {
> - u64 reg;
> + u64 reg, counter, old_sample_period;
> + struct kvm_pmu *pmu = &vcpu->arch.pmu;
> + struct kvm_pmc *pmc = &pmu->pmc[select_idx];
> + struct perf_event *event;
> + struct perf_event_attr attr;
>
> reg = (select_idx == ARMV8_PMU_CYCLE_IDX)
> ? PMCCNTR_EL0 : PMEVCNTR0_EL0 + select_idx;
> __vcpu_sys_reg(vcpu, reg) += (s64)val - kvm_pmu_get_counter_value(vcpu,
> select_idx);
> +
> + if (pmc->perf_event) {
> + attr = pmc->perf_event->attr;
> + old_sample_period = attr.sample_period;
> + counter = kvm_pmu_get_counter_value(vcpu, select_idx);
> + attr.sample_period = (-counter) & pmc->bitmask;
> + if (attr.sample_period == old_sample_period)
> + return;
I'd be interested to know how often this would evaluate to true.
> +
> + kvm_pmu_stop_counter(vcpu, pmc);
> + event = kvm_pmu_create_perf_event(vcpu, pmc, &attr);
I'm not sure it's necessary to change the prototype of kvm_pmu_create_perf_event
as this function will recalculate the sample period based on the updated counter
value anyway.
Thanks,
Andrew Murray
> + if (event)
> + pmc->perf_event = event;
> + }
> }
>
> /**
> @@ -303,6 +326,24 @@ static void kvm_pmu_perf_overflow(struct perf_event
> *perf_event,
> }
> }
>
> +static struct perf_event *kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu,
> + struct kvm_pmc *pmc,
> + struct perf_event_attr
> *attr)
> +{
> + struct perf_event *event;
> +
> + event = perf_event_create_kernel_counter(attr, -1, current,
> + kvm_pmu_perf_overflow, pmc);
> +
> + if (IS_ERR(event)) {
> + pr_err_once("kvm: pmu event creation failed %ld\n",
> + PTR_ERR(event));
> + return NULL;
> + }
> +
> + return event;
> +}
> +
> /**
> * kvm_pmu_software_increment - do software increment
> * @vcpu: The vcpu pointer
> @@ -416,15 +457,10 @@ void kvm_pmu_set_counter_event_type(struct kvm_vcpu
> *vcpu, u64 data,
> /* The initial sample period (overflow count) of an event. */
> attr.sample_period = (-counter) & pmc->bitmask;
>
> - event = perf_event_create_kernel_counter(&attr, -1, current,
> - kvm_pmu_perf_overflow, pmc);
> - if (IS_ERR(event)) {
> - pr_err_once("kvm: pmu event creation failed %ld\n",
> - PTR_ERR(event));
> - return;
> - }
> + event = kvm_pmu_create_perf_event(vcpu, pmc, &attr);
>
> - pmc->perf_event = event;
> + if (event)
> + pmc->perf_event = event;
> }
>
> bool kvm_arm_support_pmu_v3(void)
> --
> 1.8.3.1
>
>
> ___
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v7 5/5] KVM: arm/arm64: support chained PMU counters
On 21/05/2019 16:52, Andrew Murray wrote:
> ARMv8 provides support for chained PMU counters, where an event type
> of 0x001E is set for odd-numbered counters, the event counter will
> increment by one for each overflow of the preceding even-numbered
> counter. Let's emulate this in KVM by creating a 64 bit perf counter
> when a user chains two emulated counters together.
>
> For chained events we only support generating an overflow interrupt
> on the high counter. We use the attributes of the low counter to
> determine the attributes of the perf event.
>
> Suggested-by: Marc Zyngier
> Signed-off-by: Andrew Murray
> ---
> include/kvm/arm_pmu.h | 2 +
> virt/kvm/arm/pmu.c| 246 --
> 2 files changed, 215 insertions(+), 33 deletions(-)
>
> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> index b73f31baca52..8b434745500a 100644
> --- a/include/kvm/arm_pmu.h
> +++ b/include/kvm/arm_pmu.h
> @@ -22,6 +22,7 @@
> #include
>
> #define ARMV8_PMU_CYCLE_IDX (ARMV8_PMU_MAX_COUNTERS - 1)
> +#define ARMV8_PMU_MAX_COUNTER_PAIRS ((ARMV8_PMU_MAX_COUNTERS + 1) >> 1)
>
> #ifdef CONFIG_KVM_ARM_PMU
>
> @@ -34,6 +35,7 @@ struct kvm_pmc {
> struct kvm_pmu {
> int irq_num;
> struct kvm_pmc pmc[ARMV8_PMU_MAX_COUNTERS];
> + DECLARE_BITMAP(chained, ARMV8_PMU_MAX_COUNTER_PAIRS);
> bool ready;
> bool created;
> bool irq_level;
> diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
> index ae1e886d4a1a..4b0981c402c6 100644
> --- a/virt/kvm/arm/pmu.c
> +++ b/virt/kvm/arm/pmu.c
> @@ -25,28 +25,128 @@
> #include
>
> static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx);
> +
> +#define PERF_ATTR_CFG1_KVM_PMU_CHAINED 0x1
> +
> +static struct kvm_vcpu *kvm_pmc_to_vcpu(struct kvm_pmc *pmc)
> +{
> + struct kvm_pmu *pmu;
> + struct kvm_vcpu_arch *vcpu_arch;
> +
> + pmc -= pmc->idx;
> + pmu = container_of(pmc, struct kvm_pmu, pmc[0]);
> + vcpu_arch = container_of(pmu, struct kvm_vcpu_arch, pmu);
> + return container_of(vcpu_arch, struct kvm_vcpu, arch);
> +}
> +
> /**
> - * kvm_pmu_get_counter_value - get PMU counter value
> + * kvm_pmu_pmc_is_chained - determine if the pmc is chained
> + * @pmc: The PMU counter pointer
> + */
> +static bool kvm_pmu_pmc_is_chained(struct kvm_pmc *pmc)
> +{
> + struct kvm_vcpu *vcpu = kvm_pmc_to_vcpu(pmc);
> +
> + return test_bit(pmc->idx >> 1, vcpu->arch.pmu.chained);
> +}
> +
> +/**
> + * kvm_pmu_pmc_is_high_counter - determine if select_idx is a high/low
> counter
> + * @select_idx: The counter index
> + */
> +static bool kvm_pmu_pmc_is_high_counter(u64 select_idx)
> +{
> + return select_idx & 0x1;
> +}
> +
> +/**
> + * kvm_pmu_get_canonical_pmc - obtain the canonical pmc
> + * @pmc: The PMU counter pointer
> + *
> + * When a pair of PMCs are chained together we use the low counter
> (canonical)
> + * to hold the underlying perf event.
> + */
> +static struct kvm_pmc *kvm_pmu_get_canonical_pmc(struct kvm_pmc *pmc)
> +{
> + if (kvm_pmu_pmc_is_chained(pmc) &&
> + kvm_pmu_pmc_is_high_counter(pmc->idx))
> + return pmc - 1;
> +
> + return pmc;
> +}
> +
> +/**
> + * kvm_pmu_idx_has_chain_evtype - determine if the event type is chain
> * @vcpu: The vcpu pointer
> * @select_idx: The counter index
> */
> -u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64 select_idx)
> +static bool kvm_pmu_idx_has_chain_evtype(struct kvm_vcpu *vcpu, u64
> select_idx)
> {
> - u64 counter, reg, enabled, running;
> - struct kvm_pmu *pmu = &vcpu->arch.pmu;
> - struct kvm_pmc *pmc = &pmu->pmc[select_idx];
> + u64 eventsel, reg;
>
> - reg = (select_idx == ARMV8_PMU_CYCLE_IDX)
> - ? PMCCNTR_EL0 : PMEVCNTR0_EL0 + select_idx;
> - counter = __vcpu_sys_reg(vcpu, reg);
> + select_idx |= 0x1;
> +
> + if (select_idx == ARMV8_PMU_CYCLE_IDX)
> + return false;
> +
> + reg = PMEVTYPER0_EL0 + select_idx;
> + eventsel = __vcpu_sys_reg(vcpu, reg) & ARMV8_PMU_EVTYPE_EVENT;
> +
> + return armv8pmu_evtype_is_chain(eventsel);
> +}
> +
> +/**
> + * kvm_pmu_get_pair_counter_value - get PMU counter value
> + * @vcpu: The vcpu pointer
> + * @pmc: The PMU counter pointer
> + */
> +static u64 kvm_pmu_get_pair_counter_value(struct kvm_vcpu *vcpu,
> + struct kvm_pmc *pmc)
> +{
> + u64 counter, counter_high, reg, enabled, running;
> +
> + if (kvm_pmu_pmc_is_chained(pmc)) {
> + pmc = kvm_pmu_get_canonical_pmc(pmc);
> + reg = PMEVCNTR0_EL0 + pmc->idx;
> +
> + counter = __vcpu_sys_reg(vcpu, reg);
> + counter_high = __vcpu_sys_reg(vcpu, reg + 1);
> +
> + counter = lower_32_bits(counter) | (counter_high << 32);
> + } else {
> + reg = (pmc->idx == ARMV8_PMU_CYCLE_IDX)
> + ? PMCCNTR_EL0 : PMEVCNTR0_EL0 + pmc->idx;
> + counter = __vcpu_sys_
Re: [PATCH v7 4/5] arm64: perf: extract chain helper into header
On 21/05/2019 16:52, Andrew Murray wrote:
The ARMv8 Performance Monitors Extension includes an architectural
event type named CHAIN which allows for chaining counters together.
Let's extract the test for this event into a header file such that
other users, such as KVM (for PMU emulation) can make use of.
Signed-off-by: Andrew Murray
---
arch/arm64/include/asm/perf_event.h | 5 +
arch/arm64/kernel/perf_event.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/perf_event.h
b/arch/arm64/include/asm/perf_event.h
index c593761ba61c..cd13f3fd1055 100644
--- a/arch/arm64/include/asm/perf_event.h
+++ b/arch/arm64/include/asm/perf_event.h
@@ -219,6 +219,11 @@
#define ARMV8_PMU_USERENR_CR (1 << 2) /* Cycle counter can be read at EL0 */
#define ARMV8_PMU_USERENR_ER (1 << 3) /* Event counter can be read at EL0 */
+static inline bool armv8pmu_evtype_is_chain(u64 evtype)
+{
+ return (evtype == ARMV8_PMUV3_PERFCTR_CHAIN);
+}
+
#ifdef CONFIG_PERF_EVENTS
struct pt_regs;
extern unsigned long perf_instruction_pointer(struct pt_regs *regs);
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 314b1adedf06..265bd835a724 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -879,7 +879,7 @@ static int armv8pmu_set_event_filter(struct hw_perf_event
*event,
static int armv8pmu_filter_match(struct perf_event *event)
{
unsigned long evtype = event->hw.config_base & ARMV8_PMU_EVTYPE_EVENT;
- return evtype != ARMV8_PMUV3_PERFCTR_CHAIN;
+ return !armv8pmu_evtype_is_chain(evtype);
}
static void armv8pmu_reset(void *info)
Reviewed-by: Suzuki K Poulose
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v7 3/5] KVM: arm/arm64: re-create event when setting counter value
The perf event sample_period is currently set based upon the current
counter value, when PMXEVTYPER is written to and the perf event is created.
However the user may choose to write the type before the counter value in
which case sample_period will be set incorrectly. Let's instead decouple
event creation from PMXEVTYPER and (re)create the event in either
suitation.
Signed-off-by: Andrew Murray
Reviewed-by: Julien Thierry
Reviewed-by: Suzuki K Poulose
---
virt/kvm/arm/pmu.c | 42 +-
1 file changed, 33 insertions(+), 9 deletions(-)
diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
index 6e7c179103a6..ae1e886d4a1a 100644
--- a/virt/kvm/arm/pmu.c
+++ b/virt/kvm/arm/pmu.c
@@ -24,6 +24,7 @@
#include
#include
+static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx);
/**
* kvm_pmu_get_counter_value - get PMU counter value
* @vcpu: The vcpu pointer
@@ -62,6 +63,9 @@ void kvm_pmu_set_counter_value(struct kvm_vcpu *vcpu, u64
select_idx, u64 val)
reg = (select_idx == ARMV8_PMU_CYCLE_IDX)
? PMCCNTR_EL0 : PMEVCNTR0_EL0 + select_idx;
__vcpu_sys_reg(vcpu, reg) += (s64)val - kvm_pmu_get_counter_value(vcpu,
select_idx);
+
+ /* Recreate the perf event to reflect the updated sample_period */
+ kvm_pmu_create_perf_event(vcpu, select_idx);
}
/**
@@ -378,23 +382,21 @@ static bool kvm_pmu_counter_is_enabled(struct kvm_vcpu
*vcpu, u64 select_idx)
}
/**
- * kvm_pmu_set_counter_event_type - set selected counter to monitor some event
+ * kvm_pmu_create_perf_event - create a perf event for a counter
* @vcpu: The vcpu pointer
- * @data: The data guest writes to PMXEVTYPER_EL0
* @select_idx: The number of selected counter
- *
- * When OS accesses PMXEVTYPER_EL0, that means it wants to set a PMC to count
an
- * event with given hardware event number. Here we call perf_event API to
- * emulate this action and create a kernel perf event for it.
*/
-void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data,
- u64 select_idx)
+static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx)
{
struct kvm_pmu *pmu = &vcpu->arch.pmu;
struct kvm_pmc *pmc = &pmu->pmc[select_idx];
struct perf_event *event;
struct perf_event_attr attr;
- u64 eventsel, counter;
+ u64 eventsel, counter, reg, data;
+
+ reg = (select_idx == ARMV8_PMU_CYCLE_IDX)
+ ? PMCCFILTR_EL0 : PMEVTYPER0_EL0 + select_idx;
+ data = __vcpu_sys_reg(vcpu, reg);
kvm_pmu_stop_counter(vcpu, pmc);
eventsel = data & ARMV8_PMU_EVTYPE_EVENT;
@@ -431,6 +433,28 @@ void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu,
u64 data,
pmc->perf_event = event;
}
+/**
+ * kvm_pmu_set_counter_event_type - set selected counter to monitor some event
+ * @vcpu: The vcpu pointer
+ * @data: The data guest writes to PMXEVTYPER_EL0
+ * @select_idx: The number of selected counter
+ *
+ * When OS accesses PMXEVTYPER_EL0, that means it wants to set a PMC to count
an
+ * event with given hardware event number. Here we call perf_event API to
+ * emulate this action and create a kernel perf event for it.
+ */
+void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data,
+ u64 select_idx)
+{
+ u64 reg, event_type = data & ARMV8_PMU_EVTYPE_MASK;
+
+ reg = (select_idx == ARMV8_PMU_CYCLE_IDX)
+ ? PMCCFILTR_EL0 : PMEVTYPER0_EL0 + select_idx;
+
+ __vcpu_sys_reg(vcpu, reg) = event_type;
+ kvm_pmu_create_perf_event(vcpu, select_idx);
+}
+
bool kvm_arm_support_pmu_v3(void)
{
/*
--
2.21.0
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v7 5/5] KVM: arm/arm64: support chained PMU counters
ARMv8 provides support for chained PMU counters, where an event type
of 0x001E is set for odd-numbered counters, the event counter will
increment by one for each overflow of the preceding even-numbered
counter. Let's emulate this in KVM by creating a 64 bit perf counter
when a user chains two emulated counters together.
For chained events we only support generating an overflow interrupt
on the high counter. We use the attributes of the low counter to
determine the attributes of the perf event.
Suggested-by: Marc Zyngier
Signed-off-by: Andrew Murray
---
include/kvm/arm_pmu.h | 2 +
virt/kvm/arm/pmu.c| 246 --
2 files changed, 215 insertions(+), 33 deletions(-)
diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index b73f31baca52..8b434745500a 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -22,6 +22,7 @@
#include
#define ARMV8_PMU_CYCLE_IDX(ARMV8_PMU_MAX_COUNTERS - 1)
+#define ARMV8_PMU_MAX_COUNTER_PAIRS((ARMV8_PMU_MAX_COUNTERS + 1) >> 1)
#ifdef CONFIG_KVM_ARM_PMU
@@ -34,6 +35,7 @@ struct kvm_pmc {
struct kvm_pmu {
int irq_num;
struct kvm_pmc pmc[ARMV8_PMU_MAX_COUNTERS];
+ DECLARE_BITMAP(chained, ARMV8_PMU_MAX_COUNTER_PAIRS);
bool ready;
bool created;
bool irq_level;
diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
index ae1e886d4a1a..4b0981c402c6 100644
--- a/virt/kvm/arm/pmu.c
+++ b/virt/kvm/arm/pmu.c
@@ -25,28 +25,128 @@
#include
static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx);
+
+#define PERF_ATTR_CFG1_KVM_PMU_CHAINED 0x1
+
+static struct kvm_vcpu *kvm_pmc_to_vcpu(struct kvm_pmc *pmc)
+{
+ struct kvm_pmu *pmu;
+ struct kvm_vcpu_arch *vcpu_arch;
+
+ pmc -= pmc->idx;
+ pmu = container_of(pmc, struct kvm_pmu, pmc[0]);
+ vcpu_arch = container_of(pmu, struct kvm_vcpu_arch, pmu);
+ return container_of(vcpu_arch, struct kvm_vcpu, arch);
+}
+
/**
- * kvm_pmu_get_counter_value - get PMU counter value
+ * kvm_pmu_pmc_is_chained - determine if the pmc is chained
+ * @pmc: The PMU counter pointer
+ */
+static bool kvm_pmu_pmc_is_chained(struct kvm_pmc *pmc)
+{
+ struct kvm_vcpu *vcpu = kvm_pmc_to_vcpu(pmc);
+
+ return test_bit(pmc->idx >> 1, vcpu->arch.pmu.chained);
+}
+
+/**
+ * kvm_pmu_pmc_is_high_counter - determine if select_idx is a high/low counter
+ * @select_idx: The counter index
+ */
+static bool kvm_pmu_pmc_is_high_counter(u64 select_idx)
+{
+ return select_idx & 0x1;
+}
+
+/**
+ * kvm_pmu_get_canonical_pmc - obtain the canonical pmc
+ * @pmc: The PMU counter pointer
+ *
+ * When a pair of PMCs are chained together we use the low counter (canonical)
+ * to hold the underlying perf event.
+ */
+static struct kvm_pmc *kvm_pmu_get_canonical_pmc(struct kvm_pmc *pmc)
+{
+ if (kvm_pmu_pmc_is_chained(pmc) &&
+ kvm_pmu_pmc_is_high_counter(pmc->idx))
+ return pmc - 1;
+
+ return pmc;
+}
+
+/**
+ * kvm_pmu_idx_has_chain_evtype - determine if the event type is chain
* @vcpu: The vcpu pointer
* @select_idx: The counter index
*/
-u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64 select_idx)
+static bool kvm_pmu_idx_has_chain_evtype(struct kvm_vcpu *vcpu, u64 select_idx)
{
- u64 counter, reg, enabled, running;
- struct kvm_pmu *pmu = &vcpu->arch.pmu;
- struct kvm_pmc *pmc = &pmu->pmc[select_idx];
+ u64 eventsel, reg;
- reg = (select_idx == ARMV8_PMU_CYCLE_IDX)
- ? PMCCNTR_EL0 : PMEVCNTR0_EL0 + select_idx;
- counter = __vcpu_sys_reg(vcpu, reg);
+ select_idx |= 0x1;
+
+ if (select_idx == ARMV8_PMU_CYCLE_IDX)
+ return false;
+
+ reg = PMEVTYPER0_EL0 + select_idx;
+ eventsel = __vcpu_sys_reg(vcpu, reg) & ARMV8_PMU_EVTYPE_EVENT;
+
+ return armv8pmu_evtype_is_chain(eventsel);
+}
+
+/**
+ * kvm_pmu_get_pair_counter_value - get PMU counter value
+ * @vcpu: The vcpu pointer
+ * @pmc: The PMU counter pointer
+ */
+static u64 kvm_pmu_get_pair_counter_value(struct kvm_vcpu *vcpu,
+ struct kvm_pmc *pmc)
+{
+ u64 counter, counter_high, reg, enabled, running;
+
+ if (kvm_pmu_pmc_is_chained(pmc)) {
+ pmc = kvm_pmu_get_canonical_pmc(pmc);
+ reg = PMEVCNTR0_EL0 + pmc->idx;
+
+ counter = __vcpu_sys_reg(vcpu, reg);
+ counter_high = __vcpu_sys_reg(vcpu, reg + 1);
+
+ counter = lower_32_bits(counter) | (counter_high << 32);
+ } else {
+ reg = (pmc->idx == ARMV8_PMU_CYCLE_IDX)
+ ? PMCCNTR_EL0 : PMEVCNTR0_EL0 + pmc->idx;
+ counter = __vcpu_sys_reg(vcpu, reg);
+ }
- /* The real counter value is equal to the value of counter register plus
+ /*
+* The real counter value is equal to the value of counter register plus
* the value perf event counts.
[PATCH v7 4/5] arm64: perf: extract chain helper into header
The ARMv8 Performance Monitors Extension includes an architectural
event type named CHAIN which allows for chaining counters together.
Let's extract the test for this event into a header file such that
other users, such as KVM (for PMU emulation) can make use of.
Signed-off-by: Andrew Murray
---
arch/arm64/include/asm/perf_event.h | 5 +
arch/arm64/kernel/perf_event.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/perf_event.h
b/arch/arm64/include/asm/perf_event.h
index c593761ba61c..cd13f3fd1055 100644
--- a/arch/arm64/include/asm/perf_event.h
+++ b/arch/arm64/include/asm/perf_event.h
@@ -219,6 +219,11 @@
#define ARMV8_PMU_USERENR_CR (1 << 2) /* Cycle counter can be read at EL0 */
#define ARMV8_PMU_USERENR_ER (1 << 3) /* Event counter can be read at EL0 */
+static inline bool armv8pmu_evtype_is_chain(u64 evtype)
+{
+ return (evtype == ARMV8_PMUV3_PERFCTR_CHAIN);
+}
+
#ifdef CONFIG_PERF_EVENTS
struct pt_regs;
extern unsigned long perf_instruction_pointer(struct pt_regs *regs);
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 314b1adedf06..265bd835a724 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -879,7 +879,7 @@ static int armv8pmu_set_event_filter(struct hw_perf_event
*event,
static int armv8pmu_filter_match(struct perf_event *event)
{
unsigned long evtype = event->hw.config_base & ARMV8_PMU_EVTYPE_EVENT;
- return evtype != ARMV8_PMUV3_PERFCTR_CHAIN;
+ return !armv8pmu_evtype_is_chain(evtype);
}
static void armv8pmu_reset(void *info)
--
2.21.0
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v7 0/5] KVM: arm/arm64: add support for chained counters
ARMv8 provides support for chained PMU counters, where an event type
of 0x001E is set for odd-numbered counters, the event counter will
increment by one for each overflow of the preceding even-numbered
counter. Let's emulate this in KVM by creating a 64 bit perf counter
when a user chains two emulated counters together.
Testing has been performed by hard-coding hwc->sample_period in
__hw_perf_event_init (arm_pmu.c) to a small value, this results in
regular overflows (for non sampling events). The following command
was then used to measure chained and non-chained instruction cycles:
perf stat -e armv8_pmuv3/long=1,inst_retired/u \
-e armv8_pmuv3/long=0,inst_retired/u dd if=/dev/zero bs=1M \
count=10 | gzip > /dev/null
The reported values were identical (and for non-chained was in the
same ballpark when running on a kernel without this patchset). Debug
was added to verify that the guest received overflow interrupts for
the chain counter.
For chained events we only support generating an overflow interrupt
on the high counter. We use the attributes of the low counter to
determine the attributes of the perf event.
Changes since v6:
- Drop kvm_pmu_{get,set}_perf_event
- Avoid duplicate work by using kvm_pmu_get_pair_counter_value inside
kvm_pmu_stop_counter
- Use GENMASK for 64bit mask
Changes since v5:
- Use kvm_pmu_pmc_is_high_counter instead of open coding
- Rename kvm_pmu_event_is_chained to kvm_pmu_idx_has_chain_evtype
- Use kvm_pmu_get_canonical_pmc only where needed and reintroduce
the kvm_pmu_{set, get}_perf_event functions
- Drop masking of counter in kvm_pmu_get_pair_counter_value
- Only initialise pmc once in kvm_pmu_create_perf_event and other
minor changes.
Changes since v4:
- Track pairs of chained counters with a bitmap instead of using
a struct kvm_pmc_pair.
- Rebase onto kvmarm/queue
Changes since v3:
- Simplify approach by not creating events lazily and by introducing
a struct kvm_pmc_pair to represent the relationship between
adjacent counters.
- Rebase onto v5.1-rc2
Changes since v2:
- Rebased onto v5.0-rc7
- Add check for cycle counter in correct patch
- Minor style, naming and comment changes
- Extract armv8pmu_evtype_is_chain from arch/arm64/kernel/perf_event.c
into a common header that KVM can use
Changes since v1:
- Rename kvm_pmu_{enable,disable}_counter to reflect that they can
operate on multiple counters at once and use these functions where
possible
- Fix bugs with overflow handing, kvm_pmu_get_counter_value did not
take into consideration the perf counter value overflowing the low
counter
- Ensure PMCCFILTR_EL0 is used when operating on the cycle counter
- Rename kvm_pmu_reenable_enabled_{pair, single} and similar
- Always create perf event disabled to simplify logic elsewhere
- Move PMCNTENSET_EL0 test to kvm_pmu_enable_counter_mask
Andrew Murray (5):
KVM: arm/arm64: rename kvm_pmu_{enable/disable}_counter functions
KVM: arm/arm64: extract duplicated code to own function
KVM: arm/arm64: re-create event when setting counter value
arm64: perf: extract chain helper into header
KVM: arm/arm64: support chained PMU counters
arch/arm64/include/asm/perf_event.h | 5 +
arch/arm64/kernel/perf_event.c | 2 +-
arch/arm64/kvm/sys_regs.c | 4 +-
include/kvm/arm_pmu.h | 10 +-
virt/kvm/arm/pmu.c | 322 +++-
5 files changed, 279 insertions(+), 64 deletions(-)
--
2.21.0
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v7 2/5] KVM: arm/arm64: extract duplicated code to own function
Let's reduce code duplication by extracting common code to its own
function.
Signed-off-by: Andrew Murray
Reviewed-by: Suzuki K Poulose
---
virt/kvm/arm/pmu.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
index c5a722ad283f..6e7c179103a6 100644
--- a/virt/kvm/arm/pmu.c
+++ b/virt/kvm/arm/pmu.c
@@ -64,6 +64,19 @@ void kvm_pmu_set_counter_value(struct kvm_vcpu *vcpu, u64
select_idx, u64 val)
__vcpu_sys_reg(vcpu, reg) += (s64)val - kvm_pmu_get_counter_value(vcpu,
select_idx);
}
+/**
+ * kvm_pmu_release_perf_event - remove the perf event
+ * @pmc: The PMU counter pointer
+ */
+static void kvm_pmu_release_perf_event(struct kvm_pmc *pmc)
+{
+ if (pmc->perf_event) {
+ perf_event_disable(pmc->perf_event);
+ perf_event_release_kernel(pmc->perf_event);
+ pmc->perf_event = NULL;
+ }
+}
+
/**
* kvm_pmu_stop_counter - stop PMU counter
* @pmc: The PMU counter pointer
@@ -79,9 +92,7 @@ static void kvm_pmu_stop_counter(struct kvm_vcpu *vcpu,
struct kvm_pmc *pmc)
reg = (pmc->idx == ARMV8_PMU_CYCLE_IDX)
? PMCCNTR_EL0 : PMEVCNTR0_EL0 + pmc->idx;
__vcpu_sys_reg(vcpu, reg) = counter;
- perf_event_disable(pmc->perf_event);
- perf_event_release_kernel(pmc->perf_event);
- pmc->perf_event = NULL;
+ kvm_pmu_release_perf_event(pmc);
}
}
@@ -112,15 +123,8 @@ void kvm_pmu_vcpu_destroy(struct kvm_vcpu *vcpu)
int i;
struct kvm_pmu *pmu = &vcpu->arch.pmu;
- for (i = 0; i < ARMV8_PMU_MAX_COUNTERS; i++) {
- struct kvm_pmc *pmc = &pmu->pmc[i];
-
- if (pmc->perf_event) {
- perf_event_disable(pmc->perf_event);
- perf_event_release_kernel(pmc->perf_event);
- pmc->perf_event = NULL;
- }
- }
+ for (i = 0; i < ARMV8_PMU_MAX_COUNTERS; i++)
+ kvm_pmu_release_perf_event(&pmu->pmc[i]);
}
u64 kvm_pmu_valid_counter_mask(struct kvm_vcpu *vcpu)
--
2.21.0
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v7 1/5] KVM: arm/arm64: rename kvm_pmu_{enable/disable}_counter functions
The kvm_pmu_{enable/disable}_counter functions can enabled/disable
multiple counters at once as they operate on a bitmask. Let's
make this clearer by renaming the function.
Suggested-by: Suzuki K Poulose
Signed-off-by: Andrew Murray
Reviewed-by: Julien Thierry
Reviewed-by: Suzuki K Poulose
---
arch/arm64/kvm/sys_regs.c | 4 ++--
include/kvm/arm_pmu.h | 8
virt/kvm/arm/pmu.c| 12 ++--
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 9d02643bc601..8e98fb173ed3 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -876,12 +876,12 @@ static bool access_pmcnten(struct kvm_vcpu *vcpu, struct
sys_reg_params *p,
if (r->Op2 & 0x1) {
/* accessing PMCNTENSET_EL0 */
__vcpu_sys_reg(vcpu, PMCNTENSET_EL0) |= val;
- kvm_pmu_enable_counter(vcpu, val);
+ kvm_pmu_enable_counter_mask(vcpu, val);
kvm_vcpu_pmu_restore_guest(vcpu);
} else {
/* accessing PMCNTENCLR_EL0 */
__vcpu_sys_reg(vcpu, PMCNTENSET_EL0) &= ~val;
- kvm_pmu_disable_counter(vcpu, val);
+ kvm_pmu_disable_counter_mask(vcpu, val);
}
} else {
p->regval = __vcpu_sys_reg(vcpu, PMCNTENSET_EL0) & mask;
diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index f87fe20fcb05..b73f31baca52 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -46,8 +46,8 @@ void kvm_pmu_set_counter_value(struct kvm_vcpu *vcpu, u64
select_idx, u64 val);
u64 kvm_pmu_valid_counter_mask(struct kvm_vcpu *vcpu);
void kvm_pmu_vcpu_reset(struct kvm_vcpu *vcpu);
void kvm_pmu_vcpu_destroy(struct kvm_vcpu *vcpu);
-void kvm_pmu_disable_counter(struct kvm_vcpu *vcpu, u64 val);
-void kvm_pmu_enable_counter(struct kvm_vcpu *vcpu, u64 val);
+void kvm_pmu_disable_counter_mask(struct kvm_vcpu *vcpu, u64 val);
+void kvm_pmu_enable_counter_mask(struct kvm_vcpu *vcpu, u64 val);
void kvm_pmu_flush_hwstate(struct kvm_vcpu *vcpu);
void kvm_pmu_sync_hwstate(struct kvm_vcpu *vcpu);
bool kvm_pmu_should_notify_user(struct kvm_vcpu *vcpu);
@@ -83,8 +83,8 @@ static inline u64 kvm_pmu_valid_counter_mask(struct kvm_vcpu
*vcpu)
}
static inline void kvm_pmu_vcpu_reset(struct kvm_vcpu *vcpu) {}
static inline void kvm_pmu_vcpu_destroy(struct kvm_vcpu *vcpu) {}
-static inline void kvm_pmu_disable_counter(struct kvm_vcpu *vcpu, u64 val) {}
-static inline void kvm_pmu_enable_counter(struct kvm_vcpu *vcpu, u64 val) {}
+static inline void kvm_pmu_disable_counter_mask(struct kvm_vcpu *vcpu, u64
val) {}
+static inline void kvm_pmu_enable_counter_mask(struct kvm_vcpu *vcpu, u64 val)
{}
static inline void kvm_pmu_flush_hwstate(struct kvm_vcpu *vcpu) {}
static inline void kvm_pmu_sync_hwstate(struct kvm_vcpu *vcpu) {}
static inline bool kvm_pmu_should_notify_user(struct kvm_vcpu *vcpu)
diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
index 1c5b76c46e26..c5a722ad283f 100644
--- a/virt/kvm/arm/pmu.c
+++ b/virt/kvm/arm/pmu.c
@@ -135,13 +135,13 @@ u64 kvm_pmu_valid_counter_mask(struct kvm_vcpu *vcpu)
}
/**
- * kvm_pmu_enable_counter - enable selected PMU counter
+ * kvm_pmu_enable_counter_mask - enable selected PMU counters
* @vcpu: The vcpu pointer
* @val: the value guest writes to PMCNTENSET register
*
* Call perf_event_enable to start counting the perf event
*/
-void kvm_pmu_enable_counter(struct kvm_vcpu *vcpu, u64 val)
+void kvm_pmu_enable_counter_mask(struct kvm_vcpu *vcpu, u64 val)
{
int i;
struct kvm_pmu *pmu = &vcpu->arch.pmu;
@@ -164,13 +164,13 @@ void kvm_pmu_enable_counter(struct kvm_vcpu *vcpu, u64
val)
}
/**
- * kvm_pmu_disable_counter - disable selected PMU counter
+ * kvm_pmu_disable_counter_mask - disable selected PMU counters
* @vcpu: The vcpu pointer
* @val: the value guest writes to PMCNTENCLR register
*
* Call perf_event_disable to stop counting the perf event
*/
-void kvm_pmu_disable_counter(struct kvm_vcpu *vcpu, u64 val)
+void kvm_pmu_disable_counter_mask(struct kvm_vcpu *vcpu, u64 val)
{
int i;
struct kvm_pmu *pmu = &vcpu->arch.pmu;
@@ -347,10 +347,10 @@ void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val)
mask = kvm_pmu_valid_counter_mask(vcpu);
if (val & ARMV8_PMU_PMCR_E) {
- kvm_pmu_enable_counter(vcpu,
+ kvm_pmu_enable_counter_mask(vcpu,
__vcpu_sys_reg(vcpu, PMCNTENSET_EL0) & mask);
} else {
- kvm_pmu_disable_counter(vcpu, mask);
+ kvm_pmu_disable_counter_mask(vcpu, mask);
}
if (val & ARMV8_PMU_PMCR_C)
--
2.21.0
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v6 5/5] KVM: arm/arm64: support chained PMU counters
On Wed, May 15, 2019 at 09:25:46AM +0100, Julien Thierry wrote:
> Hi Andrew,
>
> It might be just my personal taste, but I'm not entirely sold on the
> introduction of get/set_perf_event. Having both this and the concept of
> getting the "canonical" pmc feels a bit redundant, or that we're
> hiding/mixing stuff.
>
> In most cases not calling the getter/setter feels simpler, see below.
Yes this is very similar to the canonical stuff. I had a go at completely
removing the get/set_perf_event calls - I think it's better (and certainly
less code). I'll post the respin shortly. Thanks for this suggestion.
>
> On 09/05/2019 16:32, Andrew Murray wrote:
> > ARMv8 provides support for chained PMU counters, where an event type
> > of 0x001E is set for odd-numbered counters, the event counter will
> > increment by one for each overflow of the preceding even-numbered
> > counter. Let's emulate this in KVM by creating a 64 bit perf counter
> > when a user chains two emulated counters together.
> >
> > For chained events we only support generating an overflow interrupt
> > on the high counter. We use the attributes of the low counter to
> > determine the attributes of the perf event.
> >
> > Suggested-by: Marc Zyngier
> > Signed-off-by: Andrew Murray
> > ---
> > include/kvm/arm_pmu.h | 2 +
> > virt/kvm/arm/pmu.c| 298 +++---
> > 2 files changed, 256 insertions(+), 44 deletions(-)
> >
> > diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> > index b73f31baca52..8b434745500a 100644
> > --- a/include/kvm/arm_pmu.h
> > +++ b/include/kvm/arm_pmu.h
> > @@ -22,6 +22,7 @@
> > #include
> >
> > #define ARMV8_PMU_CYCLE_IDX(ARMV8_PMU_MAX_COUNTERS - 1)
> > +#define ARMV8_PMU_MAX_COUNTER_PAIRS((ARMV8_PMU_MAX_COUNTERS + 1)
> > >> 1)
>
> Minor nit: I should have pointed that out on the previous version. To me
> it would seem more obvious to use "(ARMV8_PMU_MAX_COUNTERS + 1) / 2"
> rather than shifting right. Granted that anyone reading these sources
> are likely to know that both are equivalent, but logically there are
> half the number of counter pairs than there are counters.
>
I think I'd prefer using the shift. In the unlikely event that
ARMV8_PMU_MAX_COUNTERS becomes an odd value - then it's less ambiguous
how we handle it. Also we use shift throughout the code to get from the
pmc to the canonical pmc.
Thanks,
Andrew Murray
> >
> > #ifdef CONFIG_KVM_ARM_PMU
> >
> > @@ -34,6 +35,7 @@ struct kvm_pmc {
> > struct kvm_pmu {
> > int irq_num;
> > struct kvm_pmc pmc[ARMV8_PMU_MAX_COUNTERS];
> > + DECLARE_BITMAP(chained, ARMV8_PMU_MAX_COUNTER_PAIRS);
> > bool ready;
> > bool created;
> > bool irq_level;
> > diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
> > index ae1e886d4a1a..6cdd9746f071 100644
> > --- a/virt/kvm/arm/pmu.c
> > +++ b/virt/kvm/arm/pmu.c
> > @@ -25,6 +25,138 @@
> > #include
> >
> > static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64
> > select_idx);
> > +
> > +#define PERF_ATTR_CFG1_KVM_PMU_CHAINED 0x1
> > +
> > +static struct kvm_vcpu *kvm_pmc_to_vcpu(struct kvm_pmc *pmc)
> > +{
> > + struct kvm_pmu *pmu;
> > + struct kvm_vcpu_arch *vcpu_arch;
> > +
> > + pmc -= pmc->idx;
> > + pmu = container_of(pmc, struct kvm_pmu, pmc[0]);
> > + vcpu_arch = container_of(pmu, struct kvm_vcpu_arch, pmu);
> > + return container_of(vcpu_arch, struct kvm_vcpu, arch);
> > +}
> > +
> > +/**
> > + * kvm_pmu_pmc_is_chained - determine if the pmc is chained
> > + * @pmc: The PMU counter pointer
> > + */
> > +static bool kvm_pmu_pmc_is_chained(struct kvm_pmc *pmc)
> > +{
> > + struct kvm_vcpu *vcpu = kvm_pmc_to_vcpu(pmc);
> > +
> > + return test_bit(pmc->idx >> 1, vcpu->arch.pmu.chained);
> > +}
> > +
> > +/**
> > + * kvm_pmu_pmc_is_high_counter - determine if select_idx is a high/low
> > counter
> > + * @select_idx: The counter index
> > + */
> > +static bool kvm_pmu_pmc_is_high_counter(u64 select_idx)
> > +{
> > + return select_idx & 0x1;
> > +}
> > +
> > +/**
> > + * kvm_pmu_get_canonical_pmc - obtain the canonical pmc
> > + * @pmc: The PMU counter pointer
> > + *
> > + * When a pair of PMCs are chained together we use the low counter
> > (canonical)
> > + * to hold the underlying perf event.
> > + */
> > +static struct kvm_pmc *kvm_pmu_get_canonical_pmc(struct kvm_pmc *pmc)
> > +{
> > + if (kvm_pmu_pmc_is_chained(pmc) &&
> > + kvm_pmu_pmc_is_high_counter(pmc->idx))
> > + return pmc - 1;
> > +
> > + return pmc;
> > +}
> > +
> > +/**
> > + * kvm_pmu_idx_has_chain_evtype - determine if the event type is chain
> > + * @vcpu: The vcpu pointer
> > + * @select_idx: The counter index
> > + */
> > +static bool kvm_pmu_idx_has_chain_evtype(struct kvm_vcpu *vcpu, u64
> > select_idx)
> > +{
> > + u64 eventsel, reg;
> > +
> > + select_idx |= 0x1;
> > +
> > + if (select_idx == ARMV8_PMU_CYCLE_IDX)
> > + return false;
> > +
> > + reg = PMEVTYPER0_EL0
[PATCH] MAINTAINERS: KVM: arm/arm64: Remove myself as maintainer
I no longer have time to actively review patches and manage the tree and it's time to make that official. Huge thanks to the incredible Linux community and all the contributors who have put up with me over the past years. I also take this opportunity to remove the website link to the Columbia web page, as that information is no longer up to date and I don't know who manages that anymore. Signed-off-by: Christoffer Dall --- MAINTAINERS | 2 -- 1 file changed, 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 5cfbea4ce575..4ba271a8e0ef 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8611,14 +8611,12 @@ F: arch/x86/include/asm/svm.h F: arch/x86/kvm/svm.c KERNEL VIRTUAL MACHINE FOR ARM/ARM64 (KVM/arm, KVM/arm64) -M: Christoffer Dall M: Marc Zyngier R: James Morse R: Julien Thierry R: Suzuki K Pouloze L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) L: kvmarm@lists.cs.columbia.edu -W: http://systems.cs.columbia.edu/projects/kvm-arm T: git git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git S: Maintained F: arch/arm/include/uapi/asm/kvm* -- 2.18.0 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
