Re: [PATCH RFC 0/2] ignore LBR-related MSRs

From: Jiri Olsa
Date: Wed Dec 06 2017 - 11:23:14 EST


On Wed, Dec 06, 2017 at 02:43:01PM +0300, Jan Dakinevich wrote:
> w2k16 essentials fails to boot if underlying hypervisor lacks of support for
> LBR MSRs. To workaround the issue, it suggessted to ignore these MSRs (but not
> all).
>
> The information, which MSRs are supported for specific platform is taken from
> perf, it is the subject of the first patch. The second patch adds ignoring for
> these MSRs to pmu_intel code of KVM.
>
> TODO: use MSR load/store areas to make full support of LBR debug.
>
> Jan Dakinevich (2):
> perf/x86/intel: make reusable LBR initialization code
> KVM: x86/vPMU: ignore access to LBR-related MSRs
>
> arch/x86/events/core.c | 8 +-
> arch/x86/events/intel/core.c | 59 +++------
> arch/x86/events/intel/lbr.c | 272 +++++++++++++++++++++++++-------------
> arch/x86/events/perf_event.h | 27 +---
> arch/x86/include/asm/kvm_host.h | 2 +
> arch/x86/include/asm/perf_event.h | 11 ++
> arch/x86/kvm/pmu_intel.c | 33 +++++
> 7 files changed, 250 insertions(+), 162 deletions(-)

unrelated, but while looking on this, dont we miss task_ctx_data
allocation for child context? like in attached change

jirka


---
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 494eca1bc760..7c5de1160545 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10699,6 +10699,7 @@ const struct perf_event_attr *perf_event_attrs(struct perf_event *event)
struct perf_event_context *child_ctx)
{
enum perf_event_state parent_state = parent_event->state;
+ void *task_ctx_data = NULL;
struct perf_event *child_event;
unsigned long flags;

@@ -10719,6 +10720,19 @@ const struct perf_event_attr *perf_event_attrs(struct perf_event *event)
if (IS_ERR(child_event))
return child_event;

+
+ if ((child_event->attach_state & PERF_ATTACH_TASK_DATA) &&
+ !child_ctx->task_ctx_data) {
+ struct pmu *pmu = child_event->pmu;
+
+ task_ctx_data = kzalloc(pmu->task_ctx_size, GFP_KERNEL);
+ if (!task_ctx_data) {
+ free_event(child_event);
+ return NULL;
+ }
+ child_ctx->task_ctx_data = task_ctx_data;
+ }
+
/*
* is_orphaned_event() and list_add_tail(&parent_event->child_list)
* must be under the same lock in order to serialize against
@@ -10729,6 +10743,7 @@ const struct perf_event_attr *perf_event_attrs(struct perf_event *event)
if (is_orphaned_event(parent_event) ||
!atomic_long_inc_not_zero(&parent_event->refcount)) {
mutex_unlock(&parent_event->child_mutex);
+ /* task_ctx_data is freed with child_ctx */
free_event(child_event);
return NULL;
}