Re: [PATCH 3/4] x86/intel lbr: down with test_thread_flag(TIF_IA32)
From: Andy Lutomirski
Date: Thu Apr 14 2016 - 15:29:38 EST
On Thu, Apr 14, 2016 at 11:10 AM, Dmitry Safonov <dsafonov@xxxxxxxxxxxxx> wrote:
> Use user_mode64_bit to check process state. For that pass
> interrupt register set from irq handler.
> This should fix opcode decoder misinterpreting ABI for
> tasks that change their code selector.
>
> Signed-off-by: Dmitry Safonov <dsafonov@xxxxxxxxxxxxx>
> ---
> arch/x86/events/intel/core.c | 2 +-
> arch/x86/events/intel/lbr.c | 17 ++++++++++-------
> arch/x86/events/perf_event.h | 2 +-
> 3 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 68fa55b4d42e..df13d1d6dbf6 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -1860,7 +1860,7 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
>
> loops = 0;
> again:
> - intel_pmu_lbr_read();
> + intel_pmu_lbr_read(regs);
> intel_pmu_ack_status(status);
> if (++loops > 100) {
> static bool warned = false;
> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index 6c3b7c1780c9..f1a1dbc77dea 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -136,7 +136,9 @@ enum {
> X86_BR_IRQ |\
> X86_BR_INT)
>
> -static void intel_pmu_lbr_filter(struct cpu_hw_events *cpuc);
> +
> +static void
> +intel_pmu_lbr_filter(struct pt_regs *regs, struct cpu_hw_events *cpuc);
>
> /*
> * We only support LBR implementations that have FREEZE_LBRS_ON_PMI
> @@ -500,7 +502,7 @@ static void intel_pmu_lbr_read_64(struct cpu_hw_events *cpuc)
> cpuc->lbr_stack.nr = out;
> }
>
> -void intel_pmu_lbr_read(void)
> +void intel_pmu_lbr_read(struct pt_regs *regs)
> {
> struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>
> @@ -512,7 +514,7 @@ void intel_pmu_lbr_read(void)
> else
> intel_pmu_lbr_read_64(cpuc);
>
> - intel_pmu_lbr_filter(cpuc);
> + intel_pmu_lbr_filter(regs, cpuc);
> }
>
> /*
> @@ -658,7 +660,8 @@ int intel_pmu_setup_lbr_filter(struct perf_event *event)
> * decoded (e.g., text page not present), then X86_BR_NONE is
> * returned.
> */
> -static int branch_type(unsigned long from, unsigned long to, int abort)
> +static int branch_type(unsigned long from, unsigned long to, int abort,
> + struct pt_regs *regs)
> {
> struct insn insn;
> void *addr;
> @@ -724,7 +727,7 @@ static int branch_type(unsigned long from, unsigned long to, int abort)
> * on 64-bit systems running 32-bit apps
> */
> #ifdef CONFIG_X86_64
> - is64 = kernel_ip((unsigned long)addr) || !test_thread_flag(TIF_IA32);
> + is64 = kernel_ip((unsigned long)addr) || user_64bit_mode(regs);
Peterz, looking at this some more, would it make sense to pass
user_regs and interrupt_regs (or whatever we'd call it) all the way
through to here?
--Andy